Levels within levels: BubbleChoice #41446
Conversation
Hamms
force-pushed
the
levels-within-levels-bubble-choice
branch
from
4d21415
to
69d3bfe
Compare
…e parent_level querying
| includes(:child_level). | ||
| where(kind: ParentLevelsChildLevel::SUBLEVEL). | ||
| order('position ASC'). | ||
| map(&:child_level) |
There was a problem hiding this comment.
this looks like a good alternative to adding a new rails association for each sublevel type. rails associations do have the benefit that you can chain queries and other commands onto them efficiently, such as my_bubble_choice_level.child_levels.where(...), but until/unless we find we need that behavior I'm on board with this solution. optimize later :-)
There was a problem hiding this comment.
a big part of the generalization stuff that I'm working on today is addressing this; my current strategy is to add an ActiveRecord Extension to the existing Association, so we can do stuff like my_bubble_choice_level.child_levels.sublevels.where(...)
There was a problem hiding this comment.
wow, cool! I look forward to seeing what that looks like!
There was a problem hiding this comment.
Here's my first draft: #41492
| parent_levels(level.name). | ||
| map(&:script_levels). | ||
| flatten. | ||
| find {|sl| sl.script_id == script_id} |
There was a problem hiding this comment.
I would suggest removing the warning comment from lines 53-54, since the new implementation looks more efficient.
Updates BubbleChoice levels to use the new many-to-many table for referencing sublevels. In the vein of #35827
Note that we did have to update the
create_rollup_tablesscript as part of this work; thanks to Dave for pointing that out, because it was not caught by any tests. I spent some time trying to find a good place to add some, but came up short. So FYI that rollup tables remains an area of the codebase with poor test coverage.PR Checklist: