-
-
Notifications
You must be signed in to change notification settings - Fork 223
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
!!! BUGFIX: Actually execute moveSubtreeTags tests and fix moveSubtreeTags #5025
Conversation
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.
Now I understood the issue. Thanks for fixing this, looks great!
As for the failing test: I think the code is right, could you please check the test case @bwaidelich ? |
Okay, this is a bigger issue as the moveSubtreeTags CTE needs to be rewritten. |
and make tests more reliable by sorting tags
BUGFIX: Fix Subtree tagging CTE implementation
Applied a few final fixes to the moveSubtreeTags CTE and tadaaa: Tests that make assertions and are green |
Neos.ContentGraph.DoctrineDbalAdapter/src/Domain/Projection/Feature/SubtreeTagging.php
Show resolved
Hide resolved
Neos.ContentGraph.DoctrineDbalAdapter/src/Domain/Projection/Feature/SubtreeTagging.php
Outdated
Show resolved
Hide resolved
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.
Thanks a lot, +1 by 👁️
Thanks ;) as by reading would smoke my brain, i was trying to test this change against the NeosUi for some visual reference but i cannot come up with a test scenario that fails currently :O ... so this must be an absolute edgecase? Feel free to merge this with bastian's approval though ^^ |
Does this require a cr:projectionReplay? Or was this not a bug in beta 9? |
there are cases where the graph projection needs to be replayed, yes |
Fixup #4993
This now actually asserts stuff when testing moveSubtreeTags, thanks to @mhsdesign for detecting this.
Also, the acutal moveSubtreeTags method is fixed to properly apply tags to the moved node and its descendants
Upgrade instructions
This raises the supported database platforms. Mariadb 10.4 for example will no longer be supported.See #4337
Additionally there are cases where it would make sense to replay the projection to apply this fix:
Review instructions
none
Checklist
FEATURE|TASK|BUGFIX
!!!
and have upgrade-instructions