Skip to content

Commit

Permalink
gradebook csv: show correct totals for mgp
Browse files Browse the repository at this point in the history 
fixes CNVS-22054 (frd)

Test plan:
  * enable multiple grading periods
  * download the gradebook csv, make sure totals are correct

Change-Id: I24c50f47f112551a56da7cbcecc92acf5996ff48
Reviewed-on: https://gerrit.instructure.com/66342
Reviewed-by: Spencer Olson <[email protected]>
Tested-by: Jenkins
QA-Review: Jason Carter <[email protected]>
Product-Review: Keith T. Garner <[email protected]>
  • Loading branch information
@cmatheson @ccutrer
cmatheson authored and ccutrer committed Nov 17, 2015
1 parent db96a71 commit a4d9898
Show file tree
Hide file tree
Showing 4 changed files with 70 additions and 53 deletions.
13 changes: 10 additions & 3 deletions lib/gradebook_exporter.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,20 @@ def to_csv
# remove duplicate enrollments for students enrolled in multiple sections
student_enrollments = student_enrollments.uniq(&:user_id)

calc = GradeCalculator.new(student_enrollments.map(&:user_id), @course, :ignore_muted => false)
# grading_period_id == 0 means no grading period selected
unless @options[:grading_period_id].try(:to_i) == 0
grading_period = GradingPeriod.context_find @course, @options[:grading_period_id]
end

calc = GradeCalculator.new(student_enrollments.map(&:user_id), @course,
ignore_muted: false,
grading_period: grading_period)
grades = calc.compute_scores

submissions = {}
calc.submissions.each { |s| submissions[[s.user_id, s.assignment_id]] = s }

assignments = select_in_grading_period calc.assignments, @course, @options[:grading_period_id]
assignments = select_in_grading_period calc.assignments, @course, grading_period

assignments = assignments.sort_by do |a|
[a.assignment_group_id, a.position, a.due_at || CanvasSort::Last, a.title]
Expand Down Expand Up @@ -189,7 +196,7 @@ def enrollments_for_csv(scope, options={})

def show_totals?
return true if !@course.feature_enabled?(:multiple_grading_periods)
return true if @options[:grading_period_id] != "0"
return true if @options[:grading_period_id].try(:to_i) != 0
@course.feature_enabled?(:all_grading_periods_totals)
end

Expand Down
2 changes: 1 addition & 1 deletion lib/gradebook_importer.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ def parse!
current_grading_period = GradingPeriod.for(@context).current

unless current_grading_period.empty?
@assignments = select_in_grading_period @assignments, @context, current_grading_period.first.id
@assignments = select_in_grading_period @assignments, @context, current_grading_period.first
end

@assignments_outside_current_periods = memo - @assignments
Expand Down
5 changes: 2 additions & 3 deletions lib/gradebook_transformer.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,8 @@

module GradebookTransformer
private
def select_in_grading_period(assignments, course, grading_period_id)
if course.feature_enabled?(:multiple_grading_periods) && grading_period_id != "0"
grading_period = GradingPeriod.context_find course, grading_period_id
def select_in_grading_period(assignments, course, grading_period)
if grading_period && course.feature_enabled?(:multiple_grading_periods)
grading_period.assignments(assignments)
else
assignments
Expand Down
103 changes: 57 additions & 46 deletions spec/lib/gradebook_exporter_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,12 @@

describe "#to_csv" do
let(:course) { @course }

def exporter(opts = {})
GradebookExporter.new(course, @teacher, opts)
end

describe "default output with blank course" do
let(:exporter) { GradebookExporter.new(course, @teacher) }
subject(:csv) { exporter.to_csv }

it "produces a String" do
Expand Down Expand Up @@ -57,21 +61,32 @@
end

context "a course has assignments with due dates" do
let(:assignments) { course.assignments }
before(:once) do
@no_due_date_assignment = assignments.create! title: "no due date",
points_possible: 10

let!(:no_due_date_assignment) { assignments.create title: "no due date" }
let!(:past_assignment) do
assignments.create due_at: 5.weeks.ago, title: "past"
end
@past_assignment = assignments.create! due_at: 5.weeks.ago,
title: "past",
points_possible: 10

let!(:current_assignment) do
assignments.create due_at: 1.weeks.from_now, title: "current"
end
@current_assignment = assignments.create! due_at: 1.weeks.from_now,
title: "current",
points_possible: 10

