diff --git a/app/jsx/grading/GradeSummary.js b/app/jsx/grading/GradeSummary.js index 36937f39acc..e224139424a 100644 --- a/app/jsx/grading/GradeSummary.js +++ b/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') diff --git a/app/presenters/grade_summary_assignment_presenter.rb b/app/presenters/grade_summary_assignment_presenter.rb index da418d39c73..d5811485da4 100644 --- a/app/presenters/grade_summary_assignment_presenter.rb +++ b/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? diff --git a/app/presenters/grade_summary_presenter.rb b/app/presenters/grade_summary_presenter.rb index a448f03cb9c..f3a093bc4b6 100644 --- a/app/presenters/grade_summary_presenter.rb +++ b/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 diff --git a/app/views/gradebooks/grade_summary.html.erb b/app/views/gradebooks/grade_summary.html.erb index aae243dfbdb..2d8a43b0e60 100644 --- a/app/views/gradebooks/grade_summary.html.erb +++ b/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? %> " <% if assignment_presenter.excused? %> <% excused_label = t "This assignment is excused and will not be considered in the total calculation" %> @@ -202,7 +202,7 @@ - <% if !assignment_presenter.posted_to_student? %> + <% if assignment_presenter.hide_grade_from_student? %> <% else %> @@ -267,7 +267,7 @@ + <% 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 @@ - <% if assignment_presenter.posted_to_student? && assignment_presenter.has_comments? %> + <% if !assignment_presenter.hide_grade_from_student? && assignment_presenter.has_comments? %> diff --git a/spec/javascripts/jsx/grading/GradeSummarySpec.js b/spec/javascripts/jsx/grading/GradeSummarySpec.js index 0d5662c7fe9..eeec6090cd7 100644 --- a/spec/javascripts/jsx/grading/GradeSummarySpec.js +++ b/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) diff --git a/spec/presenters/grade_summary_assignment_presenter_spec.rb b/spec/presenters/grade_summary_assignment_presenter_spec.rb index e0cf8fc5308..d13b4f954bf 100644 --- a/spec/presenters/grade_summary_assignment_presenter_spec.rb +++ b/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 diff --git a/spec/presenters/grade_summary_presenter_spec.rb b/spec/presenters/grade_summary_presenter_spec.rb index 29b79e8b994..6b9bf266972 100644 --- a/spec/presenters/grade_summary_presenter_spec.rb +++ b/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 diff --git a/spec/views/gradebooks/grade_summary.html.erb_spec.rb b/spec/views/gradebooks/grade_summary.html.erb_spec.rb index 494d40db225..90ce3022c8f 100644 --- a/spec/views/gradebooks/grade_summary.html.erb_spec.rb +++ b/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 @@ -324,7 +332,7 @@ describe "/gradebooks/grade_summary" do view_context(course, student) assign(:presenter, GradeSummaryPresenter.new(course, student, nil)) end - + context "when comments exist" do before (:each) do submission = assignment.submission_for_student(student)