From 79ce88779e235b7c97402ca7ef509ad3ff8e6034 Mon Sep 17 00:00:00 2001
From: Han Yan <hyan@instructure.com>
Date: Mon, 1 Feb 2021 23:30:50 -0600
Subject: [PATCH] fix errors on dup draft records

closes DEMO-114
flag=none

test plan:
- make sure `Assignment Enhancements - Student` FF is enabled in Canvas
- create online assignments with text entry (or file uploads ...)
- in Canvas rails console, find the text entry draft (SubmissionDraft)
- create another SubmissionDraft records with the same submission and attempt
  (see new addtions in create_submission_draft_spec.rb, to bypass activerecord
  validations)
- add a new text entry draft without submitting
- update the text entry, draft is persited without errors

Change-Id: I47a9aa2420e6e2e9f200c0c66b60d80bf5ad8a3e
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/257920
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Stephen Kacsmark <skacsmark@instructure.com>
QA-Review: Mark McDermott <mmcdermott@instructure.com>
Product-Review: Han Yan <hyan@instructure.com>
---
 .../mutations/create_submission_draft.rb      | 20 +++++++
 .../mutations/create_submission_draft_spec.rb | 52 +++++++++++++++++++
 2 files changed, 72 insertions(+)

diff --git a/app/graphql/mutations/create_submission_draft.rb b/app/graphql/mutations/create_submission_draft.rb
index c43ca999d03..8ffed76545d 100644
--- a/app/graphql/mutations/create_submission_draft.rb
+++ b/app/graphql/mutations/create_submission_draft.rb
@@ -39,7 +39,10 @@ class Mutations::CreateSubmissionDraft < Mutations::BaseMutation
   argument :url, String, required: false
 
   field :submission_draft, Types::SubmissionDraftType, null: true
+
   def resolve(input:)
+    @retried ||= false
+
     submission = find_submission(input[:submission_id])
 
     file_ids = (input[:file_ids] || []).compact.uniq
@@ -73,6 +76,23 @@ class Mutations::CreateSubmissionDraft < Mutations::BaseMutation
   rescue ActiveRecord::RecordNotFound
     raise GraphQL::ExecutionError, 'not found'
   rescue ActiveRecord::RecordInvalid => invalid
+    # activerecord validation is not robust to race condition
+    #   multiple concurrent requests may penetrate activerecord validations
+    #   and save dup records for a combination of submission_id and attempt
+    # If it happened, following saves will be blocked by activerecord validation
+    # Ideally, an unique index should be defined, but with many existing records,
+    #   creating unique index may fail without cleaning data first.
+    if submission_draft.present?
+      submission_drafts = SubmissionDraft.where(
+        submission: submission_draft.submission,
+        submission_attempt: submission_draft.submission_attempt
+      )
+      if submission_drafts.count > 1 && !@retried
+        @retried = true
+        submission_drafts.where.not(id: submission_draft.id).destroy_all
+        retry
+      end
+    end
     errors_for(invalid.record)
   rescue SubmissionError => e
     return validation_error(e.message)
diff --git a/spec/graphql/mutations/create_submission_draft_spec.rb b/spec/graphql/mutations/create_submission_draft_spec.rb
index d660fb59143..7e4312bca83 100644
--- a/spec/graphql/mutations/create_submission_draft_spec.rb
+++ b/spec/graphql/mutations/create_submission_draft_spec.rb
@@ -332,4 +332,56 @@ RSpec.describe Mutations::CreateSubmissionDraft do
       result.dig(:data, :createSubmissionDraft, :submissionDraft, :submissionAttempt)
     ).to eq 1
   end
+
+  context 'when dup records exist' do
+    before do
+      # simulate creating a dup record
+      submission_draft = SubmissionDraft.where(
+        submission: @submission,
+        submission_attempt: @submission.attempt
+      ).first_or_create!
+      submission_draft.update_attribute(:submission_attempt, @submission.attempt + 1)
+      SubmissionDraft.where(
+        submission: @submission,
+        submission_attempt: @submission.attempt
+      ).first_or_create!
+      submission_draft.update_attribute(:submission_attempt, @submission.attempt)
+    end
+
+    it 'updates an existing submission draft without error' do
+      result = run_mutation(
+        active_submission_type: 'online_text_entry',
+        attempt: @submission.attempt,
+        body: 'some text body 123',
+        submission_id: @submission.id
+      )
+
+      drafts = SubmissionDraft.where(
+        submission: @submission, submission_attempt: @submission.attempt
+      )
+      expect(drafts.first.body).to eq 'some text body 123'
+      expect(
+        result.dig(:data, :createSubmissionDraft, :submissionDraft, :activeSubmissionType)
+      ).to eq 'online_text_entry'
+    end
+
+    it 'removes dup records' do
+      drafts = SubmissionDraft.where(
+        submission: @submission, submission_attempt: @submission.attempt
+      )
+
+      expect(drafts.count).to be 2
+      run_mutation(
+        active_submission_type: 'online_text_entry',
+        attempt: @submission.attempt,
+        body: 'some text body 123',
+        submission_id: @submission.id
+      )
+
+      drafts = SubmissionDraft.where(
+        submission: @submission, submission_attempt: @submission.attempt
+      )
+      expect(drafts.count).to be 1
+    end
+  end
 end