-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Block level variance: fix values being polluted when changing variance before publish #21121
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
base: main
Are you sure you want to change the base?
Block level variance: fix values being polluted when changing variance before publish #21121
Conversation
…ce retaining block level variance values and vica versa
|
I'm having trouble replicating the original issue for this PR @Migaroez. Please could you add manual testing steps (or review what I've done to see what I've missed) please?
So this seems OK - only one property, and it has the expected culture.
Resaved and published - still looks OK. Going back again, have set property to be variant by language, and get: Again, seems OK. |
|
Sorry bout that Andy, updated the PR with relevant info |
AndyButland
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've not been able to replicate the issue still unfortunately, so will have to rely on you to confirm this does resolve a problem that you can see when running the code from main. I don't see an Examine error and would have expected to see the issue via the process I described in my earlier comment if invalid values remain after changing the variation status of a property.
That said, the fix itself looks good and great job on the comprehensive tests. I tried copying the tests back to main and can see that most of them fail there, so it does seem clear there is some situations where things are going wrong and this fix resolves them.
My only comments then are some minor suggestions for clean-up of test code.
...ts.Integration/Umbraco.Infrastructure/PropertyEditors/BlockGridElementLevelVariationTests.cs
Outdated
Show resolved
Hide resolved
...ts.Integration/Umbraco.Infrastructure/PropertyEditors/BlockGridElementLevelVariationTests.cs
Outdated
Show resolved
Hide resolved
...ts.Integration/Umbraco.Infrastructure/PropertyEditors/BlockGridElementLevelVariationTests.cs
Outdated
Show resolved
Hide resolved
...ts.Integration/Umbraco.Infrastructure/PropertyEditors/BlockGridElementLevelVariationTests.cs
Outdated
Show resolved
Hide resolved
...ion/Umbraco.Infrastructure/PropertyEditors/BlockListElementLevelVariationTests.Publishing.cs
Outdated
Show resolved
Hide resolved
...ion/Umbraco.Infrastructure/PropertyEditors/BlockListElementLevelVariationTests.Publishing.cs
Outdated
Show resolved
Hide resolved
...ion/Umbraco.Infrastructure/PropertyEditors/BlockListElementLevelVariationTests.Publishing.cs
Outdated
Show resolved
Hide resolved
...ion/Umbraco.Infrastructure/PropertyEditors/BlockListElementLevelVariationTests.Publishing.cs
Outdated
Show resolved
Hide resolved
...sts.Integration/Umbraco.Infrastructure/PropertyEditors/RichTextElementLevelVariationTests.cs
Outdated
Show resolved
Hide resolved
...sts.Integration/Umbraco.Infrastructure/PropertyEditors/RichTextElementLevelVariationTests.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Andy Butland <[email protected]>
…es-on-variance-change-before-publish
Summary
These issues do not show up during rendering (delivery api or razor views) but they do show up when examine tries to index the properties. Publishing should throw an examine error.
Changes
BlockValuePropertyValueEditorBase.MergePartialPropertyValueForCultureto remove stale values after mergingTest plan