allowed_attempts enforcement

* Adds a validation on the backend to
  prevent a new submission when necessary
* Adds indicators to the UI that are shown
  when allowed_attempts are set on an
  assignment

Test Plan:

* Set allowed_attempts and extra_attempts for an
  assignment and student, using the instructions from
  the test plan on PFS-11440
* Validate that the UI is updated to show Attempts and
  Allowed Attempts in the following locations:
  - Student assignment submission page
  - Teacher assignment page
  - Student Assignment Details page
* Create multiple submissions as a student until you
  reach the max allowed for that student. Validate that
  the "Submit Assignment" button is now disabled.
* Use the API to try and submit an assignment (since
  the UI won't let you):
  https://canvas.instructure.com/doc/api/submissions.html#method.submissions.create
  Validate that the submission errors out and is not created.
* Validate that if "allowed_attempts" is not set on an assignment,
  that there are no changes to the UI on the pages listed above.

refs PFS-11441

Change-Id: I76a81d4d3c58ca5266645a3cb7b3d3cbc5bedb22
Reviewed-on: https://gerrit.instructure.com/173320
Tested-by: Jenkins
Product-Review: Bryce Stevenson <bstevenson@instructure.com>
QA-Review: Marisa Jense <mjense@instructure.com>
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>

app/helpers/assignments_helper.rb

@@ -65,7 +65,13 @@ module AssignmentsHelper
     if assignment.expects_submission? && can_do(assignment, user, :submit)
       submit_text = user_submission.try(:has_submission?) ? I18n.t("Re-submit Assignment") : I18n.t("Submit Assignment")
       late = user_submission.try(:late?) ? "late" : ""
-      link_to(submit_text, '#', :role => "button", :class => "Button Button--primary submit_assignment_link #{late}")
+      link_to(
+        submit_text,
+        '#',
+        :role => "button",
+        :class => "Button Button--primary submit_assignment_link #{late}",
+        :disabled => user_submission && user_submission.attempts_left == 0,
+      )
     end
   end
 

app/models/submission.rb

@@ -58,6 +58,8 @@ class Submission < ActiveRecord::Base
     }.freeze
   }.freeze
 
+  SUBMISSION_TYPES_GOVERNED_BY_ALLOWED_ATTEMPTS = %w[online_upload online_url online_text_entry].freeze
+
   attr_readonly :assignment_id
   attr_accessor :visible_to_user,
                 :skip_grade_calc,
@@ -119,6 +121,7 @@ class Submission < ActiveRecord::Base
   validates :late_policy_status, inclusion: ['none', 'missing', 'late'], allow_nil: true
   validate :ensure_grader_can_grade
   validate :extra_attempts_can_only_be_set_on_online_uploads
+  validate :ensure_attempts_are_in_range
 
   scope :active, -> { where("submissions.workflow_state <> 'deleted'") }
   scope :for_enrollments, -> (enrollments) { where(user_id: enrollments.select(:user_id)) }
@@ -1518,14 +1521,26 @@ class Submission < ActiveRecord::Base
 
   def extra_attempts_can_only_be_set_on_online_uploads
     return true unless changes.key?("extra_attempts") && assignment
-    allowed_submission_types = %w[online_upload online_url online_text_entry]
-    return true if (assignment.submission_types.split(",") & allowed_submission_types).any?
+    return true if (assignment.submission_types.split(",") & SUBMISSION_TYPES_GOVERNED_BY_ALLOWED_ATTEMPTS).any?
 
-    error_msg = 'extra_attempts can only be set on submissions for an assignment with a type of online_upload, online_url, or online_text_entry'
+    error_msg = 'can only be set on submissions for an assignment with a type of online_upload, online_url, or online_text_entry'
     errors.add(:extra_attempts, error_msg)
     false
   end
 
+  def attempts_left
+    return nil if self.assignment.allowed_attempts.nil? || self.assignment.allowed_attempts < 0
+    [0, self.assignment.allowed_attempts + (self.extra_attempts || 0) - (self.attempt || 0)].max
+  end
+
+  def ensure_attempts_are_in_range
+    return true unless changes.key?("submitted_at") && assignment
+    return true unless (assignment.submission_types.split(",") & SUBMISSION_TYPES_GOVERNED_BY_ALLOWED_ATTEMPTS).any?
+    return true if attempts_left.nil? || attempts_left > 0
+    errors.add(:attempt, 'you have reached the maximum number of allowed attempts for this assignment')
+    false
+  end
+
   def can_autograde?
     result = GRADE_STATUS_MESSAGES_MAP[can_autograde_symbolic_status]
     result ||= { status: false, message: I18n.t('Cannot autograde at this time') }

