fix several N+1 found by Bullet

in spec/lib/*

Change-Id: Ia689e76f0f2bd435909a7d87ba2b1b6ba77fee93
Reviewed-on: https://gerrit.instructure.com/159150
Tested-by: Jenkins
Reviewed-by: Rob Orton <rob@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
This commit is contained in:
Cody Cutrer 2018-07-30 15:49:56 -06:00
parent c4d9603edc
commit 72ce9c47c4
10 changed files with 25 additions and 14 deletions

View File

@ -45,7 +45,7 @@ class Enrollment < ActiveRecord::Base
has_many :role_overrides, :as => :context, :inverse_of => :context
has_many :pseudonyms, :primary_key => :user_id, :foreign_key => :user_id
has_many :course_account_associations, :foreign_key => 'course_id', :primary_key => 'course_id'
has_many :scores, -> { active }
has_many :scores, -> { active }, inverse_of: :enrollment
validates_presence_of :user_id, :course_id, :type, :root_account_id, :course_section_id, :workflow_state, :role_id
validates_inclusion_of :limit_privileges_to_course_section, :in => [true, false]
@ -1080,11 +1080,21 @@ class Enrollment < ActiveRecord::Base
id_opts ||= Score.params_for_course
valid_keys = %i(course_score grading_period grading_period_id assignment_group assignment_group_id)
return nil if id_opts.except(*valid_keys).any?
if scores.loaded?
result = if scores.loaded?
scores.detect { |score| score.attributes >= id_opts.with_indifferent_access }
else
scores.where(id_opts).first
end
if result
result.enrollment = self
# have to go through gymnastics to force-preload a has_one :through without causing a db transaction
if association(:course).loaded?
assn = result.association(:course)
assn.target = course
Bullet::Detector::Association.add_object_associations(result, :course) if defined?(Bullet) && Bullet.start?
end
end
result
end
def graded_at

View File

@ -19,7 +19,7 @@
class Score < ActiveRecord::Base
include Canvas::SoftDeletable
belongs_to :enrollment
belongs_to :enrollment, inverse_of: :scores
belongs_to :grading_period, optional: true
belongs_to :assignment_group, optional: true
has_one :course, through: :enrollment

View File

@ -262,6 +262,7 @@ class GradebookExporter
includes = {:user => {:pseudonyms => :account}, :course_section => [], :scores => []}
enrollments = scope.preload(includes).eager_load(:user).order_by_sortable_name.to_a
enrollments.each { |e| e.course = @course }
enrollments.partition { |e| e.type != "StudentViewEnrollment" }.flatten
end

View File

@ -55,6 +55,7 @@ class LatePolicyApplicator
@assignments.each do |assignment|
relevant_submissions(assignment).find_each do |submission|
submission.assignment = assignment
user_ids << submission.user_id if process_submission(late_policy, assignment, submission)
end
end
@ -85,7 +86,7 @@ class LatePolicyApplicator
query = missing_submissions_for(assignment)
end
query.left_joins(:grading_period).merge(GradingPeriod.open)
query.eager_load(:grading_period).merge(GradingPeriod.open).preload(:stream_item, :user)
end
end

View File

@ -52,7 +52,7 @@ describe DataFixup::PopulateScoresAndMetadataForAssignmentGroupsAndTeacherView d
ScoreMetadata.joins(:score).where(scores: { enrollment: @concluded_student_enrollment }).delete_all
Score.where(enrollment: @concluded_student_enrollment).where.not(assignment_group_id: nil).delete_all
Score.where(enrollment: @concluded_student_enrollment).update(unposted_current_score: nil, unposted_final_score: nil)
Score.where(enrollment: @concluded_student_enrollment).preload(:enrollment).update(unposted_current_score: nil, unposted_final_score: nil)
@assignment_group_two = @course.assignment_groups.create!(:name => "some other group")
@assignment2 = @course.assignments.create!(
@ -105,7 +105,7 @@ describe DataFixup::PopulateScoresAndMetadataForAssignmentGroupsAndTeacherView d
it 'creates assignment groups scores and metadata for concluded students' do
DataFixup::PopulateScoresAndMetadataForAssignmentGroupsAndTeacherView.run
scores = @concluded_student_enrollment.scores.where.not(assignment_group_id: nil)
scores = @concluded_student_enrollment.scores.where.not(assignment_group_id: nil).preload(:score_metadata)
expect(scores.count).to eq(2)
scores.each { |score| expect(score.score_metadata).not_to be_nil }
end

View File

@ -285,7 +285,7 @@ describe DueDateCacher do
@assignment2.only_visible_to_overrides = true
@assignment2.save!
Submission.destroy_all
Submission.delete_all
expect { DueDateCacher.recompute_course(@course) }.to change {
Submission.active.count

View File

@ -884,7 +884,7 @@ describe GradeCalculator do
end
let(:student_enrollment) { @student.enrollments.first }
let(:scores) { @student.enrollments.first.scores.index_by(&:grading_period_id) }
let(:scores) { @student.enrollments.first.scores.preload(:score_metadata).index_by(&:grading_period_id) }
let(:overall_course_score) { @student.enrollments.first.scores.find_by(course_score: true) }
let(:submission_for_first_assignment) { Submission.find_by(user: @student, assignment: @assignments[1]) }
let(:submission_for_second_assignment) { Submission.find_by(user: @student, assignment: @assignments[4]) }

View File

@ -79,7 +79,7 @@ describe GradebookGradingPeriodAssignments do
it "excludes deleted submissions" do
assignment_in_gp2 = @example_course.assignments.create!(due_at: 1.day.from_now)
assignment_in_gp2.destroy
assignment_in_gp2.submissions.map(&:destroy)
assignment_in_gp2.submissions.preload(:all_submission_comments, :lti_result, :versions, :current_version_unidirectional).map(&:destroy)
expect(hash[@period2.id]).not_to include(assignment_in_gp2.id.to_s)
end

View File

@ -177,7 +177,6 @@ describe Lti::Security do
nonce: nonce,
timestamp: timestamp
)
puts header.send(:signature_base)
expect(header.valid?(signature: signed_params['oauth_signature'])).to eq true
end

View File

@ -47,13 +47,13 @@ describe SubmissionList do
@assignment3 = @course.assignments.create!(:title => 'three', :points_possible => 10)
Time.zone = 'Alaska'
allow(Time).to receive(:now).and_return(Time.utc(2011, 12, 31, 23, 0)).ordered # 12/31 14:00 local time
allow(Time).to receive(:now).and_return(Time.utc(2011, 12, 31, 23, 0)) # 12/31 14:00 local time
@assignment1.grade_student(@student, {:grade => 10, :grader => @teacher})
allow(Time).to receive(:now).and_return(Time.utc(2012, 1, 1, 1, 0)).ordered # 12/31 16:00 local time
allow(Time).to receive(:now).and_return(Time.utc(2012, 1, 1, 1, 0)) # 12/31 16:00 local time
@assignment2.grade_student(@student, {:grade => 10, :grader => @teacher})
allow(Time).to receive(:now).and_return(Time.utc(2012, 1, 1, 10, 0)).ordered # 1/01 01:00 local time
allow(Time).to receive(:now).and_return(Time.utc(2012, 1, 1, 10, 0)) # 1/01 01:00 local time
@assignment3.grade_student(@student, {:grade => 10, :grader => @teacher})
allow(Time).to receive(:now).and_call_original.ordered
allow(Time).to receive(:now).and_call_original
@days = SubmissionList.new(@course).days
expect(@days.size).to eq 2