ref(grouping): Remove hierarchical code from `_save_aggregate_new`
Since the new group assignment logic in event_manager won't apply to anyone on the mobile grouping config, we have the freedom to remove the hierarchical code required by that config. This should make further refactoring easier, since it simplifies the code quite a bit.
out of curiousity, why did no tests have to change?
This PR is part of a series of refactorings in which the following has already happened:
- A flag was created
- A new function with identical logic to
_save_aggregate(_save_aggregate_new) was created. (The same was done forfind_existing_grouphash, which is called by_save_aggregate.) - A wrapper for
_save_aggregateand_save_aggregate_newwas added.- If the flag is off or if the project uses (or has used) the mobile grouping config (which needs the hierarchical code), we run
_save_aggregate(which calls the existingfind_existing_grouphash) - in other words, all of the current logic is retained. - If the flag is on and there's no mobile config (and therefore no need for the hierarchical code), we run
_save_aggregate_new, which then callsfind_existing_grouphash_new.
- If the flag is off or if the project uses (or has used) the mobile grouping config (which needs the hierarchical code), we run
Now there's this PR which pulls the hierarchical code out of _save_aggregate_new and find_existing_grouphash_new.
The reason this doesn't actually change anything is that - because of the mobile config check in the wrapper - any event which would care about the now-missing hierarchical code (i.e., an event processed through the mobile config) runs through the unchanged codepath. Conversely, the only events which run through this new codepath are ones for whom the hierarchical code is irrelevant.
(FWIW, the plan is not to have it be like this forever. Once the new logic is fully rolled out, we're then going to start moving people off of the mobile config. Once they're all off, we can then get rid of the current versions of _save_aggregate and find_existing_grouphash. Also, there are more changes to come in _save_aggregate_new. Getting rid of the now-unnecessary hierarchical code is just cleanup to make future changes easier to reason about.)
Going to be pretty nice to remove all of this!
nice, so much easier to read
I so agree with both of you! There are a lot of steps between here and there, but I'm excited for when it finally happens.
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Comparison is base (
6cfc89e) 81.39% compared to head (3bb22e9) 81.40%. Report is 3 commits behind head on master.
Additional details and impacted files
@@ Coverage Diff @@
## master #64858 +/- ##
=======================================
Coverage 81.39% 81.40%
=======================================
Files 5251 5251
Lines 232182 232182
Branches 45592 45592
=======================================
+ Hits 188994 188998 +4
+ Misses 37301 37298 -3
+ Partials 5887 5886 -1