Skip to content

V15: Add MNTP serverside validation#18526

Merged
AndyButland merged 10 commits intov15/devfrom
v15/feature/mntp-serverside-validation
Mar 4, 2025
Merged

V15: Add MNTP serverside validation#18526
AndyButland merged 10 commits intov15/devfrom
v15/feature/mntp-serverside-validation

Conversation

@nikolajlauridsen
Copy link
Contributor

Adds serverside validation for the multinode tree picker.

Adds amount and object type validation.

Dynamic root validation is skipped for now, since the validator has no concept of where it is in the tree, this means that the dynamic root query cannot be resolved.

For testing see #18429 but tests should cover it.

@AndyButland AndyButland self-requested a review March 3, 2025 12:39
# Conflicts:
#	src/Umbraco.Core/EmbeddedResources/Lang/da.xml
#	src/Umbraco.Core/EmbeddedResources/Lang/en.xml
#	src/Umbraco.Core/EmbeddedResources/Lang/en_us.xml
Copy link
Contributor

@AndyButland AndyButland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. I made a few updates but only minor things to resolve a few code warnings.

I did some manual testing on this via manipulating requests to the API, and that verified the functionality, on top of the tests you already had.

I found one issue though - but might be one to look at in a separate PR as I don't suspect it's particularly related to the MNTP, but could be anything that takes an array as a value.

If you post an empty array for the value, e.g.:

{
   "template":null,
   "values":[
      {
         "editorAlias":"Umbraco.MultiNodeTreePicker",
         "alias":"pickedContent",
         "culture":null,
         "segment":null,
         "value":[
         ]
      }
   ],
   "variants":[
      ...
   ],
   "cultures":[     
   ]
}

When you inspect the ValidateUpdateDocumentRequestModel requestModel parameter passed in ValidateUpdateDocumentController, you'll see the value is null not an empty array. And because of that, none of the validation runs. This means if you set a minimum value, but provide no items, it won't get flagged as invalid.

I'll approve - but up to you if you want to look at that as part of this PR, or merge this and tackle it separately.

Might be related to: dotnet/aspnetcore#37630

# Conflicts:
#	src/Umbraco.Core/EmbeddedResources/Lang/en.xml
#	src/Umbraco.Core/EmbeddedResources/Lang/en_us.xml
#	src/Umbraco.Infrastructure/PropertyEditors/MediaPicker3PropertyEditor.cs
@nikolajlauridsen
Copy link
Contributor Author

Good catch, thanks Andy, I've fixed it and amended the tests, turns out it was also a mistake in the TypedJsonValidatorRunner 😄

And thanks for the fixes 👍

@AndyButland AndyButland merged commit a99c581 into v15/dev Mar 4, 2025
20 of 21 checks passed
@AndyButland AndyButland deleted the v15/feature/mntp-serverside-validation branch March 4, 2025 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants