add unposted scores to gradebook import/export

Include unposted scores and grades for assignment groups as well as
current/final scores in the Gradebook CSV export.

closes GRADE-762

Test plan:
- Set up a course with some students and one or more assignment groups.
- Add some assignments and record some grades. Mute at least one of the
  assignments.
- Export the grades from the course. Check that the resulting CSV file
  includes columns for unposted scores for each assignment group you
  exported and that they correctly reflect the values for unposted
  scores.
- Also check that the CSV includes corresponding unposted columns for
  each summary column (current score, final score, and so on) and that
  each new column's value takes into account the scores of unposted
  assignments.
- Finally, import the CSV file you just exported and make sure there
  are no errors due to the addition of the new columns (as with the
  rest of the summary columns, they should be ignored by the importer).

Change-Id: I3bbbe6b5ad93816d929fa72b48a6006271825ed4
Reviewed-on: https://gerrit.instructure.com/138047
Reviewed-by: Spencer Olson <solson@instructure.com>
Tested-by: Jenkins
Reviewed-by: Jeremy Neander <jneander@instructure.com>
Reviewed-by: Derek Bender <djbender@instructure.com>
QA-Review: Indira Pai <ipai@instructure.com>
Product-Review: Keith T. Garner <kgarner@instructure.com>
This commit is contained in:
Adrian Packel 2018-01-15 16:16:25 -06:00
parent 12d9d2d3e7
commit 7d2a89fbbb
5 changed files with 205 additions and 89 deletions

View File

@ -70,14 +70,9 @@ class GradebookExporter
# remove duplicate enrollments for students enrolled in multiple sections
student_enrollments = student_enrollments.uniq(&:user_id)
# grading_period_id == 0 means no grading period selected
unless @options[:grading_period_id].to_i == 0
grading_period = GradingPeriod.for(@course).find_by(id: @options[:grading_period_id])
end
# TODO: Stop using the grade calculator and instead use the scores table. This cannot be done until
# we start storing total scores that include muted assignments on the scores table, which will be
# implemented as part of CNVS-27558.
# TODO: Stop using the grade calculator and instead use the scores table entirely.
# This cannot be done until we are storing points values in the scores table, which
# will be implemented as part of GRADE-8.
calc = GradeCalculator.new(student_enrollments.map(&:user_id), @course,
ignore_muted: false,
grading_period: grading_period)
@ -86,7 +81,7 @@ class GradebookExporter
submissions = {}
calc.submissions.each { |s| submissions[[s.user_id, s.assignment_id]] = s }
assignments = select_in_grading_period calc.assignments, grading_period
assignments = select_in_grading_period calc.assignments
assignments = assignments.sort_by do |a|
[a.assignment_group_id, a.position || 0, a.due_at || CanvasSort::Last, a.title]
@ -106,24 +101,26 @@ class GradebookExporter
row << "Root Account" if include_sis_id && include_root_account
row << "Section"
row.concat assignments.map(&:title_with_id)
include_points = !@course.apply_group_weights?
if should_show_totals
groups.each do |group|
if include_points
if include_points?
row << "#{group.name} Current Points" << "#{group.name} Final Points"
end
row << "#{group.name} Current Score" << "#{group.name} Final Score"
row << "#{group.name} Current Score"
row << "#{group.name} Unposted Current Score"
row << "#{group.name} Final Score"
row << "#{group.name} Unposted Final Score"
end
row << "Current Points" << "Final Points" if include_points
row << "Current Score" << "Final Score"
row << "Current Points" << "Final Points" if include_points?
row << "Current Score" << "Unposted Current Score" << "Final Score" << "Unposted Final Score"
if @course.grading_standard_enabled?
row << "Current Grade" << "Final Grade"
row << "Current Grade" << "Unposted Current Grade" << "Final Grade" << "Unposted Final Grade"
end
end
csv << row
group_filler_length = groups.size * (include_points ? 4 : 2)
group_filler_length = groups.size * (include_points? ? 6 : 2)
# Possible muted row
if assignments.any?(&:muted)
@ -134,11 +131,13 @@ class GradebookExporter
if should_show_totals
row.concat([nil] * group_filler_length)
row << nil << nil if include_points
row << nil << nil
row << nil << nil if include_points?
row << nil << nil << nil << nil
end
row << nil if @course.grading_standard_enabled?
if @course.grading_standard_enabled?
row << nil << nil << nil << nil
end
csv << row
end
@ -152,9 +151,12 @@ class GradebookExporter
if should_show_totals
row.concat([read_only] * group_filler_length)
row << read_only << read_only if include_points
row << read_only << read_only
row << read_only if @course.grading_standard_enabled?
row << read_only << read_only if include_points?
row << read_only << read_only << read_only << read_only
if @course.grading_standard_enabled?
row << read_only << read_only << read_only << read_only
end
end
csv << row
@ -191,7 +193,10 @@ class GradebookExporter
row.concat(student_submissions)
if should_show_totals
row += show_totals(grades.shift, groups, include_points)
student_grades = grades.shift
row += show_group_totals(student_enrollment, student_grades, groups)
row += show_overall_totals(student_enrollment, student_grades)
end
csv << row
@ -205,7 +210,7 @@ class GradebookExporter
# course_section: used for display_name in csv output
# user > pseudonyms: used for sis_user_id/unique_id if options[:include_sis_id]
# user > pseudonyms > account: used in SisPseudonym > works_for_account
includes = {:user => {:pseudonyms => :account}, :course_section => []}
includes = {:user => {:pseudonyms => :account}, :course_section => [], :scores => []}
enrollments = scope.preload(includes).eager_load(:user).order_by_sortable_name.to_a
enrollments.partition { |e| e.type != "StudentViewEnrollment" }.flatten
@ -220,30 +225,43 @@ class GradebookExporter
number
end
def show_totals(grade, groups, include_points)
def show_group_totals(student_enrollment, grade, groups)
result = []
groups.each do |group|
if include_points
if include_points?
result << format_numbers(grade[:current_groups][group.id][:score])
result << format_numbers(grade[:final_groups][group.id][:score])
end
result << format_numbers(grade[:current_groups][group.id][:grade])
result << format_numbers(grade[:final_groups][group.id][:grade])
result << format_numbers(student_enrollment.computed_current_score(assignment_group_id: group.id))
result << format_numbers(student_enrollment.unposted_current_score(assignment_group_id: group.id))
result << format_numbers(student_enrollment.computed_final_score(assignment_group_id: group.id))
result << format_numbers(student_enrollment.unposted_final_score(assignment_group_id: group.id))
end
if include_points
result
end
def show_overall_totals(student_enrollment, grade)
result = []
if include_points?
result << format_numbers(grade[:current][:total])
result << format_numbers(grade[:final][:total])
end
result << format_numbers(grade[:current][:grade])
result << format_numbers(grade[:final][:grade])
score_opts = grading_period ? { grading_period_id: grading_period.id } : Score.params_for_course
result << format_numbers(student_enrollment.computed_current_score(score_opts))
result << format_numbers(student_enrollment.unposted_current_score(score_opts))
result << format_numbers(student_enrollment.computed_final_score(score_opts))
result << format_numbers(student_enrollment.unposted_final_score(score_opts))
if @course.grading_standard_enabled?
result << @course.score_to_grade(grade[:current][:grade])
result << @course.score_to_grade(grade[:final][:grade])
result << student_enrollment.computed_current_grade(score_opts)
result << student_enrollment.unposted_current_grade(score_opts)
result << student_enrollment.computed_final_grade(score_opts)
result << student_enrollment.unposted_final_grade(score_opts)
end
result
end
@ -266,11 +284,25 @@ class GradebookExporter
name
end
def select_in_grading_period(assignments, grading_period)
def grading_period
return @grading_period if defined? @grading_period
@grading_period = nil
# grading_period_id == 0 means no grading period selected
if @options[:grading_period_id].to_i != 0
@grading_period = GradingPeriod.for(@course).find_by(id: @options[:grading_period_id])
end
end
def select_in_grading_period(assignments)
if grading_period
grading_period.assignments(assignments)
else
assignments
end
end
def include_points?
!@course.apply_group_weights?
end
end

