Add cross-sharding support for discussion topics index endpoint

- Refactors related scopes for Switchman support.
- Adds an explicit shard context around the ScopedToUser call.
- Works around Rails loading with ActiveRecord descendants:
  https://github.com/rails/rails/issues/3364

fixes CNVS-24104

test plan:
- Enroll a student in two different institutions on different shards
- Merge the student if necessary
- Enroll the student in course 1 on institution 1
- Add a discussion topic for course 1 from the student.
- Access the API discussion endpoint from institution 2
- Verify that you get results.

The cross-sharded API endpoint should look something like:

institution_2.instructure.com/api/v1/courses/{global_course_id}/discussion_topics

eg

institution_2.instructure.com/api/v1/courses/10000000000001/discussion_topics

The non-sharded API endpoint should look something like:

instiution_1.instructure.com/api/v1/courses/1/discussion_topics

Both should work.

Change-Id: I54da2f5e012924c10923a8894ad8263a67fcdb06
Reviewed-on: https://gerrit.instructure.com/66599
Reviewed-by: Cody Cutrer <cody@instructure.com>
Tested-by: Jenkins
QA-Review: Michael Hargiss <mhargiss@instructure.com>
Product-Review: Chris Wang <cwang@instructure.com>
This commit is contained in:
Chris Wang 2015-11-05 13:18:56 -06:00
parent 5f364e8e91
commit ea9e89c5ac
3 changed files with 33 additions and 11 deletions

View File

@ -277,7 +277,11 @@ class DiscussionTopicsController < ApplicationController
@context.active_discussion_topics.only_discussion_topics
end
scope = DiscussionTopic::ScopedToUser.new(@context, @current_user, scope).scope
# Specify the shard context, because downstream we use `union` which isn't
# cross-shard compatible.
@context.shard.activate do
scope = DiscussionTopic::ScopedToUser.new(@context, @current_user, scope).scope
end
scope = if params[:order_by] == 'recent_activity'
scope.by_last_reply_at

View File

@ -19,6 +19,9 @@
require 'atom'
# Force loaded so that it will be in ActiveRecord::Base.descendants for switchman to use
require_dependency 'assignment_student_visibility'
class DiscussionTopic < ActiveRecord::Base
include Workflow
@ -64,6 +67,9 @@ class DiscussionTopic < ActiveRecord::Base
has_many :child_topics, :class_name => 'DiscussionTopic', :foreign_key => :root_topic_id, :dependent => :destroy
has_many :discussion_topic_participants, :dependent => :destroy
has_many :discussion_entry_participants, :through => :discussion_entries
has_many :assignment_student_visibilities, :through => :assignment
belongs_to :user
EXPORTABLE_ATTRIBUTES = [
@ -508,15 +514,8 @@ class DiscussionTopic < ActiveRecord::Base
}
scope :joins_assignment_student_visibilities, lambda { |user_ids, course_ids|
user_ids = Array.wrap(user_ids).join(',')
course_ids = Array.wrap(course_ids).join(',')
joins(sanitize_sql([<<-SQL, user_ids, course_ids]))
JOIN assignment_student_visibilities
ON (assignment_student_visibilities.assignment_id = discussion_topics.assignment_id
AND assignment_student_visibilities.user_id IN (%s)
AND assignment_student_visibilities.course_id IN (%s)
)
SQL
joins(:assignment_student_visibilities)
.where(assignment_student_visibilities: { user_id: user_ids, course_id: course_ids })
}
alias_attribute :available_from, :delayed_post_at

View File

@ -1,4 +1,4 @@
#
# Copyright (C) 2012 Instructure, Inc.
#
# This file is part of Canvas.
@ -17,6 +17,7 @@
#
require File.expand_path(File.dirname(__FILE__) + '/../spec_helper')
require File.expand_path(File.dirname(__FILE__) + '/../sharding_spec_helper.rb')
describe DiscussionTopicsController do
before :once do
@ -111,6 +112,24 @@ describe DiscussionTopicsController do
end
end
context "cross-sharding" do
specs_require_sharding
it 'returns the topic across shards' do
@topic = @course.discussion_topics.create!(title: 'student topic', message: 'Hello', user: @student)
user_session(@student)
@shard1.activate do
get 'index', { format: :json, course_id: @course.id }
expect(assigns[:topics]).to include(@topic)
end
@shard2.activate do
get 'index', { format: :json, course_id: @course.id }
expect(assigns[:topics]).to include(@topic)
end
end
end
it "should return non-graded group discussions properly" do
@course.account.role_overrides.create!(
role: student_role,