Skip to content

Commit 05a4901

Browse files
zetter-rpfCopilot
andcommitted
Reduce N+1s by caching submitted project count
There's currently N+1 queries on both the lessons and classes controller as they both try to show the number of submitted projects. Originally I tired to solve this by using 'include' to load the correct records, however I don't think this is a very scalable solutions as it means loading every submitted project object memory for classes or lessons (which there may be 1000s). Instead use the pattern of a counter cache. Each time a project transitions to/from submitted, we recalculate the number. This is the first part of two. I've broken up the work so we can deploy this and check that the counts are correct. If we deployed this in one go there would be a chance the counts are inaccurate if a project transition happened during the deploy. ### After deploy Run `rails lessons:backfill_submitted_projects_count` and verify the results by checking a few lessons and comparing it to `lesson#submitted_count` Co-authored-by: Copilot <copilot@github.com>
1 parent d82883f commit 05a4901

9 files changed

Lines changed: 131 additions & 1 deletion

File tree

app/models/lesson.rb

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ class Lesson < ApplicationRecord
88
belongs_to :parent, optional: true, class_name: :Lesson, foreign_key: :copied_from_id, inverse_of: :copies
99
has_many :copies, dependent: :nullify, class_name: :Lesson, foreign_key: :copied_from_id, inverse_of: :parent
1010
has_one :project, dependent: :destroy
11+
has_many :remixes, through: :project
12+
has_many :school_projects, through: :remixes
1113
accepts_nested_attributes_for :project
1214

1315
before_validation :assign_school_from_school_class
@@ -38,6 +40,11 @@ def submitted_count
3840
project.remixes.count { |remix| remix.school_project&.submitted? }
3941
end
4042

43+
def recalculate_submitted_projects_count!
44+
count = school_projects.in_state(:submitted).count
45+
update!(submitted_projects_count: count)
46+
end
47+
4148
private
4249

4350
def assign_school_from_school_class

app/models/school_project.rb

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,10 @@ class SchoolProject < ApplicationRecord
1111
initial_state: :unsubmitted
1212
]
1313

14+
def lesson
15+
project.lesson || project.parent&.lesson
16+
end
17+
1418
def status
1519
state_machine.current_state
1620
end
@@ -23,6 +27,10 @@ def unread_feedback?
2327
feedback.exists?(read_at: nil)
2428
end
2529

30+
def recalculate_lesson_submitted_projects_count!(_transition = nil)
31+
lesson&.recalculate_submitted_projects_count!
32+
end
33+
2634
# Add convenience methods for each state
2735
def unsubmitted?
2836
state_machine.in_state?(:unsubmitted)

app/state_machines/school_project_state_machine.rb

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,4 +14,7 @@ class SchoolProjectStateMachine
1414
transition from: :submitted, to: %i[unsubmitted returned complete]
1515
transition from: :returned, to: %i[submitted complete]
1616
transition from: :complete, to: [:unsubmitted]
17+
18+
after_transition(to: :submitted, &:recalculate_lesson_submitted_projects_count!)
19+
after_transition(from: :submitted, &:recalculate_lesson_submitted_projects_count!)
1720
end
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
# frozen_string_literal: true
2+
3+
class AddSubmittedProjectsCountToLessons < ActiveRecord::Migration[7.2]
4+
def change
5+
add_column :lessons, :submitted_projects_count, :integer, null: false, default: 0
6+
end
7+
end

db/schema.rb

Lines changed: 2 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

lib/tasks/lessons.rake

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
# frozen_string_literal: true
2+
3+
namespace :lessons do
4+
desc 'Backfill cached submitted project counts for lessons'
5+
task backfill_submitted_projects_count: :environment do
6+
Lesson.connection.execute <<~SQL.squish
7+
WITH submitted_counts AS (
8+
SELECT lesson_projects.lesson_id, COUNT(*) AS submitted_projects_count
9+
FROM projects lesson_projects
10+
INNER JOIN projects remixes ON remixes.remixed_from_id = lesson_projects.id
11+
INNER JOIN school_projects ON school_projects.project_id = remixes.id
12+
INNER JOIN school_project_transitions ON school_project_transitions.school_project_id = school_projects.id
13+
WHERE school_project_transitions.most_recent = TRUE
14+
AND school_project_transitions.to_state = 'submitted'
15+
AND lesson_projects.lesson_id IS NOT NULL
16+
GROUP BY lesson_projects.lesson_id
17+
)
18+
UPDATE lessons
19+
SET submitted_projects_count = COALESCE(submitted_counts.submitted_projects_count, 0)
20+
FROM lessons target_lessons
21+
LEFT JOIN submitted_counts ON submitted_counts.lesson_id = target_lessons.id
22+
WHERE lessons.id = target_lessons.id
23+
SQL
24+
end
25+
end

spec/lib/tasks/lessons_spec.rb

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
# frozen_string_literal: true
2+
3+
require 'rails_helper'
4+
require 'rake'
5+
6+
RSpec.describe 'lessons', type: :task do
7+
describe ':backfill_submitted_projects_count' do
8+
let(:task) { Rake::Task['lessons:backfill_submitted_projects_count'] }
9+
let(:school) { create(:school) }
10+
let(:teacher) { create(:teacher, school:) }
11+
let(:student) { create(:student, school:) }
12+
let(:lesson) { create(:lesson, school:, user_id: teacher.id) }
13+
14+
before do
15+
task.reenable
16+
end
17+
18+
it 'sets cached submitted project counts for all lessons' do
19+
submitted_remix = create(:project, school:, remixed_from_id: lesson.project.id, user_id: student.id)
20+
submitted_remix.school_project.transition_status_to!(:submitted, student.id)
21+
22+
returned_remix = create(:project, school:, remixed_from_id: lesson.project.id, user_id: student.id)
23+
returned_remix.school_project.transition_status_to!(:submitted, student.id)
24+
returned_remix.school_project.transition_status_to!(:returned, teacher.id)
25+
26+
other_lesson = create(:lesson, school:, user_id: teacher.id, submitted_projects_count: 7)
27+
28+
lesson.update!(submitted_projects_count: 0)
29+
30+
task.invoke
31+
32+
expect(lesson.reload.submitted_projects_count).to eq(1)
33+
expect(other_lesson.reload.submitted_projects_count).to eq(0)
34+
end
35+
end
36+
end

spec/models/lesson_spec.rb

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,4 +227,32 @@
227227
expect(lesson.submitted_count).to eq(2)
228228
end
229229
end
230+
231+
describe '#recalculate_submitted_projects_count!' do
232+
it 'sets the submitted projects count to 0 if there is no project' do
233+
lesson = create(:lesson, project: nil, submitted_projects_count: 3)
234+
235+
lesson.recalculate_submitted_projects_count!
236+
237+
expect(lesson.reload.submitted_projects_count).to eq(0)
238+
end
239+
240+
it 'returns the count of submitted remixes of the lesson project' do
241+
student = create(:student, school:)
242+
lesson = create(:lesson, school:, user_id: teacher.id)
243+
244+
remix_1 = create(:project, school:, remixed_from_id: lesson.project.id, user_id: student.id)
245+
remix_1.school_project.transition_status_to!(:submitted, remix_1.user_id)
246+
247+
remix_2 = create(:project, school:, remixed_from_id: lesson.project.id, user_id: student.id)
248+
remix_2.school_project.transition_status_to!(:submitted, remix_2.user_id)
249+
250+
create(:project, school:, remixed_from_id: lesson.project.id, user_id: student.id) # Not submitted
251+
252+
lesson.update!(submitted_projects_count: 0)
253+
lesson.recalculate_submitted_projects_count!
254+
255+
expect(lesson.reload.submitted_projects_count).to eq(2)
256+
end
257+
end
230258
end

spec/state_machines/school_project_state_machine_spec.rb

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,21 @@
1616
end
1717

1818
describe 'transitions' do
19+
it 'recalculates the parent lesson submitted projects count' do
20+
teacher = create(:teacher, school:)
21+
lesson = create(:lesson, school:, user_id: teacher.id)
22+
remix = create(:project, school:, user_id: student.id, remixed_from_id: lesson.project.id)
23+
state_machine = described_class.new(remix.school_project, transition_class: SchoolProjectTransition)
24+
25+
expect do
26+
state_machine.transition_to!(:submitted)
27+
end.to change { lesson.reload.submitted_projects_count }.from(0).to(1)
28+
29+
expect do
30+
state_machine.transition_to!(:returned)
31+
end.to change { lesson.reload.submitted_projects_count }.from(1).to(0)
32+
end
33+
1934
context 'when in unsubmitted state' do
2035
it 'can transition to submitted' do
2136
expect(state_machine.can_transition_to?(:submitted)).to be true

0 commit comments

Comments
 (0)