View File

@ -317,6 +317,9 @@ class GradebookImporter
def strip_non_assignment_columns(row)
drop_student_information_columns(row)
# This regex will also include columns for unposted scores, which
# will be one of these values with "Unposted" prepended.
while row.last =~ /Current Score|Current Points|Current Grade|Final Score|Final Points|Final Grade/
row.pop
end

View File

@ -45,10 +45,9 @@ describe GradebookExporter do
it "has headers in a default order" do
expected_headers = [
"\xEF\xBB\xBFStudent", "ID", "SIS Login ID", "Section",
"Current Points", "Final Points",
"Current Score", "Final Score",
"Current Grade", "Final Grade"
"\xEF\xBB\xBFStudent", "ID", "SIS Login ID", "Section", "Current Points", "Final Points",
"Current Score", "Unposted Current Score", "Final Score", "Unposted Final Score",
"Current Grade", "Unposted Current Grade", "Final Grade", "Unposted Final Grade"
]
actual_headers = CSV.parse(subject, headers: true).headers
@ -251,4 +250,66 @@ describe GradebookExporter do
expect(rows[1][0]).to eql('="=sum(A)"')
end
end
context "when a course has unposted assignments" do
let(:posted_assignment) { @course.assignments.create!(title: "Posted", points_possible: 10) }
let(:unposted_assignment) { @course.assignments.create!(title: "Unposted", points_possible: 10, muted: true) }
before(:each) do
@course.assignments.create!(title: "Ungraded", points_possible: 10)
student_in_course active_all: true
posted_assignment.grade_student @student, grade: 9, grader: @teacher
unposted_assignment.grade_student @student, grade: 3, grader: @teacher
end
it "calculates assignment group scores correctly" do
csv = GradebookExporter.new(@course, @teacher, {}).to_csv
rows = CSV.parse(csv, headers: true)
expect(rows[2]["Assignments Current Score"].try(:to_f)).to eq 90
expect(rows[2]["Assignments Unposted Current Score"].try(:to_f)).to eq 60
expect(rows[2]["Assignments Final Score"].try(:to_f)).to eq 30
expect(rows[2]["Assignments Unposted Final Score"].try(:to_f)).to eq 40
end
it "calculates totals correctly" do
csv = GradebookExporter.new(@course, @teacher, {}).to_csv
rows = CSV.parse(csv, headers: true)
expect(rows[2]["Current Score"].try(:to_f)).to eq 90
expect(rows[2]["Unposted Current Score"].try(:to_f)).to eq 60
expect(rows[2]["Final Score"].try(:to_f)).to eq 30
expect(rows[2]["Unposted Final Score"].try(:to_f)).to eq 40
end
end
describe "#show_overall_totals" do
before(:each) do
course_with_teacher
student_in_course(course: @course, active_all: true)
end
context "when a grading period is supplied" do
it "fetches scores from the Enrollment object using the grading period ID" do
@group = Factories::GradingPeriodGroupHelper.new.legacy_create_for_course(@course)
grading_period = @group.grading_periods.create!(
start_date: 1.week.ago, end_date: 1.week.from_now, title: "test period"
)
expect_any_instance_of(StudentEnrollment).to receive(:computed_current_score)
.with({ grading_period_id: grading_period.id })
GradebookExporter.new(@course, @teacher, { grading_period_id: grading_period.id }).to_csv
end
end
context "when no grading period is supplied" do
it "fetches scores from the Enrollment object using the default Course parameters" do
expect_any_instance_of(StudentEnrollment).to receive(:computed_current_score).with(Score.params_for_course)
GradebookExporter.new(@course, @teacher, {}).to_csv
end
end
end
end

View File

