ylqjgm
/
canvas-lms
mirror of https://github.com/instructure/canvas-lms.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

store scores for each assignment group 

closes GRADE-19

Test Plan:
1. Using the master branch prior to this commit:
   a. Seed a course with all of the following:
      - weighted assignment groups
      - weighted grading periods
      - ungraded assignments
      - quizzes
      - late and missing policies
   b. Record all data displayed in the gradebook
2. Using the code in this commit:
   a. Verify GRADEBOOK_COURSE_SCORE_POPULATED is
      not set in your environment
   b. Run database migrations
   c. Repeat steps in (1a)
   d. Verify that results are identical to (1b)
3. Using the code in this commit:
   a. Set GRADEBOOK_COURSE_SCORE_POPULATED="true"
      in your environment
   b. Repeat steps in (1a)
   c. Verify that results are identical to (1b)
   d. (Dev-QA only)
      Verify that assignment group grades recorded in the
      scores table match those calculated by the front end
4. (Dev-QA only)
   run scores_spec and grade_calculator_spec tests
   with GRADEBOOK_COURSE_SCORE_POPULATED="true"
5. (Dev-QA only)
   After running grade calculations with the new migrations
   in place, revert those migrations and verify that all
   scores created for assignment groups are deleted

Change-Id: Ie5e06eb5ad7c0d3958a686e7f0b054e6585be37d
Reviewed-on: https://gerrit.instructure.com/123085
Reviewed-by: Rob Orton <rob@instructure.com>
Reviewed-by: Jeremy Neander <jneander@instructure.com>
QA-Review: Spencer Olson <solson@instructure.com>
Tested-by: Jenkins
Reviewed-by: Keith T. Garner <kgarner@instructure.com>
Product-Review: Keith T. Garner <kgarner@instructure.com>
This commit is contained in:
Matt Taylor 2017-08-17 08:47:36 -05:00
parent 84140bb6a2
commit 42be7c1a8a
13 changed files with 614 additions and 113 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
app/models/assignment_group.rb
View File

@ -26,14 +26,21 @@ class AssignmentGroup < ActiveRecord::Base
has_a_broadcast_policy
serialize :integration_data, Hash
has_many :scores, -> { active }
has_many :assignments, -> { order('position, due_at, title') }, dependent: :destroy
has_many :active_assignments, -> { where("assignments.workflow_state<>'deleted'").order('assignments.position, assignments.due_at, assignments.title') }, class_name: 'Assignment'
has_many :published_assignments, -> { where(workflow_state: 'published').order('assignments.position, assignments.due_at, assignments.title') }, class_name: 'Assignment'
validates_presence_of :context_id, :context_type, :workflow_state
validates_length_of :rules, :maximum => maximum_text_length, :allow_nil => true, :allow_blank => true
validates_length_of :default_assignment_name, :maximum => maximum_string_length, :allow_nil => true
validates_length_of :name, :maximum => maximum_string_length, :allow_nil => true
has_many :active_assignments, -> {
where("assignments.workflow_state<>'deleted'").order('assignments.position, assignments.due_at, assignments.title')
}, class_name: 'Assignment'
has_many :published_assignments, -> {
where(workflow_state: 'published').order('assignments.position, assignments.due_at, assignments.title')
}, class_name: 'Assignment'
validates :context_id, :context_type, :workflow_state, presence: true
validates :rules, length: { maximum: maximum_text_length }, allow_nil: true, allow_blank: true
validates :default_assignment_name, length: { maximum: maximum_string_length }, allow_nil: true
validates :name, length: { maximum: maximum_string_length }, allow_nil: true
before_save :set_context_code
before_save :generate_default_values

30
app/models/enrollment.rb
View File

@ -1018,34 +1018,36 @@ class Enrollment < ActiveRecord::Base
end
end
def computed_current_grade(grading_period_id: nil)
cached_score_or_grade(:current, :grade, grading_period_id: grading_period_id)
def computed_current_grade(id_opts=nil)
cached_score_or_grade(:current, :grade, id_opts)
end
def computed_final_grade(grading_period_id: nil)
cached_score_or_grade(:final, :grade, grading_period_id: grading_period_id)
def computed_final_grade(id_opts=nil)
cached_score_or_grade(:final, :grade, id_opts)
end
def computed_current_score(grading_period_id: nil)
cached_score_or_grade(:current, :score, grading_period_id: grading_period_id)
def computed_current_score(id_opts=nil)
cached_score_or_grade(:current, :score, id_opts)
end
def computed_final_score(grading_period_id: nil)
cached_score_or_grade(:final, :score, grading_period_id: grading_period_id)
def computed_final_score(id_opts=nil)
cached_score_or_grade(:final, :score, id_opts)
end
def cached_score_or_grade(current_or_final, score_or_grade, grading_period_id: nil)
score = find_score(grading_period_id: grading_period_id)
def cached_score_or_grade(current_or_final, score_or_grade, id_opts=nil)
score = find_score(id_opts)
score&.send("#{current_or_final}_#{score_or_grade}")
end
private :cached_score_or_grade
# when grading period is nil, we are fetching the overall course score
def find_score(grading_period_id: nil)
def find_score(id_opts=nil)
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?
scores.find { |score| score.grading_period_id == grading_period_id }
scores.detect { |score| score.attributes >= id_opts.with_indifferent_access }
else
scores.where(grading_period_id: grading_period_id).first
scores.where(id_opts).first
end
end

