3
\$\begingroup\$

I have a dropdown and when that dropdown changes I want to display/render another dropdown. Can someone help me improve it?

So, a person requests for getting referred in a company for a job. When that person selects a company name in the "Referral Form" I want to show only the cover letters for specific to that company.

My JSON Action:

public JsonResult ListOfCoverLetterByCompanyId(int companyId)
{
    var coverletters = _context.CoverLetters
        .Where(c => c.CompanyId == companyId)
        .ToList();

    var dropdown = new List<SelectListItem>();
    foreach (var cl in coverletters)
    {
        dropdown.Add(new SelectListItem { Text = cl.CoverLetterName, Value = cl.CoverLetterId.ToString() });
    }
    return Json(dropdown, JsonRequestBehavior.AllowGet);
}

My View File:

@model Bridge.ViewModels.ReferralViewModel

@using (Html.BeginForm())
{
    @Html.AntiForgeryToken()
        <h4>Job Request</h4>
        <hr />
        @Html.ValidationSummary(true, "", new { @class = "text-danger" })
        <div class="form-group">
            @Html.LabelFor(model => model.CompanyId, htmlAttributes: new { @class = "control-label col-md-2" })
            <div class="col-md-10">
                @Html.DropDownListFor(m => m.CompanyId, Model.Companies, new { @class = "form-control js-change", onchange = "companyChanged()" })
                @Html.ValidationMessageFor(model => model.CompanyId, "", new { @class = "text-danger" })
            </div>
        </div>
        <select id="ajaxdrop" name="ajaxdrop"></select>
        <div class="form-group">
            <div class="col-md-offset-2 col-md-10">
                <input type="submit" value="Create" class="btn btn-default" />
            </div>
        </div>
    </div>
}

@section scripts{
    <script>
        function companyChanged() {
            var companyId = $(".js-change").val();
            $.ajax({
                url: "/Referral/ListOfCoverLetterByCompanyId",
                data: {companyId : companyId},
                contentType: "application/json; charset-utf-8",
                success: function (datas) {
                    debugger;
                    $.each(datas, function (i,data) {
                        debugger;
                        $("#ajaxdrop").append('<option value="' + data.Value + '">' + data.Text + '</option>');
                    });
                    alert("sucsess");
                },
                error: function () {
                    alert("something wrong");
                }
            });
        }
    </script>
}

Model:

public class ReferralViewModel
{
    public int ReferralViewModelId { get; set; }
    [Display(Name = "Resume")]
    public int ResumeId { get; set; }
    public IEnumerable<Resume> Resumes { get; set; }
    public int CoverLetterId { get; set; }
}
\$\endgroup\$
1
\$\begingroup\$

First to address the controller method your calling in the ajax function.

Your currently returning a serialized collection of SelectListItem which means your unnecessarily passing all the values of the other properties of SelectListItem (Selected, Group and Disabled) back to the view when you don't need or use them. You should only pass back the values you need, by either using a view model, or by creating anonymous objects. Note also that you do not need .ToList() in your query. Your code can simply be

public JsonResult ListOfCoverLetterByCompanyId(int companyId)
{
    var coverletters = _context.CoverLetters.Where(x => x.CompanyId == companyId).Select(x => new
    {
        Value = x.CoverLetterId,
        Text = x.CoverLetterName
    });
    return Json(coverletters, JsonRequestBehavior.AllowGet);
}

Your method is also a [HttpGet] meaning that users can navigate to it via the address bar, which is probably not what you want. You should mark the method [HttpPost] (in which case you do not need the JsonRequestBehavior.AllowGet argument). Alternatively you can check if the request is an ajax request. A simple way to do that is to create a FilterAttribute. A typical example is shown in MVC Set accessibility level on a method called from ajax.

The main issue with your code however, is that your not binding the 2nd dropdownlist to your model. Not only does your name attribute of the <select> not match the property you want to bind it to (CoverLetterId), but if the user submits the form and you need to return the view because ModelState is invalid, then the data the user entered in the 2nd dropdownlist is lost (in fact the 2nd dropdownlist will be empty). You also will not be able to use you code for editing an existing Referral for the same reason. To address this, first modify your model to

public class ReferralViewModel
{
    public int? Id { get; set; }
    [Display(Name = "Company")]
    [Required(ErrorMessage = "Please select a Company")]
    public int? CompanyId { get; set; }
    [Display(Name = "Cover Letter")]
    [Required(ErrorMessage = "Please select a Cover Letter")]
    public int? CoverLetterId { get; set; }

    public IEnumerable<SelectListItem> Companies { get; set; }
    public IEnumerable<SelectListItem> CoverLetters { get; set; }
}

A few things to note.

  1. Name your PK property just Id. This means that if your using the same view model for editing an existing Referral, that you do not need to include a hidden input in the view for the property (assuming your using the default route, and that your method is public ActionResult Edit(int id)). The framework will automatically add a route value in the forms action attribute and it will automatically be bound in the POST method.
  2. Assuming that both ResumeId and CoverLetterId are required in the database, you should make your properties nullable and add a [Required] attribute to protect against under-posting attacks. Refer What does it mean for a property to be [Required] and nullable? for more detail.

Your model is also referring to ResumeId, but the view code is creating the first dropdownlist for CompanyId so I assume that was just a typo.

Now your view code for the dropdownlists will then be (<div> elements omitted for clarity)

@Html.LabelFor(m => model.CompanyId, new { @class = "control-label col-md-2" })
@Html.DropDownListFor(m => m.CompanyId, Model.Companies, "Please select", new { @class = "form-control" })
@Html.ValidationMessageFor(m => m.CompanyId, "", new { @class = "text-danger" })

@Html.LabelFor(m => model.CoverLetterId, new { @class = "control-label col-md-2" })
@Html.DropDownListFor(m => m.CoverLetterId, Model.CoverLetters, new { @class = "form-control" })
@Html.ValidationMessageFor(m => m.CompanyId, "", new { @class = "text-danger" })

Note that I have removed the additional class name, and the onchange attribute which I will discuss later. Note also the 3rd parameter in the 1st DropDownListFor() method, which adds a <option value="">Please select</option> which will be selected if a value for CompanyId has not been set, and forces the user to make a selection and trigger the ajax call. It also means that when the user submits the form, a client side validation error will be shown if they have not selected a Company.

Then you need to modify your controller code so that if the value of CompanyId is null, then you set the value of CoverLetters to be an empty SelectList (i.e when the page is first rendered in your Create() method, but if its not null, then populate it based on the value of CompanyId (i.e. in a POST method if you need to return the view, or in an Edit() method. You controller code would typically look like

public ActionResult Create()
{
    ReferralViewModel model = new ReferralViewModel();
    ConfigureViewModel(model);
    return View(model);
}
[HttpPost]
public ActionResult Create(ReferralViewModel model)
{
    if (!ModelState.IsValid)
    {
        ConfigureViewModel(model);
        return View(model);
    }
    .... // Initialize a new instance of the data model, set its properties based on the view model, save and redirect
}
private void ConfigureViewModel(ReferralViewModel model)
{
    // Populate companies always
    model.Companies = _context.Companies.Select(x => new SelectListItem
    {
        Text = x.CompanyName,
        Value = x.CompanyId.ToString()
    });
    // Populate cover letters only if a company has been selected
    // i.e. if your editing an existing Referral, or if you return the view in the POST method
    if (model.CompanyId.HasValue)
    {
        model.CoverLetters = _context.CoverLetters.Where(x => x.CompanyId == model.CompanyId).Select(x => new SelectListItem
        {
            Value = x.CoverLetterId.ToString(),
            Text = x.CoverLetterName
        });
    }
    else
    {
        model.CoverLetters  = new SelectList(Enumerable.Empty<SelectListItem>());
    }
}

Finally, your scripts.

It is better practice to use Unobtrusive Javascript rather than polluting your htlm markup with behavior, so rather that using an onchange attribute, use $('#CompanyId').change(function() { ...

You should also cache elements that you repeatedly refer to by assigning them to a javascript variable so that your functions are not searching through the DOM again. You script would look like

var coverLetterSelect = $('#CoverLetterId'); // cache it
$('#CompanyId').change(function() {
    var companyId = $(this).val(); // use $(this)
    coverLetterSelect.empty(); // clear existing options
    if (!companyId) {
        return; // there is no point making an ajax call so just exit
    }
    $.ajax({
        type: 'POST', // or 'GET' depending on how you have modified the method above
        url: "@Url.Action("ListOfCoverLetterByCompanyId", "Referral")', // use @Url.Action() to generate urls
        data: {companyId : companyId },
        success: function (response) {
            coverLetterSelect.append($('<option></option>').val('').text('Please select')); // add a default null option
            $.each(response, function (i, item) {
                coverLetterSelect.append($('<option></option>').val(item.Value).text(item.Text));
            });
        },
        ....
    });
})

Note that you need to remove the contentType option.

\$\endgroup\$
  • \$\begingroup\$ I will make all the necessary changes and will accept/update/askforclarification accordingly. Thank you. \$\endgroup\$ – Unbreakable Aug 28 '17 at 3:10
  • \$\begingroup\$ Do, I need to remove Ienummerable<SelectListItem> from my ViewModel properties too? Or that anonymous object suggestion applies only to Ajax controller function? \$\endgroup\$ – Unbreakable Aug 29 '17 at 3:27
  • \$\begingroup\$ No, that only applies to controller method called by the ajax function \$\endgroup\$ – Stephen Muecke Aug 29 '17 at 3:28
  • \$\begingroup\$ Doubt 2: When shall I put .ToList(), I mean how do I know at what place in I need to put .ToList in LINQ Query? Currently, I put it everywhere. How to determine it? \$\endgroup\$ – Unbreakable Aug 29 '17 at 3:30
  • \$\begingroup\$ Can you kindly illustrate with one example for both scenario. \$\endgroup\$ – Unbreakable Aug 29 '17 at 3:32

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.