craft/fields/Matrix::validateEntries use new:X index as fallback if uid is empty to index new entries individually

[Anubarak]

Lets say we have an Entry with a matrix field and want to be able to create such an entry via site request via ajax.
Imagine we add 3 matrix blocks and all have a required title field. Our HTML is kinda like

{{ hiddenInput('fields[matrix][sortOrder][]', 'new:1') }}
{{ hiddenInput('fields[matrix][sortOrder][]', 'new:2') }}
{{ hiddenInput('fields[matrix][sortOrder][]', 'new:3') }}
{% namespace "fields[matrix][blocks][new:1]" %}
    {{ hiddenInput('type', 'text') }}
    <input type="text" name="title">
{% endnamespace %}


{% namespace "fields[matrix][blocks][new:2]" %}
    {{ hiddenInput('type', 'text') }}
    <input type="text" name="title">
{% endnamespace %}

{% namespace "fields[matrix][blocks][new:3]" %}
    {{ hiddenInput('type', 'quoteModule') }}
    <input type="text" name="title">
{% endnamespace %}

leaving all titles blank results in errors for all fields, unfortunately those have no uid yet, so this line https://github.com/craftcms/cms/blob/5.1.7/src/fields/Matrix.php#L1014

if (!$entry->validate()) {
    $element->addModelErrors($entry, "$this->handle[$entry->uid]");
    $allEntriesValidate = false;
}

will create an array like

    "matrix[].title": [
        "Title cannot be blank.",
        "Title cannot be blank.",
        "Title cannot be blank.",
    ]

I hope I don't need to explain why it could be tedious to find the exact matching HTML element / matrix block for each possible error.
I would suggest to optionally use an index as fallback:

// use a counter 
$i = 1;


if ($value instanceof EntryQuery) {
    /** @var Entry[] $entries */
    $entries = $value->getCachedResult() ?? (clone $value)->status(null)->limit(null)->all();

    $allEntriesValidate = true;
    $scenario = $element->getScenario();

    foreach ($entries as $entry) {
        $entry->setOwner($element);

        /** @var Entry $entry */
        if (
            $scenario === Element::SCENARIO_ESSENTIALS ||
            ($entry->enabled && $scenario === Element::SCENARIO_LIVE)
        ) {
            $entry->setScenario($scenario);
        }

        if (!$entry->validate()) {


            $uniqueId = $entry->uid;
            // if empty index by new + counter
            if(empty($uniqueId)){
                $uniqueId = "new:" . ($i++);
            }


            $element->addModelErrors($entry, "$this->handle[$uniqueId]");
            $allEntriesValidate = false;
        }
    }

    if (!$allEntriesValidate) {
        // Just in case the entries weren't already cached
        $value->setCachedResult($entries);
    }
} else {
    $entries = $value->all();
}

This way we have an array like

{
    "matrix[new:1].title": [
        "Title cannot be blank.",
    ],
    "matrix[new:2].title": [
        "Title cannot be blank.",
    ],
    "matrix[new:3].title": [
        "Title cannot be blank.",
    ]
}

And we can return $entry->getErrors() from our controller to easily identify what block is invalid.

[brandonkelly]

Thanks for pointing that out. That’s actually a bug, as for blocks with multiple errors, it would just be impossible to know how they’re grouped (at least without looking at the errors on each nested entry).

I’ve just fixed that for the next release. Going with the format newX instead of new:X, to be consistent with Matrix::serializeValue(). (95205c2)

[brandonkelly]

Craft 5.1.8 is out with that change.

[Anubarak]

Thank you very much for your fast fix.
Unfortunately I need to open this again.

I forgot to increase the new counter for each new block, not only in case there is an error.

As a basic example what I mean: Post values in the following order

matrix[new1][title] = "foo"
matrix[new2][title] = ""
matrix[new3][title] = "bar"
matrix[new4][title] = ""

will produce a result like

matrix[new1].title
matrix[new2].title

instead of

matrix[new2].title
matrix[new4].title

that makes it hard to render errors in JS requests as well without PHP parsing.
Could you please increase your $new variable for each new block instead of only in validation errors?
I am very sorry for the trouble.

[brandonkelly]

I considered that, but I think it could end up causing more confusion, because it could imply that the numbers are somehow correlated with whatever keys you posted the data in, in the first place. Which is not the case. (When setting the validation errors, we have no idea what the original “IDs” were.)

[Anubarak]

I already thought so after I posted yesterday 😓
As always thank you very much - your support is incredibly awesome 👍