Fix: #18421 - Added Max Length validation to PropertyTypeBasic Alias

[danielhnelson]

Description

Fixes Umbraco 13.X Back Office exception caused by a user attempting to save a PropertyType with an alias over 255 characters long.
This is fixed by adding a MaxLength attribute to the property in the PropertyTypeBasic class.

Fixes #18421.

[github-actions]

Hi there @danielhnelson, thank you for this contribution! 👍

While we wait for one of the Core Collaborators team to have a look at your work, we wanted to let you know about that we have a checklist for some of the things we will consider during review:

• It's clear what problem this is solving, there's a connected issue or a description of what the changes do and how to test them
• The automated tests all pass (see "Checks" tab on this PR)
• The level of security for this contribution is the same or improved
• The level of performance for this contribution is the same or improved
• Avoids creating breaking changes; note that behavioral changes might also be perceived as breaking
• If this is a new feature, Umbraco HQ provided guidance on the implementation beforehand
• 💡 The contribution looks original and the contributor is presumably allowed to share it

Don't worry if you got something wrong. We like to think of a pull request as the start of a conversation, we're happy to provide guidance on improving your contribution.

If you realize that you might want to make some changes then you can do that by adding new commits to the branch you created for this work and pushing new commits. They should then automatically show up as updates to this pull request.

Thanks, from your friendly Umbraco GitHub bot 🤖 🙂

[bjarnef]

I think it could also add maxlength in view, which should trigger client side validation:
https://github.com/search?q=repo%3Aumbraco%2FUmbraco-CMS+maxlength+language%3ATypeScript&type=code&l=TypeScript

[danielhnelson]

Hi Bjarne, that sounds like the equivalent fix for V14+, which would be great to get opened if the issue is also present there, however this fix is targeting v13, which doesn't use the new UI.

[mikecp]

Hi @danielhnelson ,

Thanks a lot for spotting the issue and providing the fix 👍
Someone from the core collaborators team will have a look and validate it soon.

In the meantime, I see this is your first contribution to the Umbraco CMS. Thanks again, and congratulations 😁
If you could already provide us with your account name on Our.umbraco.com, then we will be able to assign our contributor's badge to you as soon as your PR is merged.

Cheers!

[danielhnelson]

Hi Mike, thank you!

My account name on our.umbraco.com is the same as this one - danielhnelson.

Thanks!

[bjarnef]

Hi Bjarne, that sounds like the equivalent fix for V14+, which would be great to get opened if the issue is also present there, however this fix is targeting v13, which doesn't use the new UI.

Yes, it could however still add it in the property editor component.
I haven't checked, but it should probably be a similar issue if setting alias of a content type, template, relation type etc. to move than 255 characters.

It seems already to handle it here for name field:
https://github.com/umbraco/Umbraco-CMS/blob/v13/contrib/src/Umbraco.Web.UI.Client/src/views/components/editor/umb-editor-header.html#L55

However it may be possible to edit alias afterwards to move than 255 characters.
IIRC the alias at top and in properties are using this component:
https://github.com/umbraco/Umbraco-CMS/blob/v13/contrib/src/Umbraco.Web.UI.Client/src/views/components/umb-locked-field.html#L13-L28

[danielhnelson]

Hi both, just returning to this issue finally, I've looked into the client-side validation further following the comment from @bjarnef.

I think this is a wider question than how we add validation to umb-locked-field.html, but also whether we should modify how we generate Aliases in general, so the generated alias is already truncated before returning.

We could update the umbGenerateAlias.directive.js to truncate the alias before returning it - or go all the way through to String Helpers in the back end that are used to generate 'Safe' Aliases (for example the GetSafeAlias method in EntityController.cs that uses StringExtensions.ToSafeAlias). This would depend on whether all aliases that are generated for the system need to be 255 or if this is unique.

Does anyone have any thoughts on this? Happy to have a go at implementing the client-side validation to prevent the user saving the long alias and not worry about this wider question for now. 🙂

[bjarnef]

I think it would be fine to set maxlength to 255 characters where to locked alias field and let the client side validation kick in and to avoid to server side error (of course one would get this if creating the entities programmally).

And if this it would be better to avoid truncating text. Generally this is not great UX in textbox/textarea, but better warn about it.

[danielhnelson]

I've now added max length validation to the umb-locked-field component and a corresponding validation label. The label is using the same 'Invalid Alias' localized text as there isn't currently a localization key for maximum length. Happy to make further tweaks as appropriate!