gradebook exporter: don't show scores for anonymized submissions

closes EVAL-1314
flag=none

Test Plan:
1. Create an anonymous assignment. Grade a few students anonymously in
   SpeedGrader. Don't post grades yet.
2. Export the Gradebook.
3. Open the exported CSV. Verify that all scores for the anonymous
   assignment show as "N/A" and the header shows "Manual Posting (scores
   hidden from instructors)".
4. Make some changes to scores in the CSV, making sure to change at
   least one score for the anonymous unposted assignment, and one score
   for an active, non-anonymous assignment.
5. Import the changed CSV. Verify the changes to the anonymous
   assignment are ignored, and the changes to the non-anonymous
   assignment are not ignored.
6. Post grades for the anonymous assignment.
7. Re-export the Gradebook.
8. Open the exported CSV. Verify that all scores for the anonymous
   assignment are now shown.
9. Make some changes to scores in the CSV for the anonymous assignment.
10. Import the changed CSV. Verify the changes to the anonymous
    assignment are no longer ignored.

* Edge case: try steps 1-10 but with your locale set to anything other
  than English. This will ensure the Gradebook Importer logic can
  handle and identify headers such as, "Manual Posting (scores cachés
  aux instructeurs)". Note that "Manual Posting" will never be
  translated (always English), but the content in parentheses will be
  translated. This is expected.

Change-Id: Ief17a06bd97c15b4138531030ed50ba19f2e03fa
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/253151
Tested-by: Service Cloud Jenkins <[email protected]>
QA-Review: Kai Bjorkman <[email protected]>
Product-Review: Syed Hussain <[email protected]>
Reviewed-by: Adrian Packel <[email protected]>
Reviewed-by: Gary Mei <[email protected]>

lib/gradebook_exporter.rb

@@ -109,6 +109,7 @@ def csv_data
     calc.submissions.each { |s| submissions[[s.user_id, s.assignment_id]] = s }
 
     assignments = select_in_grading_period calc.gradable_assignments
+    Assignment.preload_unposted_anonymous_submissions(assignments)
 
     ActiveRecord::Associations::Preloader.new.preload(assignments, :assignment_group)
     assignments.sort_by! do |a|
@@ -170,8 +171,8 @@ def csv_data
           row << nil
         end
 
-        # This is not translated since we look for this exact string when we upload to gradebook.
-        row.concat(assignments.map { |assignment| show_as_hidden?(assignment) ? hidden_assignment_text : nil })
+        hidden_assignments_text = assignments.map { |assignment| hidden_assignment_text(assignment) }
+        row.concat(hidden_assignments_text)
 
         if should_show_totals
           row.concat([nil] * group_filler_length)
@@ -250,7 +251,7 @@ def csv_data
           student = student_enrollment.user
           student_sections = student_section_names[student.id].sort.to_sentence
           student_submissions = assignments.map do |a|
-            if visible_assignments[a.id].include? student.id
+            if visible_assignments[a.id].include?(student.id) && !a.unposted_anonymous_submissions?
               submission = submissions[[student.id, a.id]]
               if submission.try(:excused?)
                 "EX"
@@ -416,8 +417,15 @@ def show_as_hidden?(assignment)
     assignment.post_manually?
   end
 
-  def hidden_assignment_text
-    "Manual Posting"
+  def hidden_assignment_text(assignment)
+    return nil unless show_as_hidden?(assignment)
+
+    # We don't translate "Manual Posting" since we look for this exact string when we upload to gradebook.
+    if assignment.unposted_anonymous_submissions?
+      I18n.t("%{manual_posting} (scores hidden from instructors)", { manual_posting: "Manual Posting" })
+    else
+      "Manual Posting"
+    end
   end
 
   def include_grading_period_in_headers?

lib/gradebook_importer.rb

@@ -108,12 +108,15 @@ def initialize(upload=nil, attachment=nil, user=nil, progress=nil)
   def parse!
     # preload a ton of data that presumably we'll be querying
     @context.preload_user_roles!
-    @all_assignments = @context.assignments
-      .preload({ context: :account })
-      .published
-      .gradeable
-      .select(ASSIGNMENT_PRELOADED_FIELDS)
-      .index_by(&:id)
+    @all_assignments = @context.assignments.
+      preload({ context: :account }).
+      published.
+      gradeable.
+      select(ASSIGNMENT_PRELOADED_FIELDS).
+      to_a
+
+    Assignment.preload_unposted_anonymous_submissions(@all_assignments)
+    @all_assignments = @all_assignments.index_by(&:id)
     @all_students = @context.all_students
       .select(['users.id', :name, :sortable_name, 'users.updated_at'])
       .index_by(&:id)
@@ -657,7 +660,7 @@ def check_for_non_student_row(row)
       return true
     end
 
-    if row.compact.all? { |column| column =~ /^\s*(Muted|Manual Posting)?\s*$/i }
+    if row.compact.all? { |column| column =~ /^\s*(Muted|Manual Posting.*)?\s*$/i }
       # This row indicates muted assignments (or manually posted assignments if
       # post policies is enabled) and should not be processed at all
       return true
@@ -748,6 +751,9 @@ def gradeable?(submission:, is_admin: false)
     # `submission#grants_right?` will check if the user
     # is an admin, but if we've pre-loaded that value already
     # to avoid an N+1, check that first.
+    assignment = @all_assignments[submission.assignment_id]
+    return false if assignment&.unposted_anonymous_submissions?
+
     is_admin || submission.grants_right?(@user, :grade)
   end
 

spec/lib/gradebook_exporter_spec.rb

@@ -279,6 +279,12 @@ def exporter(opts = {})
           expect(manual_posting_row[manual_header]).to eq "Manual Posting"
         end
 
+        it "has a special designation for anonymous unposted assignments" do
+          course_with_student(course: @course, active_all: true)
+          manual_assignment.update!(anonymous_grading: true)
+          expect(manual_posting_row[manual_header]).to eq "Manual Posting (scores hidden from instructors)"
+        end
+
         it "emits an empty value for auto-posted assignments" do
           expect(manual_posting_row[auto_header]).to be nil
         end
@@ -655,6 +661,31 @@ def exporter(opts = {})
     end
   end
 
+  context "when a course has anonymous assignments" do
+    before(:each) do
+      @student = User.create!
+      student_in_course(user: @student, course: @course, active_all: true)
+      @assignment = @course.assignments.create!(title: "Anon Assignment", points_possible: 10, anonymous_grading: true)
+      @assignment.ensure_post_policy(post_manually: true)
+      @assignment.grade_student(@student, grade: 8, grader: @teacher)
+    end
+
+    let(:submission_score) do
+      csv = GradebookExporter.new(@course, @teacher, {}).to_csv
+      rows = CSV.parse(csv, headers: true)
+      rows[2]["Anon Assignment (#{@assignment.id})"]
+    end
+
+    it "shows 'N/A' for submission scores in the export when the assignment is unposted" do
+      expect(submission_score).to eq "N/A"
+    end
+
+    it "shows actual submission scores in the export when the assignment is posted" do
+      @assignment.post_submissions
+      expect(submission_score).to eq "8.00"
+    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) }

spec/lib/gradebook_importer_spec.rb

@@ -99,6 +99,17 @@
       end.to raise_error(Delayed::RetriableError)
     end
 
+    it "ignores the line denoting manually posted anonymous assignments if present" do
+      importer_with_rows(
+        'Student,ID,Section,Assignment 1,Final Score',
+        'Points Possible,,,10,',
+        ', ,,Manual Posting (scores cachés aux instructeurs),',
+        '"Blend, Bill",6,My Course,-,',
+        '"Farner, Todd",4,My Course,-,'
+      )
+      expect(@gi.students.length).to eq 2
+    end
+
     context 'when dealing with a file containing semicolon field separators' do
       context 'with interspersed commas to throw you off' do
         before(:each) do
@@ -673,6 +684,32 @@
     expect(submission['assignment_id']).to eq @assignment1.id
   end
 
+  context "anonymous assignments" do
+    before(:each) do
+      @student = User.create!
+      course_with_student(user: @student, active_all: true)
+      @assignment = @course.assignments.create!(name: "Assignment 1", anonymous_grading: true, points_possible: 10)
+      @assignment.grade_student(@student, grade: 8, grader: @teacher)
+    end
+
+    it "does not include grade changes for anonymous unposted assignments" do
+      importer_with_rows(
+        "Student,ID,Section,Assignment 1",
+        ",#{@student.id},,10"
+      )
+      expect(@gi.assignments).to be_empty
+    end
+
+    it "includes grade changes for anonymous posted assignments" do
+      @assignment.post_submissions
+      importer_with_rows(
+        "Student,ID,Section,Assignment 1",
+        ",#{@student.id},,10"
+      )
+      expect(@gi.assignments).not_to be_empty
+    end
+  end
+
   context "custom gradebook columns" do
     let(:uploaded_custom_columns) { @gi.upload.gradebook["custom_columns"] }
     let(:uploaded_student_custom_column_data) do