51
app/models/score.rb
View File

@ -20,19 +20,23 @@ class Score < ActiveRecord::Base
include Canvas::SoftDeletable
belongs_to :enrollment
belongs_to :grading_period
belongs_to :grading_period, optional: true
belongs_to :assignment_group, optional: true
has_one :course, through: :enrollment
validates :enrollment, presence: true
validates :current_score, :final_score, numericality: true, allow_nil: true
validates_uniqueness_of :enrollment_id, scope: :grading_period_id
validate :scorable_association_check, if: -> { Score.course_score_populated? }
before_validation :set_course_score, unless: :course_score_changed?
set_policy do
given { |user, session|
given do |user, _session|
(user&.id == enrollment.user_id && !course.hide_final_grades?) ||
course.grants_any_right?(user, :manage_grades, :view_all_grades) ||
enrollment.user.grants_right?(user, :read_as_parent)
}
end
can :read
end
@ -44,5 +48,44 @@ class Score < ActiveRecord::Base
score_to_grade(final_score)
end
def scorable
# if you're calling this method, you might want to preload objects to avoid N+1
grading_period || assignment_group || enrollment.course
end
def self.course_score_populated?
ENV['GRADEBOOK_COURSE_SCORE_POPULATED'].present?
end
def self.params_for_course
Score.course_score_populated? ? { course_score: true } : { grading_period_id: nil }
end
delegate :score_to_grade, to: :course
private
def set_course_score
gpid = read_attribute(:grading_period_id)
agid = read_attribute(:assignment_group_id)
write_attribute(:course_score, (gpid || agid).nil?)
true
end
def scorable_association_check
scc = scorable_association_count
if scc == 0 && !course_score
errors.add(:course_score, "should be true when there are no scorable associations")
elsif scc == 1 && course_score
errors.add(:course_score, "should be false when there is a scorable association")
elsif scc > 1
errors.add(:base, "may not have multiple scorable associations")
else
return true
end
end
def scorable_association_count
[grading_period_id, assignment_group_id].compact.length
end
end

58
db/migrate/20170815211443_add_assignment_group_id_to_scores.rb Normal file
View File

@ -0,0 +1,58 @@
#
# Copyright (C) 2017 - present Instructure, Inc.
#
# This file is part of Canvas.
#
# Canvas is free software: you can redistribute it and/or modify it under
# the terms of the GNU Affero General Public License as published by the Free
# Software Foundation, version 3 of the License.
#
# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
# details.
#
# You should have received a copy of the GNU Affero General Public License along
# with this program. If not, see <http://www.gnu.org/licenses/>.
#
class AddAssignmentGroupIdToScores < ActiveRecord::Migration[5.0]
tag :predeploy
disable_ddl_transaction!
def change
add_belongs_to :scores, :assignment_group, limit: 8, null: true
add_column :scores, :course_score, :boolean
change_column_default :scores, :course_score, from: nil, to: false
add_index :scores, :enrollment_id,
algorithm: :concurrently,
name: :index_enrollment_scores
add_index :scores, %i(enrollment_id grading_period_id), unique: true,
where: "grading_period_id IS NOT NULL",
algorithm: :concurrently,
name: :index_grading_period_scores
add_index :scores, %i(enrollment_id assignment_group_id), unique: true,
where: "assignment_group_id IS NOT NULL",
algorithm: :concurrently,
name: :index_assignment_group_scores
add_index :scores, :enrollment_id, unique: true,
where: "course_score", # course_score is already boolean
algorithm: :concurrently,
name: :index_course_scores
remove_index :scores, column: :enrollment_id,
name: :index_scores_on_enrollment_id
remove_index :scores, column: [:enrollment_id, :grading_period_id],
name: :index_scores_on_enrollment_id_and_grading_period_id
reversible do |direction|
direction.down { DataFixup::DeleteScoresForAssignmentGroups.run }
end
end
end

