leekelleher / umbraco-contentment

Contentment for Umbraco - a state of happiness and satisfaction
https://marketplace.umbraco.com/package/umbraco.community.contentment
Mozilla Public License 2.0
155 stars 70 forks source link

Data List: Item Picker - Reselecting an item after validation error breaks save and publish #242

Closed Ambertvu closed 11 months ago

Ambertvu commented 2 years ago

Which Contentment version are you using?

4.1.1

Which Umbraco version are you using? For example: 8.14.1 - don't just write v8

10.0.0 and 10.0.1 clean

Bug summary

Save and publish breaks when Item picker entry is invalid.

Steps to reproduce

Create a contentment datatype with a datasource (don't think this matters) and a Item picker (this matters) (tried it with default values).

Create a documenttype with the datatype set as required. Create a content item with the datatype.

Go to the content, select the documentType, select a value in your created dataType, save and publish. It saves correctly

Deselect the value, click save and publish. Error appears that the value is required as expected. In console a PostSave 400 error appears. When re-selecting a value and clicking save and publish, the button no longer works.

First clicking save and then save and publish(or reloading the page) fixes it again.

Expected result / actual result

Clicking save and publish should still work after reselecting a value after a validation error.

Do you have Umbraco ModelsBuilder enabled?

What browsers are you seeing the problem on?

Firefox, Chrome

leekelleher commented 2 years ago

@Ambertvu Thanks for raising this, I can reproduce the bug. 👍 Now to figure out why the Item Picker editor is doing that. 😆

leekelleher commented 2 years ago

Well, I'm stumped. 😖 I've deep dived into Umbraco source to try to figure out why the validity isn't being reset after a new value is selected, but I'm no closer to figuring out why it doesn't work... and I couldn't find any documentation on property-editor validation (specifically).

Adding the help-wanted label. I'm laying my hopes on someone out there has experienced this before, somehow. 😬

mennomout commented 1 year ago

@leekelleher I can confirm that this issue also excists for non-contentment data types. I made a new data type with a checkbox list and the same issue occurs when making it required and then pressing save & publish.

leekelleher commented 1 year ago

@mennomout Thank you for confirming this. 👍

Do you know if this bug has been reported with Umbraco-CMS? If so, I'll cross-link to it. Could be this one? https://github.com/umbraco/Umbraco-CMS/issues/13082

mennomout commented 1 year ago

@leekelleher Sorry for the late response. But yes the issue you linked seems to be the one that is connected to this one.

leekelleher commented 1 year ago

I'm closing this issue, as it appears to be an underlying issue with Umbraco CMS, (please see https://github.com/umbraco/Umbraco-CMS/issues/13082). I have tried to see if I can resolve or workaround the issue within the Contentment codebase, but no success.

leekelleher commented 1 year ago

Reopening this issue. Whilst on an unrelated project, I hit a similar issue and finally figured it out.

It's all to do with the initial/default value set by the DataValueEditor.ToEditor. Of which, Data List uses Umbraco's DataValueEditor so would default to an empty string "" whereas ideally it should be an empty array [].

I'll patch up a fix in due course.

leekelleher commented 11 months ago

Closing issue. I'd forgotten that I'd already fixed this in commit e685cb8fc03aebc5eb3495fe1e247904ef5179da. 🤦 It was already released in Contentment v4.4.2.

Ambertvu commented 10 months ago

Hi Lee

I've just tested this in 10.7.0 with Contentment 4.5.0, and it seems the issue still appears.

Tested with User Defined List and Buttons & Item Picker type image