Skip to content

Commit

Permalink
add scores table
Browse files Browse the repository at this point in the history
add scores table, which will hold the
computed_current_score and computed_final_score
which is currently stored on the enrollment
object. in addition, grading period scores will
be stored on the scores table.

closes CNVS-33573

Test plan:
* Create a course
* Make sure should total for all grading periods is enabled
* As a teacher, create an assignment that falls in a grading period
* As a student, submit something for that assignment
* As a teacher, grade that submission
* As a student, view your grades
* You should see the student's grade for both the grading period and
  all grading periods.
* Repeat this process for another assignment and make sure the total
  grades get updated for the student.

Basically, we want to smoke test test saving scores for an assignment
and re-calculating course grades with new scores. Everything should
behave the same as before.

Change-Id: Ib7241d55c0fa52e01441671f17f65675f8e10564
Reviewed-on: https://gerrit.instructure.com/96825
Reviewed-by: Spencer Olson <solson@instructure.com>
Reviewed-by: Shahbaz Javeed <sjaveed@instructure.com>
Tested-by: Keith T. Garner <kgarner@instructure.com>
QA-Review: Anju Reddy <areddy@instructure.com>
Product-Review: Keith T. Garner <kgarner@instructure.com>
  • Loading branch information
neilgupta committed Dec 28, 2016
1 parent ac8f6c5 commit 8edf09d
Show file tree
Hide file tree
Showing 25 changed files with 561 additions and 121 deletions.
49 changes: 45 additions & 4 deletions app/models/enrollment.rb
Expand Up @@ -44,7 +44,7 @@ class Enrollment < ActiveRecord::Base
has_many :role_overrides, :as => :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 :grading_period_grades, dependent: :destroy
has_many :scores, -> { active }, dependent: :destroy

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]
Expand Down Expand Up @@ -811,7 +811,7 @@ def destroy
if result
self.user.try(:update_account_associations)
self.user.touch
grading_period_grades.destroy_all
scores.destroy_all
end
result
end
Expand Down Expand Up @@ -995,11 +995,52 @@ def self.recompute_final_score_if_stale(course, user=nil)
end

def computed_current_grade
self.course.score_to_grade(self.computed_current_score)
cached_score_or_grade(:current, :grade)
end

def computed_final_grade
self.course.score_to_grade(self.computed_final_score)
cached_score_or_grade(:final, :grade)
end

def computed_current_score
cached_score_or_grade(:current, :score)
end

def computed_final_score
cached_score_or_grade(:final, :score)
end

def cached_score_or_grade(current_or_final, score_or_grade)
score = find_score
if score.present?
score.send("#{current_or_final}_#{score_or_grade}")
else
# TODO: drop the computed_current_score / computed_final_score columns
# after the data fixup to populate the scores table completes
score = read_attribute("computed_#{current_or_final}_score")
score_or_grade == :score ? score : course.score_to_grade(score)
end
end
private :cached_score_or_grade

def find_score(grading_period_id: nil)
if scores.loaded?
scores.find { |score| score.grading_period_id == grading_period_id }
else
scores.where(grading_period_id: grading_period_id).first
end
end
private :find_score

def graded_at
score = find_score
if score.present?
score.updated_at
else
# TODO: drop the graded_at column after the data fixup to populate
# the scores table completes
read_attribute(:graded_at)
end
end

def self.typed_enrollment(type)
Expand Down
2 changes: 1 addition & 1 deletion app/models/grading_period.rb
Expand Up @@ -22,7 +22,7 @@ class GradingPeriod < ActiveRecord::Base
attr_accessible :weight, :start_date, :end_date, :close_date, :title

belongs_to :grading_period_group, inverse_of: :grading_periods
has_many :grading_period_grades, dependent: :destroy
has_many :scores, -> { active }, dependent: :destroy

validates :title, :start_date, :end_date, :close_date, :grading_period_group_id, presence: true
validate :start_date_is_before_end_date
Expand Down
18 changes: 0 additions & 18 deletions app/models/grading_period_grade.rb

This file was deleted.

