0

i am creating a form which can be accessed based on condition in MVC. I have first view with dropdownlist and submit button, i want when the submit button is clicked, the value in dropdownlist is passed and compared with condition set for that value, and if the condition is not ok, it shows alert rather than processing to the form.

Here is my code:

public ActionResult ChooseType()
{
    var x = DataAccess.GetEmployee(@User.Identity.Name);

    var lists = new SelectList(RuleAccess.GetAllRule(), "ID", "TypeDetail");
    ViewBag.CategoryId = lists;

    /*rule*/
    ViewBag.comp1 = Logic.AnnualToogle(@User.Identity.Name);

    if (x.EmpSex == "F" && x.EmpMaritalSt == "NIKAH")
    { ViewBag.comp2 = 1; }
    else ViewBag.comp2 = 0;

    return View();
}


[HttpGet]
public ActionResult Create(int lv_type)
{

    var type = RuleAccess.GetTypeByID(lv_type);
    ViewBag.type = type;
    var model = new LeaveApplicationViewModels();
    model.X = DataAccess.GetEmployee(@User.Identity.Name);
    model.C = DataAccess.GetLeaveApp(@User.Identity.Name);

    /*disable*/
    ViewBag.dis = DataAccess.GetDisabledDate(@User.Identity.Name); 

    /*max*/
    var max= RuleAccess.GetMaxByID(lv_type);
    ViewBag.c = max;
    if (lv_type == 1)
    {
        var used = RuleAccess.CountUsedAnnual(@User.Identity.Name);
        var rem = max - used;
        ViewBag.a = used;
        ViewBag.b = rem;
    }
    else 
    {
        ViewBag.b = max;
    }
    return View(model);
}

I used the Viewbag.comp 1 & 2 in my view:

<script type="text/javascript">

var x = @ViewBag.comp1;
var y = @ViewBag.comp2;

function validatecreate()
{
   var value= document.getElementById("lv_type").value;

    if (value==1)
    {
        if(x==1)
            document.getElementById('validatecreate').submit();
        else { alert('Action cant be done. You either have another annual leave application in pending status or you have reach the limit of annual leave'); }
    }
    else if(value==2)
    {
        if(y==1)
            document.getElementById('validatecreate').submit();
        else { alert('Action cant be done. You either are Male or Not Married Yet'); }
    }
    else if(value==3)
    {
        document.getElementById('validatecreate').submit();

    }

    else { 
        document.getElementById('validatecreate').submit();
        //alert('Http Not Found'); 
    }
}

@Html.DropDownList(
    "lv_type", (SelectList) ViewBag.CategoryId, 
    "--Select One--", 
    new{ //anonymous type
        @class = "form-control input-sm"
    }
) 

