diff --git a/app/models/assignment_group.rb b/app/models/assignment_group.rb index 4f8839fc43b..858c1e9cdc4 100644 --- a/app/models/assignment_group.rb +++ b/app/models/assignment_group.rb @@ -17,8 +17,13 @@ # class AssignmentGroup < ActiveRecord::Base - include Workflow + # Unlike our other soft-deletable models, assignment groups use 'available' instead of 'active' + # to indicate a not-deleted state. This means we have to add the 'available' state here before + # Canvas::SoftDeletable adds the 'active' and 'deleted' states, so that 'available' becomes the + # initial state for this model. + workflow { state :available } + include Canvas::SoftDeletable attr_readonly :context_id, :context_type belongs_to :context, polymorphic: [:course] @@ -48,6 +53,8 @@ class AssignmentGroup < ActiveRecord::Base after_save :touch_context after_save :update_student_grades + before_destroy :destroy_scores + def generate_default_values if self.name.blank? self.name = t 'default_title', "Assignments" @@ -85,18 +92,6 @@ class AssignmentGroup < ActiveRecord::Base can :delete end - workflow do - state :available - state :deleted - end - - alias_method :destroy_permanently!, :destroy - def destroy - self.workflow_state = 'deleted' - self.assignments.active.include_submittables.each(&:destroy) - self.save - end - def restore(try_to_selectively_undelete_assignments = true) to_restore = self.assignments.include_submittables if try_to_selectively_undelete_assignments @@ -106,8 +101,8 @@ class AssignmentGroup < ActiveRecord::Base # were deleted earlier. to_restore = to_restore.where('updated_at >= ?', self.updated_at.utc) end - self.workflow_state = 'available' - self.save + undestroy(active_state: 'available') + restore_scores to_restore.each { |assignment| assignment.restore(:assignment_group) } end @@ -208,11 +203,6 @@ class AssignmentGroup < ActiveRecord::Base effective_due_dates.any_in_closed_grading_period? end - def effective_due_dates - @effective_due_dates ||= EffectiveDueDates.for_course(context, published_assignments) - end - private :effective_due_dates - def visible_assignments(user, includes=[]) self.class.visible_assignments(user, self.context, [self], includes) end @@ -239,4 +229,37 @@ class AssignmentGroup < ActiveRecord::Base new_group.touch self.reload end + + private + + def destroy_scores + # TODO: soft-delete score metadata as part of GRADE-746 + set_scores_workflow_state_in_batches(:deleted) + end + + def restore_scores + # TODO: restore score metadata as part of GRADE-746 + set_scores_workflow_state_in_batches(:active, exclude_workflow_states: [:completed, :deleted]) + end + + def set_scores_workflow_state_in_batches(new_workflow_state, exclude_workflow_states: [:completed]) + student_enrollments = Enrollment.where( + course_id: context_id, + type: [:StudentEnrollment, :StudentViewEnrollment] + ).where.not(workflow_state: exclude_workflow_states) + + score_ids = Score.where( + assignment_group_id: self, + enrollment_id: student_enrollments, + workflow_state: new_workflow_state == :active ? :deleted : :active + ).pluck(:id) + + score_ids.each_slice(1000) do |score_ids_batch| + Score.where(id: score_ids_batch).update_all(workflow_state: new_workflow_state, updated_at: Time.zone.now) + end + end + + def effective_due_dates + @effective_due_dates ||= EffectiveDueDates.for_course(context, published_assignments) + end end diff --git a/lib/canvas/soft_deletable.rb b/lib/canvas/soft_deletable.rb index 4d91175c490..a215204da21 100644 --- a/lib/canvas/soft_deletable.rb +++ b/lib/canvas/soft_deletable.rb @@ -41,8 +41,8 @@ module Canvas::SoftDeletable end # `restore` was taken by too many other methods... - def undestroy - self.workflow_state = 'active' + def undestroy(active_state: 'active') + self.workflow_state = active_state save! true end diff --git a/spec/models/assignment_group_spec.rb b/spec/models/assignment_group_spec.rb index 6f2e13cbe00..cdfe31f3a15 100644 --- a/spec/models/assignment_group_spec.rb +++ b/spec/models/assignment_group_spec.rb @@ -447,6 +447,56 @@ describe AssignmentGroup do expect(@ag.any_assignment_in_closed_grading_period?).to eq(true) end end + + describe "#destroy" do + before(:once) do + @student_enrollment = @student.enrollments.find_by(course_id: @course) + @group = @course.assignment_groups.create!(@valid_attributes) + end + + let(:student_score) do + Score.find_by(enrollment_id: @student_enrollment, assignment_group_id: @group) + end + + it "destroys scores belonging to active students" do + expect { @group.destroy }.to change { student_score.reload.state }.from(:active).to(:deleted) + end + + it "does not destroy scores belonging to concluded students" do + @student_enrollment.conclude + expect { @group.destroy }.not_to change { student_score.reload.state } + end + end + + describe "#restore" do + before(:once) do + @student_enrollment = @student.enrollments.find_by(course_id: @course) + @group = @course.assignment_groups.create!(@valid_attributes) + @group.destroy + end + + let(:student_score) do + Score.find_by(enrollment_id: @student_enrollment, assignment_group_id: @group) + end + + it "restores the assignment group back to an 'available' state" do + expect { @group.restore }.to change { @group.state }.from(:deleted).to(:available) + end + + it "restores scores belonging to active students" do + expect { @group.restore }.to change { student_score.reload.state }.from(:deleted).to(:active) + end + + it "does not restore scores belonging to concluded students" do + @student_enrollment.conclude + expect { @group.restore }.not_to change { student_score.reload.state } + end + + it "does not restore scores belonging to deleted students" do + @student_enrollment.destroy + expect { @group.restore }.not_to change { student_score.reload.state } + end + end end def assignment_group_model(opts={})