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>
This commit is contained in:
parent
cf13cec184
commit
79ce88779e
|
@ -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)
|
||||
|
|
|
@ -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
|
||||
|
|
Loading…
Reference in New Issue