-
Notifications
You must be signed in to change notification settings - Fork 458
feat: Add version_of and matches to rules and conditions
#4988
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
Docker builds report
|
Uffizzi Ephemeral Environment
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4988 +/- ##
==========================================
+ Coverage 97.43% 97.45% +0.01%
==========================================
Files 1214 1216 +2
Lines 42253 42544 +291
==========================================
+ Hits 41169 41460 +291
Misses 1084 1084 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| pyotp = "^2.9.0" | ||
| flagsmith-task-processor = { git = "https://github.com/Flagsmith/flagsmith-task-processor", tag = "v1.1.1" } | ||
| flagsmith-common = { git = "https://github.com/Flagsmith/flagsmith-common", tag = "v1.4.2" } | ||
| flagsmith-common = { git = "https://github.com/Flagsmith/flagsmith-common", branch = "feat/add_version_of_to_rules_and_conditions" } |
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.
Let's remember to update these once we've got builds in those packages.
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.
Good idea.
af6df13 to
6e5894e
Compare
Co-authored-by: Zach Aysan <[email protected]>
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.
Since I'm on my last day I'd like to add some comments to this PR about where we are at.
Overall this work should be considered to be feature complete. Change requests for segments can be created, updated, approved, and committed in order to update the underlying segment. The attribute version_of was added to rules and conditions to allow for the frontend to assign diffs in conjunction with what a likely diff should be.
In order to properly merge this code the following updates need to happen:
flagsmith-commoninpyproject.tomlmust be updated to a new tag that include previously merged work, which includes Flagsmith/flagsmith-common#11.- This PR https://github.com/Flagsmith/flagsmith-workflows/pull/63 needs updating as well, both for the same
flagsmith-commonlibrary as well as for the image in thedocker-composefile which must be set toghcr.io/flagsmith/flagsmith-api-test:main. I'm not sure how to solve the circular dependency because we also must... - Update the
workflows-logicinpyproject.tomlin this repo once step 2 can be merged into the workflows repository and tagged as a release.
When in doubt about how to run some chunk of functionality, look into the tests in the workflows PR here: https://github.com/Flagsmith/flagsmith-workflows/pull/63 including tests that pre-exist that PR.
|
Moving the effort to #5768. |
Changes
When creating a new change request for segments we need a way of tracing which provided rules and conditions match the candidate change request so that the frontend can more visually associate which rules and conditions match between the change request and the targeted segment (the one that will be updated).
To do so I've reused the concept of
version_ofwhich was originally used for segments and I associate a rule to another rule (or sub-rule to sub-rule) and in that process also match up conditions at the same layer. If at least one condition matches, then the sub-rule and rule match and future matching for the subsequent sub-rule is guided by holding onto the match between the parent rule and its sister rule. This way matches are only ever tree like in nature.Remaining work
Since the matching code is ready in this PR another PR will need to be opened to tie in these changes into the relevant code. Probably both the Workflows and Commons will need to be updated.
How did you test this code?
Lots of new tests. Some are simple, like checking that
version_ofgets correctly set, which is necessary due to an audit log bug we had whenversion_ofwas setup for the segments work.