0

I have a simple ASP.NET MVC application, where our users have a table which has some options to edit some settings.

This table is build dynamically using jQuery since this is some legacy code. The code to make the table is:

function setInvoiceTable(result) {
        result.OverduesPaged.forEach(function (item, index) {

            var externalName = 'OverduesPaged[' + index + '].ExternalInvoiceGuid';
            var debtorName = 'OverduesPaged[' + index + '].Debtor';
            var visibleName = 'OverduesPaged[' + index + '].IsSelected';

            var externalInvoiceGuidField = '<input type=\'hidden\' value=\'' + item.ExternalInvoiceGuid + '\' name=\'' + externalName+ '\' />';
            var debtorField = '<input type=\'hidden\' value=\'' + item.Debtor + '\' name=\'' + debtorName + '\' />';
            var invisibleField = '<input type=\'hidden\' value=\'false\' name=\'' + visibleName + '\' id=\'' + visibleName + '\' /></label>';
            var visibleField = '<label><input type=\'checkbox\' value=\'true\' name=\'' + visibleName + '\' id=\'' + visibleName + '\' />  Start sag';

            var insertStr = "<tr>" +
                "<td>" + externalInvoiceGuidField + debtorField + visibleField + invisibleField + "</td>" +
                "<td>" + item.InvoiceId + "</td>" +
                "<td>" + item.Debtor + "</td>" +
                "<td>" + item.DateString +"</td>" +
                "<td>" + item.DaysSinceDueString +"</td>" +
                "<td>" + item.GrossAmountString + "</td>" +
                "<td>" + item.RemainderAmountString + "</td>" +
                "</tr>";
            $('#accounting_invoices_table').append(insertStr);
        });
    }

When I POST, this works perfectly for us. We have an endpoint in our controller which has this signature:

[System.Web.Mvc.HttpPost]
public ActionResult StartDebtCollectionCases(UpcomingDashboardViewModel vm)
{
    // code
}

The collection I'm building is the OverduesPaged which is a part of our view model:

public class UpcomingDashboardViewModel
{
    public List<OverdueUpcomingInvoiceViewModel> OverduesPaged { get; set; }
    // more stuff not relevant to question
}

This is my challenge:

I've added a css class to the <tr> row like this:

            // new code added
            var removeClass = 'ok';
            if (item.IsAlreadyCase) {
                visibleField = '<label type="text">Allerede startet</label>';
                removeClass = 'remove';
            } else if (item.IsBlocked) {
                visibleField = '<label type="text">Sat i bero</label>';
                removeClass = 'remove';
            }
            else if (item.Currency != "DKK") {
                visibleField = '<label type="text">Faktura skal være dansk valuta</label>';
                removeClass = 'remove';
            }

            var insertStr = "<tr class='" + removeClass + "'>" +
                "<td>" + externalInvoiceGuidField + debtorField + visibleField + invisibleField + "</td>" +
            // rest of code from above

Now I end up with a table exactly like before, but with a class which is either OK or REMOVE.

However, when I now do the POST, the whole collection, OverduesPaged, is null/empty:

enter image description here

I tried to add the class to the <td> instead, but then the collection was null/empty.

Any ideas what is wrong here?

Lars Holdgaard
  • 9,496
  • 26
  • 102
  • 182
  • 1
    Note relatd, but that is a fragile/error prone way to generate your html. Consider a template that you clone and update using jQuery (similar to [this answer](https://stackoverflow.com/questions/24026374/adding-another-pet-to-a-model-form/24027152#24027152)) –  Jul 27 '18 at 11:43
  • @StephenMuecke It's completely bullocks to create it like this and it's being re-written as we speak :) however, I just tried to implement this hack in a quick/dirty way and can't understand why this doesn't work – Lars Holdgaard Jul 27 '18 at 11:45
  • I assume because if you execute any of those `if` statements, then `visibleField` is no longer an ``, its a ` –  Jul 27 '18 at 11:51
  • Note also your `id` attributes are also not valid (the contain `[`, `]` and `.` characters (The `HtmlHelper` method replace those characters with a `_`). And your missing the closing `` in the initial `var visibleField = ...` –  Jul 27 '18 at 11:58
  • @StephenMuecke You're right. Adding the input inside the label worked.. So we're golden now :) Now we just need to look forward to the re-write of this crazy-code :-D Want to make an answer? Then I*ll award it! – Lars Holdgaard Jul 27 '18 at 12:07
  • Will do, but may be an hour or so (although I think there might be a bit more to it than I noted above because it does not explain the difference you noted when you added the class to the ``) –  Jul 27 '18 at 12:13

1 Answers1

1

The issue is not related to adding the class name itself, but rather the invalid html that you generating if ant of the code in the if or else if blocks are executed. In those blocks you are creating <label> element with a closing tag, but no input (its not clear if you need the checkbox or not in those cases), so when you combine visibleField + invisibleField you get unbalanced <label> tags (visibleField has opening and closing tags, but invisibleField only has a closing </label> tag.

You code in the if blocks to generate visibleField would need to be similar to what you generate in the original code - i.e. just an opening <label> with the html for the checkbox.

As you have acknowledged in the comments, this is very fragile and error prone code that is difficult to debug. As you are intending to rewrite it, consider returning a partial view in your ajax call (that is strongly bound to the model), or if you need to return Json, then consider a hidden template (outside the <form> tags) that you clone and update in the script, for example

<div id="newrow" style="display:none">
    <tr>
        <td>
            <input class="invoice" type="hidden" name="OverduesPaged[#].ExternalInvoiceGuid value />
            <input class="debtor" type="hidden" name="OverduesPaged[#].Debtor value />
            ....
        </td>
        ....
    </tr>
</div>

and then you script becomes

var table = $('#accounting_invoices_table');
result.OverduesPaged.forEach(function (item, index) {
    var clone = $('#newrow').clone();
    // Update indexer
    clone.html($(clone).html().replace(/\[#\]/g, '[' + index + ']'));
    // Update values
    clone.find('.invoice').val(item.ExternalInvoiceGuid);
    ....
    table.append(clone.html());
});

As a side note, your existing id attributes are invalid because they contain ., [ and ] characters (if you attempted to use these in jQuery selectors, they would fail (for example the . will be interpreted as a class name), so they should be omitted in the html