return cross-shard course rubrics for instructors

test plan:
* when using "find a rubric" should be able to find
 rubrics from cross-shard courses the user is enrolled
 as a teacher in

closes #CORE-1934

Change-Id: Ic69c571cf8013af1894d9980df4ccc9e20121766
Reviewed-on: https://gerrit.instructure.com/165954
Tested-by: Jenkins
QA-Review: Jeremy Putnam <jeremyp@instructure.com>
Reviewed-by: Rob Orton <rob@instructure.com>
Product-Review: James Williams <jamesw@instructure.com>
This commit is contained in:
James Williams 2018-09-26 14:49:21 -06:00
parent bbc37a7158
commit 3e7c064061
5 changed files with 86 additions and 19 deletions

View File

@ -181,8 +181,15 @@ class GradebooksController < ApplicationController
if context
@rubric_context = Context.find_by_asset_string(params[:context_code])
end
@rubric_associations = @context.sorted_rubrics(@current_user, @rubric_context)
render :json => @rubric_associations.map{ |r| r.as_json(methods: [:context_name], include: {:rubric => {:include_root => false}}) }
@rubric_associations = @rubric_context.shard.activate { Context.sorted_rubrics(@current_user, @rubric_context) }
data = @rubric_associations.map{ |ra|
json = ra.as_json(methods: [:context_name], include: {:rubric => {:include_root => false}})
# return shard-aware context codes
json["rubric_association"]["context_code"] = ra.context.asset_string
json["rubric_association"]["rubric"]["context_code"] = ra.rubric.context.asset_string
json
}
render :json => StringifyIds.recursively_stringify_ids(data)
else
render :json => @rubric_contexts
end

View File

@ -82,27 +82,33 @@ module Context
end
end
def sorted_rubrics(user, context)
associations = RubricAssociation.bookmarked.for_context_codes(context.asset_string).include_rubric
def self.sorted_rubrics(user, context)
associations = RubricAssociation.bookmarked.for_context_codes(context.asset_string).preload(:rubric => :context)
Canvas::ICU.collate_by(associations.to_a.uniq(&:rubric_id).select{|r| r.rubric }) { |r| r.rubric.title || CanvasSort::Last }
end
def rubric_contexts(user)
context_codes = [self.asset_string]
context_codes.concat(([user] + user.management_contexts).uniq.map(&:asset_string)) if user
context = self
while context && context.respond_to?(:account) || context.respond_to?(:parent_account)
context = context.respond_to?(:account) ? context.account : context.parent_account
context_codes << context.asset_string if context
associations = []
course_ids = [self.id]
course_ids = (course_ids + user.participating_instructor_course_ids.map{|id| Shard.relative_id_for(id, user.shard, Shard.current)}).uniq if user
Shard.partition_by_shard(course_ids) do |sharded_course_ids|
context_codes = sharded_course_ids.map{|id| "course_#{id}"}
if Shard.current == self.shard
context = self
while context && context.respond_to?(:account) || context.respond_to?(:parent_account)
context = context.respond_to?(:account) ? context.account : context.parent_account
context_codes << context.asset_string if context
end
end
associations += RubricAssociation.bookmarked.for_context_codes(context_codes).include_rubric.preload(:context).to_a
end
associations = RubricAssociation.bookmarked.for_context_codes(context_codes).include_rubric
associations = associations.to_a.select(&:rubric).uniq{|a| [a.rubric_id, a.context_code] }
contexts = associations.group_by(&:context_code).map do |code, code_associations|
context_name = code_associations.first.context_name
associations = associations.select(&:rubric).uniq{|a| [a.rubric_id, a.context.asset_string] }
contexts = associations.group_by{|a| a.context.asset_string}.map do |code, code_associations|
{
:rubrics => code_associations.length,
:context_code => code,
:name => context_name
:name => code_associations.first.context_name
}
end
Canvas::ICU.collate_by(contexts) { |r| r[:name] }

View File

@ -133,8 +133,10 @@ class RubricAssociation < ActiveRecord::Base
end
def context_name
@cached_context_name ||= Rails.cache.fetch(['short_name_lookup', self.context_code].cache_key) do
self.context.short_name rescue ""
@cached_context_name ||= self.shard.activate do
Rails.cache.fetch(['short_name_lookup', self.context_code].cache_key) do
self.context.short_name rescue ""
end
end
end

View File

@ -17,7 +17,7 @@
# with this program. If not, see <http://www.gnu.org/licenses/>.
#
require File.expand_path(File.dirname(__FILE__) + '/../spec_helper')
require File.expand_path(File.dirname(__FILE__) + '/../sharding_spec_helper')
describe GradebooksController do
before :once do
@ -2111,4 +2111,26 @@ describe GradebooksController do
expect(@controller.post_grades_feature?).to eq(true)
end
end
describe "#grading_rubrics" do
context "sharding" do
specs_require_sharding
it "should fetch rubrics from a cross-shard course" do
user_session(@teacher)
@shard1.activate do
a = Account.create!
@cs_course = Course.create!(:name => 'cs_course', :account => a)
@rubric = Rubric.create!(context: @cs_course, title: 'testing')
RubricAssociation.create!(context: @cs_course, rubric: @rubric, purpose: :bookmark, association_object: @cs_course)
@cs_course.enroll_user(@teacher, "TeacherEnrollment", :enrollment_state => "active")
end
get "grading_rubrics", params: {:course_id => @course, :context_code => @cs_course.asset_string}
json = json_parse(response.body)
expect(json.first["rubric_association"]["rubric_id"]).to eq @rubric.global_id.to_s
expect(json.first["rubric_association"]["context_code"]).to eq @cs_course.global_asset_string
end
end
end
end

View File

@ -16,7 +16,7 @@
# with this program. If not, see <http://www.gnu.org/licenses/>.
#
require File.expand_path(File.dirname(__FILE__) + '/../spec_helper.rb')
require File.expand_path(File.dirname(__FILE__) + '/../sharding_spec_helper.rb')
describe Context do
context "find_by_asset_string" do
@ -219,6 +219,36 @@ describe Context do
{ name: 'ZZZ', rubrics: 1}
])
end
context "sharding" do
specs_require_sharding
it "should retrieve rubrics from other shard courses the teacher belongs to" do
course1 = Course.create!(:name => 'c1')
course2 = Course.create!(:name => 'c2')
course3 = @shard1.activate do
a = Account.create!
Course.create!(:name => 'c3', :account => a)
end
user = user_factory(:active_all => true)
[course1, course2, course3].each do |c|
c.shard.activate do
r = Rubric.create!(context: c, title: 'testing')
RubricAssociation.create!(context: c, rubric: r, purpose: :bookmark, association_object: c)
c.enroll_user(user, "TeacherEnrollment", :enrollment_state => "active")
end
end
expected = -> { [
{ name: 'c1', rubrics: 1, context_code: course1.asset_string},
{ name: 'c2', rubrics: 1, context_code: course2.asset_string},
{ name: 'c3', rubrics: 1, context_code: course3.asset_string}
] }
expect(course1.rubric_contexts(user)).to match_array(expected.call)
@shard1.activate do
expect(course2.rubric_contexts(user)).to match_array(expected.call)
end
end
end
end
describe "#active_record_types" do