Correctly check observer self-enrollments across shards
When loading dashboard cards for users who are observers but have their own enrollments as well, we check if the selected observee has the same ID as the current user when determining which enrollments to load. This causes issues if some the logged-in users enrollments are on a different shard from their home shard, since we were comparing a passed-in global ID to the user's local ID in that scenario. This PS updates the observed user's own-enrollments check to actually load the associated user before comparisons, ensuring that Switchman takes the cross-shard nature of this check into account. fixes LS-2820, LS-2834 flag = k5_parent_support Test plan: - Make sure you have muliple shards set up - Find a user with the following properties: * Is not observing any users * Is enrolled in some courses in a C4E account * Has logins on multiple trusted shards - Enroll a user as an observer in a concluded course on one shard - Log in as the user on a different shard - Expect the user's enrollments to show up as usual - To verify the behavior is correct, you can compare the results of these API calls (they should return the same results) * /api/v1/dashboard/dashboard_cards * /api/v1/dashboard/dashboard_cards?observed_user=<user's gid> Change-Id: Ibbdf6cd0d80493198203c59a62b2e07464b13cf6 Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/279002 Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com> Reviewed-by: Jackson Howe <jackson.howe@instructure.com> QA-Review: Jackson Howe <jackson.howe@instructure.com> Product-Review: Jeff Largent <jeff.largent@instructure.com>
This commit is contained in:
parent
0bffa30526
commit
6af66eb0f9
|
@ -70,7 +70,7 @@ class FavoritesController < ApplicationController
|
|||
def list_favorite_courses
|
||||
includes = Set.new(Array(params[:include]))
|
||||
opts = {}
|
||||
opts[:observee_user] = params[:observed_user].to_i if params.key?(:observed_user)
|
||||
opts[:observee_user] = User.find_by(id: params[:observed_user].to_i) || @current_user if params.key?(:observed_user)
|
||||
|
||||
courses = @current_user.menu_courses(nil, opts)
|
||||
if courses.any? && value_to_boolean(params[:exclude_blueprint_courses])
|
||||
|
|
|
@ -583,7 +583,7 @@ class UsersController < ApplicationController
|
|||
|
||||
def dashboard_cards
|
||||
opts = {}
|
||||
opts[:observee_user] = params[:observed_user].to_i if params.key?(:observed_user)
|
||||
opts[:observee_user] = User.find_by(id: params[:observed_user].to_i) || @current_user if params.key?(:observed_user)
|
||||
dashboard_courses = map_courses_for_menu(@current_user.menu_courses(nil, opts), tabs: DASHBOARD_CARD_TABS)
|
||||
published, unpublished = dashboard_courses.partition { |course| course[:published] }
|
||||
Rails.cache.write(['last_known_dashboard_cards_published_count', @current_user.global_id].cache_key, published.count)
|
||||
|
|
|
@ -425,12 +425,12 @@ class User < ActiveRecord::Base
|
|||
after_save :self_enroll_if_necessary
|
||||
|
||||
def courses_for_enrollments(enrollment_scope, associated_user = nil)
|
||||
if associated_user && associated_user != id
|
||||
if associated_user && associated_user != self
|
||||
Course.active.joins(:observer_enrollments)
|
||||
.merge(enrollment_scope.except(:joins))
|
||||
.where(enrollments: { associated_user_id: associated_user })
|
||||
.where(enrollments: { associated_user_id: associated_user.id })
|
||||
else
|
||||
enrollments_to_include = associated_user == id ? :non_observer_enrollments : :all_enrollments
|
||||
enrollments_to_include = associated_user == self ? :non_observer_enrollments : :all_enrollments
|
||||
Course.active.joins(enrollments_to_include).merge(enrollment_scope.except(:joins)).distinct
|
||||
end
|
||||
end
|
||||
|
|
|
@ -806,16 +806,16 @@ describe User do
|
|||
observer_enrollment2.save!
|
||||
|
||||
@teacher_course = course_factory(:course_name => "English ", :active_course => true)
|
||||
teacher_entollment = @teacher_course.enroll_user(@user, 'TeacherEnrollment', :enrollment_state => 'active')
|
||||
teacher_entollment.save!
|
||||
teacher_enrollment = @teacher_course.enroll_user(@user, 'TeacherEnrollment', :enrollment_state => 'active')
|
||||
teacher_enrollment.save!
|
||||
|
||||
@student_course = course_factory(:course_name => "Leadership", :active_course => true)
|
||||
teacher_entollment = @student_course.enroll_user(@user, 'StudentEnrollment', :enrollment_state => 'active')
|
||||
teacher_entollment.save!
|
||||
student_enrollment = @student_course.enroll_user(@user, 'StudentEnrollment', :enrollment_state => 'active')
|
||||
student_enrollment.save!
|
||||
end
|
||||
|
||||
it "returns observed courses related to the associated_user" do
|
||||
expect(@observer.courses_with_primary_enrollment(:current_and_invited_courses, nil, :observee_user => @student.id)
|
||||
expect(@observer.courses_with_primary_enrollment(:current_and_invited_courses, nil, :observee_user => @student)
|
||||
.map { |c| [c.id, c.primary_enrollment_type] }).to eq [
|
||||
[@observer_course2.id, 'ObserverEnrollment'],
|
||||
[@observer_course.id, 'ObserverEnrollment']
|
||||
|
@ -824,12 +824,39 @@ describe User do
|
|||
|
||||
it "returns only own courses if the associated_user is the current user" do
|
||||
expect(@observer
|
||||
.courses_with_primary_enrollment(:current_and_invited_courses, nil, :observee_user => @observer.id)
|
||||
.courses_with_primary_enrollment(:current_and_invited_courses, nil, :observee_user => @observer)
|
||||
.map { |c| [c.id, c.primary_enrollment_type] }).to eq [
|
||||
[@teacher_course.id, 'TeacherEnrollment'],
|
||||
[@student_course.id, 'StudentEnrollment']
|
||||
]
|
||||
end
|
||||
|
||||
describe "with cross sharding" do
|
||||
specs_require_sharding
|
||||
|
||||
before(:once) do
|
||||
@shard2.activate do
|
||||
account = Account.create!
|
||||
course_with_teacher(account: account, active_all: true)
|
||||
|
||||
observer_enrollment = @observer_course.enroll_user(@teacher, 'ObserverEnrollment', :enrollment_state => 'active')
|
||||
observer_enrollment.associated_user_id = @student.id
|
||||
observer_enrollment.save!
|
||||
end
|
||||
end
|
||||
|
||||
it "returns the user's courses across shards when they are the observee" do
|
||||
own_courses = @teacher.courses_with_primary_enrollment(:current_and_invited_courses, nil, :observee_user => @teacher)
|
||||
expect(own_courses.count).to be 1
|
||||
expect(own_courses.first.id).to be @course.id
|
||||
end
|
||||
|
||||
it "returns the observer's courses across shards when observing someone else" do
|
||||
observed_courses = @teacher.courses_with_primary_enrollment(:current_and_invited_courses, nil, :observee_user => @student)
|
||||
expect(observed_courses.count).to be 1
|
||||
expect(observed_courses.first.id).to be @observer_course.id
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
it "includes invitations to temporary users" do
|
||||
|
|
Loading…
Reference in New Issue