Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v8: 7647 Can't save content after clearing property with validation error #7662

Conversation

nathanwoulfe
Copy link
Contributor

Prerequisites

  • I have added steps to test this contribution in the description below

If there's an existing issue for this PR then this fixes #7647

Description

This was due to validation not checking for an empty value, which is a valid state for non-mandatory fields.

Changes here ensure that when we set the validation state on a property form, we check that the property is not mandatory and has no value - if both those flags are true, existing errors are cleared and form controls all reset.

I'm not really across the intricacies of the client-side validation, but from what I can see, the changes fix the problem, and don't impact on existing validation logic. Needs a good sanity check though from someone saner than I.

I've tested by reproducing the scenario from the original bug report. All works as expected - saving with an invalid value displays the validation messages, clearing the value clears the validation.

@poornimanayar
Copy link
Contributor

Thank you @nathanwoulfe ! We shall review the work and get back to you if we need anything more.

Regards,
Poornima

@nathanwoulfe
Copy link
Contributor Author

Useful bits are down around line 142 - rest is mainly whitespace/line-length stuff.

@poornimanayar
Copy link
Contributor

I am sure I have seen this during my v7 training days, so well done on getting this fixed. On it now!

Copy link
Contributor

@poornimanayar poornimanayar left a comment

Choose a reason for hiding this comment

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

@nathanwoulfe , this totally works and happy to approve it! Thanks for your work as always!

@poornimanayar poornimanayar merged commit 9e2a471 into umbraco:v8/contrib Feb 18, 2020
@poornimanayar
Copy link
Contributor

And merged :-) ⭐️ Thanks for the work once again

@nathanwoulfe
Copy link
Contributor Author

No worries at all! Was a tricky one to track down :)

@nul800sebastiaan nul800sebastiaan added release/8.7.0 release/no-notes This is too small to add to the release notes or fixed after a beta/RC labels Sep 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release/no-notes This is too small to add to the release notes or fixed after a beta/RC release/8.7.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can't save content after clearing property with validation error
3 participants