Speed up Lesson#unplugged_lesson?

[Hamms]

Currently, this method is implemented by querying for all ScriptLevels in the Lesson's Script, and manually filtering them down to just the one ScriptLevel we actually care about. We should instead query directly for the ScriptLevel we care about.

Right now, calling Script#summarize_for_unit_edit invokes this method quite a lot. Specifically:

• Script#summarize_for_unit_edit invokes LessonGroup#summarize_for_unit_edit for each LessonGroup in the Script
• LessonGroup#summarize_for_unit_edit invokes Lesson#summarize_for_unit_edit for each Lesson in the LessonGroup
• Lesson#summarize_for_unit_edit invokes Lesson#summarize
• Lesson#summarize invokes ScriptLevel#summarize for each ScriptLevel in the Lesson
• ScriptLevel#summarize invokes ScriptLevel#level_display_text
• ScriptLevel#level_display_text invokes Lesson#unplugged_lesson? for the Lesson associated with the ScriptLevel

What this means is that when we call Script#summarize_for_unit_edit, we are querying for all ScriptLevels in the entire Script not just once, but once for every ScriptLevel in the entire Script.

Also fix Lesson#spelling_bee? in the same way, while I'm in the area.

PR Checklist:

• Tests provide adequate coverage
• Privacy and Security impacts have been assessed
• Code is well-commented
• New features are translatable or updates will not break translations
• Relevant documentation has been added or updated
• User impact is well-understood and desirable
• Pull Request is labeled appropriately
• Follow-up work items (including potential tech debt) are tracked and linked

[Hamms]

My current guesstimate (based on local testing) is that this should approximately halve the time it takes to render the script edit page for migrated scripts.

[davidsbailey]

I'm not 100% sure this change jives with the current strategy of caching script objects and their associated models -- in production, none of these queries will be actually hitting the database, just the script cache.

even worse, there is a chance that this will actually be adding queries in production, since this change could take us from hitting the script cache to not hitting the script cache.

can you confirm that this does not cause an increased query count in production before proceeding?

[Hamms]

Good call, Dave! Looks like your suspicions were correct, and the original change would result in a performance hit. I refactored this a bit so it now uses whichever query strategy is appropriate based on Script.should_cache?

[davidsbailey]

Thanks for checking that, Elijah! This confirms my suspicion that rails is out to trick us 😛 This solution looks good to me.