limit global announcements to active enrollments

Only students/staff who have active enrollments in the current term,
should get global announcements.

fixes KNO-399
flag=none

/ ---- ---- \
| Test Plan |
\ ---- ---- /

- In an account create a user and two sub accounts
- In the parent account navigate to Settings > Feature Options and
  enable the `Immediate notifications for global announcements` feature
  flag
- As the user navigate to /profile/communication and update the global
  announcements preference to be immediate
- In both sub accounts create a course and add a student to it, publish
  the course, and as the user accept the enrollment invitation
- In both sub accounts navigate to Settings > Announcements and create
  an announcement
- Navigate to /users/<user_id>/messages and note the existence of the
  two announcements
- In sub account B navigate to the course settings page, update the
  start and end dates to be in the past, check the `Students can only
  participate in the course between these dates` check box, and save the
  changes
- Again in both sub accounts navigate to Settings > Announcements and
  create an announcement
- Navigate back to /users/<user_id>/messages and note that there should
  only be a new message from sub account A

Change-Id: I1f1fc7949d394e39ace73adf3d895f98c1c2fcb3
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/237771
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Rob Orton <rob@instructure.com>
QA-Review: Rob Orton <rob@instructure.com>
Product-Review: Rob Orton <rob@instructure.com>
This commit is contained in:
Matthew Lemon 2020-05-18 14:28:15 -06:00
parent 2fd60b2131
commit 1b0fbd4b68
2 changed files with 12 additions and 12 deletions

View File

@ -71,7 +71,7 @@ class AccountNotification < ActiveRecord::Base
if root_account.site_admin?
current = self.for_account(root_account, include_past: include_past)
else
course_ids = user.enrollments.active_or_pending.shard(user.in_region_associated_shards).distinct.pluck(:course_id) # fetch sharded course ids
course_ids = user.enrollments.active_or_pending_by_date.shard(user.in_region_associated_shards).distinct.pluck(:course_id) # fetch sharded course ids
# and then fetch account_ids separately - using pluck on a joined column doesn't give relative ids
all_account_ids = Course.where(:id => course_ids).not_deleted.
distinct.pluck(:account_id, :root_account_id).flatten.uniq
@ -95,13 +95,13 @@ class AccountNotification < ActiveRecord::Base
unless role_ids.empty? || user_role_ids.key?(announcement.account_id)
# choose enrollments and account users to inspect
if announcement.account.site_admin?
enrollments = user.enrollments.shard(user.in_region_associated_shards).active_or_pending.distinct.select(:role_id).to_a
enrollments = user.enrollments.shard(user.in_region_associated_shards).active_or_pending_by_date.distinct.select(:role_id).to_a
account_users = user.account_users.shard(user.in_region_associated_shards).distinct.select(:role_id).to_a
else
announcement.shard.activate do
sub_account_ids_map[announcement.account_id] ||=
Account.sub_account_ids_recursive(announcement.account_id) + [announcement.account_id]
enrollments = Enrollment.where(user_id: user).active_or_pending.joins(:course).
enrollments = Enrollment.where(user_id: user).active_or_pending_by_date.joins(:course).
where(:courses => {:account_id => sub_account_ids_map[announcement.account_id]}).select(:role_id).to_a
account_users = announcement.account.root_account.cached_all_account_users_for(user)
end
@ -145,7 +145,7 @@ class AccountNotification < ActiveRecord::Base
end
end
roles = user.enrollments.shard(user.in_region_associated_shards).active_or_pending.distinct.pluck(:type)
roles = user.enrollments.shard(user.in_region_associated_shards).active_or_pending_by_date.distinct.pluck(:type)
if roles == ['StudentEnrollment'] && !root_account.include_students_in_global_survey?
current.reject! { |announcement| announcement.required_account_service == 'account_survey_notifications' }
@ -278,7 +278,7 @@ class AccountNotification < ActiveRecord::Base
course_ids = Course.active.where(:id => min_id..max_id, :account_id => all_account_ids).pluck(:id)
next unless course_ids.any?
course_ids.each_slice(50) do |sliced_course_ids|
scope = Enrollment.active_or_pending.where(:course_id => sliced_course_ids)
scope = Enrollment.active_or_pending_by_date.where(:course_id => sliced_course_ids)
scope = scope.where(:role_id => course_roles) unless get_everybody
user_ids += scope.distinct.pluck(:user_id)
end

View File

@ -50,7 +50,7 @@ describe AccountNotification do
@teacher = @user
account_admin_user(:account => @account)
@admin = @user
course_with_student(:course => @course)
course_with_student(:course => @course).accept(true)
@student = @user
expect(AccountNotification.for_user_and_account(@teacher, @account).map(&:id).sort).to eq [@a1.id, @a3.id]
@ -184,14 +184,14 @@ describe AccountNotification do
it "scopes to active enrollment accounts" do
sub_announcement = sub_account_notification(subject: 'blah', account: @sub_account)
course_with_student(user: @user, account: @sub_account, active_all: true)
course_with_student(user: @user, account: @sub_account, active_all: true).accept(true)
other_root_account = Account.create!
other_announcement = account_notification(account: other_root_account)
course_with_student(user: @user, account: other_root_account, active_all: true)
course_with_student(user: @user, account: other_root_account, active_all: true).accept(true)
nother_root_account = Account.create!(name: 'nother account')
nother_announcement = account_notification(account: nother_root_account)
# not an active course and will be excluded
course_with_student(user: @user, account: nother_root_account)
course_with_student(user: @user, account: nother_root_account).accept(true)
notes = AccountNotification.for_user_and_account(@user, Account.default)
expect(notes).to include sub_announcement
@ -212,7 +212,7 @@ describe AccountNotification do
it "still show sub-account announcements even if the course is unpublished" do
# because that makes sense i guess?
unpub_sub_announcement = sub_account_notification(subject: 'blah', account: @sub_account)
course_with_student(user: @user, account: @sub_account)
course_with_student(user: @user, account: @sub_account).accept(true)
notes = AccountNotification.for_user_and_account(@user, Account.default)
expect(notes).to include unpub_sub_announcement
@ -285,12 +285,12 @@ describe AccountNotification do
@unenrolled = @user
course_with_teacher(account: @a1)
@student_teacher = @user
course_with_student(course: @course, user: @student_teacher)
course_with_student(course: @course, user: @student_teacher).accept(true)
course_with_teacher(course: @course, :account => @a1)
@teacher = @user
account_admin_user(:account => @a1)
@admin = @user
course_with_student(:course => @course)
course_with_student(:course => @course).accept(true)
@student = @user
end