From b9121aeb0b84b23d7bfe486a0b9d6e7e1b5387e0 Mon Sep 17 00:00:00 2001 From: James Williams Date: Wed, 23 May 2018 08:03:36 -0600 Subject: [PATCH] don't show overridden assignments for inactive enrollments test plan: * have a course with multiple sections * create an assignment assigned to only one section * add a student to the course in the assigned section * deactivate the user from the course people page * the student should not see the assignment anymore closes #COMMS-484 Change-Id: I2a604c091ad319e8bfa08b6156b02ef211e4d938 Reviewed-on: https://gerrit.instructure.com/151171 Tested-by: Jenkins Reviewed-by: Steven Burnett QA-Review: KC Naegle Product-Review: KC Naegle --- ...0523134906_update_asv_enrollment_filter.rb | 120 ++++++++++++++++++ lib/effective_due_dates.rb | 2 +- spec/apis/v1/assignments_api_spec.rb | 10 ++ .../assignment_student_visibility_spec.rb | 12 +- 4 files changed, 142 insertions(+), 2 deletions(-) create mode 100644 db/migrate/20180523134906_update_asv_enrollment_filter.rb diff --git a/db/migrate/20180523134906_update_asv_enrollment_filter.rb b/db/migrate/20180523134906_update_asv_enrollment_filter.rb new file mode 100644 index 00000000000..796f7079f9b --- /dev/null +++ b/db/migrate/20180523134906_update_asv_enrollment_filter.rb @@ -0,0 +1,120 @@ +class UpdateAsvEnrollmentFilter < ActiveRecord::Migration[5.1] + tag :postdeploy + + def up + # don't include inactive/rejected enrollments + self.connection.execute %(CREATE OR REPLACE VIEW #{connection.quote_table_name('assignment_student_visibilities')} AS + SELECT DISTINCT a.id as assignment_id, + e.user_id as user_id, + c.id as course_id + + FROM #{Assignment.quoted_table_name} a + + JOIN #{Course.quoted_table_name} c + ON a.context_id = c.id + AND a.context_type = 'Course' + + JOIN #{Enrollment.quoted_table_name} e + ON e.course_id = c.id + AND e.type IN ('StudentEnrollment', 'StudentViewEnrollment') + AND e.workflow_state NOT IN ('deleted', 'rejected', 'inactive') + + JOIN #{CourseSection.quoted_table_name} cs + ON cs.course_id = c.id + AND e.course_section_id = cs.id + + LEFT JOIN #{GroupMembership.quoted_table_name} gm + ON gm.user_id = e.user_id + AND gm.workflow_state = 'accepted' + + LEFT JOIN #{Group.quoted_table_name} g + ON g.context_type = 'Course' + AND g.context_id = c.id + AND g.workflow_state = 'available' + AND gm.group_id = g.id + + LEFT JOIN #{AssignmentOverrideStudent.quoted_table_name} aos + ON aos.assignment_id = a.id + AND aos.user_id = e.user_id + AND aos.workflow_state = 'active' + + LEFT JOIN #{AssignmentOverride.quoted_table_name} ao + ON ao.assignment_id = a.id + AND ao.workflow_state = 'active' + AND ( + (ao.set_type = 'CourseSection' AND ao.set_id = cs.id) + OR (ao.set_type = 'ADHOC' AND ao.set_id IS NULL AND ao.id = aos.assignment_override_id) + OR (ao.set_type = 'Group' AND ao.set_id = g.id) + ) + + LEFT JOIN #{Submission.quoted_table_name} s + ON s.user_id = e.user_id + AND s.assignment_id = a.id + AND s.workflow_state NOT IN ('deleted', 'unsubmitted') + + WHERE a.workflow_state NOT IN ('deleted','unpublished') + AND( + ( a.only_visible_to_overrides = 'true' AND (ao.id IS NOT NULL OR s.id IS NOT NULL)) + OR (COALESCE(a.only_visible_to_overrides, 'false') = 'false') + ) + ) + end + + def down + self.connection.execute %(CREATE OR REPLACE VIEW #{connection.quote_table_name('assignment_student_visibilities')} AS + SELECT DISTINCT a.id as assignment_id, + e.user_id as user_id, + c.id as course_id + + FROM #{Assignment.quoted_table_name} a + + JOIN #{Course.quoted_table_name} c + ON a.context_id = c.id + AND a.context_type = 'Course' + + JOIN #{Enrollment.quoted_table_name} e + ON e.course_id = c.id + AND e.type IN ('StudentEnrollment', 'StudentViewEnrollment') + AND e.workflow_state != 'deleted' + + JOIN #{CourseSection.quoted_table_name} cs + ON cs.course_id = c.id + AND e.course_section_id = cs.id + + LEFT JOIN #{GroupMembership.quoted_table_name} gm + ON gm.user_id = e.user_id + AND gm.workflow_state = 'accepted' + + LEFT JOIN #{Group.quoted_table_name} g + ON g.context_type = 'Course' + AND g.context_id = c.id + AND g.workflow_state = 'available' + AND gm.group_id = g.id + + LEFT JOIN #{AssignmentOverrideStudent.quoted_table_name} aos + ON aos.assignment_id = a.id + AND aos.user_id = e.user_id + AND aos.workflow_state = 'active' + + LEFT JOIN #{AssignmentOverride.quoted_table_name} ao + ON ao.assignment_id = a.id + AND ao.workflow_state = 'active' + AND ( + (ao.set_type = 'CourseSection' AND ao.set_id = cs.id) + OR (ao.set_type = 'ADHOC' AND ao.set_id IS NULL AND ao.id = aos.assignment_override_id) + OR (ao.set_type = 'Group' AND ao.set_id = g.id) + ) + + LEFT JOIN #{Submission.quoted_table_name} s + ON s.user_id = e.user_id + AND s.assignment_id = a.id + AND s.workflow_state NOT IN ('deleted', 'unsubmitted') + + WHERE a.workflow_state NOT IN ('deleted','unpublished') + AND( + ( a.only_visible_to_overrides = 'true' AND (ao.id IS NOT NULL OR s.id IS NOT NULL)) + OR (COALESCE(a.only_visible_to_overrides, 'false') = 'false') + ) + ) + end +end diff --git a/lib/effective_due_dates.rb b/lib/effective_due_dates.rb index fe835b6cb90..c4aba576732 100644 --- a/lib/effective_due_dates.rb +++ b/lib/effective_due_dates.rb @@ -267,7 +267,7 @@ class EffectiveDueDates WHERE o.set_type = 'CourseSection' AND s.workflow_state <> 'deleted' AND - e.workflow_state NOT IN ('rejected', 'deleted') AND + e.workflow_state NOT IN ('rejected', 'deleted', 'inactive') AND e.type IN ('StudentEnrollment', 'StudentViewEnrollment') #{filter_students_sql('e')} ), diff --git a/spec/apis/v1/assignments_api_spec.rb b/spec/apis/v1/assignments_api_spec.rb index f104b165cc7..046ad31d94b 100644 --- a/spec/apis/v1/assignments_api_spec.rb +++ b/spec/apis/v1/assignments_api_spec.rb @@ -861,6 +861,16 @@ describe AssignmentsApiController, type: :request do json = api_get_assignments_index_from_course(@course) expect(json).to eq [] end + + it "should not show assignments assigned to the user has an inactive sectionenrollments for" do + inactive_enrollment = @course.enroll_student(@student2, :allow_multiple_enrollments => true, + :enrollment_state => 'inactive', :section => @section2) + + user_session @student2 + @user = @student2 + json = api_get_assignments_index_from_course(@course) + expect(json).to eq [] + end end context "as an observer" do diff --git a/spec/models/assignment_student_visibility_spec.rb b/spec/models/assignment_student_visibility_spec.rb index cf75155bd69..aad90c303dc 100644 --- a/spec/models/assignment_student_visibility_spec.rb +++ b/spec/models/assignment_student_visibility_spec.rb @@ -332,22 +332,32 @@ describe "differentiated_assignments" do it "should show the assignment to the user" do ensure_user_sees_assignment end + it "should not show unpublished assignments" do @assignment.workflow_state = "unpublished" @assignment.save! ensure_user_does_not_see_assignment end - it "should update when enrollments change" do + + it "should update when enrollments are destroyed" do ensure_user_sees_assignment enrollments = StudentEnrollment.where(:user_id => @user.id, :course_id => @course.id, :course_section_id => @section_foo.id) enrollments.each(&:destroy_permanently!) ensure_user_does_not_see_assignment end + + it "should update when enrollments are inactive" do + ensure_user_sees_assignment + @user.enrollments.where(:course_id => @course.id, :course_section_id => @section_foo.id).first.deactivate + ensure_user_does_not_see_assignment + end + it "should update when the override is deleted" do ensure_user_sees_assignment @assignment.assignment_overrides.each(&:destroy!) ensure_user_does_not_see_assignment end + it "should not return duplicate visibilities with multiple visible sections" do enroller_user_in_section(@section_bar, {user: @user}) give_section_due_date(@assignment, @section_bar)