Don't enumerate page ids in WikiPage#visible_to_user

The WikiPageVisibilityService needs both a user id and either course
ids or page ids to run performantly. Previously, we were providing
the ids of the pages currently selected by the scope, but the way this
scope is used, the scope was targeting almost all pages on the shard.
Thus, we'd be enumerating quite a few page ids on larger shards.

Fortunately, this scope is only used in 1 place (the planner), so
instead of providing user_id+wiki_page_ids to the service, we'll
have the planner pass the course_ids it knows the user is interested
in and the scope will pass those to the visibility service.

closes LX-1824
flag = selective_release_backend

Test plan:
 - Create some pages with todo dates in the next week
 - Assign some of those pages to a student
 - Visit the planner as a student and expect to see only
   the pages they've been assigned

Change-Id: I500ab52959dbc28764d84026d8732f2706ad8888
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/349987
Reviewed-by: Sarah Gerard <sarah.gerard@instructure.com>
QA-Review: Sarah Gerard <sarah.gerard@instructure.com>
Product-Review: Jackson Howe <jackson.howe@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
This commit is contained in:
Jackson Howe 2024-06-12 10:44:22 -06:00
parent 0397316105
commit 195284c4c7
5 changed files with 53 additions and 18 deletions

View File

@ -453,8 +453,7 @@ module UserLearningObjectScopes
objects_needing("WikiPage", "viewing", :student, params, 120.minutes, **opts) do |wiki_pages_context, shard_course_ids, shard_group_ids|
wiki_pages_context
.available_to_planner
.visible_to_user(self)
.for_courses_and_groups(shard_course_ids, shard_group_ids)
.visible_to_user_in_courses_and_groups(self, shard_course_ids, shard_group_ids)
.todo_date_between(due_after, due_before)
end
end

View File

