SubmissionDraft model initial commit

This is the first part of submission drafts that only account for file
uploads. Other types (body, media comments, etc) will be coming in
future commits.

Fixes COMMS-2050

Test Plan:
  - Migrate your database with this patchset checked out
    `bundle exec rake db:migrate:up VERSION=20190503151652`
  - Have some attachments in your database
  - Have a course with a studnet in it
  - Create and publish a new assignment in this course
  - Open the rails console, and notice that the submission for the
    student does not have any submission drafts associated with it

    ```
    s = Submission.last
    s.submission_drafts.empty?  # Should be true
    ```

  - Create a new submission draft with some attachments in the console,
    and notice that you can see the draft and attachmetns from the
    submission

    ```
    draft = SubmissionDraft.new(submission_attempt: 0)
    submission = Submission.last
    submission.submission_drafts << draft
    attachments = Attachment.limit 3
    draft.attachments = attachments

    submission.submission_drafts # Should show the new draft
    submission.submission_drafts.first.attachments # The attachments
    ```

  - From the web ui, submit the assignment as the student.

  - Back in the rails console, and notice that the submission draft
    has now been deleted

    ```
    submission.reload
    submission.submission_drafts # Should now be []
    SubmissionDraft.count # Should be 0
    SubmissionDraftAttachment.count # Should be 0
    ```

  - Notice that the attachments in the rails console have not been
    deleted.

    ```
    attachments.each(&:reload)
    attachments.map(&:persisted?) # Should all be true
    ```

  - Make sure you cannot create a draft that has a higher attempt then
    the  root submission:

    ```
    SubmissionDraft.create!(
      submisison: submission,
      submission_attempt: 0
    )  # Should raise a RecordInvalid error
    ```

  - Undo the database migration until this gets merged.
    `bundle exec rake db:migrate:down VERSION=20190503151652`

Change-Id: Ibedda3719e582b3aa67f049699603f3fc7cbc145
Reviewed-on: https://gerrit.instructure.com/192212
Tested-by: Jenkins
QA-Review: Matthew Lemon <mlemon@instructure.com>
Reviewed-by: Rob Orton <rob@instructure.com>
Product-Review: Landon Gilbert-Bland <lbland@instructure.com>
This commit is contained in:
Landon Gilbert-Bland 2019-05-03 10:38:53 -06:00
parent bb5a3ac234
commit 7c59c834b5
11 changed files with 363 additions and 6 deletions

View File

@ -2843,7 +2843,7 @@ class CoursesController < ApplicationController
pg_scope.delete_all
OriginalityReport.where(:submission_id => @fake_student.all_submissions).delete_all
AnonymousOrModerationEvent.where(submission: @fake_student.all_submissions).destroy_all
@fake_student.all_submissions.preload(:all_submission_comments, :lti_result, :versions).destroy_all
@fake_student.all_submissions.preload(:all_submission_comments, :submission_drafts, :lti_result, :versions).destroy_all
@fake_student.quiz_submissions.each{|qs| qs.events.destroy_all}
@fake_student.quiz_submissions.destroy_all

View File

@ -2003,10 +2003,12 @@ class Assignment < ActiveRecord::Base
homework.turnitin_data[:eula_agreement_timestamp] = eula_timestamp if eula_timestamp.present?
homework.with_versioning(:explicit => (homework.submission_type != "discussion_topic")) do
if group
if student == original_student
homework.broadcast_group_submission
else
homework.save_without_broadcasting!
Submission.suspend_callbacks(:delete_submission_drafts!) do
if student == original_student
homework.broadcast_group_submission
else
homework.save_without_broadcasting!
end
end
else
homework.save!

View File

@ -66,6 +66,7 @@ class Attachment < ActiveRecord::Base
belongs_to :user
has_one :account_report
has_one :media_object
has_many :submission_draft_attachments, inverse_of: :attachment
has_many :submissions, -> { active }
has_many :attachment_associations
belongs_to :root_attachment, :class_name => 'Attachment'

View File

@ -99,6 +99,7 @@ class Submission < ActiveRecord::Base
has_many :originality_reports
has_one :rubric_assessment, -> { where(assessment_type: 'grading') }, as: :artifact, inverse_of: :artifact
has_one :lti_result, inverse_of: :submission, class_name: 'Lti::Result', dependent: :destroy
has_many :submission_drafts, inverse_of: :submission, dependent: :destroy
# we no longer link submission comments and conversations, but we haven't fixed up existing
# linked conversations so this relation might be useful
@ -313,6 +314,7 @@ class Submission < ActiveRecord::Base
after_save :reset_regraded
after_save :create_audit_event!
after_save :handle_posted_at_changed, if: :saved_change_to_posted_at?
after_save :delete_submission_drafts!, if: :saved_change_to_attempt?
def reset_regraded
@regraded = false
@ -2363,6 +2365,10 @@ class Submission < ActiveRecord::Base
true
end
def delete_submission_drafts!
self.submission_drafts.destroy_all
end
def point_data?
!!(self.score || self.grade)
end