I feel like im doing it wrong especially because if someone manually put the url with ?lv_type=2, they not validate and can go to the form directly. But i need the value of lv_type bcs i use that in my view. Please Helpp :(

ADreNaLiNe-DJ
  • 4,787
  • 3
  • 26
  • 35
Fimblutterr
  • 89
  • 2
  • 10
  • You taking the wrong approach. Client side validation should only be considered a bonus. Validation must always be done on the server (no exceptions). And none of this view code is necessary anyway - in the GET method that generated the view, you should be generating your `IEnumerable CategoryId` for the dropdownlist with only those options which can be selected by the user based on your conditions - why allow a user to select something they should not be able to and then show them an annoying message. But still include server side validation to protect against malicious users. –  Jun 03 '16 at 12:00

1 Answers1

1

Validation must always be done on the server, and client side validation should only be considered a nice bonus that minimizes the need for to a call to the server. And presenting options in a dropdownlist to a user and then telling them thay can't select that option is an awful user experience. Instead, you should be presenting only those options which are applicable to the user (and delete all the scripts you have shown).

Create an additional method in your RuleAccess class, say GetEmployeeRules(Employee employee) which returns only the rules that are applicable to that employee, for example

public static List<Rule> GetEmployeeRules(Employee employee)
{
    // Get the list of all rules
    if (employee.EmpSex == "F" && employee.EmpMaritalSt == "NIKAH")
    {
        // Remove the appropriate Rule from the list
    }
    .....
    // Return the filtered list
}

In addition, you should be using a view model in the view

public class LeaveTypeVM
{
    [Required(ErrorMessage = "Please select a leave type")]
    public int SelectedLeaveType { get; set; }
    public IEnumerable<SelectListItem> LeaveTypeList { get; set; }
}

Then in the ChooseType() method

public ActionResult ChooseType()
{
    var employee = DataAccess.GetEmployee(@User.Identity.Name);
    var rules = RuleAccess.GetEmployeeRules(employee);
    var model = new LeaveTypeVM()
    {
        LeaveTypeList = new SelectList(rules, "ID", "TypeDetail")
    };
    return View(model);
}

and in the view

@model LeaveTypeVM
@using (Html.BeginForm())
{
    @Html.DropDownListFor(m => m.SelectedLeaveType, Model.LeaveTypeList, "--Select One--", new { @class = "form-control input-sm" }
    @Html.ValidationMessageFor(m => m.SelectedLeaveType)
    <input type="submit" value="Submit" />
}

and submit to a POST method which allows you to easily return the view if its invalid, or to redirect to the Create method.

[HttpPost]
public ActionResult ChooseType(LeaveTypeVM model)
{
    if (!ModelState.IsValid)
    {
        model.LeaveTypeList = .... // as per GET method
    }
    return RedirectToAction("Create", new { leaveType = model.SelectedLeaveType });

and in the Create() method

public ActionResult Create(int leaveType)
{
    var employee = DataAccess.GetEmployee(@User.Identity.Name);
    var rule = RuleAccess.GetEmployeeRules(employee).Where(x => x.ID == leaveType).FirstOrDefault();
    if (rule == null)
    {
        // Throw exception or redirect to an error page
    }
    var model = new LeaveApplicationViewModels();
    ....
    return View(model);
}

Note your LeaveApplicationViewModels should contain additional properties so that you can avoid all those ViewBag properties and generate a strongly typed view.

  • Thankyou i think i will use your answers. But im still dont know what to put in "// Remove the appropriate Rule from the list" part. Can u help me with that? – Fimblutterr Jun 06 '16 at 01:56
  • I don't know what your models are, or what properties they contain so its impossible to give you the correct solution. It might be as simple as `List rules = // get all rules;` the inside the `if` block use a `rules = rules.Where(x => x.ID != 1);` to remove it (see also [this answer](http://stackoverflow.com/questions/853526/using-linq-to-remove-elements-from-a-listt)). –  Jun 06 '16 at 02:04
  • If your having problems, ask a new question with more details (show the existing `GetAllRule()` method, the models that your returning and the code you have tried in the new method with an explanation of what you want to do. –  Jun 06 '16 at 02:04
  • Ok thankss, one last question is in if !model.isvalid, what should i write to show error message? bcs its too simple to ask in new question – Fimblutterr Jun 06 '16 at 02:14
  • You don't need to do anything. I have just edited the view model to add a `[Required]` attribute for the property your binding to. If the user selected the first `null` option (or a malicious user tried to post some invalid data), then `ModelState` will be invalid and when you return the view, the "Please select a leave type" error message will be displayed (because of the `@Html.ValidationMessageFor()` line of code. The only thing you need to do is make sure you repopulate the `SelectList` before you return the view (just as you did in the GET method). –  Jun 06 '16 at 02:19
  • Note that adding the `[Required]` attribute is not essential, but if you don't, then you get a 'generic' - _The field SelectedLeaveType is required_ message so adding the attribute just makes it a more friendly message –  Jun 06 '16 at 02:20
  • yes but you write model.LeaveTypeList= . . .. what to write in that ...? thankyouu for your help :( – Fimblutterr Jun 06 '16 at 02:25
  • Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/113875/discussion-between-stephen-muecke-and-fimblutterr). –  Jun 06 '16 at 02:28