@ -103,23 +103,21 @@ class WikiPage < ActiveRecord::Base
where.not(Ignore.where(asset_type: "WikiPage", user_id: user, purpose:).where("asset_id=wiki_pages.id").arel.exists)
}
scope :todo_date_between, ->(starting, ending) { where(todo_date: starting...ending) }
scope :for_courses_and_groups, lambda { |course_ids, group_ids|
scope :visible_to_user_in_courses_and_groups, lambda { |user_id, course_ids, group_ids|
wiki_ids = []
wiki_ids += Course.where(id: course_ids).pluck(:wiki_id) if course_ids.any?
wiki_ids += Group.where(id: group_ids).pluck(:wiki_id) if group_ids.any?
where(wiki_id: wiki_ids)
}
scope :visible_to_user, lambda { |user_id|
context_pages = where(wiki_id: wiki_ids)
if Account.site_admin.feature_enabled?(:selective_release_backend)
scope_assignments = where.not(assignment_id: nil).pluck(:assignment_id)
visible_wiki_pages = WikiPageVisibility::WikiPageVisibilityService.wiki_pages_visible_to_student_by_pages(user_id:, wiki_page_ids: ids).map(&:wiki_page_id)
scope_assignments = context_pages.where.not(assignment_id: nil).pluck(:assignment_id)
visible_wiki_pages = WikiPageVisibility::WikiPageVisibilityService.wiki_pages_visible_to_student_in_courses(user_id:, course_ids:).map(&:wiki_page_id)
visible_assignments = AssignmentVisibility::AssignmentVisibilityService.assignments_visible_to_student_by_assignment(user_id:, assignment_ids: scope_assignments).map(&:assignment_id)
where("wiki_pages.assignment_id IS NULL AND (wiki_pages.id IN (?) OR wiki_pages.context_type = 'Group')", visible_wiki_pages)
.or(where("wiki_pages.assignment_id IS NOT NULL AND wiki_pages.assignment_id IN (?)", visible_assignments))
context_pages.where("wiki_pages.assignment_id IS NULL AND (wiki_pages.id IN (?) OR wiki_pages.context_type = 'Group')", visible_wiki_pages)
.or(context_pages.where("wiki_pages.assignment_id IS NOT NULL AND wiki_pages.assignment_id IN (?)", visible_assignments))
else
where("wiki_pages.assignment_id IS NULL OR EXISTS (SELECT 1 FROM #{AssignmentStudentVisibility.quoted_table_name} asv WHERE wiki_pages.assignment_id = asv.assignment_id AND asv.user_id = ?)", user_id)
context_pages.where("wiki_pages.assignment_id IS NULL OR EXISTS (SELECT 1 FROM #{AssignmentStudentVisibility.quoted_table_name} asv WHERE wiki_pages.assignment_id = asv.assignment_id AND asv.user_id = ?)", user_id)
end
}

View File

@ -38,6 +38,15 @@ module WikiPageVisibility
wiki_pages_visible_to_students(course_id_params: course_ids, user_id_params: user_ids)
end
def wiki_pages_visible_to_student_in_courses(user_id:, course_ids:)
raise ArgumentError, "course_ids cannot be nil" if course_ids.nil?
raise ArgumentError, "course_ids must be an array" unless course_ids.is_a?(Array)
raise ArgumentError, "user_id cannot be nil" if user_id.nil?
raise ArgumentError, "user_id must not be an array" if user_id.is_a?(Array)
wiki_pages_visible_to_students(course_id_params: course_ids, user_id_params: user_id)
end
def wiki_page_visible_to_student(wiki_page_id:, user_id:)
raise ArgumentError, "wiki_page_id cannot be nil" if wiki_page_id.nil?
raise ArgumentError, "wiki_page_id must not be an array" if wiki_page_id.is_a?(Array)

View File

@ -1219,6 +1219,16 @@ describe UserLearningObjectScopes do
expect(student2.wiki_pages_needing_viewing(**opts)).to eq [@course_page]
end
it "does not show wiki pages that are not visible to the user" do
Account.site_admin.enable_feature! :selective_release_backend
@course_page.update!(todo_date: 1.day.from_now, only_visible_to_overrides: true)
section2 = add_section("Section 2")
student2 = student_in_section(section2)
@course_page.assignment_overrides.create!(set: section2)
expect(@student.wiki_pages_needing_viewing(**opts)).to eq []
expect(student2.wiki_pages_needing_viewing(**opts)).to eq [@course_page]
end
context "include_concluded" do
before :once do
@u = User.create!

View File

@ -1251,7 +1251,7 @@ describe WikiPage do
end
end
describe "visible_ids_by_user and visible_to_user" do
describe "visible_ids_by_user and visible_to_user_in_courses_and_groups" do
before :once do
@course1 = course_factory(active_all: true)
@page1 = @course1.wiki_pages.create!(title: "page1")
@ -1264,7 +1264,7 @@ describe WikiPage do
def assert_visible(user, pages)
visible_ids_by_user = WikiPage.visible_ids_by_user({ user_id: [user.id], course_id: [@course1.id] })
visible_to_user = WikiPage.visible_to_user(user.id).pluck(:id)
visible_to_user = WikiPage.visible_to_user_in_courses_and_groups(user.id, [@course1.id], []).pluck(:id)
expect(visible_ids_by_user[user.id]).to contain_exactly(*pages.map(&:id))
expect(visible_to_user).to contain_exactly(*pages.map(&:id))
end
@ -1295,10 +1295,19 @@ describe WikiPage do
expect(visible_ids_by_user[@student1.id]).to contain_exactly(@page1.id)
end
it "includes group pages" do
group = group_model(context: @course1)
group_page = group.wiki_pages.create!(title: "group page")
expect(WikiPage.visible_to_user(@student1.id)).to include(group_page)
context "group pages" do
before :once do
@group = group_model(context: @course1)
@group_page = @group.wiki_pages.create!(title: "group page")
end
it "includes group pages" do
expect(WikiPage.visible_to_user_in_courses_and_groups(@student1.id, [@course1.id], [@group.id])).to include(@group_page)
end
it "does not include group pages not in group_ids" do
expect(WikiPage.visible_to_user_in_courses_and_groups(@student1.id, [@course1.id], [])).not_to include(@group_page)
end
end
context "with selective_release_backend disabled" do
@ -1346,6 +1355,16 @@ describe WikiPage do
assert_visible(@student1, [])
assert_visible(@student2, [])
end
it "does not include pages from another course" do
course2 = course_factory(active_all: true)
course2.wiki_pages.create!(title: "page3")
student_in_course(course: course2, user: @student1, active_all: true)
page4 = course2.wiki_pages.create!(title: "page2")
assignment = course2.assignments.create!(title: "assignment")
page4.update!(assignment_id: assignment.id)
assert_visible(@student1, [@page1])
end
end
end
end