@ -426,7 +426,7 @@ describe GradebookImporter do
expect(@gi.assignments.first.points_possible).to eq 20
end
it "does not try to create assignments for the totals columns" do
it "does not create assignments for the totals columns" do
course_model
@assignment1 = @course.assignments.create!(:name => 'Assignment 1', :points_possible => 10)
importer_with_rows(
@ -437,6 +437,18 @@ describe GradebookImporter do
expect(@gi.missing_assignments).to be_empty
end
it "does not create assignments for unposted columns" do
course_model
@assignment1 = @course.assignments.create!(:name => 'Assignment 1', :points_possible => 10)
importer_with_rows(
"Student,ID,Section,Assignment 1,Current Points,Final Points,Unposted Current Score," \
"Unposted Final Score,Unposted Final Grade",
"Points Possible,,,20,,,,,"
)
expect(@gi.assignments).to eq [@assignment1]
expect(@gi.missing_assignments).to be_empty
end
it "parses new and existing users" do
course_with_student(active_all: true)
@student1 = @student

View File

@ -1350,14 +1350,16 @@ describe Course, "gradebook_to_csv" do
csv = GradebookExporter.new(@course, @teacher).to_csv
expect(csv).not_to be_nil
rows = CSV.parse(csv)
expect(rows.length).to equal(3)
expect(rows[0][-1]).to eq "Final Score"
expect(rows[1][-1]).to eq "(read only)"
expect(rows[2][-1]).to eq "50.0"
expect(rows[0][-2]).to eq "Current Score"
expect(rows[1][-2]).to eq "(read only)"
expect(rows[2][-2]).to eq "100.0"
rows = CSV.parse(csv, headers: true)
expect(rows.length).to equal(2)
expect(rows[0]["Unposted Final Score"]).to eq "(read only)"
expect(rows[1]["Unposted Final Score"]).to eq "50.0"
expect(rows[0]["Final Score"]).to eq "(read only)"
expect(rows[1]["Final Score"]).to eq "50.0"
expect(rows[0]["Unposted Current Score"]).to eq "(read only)"
expect(rows[1]["Unposted Current Score"]).to eq "100.0"
expect(rows[0]["Current Score"]).to eq "(read only)"
expect(rows[1]["Current Score"]).to eq "100.0"
end
it "should order assignments and groups by position" do
@ -1389,27 +1391,33 @@ describe Course, "gradebook_to_csv" do
csv = GradebookExporter.new(@course, @teacher).to_csv
expect(csv).not_to be_nil
rows = CSV.parse(csv)
expect(rows.length).to equal(3)
rows = CSV.parse(csv, headers: true)
expect(rows.length).to equal(2)
assignments, groups = [], []
rows[0].each do |column|
rows.headers.each do |column|
assignments << column.sub(/ \([0-9]+\)/, '') if column =~ /Assignment \d+/
groups << column if column =~ /Some Assignment Group/
end
expect(assignments).to eq ["Assignment 02", "Assignment 03", "Assignment 01", "Assignment 05", "Assignment 04", "Assignment 06", "Assignment 07", "Assignment 09", "Assignment 11", "Assignment 12", "Assignment 13", "Assignment 14", "Assignment 08", "Assignment 10"]
expect(groups).to eq ["Some Assignment Group 1 Current Points",
"Some Assignment Group 1 Final Points",
"Some Assignment Group 1 Current Score",
"Some Assignment Group 1 Final Score",
"Some Assignment Group 2 Current Points",
"Some Assignment Group 2 Final Points",
"Some Assignment Group 2 Current Score",
"Some Assignment Group 2 Final Score"]
expect(groups).to eq [
"Some Assignment Group 1 Current Points",
"Some Assignment Group 1 Final Points",
"Some Assignment Group 1 Current Score",
"Some Assignment Group 1 Unposted Current Score",
"Some Assignment Group 1 Final Score",
"Some Assignment Group 1 Unposted Final Score",
"Some Assignment Group 2 Current Points",
"Some Assignment Group 2 Final Points",
"Some Assignment Group 2 Current Score",
"Some Assignment Group 2 Unposted Current Score",
"Some Assignment Group 2 Final Score",
"Some Assignment Group 2 Unposted Final Score"
]
expect(rows[2][-10]).to eq "100.0" # ag1 current score
expect(rows[2][-9]).to eq "50.0" # ag1 final score
expect(rows[2][-6]).to eq "50.0" # ag2 current score
expect(rows[2][-5]).to eq "25.0" # ag2 final score
expect(rows[1]["Some Assignment Group 1 Current Score"]).to eq "100.0"
expect(rows[1]["Some Assignment Group 1 Final Score"]).to eq "50.0"
expect(rows[1]["Some Assignment Group 2 Current Score"]).to eq "50.0"
expect(rows[1]["Some Assignment Group 2 Final Score"]).to eq "25.0"
end
it "handles nil assignment due_dates if the group and position are the same" do
@ -1517,20 +1525,24 @@ describe Course, "gradebook_to_csv" do
csv = GradebookExporter.new(@course, @teacher).to_csv
expect(csv).not_to be_nil
rows = CSV.parse(csv)
expect(rows.length).to equal(3)
expect(rows[0][-1]).to eq "Final Grade"
expect(rows[1][-1]).to eq "(read only)"
expect(rows[2][-1]).to eq "A-"
expect(rows[0][-2]).to eq "Current Grade"
expect(rows[1][-2]).to eq "(read only)"
expect(rows[2][-2]).to eq "A-"
expect(rows[0][-3]).to eq "Final Score"
expect(rows[1][-3]).to eq "(read only)"
expect(rows[2][-3]).to eq "90.0"
expect(rows[0][-4]).to eq "Current Score"
expect(rows[1][-4]).to eq "(read only)"
expect(rows[2][-4]).to eq "90.0"
rows = CSV.parse(csv, headers: true)
expect(rows.length).to equal(2)
expect(rows[0]["Unposted Final Grade"]).to eq "(read only)"
expect(rows[1]["Unposted Final Grade"]).to eq "A-"
expect(rows[0]["Final Grade"]).to eq "(read only)"
expect(rows[1]["Final Grade"]).to eq "A-"
expect(rows[0]["Unposted Current Grade"]).to eq "(read only)"
expect(rows[1]["Unposted Current Grade"]).to eq "A-"
expect(rows[0]["Current Grade"]).to eq "(read only)"
expect(rows[1]["Current Grade"]).to eq "A-"
expect(rows[0]["Unposted Final Score"]).to eq "(read only)"
expect(rows[1]["Unposted Final Score"]).to eq "90.0"
expect(rows[0]["Final Score"]).to eq "(read only)"
expect(rows[1]["Final Score"]).to eq "90.0"
expect(rows[0]["Unposted Current Score"]).to eq "(read only)"
expect(rows[1]["Unposted Current Score"]).to eq "90.0"
expect(rows[0]["Current Score"]).to eq "(read only)"
expect(rows[1]["Current Score"]).to eq "90.0"
end
it "should include sis ids if enabled" do
@ -1646,19 +1658,15 @@ describe Course, "gradebook_to_csv" do
end
it "includes points for unweighted courses" do
csv = CSV.parse(GradebookExporter.new(@course, @teacher).to_csv)
expect(csv[0][-8]).to eq "Assignments Current Points"
expect(csv[0][-7]).to eq "Assignments Final Points"
expect(csv[1][-8]).to eq "(read only)"
expect(csv[1][-7]).to eq "(read only)"
expect(csv[2][-8]).to eq "8.0"
expect(csv[2][-7]).to eq "8.0"
expect(csv[0][-4]).to eq "Current Points"
expect(csv[0][-3]).to eq "Final Points"
expect(csv[1][-4]).to eq "(read only)"
expect(csv[1][-3]).to eq "(read only)"
expect(csv[2][-4]).to eq "8.0"
expect(csv[2][-3]).to eq "8.0"
csv = CSV.parse(GradebookExporter.new(@course, @teacher).to_csv, headers: true)
expect(csv[0]["Assignments Current Points"]).to eq "(read only)"
expect(csv[1]["Assignments Current Points"]).to eq "8.0"
expect(csv[0]["Assignments Final Points"]).to eq "(read only)"
expect(csv[1]["Assignments Final Points"]).to eq "8.0"
expect(csv[0]["Current Points"]).to eq "(read only)"
expect(csv[1]["Current Points"]).to eq "8.0"
expect(csv[0]["Final Points"]).to eq "(read only)"
expect(csv[1]["Final Points"]).to eq "8.0"
end
it "doesn't include points for weighted courses" do