-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[Tree] Allow uuid as path source in materialized path strategy #2897
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2897 +/- ##
=======================================
Coverage 78.66% 78.66%
=======================================
Files 167 167
Lines 8746 8746
=======================================
Hits 6880 6880
Misses 1866 1866 ☔ View full report in Codecov by Sentry. |
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.
Thank you @KeeSystemOrg!
Is there a way to test the expected behavior in order to avoid regressions?
Hi @phansys I just pushed a commit that contains a new fixture and test class. When removing the change in Validator.php the test breaks with the same error reported in the PR description:
|
Hi @phansys I downgraded Uuid to V4 for PHP 7.4 compatibility and added some CS fixes. |
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.
In order to keep the changes as narrow as possible regarding the PR goal and to ease the review process, please remove the unrelated changes.
src/Tree/Document/MongoDB/Repository/AbstractTreeRepository.php
Outdated
Show resolved
Hide resolved
c987cc1
to
feb082c
Compare
cb3ce6e
to
f9be21d
Compare
Triggering the CI after the merge of #2900. |
- Fixes a path validation error when using an Uuid as primary key in an entity and also as TreePathSource in the MaterializedPath strategy - Added a test to confirm the regression if 'uuid' is removed from the allowed types list in Validator.php
Fixes the following error in Symfony 7.* during cache warmup, with an entity that implements Tree with the materialized path strategy and uses Uuidv7 as primary key: