fix eye icon being used when submission not hidden

refs GRADE-2291

Test Plan
- Create an automatically posting assignment.
- Create a manually posting assignment.
- Load the student grades page.
- The student's score for the automatically posting assignment should
  not have the eye icon; it should show a "-".
- The student's score for the manually posting assignment should have
  the eye icon.
- This behavior should be consistent even when using What-If scores.

Change-Id: I4e7bd35ad0baf5a0a77065f42802c3e5fe3a0d7e
Reviewed-on: https://gerrit.instructure.com/201119
Tested-by: Jenkins
Reviewed-by: Keith Garner <kgarner@instructure.com>
Reviewed-by: Adrian Packel <apackel@instructure.com>
QA-Review: Gary Mei <gmei@instructure.com>
Product-Review: Keith Garner <kgarner@instructure.com>

app/jsx/grading/GradeSummary.js

@@ -196,6 +196,10 @@ const GradeSummary = {
     const score = GradeSummary.getOriginalScore($assignment)
     let title
 
+    if (score.numericalValue == null) {
+      score.formattedValue = GradeSummary.parseScoreText(null).formattedValue
+    }
+
     if ($assignment.data('muted')) {
       if (ENV.post_policies_enabled) {
         title = I18n.t('Hidden')

app/presenters/grade_summary_assignment_presenter.rb

@@ -52,13 +52,14 @@ class GradeSummaryAssignmentPresenter
     (submission.present? ? @summary.unread_submission_ids.include?(submission.id) : false)
   end
 
-  def posted_to_student?
-    assignment.course&.post_policies_enabled? ? submission.posted? : !assignment.muted?
+  def hide_grade_from_student?
+    return assignment.muted? unless assignment.course&.post_policies_enabled?
+    assignment.post_manually? ? !submission.posted? : (submission.graded? && !submission.posted?)
   end
 
   def graded?
     return false if submission.blank?
-    (submission.grade || submission.excused?) && posted_to_student?
+    (submission.grade || submission.excused?) && !hide_grade_from_student?
   end
 
   def is_letter_graded?
@@ -82,7 +83,7 @@ class GradeSummaryAssignmentPresenter
   end
 
   def has_no_score_display?
-    !posted_to_student? || submission.nil?
+    hide_grade_from_student? || submission.nil?
   end
 
   def original_points
@@ -99,12 +100,12 @@ class GradeSummaryAssignmentPresenter
 
   def has_scoring_details?
     return false unless submission&.score.present? && assignment&.points_possible.present?
-    assignment.points_possible > 0 && posted_to_student?
+    assignment.points_possible > 0 && !hide_grade_from_student?
   end
 
   def has_grade_distribution?
     return false if assignment&.points_possible.blank?
-    assignment.points_possible > 0 && posted_to_student?
+    assignment.points_possible > 0 && !hide_grade_from_student?
   end
 
   def has_rubric_assessments?

app/presenters/grade_summary_presenter.rb

@@ -234,7 +234,10 @@ class GradeSummaryPresenter
 
   def hidden_submissions?
     if @context.post_policies_enabled?
-      !submissions.all?(&:posted?)
+      submissions.any? do |sub|
+        return !sub.posted? if sub.assignment.post_manually?
+        sub.graded? && !sub.posted?
+      end
     else
       assignments.any?(&:muted?)
     end

app/views/gradebooks/grade_summary.html.erb

@@ -148,7 +148,7 @@
          can_view_distribution = can_do(@context, @current_user, :read_as_admin) || !assignment_presenter.hide_distribution_graphs?
       %>
     <tr class="<%= assignment_presenter.classes %>"
-        data-muted="<%= !assignment_presenter.posted_to_student? %>"
+        data-muted="<%= assignment_presenter.hide_grade_from_student? %>"
         id="<%= "submission_" + assignment.id.to_s %>"
         <% if assignment_presenter.excused? %>
           <% excused_label = t "This assignment is excused and will not be considered in the total calculation" %>
@@ -202,7 +202,7 @@
                 <span class="tooltip_wrap right" aria-hidden="true">
                   <% if @presenter.editable? || assignment.special_class %>
                     <span class="tooltip_text score_teaser">
-                      <% if !assignment_presenter.posted_to_student? %>
+                      <% if assignment_presenter.hide_grade_from_student? %>
                         <%= t(:student_mute_notification, "Instructor is working on grades") %>
                       <% elsif submission.try :pending_review? %>
                         <%= t(:grading_in_progress, "Instructor is working on grades") %>
@@ -214,7 +214,7 @@
                     </span>
                   <% end %>
                 </span>
-                <% if !assignment_presenter.posted_to_student? %>
+                <% if assignment_presenter.hide_grade_from_student? %>
                   <i class="<%= hidden_icon_class %>" title="<%= hidden_text %>"></i>
                 <% else %>
                   <span class="screenreader-only" role="button">
@@ -267,7 +267,7 @@
           <a href="#"
               class="toggle_final_grade_info tooltip"
               aria-label="<%= t('This assignment does not count toward the final grade.') %>"
-            <% if assignment.omit_from_final_grade? && assignment_presenter.posted_to_student? %>
+            <% if assignment.omit_from_final_grade? && !assignment_presenter.hide_grade_from_student? %>
                 aria-expanded="false"
                 aria-controls="final_grade_info_<%= assignment.id %>"
             <% else %>
@@ -285,7 +285,7 @@
             class="toggle_comments_link tooltip"
             aria-label="<%= t('Read comments') %>"
             aria-describedby="comment_count_for_assignment_<%= assignment.id %>"
-            <% if assignment_presenter.has_comments? && assignment_presenter.posted_to_student? %>
+            <% if assignment_presenter.has_comments? && !assignment_presenter.hide_grade_from_student? %>
               aria-expanded="false"
               aria-controls="comments_thread_<%= assignment.id %>"
               role="button"
@@ -495,7 +495,7 @@
           </tr>
       <tr id="comments_thread_<%= assignment.id %>" class="comments comments_thread <%= 'assignment_graded' if assignment_presenter.graded? %>" style="display: none;">
         <td colspan="6">
-          <% if assignment_presenter.posted_to_student? && assignment_presenter.has_comments? %>
+          <% if !assignment_presenter.hide_grade_from_student? && assignment_presenter.has_comments? %>
               <table class="score_details_table ic-Table ic-Table--condensed">
                 <thead>
                   <tr>

spec/javascripts/jsx/grading/GradeSummarySpec.js

@@ -1182,6 +1182,13 @@ QUnit.module('GradeSummary - Revert Score', hooks => {
     equal($assignment.find('.score_value').text(), '5')
   })
 
+  test('sets the .score value text to "-" when the submission was ungraded', function() {
+    $assignment.find('.original_points').text('')
+    $assignment.find('.original_score').text('')
+    GradeSummary.onScoreRevert($assignment)
+    equal($assignment.find('.score_value').text(), '-')
+  })
+
   test('sets the .grade html to the "muted assignment" indicator when the assignment is muted', function() {
     $assignment.data('muted', true)
     GradeSummary.onScoreRevert($assignment)

spec/presenters/grade_summary_assignment_presenter_spec.rb

@@ -237,31 +237,60 @@ describe GradeSummaryAssignmentPresenter do
     end
   end
 
-  describe "#posted_to_student?" do
+  describe "#hide_grade_from_student?" do
     context "when post policies are enabled" do
       before(:each) { @course.enable_feature!(:new_gradebook) }
       before(:each) { PostPolicy.enable_feature! }
 
-      it "returns true when the student's submission is posted" do
-        @submission.update!(posted_at: Time.zone.now)
-        expect(presenter).to be_posted_to_student
+      context "when assignment posts manually" do
+        before(:each) do
+          @assignment.ensure_post_policy(post_manually: true)
+        end
+
+        it "returns false when the student's submission is posted" do
+          @submission.update!(posted_at: Time.zone.now)
+          expect(presenter).not_to be_hide_grade_from_student
+        end
+
+        it "returns true when the student's submission is not posted" do
+          @submission.update!(posted_at: nil)
+          expect(presenter).to be_hide_grade_from_student
+        end
       end
 
-      it "returns false when the student's submission is not posted" do
-        @submission.update!(posted_at: nil)
-        expect(presenter).not_to be_posted_to_student
+      context "when assignment posts automatically" do
+        before(:each) do
+          @assignment.ensure_post_policy(post_manually: false)
+        end
+
+        it "returns false when the student's submission is posted" do
+          @submission.update!(posted_at: Time.zone.now)
+          expect(presenter).not_to be_hide_grade_from_student
+        end
+
+        it "returns false when the student's submission is not posted" do
+          @submission.update!(posted_at: nil)
+          expect(presenter).not_to be_hide_grade_from_student
+        end
+
+        it "returns true when the student's submission is graded and not posted" do
+          @assignment.grade_student(@student, grader: @teacher, score: 5)
+          @submission.reload
+          @submission.update!(posted_at: nil)
+          expect(presenter).to be_hide_grade_from_student
+        end
       end
     end
 
     context "when post policies are not enabled" do
-      it "returns true when the assignment is unmuted" do
+      it "returns false when the assignment is unmuted" do
         @assignment.unmute!
-        expect(presenter).to be_posted_to_student
+        expect(presenter).not_to be_hide_grade_from_student
       end
 
-      it "returns false when the assignment is muted" do
+      it "returns true when the assignment is muted" do
         @assignment.mute!
-        expect(presenter).not_to be_posted_to_student
+        expect(presenter).to be_hide_grade_from_student
       end
     end
   end

spec/presenters/grade_summary_presenter_spec.rb

@@ -608,6 +608,7 @@ describe GradeSummaryPresenter do
   describe "#hidden_submissions?" do
     let_once(:course) { Course.create! }
     let_once(:student) { course.enroll_student(User.create!, enrollment_state: :active).user }
+    let_once(:teacher) { course.enroll_teacher(User.create!, enrollment_state: :active).user }
 
     let_once(:assignment1) { course.assignments.create!(title: "a1") }
     let_once(:assignment2) { course.assignments.create!(title: "a2") }
@@ -615,15 +616,23 @@ describe GradeSummaryPresenter do
     let_once(:presenter) { GradeSummaryPresenter.new(course, student, student.id) }
 
     context "when post policies are enabled" do
-      before(:once) { course.enable_feature!(:new_gradebook) }
-      before(:once) { PostPolicy.enable_feature! }
+      before(:once) do
+        course.enable_feature!(:new_gradebook)
+        PostPolicy.enable_feature!
+        assignment1.ensure_post_policy(post_manually: true)
+        assignment2.ensure_post_policy(post_manually: false)
+      end
 
-      it "returns true if any of the student's submissions in the course is unposted" do
-        assignment2.post_submissions
+      it "returns true if any of the student's submissions in the course are graded and unposted" do
+        assignment1.grade_student(student, grader: teacher, score: 1)
 
         expect(presenter).to be_hidden_submissions
       end
 
+      it "returns true if any of the student's submissions are unposted and assignment posts manually" do
+        expect(presenter).to be_hidden_submissions
+      end
+
       it "returns false if all of the student's submissions in the course are posted" do
         assignment1.post_submissions
         assignment2.post_submissions

spec/views/gradebooks/grade_summary.html.erb_spec.rb

@@ -272,7 +272,11 @@ describe "/gradebooks/grade_summary" do
         PostPolicy.enable_feature!
       end
 
-      context "when a submission is unposted" do
+      context "when submission is hidden" do
+        before(:once) do
+          assignment.ensure_post_policy(post_manually: true)
+        end
+
         it "displays the 'hidden' icon" do
           render "gradebooks/grade_summary"
           expect(response).to have_tag(".assignment_score i[@class='icon-off']")
@@ -284,11 +288,15 @@ describe "/gradebooks/grade_summary" do
         end
       end
 
-      it "does not display the 'hidden' icon when a submission is posted" do
-        assignment.post_submissions
+      context "when submission is not hidden" do
+        before(:once) do
+          assignment.ensure_post_policy(post_manually: false)
+        end
 
-        render "gradebooks/grade_summary"
-        expect(response).not_to have_tag(".assignment_score i[@class='icon-off']")
+        it "does not display the 'hidden' icon" do
+          render "gradebooks/grade_summary"
+          expect(response).not_to have_tag(".assignment_score i[@class='icon-off']")
+        end
       end
     end