29
db/migrate/20170816172211_populate_course_score_on_scores.rb Normal file
View File

@ -0,0 +1,29 @@
#
# Copyright (C) 2017 - present Instructure, Inc.
#
# This file is part of Canvas.
#
# Canvas is free software: you can redistribute it and/or modify it under
# the terms of the GNU Affero General Public License as published by the Free
# Software Foundation, version 3 of the License.
#
# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
# details.
#
# You should have received a copy of the GNU Affero General Public License along
# with this program. If not, see <http://www.gnu.org/licenses/>.
#
class PopulateCourseScoreOnScores < ActiveRecord::Migration[5.0]
tag :postdeploy
def up
DataFixup::PopulateScoresCourseScore.send_later_if_production_enqueue_args(
:run,
priority: Delayed::LOW_PRIORITY,
strand: "populate_scores_course_score_#{Shard.current.database_server.id}"
)
end
end

23
lib/data_fixup/delete_scores_for_assignment_groups.rb Normal file
View File

@ -0,0 +1,23 @@
#
# Copyright (C) 2017 - present Instructure, Inc.
#
# This file is part of Canvas.
#
# Canvas is free software: you can redistribute it and/or modify it under
# the terms of the GNU Affero General Public License as published by the Free
# Software Foundation, version 3 of the License.
#
# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
# details.
#
# You should have received a copy of the GNU Affero General Public License along
# with this program. If not, see <http://www.gnu.org/licenses/>.
#
module DataFixup::DeleteScoresForAssignmentGroups
def self.run
Score.where.not(assignment_group_id: nil).in_batches { |scores| scores.delete_all }
end
end

25
lib/data_fixup/populate_scores_course_score.rb Normal file
View File

@ -0,0 +1,25 @@
#
# Copyright (C) 2017 - present Instructure, Inc.
#
# This file is part of Canvas.
#
# Canvas is free software: you can redistribute it and/or modify it under
# the terms of the GNU Affero General Public License as published by the Free
# Software Foundation, version 3 of the License.
#
# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
# details.
#
# You should have received a copy of the GNU Affero General Public License along
# with this program. If not, see <http://www.gnu.org/licenses/>.
#
module DataFixup::PopulateScoresCourseScore
def self.run
Score.where(course_score: nil).in_batches do |scores|
scores.update_all("course_score = (grading_period_id IS NULL)")
end
end
end

203
lib/grade_calculator.rb
View File