View File

@ -0,0 +1,38 @@
#
# Copyright (C) 2019 - 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/>.
#
class SubmissionDraft < ActiveRecord::Base
belongs_to :submission, inverse_of: :submission_drafts
has_many :submission_draft_attachments, inverse_of: :submission_draft, dependent: :delete_all
has_many :attachments, through: :submission_draft_attachments
validates :submission, presence: true
validates :submission_attempt, numericality: { only_integer: true }
validates :submission, uniqueness: { scope: :submission_attempt }
validate :submission_attempt_matches_submission
def submission_attempt_matches_submission
root_submission_attempt = self.submission&.attempt || 0
this_submission_attempt = self.submission_attempt || 0
if this_submission_attempt > root_submission_attempt
err = 'submission draft attempt cannot be larger then the submission attempt'
errors.add(:submission_draft_attempt, err)
end
end
private :submission_attempt_matches_submission
end

View File

@ -0,0 +1,26 @@
#
# Copyright (C) 2019 - 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/>.
#
class SubmissionDraftAttachment < ActiveRecord::Base
belongs_to :submission_draft, inverse_of: :submission_draft_attachments
belongs_to :attachment, inverse_of: :submission_draft_attachments
validates :submission_draft, presence: true
validates :attachment, presence: true
validates :submission_draft, uniqueness: { scope: :attachment }
end

View File

@ -0,0 +1,39 @@
#
# Copyright (C) 2019 - 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/>.
#
class CreateSubmissionDraft < ActiveRecord::Migration[5.1]
tag :predeploy
def change
create_table :submission_drafts do |t|
t.references :submission, limit: 8, foreign_key: true, index: true, null: false
t.integer :submission_attempt, index:true, null: false
end
# Attachments can be cross shard, so we can't use a proper foreign key for them
create_table :submission_draft_attachments do |t|
t.references :submission_draft, limit: 8, foreign_key: true, index: true, null: false
t.integer :attachment_id, limit: 8, index:true, null: false
end
add_index :submission_draft_attachments,
[:submission_draft_id, :attachment_id],
name: 'index_submission_draft_and_attachment_unique',
unique: true
end
end

View File

@ -79,7 +79,7 @@ describe GradebookGradingPeriodAssignments do
it "excludes deleted submissions" do
assignment_in_gp2 = @example_course.assignments.create!(due_at: 1.day.from_now)
assignment_in_gp2.destroy
assignment_in_gp2.submissions.preload(:all_submission_comments, :lti_result, :versions).map(&:destroy)
assignment_in_gp2.submissions.preload(:all_submission_comments, :lti_result, :versions, :submission_drafts).map(&:destroy)
expect(hash[@period2.id]).not_to include(assignment_in_gp2.id.to_s)
end

View File

@ -0,0 +1,111 @@
#
# Copyright (C) 2019 - 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 '../spec_helper'
require File.expand_path(File.dirname(__FILE__) + '/../sharding_spec_helper.rb')
describe SubmissionDraftAttachment do
before :once do
@submission = submission_model
@submission_draft = SubmissionDraft.create!(
submission: @submission,
submission_attempt: @submission.attempt
)
@attachment = attachment_model
@submission_draft_attachment = SubmissionDraftAttachment.create!(
submission_draft: @submission_draft,
attachment: @attachment
)
end
it 'submission draft attachment has one attachment' do
expect(@submission_draft_attachment.attachment).to eq @attachment
end
it 'attachments can have multiple submission draft attachments' do
submission2 = submission_model
submission_draft2 = SubmissionDraft.create!(
submission: submission2,
submission_attempt: submission2.attempt
)
submission_draft_attachment2 = SubmissionDraftAttachment.create!(
submission_draft: submission_draft2,
attachment: @attachment
)
expect(@attachment.submission_draft_attachments.sort).to eq [
@submission_draft_attachment,
submission_draft_attachment2
]
end
context 'validation' do
it 'will not let you have multiple of the same attachment to submission draft' do
expect{
SubmissionDraftAttachment.create!(
submission_draft: @submission_draft,
attachment: @attachment
)
}.to raise_error(ActiveRecord::RecordInvalid)
end
it 'requires an attachment' do
expect{
SubmissionDraftAttachment.create!(
submission_draft: @submission_draft,
attachment: nil
)
}.to raise_error(ActiveRecord::RecordInvalid)
end
it 'requires a submission draft' do
expect{
SubmissionDraftAttachment.create!(
submission_draft: nil,
attachment: @attachment
)
}.to raise_error(ActiveRecord::RecordInvalid)
end
end
context 'sharding' do
specs_require_sharding
before(:once) do
@shard1.activate { @attachment1 = attachment_model }
@shard2.activate { @attachment2 = attachment_model }
@shard1.activate do
@submission_draft.attachments = [@attachment1, @attachment2]
@submission_draft.save!
end
end
it 'can have attachments saved that are cross shard' do
@shard1.activate do
expect(
@submission_draft.attachments.pluck(:id).sort
).to eq [@attachment1.id, @attachment2.global_id].sort
end
@shard2.activate do
expect(
@submission_draft.attachments.pluck(:id).sort
).to eq [@attachment1.global_id, @attachment2.id].sort
end
end
end
end

