Basic outcomes: set submitted at but don't increment attempt

fixes PLAT-3726

Test Plan:
- create a course and enroll a student
- create an assignment that uses LTI outcomes service
  (but does not send resultData)
- submit to the assignment as the student
- verify the submitted_at timestamp is equal to the time
  of the submission
- verify the attempt number of the submission is 1
Change-Id: I2b638987c42e7815b2b5d450999bb9ef1004ea24
Reviewed-on: https://gerrit.instructure.com/164285
Tested-by: Jenkins
Reviewed-by: Marc Alan Phillips <mphillips@instructure.com>
QA-Review: August Thornton <august@instructure.com>
Product-Review: Jesse Poulos <jpoulos@instructure.com>

lib/basic_lti/basic_outcomes.rb

@@ -281,7 +281,7 @@ to because the assignment has no points possible.
           submission_hash[:grade] = (new_score >= 1 ? "pass" : "fail") if assignment.grading_type == "pass_fail"
           submission_hash[:grader_id] = -_tool.id
           @submission = assignment.grade_student(user, submission_hash).first
-          if @submission.submitted_at.blank? && submission_hash[:submission_type] == 'external_tool' && submitted_at_date.nil?
+          if submission_hash[:submission_type] == 'external_tool' && submitted_at_date.nil?
             @submission.submitted_at = Time.zone.now
           end
         end
@@ -291,6 +291,7 @@ to because the assignment has no points possible.
         end
 
         if @submission
+          @submission.attempt -= 1 if @submission.attempt.try(:'>', 0) && @submission.submitted_at_changed?
           @submission.save
         else
           self.code_major = 'failure'

spec/lib/basic_lti/basic_outcomes_spec.rb

@@ -276,14 +276,22 @@ describe BasicLTI::BasicOutcomes do
       expect(request.body).to eq '<replaceResultResponse />'
     end
 
-    it "Does not change the attempt number" do
+    it "Does not increment the attempt number" do
       xml.css('resultData').remove
-      now = Time.now.utc
       BasicLTI::BasicOutcomes.process_request(tool, xml)
       submission = assignment.submissions.where(user_id: @user.id).first
       expect(submission.attempt).to eq 1
     end
 
+    it "sets 'submitted_at' to the current time when result data is not sent" do
+      xml.css('resultData').remove
+      Timecop.freeze do
+        BasicLTI::BasicOutcomes.process_request(tool, xml)
+        submission = assignment.submissions.where(user_id: @user.id).first
+        expect(submission.submitted_at).to eq Time.zone.now
+      end
+    end
+
     context 'with submitted_at details' do
       let(:timestamp) { 1.day.ago.iso8601(3) }
 
@@ -297,6 +305,16 @@ describe BasicLTI::BasicOutcomes do
         expect(submission.submitted_at.iso8601(3)).to eq timestamp
       end
 
+      it "does not increment the submision count" do
+        xml.css('resultData').remove
+        xml.at_css('imsx_POXBody > replaceResultRequest').add_child(
+          "<submissionDetails><submittedAt>#{timestamp}</submittedAt></submissionDetails>"
+        )
+        BasicLTI::BasicOutcomes.process_request(tool, xml)
+        submission = assignment.submissions.where(user_id: @user.id).first
+        expect(submission.attempt).to eq 1
+      end
+
       it "sets submitted_at to submitted_at details if resultData is present" do
         xml.at_css('imsx_POXBody > replaceResultRequest').add_child(
           "<submissionDetails><submittedAt>#{timestamp}</submittedAt></submissionDetails>"