8 changes: 4 additions & 4 deletions app/models/grading_standard.rb
Expand Up @@ -129,15 +129,15 @@ def data=(new_val)
# round values to the nearest 0.01 (0.0001 since e.g. 78 is stored as .78)
# and dup the data while we're at it. (new_val.dup only dups one level, the
# elements of new_val.dup are the same objects as the elements of new_val)
new_val = new_val.map{ |grade_name, lower_bound| [ grade_name, lower_bound.round(4) ] }
new_val = new_val.map{ |grade_name, lower_bound| [ grade_name, lower_bound.round(4) ] } unless new_val.nil?
write_attribute(:data, new_val)
@ordered_scheme = nil
end

def data
unless self.version == VERSION
data = read_attribute(:data)
data = GradingStandard.upgrade_data(data, self.version)
data = GradingStandard.upgrade_data(data, self.version) unless data.nil?
self.data = data
end
read_attribute(:data)
Expand Down Expand Up @@ -207,8 +207,8 @@ def standard_data=(params={})
end

def valid_grading_scheme_data
self.errors.add(:data, 'grading scheme values cannot be negative') if self.data.any?{ |v| v[1] < 0 }
self.errors.add(:data, 'grading scheme cannot contain duplicate values') if self.data.map{|v| v[1]} != self.data.map{|v| v[1]}.uniq
self.errors.add(:data, 'grading scheme values cannot be negative') if self.data.present? && self.data.any?{ |v| v[1] < 0 }
self.errors.add(:data, 'grading scheme cannot contain duplicate values') if self.data.present? && self.data.map{|v| v[1]} != self.data.map{|v| v[1]}.uniq
end

def self.default_grading_standard
Expand Down
41 changes: 41 additions & 0 deletions app/models/score.rb
@@ -0,0 +1,41 @@
#
# Copyright (C) 2016 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 Score < ActiveRecord::Base
include Canvas::SoftDeletable

attr_accessible :current_score, :final_score, :grading_period

belongs_to :enrollment
belongs_to :grading_period
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, conditions: -> { active }

def current_grade
score_to_grade(current_score)
end

def final_grade
score_to_grade(final_score)
end

delegate :score_to_grade, to: :course
end
5 changes: 4 additions & 1 deletion app/presenters/grades_presenter.rb
Expand Up @@ -24,7 +24,10 @@ def course_grade_summaries
teacher_enrollments.each_with_object({}) do |e, hash|
hash[e.course_id] = e.shard.activate do
Rails.cache.fetch(['computed_avg_grade_for', e.course].cache_key) do
current_scores = e.course.student_enrollments.not_fake.group(:user_id).maximum(:computed_current_score).values.compact
student_enrollments = e.course.student_enrollments.not_fake.preload(:scores)
current_scores = student_enrollments.group_by(&:user_id).map do |_, enrollments|
enrollments.max(&:computed_current_score).computed_current_score
end.compact
score = (current_scores.sum.to_f * 100.0 / current_scores.length.to_f).round.to_f / 100.0 rescue nil
{:score => score, :students => current_scores.length }
end
Expand Down
Expand Up @@ -6,9 +6,6 @@ def up
unless column_exists?(:grading_period_grades, :workflow_state)
add_column :grading_period_grades, :workflow_state, :string
end
GradingPeriodGrade.where(workflow_state: nil).find_ids_in_batches do |ids|
GradingPeriodGrade.where(id: ids).update_all(workflow_state: 'active')
end
change_column_default :grading_period_grades, :workflow_state, 'active'
add_index :grading_period_grades, :workflow_state, algorithm: :concurrently
end
Expand Down
23 changes: 23 additions & 0 deletions db/migrate/20161212012659_create_scores_table.rb
@@ -0,0 +1,23 @@
class CreateScoresTable < ActiveRecord::Migration[4.2]
tag :predeploy

def up
create_table :scores do |t|
t.integer :enrollment_id, limit: 8, null: false
t.integer :grading_period_id, limit: 8
t.string :workflow_state, default: :active, null: false, limit: 255
t.float :current_score
t.float :final_score
t.timestamps
end

add_foreign_key :scores, :enrollments
add_foreign_key :scores, :grading_periods