View File

@ -0,0 +1,79 @@
#
# Copyright (C) 2019 - 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 '../spec_helper'
RSpec.describe SubmissionDraft do
before(:once) do
@submission = submission_model
@submission_draft = SubmissionDraft.create!(
submission: @submission,
submission_attempt: @submission.attempt
)
end
describe 'attachments' do
before(:once) do
@attachment1 = attachment_model
@attachment2 = attachment_model
@attachment3 = attachment_model
@submission_draft.attachments = [@attachment1, @attachment2, @attachment3]
end
it 'can be accessed on a submission draft' do
expect(@submission_draft.attachments).to eq [@attachment1, @attachment2, @attachment3]
end
it 'can set different attachments on a submission draft' do
attachment4 = attachment_model
@submission_draft.attachments = [attachment4]
expect(@submission_draft.attachments).to eq [attachment4]
end
it 'are deleted if a submission draft is deleted' do
@submission_draft.destroy!
expect(SubmissionDraftAttachment.count).to eq 0
end
end
describe 'validation' do
it 'submission cannot be nil' do
expect{
SubmissionDraft.create!(submission: nil, submission_attempt: @submission.attempt)
}.to raise_error(ActiveRecord::RecordInvalid)
end
it 'submission_attempt cannot be nil' do
expect{
SubmissionDraft.create!(submission: @submission, submission_attempt: nil)
}.to raise_error(ActiveRecord::RecordInvalid)
end
it 'cannot have duplicate drafts for the same submission and attempt' do
expect{
SubmissionDraft.create!(submission: @submission, submission_attempt: @submission.attempt)
}.to raise_error(ActiveRecord::RecordInvalid)
end
it 'submission_attempt cannot be larger then the root submissions attempt' do
expect{
SubmissionDraft.create!(submission: @submission, submission_attempt: @submission.attempt + 1)
}.to raise_error(ActiveRecord::RecordInvalid)
end
end
end

View File

@ -6269,6 +6269,61 @@ describe Submission do
end
end
describe '#submission_drafts' do
before(:once) do
@submission = Submission.find_by(user: @student)
end
it 'is empty by default' do
expect(@submission.submission_drafts).to eq []
end
describe 'with drafts for multiple attempts' do
before(:once) do
@submission = @assignment.submit_homework(@student, submission_type: 'online_text_entry', body: 'foo')
@draft1 = SubmissionDraft.new(submission: @submission, submission_attempt: 0)
@draft2 = SubmissionDraft.new(submission: @submission, submission_attempt: 1)
@submission.submission_drafts << @draft1
@submission.submission_drafts << @draft2
end
it 'can have drafts for different submission attempts' do
expect(@submission.submission_drafts.sort).to eq [@draft1, @draft2]
end
it 'deletes all drafts for all submission attempts when homework is submitted' do
@assignment.submit_homework(@student, submission_type: 'online_text_entry', body: 'foo')
@submission.reload
expect(@submission.submission_drafts).to eq []
expect(SubmissionDraft.count).to be 0
end
end
describe 'with attachments' do
before(:once) do
@attachment1 = attachment_model
@attachment2 = attachment_model
@submission_draft = SubmissionDraft.create!(
submission: @submission,
submission_attempt: 0
)
@submission_draft.attachments = [@attachment1, @attachment2]
end
it 'can access the attachments' do
expect(@submission.submission_drafts.first.attachments.sort).to eq [@attachment1, @attachment2]
end
it 'will cascade deletes to SubmissionDraftAttachments when homework is submitted' do
@assignment.submit_homework(@student, submission_type: 'online_text_entry', body: 'foo')
@submission.reload
expect(@submission.submission_drafts).to eq []
expect(SubmissionDraft.count).to be 0
expect(SubmissionDraftAttachment.count).to be 0
end
end
end
describe "posting and unposting" do
before(:each) do
@assignment.course.enable_feature!(:post_policies)