@future_assignment = assignments.create! due_at: 8.weeks.from_now,
title: "future",
points_possible: 10

student_in_course active_all: true

let!(:future_assignment) do
assignments.create due_at: 8.weeks.from_now, title: "future"
@no_due_date_assignment.grade_student @student, grade: 1
@past_assignment.grade_student @student, grade: 2
@current_assignment.grade_student @student, grade: 3
@future_assignment.grade_student @student, grade: 4
end

let(:assignments) { course.assignments }

let!(:group) do
course.grading_period_groups.create!
end
Expand All @@ -84,7 +99,6 @@
}

group.grading_periods.create! args

end

let!(:last_period) do
Expand All @@ -97,49 +111,53 @@
group.grading_periods.create! args
end

let(:csv) { GradebookExporter.new(course, @teacher, grading_period_id: last_period.id).to_csv }
let(:headers) { CSV.parse(csv, headers: true).headers }

let(:csv_id_0) { GradebookExporter.new(course, @teacher, grading_period_id: "0").to_csv }
let(:headers_id_0) { CSV.parse(csv_id_0, headers: true).headers }

let(:csv_first_period) { GradebookExporter.new(course, @teacher, grading_period_id: first_period.id).to_csv }
let(:headers_first_period) {CSV.parse(csv_first_period, headers: true).headers }
let(:csv) { exporter(grading_period_id: @grading_period_id).to_csv }
let(:rows) { CSV.parse(csv, headers: true) }
let(:headers) { rows.headers }

describe "when multiple grading periods is on" do
before { @grading_period_id = last_period.id }

describe "assignments in the selected grading period are exported" do
let!(:enable_mgp) do
course.enable_feature!(:multiple_grading_periods)
end

it "exports selected grading period's assignments" do
expect(headers).to include no_due_date_assignment.title_with_id,
current_assignment.title_with_id
expect(headers).to include @no_due_date_assignment.title_with_id,
@current_assignment.title_with_id
final_grade = rows[1]["Final Score"].try(:to_f)
expect(final_grade).to eq 20
end

it "exports assignments without due dates if exporting last grading period" do
expect(headers).to include current_assignment.title_with_id,
no_due_date_assignment.title_with_id
expect(headers).to include @current_assignment.title_with_id,
@no_due_date_assignment.title_with_id
final_grade = rows[1]["Final Score"].try(:to_f)
expect(final_grade).to eq 20
end

it "does not export assignments without due date" do
expect(headers_first_period).to_not include no_due_date_assignment.title_with_id
@grading_period_id = first_period.id
expect(headers).to_not include @no_due_date_assignment.title_with_id
end

it "does not export assignments in other grading periods" do
expect(headers).to_not include past_assignment.title_with_id,
future_assignment.title_with_id
expect(headers).to_not include @past_assignment.title_with_id,
@future_assignment.title_with_id
end

it "does not export future assignments" do
expect(headers).to_not include future_assignment.title_with_id
expect(headers).to_not include @future_assignment.title_with_id
end

it "exports the entire gradebook when grading_period_id is 0" do
expect(headers_id_0).to include past_assignment.title_with_id,
current_assignment.title_with_id,
future_assignment.title_with_id,
no_due_date_assignment.title_with_id
@grading_period_id = 0
expect(headers).to include @past_assignment.title_with_id,
@current_assignment.title_with_id,
@future_assignment.title_with_id,
@no_due_date_assignment.title_with_id
expect(headers).not_to include "Final Score"
end
end
end
Expand All @@ -151,20 +169,13 @@
course.disable_feature!(:multiple_grading_periods)
end

it "exports current assignments" do
expect(headers).to include no_due_date_assignment.title_with_id
end

it "exports assignments without due dates" do
expect(headers).to include current_assignment.title_with_id
end

it "exports past assignments" do
expect(headers).to include past_assignment.title_with_id
end

it "exports future assignments" do
expect(headers).to include future_assignment.title_with_id
it "includes all assignments" do
expect(headers).to include @no_due_date_assignment.title_with_id,
@current_assignment.title_with_id,
@past_assignment.title_with_id,
@future_assignment.title_with_id
final_grade = rows[1]["Final Score"].try(:to_f)
expect(final_grade).to eq 25
end
end
end
Expand Down

0 comments on commit a4d9898

Please sign in to comment.