diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index d0b16226d32..66b313afac9 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -648,18 +648,17 @@ 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 = @context.enrollments.current.shard(@context).preload(:course, :enrollment_state) + enrollment_scope = Enrollment.for_user(@context).active_by_date group_scope = opts[:include_groups] ? @context.current_groups : nil + courses = [] if only_contexts.present? # find only those courses and groups passed in the only_contexts # parameter, but still scoped by user so we know they have rights to # view them. course_ids = only_contexts.select { |c| c.first == "Course" }.map(&:last) - if course_ids.empty? - enrollment_scope = enrollment_scope.none - else - enrollment_scope = enrollment_scope.where(:course_id => course_ids) + unless course_ids.empty? + courses = Course.where(:id => course_ids).where(:id => enrollment_scope.select(:course_id)).to_a end if group_scope group_ids = only_contexts.select { |c| c.first == "Group" }.map(&:last) @@ -669,8 +668,11 @@ class ApplicationController < ActionController::Base group_scope = group_scope.where(:id => group_ids) end end + else + courses = Course.shard(@context).where(:id => enrollment_scope.select(:course_id)).to_a + enrollment_scope = Enrollment.for_user(@context).active_by_date end - courses = enrollment_scope.select(&:active?).map(&:course).uniq + groups = group_scope ? group_scope.shard(@context).to_a.reject{|g| g.context_type == "Course" && g.context.concluded?} : [] if opts[:favorites_first] diff --git a/app/controllers/conferences_controller.rb b/app/controllers/conferences_controller.rb index 56d567a2296..244c70b4cb9 100644 --- a/app/controllers/conferences_controller.rb +++ b/app/controllers/conferences_controller.rb @@ -183,9 +183,7 @@ class ConferencesController < ApplicationController } log_asset_access([ "conferences", @context ], "conferences", "other") if @context.is_a? Course - enrollments = @context.typical_current_enrollments.eager_load(:user).where.not(user_id: @current_user.id).order(User.sortable_name_order_by_clause).to_a - Canvas::Builders::EnrollmentDateBuilder.preload_state(enrollments) - @users = enrollments.select(&:active?).map(&:user).uniq + @users = User.where(:id => @context.typical_current_enrollments.active_by_date.select(:user_id)).where.not(id: @current_user.id).order(User.sortable_name_order_by_clause).to_a else @users = @context.users.where("users.id<>?", @current_user).order(User.sortable_name_order_by_clause).to_a.uniq end diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index a2b2c6df1bc..aff6e80f54b 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -498,14 +498,14 @@ class UsersController < ApplicationController }) @announcements = AccountNotification.for_user_and_account(@current_user, @domain_root_account) - @pending_invitations = @current_user.cached_current_enrollments(:include_enrollment_uuid => session[:enrollment_uuid], :preload_dates => true).select { |e| e.invited? } + @pending_invitations = @current_user.cached_invitations(:include_enrollment_uuid => session[:enrollment_uuid], :preload_course => true) @stream_items = @current_user.try(:cached_recent_stream_items) || [] end def cached_upcoming_events(user) Rails.cache.fetch(['cached_user_upcoming_events', user].cache_key, :expires_in => 3.minutes) do - user.upcoming_events :contexts => ([user] + user.cached_contexts) + user.upcoming_events :context_codes => ([user.asset_string] + user.cached_context_codes) end end diff --git a/app/helpers/dashboard_helper.rb b/app/helpers/dashboard_helper.rb index 4afe6c81df2..d7ae6d9625d 100644 --- a/app/helpers/dashboard_helper.rb +++ b/app/helpers/dashboard_helper.rb @@ -22,12 +22,11 @@ module DashboardHelper end def show_welcome_message? - @current_user.present? && - @current_user.cached_current_enrollments(:include_enrollment_uuid => session[:enrollment_uuid], :preload_dates => true).all?{|e| !e.active?} + @current_user.present? && !@current_user.has_active_enrollment? end def welcome_message - if @current_user.cached_current_enrollments(:include_future => true).present? + if @current_user.has_future_enrollment? t('#users.welcome.unpublished_courses_message', <<-BODY) You've enrolled in one or more courses that have not started yet. Once those courses are available, you will see information about them here diff --git a/app/models/course.rb b/app/models/course.rb index 8ae92fe983f..7a802b70ac1 100644 --- a/app/models/course.rb +++ b/app/models/course.rb @@ -226,6 +226,7 @@ class Course < ActiveRecord::Base after_save :update_final_scores_on_weighting_scheme_change after_save :update_account_associations_if_changed after_save :update_enrollment_states_if_necessary + after_save :touch_students_if_necessary after_save :set_self_enrollment_code before_save :touch_root_folder_if_necessary @@ -726,24 +727,29 @@ class Course < ActiveRecord::Base return unless user RequestCache.cache('user_is_instructor', self, user) do Rails.cache.fetch([self, user, "course_user_is_instructor"].cache_key) do - user.cached_current_enrollments(preload_dates: true).any? { |e| e.course_id == self.id && e.participating_instructor? } + self.enrollments.for_user(user).active_by_date.of_instructor_type.exists? end end end def user_is_student?(user, opts = {}) return unless user + RequestCache.cache('user_is_student', self, user, opts) do - Rails.cache.fetch([self, user, "course_user_is_student", opts[:include_future]].cache_key) do - current_enrollments = user.cached_current_enrollments( - preload_dates: true, include_future: opts[:include_future]) - current_enrollments.any? do |enrollment| - enrollment.course_id == self.id && - enrollment.student_with_conditions?( - include_future: opts[:include_future], - include_fake_student: opts[:include_fake_student] - ) + Rails.cache.fetch([self, user, "course_user_is_student", opts].cache_key) do + enroll_types = ["StudentEnrollment"] + enroll_types << ["StudentViewEnrollment"] if opts[:include_fake_student] + + enroll_scope = self.enrollments.for_user(user).where(:type => enroll_types) + if opts[:include_future] + enroll_scope = enroll_scope.active_or_pending_by_date_ignoring_access + elsif opts[:include_all] + enroll_scope = enroll_scope.not_inactive_by_date_ignoring_access + else + return false unless self.available? + enroll_scope = enroll_scope.active_by_date end + enroll_scope.exists? end end end @@ -1270,56 +1276,51 @@ class Course < ActiveRecord::Base given { |user, session| session && session[:enrollment_uuid] && (hash = Enrollment.course_user_state(self, session[:enrollment_uuid]) || {}) && (hash[:enrollment_state] == "invited" || hash[:enrollment_state] == "active" && hash[:user_state].to_s == "pre_registered") && (self.available? || self.completed? || self.claimed? && hash[:is_admin]) } can :read and can :read_outcomes - given { |user| (self.available? || self.completed?) && user && user.cached_current_enrollments(preload_dates: true).any?{|e| e.course_id == self.id && [:active, :invited, :accepted, :completed].include?(e.state_based_on_date) } } + given { |user| (self.available? || self.completed?) && user && enrollments.for_user(user).not_inactive_by_date.exists? } can :read and can :read_outcomes # Active students given { |user| - available? && user && - user.cached_current_enrollments(preload_dates: true).any? { |e| - e.course_id == id && e.participating_student? - } + available? && user && enrollments.for_user(user).active_by_date.of_student_type.exists? } can :read and can :participate_as_student and can :read_grades and can :read_outcomes - given { |user| (self.available? || self.completed?) && user && user.cached_not_ended_enrollments.any?{|e| e.course_id == self.id && e.participating_observer? && e.associated_user_id} } + given { |user| (self.available? || self.completed?) && user && + enrollments.for_user(user).active_by_date.where(:type => "ObserverEnrollment").where.not(:associated_user_id => nil).exists? } can :read_grades - given { |user| self.available? && self.teacherless? && user && user.cached_not_ended_enrollments.any?{|e| e.course_id == self.id && e.participating_student? } } + given { |user| self.available? && self.teacherless? && user && enrollments.for_user(user).active_by_date.of_student_type.exists? } can :update and can :delete and RoleOverride.teacherless_permissions.each{|p| can p } # Active admins (Teacher/TA/Designer) - given { |user| (self.available? || self.created? || self.claimed?) && user && user.cached_not_ended_enrollments.any?{|e| e.course_id == self.id && e.participating_admin? } } + given { |user| (self.available? || self.created? || self.claimed?) && user && + enrollments.for_user(user).of_admin_type.active_by_date.exists? } can :read_as_admin and can :read and can :manage and can :update and can :use_student_view and can :read_outcomes and can :view_unpublished_items and can :manage_feature_flags # Teachers and Designers can delete/reset, but not TAs - given { |user| !self.deleted? && !self.sis_source_id && user && user.cached_not_ended_enrollments.any?{|e| - e.course_id == self.id && e.participating_content_admin? && e.has_permission_to?(:change_course_state) - } } + given { |user| !self.deleted? && !self.sis_source_id && user && + enrollments.for_user(user).of_content_admins.active_by_date.to_a.any?{|e| e.has_permission_to?(:change_course_state)} + } can :delete - given { |user| !self.deleted? && user && user.cached_not_ended_enrollments.any?{|e| e.course_id == self.id && e.participating_content_admin? } } + given { |user| !self.deleted? && user && enrollments.for_user(user).of_content_admins.active_by_date.exists? } can :reset_content # Student view student - given { |user| user && user.fake_student? && user.cached_not_ended_enrollments.any?{ |e| e.course_id == self.id } } + given { |user| user && user.fake_student? && current_enrollments.for_user(user).exists? } can :read and can :participate_as_student and can :read_grades and can :read_outcomes # Prior users given do |user| (available? || completed?) && user && - prior_enrollments.for_user(user).preload(:enrollment_state).any?{|e| !e.inactive?} + enrollments.for_user(user).completed_by_date.exists? end can :read, :read_outcomes # Admin (Teacher/TA/Designer) of a concluded course given do |user| !self.deleted? && user && - (prior_enrollments.for_user(user).any?{|e| e.admin? } || - user.cached_not_ended_enrollments.any? do |e| - e.course_id == self.id && e.admin? && e.completed? - end - ) + enrollments.for_user(user).of_admin_type.completed_by_date.exists? end can [:read, :read_as_admin, :use_student_view, :read_outcomes, :view_unpublished_items] @@ -1329,11 +1330,7 @@ class Course < ActiveRecord::Base given do |user| !self.deleted? && user && - (prior_enrollments.for_user(user).preload(:enrollment_state).any?{|e| !e.inactive? && e.has_permission_to?(permission) && (!applicable_roles || applicable_roles.include?(e.type))} || - user.cached_not_ended_enrollments.any? do |e| - e.course_id == self.id && e.completed? && e.has_permission_to?(permission) && (!applicable_roles || applicable_roles.include?(e.type)) - end - ) + enrollments.for_user(user).completed_by_date.to_a.any?{|e| e.has_permission_to?(permission) && (!applicable_roles || applicable_roles.include?(e.type))} end can permission end @@ -1341,22 +1338,15 @@ class Course < ActiveRecord::Base # Teacher or Designer of a concluded course given do |user| !self.deleted? && !self.sis_source_id && user && - (prior_enrollments.for_user(user).any?{|e| e.content_admin? && e.has_permission_to?(:change_course_state)} || - user.cached_not_ended_enrollments.any? do |e| - e.course_id == self.id && e.content_admin? && e.state_based_on_date == :completed && e.has_permission_to?(:change_course_state) - end - ) + enrollments.for_user(user).of_content_admins.completed_by_date.to_a.any?{|e| e.has_permission_to?(:change_course_state)} end can :delete # Student of a concluded course given do |user| (self.available? || self.completed?) && user && - (prior_enrollments.for_user(user).preload(:enrollment_state).any?{|e| (e.student? || e.assigned_observer?) && !e.inactive?} || - user.cached_not_ended_enrollments.any? do |e| - e.course_id == self.id && (e.student? || e.assigned_observer?) && e.state_based_on_date == :completed - end - ) + enrollments.for_user(user).completed_by_date. + where("enrollments.type = ? OR (enrollments.type = ? AND enrollments.associated_user_id IS NOT NULL)", "StudentEnrollment", "ObserverEnrollment").exists? end can :read, :read_grades, :read_outcomes @@ -2994,6 +2984,21 @@ class Course < ActiveRecord::Base User.where(id: self.admins).touch_all end + def touch_students_if_necessary + # to update the cached current enrollments + if workflow_state_changed? && (workflow_state == 'available' || workflow_state_was == 'available') + touch_students_later if self.students.exists? + end + end + + def touch_students_later + send_later_enqueue_args(:touch_students, { :run_at => 15.seconds.from_now, :singleton => "course_touch_students_#{global_id}" }) + end + + def touch_students + User.where(id: self.students).touch_all + end + def list_students_by_sortable_name? feature_enabled?(:gradebook_list_students_by_sortable_name) end diff --git a/app/models/course_progress.rb b/app/models/course_progress.rb index b0bad87b44b..df5bbd68d69 100644 --- a/app/models/course_progress.rb +++ b/app/models/course_progress.rb @@ -127,7 +127,7 @@ class CourseProgress end def to_json - if course.module_based? && course.user_is_student?(user, include_future: true) + if course.module_based? && course.user_is_student?(user, include_all: true) { requirement_count: requirement_count, requirement_completed_count: requirement_completed_count, diff --git a/app/models/enrollment.rb b/app/models/enrollment.rb index 0cf03fd409b..37807c338da 100644 --- a/app/models/enrollment.rb +++ b/app/models/enrollment.rb @@ -64,6 +64,7 @@ class Enrollment < ActiveRecord::Base before_validation :ensure_role_id before_validation :infer_privileges after_create :create_linked_enrollments + after_create :create_enrollment_state after_save :clear_email_caches after_save :cancel_future_appointments after_save :update_linked_enrollments @@ -239,6 +240,8 @@ class Enrollment < ActiveRecord::Base joins(:course). where("enrollments.type IN ('TeacherEnrollment','TaEnrollment') AND (courses.workflow_state='claimed' OR (enrollments.workflow_state='active' AND courses.workflow_state='available'))") } + scope :of_student_type, -> { where(:type => "StudentEnrollment") } + scope :of_admin_type, -> { where(:type => ['TeacherEnrollment','TaEnrollment', 'DesignerEnrollment']) } scope :of_instructor_type, -> { where(:type => ['TeacherEnrollment', 'TaEnrollment']) } @@ -672,6 +675,13 @@ class Enrollment < ActiveRecord::Base def enrollment_state raise "cannot call enrollment_state on a new record" if new_record? state = self.association(:enrollment_state).target ||= + self.shard.activate { EnrollmentState.where(:enrollment_id => self).first } + state.association(:enrollment).target ||= self # ensure reverse association + state + end + + def create_enrollment_state + self.enrollment_state = self.shard.activate do Shackles.activate(:master) do EnrollmentState.unique_constraint_retry do @@ -679,8 +689,6 @@ class Enrollment < ActiveRecord::Base end end end - state.association(:enrollment).target ||= self # ensure reverse association - state end def recalculate_enrollment_state @@ -1062,6 +1070,22 @@ class Enrollment < ActiveRecord::Base scope :active_or_pending, -> { where("enrollments.workflow_state NOT IN ('rejected', 'completed', 'deleted', 'inactive')") } scope :all_active_or_pending, -> { where("enrollments.workflow_state NOT IN ('rejected', 'completed', 'deleted')") } # includes inactive + scope :active_by_date, -> { joins(:enrollment_state).where("enrollment_states.restricted_access = ?", false). + where("enrollment_states.state = 'active'") } + scope :invited_by_date, -> { joins(:enrollment_state).where("enrollment_states.restricted_access = ?", false). + where("enrollment_states.state IN ('invited', 'pending_invited')") } + scope :active_or_pending_by_date, -> { joins(:enrollment_state).where("enrollment_states.restricted_access = ?", false). + where("enrollment_states.state IN ('active', 'invited', 'pending_invited', 'pending_active')") } + scope :completed_by_date, -> { joins(:enrollment_state).where("enrollment_states.restricted_access = ?", false). + where("enrollment_states.state = ?", "completed") } + scope :not_inactive_by_date, -> { joins(:enrollment_state).where("enrollment_states.restricted_access = ?", false). + where("enrollment_states.state IN ('active', 'invited', 'completed', 'pending_invited', 'pending_active')") } + + scope :active_or_pending_by_date_ignoring_access, -> { joins(:enrollment_state). + where("enrollment_states.state IN ('active', 'invited', 'pending_invited', 'pending_active')") } + scope :not_inactive_by_date_ignoring_access, -> { joins(:enrollment_state). + where("enrollment_states.state IN ('active', 'invited', 'completed', 'pending_invited', 'pending_active')") } + scope :currently_online, -> { joins(:pseudonyms).where("pseudonyms.last_request_at>?", 5.minutes.ago) } # this returns enrollments for creation_pending users; should always be used in conjunction with the invited scope scope :for_email, lambda { |email| diff --git a/app/models/enrollment_state.rb b/app/models/enrollment_state.rb index 3920c77025d..ee8ecb0472d 100644 --- a/app/models/enrollment_state.rb +++ b/app/models/enrollment_state.rb @@ -74,7 +74,7 @@ class EnrollmentState < ActiveRecord::Base self.access_is_current = false end - if self.state_changed? && self.state == "active" + if self.state_changed? && (self.state == "active" || self.state_was == "active") # we could make this happen on every state change, to expire cached authorization right away but # the permissions cache expires within an hour anyway # this will at least prevent cached unauthorization @@ -83,9 +83,6 @@ class EnrollmentState < ActiveRecord::Base self.enrollment.user.touch end end - - # TODO: remove when diagnostic columns are removed - self.state_recalculated_at = Time.now.utc unless is_direct_recalculation end def calculate_state_based_on_dates @@ -143,14 +140,11 @@ class EnrollmentState < ActiveRecord::Base self.restricted_access = false end self.access_is_current = true - - # TODO: remove when diagnostic columns are removed - self.access_recalculated_at = Time.now.utc end def self.enrollments_needing_calculation(scope=Enrollment.all) - scope.joins("LEFT OUTER JOIN #{EnrollmentState.quoted_table_name} ON enrollment_states.enrollment_id = enrollments.id"). - where("enrollment_states IS NULL OR enrollment_states.state_is_current = ? OR enrollment_states.access_is_current = ?", false, false) + scope.joins(:enrollment_state). + where("enrollment_states.state_is_current = ? OR enrollment_states.access_is_current = ?", false, false) end def self.process_states_in_ranges(start_at, end_at, enrollment_scope=Enrollment.all) @@ -196,7 +190,7 @@ class EnrollmentState < ActiveRecord::Base INVALIDATEABLE_STATES = %w{pending_invited pending_active invited active completed inactive}.freeze # don't worry about creation_pending or rejected, etc def self.invalidate_states(enrollment_scope) - EnrollmentState.where(:enrollment_id => enrollment_scope, :state_is_current => true, :state => INVALIDATEABLE_STATES).update_all(:state_is_current => false, :state_invalidated_at => Time.now.utc) + EnrollmentState.where(:enrollment_id => enrollment_scope, :state_is_current => true, :state => INVALIDATEABLE_STATES).update_all(:state_is_current => false) end def self.force_recalculation(enrollment_ids) @@ -207,7 +201,7 @@ class EnrollmentState < ActiveRecord::Base end def self.invalidate_access(enrollment_scope, states_to_update) - EnrollmentState.where(:enrollment_id => enrollment_scope, :access_is_current => true, :state => states_to_update).update_all(:access_is_current => false, :access_invalidated_at => Time.now.utc) + EnrollmentState.where(:enrollment_id => enrollment_scope, :access_is_current => true, :state => states_to_update).update_all(:access_is_current => false) end def self.enrollments_for_account_ids(account_ids) @@ -263,7 +257,6 @@ class EnrollmentState < ActiveRecord::Base def self.recalculate_expired_states while (enrollments = Enrollment.joins(:enrollment_state).where("enrollment_states.state_valid_until IS NOT NULL AND enrollment_states.state_valid_until < ?", Time.now.utc).limit(250).to_a) && enrollments.any? - EnrollmentState.where(:enrollment_id => enrollments).update_all("state_invalidated_at = state_valid_until") # temporary, to determine how long it took to update process_states_for(enrollments) end end diff --git a/app/models/group.rb b/app/models/group.rb index 3533a89d36c..e800ead879e 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -563,7 +563,7 @@ class Group < ActiveRecord::Base return false unless user.present? && self.context.present? return true if self.group_category.try(:communities?) if self.context.is_a?(Course) - return self.context.enrollments.not_fake.except(:preload).preload(:enrollment_state).where(:user_id => user.id).any?(&:participating?) + return self.context.enrollments.not_fake.where(:user_id => user.id).active_by_date.exists? elsif self.context.is_a?(Account) return self.context.root_account.user_account_associations.where(:user_id => user.id).exists? end diff --git a/app/models/user.rb b/app/models/user.rb index 43d643e66a7..443c00aeda8 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1753,15 +1753,29 @@ class User < ActiveRecord::Base end end - def cached_not_ended_enrollments - RequestCache.cache("not_ended_enrollments", self) do - self.shard.activate do - enrollments = Rails.cache.fetch([self, 'not_ended_enrollments2'].cache_key) do - self.not_ended_enrollments.to_a - end - Canvas::Builders::EnrollmentDateBuilder.preload_state(enrollments) - enrollments - end + def cached_invitations(opts={}) + enrollments = Rails.cache.fetch([self, 'invited_enrollments', ApplicationController.region ].cache_key) do + self.enrollments.shard(in_region_associated_shards).invited_by_date.to_a + end + if opts[:include_enrollment_uuid] && !enrollments.find { |e| e.uuid == opts[:include_enrollment_uuid] } && + (pending_enrollment = Enrollment.invited_by_date.where(uuid: opts[:include_enrollment_uuid]).first) + enrollments << pending_enrollment + end + enrollments += temporary_invitations + ActiveRecord::Associations::Preloader.new.preload(enrollments, :course) if opts[:preload_course] + enrollments + end + + def has_active_enrollment? + # don't need an expires_at here because user will be touched upon enrollment activation + Rails.cache.fetch([self, 'has_active_enrollment', ApplicationController.region ].cache_key) do + self.enrollments.shard(in_region_associated_shards).current.active_by_date.exists? + end + end + + def has_future_enrollment? + Rails.cache.fetch([self, 'has_future_enrollment', ApplicationController.region ].cache_key, :expires_in => 1.hour) do + self.enrollments.shard(in_region_associated_shards).active_or_pending_by_date.exists? end end @@ -1778,17 +1792,25 @@ class User < ActiveRecord::Base end def participating_student_course_ids - participating_enrollments.select(&:student?).map(&:course_id).uniq + @participating_student_course_ids ||= self.shard.activate do + Rails.cache.fetch([self, 'participating_student_course_ids', ApplicationController.region].cache_key) do + self.enrollments.shard(in_region_associated_shards).of_student_type.current.active_by_date.distinct.pluck(:course_id) + end + end end def participating_instructor_course_ids - participating_enrollments.select(&:instructor?).map(&:course_id).uniq + @participating_instructor_course_ids ||= self.shard.activate do + Rails.cache.fetch([self, 'participating_instructor_course_ids', ApplicationController.region].cache_key) do + self.enrollments.shard(in_region_associated_shards).of_instructor_type.current.active_by_date.distinct.pluck(:course_id) + end + end end def participating_enrollments @participating_enrollments ||= self.shard.activate do - Rails.cache.fetch([self, 'participating_enrollments'].cache_key, :expires_in => 1.hour) do - self.cached_current_enrollments(:preload_dates => true).select(&:participating?) + Rails.cache.fetch([self, 'participating_enrollments', ApplicationController.region].cache_key) do + self.enrollments.shard(in_region_associated_shards).current.active_by_date.to_a end end end @@ -1861,8 +1883,7 @@ class User < ActiveRecord::Base # still need to optimize the query to use a root_context_code. that way a # users course dashboard even if they have groups does a query with # "context_code=..." instead of "context_code IN ..." - conditions = setup_context_association_lookups("stream_item_instances.context", opts[:contexts]) - instances = instances.where(conditions) unless conditions.first.empty? + instances = instances.polymorphic_where('stream_item_instances.context' => opts[:contexts]) elsif opts[:context] instances = instances.where(:context_type => opts[:context].class.base_class.name, :context_id => opts[:context]) end @@ -1987,12 +2008,10 @@ class User < ActiveRecord::Base def select_available_assignments(assignments) return [] if assignments.empty? - enrollments = Shard.partition_by_shard(assignments.map(&:context_id)) do |course_ids| - self.enrollments.shard(Shard.current).where(course_id: course_ids).to_a + available_course_ids = Shard.partition_by_shard(assignments.map(&:context_id)) do |course_ids| + self.enrollments.shard(Shard.current).where(course_id: course_ids).active_by_date.pluck(:course_id) end - Canvas::Builders::EnrollmentDateBuilder.preload_state(enrollments) - enrollments.select! {|e| e.participating? } - assignments.select {|a| enrollments.any? {|e| e.course_id == a.context_id} } + assignments.select {|a| available_course_ids.include?(a.context_id) } end def select_upcoming_assignments(assignments,opts) @@ -2019,60 +2038,34 @@ class User < ActiveRecord::Base Canvas::ICU.collate_by(undated_events, &:title) end - def setup_context_lookups(contexts=nil) + def setup_context_lookups(contexts) # TODO: All the event methods use this and it's really slow. - Array(contexts || cached_contexts).map(&:asset_string) + Array(contexts).map(&:asset_string) end - def setup_context_association_lookups(column, contexts=nil, opts = {}) - contexts = Array(contexts || cached_contexts) - conditions = [[]] - backcompat = opts[:backcompat] - contexts.map do |context| - if backcompat - conditions.first << "((#{column}_type=? AND #{column}_id=?) OR (#{column}_code=? AND #{column}_type IS NULL))" - else - conditions.first << "(#{column}_type=? AND #{column}_id=?)" - end - conditions.concat [context.class.base_class.name, context.id] - conditions << context.asset_string if backcompat - end - conditions[0] = conditions[0].join(" OR ") - conditions - end - - # TODO: doesn't actually cache, needs to be optimized - def cached_contexts - @cached_contexts ||= begin - context_groups = [] - # according to the set_policy block in group.rb, user u can manage group - # g if either: - # (a) g.context.grants_right?(u, :manage_groups) - # (b) g.has_member?(u) - # this is a very performance sensitive method, so we're bypassing the - # normal policy checking and somewhat duplicating auth logic here. which - # is a shame. it'd be really nice to add support to our policy framework - # for understanding how to load associations based on policies. - - # :manage_groups is only available for admin enrollments - admin_enrolls = self.enrollments.current.of_admin_type - group_admin_courses = self.courses_for_enrollments(admin_enrolls).preload(:active_groups).select do |c| - c.active_groups.any? && c.grants_right?(self, :manage_groups) - end - group_admin_courses.each do |c| - context_groups += c.active_groups - end - active_courses = cached_current_enrollments(preload_dates: true, preload_courses: true). - select(&:participating?). - map(&:course). - uniq - active_courses + (self.groups.active + context_groups).uniq - end - end - - # TODO: doesn't actually cache, needs to be optimized def cached_context_codes - Array(self.cached_contexts).map(&:asset_string) + # (hopefully) don't need to include cross-shard because calendar events/assignments/etc are only seached for on current shard anyway + @cached_context_codes ||= + Rails.cache.fetch([self, 'cached_context_codes', Shard.current].cache_key, :expires_in => 15.minutes) do + group_admin_course_ids = + Rails.cache.fetch([self, 'group_admin_course_ids', Shard.current].cache_key, :expires_in => 1.hour) do + # permissions are cached for an hour anyways + admin_enrolls = self.enrollments.shard(Shard.current).of_admin_type.active_by_date + Course.where(:id => admin_enrolls.select(:course_id)).to_a.select{|c| c.grants_right?(self, :manage_groups)}.map(&:id) + end + + group_ids = group_admin_course_ids.any? ? + Group.active.where(:context_type => "Course", :context_id => group_admin_course_ids).pluck(:id) : [] + group_ids += self.groups.active.pluck(:id) + group_ids.uniq! + + cached_current_course_ids = Rails.cache.fetch([self, 'cached_current_course_ids', Shard.current].cache_key) do + # don't need an expires at because user will be touched if enrollment state changes from 'active' + self.enrollments.shard(Shard.current).active_by_date.distinct.pluck(:course_id) + end + + cached_current_course_ids.map{|id| "course_#{id}" } + group_ids.map{|id| "group_#{id}"} + end end # context codes of things that might have a schedulable appointment for the @@ -2094,7 +2087,7 @@ class User < ActiveRecord::Base def manageable_appointment_context_codes @manageable_appointment_context_codes ||= Rails.cache.fetch([self, 'cached_manageable_appointment_codes', ApplicationController.region ].cache_key, expires_in: 1.day) do ret = {:full => [], :limited => [], :secondary => []} - cached_current_enrollments(preload_dates: true).each do |e| + cached_current_enrollments(preload_courses: true).each do |e| next unless e.course.grants_right?(self, :manage_calendar) if e.course.visibility_limited_to_course_sections?(self) ret[:limited] << "course_#{e.course_id}" diff --git a/config/initializers/dropped_columns.rb b/config/initializers/dropped_columns.rb index 34793819c8b..34a43cfc7c6 100644 --- a/config/initializers/dropped_columns.rb +++ b/config/initializers/dropped_columns.rb @@ -101,6 +101,7 @@ class ActiveRecord::Base limit_priveleges_to_course_sections role_name sis_source_id).freeze, + 'enrollment_states' => %w{state_invalidated_at state_recalculated_at access_invalidated_at access_recalculated_at}.freeze, 'eportfolio_entries' => %w(attachment_id artifact_type url).freeze, 'eportfolios' => %w{context_id context_type}.freeze, 'external_feeds' => %w(body_match feed_type feed_purpose).freeze, diff --git a/db/migrate/20160810134616_drop_enrollment_state_log_columns.rb b/db/migrate/20160810134616_drop_enrollment_state_log_columns.rb new file mode 100644 index 00000000000..dc9d9124080 --- /dev/null +++ b/db/migrate/20160810134616_drop_enrollment_state_log_columns.rb @@ -0,0 +1,17 @@ +class DropEnrollmentStateLogColumns < ActiveRecord::Migration + tag :postdeploy + + def up + remove_column :enrollment_states, :state_invalidated_at + remove_column :enrollment_states, :state_recalculated_at + remove_column :enrollment_states, :access_invalidated_at + remove_column :enrollment_states, :access_recalculated_at + end + + def down + add_column :enrollment_states, :state_invalidated_at, :datetime + add_column :enrollment_states, :state_recalculated_at, :datetime + add_column :enrollment_states, :access_invalidated_at, :datetime + add_column :enrollment_states, :access_recalculated_at, :datetime + end +end diff --git a/lib/canvas/live_events.rb b/lib/canvas/live_events.rb index d622f98f95c..9b8ac60ed54 100644 --- a/lib/canvas/live_events.rb +++ b/lib/canvas/live_events.rb @@ -158,12 +158,7 @@ module Canvas::LiveEvents state_is_current: enrollment_state.state_is_current, state_valid_until: enrollment_state.state_valid_until, restricted_access: enrollment_state.restricted_access, - access_is_current: enrollment_state.access_is_current, - state_invalidated_at: enrollment_state.state_invalidated_at, - state_recalculated_at: enrollment_state.state_recalculated_at, - access_invalidated_at: enrollment_state.access_invalidated_at, - access_recalculated_at: enrollment_state.access_recalculated_at - + access_is_current: enrollment_state.access_is_current } end diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index 37a29176b98..b87b147ed5d 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -667,12 +667,7 @@ describe ApplicationController do user controller.instance_variable_set(:@context, @user) - course_scope = stub('current_enrollments') - course_scope.stubs(:current).returns(course_scope) - course_scope.stubs(:shard).returns(course_scope) - course_scope.stubs(:preload).returns(course_scope) - course_scope.expects(:none).returns(Enrollment.none) - @user.stubs(:enrollments).returns(course_scope) + Course.expects(:where).never controller.send(:get_all_pertinent_contexts, only_contexts: 'Group_1') end diff --git a/spec/models/enrollment_spec.rb b/spec/models/enrollment_spec.rb index 452814aa6c7..03c3db11d23 100644 --- a/spec/models/enrollment_spec.rb +++ b/spec/models/enrollment_spec.rb @@ -755,6 +755,7 @@ describe Enrollment do @enrollment.workflow_state = 'active' expect(@enrollment.reload.state).to eql(:active) expect(@enrollment.state_based_on_date).to eql(:active) + expect(Enrollment.where(:id => @enrollment).active_by_date.first).to eq @enrollment end it "should return completed" do @@ -764,6 +765,7 @@ describe Enrollment do @term.save! expect(@enrollment.reload.state).to eql(:active) expect(@enrollment.state_based_on_date).to eql(:completed) + expect(Enrollment.where(:id => @enrollment).active_by_date.first).to be_nil end it "should return accepted for students (inactive for admins) if upcoming and available" do @@ -784,6 +786,9 @@ describe Enrollment do @course.save! expect(@enrollment.reload.state).to eql(:active) expect(@enrollment.state_based_on_date).to eql(@enrollment.admin? ? :active : :inactive) + if @enrollment.student? + expect(Enrollment.where(:id => @enrollment).active_by_date.first).to be_nil + end end end diff --git a/spec/models/enrollment_state_spec.rb b/spec/models/enrollment_state_spec.rb index 593b7ce942d..b2a6af55fbd 100644 --- a/spec/models/enrollment_state_spec.rb +++ b/spec/models/enrollment_state_spec.rb @@ -3,7 +3,7 @@ require_relative "../spec_helper" describe EnrollmentState do describe "#enrollments_needing_calculation" do - it "should find enrollments that don't have enrollment states (yet) as well" do + it "should find enrollments that need calculation" do course normal_enroll = student_in_course(:course => @course) @@ -12,16 +12,13 @@ describe EnrollmentState do invalidated_enroll2 = student_in_course(:course => @course) EnrollmentState.where(:enrollment_id => invalidated_enroll2).update_all(:access_is_current => false) - missing_enroll = student_in_course(:course => @course) - EnrollmentState.where(:enrollment_id => missing_enroll).delete_all - - expect(EnrollmentState.enrollments_needing_calculation.to_a).to match_array([invalidated_enroll1, invalidated_enroll2, missing_enroll]) + expect(EnrollmentState.enrollments_needing_calculation.to_a).to match_array([invalidated_enroll1, invalidated_enroll2]) end it "should be able to use a scope" do course enroll = student_in_course(:course => @course) - EnrollmentState.where(:enrollment_id => enroll).delete_all + EnrollmentState.where(:enrollment_id => enroll).update_all(:state_is_current => false) expect(EnrollmentState.enrollments_needing_calculation(Enrollment.where.not(:id => nil)).to_a).to eq [enroll] expect(EnrollmentState.enrollments_needing_calculation(Enrollment.where(:id => nil)).to_a).to be_empty @@ -34,18 +31,6 @@ describe EnrollmentState do @enrollment = student_in_course(:course => @course) end - it "should create missing states" do - EnrollmentState.where(:enrollment_id => @enrollment).delete_all - - @enrollment.reload - - EnrollmentState.process_states_for(@enrollment) - - @enrollment.reload - expect(@enrollment.enrollment_state).to be_present - expect(@enrollment.enrollment_state.state).to eq 'invited' - end - it "should reprocess invalidated states" do EnrollmentState.where(:enrollment_id => @enrollment).update_all(:state_is_current => false, :state => "somethingelse") @@ -319,7 +304,6 @@ describe EnrollmentState do EnrollmentState.recalculate_expired_states enroll_state.reload expect(enroll_state.state).to eq 'completed' - expect(enroll_state.state_invalidated_at).to eq end_at # for diagnostic purposes end end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 66e4107681c..4f336bd04e3 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -1691,6 +1691,7 @@ describe User do event = @course.calendar_events.create!(title: 'published', start_at: 4.days.from_now) expect(@user.upcoming_events).to include(event) Timecop.freeze(3.days.from_now) do + EnrollmentState.recalculate_expired_states # runs periodically in background expect(User.find(@user).upcoming_events).not_to include(event) # re-find user to clear cached_contexts end end @@ -1921,6 +1922,7 @@ describe User do @quiz.due_at = 3.days.from_now @quiz.save! Timecop.travel(2.days) do + EnrollmentState.recalculate_expired_states # runs periodically in background expect(@student.assignments_needing_submitting(:contexts => [@course]).count).to eq 0 end end @@ -2297,6 +2299,7 @@ describe User do it "should not count assignments in soft concluded courses" do @course.enrollment_term.update_attribute(:end_at, 1.day.from_now) Timecop.travel(1.week) do + EnrollmentState.recalculate_expired_states # runs periodically in background expect(@teacher.reload.assignments_needing_grading.size).to eql(0) end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 592e5bc8442..82a00f1c75c 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -865,6 +865,12 @@ RSpec.configure do |config| klass.transaction do klass.connection.bulk_insert klass.table_name, records scope = klass.order("id DESC").limit(records.size) + if klass == Enrollment + scope.to_a.each do |enrollment| + enrollment.create_enrollment_state + enrollment.enrollment_state.ensure_current_state + end + end return_type == :record ? scope.to_a.reverse : scope.pluck(:id).reverse