app/stylesheets/bundles/submission.scss

@@ -62,6 +62,15 @@
 }
 .submission-details-header__info {
   margin-top: $ic-sp / 3;
+  display: flex;
+
+  .submission-details-header__attempts_info {
+    margin: 0px 0px 0px 40px;
+
+    > span {
+      margin: 0px 20px 0px 0px;
+    }
+  }
 }
 .submission-details-header__time {
   margin-#{direction(right)}: $ic-sp / 4;

app/stylesheets/pages/assignments/_assignments.scss

@@ -139,7 +139,7 @@
       padding-#{direction(right)}: 0.5em;
     }
     .value {
-      padding-#{direction(right)}: 2.5em;
+      padding-#{direction(right)}: 2.0em;
     }
   }
   .form-horizontal.display-only .control-group {

app/views/assignments/show.html.erb

@@ -151,6 +151,16 @@
           </li>
         <% end %>
       <% end %>
+      <% if @assignment.allowed_attempts&.> 0 %>
+        <li>
+          <span class='title'><%= t :attempts, 'Attempts' %></span>
+          <span class='value'><%= @current_user_submission&.attempt || 0 %></span>
+        </li>
+        <li>
+          <span class='title'><%= t :allowed_attempts, 'Allowed Attempts' %></span>
+          <span class='value'><%= @assignment.allowed_attempts + (@current_user_submission&.extra_attempts || 0) %></span>
+        </li>
+      <% end %>
       <%= render :partial => "shared/available_dates", :locals => {:association => @assignment} %>
       <div class="clear"></div>
     </ul>
@@ -192,6 +202,14 @@
             </div>
           </div>
         <% end -%>
+        <% if @assignment.allowed_attempts&.> 0 %>
+          <div class="control-group">
+            <div class="control-label"><%= t :allowed_attempts, 'Allowed Attempts' %></div>
+            <div class="controls">
+              <span class="value"><%= @assignment.allowed_attempts %></span>
+            </div>
+          </div>
+        <% end -%>
         <% if turnitin_active? %>
           <div class="control-group">
             <div class="control-label"><%= t :turnitin, 'Turnitin' %></div>

app/views/submissions/show.html.erb

@@ -165,6 +165,16 @@
               <span class="submission-missing-pill"></span>
             <% end %>
           <% end %>
+          <% if @assignment.allowed_attempts&.> 0 %>
+            <div class="submission-details-header__attempts_info">
+              <span class="bold"><%= t :attempts, 'Attempts' %></span>
+              <span><%= @submission.attempt || 0 %></span>
+            </div>
+            <div class="submission-details-header__attempts_info">
+              <span class="bold"><%= t :allowed_attempts, 'Allowed Attempts' %></span>
+              <span><%= @assignment.allowed_attempts + (@submission.extra_attempts || 0) %></span>
+            </div>
+          <% end %>
         </div>
       </div>
       <div class="ic-Action-header__Secondary ic-Action-header__Secondary--auto">
@@ -175,7 +185,7 @@
           </a>
         <% end %>
         <% if @assignment.expects_submission? && can_do(@assignment, @current_user, :submit) && @current_user == @submission.user %>
-          <a href="<%= context_url(@context, :context_assignment_url, @assignment.id) %>#submit" class="Button Button--primary" >
+          <a href="<%= context_url(@context, :context_assignment_url, @assignment.id) %>#submit" class="Button Button--primary" <%= 'disabled' if @submission&.attempts_left == 0 %>>
             <%= @submission.has_submission? ? t('links.resubmit', "Re-submit Assignment") : t('links.submit', "Submit Assignment") %>
           </a>
         <% end %>

spec/helpers/assignments_helper_spec.rb

@@ -119,4 +119,43 @@ describe AssignmentsHelper do
       expect(turnitin_active?).to be_falsey
     end
   end
+
+  describe "#assignment_submission_button" do
+    before do
+      student_in_course
+      assignment_model(course: @course)
+      @assignment.update_attribute(:submission_types, "online_upload")
+      allow(self).to receive(:can_do).and_return true
+    end
+
+    context "the submission has 0 attempts left" do
+      it "returns a disabled button" do
+        @assignment.update_attribute(:allowed_attempts, 2)
+        submission = @assignment.submissions.find_by!(user_id: @student)
+        submission.update_attribute(:attempt, 2)
+        button = assignment_submission_button(@assignment, @student, submission)
+        expect(button["disabled"]).to eq("disabled")
+      end
+    end
+
+    context "the submission has > 0 attempts left" do
+      it "returns an enabled button" do
+        @assignment.update_attribute(:allowed_attempts, 2)
+        submission = @assignment.submissions.find_by!(user_id: @student)
+        submission.update_attribute(:attempt, 1)
+        button = assignment_submission_button(@assignment, @student, submission)
+        expect(button["disabled"]).to be_nil
+      end
+    end
+
+    context "the submission has unlimited attempts" do
+      it "returns an enabled button" do
+        @assignment.update_attribute(:allowed_attempts, -1)
+        submission = @assignment.submissions.find_by!(user_id: @student)
+        submission.update_attribute(:attempt, 3)
+        button = assignment_submission_button(@assignment, @student, submission)
+        expect(button["disabled"]).to be_nil
+      end
+    end
+  end
 end

spec/models/submission_spec.rb

@@ -5853,4 +5853,110 @@ describe Submission do
       end
     end
   end
+
+  describe '#ensure_attempts_are_in_range' do
+    let(:submission) { @assignment.submissions.first }
+
+    context 'the assignment is of a type that is restricted by attempts' do
+      before do
+        @assignment.allowed_attempts = 10
+        @assignment.submission_types = 'online_upload'
+        @assignment.save!
+      end
+
+      context 'attempts_left <= 0' do
+        before do
+          submission.attempt = 10
+          submission.save!
+        end
+
+        context 'the submitted_at changed' do
+          it 'is invalid' do
+            submission.submitted_at = Time.zone.now
+            expect(submission).to_not be_valid
+          end
+        end
+
+        context 'the submitted_at did not change' do
+          it 'is valid' do
+            expect(submission).to be_valid
+          end
+        end
+      end
+    end
+
+    context 'the assignment is of a type that is not restricted by attempts' do
+      before do
+        @assignment.allowed_attempts = 10
+        @assignment.submission_types = 'online_discussion'
+        @assignment.save!
+        submission.attempt = 10
+        submission.save!
+      end
+
+      it 'is valid' do
+        expect(submission).to be_valid
+      end
+    end
+  end
+
+  describe '#attempts_left' do
+    let(:submission) { @assignment.submissions.first }
+
+    context 'allowed_attempts is set to a number > 0 on the assignment' do
+      before do
+        @assignment.allowed_attempts = 10
+        @assignment.submission_types = 'online_upload'
+        @assignment.save!
+      end
+
+      context 'the submission has extra_attempts set to a value > 0' do
+        it 'returns assignment.allowed_attempts + submission.extra_attempts - submission.attempt' do
+          submission.extra_attempts = 12
+          submission.attempt = 6
+          submission.save!
+          expect(submission.attempts_left).to eq(10 + 12 - 6)
+        end
+
+        it 'correctly recalculates when allowed_attempts and extra_attempts change' do
+          submission.extra_attempts = 12
+          submission.attempt = 22
+          submission.save!
+          expect(submission.attempts_left).to eq(0)
+          @assignment.allowed_attempts = 11
+          @assignment.save!
+          expect(submission.attempts_left).to eq(1)
+          submission.extra_attempts = 13
+          submission.save!
+          expect(submission.attempts_left).to eq(2)
+        end
+
+        it 'will never return negative values' do
+          submission.attempt = 1000
+          submission.save!
+          expect(submission.attempts_left).to eq(0)
+        end
+      end
+
+      context 'the submission has extra_attempts set to nil' do
+        it 'returns allowed_attempts from the assignment' do
+          submission.extra_attempts = nil
+          submission.attempt = 6
+          submission.save!
+          expect(submission.attempts_left).to eq(10 - 6)
+        end
+      end
+    end
+
+    context 'allowed_attempts is set to nil or -1 on the assignment' do
+      it 'returns nil' do
+        @assignment.allowed_attempts = nil
+        @assignment.save!
+        expect(submission.attempts_left).to be_nil
+        @assignment.allowed_attempts = -1
+        @assignment.save!
+        expect(submission.attempts_left).to be_nil
+      end
+    end
+  end
 end

spec/selenium/assignments/assignment_allowed_attempts_spec.rb

@@ -0,0 +1,106 @@
+#
+# Copyright (C) 2018 - present Instructure, Inc.
+#
+# This file is part of Canvas.
+#
+# Canvas is free software: you can redistribute it and/or modify it under
+# the terms of the GNU Affero General Public License as published by the Free
+# Software Foundation, version 3 of the License.
+#
+# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
+# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
+# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
+# details.
+#
+# You should have received a copy of the GNU Affero General Public License along
+# with this program. If not, see <http://www.gnu.org/licenses/>.
+
+require_relative '../common'
+require_relative '../helpers/assignments_common'
+
+describe "allowed_attempts feature for assignments" do
+  include_context "in-process server selenium tests"
+  include AssignmentsCommon
+
+  context "as a student" do
+    before do
+      course_with_student_logged_in
+    end
+
+    describe "the assignments page" do
+      context "with allowed_attempts on the assignment" do
+        before do
+          @assignment = @course.assignments.create!({ name: "Test Assignment", allowed_attempts: 2 })
+          @assignment.update_attribute(:submission_types, "online_text_entry")
+        end
+
+        it "prevents submitting if the student has exceeded the max number of attempts" do
+          submission = @assignment.submit_homework(@student, { body: "blah" })
+          submission.update_attributes(attempt: 2, submission_type: "online_text_entry")
+          get "/courses/#{@course.id}/assignments/#{@assignment.id}"
+          expect(f(".student-assignment-overview")).to include_text("Allowed Attempts")
+          expect(f('.submit_assignment_link')).to be_disabled
+        end
+
+        it "allows submitting if the student has not exceeded the max number of attempts" do
+          get "/courses/#{@course.id}/assignments/#{@assignment.id}"
+          expect(f(".student-assignment-overview")).to include_text("Allowed Attempts")
+          expect(f('.submit_assignment_link')).to_not be_disabled
+        end
+      end
+
+      context "without allowed_attempts on the assignment" do
+        before do
+          @assignment = @course.assignments.create!({ name: "Test Assignment", allowed_attempts: -1 })
+          @assignment.update_attribute(:submission_types, "online_text_entry")
+          @assignment.submit_homework(@student, { body: "blah" })
+        end
+
+        it "does not show the attempt data" do
+          get "/courses/#{@course.id}/assignments/#{@assignment.id}"
+          expect(f(".student-assignment-overview")).to_not include_text("Allowed Attempts")
+          expect(f('.submit_assignment_link')).to_not be_disabled
+        end
+      end
+    end
+
+    describe "the assignments detail page" do
+      context "with allowed_attempts on the assignment" do
+        before do
+          @assignment = @course.assignments.create!({ name: "Test Assignment", allowed_attempts: 2 })
+          @assignment.update_attribute(:submission_types, "online_text_entry")
+          @submission = @assignment.submit_homework(@student, { body: "blah" })
+          @submission.update_attributes(submission_type: "online_text_entry")
+        end
+
+        it "prevents submitting if the student has exceeded the max number of attempts" do
+          @submission.update_attributes(attempt: 2)
+          get "/courses/#{@course.id}/assignments/#{@assignment.id}/submissions/#{@student.id}"
+          expect(f(".submission-details-header__info")).to include_text("Allowed Attempts")
+          expect(fln("Re-submit Assignment")).to be_disabled
+        end
+
+        it "allows submitting if the student has not exceeded the max number of attempts" do
+          get "/courses/#{@course.id}/assignments/#{@assignment.id}/submissions/#{@student.id}"
+          expect(f(".submission-details-header__info")).to include_text("Allowed Attempts")
+          expect(fln("Re-submit Assignment")).to_not be_disabled
+        end
+      end
+
+      context "without allowed_attempts on the assignment" do
+        before do
+          @assignment = @course.assignments.create!({ name: "Test Assignment", allowed_attempts: -1 })
+          @assignment.update_attribute(:submission_types, "online_text_entry")
+          @submission = @assignment.submit_homework(@student, { body: "blah" })
+          @submission.update_attributes(attempt: 2, submission_type: "online_text_entry")
+        end
+
+        it "does not show the attempt data and allows submitting" do
+          get "/courses/#{@course.id}/assignments/#{@assignment.id}/submissions/#{@student.id}"
+          expect(f(".submission-details-header__info")).to_not include_text("Allowed Attempts")
+          expect(fln("Re-submit Assignment")).to_not be_disabled
+        end
+      end
+    end
+  end
+end