chore(flags): More refactorings to prepare for flag dependencies
[!IMPORTANT] 👉 Stay up-to-date with PostHog coding conventions for a smoother review.
Problem
Refactored the code to make it easier to understand and change. This is in preparation for adding flags that depend on other flags.
Related to #29016
Changes
Most of this PR is a set of extract method refactorings. I'll point them out to save time in review.
I also moved all the flag_matching.rs tests into test_flag_matching.rs.
The big change is in graph_utils.rs and cohort_operations.rs where I refactored the graph to be a graph of objects (aka cohorts) rather than Ids. The main reason to do this is two-fold.
- It encapsulates the building and traversal of the graph so the caller doesn't have to understand it. All you have to do is build the graph and then call
for_each_dependencies_firstand add a callback to do what you want to each node. - In the old code, we built the graph on IDs and then sorted the IDs. Then while traversing the IDs, we needed to iterate the list of cohorts to find the cohort with the ID every iteration. In most cases, this is fine. But a larger chain of cohorts could be a problem. But since we already know which cohort should be there, might as well build a graph of the cohorts.
There should be no observable behavioral changes with this PR.
Did you write or update any docs for this change?
- [ ] I've added or updated the docs
- [ ] I've reached out for help from the docs team
- [x] No docs needed for this change
How did you test this code?
Unit tests.
Ugh, test_flag_matching.rs is so huge. I wanted to refactor it to use sub mods to group tests so they were a bit more organized (like I did with test_graph_utils.rs, but I really think we may want separate files to group these tests. For now I'll hold off.