add_index :scores, [:enrollment_id, :grading_period_id], unique: true
end

def down
drop_table :scores
end
end
11 changes: 11 additions & 0 deletions db/migrate/20161212200216_drop_grading_period_grades_table.rb
@@ -0,0 +1,11 @@
class DropGradingPeriodGradesTable < ActiveRecord::Migration[4.2]
tag :postdeploy

def up
drop_table :grading_period_grades
end

def down
raise ActiveRecord::IrreversibleMigration
end
end
104 changes: 91 additions & 13 deletions lib/grade_calculator.rb
Expand Up @@ -30,7 +30,7 @@ def initialize(user_ids, course, opts = {})
@groups = @course.assignment_groups.active
@grading_period = opts[:grading_period]
@assignments = @course.assignments.published.gradeable.to_a
@user_ids = Array(user_ids).map(&:to_i)
@user_ids = Array(user_ids).map { |id| Shard.relative_id_for(id, Shard.current, @course.shard) }
@current_updates = {}
@final_updates = {}
@ignore_muted = opts[:ignore_muted]
Expand Down Expand Up @@ -84,34 +84,112 @@ def compute_and_save_scores

private

def enrollments
@enrollments ||= Enrollment.shard(@course).active.
where(user_id: @user_ids, course_id: @course.id).
select(:id, :user_id)
end

def joined_enrollment_ids
@joined_enrollment_ids ||= enrollments.map(&:id).join(',')
end

def enrollments_by_user
@enrollments_by_user ||= begin
hsh = enrollments.group_by {|e| Shard.relative_id_for(e.user_id, Shard.current, @course.shard) }
hsh.default = []
hsh
end
end

def save_scores
raise "Can't save scores when ignore_muted is false" unless @ignore_muted

Course.where(:id => @course).not_recently_touched.update_all(:updated_at => Time.now.utc)
time = Enrollment.sanitize(Time.now.utc)
query = "updated_at=#{time}, graded_at=#{time}"
query += ", computed_current_score=CASE user_id #{@current_updates.map { |user_id, grade| "WHEN #{user_id} THEN #{grade || "NULL"}"}.
join(" ")} ELSE computed_current_score END"
query += ", computed_final_score=CASE user_id #{@final_updates.map { |user_id, grade| "WHEN #{user_id} THEN #{grade || "NULL"}"}.
join(" ")} ELSE computed_final_score END"
Enrollment.where(:user_id => @user_ids, :course_id => @course).update_all(query)
return if @current_updates.empty? && @final_updates.empty?
return if joined_enrollment_ids.blank?

@course.touch
updated_at = Score.sanitize(Time.now.utc)

Score.transaction 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 #{score || 'NULL'}"
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 #{score || 'NULL'}"
end.join(' ')
end.join(' ')}
ELSE final_score
END,
updated_at = #{updated_at}
WHERE
enrollment_id IN (#{joined_enrollment_ids}) AND
workflow_state <> 'deleted' 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 #{score || 'NULL'}"
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 #{score || 'NULL'}"
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.workflow_state <> 'deleted' 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
end

# The score ignoring unsubmitted assignments
def calculate_current_score(user_id, submissions)
calculate_score(submissions, user_id, @current_updates, true)
calculate_score(submissions, user_id, true)
end

# The final score for the class, so unsubmitted assignments count as zeros
def calculate_final_score(user_id, submissions)
calculate_score(submissions, user_id, @final_updates, false)
calculate_score(submissions, user_id, false)
end

def calculate_score(submissions, user_id, grade_updates, ignore_ungraded)
def calculate_score(submissions, user_id, ignore_ungraded)
group_sums = create_group_sums(submissions, user_id, ignore_ungraded)
info = calculate_total_from_group_scores(group_sums)
Rails.logger.info "GRADES: calculated: #{info.inspect}"
grade_updates[user_id] = info[:grade]

updates_hash = ignore_ungraded ? @current_updates : @final_updates
updates_hash[user_id] = info[:grade]

[info, group_sums.index_by { |s| s[:id] }]
end

Expand Down

0 comments on commit 8edf09d

Please sign in to comment.