@ -45,6 +45,8 @@ class GradeCalculator
@user_ids = Array(user_ids).map { |id| Shard.relative_id_for(id, Shard.current, @course.shard) }
@current_updates = {}
@final_updates = {}
@current_groups = {}
@final_groups = {}
@ignore_muted = opts[:ignore_muted]
@effective_due_dates = opts[:effective_due_dates]
end
@ -100,7 +102,7 @@ class GradeCalculator
next unless enrollments_by_user[user_id].first
group_sums = compute_group_sums_for_user(user_id)
scores = compute_scores_for_user(user_id, group_sums)
update_changes_hash_for_user(user_id, scores)
update_changes_hash_for_user(user_id, scores, group_sums)
{
current: scores[:current],
current_groups: group_sums[:current].index_by { |group| group[:id] },
@ -138,9 +140,11 @@ class GradeCalculator
scores
end
def update_changes_hash_for_user(user_id, scores)
def update_changes_hash_for_user(user_id, scores, group_sums)
@current_updates[user_id] = scores[:current][:grade]
@final_updates[user_id] = scores[:final][:grade]
@current_groups[user_id] = group_sums[:current]
@final_groups[user_id] = group_sums[:final]
end
def calculate_total_from_weighted_grading_periods(user_id)
@ -268,7 +272,24 @@ class GradeCalculator
# GradeCalculator sometimes divides by 0 somewhere,
# resulting in NaN. Treat that as null here
score = nil if score.try(:nan?)
score || 'NULL'
score || 'NULL::float'
end
def group_rows
enrollments_by_user.keys.map do |user_id|
current = @current_groups[user_id].pluck(:global_id, :grade).to_h
final = @final_groups[user_id].pluck(:global_id, :grade).to_h
@groups.map do |group|
agid = group.global_id
enrollments_by_user[user_id].map do |enrollment|
"(#{enrollment.id}, #{group.id}, #{number_or_null(current[agid])}, #{number_or_null(final[agid])})"
end
end
end.flatten
end
def updated_at
@updated_at ||= Score.connection.quote(Time.now.utc)
end
def save_scores
@ -279,71 +300,120 @@ class GradeCalculator
return if @grading_period && @grading_period.deleted?
@course.touch
updated_at = Score.connection.quote(Time.now.utc)
save_scores_in_transaction
end
def save_scores_in_transaction
Score.transaction do
@course.shard.activate do
# Construct upsert statement to update existing Scores or create them if needed.
Score.connection.execute("
UPDATE #{Score.quoted_table_name}
SET
current_score = CASE enrollment_id
#{@current_updates.map do |user_id, score|
enrollments_by_user[user_id].map do |enrollment|
"WHEN #{enrollment.id} THEN #{number_or_null(score)}"
end.join(' ')
end.join(' ')}
ELSE current_score
END,
final_score = CASE enrollment_id
#{@final_updates.map do |user_id, score|
enrollments_by_user[user_id].map do |enrollment|
"WHEN #{enrollment.id} THEN #{number_or_null(score)}"
end.join(' ')
end.join(' ')}
ELSE final_score
END,
updated_at = #{updated_at},
-- if workflow_state was previously deleted for some reason, update it to active
workflow_state = COALESCE(NULLIF(workflow_state, 'deleted'), 'active')
WHERE
enrollment_id IN (#{joined_enrollment_ids}) AND
grading_period_id #{@grading_period ? "= #{@grading_period.id}" : 'IS NULL'};
INSERT INTO #{Score.quoted_table_name}
(enrollment_id, grading_period_id, current_score, final_score, created_at, updated_at)
SELECT
enrollments.id as enrollment_id,
#{@grading_period.try(:id) || 'NULL'} as grading_period_id,
CASE enrollments.id
#{@current_updates.map do |user_id, score|
enrollments_by_user[user_id].map do |enrollment|
"WHEN #{enrollment.id} THEN #{number_or_null(score)}"
end.join(' ')
end.join(' ')}
ELSE NULL
END :: float AS current_score,
CASE enrollments.id
#{@final_updates.map do |user_id, score|
enrollments_by_user[user_id].map do |enrollment|
"WHEN #{enrollment.id} THEN #{number_or_null(score)}"
end.join(' ')
end.join(' ')}
ELSE NULL
END :: float AS final_score,
#{updated_at} as created_at,
#{updated_at} as updated_at
FROM #{Enrollment.quoted_table_name} enrollments
LEFT OUTER JOIN #{Score.quoted_table_name} scores on
scores.enrollment_id = enrollments.id AND
scores.grading_period_id #{@grading_period ? "= #{@grading_period.id}" : 'IS NULL'}
WHERE
enrollments.id IN (#{joined_enrollment_ids}) AND
scores.id IS NULL;
")
save_course_and_grading_period_scores
rows = group_rows
if @grading_period.nil? && rows.any? && Score.course_score_populated?
save_assignment_group_scores(rows.join(','))
end
end
end
end
def save_course_and_grading_period_scores
# Construct upsert statement to update existing Scores or create them if needed,
# for course and grading period Scores.
Score.connection.execute("
UPDATE #{Score.quoted_table_name}
SET
current_score = CASE enrollment_id
#{@current_updates.map do |user_id, score|
enrollments_by_user[user_id].map do |enrollment|
"WHEN #{enrollment.id} THEN #{number_or_null(score)}"
end.join(' ')
end.join(' ')}
ELSE current_score
END,
final_score = CASE enrollment_id
#{@final_updates.map do |user_id, score|
enrollments_by_user[user_id].map do |enrollment|
"WHEN #{enrollment.id} THEN #{number_or_null(score)}"
end.join(' ')
end.join(' ')}
ELSE final_score
END,
updated_at = #{updated_at},
-- if workflow_state was previously deleted for some reason, update it to active
workflow_state = COALESCE(NULLIF(workflow_state, 'deleted'), 'active')
WHERE
enrollment_id IN (#{joined_enrollment_ids}) AND
assignment_group_id IS NULL AND
grading_period_id #{@grading_period ? "= #{@grading_period.id}" : 'IS NULL'};
INSERT INTO #{Score.quoted_table_name}
(enrollment_id, grading_period_id, current_score, final_score, course_score, created_at, updated_at)
SELECT
enrollments.id as enrollment_id,
#{@grading_period.try(:id) || 'NULL'} as grading_period_id,
CASE enrollments.id
#{@current_updates.map do |user_id, score|
enrollments_by_user[user_id].map do |enrollment|
"WHEN #{enrollment.id} THEN #{number_or_null(score)}"
end.join(' ')
end.join(' ')}
ELSE NULL
END :: float AS current_score,
CASE enrollments.id
#{@final_updates.map do |user_id, score|
enrollments_by_user[user_id].map do |enrollment|
"WHEN #{enrollment.id} THEN #{number_or_null(score)}"
end.join(' ')
end.join(' ')}
ELSE NULL
END :: float AS final_score,
#{@grading_period ? 'FALSE' : 'TRUE'} AS course_score,
#{updated_at} as created_at,
#{updated_at} as updated_at
FROM #{Enrollment.quoted_table_name} enrollments
LEFT OUTER JOIN #{Score.quoted_table_name} scores on
scores.enrollment_id = enrollments.id AND
scores.grading_period_id #{@grading_period ? "= #{@grading_period.id}" : 'IS NULL'}
WHERE
enrollments.id IN (#{joined_enrollment_ids}) AND
scores.id IS NULL;
")
end
def save_assignment_group_scores(group_values)
# Construct upsert statement to update existing Scores or create them if needed,
# for assignment group Scores.
Score.connection.execute("
UPDATE #{Score.quoted_table_name} scores
SET
current_score = val.current_score,
final_score = val.final_score,
updated_at = #{updated_at},
workflow_state = COALESCE(NULLIF(workflow_state, 'deleted'), 'active')
FROM (VALUES #{group_values}) val
(enrollment_id, assignment_group_id, current_score, final_score)
WHERE val.enrollment_id = scores.enrollment_id AND
val.assignment_group_id = scores.assignment_group_id;
INSERT INTO #{Score.quoted_table_name}
(enrollment_id, assignment_group_id, current_score, final_score,
course_score, created_at, updated_at)
SELECT
val.enrollment_id AS enrollment_id,
val.assignment_group_id as assignment_group_id,
val.current_score AS current_score,
val.final_score AS final_score,
FALSE AS course_score,
#{updated_at} AS created_at,
#{updated_at} AS updated_at
FROM (VALUES #{group_values}) val
(enrollment_id, assignment_group_id, current_score, final_score)
LEFT OUTER JOIN #{Score.quoted_table_name} sc ON
sc.enrollment_id = val.enrollment_id AND
sc.assignment_group_id = val.assignment_group_id
WHERE
sc.id IS NULL;
")
end
# returns information about assignments groups in the form:
# [
# {
@ -409,11 +479,12 @@ class GradeCalculator
}
{
:id => group.id,
:score => score,
:possible => possible,
:weight => group.group_weight,
:grade => ((score.to_f / possible * 100).round(2) if possible > 0),
id: group.id,
global_id: group.global_id,
score: score,
possible: possible,
weight: group.group_weight,
grade: ((score.to_f / possible * 100).round(2) if possible > 0),
}.tap { |group_grade_info|
Rails.logger.info "GRADES: calculated #{group_grade_info.inspect}"
}

108
spec/lib/grade_calculator_spec.rb
View File

@ -102,6 +102,32 @@ describe GradeCalculator do
context "sharding" do
specs_require_sharding
let(:seed_assignment_groups_with_scores) do
now = Time.zone.now
groups = []
assignments = []
submissions = []
@shard1.activate do
account = Account.create!
course_with_student(active_all: true, account: account, user: @user)
@course.update_attribute(:group_weighting_scheme, "percent")
groups <<
@course.assignment_groups.create!(name: "some group 1", group_weight: 50) <<
@course.assignment_groups.create!(name: "some group 2", group_weight: 50)
asgt_opts = { due_at: now, points_possible: 10 }
assignments <<
@course.assignments.create!(title: "Some Assignment 1", assignment_group: groups[0], **asgt_opts) <<
@course.assignments.create!(title: "Some Assignment 2", assignment_group: groups[1], **asgt_opts) <<
@course.assignments.create!(title: "Some Assignment 3", assignment_group: groups[1], **asgt_opts)
submissions <<
assignments[0].submissions.find_by!(user: @user) <<
assignments[1].submissions.find_by!(user: @user)
end
submissions[0].update_column(:score, 5.0)
submissions[1].update_column(:score, 2.5)
groups
end
it "should delete irrelevant cross-shard scores" do
@user = User.create!
@ -181,6 +207,24 @@ describe GradeCalculator do
expect(Enrollment.shard(@course.shard).where(user_id: @user.global_id).first.computed_current_score(grading_period_id: @grading_period.id)).to eql(20.0)
expect(Enrollment.shard(@course.shard).where(user_id: @user.global_id).first.computed_final_score(grading_period_id: @grading_period.id)).to eql(10.0)
end
it("should update cross-shard scores with assignment groups", if: Score.course_score_populated?) do
@user = User.create!
allow(GradeCalculator).to receive(:recompute_final_score) {}
groups = seed_assignment_groups_with_scores
allow(GradeCalculator).to receive(:recompute_final_score).and_call_original
GradeCalculator.recompute_final_score(@user.id, @course.id)
enrollment = Enrollment.shard(@course.shard).where(user_id: @user.global_id).first
expect(enrollment.computed_current_score).to be(37.5)
expect(enrollment.computed_final_score).to be(31.25)
expect(enrollment.computed_current_score(assignment_group_id: groups[0].id)).to be(50.0)
expect(enrollment.computed_final_score(assignment_group_id: groups[0].id)).to be(50.0)
expect(enrollment.computed_current_score(assignment_group_id: groups[1].id)).to be(25.0)
expect(enrollment.computed_final_score(assignment_group_id: groups[1].id)).to be(12.5)
end
end
it "should recompute when an assignment's points_possible changes'" do
@ -613,13 +657,13 @@ describe GradeCalculator do
it "should convert NaN to NULL" do
calc = GradeCalculator.new [@user.id], @course.id
score = 0/0.0
expect(calc.send(:number_or_null, score)).to eql('NULL')
expect(calc.send(:number_or_null, score)).to eql('NULL::float')
end
it "should convert nil to NULL" do
calc = GradeCalculator.new [@user.id], @course.id
score = nil
expect(calc.send(:number_or_null, score)).to eql('NULL')
expect(calc.send(:number_or_null, score)).to eql('NULL::float')
end
end
@ -643,7 +687,7 @@ describe GradeCalculator do
end
let(:scores) { @student.enrollments.first.scores.index_by(&:grading_period_id) }
let(:overall_course_score) { scores[nil] }
let(:overall_course_score) { @student.enrollments.first.scores.find_by(course_score: true) }
let(:submission_for_first_assignment) { Submission.find_by(user: @student, assignment: @first_assignment) }
let(:submission_for_second_assignment) { Submission.find_by(user: @student, assignment: @second_assignment) }
@ -938,6 +982,58 @@ describe GradeCalculator do
expect(overall_course_score.final_score).to eq(0.0)
end
end
it("does not save assignment group scores", unless: Score.course_score_populated?) do
@course.assignment_groups.create!(name: "some group")
GradeCalculator.new(@student.id, @course).compute_and_save_scores
expect(Score.where.not(assignment_group_id: nil).count).to be 0
end
context("assignment group scores", if: Score.course_score_populated?) do
before(:each) do
@group1 = @course.assignment_groups.create!(name: "some group 1")
@assignment1 = @course.assignments.create!(name: "assignment 1", points_possible: 20, assignment_group: @group1)
@assignment1.grade_student(@student, grade: 12, grader: @teacher)
@group2 = @course.assignment_groups.create!(name: "some group 2")
@assignment2 = @course.assignments.create!(name: "assignment 2", points_possible: 20, assignment_group: @group2)
@assignment2.grade_student(@student, grade: 18, grader: @teacher)
GradeCalculator.new(@student.id, @course).compute_and_save_scores
end
let(:student_scores) { @student.enrollments.first.scores }
it "stores separate assignment group scores for each of a student’s enrollments" do
(1..2).each do |i|
section = @course.course_sections.create!(name: "section #{i}")
@course.enroll_user(@student, 'StudentEnrollment', section: section,
enrollment_state: 'active', allow_multiple_enrollments: true)
end
GradeCalculator.new(@student.id, @course).compute_and_save_scores
scored_enrollment_ids = Score.where(assignment_group_id: @group1.id).map(&:enrollment_id)
expect(scored_enrollment_ids).to contain_exactly *(@student.enrollments.map(&:id))
end
it "updates active score rows for assignment groups if they already exist" do
orig_score_id = student_scores.first.id
@assignment1.grade_student(@student, grade: 15, grader: @teacher)
GradeCalculator.new(@student.id, @course).compute_and_save_scores
new_score_id = student_scores.first.id
expect(orig_score_id).to be new_score_id
end
it "activates previously soft deleted assignment group scores when updating them" do
student_scores.update_all(workflow_state: 'deleted')
GradeCalculator.new(@student.id, @course).compute_and_save_scores
expect(student_scores.map(&:workflow_state).uniq).to contain_exactly('active')
end
it "inserts score rows for assignment groups unless they already exist" do
expect do
@course.assignment_groups.create!(name: "yet another group")
GradeCalculator.new(@student.id, @course).compute_and_save_scores
end.to change{ Score.count }.by(1)
end
end
end
it "should return grades in the order they are requested" do
@ -1232,6 +1328,12 @@ describe GradeCalculator do
expect(final_grade_info(@user, @course)[:total]).to eq 20
expect(final_grade_info(@user, @course)[:possible]).to eq 40
end
it("saves scores for all assignment group and enrollment combinations", if: Score.course_score_populated?) do
user_ids = @course.enrollments.map(&:user_id).uniq
group_ids = @assignments.map(&:assignment_group_id).uniq
GradeCalculator.new(user_ids, @course.id).compute_and_save_scores
expect(Score.where(assignment_group_id: group_ids).count).to be @course.enrollments.count * group_ids.length
end
end
end

7
spec/models/assignment_group_spec.rb
View File

@ -42,6 +42,13 @@ describe AssignmentGroup do
expect(ag.group_weight).to eq 0
end
it "allows association with scores" do
ag = @course.assignment_groups.create!(@valid_attributes)
enrollment = @course.student_enrollments.first
score = ag.scores.create!(enrollment: enrollment, current_score: 9.0, final_score: 10.0)
expect(score.assignment_group_id).to be ag.id
end
context "visible assignments" do
before(:each) do
@ag = @course.assignment_groups.create!(@valid_attributes)

37
spec/models/enrollment_spec.rb
View File

@ -187,6 +187,8 @@ describe Enrollment do
)
end
let(:a_group) { @course.assignment_groups.create!(name: 'a group') }
describe '#computed_current_score' do
it 'uses the value from the associated score object, if one exists' do
@enrollment.scores.create!(current_score: 80.3)
@ -256,6 +258,41 @@ describe Enrollment do
end
end
describe '#find_score' do
before(:each) do
@course.update!(grading_standard_enabled: true)
allow(GradeCalculator).to receive(:recompute_final_score) {}
@enrollment.scores.create!(current_score: 85.3)
@enrollment.scores.create!(grading_period: period, current_score: 99.1)
@enrollment.scores.create!(assignment_group: a_group, current_score: 66.3)
allow(GradeCalculator).to receive(:recompute_final_score).and_call_original
end
it 'returns the course score' do
expect(@enrollment.find_score.current_score).to be 85.3
end
it 'returns grading period scores' do
expect(@enrollment.find_score(grading_period_id: period.id).current_score).to be 99.1
end
it 'returns assignment group scores' do
expect(@enrollment.find_score(assignment_group_id: a_group.id).current_score).to be 66.3
end
it 'returns no score when given an invalid grading period id' do
expect(@enrollment.find_score(grading_period_id: 99999)).to be nil
end
it 'returns no score when given an invalid assignment group id' do
expect(@enrollment.find_score(assignment_group_id: 8888888)).to be nil
end
it 'returns no score when given unrecognized id keys' do
expect(@enrollment.find_score(flavor: 'Anchovied Caramel')).to be nil
end
end
describe '#graded_at' do
it 'uses the updated_at from the associated score object, if one exists' do
score = @enrollment.scores.create!(current_score: 80.3)

4
spec/models/grading_period_spec.rb
View File

@ -19,7 +19,7 @@
require_relative '../spec_helper'
describe GradingPeriod do
subject(:grading_period) { grading_period_group.grading_periods.build(params) }
subject(:grading_period) { grading_period_group.grading_periods.create!(params) }
let(:group_helper) { Factories::GradingPeriodGroupHelper.new }
let(:account) { Account.create! }
@ -890,7 +890,7 @@ describe GradingPeriod do
end
it 'creates scores for the grading period upon its creation' do
expect{ grading_period.save! }.to change{ Score.count }.from(1).to(2)
expect{ grading_period.save! }.to change{ Score.count }.by(1)
end
it 'updates grading period scores when the grading period end date is changed' do

133
spec/models/score_spec.rb
View File

@ -19,28 +19,42 @@
require 'spec_helper'
describe Score do
before(:once) do
grading_periods
test_course.assignment_groups.create!(name: 'Assignments')
end
let(:test_course) { Course.create! }
let(:student) { student_in_course(course: test_course) }
let(:params) {
let(:params) do
{
course: test_course,
current_score: 80.2,
final_score: 74.0,
updated_at: 1.week.ago
}
}
end
let(:grading_period_score_params) do
params.merge(grading_period_id: GradingPeriod.first.id)
end
let(:assignment_group_score_params) do
params.merge(assignment_group_id: AssignmentGroup.first.id)
end
let(:grading_period_score) { student.scores.create!(grading_period_score_params) }
let(:assignment_group_score) { student.scores.create!(assignment_group_score_params) }
subject_once(:score) { student.scores.create!(params) }
it_behaves_like "soft deletion" do
before do
grading_periods
end
let(:creation_arguments) { [
params.merge(grading_period: GradingPeriod.first),
params.merge(grading_period: GradingPeriod.last)
] }
subject { student.scores }
let(:creation_arguments) do
[
params.merge(grading_period: GradingPeriod.first),
params.merge(grading_period: GradingPeriod.last)
]
end
end
describe 'validations' do
@ -51,12 +65,22 @@ describe Score do
expect(score).to be_invalid
end
it 'is invalid without unique enrollment' do
it 'is invalid without unique enrollment for course' do
student.scores.create!(params)
expect { student.scores.create!(params) }.to raise_error(ActiveRecord::RecordInvalid)
expect { student.scores.create!(params) }.to raise_error(ActiveRecord::RecordNotUnique)
end
shared_context "score attribute" do
it 'is invalid without unique enrollment for grading period' do
student.scores.create!(grading_period_score_params)
expect { student.scores.create!(grading_period_score_params) }.to raise_error(ActiveRecord::RecordNotUnique)
end
it('is invalid without unique enrollment for assignment group', if: Score.course_score_populated?) do
student.scores.create!(assignment_group_score_params)
expect { student.scores.create!(assignment_group_score_params) }.to raise_error(ActiveRecord::RecordNotUnique)
end
shared_examples "score attribute" do
it 'is valid as nil' do
score.write_attribute(attribute, nil)
expect(score).to be_valid
@ -73,8 +97,43 @@ describe Score do
end
end
include_context('score attribute') { let(:attribute) { :current_score } }
include_context('score attribute') { let(:attribute) { :final_score } }
include_examples('score attribute') { let(:attribute) { :current_score } }
include_examples('score attribute') { let(:attribute) { :final_score } }
context("scorable associations", if: Score.course_score_populated?) do
before(:once) { grading_periods }
it 'is valid with course_score true and no scorable associations' do
expect(student.scores.create!(course_score: true, **params)).to be_valid
end
it 'is valid with course_score false and a grading period association' do
expect(student.scores.create!(course_score: false, **grading_period_score_params)).to be_valid
end
it 'is valid with course_score false and an assignment group association' do
expect(student.scores.create!(course_score: false, **assignment_group_score_params)).to be_valid
end
it 'is invalid with course_score false and no scorable associations' do
expect do
score = student.scores.create!(params)
score.update!(course_score: false)
end.to raise_error(ActiveRecord::RecordInvalid)
end
it 'is invalid with course_score true and a scorable association' do
expect do
student.scores.create!(course_score: true, **grading_period_score_params)
end.to raise_error(ActiveRecord::RecordInvalid)
end
it 'is invalid with multiple scorable associations' do
expect do
student.scores.create!(grading_period_id: GradingPeriod.first.id, **assignment_group_score_params)
end.to raise_error(ActiveRecord::RecordInvalid)
end
end
end
describe '#current_grade' do
@ -111,16 +170,54 @@ describe Score do
end
end
describe('#scorable', if: Score.course_score_populated?) do
it 'returns course for course score' do
expect(score.scorable).to be score.enrollment.course
end
it 'returns grading period for grading period score' do
expect(grading_period_score.scorable).to be grading_period_score.grading_period
end
it 'returns assignment group for assignment group score' do
expect(assignment_group_score.scorable).to be assignment_group_score.assignment_group
end
end
describe('#course_score', if: Score.course_score_populated?) do
it 'sets course_score to true when there are no scorable associations' do
expect(score.course_score).to be true
end
it 'sets course_score to false for grading period scores' do
expect(grading_period_score.course_score).to be false
end
it 'sets course_score to false for assignment group scores' do
expect(assignment_group_score.course_score).to be false
end
end
describe('#params_for_course') do
it('uses course_score', if: Score.course_score_populated?) do
expect(Score.params_for_course).to eq(course_score: true)
end
it('uses nil grading period id', unless: Score.course_score_populated?) do
expect(Score.params_for_course).to eq(grading_period_id: nil)
end
end
context "permissions" do
it "allows the proper people" do
expect(score.grants_right? @enrollment.user, :read).to eq true
expect(score.grants_right?(@enrollment.user, :read)).to eq true
teacher_in_course(active_all: true)
expect(score.grants_right? @teacher, :read).to eq true
expect(score.grants_right?(@teacher, :read)).to eq true
end
it "doesn't work for nobody" do
expect(score.grants_right? nil, :read).to eq false
expect(score.grants_right?(nil, :read)).to eq false
end
it "doesn't allow random classmates to read" do