ylqjgm
/
canvas-lms
mirror of https://github.com/instructure/canvas-lms.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

delete scores when an assignment group is deleted 

When an assignment group is soft-deleted, soft-delete the scores for
that assignment group. In addition, when an assignment group is
restored, restore the scores for that assignment group. The
AssignmentGroup model now uses the Canvas::SoftDeletable module.

closes GRADE-741

Test Plan 1: Active Students
1. In a course with an active student and an assignment group, go to
   the Assignments page and delete the assignment group.
2. In a rails console, verify the assignment group score was
   soft-deleted for the active student.

   score = Score.find_by(
     enrollment_id: <active-student-enrollment-id>,
     assignment_group_id: <assignment-group-that-was-deleted-id>
   )
   score.workflow_state
    => 'deleted'

3. Restore the assignment group and verify the assignment group score
   for the active student is restored as well.

   score.assignment_group.restore
   score.reload.workflow_state
    => 'active'

Test Plan 2: Students Concluded Prior to Assignment Group Deletion
1. In a course with a concluded student and an assignment group,
   go to the Assignments page and delete the assignment group.
2. In a rails console, verify the assignment group score is still
   active for the concluded student.

   score = Score.find_by(
     enrollment_id: <concluded-student-enrollment-id>,
     assignment_group_id: <assignment-group-that-was-deleted-id>
   )
   score.workflow_state
    => 'active'

Test Plan 3: Students Concluded After Assignment Group Deletion
1. In a course with an active student and an assignment group, go to
   the Assignments page and delete the assignment group.
2. Conclude the active student.
3. In a rails console, verify the assignment group score is
   soft-deleted for the student.

   score = Score.find_by(
     enrollment_id: <student-enrollment-id>,
     assignment_group_id: <assignment-group-that-was-deleted-id>
   )
   score.workflow_state
    => 'deleted'

4. Restore the assignment group and verify the assignment group score
   for the student is still deleted.

   score.assignment_group.restore
   score.reload.workflow_state
    => 'deleted'

Test Plan 4: Deleted Students
1. In a course with an active student and an assignment group, go to
   the Assignments page and delete the assignment group.
2. Remove the student from the course.
3. In a rails console, verify the assignment group score is
   soft-deleted for the student.

   score = Score.find_by(
     enrollment_id: <student-enrollment-id>,
     assignment_group_id: <assignment-group-that-was-deleted-id>
   )
   score.workflow_state
    => 'deleted'

4. Restore the assignment group and verify the assignment group score
   for the student is still deleted.

   score.assignment_group.restore
   score.reload.workflow_state
    => 'deleted'

Change-Id: Ie986a1dfcbdd2118b359ac90d3a8425a63e4c21d
Reviewed-on: https://gerrit.instructure.com/136609
Tested-by: Jenkins
Reviewed-by: Keith T. Garner <kgarner@instructure.com>
Reviewed-by: Shahbaz Javeed <sjaveed@instructure.com>
QA-Review: Indira Pai <ipai@instructure.com>
Product-Review: Keith T. Garner <kgarner@instructure.com>
This commit is contained in:
Spencer Olson 2017-12-28 13:39:50 -06:00
parent 6f773df20f
commit a3f81051be
3 changed files with 95 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

63
app/models/assignment_group.rb
View File

@ -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

4
lib/canvas/soft_deletable.rb
View File

@ -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

50
spec/models/assignment_group_spec.rb
View File

@ -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={})