From a17e7be82457b3f9dba73b10a7599466adf7c73e Mon Sep 17 00:00:00 2001 From: Tyler Pickett Date: Tue, 22 Aug 2017 09:15:37 -0500 Subject: [PATCH] Show cross-shard assignments in the calendar Refs: CNVS-37970 Test Plan: - Enroll a user in a course in a trusted shard that is not their home - Create a dated assignment (NOT calendar event) in the cross-shard course - Create an un-dated assignment (NOT calendar event) in the cross-shard course - View that user's calendar, the assignments should show up in the calendar as well as the undated assignments area Change-Id: I0c5eaa80fc78355e67a0150b760516997e66d0d9 Reviewed-on: https://gerrit.instructure.com/123495 Reviewed-by: Cody Cutrer Tested-by: Jenkins Reviewed-by: Rob Orton QA-Review: Rob Orton Product-Review: Tyler Pickett --- app/controllers/application_controller.rb | 20 ++++++--- .../calendar_events_api_controller.rb | 25 +++++------ app/controllers/calendars_controller.rb | 6 ++- app/models/context.rb | 5 +++ lib/api/v1/calendar_event.rb | 2 +- spec/apis/v1/calendar_events_api_spec.rb | 2 +- .../application_controller_spec.rb | 42 +++++++++++++++++++ 7 files changed, 79 insertions(+), 23 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 393e9b77c72..fd59300bedd 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -740,7 +740,11 @@ class ApplicationController < ActionController::Base # we already know the user can read these courses and groups, so skip # the grants_right? check to avoid querying for the various memberships # again. - enrollment_scope = Enrollment.for_user(@context).current.active_by_date + enrollment_scope = Enrollment + .shard(opts[:cross_shard] ? @context.in_region_associated_shards : Shard.current) + .for_user(@context) + .current + .active_by_date include_groups = !!opts[:include_groups] group_ids = nil @@ -751,15 +755,21 @@ class ApplicationController < ActionController::Base # view them. course_ids = only_contexts.select { |c| c.first == "Course" }.map(&:last) unless course_ids.empty? - courses = Course.where(:id => course_ids).where(:id => enrollment_scope.select(:course_id)).to_a + courses = Course + .shard(opts[:cross_shard] ? @context.in_region_associated_shards : Shard.current) + .joins(enrollments: :enrollment_state) + .merge(enrollment_scope) + .where(id: course_ids) end if include_groups group_ids = only_contexts.select { |c| c.first == "Group" }.map(&:last) - include_groups = false if group_ids.empty? + include_groups = !group_ids.empty? end else - courses = Course.shard(opts[:cross_shard] ? @context.in_region_associated_shards : Shard.current). - where(:id => enrollment_scope.select(:course_id)).to_a + courses = Course + .shard(opts[:cross_shard] ? @context.in_region_associated_shards : Shard.current) + .joins(enrollments: :enrollment_state) + .merge(enrollment_scope) end groups = [] diff --git a/app/controllers/calendar_events_api_controller.rb b/app/controllers/calendar_events_api_controller.rb index 076295198bd..118ade0a428 100644 --- a/app/controllers/calendar_events_api_controller.rb +++ b/app/controllers/calendar_events_api_controller.rb @@ -994,7 +994,12 @@ class CalendarEventsApiController < ApplicationController # only get pertinent contexts if there is a user if user joined_codes = codes && codes.join(",") - get_all_pertinent_contexts(include_groups: true, only_contexts: joined_codes, include_contexts: joined_codes) + get_all_pertinent_contexts( + include_groups: true, + cross_shard: true, + only_contexts: joined_codes, + include_contexts: joined_codes + ) end if codes @@ -1053,20 +1058,12 @@ class CalendarEventsApiController < ApplicationController contexts = @contexts.select { |c| @context_codes.include?(c.asset_string) } view_unpublished, other = contexts.partition { |c| c.grants_right?(user, session, :view_unpublished_items) } - sql = [] - conditions = [] - unless view_unpublished.empty? - sql << '(assignments.context_code IN (?))' - conditions << view_unpublished.map(&:asset_string) - end + base_scope = Assignment + .shard(user&.in_region_associated_shards || Shard.current) - unless other.empty? - sql << '(assignments.context_code IN (?) AND assignments.workflow_state = ?)' - conditions << other.map(&:asset_string) - conditions << 'published' - end - - scope = Assignment.where([sql.join(' OR ')] + conditions) + scope = base_scope + .where(context: other, workflow_state: 'published') + .or(base_scope.where(context: view_unpublished)) return scope if @public_to_auth || !user student_ids = [user.id] diff --git a/app/controllers/calendars_controller.rb b/app/controllers/calendars_controller.rb index 49a129a34c9..929bbe25aa3 100644 --- a/app/controllers/calendars_controller.rb +++ b/app/controllers/calendars_controller.rb @@ -21,8 +21,10 @@ class CalendarsController < ApplicationController def show2 get_context - get_all_pertinent_contexts(include_groups: true, favorites_first: true) - @manage_contexts = @contexts.select{|c| c.grants_right?(@current_user, session, :manage_calendar) }.map(&:asset_string) + get_all_pertinent_contexts(include_groups: true, favorites_first: true, cross_shard: true) + @manage_contexts = @contexts.select { |c| + c.grants_right?(@current_user, session, :manage_calendar) + }.map(&:asset_string) @feed_url = feeds_calendar_url((@context_enrollment || @context).feed_code) if params[:include_contexts] @selected_contexts = params[:include_contexts].split(",") diff --git a/app/models/context.rb b/app/models/context.rb index c1f0415d776..02c17656bf0 100644 --- a/app/models/context.rb +++ b/app/models/context.rb @@ -149,6 +149,11 @@ module Context result end + def self.context_code_for(record) + raise ArgumentError unless record.respond_to?(:context_type) && record.respond_to?(:context_id) + "#{record.context_type.underscore}_#{record.context_id}" + end + def self.find_polymorphic(context_type, context_id) klass_name = context_type.classify.to_sym if CONTEXT_TYPES.include?(klass_name) diff --git a/lib/api/v1/calendar_event.rb b/lib/api/v1/calendar_event.rb index 0eef8f1cb63..4cbee9d4bd8 100644 --- a/lib/api/v1/calendar_event.rb +++ b/lib/api/v1/calendar_event.rb @@ -170,7 +170,7 @@ module Api::V1::CalendarEvent hash['assignment'] = assignment_json(assignment, user, session, override_dates: false, submission: options[:submission]) hash['html_url'] = hash['assignment']['html_url'] if hash['assignment'].include?('html_url') end - hash['context_code'] = assignment.context_code + hash['context_code'] = Context.context_code_for(assignment) hash['start_at'] = hash['end_at'] = assignment.due_at hash['url'] = api_v1_calendar_event_url("assignment_#{assignment.id}") if assignment.applied_overrides.present? diff --git a/spec/apis/v1/calendar_events_api_spec.rb b/spec/apis/v1/calendar_events_api_spec.rb index ab45d0d7bfd..dd8601223a2 100644 --- a/spec/apis/v1/calendar_events_api_spec.rb +++ b/spec/apis/v1/calendar_events_api_spec.rb @@ -1,5 +1,5 @@ # -# Copyright (C) 2011 - 2014 Instructure, Inc. +# Copyright (C) 2011 - present Instructure, Inc. # # This file is part of Canvas. # diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index e871dabef64..b0309fa8c9a 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -720,6 +720,48 @@ describe ApplicationController do controller.send(:get_all_pertinent_contexts, include_groups: true, only_contexts: "group_#{@other_group.id},group_#{@group.id}") expect(controller.instance_variable_get(:@contexts).select{|c| c.is_a?(Group)}).to eq [@group] end + + it 'must select all cross-shard courses the user belongs to' do + user_factory(active_all: true) + controller.instance_variable_set(:@context, @user) + + account = Account.create! + enrollment1 = course_with_teacher(user: @user, active_all: true, account: account) + course1 = enrollment1.course + + enrollment2 = @shard1.activate do + account = Account.create! + course_with_teacher(user: @user, active_all: true, account: account) + end + course2 = enrollment2.course + + controller.send(:get_all_pertinent_contexts, cross_shard: true) + contexts = controller.instance_variable_get(:@contexts) + expect(contexts).to include course1, course2 + end + + it 'must select only the specified cross-shard courses when only_contexts is included' do + user_factory(active_all: true) + controller.instance_variable_set(:@context, @user) + + account = Account.create! + enrollment1 = course_with_teacher(user: @user, active_all: true, account: account) + course1 = enrollment1.course + + enrollment2 = @shard1.activate do + account = Account.create! + course_with_teacher(user: @user, active_all: true, account: account) + end + course2 = enrollment2.course + + controller.send(:get_all_pertinent_contexts, { + cross_shard: true, + only_contexts: "Course_#{course2.id}", + }) + contexts = controller.instance_variable_get(:@contexts) + expect(contexts).to_not include course1 + expect(contexts).to include course2 + end end end