ylqjgm
/
canvas-lms
mirror of https://github.com/instructure/canvas-lms.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

remove uses of memoize 

fixes CNVS-9331

instead, as appropriate, use one of (or a combination of, if necessary):

 * instance variable caching
 * Rails.cache
 * query caching (implicit)

also:

 * remove the buggy cc.active_pseudonyms (didn't account for
   sharding) in favor of cc.user.all_active_pseudonyms
 * streamline assignments in the menu to not need to construct method
   names

test-plan: N/A

Change-Id: Id0dec60464a283985e39493b90711b32cb5cca82
Reviewed-on: https://gerrit.instructure.com/26936
Reviewed-by: Cody Cutrer <cody@instructure.com>
Tested-by: Jenkins <jenkins@instructure.com>
Product-Review: Jacob Fugal <jacob@instructure.com>
QA-Review: Jacob Fugal <jacob@instructure.com>
This commit is contained in:
Jacob Fugal 2013-12-03 16:00:14 -07:00
parent 9d4e2891d8
commit 34893f80bd
24 changed files with 286 additions and 315 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
app/messages/forgot_password.email.erb
View File

@ -6,18 +6,20 @@
<%= t('subject', 'Forgot Password: Canvas') %>
<% end %>
<% pseudonyms = asset.user.all_active_pseudonyms %>
<% first_pseudonym = pseudonyms.first %>
<%= t('requested_password_reset', 'You requested a confirmation of your password for logging into Canvas.') %>
<% if asset.active_pseudonyms.length > 1 %><%= before_label('associated_with_accounts', 'This address is associated with the following accounts') %>
<% if pseudonyms.length > 1 %><%= before_label('associated_with_accounts', 'This address is associated with the following accounts') %>
<% asset.active_pseudonyms.each do |p| %>
<% pseudonyms.each do |p| %>
- <%= t('login_at_account', '%{login_email} at %{account_name}', :login_email => p.unique_id, :account_name => p.account.display_name) %>
<% if p.managed_password? %><%= t('login_managed_by_account', "this login's credentials are managed by %{account_name}", :account_name => p.account.name) %><% else %><%= before_label('change_password_at', "change this login's password at") %>
<%= HostUrl.protocol %>://<%= HostUrl.context_host((p.account rescue nil)) %>/pseudonyms/<%= p.id %>/change_password/<%= (p.communication_channel || asset).confirmation_code %><% end %>
<% end %>
<% else %>
<%= t('associated_login', 'This address is associated with the login, "%{login_identifier}".', :login_identifier => asset.active_pseudonyms.first.unique_id) %><% if asset.active_pseudonyms.first.managed_password? %> <%= t('password_from_account', "The password for this login should have been given to you by the system administrators at %{account_name}, and Instructure doesn't have access to your password. If your password is not working, please contact the system administrators about changing or verifying your password.", :account_name => asset.active_pseudonyms.first.account.display_name) %><% else %>
<%= t('associated_login', 'This address is associated with the login, "%{login_identifier}".', :login_identifier => first_pseudonym.unique_id) %><% if first_pseudonym.managed_password? %> <%= t('password_from_account', "The password for this login should have been given to you by the system administrators at %{account_name}, and Instructure doesn't have access to your password. If your password is not working, please contact the system administrators about changing or verifying your password.", :account_name => first_pseudonym.account.display_name) %><% else %>
<%= before_label('click_to_reset', 'To set a new password, please click the following link') %>
<%= HostUrl.protocol %>://<%= HostUrl.context_host((asset.active_pseudonyms.first.account rescue nil)) %>/pseudonyms/<%= asset.active_pseudonyms.first.id %>/change_password/<%= asset.confirmation_code %><% end %>
<%= HostUrl.protocol %>://<%= HostUrl.context_host((first_pseudonym.account rescue nil)) %>/pseudonyms/<%= first_pseudonym.id %>/change_password/<%= asset.confirmation_code %><% end %>
<% end %>

14
app/messages/forgot_password.email.html.erb
View File

@ -6,10 +6,12 @@
<% end %>
<p><%= t('requested_password_reset', 'You requested a confirmation of your password for logging into Canvas.') %></p>
<% if asset.active_pseudonyms.length > 1 -%>
<% pseudonyms = asset.user.all_active_pseudonyms %>
<% first_pseudonym = pseudonyms.first %>
<% if pseudonyms.length > 1 -%>
<p><%= before_label('associated_with_accounts', 'This address is associated with the following accounts') %></p>
<% asset.active_pseudonyms.each do |p| %>
<% pseudonyms.each do |p| %>
<p>- <%= t('login_at_account', '%{login_email} at %{account_name}', :login_email => p.unique_id, :account_name => p.account.display_name) %><br/>
<% if p.managed_password? %>
<%= t('login_managed_by_account', "this login's credentials are managed by %{account_name}", :account_name => p.account.name) %>
@ -20,9 +22,9 @@
</p>
<% end %>
<% else %>
<p><%= t('associated_login', 'This address is associated with the login, "%{login_identifier}".', :login_identifier => asset.active_pseudonyms.first.unique_id) %></p>
<% if asset.active_pseudonyms.first.managed_password? %>
<p><%= t('password_from_account', "The password for this login should have been given to you by the system administrators at %{account_name}, and Instructure doesn't have access to your password. If your password is not working, please contact the system administrators about changing or verifying your password.", :account_name => asset.active_pseudonyms.first.account.display_name) %></p>
<p><%= t('associated_login', 'This address is associated with the login, "%{login_identifier}".', :login_identifier => first_pseudonym.unique_id) %></p>
<% if first_pseudonym.managed_password? %>
<p><%= t('password_from_account', "The password for this login should have been given to you by the system administrators at %{account_name}, and Instructure doesn't have access to your password. If your password is not working, please contact the system administrators about changing or verifying your password.", :account_name => first_pseudonym.account.display_name) %></p>
<% else %>
<p><a href="<%= HostUrl.protocol %>://<%= HostUrl.context_host((asset.active_pseudonyms.first.account rescue nil)) %>/pseudonyms/<%= asset.active_pseudonyms.first.id %>/change_password/<%= asset.confirmation_code %>"><%= t('click_to_reset', 'Click here to set a new password') %></a><% end %>
<p><a href="<%= HostUrl.protocol %>://<%= HostUrl.context_host((first_pseudonym.account rescue nil)) %>/pseudonyms/<%= first_pseudonym.id %>/change_password/<%= asset.confirmation_code %>"><%= t('click_to_reset', 'Click here to set a new password') %></a><% end %>
<% end %>

56
app/models/account.rb
View File

@ -513,29 +513,32 @@ class Account < ActiveRecord::Base
end
def account_chain(opts = {})
res = [self]
if ActiveRecord::Base.configurations[Rails.env]['adapter'] == 'postgresql'
self.shard.activate do
res.concat Account.find_by_sql(<<-SQL) if self.parent_account_id
WITH RECURSIVE t AS (
SELECT * FROM accounts WHERE id=#{self.parent_account_id}
UNION
SELECT accounts.* FROM accounts INNER JOIN t ON accounts.id=t.parent_account_id
)
SELECT * FROM t
SQL
end
else
account = self
while account.parent_account
account = account.parent_account
res << account
unless @account_chain
res = [self]
if ActiveRecord::Base.configurations[Rails.env]['adapter'] == 'postgresql'
self.shard.activate do
res.concat Account.find_by_sql(<<-SQL) if self.parent_account_id
WITH RECURSIVE t AS (
SELECT * FROM accounts WHERE id=#{self.parent_account_id}
UNION
SELECT accounts.* FROM accounts INNER JOIN t ON accounts.id=t.parent_account_id
)
SELECT * FROM t
SQL
end
else
account = self
while account.parent_account
account = account.parent_account
res << account
end
end
res << self.root_account unless res.map(&:id).include?(self.root_account_id)
@account_chain = res.compact
end
res << self.root_account unless res.map(&:id).include?(self.root_account_id)
res << Account.site_admin if opts[:include_site_admin] && !self.site_admin?
res.compact
results = @account_chain.dup
results << Account.site_admin if opts[:include_site_admin] && !self.site_admin?
results
end
def account_chain_loop
@ -583,7 +586,6 @@ class Account < ActiveRecord::Base
def account_chain_ids(opts={})
account_chain(opts).map(&:id)
end
memoize :account_chain_ids
def membership_for_user(user)
self.account_users.find_by_user_id(user && user.id)
@ -951,17 +953,14 @@ class Account < ActiveRecord::Base
def course_count
self.child_courses.not_deleted.count('DISTINCT course_id')
end
memoize :course_count
def sub_account_count
self.sub_accounts.active.count
end
memoize :sub_account_count
def user_count
self.user_account_associations.count
end
memoize :user_count
def current_sis_batch
if (current_sis_batch_id = self.read_attribute(:current_sis_batch_id)) && current_sis_batch_id.present?
@ -970,10 +969,11 @@ class Account < ActiveRecord::Base
end
def turnitin_settings
return @turnitin_settings if defined?(@turnitin_settings)
if self.turnitin_account_id.present? && self.turnitin_shared_secret.present?
[self.turnitin_account_id, self.turnitin_shared_secret, self.turnitin_host]
@turnitin_settings = [self.turnitin_account_id, self.turnitin_shared_secret, self.turnitin_host]
else
self.parent_account.try(:turnitin_settings)
@turnitin_settings = self.parent_account.try(:turnitin_settings)
end
end

1
app/models/asset_user_access.rb
View File

@ -150,7 +150,6 @@ class AssetUserAccess < ActiveRecord::Base
asset = Context.find_asset_by_asset_string(asset_code, context)
asset
end
memoize :asset
def asset_class_name
self.asset.class.name.underscore if self.asset

40
app/models/attachment.rb
View File

@ -365,31 +365,25 @@ class Attachment < ActiveRecord::Base
# or just some_attachment.scribd_thumbnail #will give you the default tumbnail for the document.
def scribd_thumbnail(options={})
return unless self.scribd_doc && ScribdAPI.enabled?
if options.empty? && self.cached_scribd_thumbnail
if options.empty?
# we cache the 'default' version in the DB
unless self.cached_scribd_thumbnail
self.cached_scribd_thumbnail = self.request_scribd_thumbnail(options)
Attachment.where(:id => self).update_all(:cached_scribd_thumbnail => self.cached_scribd_thumbnail)
end
self.cached_scribd_thumbnail
else
begin
# if we aren't requesting special demensions, fetch and save it to the db.
if options.empty?
Scribd::API.instance.user = scribd_user
self.cached_scribd_thumbnail = self.scribd_doc.thumbnail(options)
# just update the cached_scribd_thumbnail column of this attachment without running callbacks
Attachment.where(:id => self).update_all(:cached_scribd_thumbnail => self.cached_scribd_thumbnail)
self.cached_scribd_thumbnail
else
Rails.cache.fetch(['scribd_thumb', self, options].cache_key) do
Scribd::API.instance.user = scribd_user
self.scribd_doc.thumbnail(options)
end
end
rescue Scribd::NotReadyError
nil
rescue => e
nil
# we cache other versions in the rails cache
Rails.cache.fetch(['scribd_thumb', self, options].cache_key) do
self.request_scribd_thumbnail(options)
end
end
end
memoize :scribd_thumbnail
def request_scribd_thumbnail(options)
Scribd::API.instance.user = scribd_user
self.scribd_doc.thumbnail(options)
end
def turnitinable?
self.content_type && [
@ -860,7 +854,6 @@ class Attachment < ActiveRecord::Base
# "still need to handle things that are not images with thumbnails, scribd_docs, or kaltura docs"
end
end
memoize :thumbnail_url
def thumbnail_for_size(geometry)
if self.class.allows_thumbnails_of_size?(geometry)
@ -1214,9 +1207,9 @@ class Attachment < ActiveRecord::Base
end
def hidden?
self.file_state == 'hidden' || (self.folder && self.folder.hidden?)
return @hidden if defined?(@hidden)
@hidden = self.file_state == 'hidden' || (self.folder && self.folder.hidden?)
end
memoize :hidden?
def published?; !locked?; end
@ -1227,7 +1220,6 @@ class Attachment < ActiveRecord::Base
def public?
self.file_state == 'public'
end
memoize :public?
def currently_locked
self.locked || (self.lock_at && Time.now > self.lock_at) || (self.unlock_at && Time.now < self.unlock_at) || self.file_state == 'hidden'

5
app/models/communication_channel.rb
View File

@ -119,11 +119,6 @@ class CommunicationChannel < ActiveRecord::Base
p.context { @root_account }
end
def active_pseudonyms
self.user.pseudonyms.active
end
memoize :active_pseudonyms
def uniqueness_of_path
return if path.nil?
return if retired?

33
app/models/conversation_participant.rb
View File

@ -233,22 +233,27 @@ class ConversationParticipant < ActiveRecord::Base
:include_indirect_participants => false
}.merge(options)
participants = conversation.participants
if options[:include_indirect_participants]
user_ids =
messages.map(&:all_forwarded_messages).flatten.map(&:author_id) |
messages.map{
|m| m.submission.submission_comments.map(&:author_id) if m.submission
}.compact.flatten
user_ids -= participants.map(&:id)
participants += Shackles.activate(:slave) { MessageableUser.available.where(:id => user_ids).all }
shard.activate do
Rails.cache.fetch([conversation, user, 'participants', options].cache_key) do
participants = conversation.participants
if options[:include_indirect_participants]
user_ids =
messages.map(&:all_forwarded_messages).flatten.map(&:author_id) |
messages.map{
|m| m.submission.submission_comments.map(&:author_id) if m.submission
}.compact.flatten
user_ids -= participants.map(&:id)
participants += Shackles.activate(:slave) { MessageableUser.available.where(:id => user_ids).all }
end
if options[:include_participant_contexts]
# we do this to find out the contexts they share with the user
user.load_messageable_users(participants, :strict_checks => false)
else
participants
end
end
end
return participants unless options[:include_participant_contexts]
# we do this to find out the contexts they share with the user
user.load_messageable_users(participants, :strict_checks => false)
end
memoize :participants
def properties(latest = last_message)
properties = []

24
app/models/course.rb
View File

@ -542,7 +542,6 @@ class Course < ActiveRecord::Base
user.cached_current_enrollments.any? { |e| e.course_id == self.id && e.participating_instructor? }
end
end
memoize :user_is_instructor?
def user_is_student?(user, opts = {})
return unless user
@ -552,7 +551,6 @@ class Course < ActiveRecord::Base
}
end
end
memoize :user_is_student?
def user_has_been_instructor?(user)
return unless user
@ -561,7 +559,6 @@ class Course < ActiveRecord::Base
self.instructor_enrollments.active.find_by_user_id(user.id).present?
end
end
memoize :user_has_been_instructor?
def user_has_been_admin?(user)
return unless user
@ -570,7 +567,6 @@ class Course < ActiveRecord::Base
self.admin_enrollments.active.find_by_user_id(user.id).present?
end
end
memoize :user_has_been_admin?
def user_has_been_observer?(user)
return unless user
@ -579,7 +575,6 @@ class Course < ActiveRecord::Base
self.observer_enrollments.active.find_by_user_id(user.id).present?
end
end
memoize :user_has_been_observer?
def user_has_been_student?(user)
return unless user
@ -587,7 +582,6 @@ class Course < ActiveRecord::Base
self.all_student_enrollments.find_by_user_id(user.id).present?
end
end
memoize :user_has_been_student?
def user_has_no_enrollments?(user)
return unless user
@ -595,7 +589,6 @@ class Course < ActiveRecord::Base
enrollments.find_by_user_id(user.id).nil?
end
end
memoize :user_has_no_enrollments?
# Public: Determine if a group weighting scheme should be applied.
@ -751,9 +744,8 @@ class Course < ActiveRecord::Base
end
def long_self_enrollment_code
Digest::MD5.hexdigest("#{uuid}_for_#{id}")
@long_self_enrollment_code ||= Digest::MD5.hexdigest("#{uuid}_for_#{id}")
end
memoize :long_self_enrollment_code
# still include the old longer format, since links may be out there
def self_enrollment_codes
@ -1173,13 +1165,11 @@ class Course < ActiveRecord::Base
def account_chain_ids
account_chain.map(&:id)
end
memoize :account_chain_ids
def institution_name
return self.root_account.name if self.root_account_id != Account.default.id
return (self.account || self.root_account).name
end
memoize :institution_name
def account_users_for(user)
return [] unless user
@ -1260,7 +1250,6 @@ class Course < ActiveRecord::Base
end
message
end
memoize :grade_publishing_status_translation
def grade_publishing_statuses
found_statuses = [].to_set
@ -1750,7 +1739,6 @@ class Course < ActiveRecord::Base
# has valid settings
account.turnitin_settings
end
memoize :turnitin_settings
def turnitin_pledge
self.account.closest_turnitin_pledge
@ -2290,7 +2278,6 @@ class Course < ActiveRecord::Base
end
end
end
memoize :section_visibilities_for
def visibility_limited_to_course_sections?(user, visibilities = section_visibilities_for(user))
visibilities.all?{|s| s[:limit_privileges_to_course_section] }
@ -2482,9 +2469,15 @@ class Course < ActiveRecord::Base
end
def tabs_available(user=nil, opts={})
opts.reverse_merge!(:include_external => true)
cache_key = [user, opts].cache_key
@tabs_available ||= {}
@tabs_available[cache_key] ||= uncached_tabs_available(user, opts)
end
def uncached_tabs_available(user, opts)
# make sure t() is called before we switch to the slave, in case we update the user's selected locale in the process
default_tabs = Course.default_tabs
opts.reverse_merge!(:include_external => true)
Shackles.activate(:slave) do
# We will by default show everything in default_tabs, unless the teacher has configured otherwise.
@ -2586,7 +2579,6 @@ class Course < ActiveRecord::Base
tabs
end
end
memoize :tabs_available
def allow_wiki_comments
read_attribute(:allow_wiki_comments)

6
app/models/delayed_notification.rb
View File

@ -59,6 +59,7 @@ class DelayedNotification < ActiveRecord::Base
end
def to_list
return @to_list if @to_list
lookups = {}
(recipient_keys || []).each do |key|
pieces = key.split('_')
@ -73,10 +74,9 @@ class DelayedNotification < ActiveRecord::Base
includes = [:user] if klass == CommunicationChannel
res += klass.where(:id => ids).includes(includes).all rescue []
end
res.uniq
@to_list = res.uniq
end
memoize :to_list
scope :to_be_processed, lambda { |limit|
where(:workflow_state => 'to_be_processed').limit(limit).order("delayed_notifications.created_at")
}

5
app/models/developer_key.rb
View File

@ -54,10 +54,7 @@ class DeveloperKey < ActiveRecord::Base
@special_keys ||= {}
if Rails.env.test?
# TODO: we have to do this because tests run in transactions. maybe it'd
# be good to create some sort of of memoize_if_safe method, that only
# memoizes when we're caching classes and not in test mode? I dunno. But
# this stinks.
# TODO: we have to do this because tests run in transactions
return @special_keys[default_key_name] = DeveloperKey.find_or_create_by_name(default_key_name)
end

2
app/models/discussion_entry.rb
View File

@ -228,7 +228,7 @@ class DiscussionEntry < ActiveRecord::Base
end
def update_topic_subscription
discussion_topic.user_ids_who_have_posted_and_admins(true) # pesky memoization
discussion_topic.user_ids_who_have_posted_and_admins
unless discussion_topic.user_can_see_posts?(user)
discussion_topic.unsubscribe(user)
end

1
app/models/discussion_topic.rb
View File

@ -518,7 +518,6 @@ class DiscussionTopic < ActiveRecord::Base
ids += self.context.admin_enrollments.active.pluck(:user_id) if self.context.respond_to?(:admin_enrollments)
ids
end
memoize :user_ids_who_have_posted_and_admins
def user_can_see_posts?(user, session=nil)
return false unless user

44
app/models/folder.rb
View File

@ -163,40 +163,40 @@ class Folder < ActiveRecord::Base
res += self.active_file_attachments unless opts[:exclude_files]
res
end
def visible?
# everything but private folders should be visible... for now...
(self.workflow_state == "visible") && (!self.parent_folder || self.parent_folder.visible?)
return @visible if defined?(@visible)
@visible = (self.workflow_state == "visible") && (!self.parent_folder || self.parent_folder.visible?)
end
memoize :visible?
def hidden?
self.workflow_state == 'hidden' || (self.parent_folder && self.parent_folder.hidden?)
return @hidden if defined?(@hidden)
@hidden = self.workflow_state == 'hidden' || (self.parent_folder && self.parent_folder.hidden?)
end
memoize :hidden?
def hidden
hidden?
end
def hidden=(val)
self.workflow_state = (val == true || val == '1' || val == 'true' ? 'hidden' : 'visible')
end
def just_hide
self.workflow_state == 'hidden'
end
def protected?
(self.workflow_state == 'protected') || (self.parent_folder && self.parent_folder.protected?)
return @protected if defined?(@protected)
@protected = (self.workflow_state == 'protected') || (self.parent_folder && self.parent_folder.protected?)
end
memoize :protected?
def public?
self.workflow_state == 'public' || (self.parent_folder && self.parent_folder.public?)
return @public if defined?(@public)
@public = self.workflow_state == 'public' || (self.parent_folder && self.parent_folder.public?)
end
memoize :public?
def mime_class
"folder"
end
@ -351,17 +351,17 @@ class Folder < ActiveRecord::Base
end
def locked?
self.locked ||
(self.lock_at && Time.now > self.lock_at) ||
(self.unlock_at && Time.now < self.unlock_at) ||
(self.parent_folder && self.parent_folder.locked?)
return @locked if defined?(@locked)
@locked = self.locked ||
(self.lock_at && Time.now > self.lock_at) ||
(self.unlock_at && Time.now < self.unlock_at) ||
(self.parent_folder && self.parent_folder.locked?)
end
memoize :locked?
def currently_locked
self.locked || (self.lock_at && Time.now > self.lock_at) || (self.unlock_at && Time.now < self.unlock_at) || self.workflow_state == 'hidden'
end
set_policy do
given { |user, session| self.visible? && self.cached_context_grants_right?(user, session, :read) }#students.include?(user) }
can :read

3
app/models/submission.rb
View File

@ -852,9 +852,8 @@ class Submission < ActiveRecord::Base
end
def commenting_instructors
comment_authors & context.instructors
@commenting_instructors ||= comment_authors & context.instructors
end
memoize :commenting_instructors
def participating_instructors
commenting_instructors.present? ? commenting_instructors : context.participating_instructors.uniq

307
app/models/user.rb
View File

@ -956,9 +956,8 @@ class User < ActiveRecord::Base
end
def courses_with_grades
self.available_courses.with_each_shard.select{|c| c.grants_right?(self, nil, :participate_as_student)}
@courses_with_grades ||= self.available_courses.with_each_shard.select{|c| c.grants_right?(self, nil, :participate_as_student)}
end
memoize :courses_with_grades
def sis_pseudonym_for(context)
root_account = context.root_account
@ -1346,12 +1345,6 @@ class User < ActiveRecord::Base
opts = { :start_at => 1.week.ago, :limit => 10 }.merge(opts)
Submission.recently_graded_assignments(id, opts[:start_at], opts[:limit])
end
memoize :assignments_recently_graded
def assignments_recently_graded_total_count(opts={})
assignments_recently_graded(opts.merge({:limit => nil})).size
end
memoize :assignments_recently_graded_total_count
def preferences
read_attribute(:preferences) || write_attribute(:preferences, {})
@ -1410,73 +1403,71 @@ class User < ActiveRecord::Base
end
def assignments_needing_submitting(opts={})
Shackles.activate(:slave) do
course_ids = if opts[:contexts]
course_ids = Shackles.activate(:slave) do
if opts[:contexts]
(Array(opts[:contexts]).map(&:id) &
current_student_enrollment_course_ids)
else
current_student_enrollment_course_ids
end
# allow explicitly passing a nil limit
limit = opts[:limit]
limit = 15 unless opts.key?(:limit)
due_after = opts[:due_after] || 4.weeks.ago
result = Shard.partition_by_shard(course_ids) do |shard_course_ids|
Assignment.for_course(shard_course_ids).
active.
due_between_with_overrides(due_after,1.week.from_now).
not_ignored_by(self, 'submitting').
expecting_submission.
need_submitting_info(id, limit).
not_locked
end
# outer limit, since there could be limit * n_shards results
result = result[0..(limit - 1)] if limit
result
end
end
memoize :assignments_needing_submitting
# every couple years i try to remove this since it appears to be unused.
# turns out we do actually use it in a view via User#send w/ string
# interpolation ಠ_ಠ
def assignments_needing_submitting_total_count(opts={})
assignments_needing_submitting(opts.merge(:limit => nil)).size
opts = {limit: 15}.merge(opts.slice(:due_after, :limit))
shard.activate do
Rails.cache.fetch([self, 'assignments_needing_submitting', course_ids, opts].cache_key, expires_in: 15.minutes) do
Shackles.activate(:slave) do
limit = opts[:limit]
due_after = opts[:due_after] || 4.weeks.ago
result = Shard.partition_by_shard(course_ids) do |shard_course_ids|
Assignment.for_course(shard_course_ids).
active.
due_between_with_overrides(due_after,1.week.from_now).
not_ignored_by(self, 'submitting').
expecting_submission.
need_submitting_info(id, limit).
not_locked
end
# outer limit, since there could be limit * n_shards results
result = result[0...limit] if limit
result
end
end
end
end
def assignments_needing_grading(opts={})
Shackles.activate(:slave) do
course_ids = if opts[:contexts]
course_ids = Shackles.activate(:slave) do
if opts[:contexts]
(Array(opts[:contexts]).map(&:id) &
current_admin_enrollment_course_ids)
else
current_admin_enrollment_course_ids
end
# allow explicitly passing a nil limit
limit = opts[:limit]
limit = 15 unless opts.key?(:limit)
result = Shard.partition_by_shard(course_ids) do |shard_course_ids|
as = Assignment.for_course(shard_course_ids).active.
expecting_submission.
not_ignored_by(self, 'grading').
need_grading_info(limit)
Assignment.send :preload_associations, as, :context
as.reject{|a| a.needs_grading_count_for_user(self) == 0}
end
# outer limit, since there could be limit * n_shards results
result = result[0..(limit - 1)] if limit
result
end
end
memoize :assignments_needing_grading
# see assignments_needing_submitting_total_count
def assignments_needing_grading_total_count(opts={})
assignments_needing_grading(opts.merge(:limit => nil)).size
opts = {limit: 15}.merge(opts.slice(:limit))
shard.activate do
Rails.cache.fetch([self, 'assignments_needing_grading', course_ids, opts].cache_key, expires_in: 15.minutes) do
Shackles.activate(:slave) do
limit = opts[:limit]
result = Shard.partition_by_shard(course_ids) do |shard_course_ids|
as = Assignment.for_course(shard_course_ids).active.
expecting_submission.
not_ignored_by(self, 'grading').
need_grading_info(limit)
Assignment.send :preload_associations, as, :context
as.reject{|a| a.needs_grading_count_for_user(self) == 0}
end
# outer limit, since there could be limit * n_shards results
result = result[0...limit] if limit
result
end
end
end
end
def generate_access_verifier(ts)
@ -1607,59 +1598,63 @@ class User < ActiveRecord::Base
end
def courses_with_primary_enrollment(association = :current_and_invited_courses, enrollment_uuid = nil, options = {})
res = self.shard.activate do
Rails.cache.fetch([self, 'courses_with_primary_enrollment', association, options].cache_key, :expires_in => 15.minutes) do
cache_key = [association, enrollment_uuid, options].cache_key
@courses_with_primary_enrollment ||= {}
@courses_with_primary_enrollment.fetch(cache_key) do
res = self.shard.activate do
Rails.cache.fetch([self, 'courses_with_primary_enrollment', association, options].cache_key, :expires_in => 15.minutes) do
# Set the actual association based on if its asking for favorite courses or not.
actual_association = association == :favorite_courses ? :current_and_invited_courses : association
# Set the actual association based on if its asking for favorite courses or not.
actual_association = association == :favorite_courses ? :current_and_invited_courses : association
send(actual_association).with_each_shard do |scope|
send(actual_association).with_each_shard do |scope|
# Limit favorite courses based on current shard.
if association == :favorite_courses
local_ids = self.favorite_context_ids("Course")
next if local_ids.length < 1
scope = scope.where(:id => local_ids)
# Limit favorite courses based on current shard.
if association == :favorite_courses
local_ids = self.favorite_context_ids("Course")
next if local_ids.length < 1
scope = scope.where(:id => local_ids)
end
courses = scope.distinct_on(["courses.id"],
:select => "courses.*, enrollments.id AS primary_enrollment_id, enrollments.type AS primary_enrollment, #{Enrollment.type_rank_sql} AS primary_enrollment_rank, enrollments.workflow_state AS primary_enrollment_state",
:order => "courses.id, #{Enrollment.type_rank_sql}, #{Enrollment.state_rank_sql}")
unless options[:include_completed_courses]
enrollments = Enrollment.where(:id => courses.map(&:primary_enrollment_id)).all
courses_hash = courses.index_by(&:id)
# prepopulate the reverse association
enrollments.each { |e| e.course = courses_hash[e.course_id] }
Canvas::Builders::EnrollmentDateBuilder.preload(enrollments)
date_restricted_ids = enrollments.select{ |e| e.completed? || e.inactive? }.map(&:id)
courses.reject! { |course| date_restricted_ids.include?(course.primary_enrollment_id.to_i) }
end
courses
end
end.dup
end
courses = scope.distinct_on(["courses.id"],
:select => "courses.*, enrollments.id AS primary_enrollment_id, enrollments.type AS primary_enrollment, #{Enrollment.type_rank_sql} AS primary_enrollment_rank, enrollments.workflow_state AS primary_enrollment_state",
:order => "courses.id, #{Enrollment.type_rank_sql}, #{Enrollment.state_rank_sql}")
unless options[:include_completed_courses]
enrollments = Enrollment.where(:id => courses.map(&:primary_enrollment_id)).all
courses_hash = courses.index_by(&:id)
# prepopulate the reverse association
enrollments.each { |e| e.course = courses_hash[e.course_id] }
Canvas::Builders::EnrollmentDateBuilder.preload(enrollments)
date_restricted_ids = enrollments.select{ |e| e.completed? || e.inactive? }.map(&:id)
courses.reject! { |course| date_restricted_ids.include?(course.primary_enrollment_id.to_i) }
end
courses
if association == :current_and_invited_courses
if enrollment_uuid && pending_course = Course.
select("courses.*, enrollments.type AS primary_enrollment, #{Enrollment.type_rank_sql} AS primary_enrollment_rank, enrollments.workflow_state AS primary_enrollment_state").
joins(:enrollments).
where(:enrollments => { :uuid => enrollment_uuid, :workflow_state => 'invited' }).first
res << pending_course
res.uniq!
end
pending_enrollments = temporary_invitations
unless pending_enrollments.empty?
Enrollment.send(:preload_associations, pending_enrollments, :course)
res.concat(pending_enrollments.map { |e| c = e.course; c.write_attribute(:primary_enrollment, e.type); c.write_attribute(:primary_enrollment_rank, e.rank_sortable.to_s); c.write_attribute(:primary_enrollment_state, e.workflow_state); c.write_attribute(:invitation, e.uuid); c })
res.uniq!
end
end.dup
end
if association == :current_and_invited_courses
if enrollment_uuid && pending_course = Course.
select("courses.*, enrollments.type AS primary_enrollment, #{Enrollment.type_rank_sql} AS primary_enrollment_rank, enrollments.workflow_state AS primary_enrollment_state").
joins(:enrollments).
where(:enrollments => { :uuid => enrollment_uuid, :workflow_state => 'invited' }).first
res << pending_course
res.uniq!
end
pending_enrollments = temporary_invitations
unless pending_enrollments.empty?
Enrollment.send(:preload_associations, pending_enrollments, :course)
res.concat(pending_enrollments.map { |e| c = e.course; c.write_attribute(:primary_enrollment, e.type); c.write_attribute(:primary_enrollment_rank, e.rank_sortable.to_s); c.write_attribute(:primary_enrollment_state, e.workflow_state); c.write_attribute(:invitation, e.uuid); c })
res.uniq!
end
end
res.sort_by{ |c| [c.primary_enrollment_rank, Canvas::ICU.collation_key(c.name)] }
@courses_with_primary_enrollment[cache_key] =
res.sort_by{ |c| [c.primary_enrollment_rank, Canvas::ICU.collation_key(c.name)] }
end
end
memoize :courses_with_primary_enrollment
def cached_active_emails
self.shard.activate do
@ -1673,7 +1668,6 @@ class User < ActiveRecord::Base
cached_active_emails.map { |email| Enrollment.cached_temporary_invitations(email).dup.reject { |e| e.user_id == self.id } }.flatten
end
# activesupport/lib/active_support/memoizable.rb from rails and
# http://github.com/seamusabshere/cacheable/blob/master/lib/cacheable.rb from the cacheable gem
# to get a head start
@ -1690,7 +1684,6 @@ class User < ActiveRecord::Base
end
end + temporary_invitations
end
memoize :cached_current_enrollments
def cached_not_ended_enrollments
self.shard.activate do
@ -1736,45 +1729,52 @@ class User < ActiveRecord::Base
end
def submissions_for_context_codes(context_codes, opts={})
return [] if (!context_codes || context_codes.empty?)
opts[:start_at] ||= 2.weeks.ago
opts[:limit] ||= 20
return [] unless context_codes.present?
Shackles.activate(:slave) do
submissions = []
submissions += self.submissions.after(opts[:start_at]).for_context_codes(context_codes).
where("submissions.score IS NOT NULL AND assignments.workflow_state<>? AND assignments.muted=?", 'deleted', false).
order('submissions.created_at DESC').
limit(opts[:limit]).all
opts = {limit: 20}.merge(opts.slice(:start_at, :limit))
shard.activate do
Rails.cache.fetch([self, 'submissions_for_context_codes', context_codes, opts].cache_key, expires_in: 15.minutes) do
opts[:start_at] ||= 2.weeks.ago
# THIS IS SLOW, it takes ~230ms for mike
submissions += Submission.for_context_codes(context_codes).
select(["submissions.*, last_updated_at_from_db"]).
joins(self.class.send(:sanitize_sql_array, [<<-SQL, opts[:start_at], self.id, self.id])).
INNER JOIN (
SELECT MAX(submission_comments.created_at) AS last_updated_at_from_db, submission_id
FROM submission_comments, submission_comment_participants
WHERE submission_comments.id = submission_comment_id
AND (submission_comments.created_at > ?)
AND (submission_comment_participants.user_id = ?)
AND (submission_comments.author_id <> ?)
GROUP BY submission_id
) AS relevant_submission_comments ON submissions.id = submission_id
INNER JOIN assignments ON assignments.id = submissions.assignment_id AND assignments.workflow_state <> 'deleted'
SQL
where(assignments: {muted: false}).
order('last_updated_at_from_db DESC').
limit(opts[:limit]).all
Shackles.activate(:slave) do
submissions = []
submissions += self.submissions.after(opts[:start_at]).for_context_codes(context_codes).
where("submissions.score IS NOT NULL AND assignments.workflow_state<>? AND assignments.muted=?", 'deleted', false).
order('submissions.created_at DESC').
limit(opts[:limit]).all
submissions = submissions.sort_by{|t| (t.last_updated_at_from_db.to_datetime.in_time_zone rescue nil) || t.created_at}.reverse
submissions = submissions.uniq
submissions.first(opts[:limit])
# THIS IS SLOW, it takes ~230ms for mike
submissions += Submission.for_context_codes(context_codes).
select(["submissions.*, last_updated_at_from_db"]).
joins(self.class.send(:sanitize_sql_array, [<<-SQL, opts[:start_at], self.id, self.id])).
INNER JOIN (
SELECT MAX(submission_comments.created_at) AS last_updated_at_from_db, submission_id
FROM submission_comments, submission_comment_participants
WHERE submission_comments.id = submission_comment_id
AND (submission_comments.created_at > ?)
AND (submission_comment_participants.user_id = ?)
AND (submission_comments.author_id <> ?)
GROUP BY submission_id
) AS relevant_submission_comments ON submissions.id = submission_id
INNER JOIN assignments ON assignments.id = submissions.assignment_id AND assignments.workflow_state <> 'deleted'
SQL
where(assignments: {muted: false}).
order('last_updated_at_from_db DESC').
limit(opts[:limit]).all
Submission.send(:preload_associations, submissions, [:assignment, :user, :submission_comments])
submissions
submissions = submissions.sort_by{|t| (t.last_updated_at_from_db.to_datetime.in_time_zone rescue nil) || t.created_at}.reverse
submissions = submissions.uniq
submissions.first(opts[:limit])
Submission.send(:preload_associations, submissions, [:assignment, :user, :submission_comments])
submissions
end
end
end
end
memoize :submissions_for_context_codes
def uncached_submissions_for_context_codes(context_codes, opts)
end
# This is only feedback for student contexts (unless specific contexts are passed in)
def recent_feedback(opts={})
@ -1786,7 +1786,6 @@ class User < ActiveRecord::Base
end
submissions_for_context_codes(context_codes, opts)
end
memoize :recent_feedback
def visible_stream_item_instances(opts={})
instances = stream_item_instances.where(:hidden => false).order('stream_item_instances.id desc')
@ -1915,7 +1914,6 @@ class User < ActiveRecord::Base
# TODO: All the event methods use this and it's really slow.
Array(contexts || cached_contexts).map(&:asset_string)
end
memoize :setup_context_lookups
def setup_context_association_lookups(column, contexts=nil, opts = {})
contexts = Array(contexts || cached_contexts)
@ -1959,6 +1957,7 @@ class User < ActiveRecord::Base
# context codes of things that might have a schedulable appointment for the
# given user, i.e. courses and sections
def appointment_context_codes
return @appointment_context_codes if @appointment_context_codes
ret = {:primary => [], :secondary => []}
cached_current_enrollments.each do |e|
next unless e.student? && e.active?
@ -1966,11 +1965,11 @@ class User < ActiveRecord::Base
ret[:secondary] << "course_section_#{e.course_section_id}"
end
ret[:secondary].concat groups.map{ |g| "group_category_#{g.group_category_id}" }
ret
@appointment_context_codes = ret
end
memoize :appointment_context_codes
def manageable_appointment_context_codes
return @manageable_appointment_context_codes if @manageable_appointment_context_codes
ret = {:full => [], :limited => [], :secondary => []}
cached_current_enrollments.each do |e|
next unless e.course.grants_right?(self, nil, :manage_calendar)
@ -1981,9 +1980,8 @@ class User < ActiveRecord::Base
ret[:full] << "course_#{e.course_id}"
end
end
ret
@manageable_appointment_context_codes = ret
end
memoize :manageable_appointment_context_codes
# Public: Return an array of context codes this user belongs to.
#
@ -2004,7 +2002,6 @@ class User < ActiveRecord::Base
end
end
end
memoize :conversation_context_codes
def self.preload_conversation_context_codes(users)
users = users.reject { |u| u.instance_variable_get(:@conversation_context_codes) }
@ -2127,21 +2124,17 @@ class User < ActiveRecord::Base
TAB_HOME = 4
def highest_role
return 'admin' unless self.all_accounts.empty?
return 'teacher' if self.cached_current_enrollments.any?(&:admin?)
return 'student' if self.cached_current_enrollments.any?(&:student?)
return 'user'
roles.last
end
memoize :highest_role
def roles
return @roles if @roles
res = ['user']
res << 'student' if self.cached_current_enrollments.any?(&:student?)
res << 'teacher' if self.cached_current_enrollments.any?(&:admin?)
res << 'admin' unless self.all_accounts.empty?
res
@roles = res
end
memoize :roles
def eportfolios_enabled?
accounts = associated_root_accounts.reject(&:site_admin?)
@ -2561,23 +2554,21 @@ class User < ActiveRecord::Base
end
def all_accounts
self.accounts.with_each_shard
@all_accounts ||= self.accounts.with_each_shard
end
memoize :all_accounts
def all_paginatable_accounts
BookmarkedCollection.with_each_shard(Account::Bookmarker, self.accounts)
end
def all_pseudonyms
self.pseudonyms.with_each_shard
@all_pseudonyms ||= self.pseudonyms.with_each_shard
end
memoize :all_pseudonyms
def all_active_pseudonyms
self.pseudonyms.with_each_shard { |scope| scope.active }
def all_active_pseudonyms(reload=false)
@all_active_pseudonyms = nil if reload
@all_active_pseudonyms ||= self.pseudonyms.with_each_shard { |scope| scope.active }
end
memoize :all_active_pseudonyms
def prefers_gradebook2?
preferences[:use_gradebook2] != false

12
app/views/layouts/application.html.erb
View File

@ -74,26 +74,26 @@
<%-
max_to_show = 8
assignment_sets = {
:needing_submitting => t('menu.assignments_needing_submitting', "To Turn In"),
:needing_grading => t('menu.assignments_needing_grading', "To Grade"),
:recently_graded => t('menu.assignments_recently_graded', "Recently Graded")
:assignments_needing_submitting => t('menu.assignments_needing_submitting', "To Turn In"),
:assignments_needing_grading => t('menu.assignments_needing_grading', "To Grade"),
:assignments_recently_graded => t('menu.assignments_recently_graded', "Recently Graded")
}
total_shown = 0
if assignment_sets.any?{ |k, v| @current_user.send("assignments_#{k}").length > 0 }
if assignment_sets.any?{ |k, v| @current_user.send(k).length > 0 }
-%>
<a href="/assignments" class="menu-item-title"><%= t('menu.assignments', 'Assignments') %><span class="menu-item-title-icon"></span> <i class="icon-mini-arrow-down"></i></a>
<div class="menu-item-drop">
<table cellspacing="0">
<tr>
<% assignment_sets.each do |k,v|
if (set = @current_user.send("assignments_#{k}")).length > 0
if (set = @current_user.send(k)).length > 0
set = set[0,(max_to_show - total_shown)]
total_shown += set.length
%><%= render(:partial => "shared/menu_section", :locals => {
:max_to_show => max_to_show,
:more_link_for_over_max => assignments_path,
:collection => set,
:collection_size => total_shown >= max_to_show ? @current_user.send("assignments_#{k}_total_count"): set.length,
:collection_size => total_shown >= max_to_show ? @current_user.send(k, limit: nil).size : set.length,
:partial => "shared/menu_assignment",
:key => k,
:title => v }) %>

8
app/views/shared/_menu_assignment.html.erb
View File

@ -1,17 +1,17 @@
<% key ||= nil %>
<li>
<% if key == :needing_grading %>
<% if key == :assignments_needing_grading %>
<a href="<%= speed_grader_course_gradebook_path( menu_assignment.context_id, :assignment_id => menu_assignment.id) %>" data-track-category="assignment dropdown" data-track-label="needs grading">
<% else %>
<a href="<%= course_assignment_path( menu_assignment.context_id, menu_assignment.id ) %>" data-track-category="assignment dropdown" data-track-label="needs submitting">
<% end %>
<span class="name-right-wrapper">
<span class="secondary-right">
<% if key == :needing_submitting %>
<% if key == :assignments_needing_submitting %>
<%= before_label :due, "due" %> <%= datetime_string(menu_assignment.due_at, :due_date, nil, true) %>
<% elsif key == :needing_grading %>
<% elsif key == :assignments_needing_grading %>
<%= t :needs_grading_count, { :one => "1 needs grading", :other => "%{count} need grading"}, :count => menu_assignment.needs_grading_count_for_user(@current_user) %>
<% elsif key == :recently_graded %>
<% elsif key == :assignments_recently_graded %>
<% if menu_assignment.grading_type == 'points' %>
<%= "#{menu_assignment.score}/#{menu_assignment.points_possible}" %>
<% else %>

6
spec/messages/forgot_password.email.erb_spec.rb
View File

@ -25,9 +25,9 @@ describe 'forgot_password.email' do
@pseudonym = @user.pseudonyms.create!(:unique_id => 'unique@example.com', :password => 'password', :password_confirmation => 'password')
@object = @user.communication_channels.create!(:path_type => 'email', :path => 'bob@example.com', :user => @user)
@object.reload
@object.active_pseudonyms.length.should > 0
@object.active_pseudonyms.first.unique_id.should_not be_nil
@object.active_pseudonyms.first.managed_password?.should eql(false)
@user.all_active_pseudonyms.length.should > 0
@user.all_active_pseudonyms.first.unique_id.should_not be_nil
@user.all_active_pseudonyms.first.managed_password?.should eql(false)
@user.reload
generate_message(:forgot_password, :email, @object)
end

8
spec/models/account_spec.rb
View File

@ -536,22 +536,22 @@ describe Account do
u = User.create!
a.add_user(u)
a.all_users.count.should == a.user_count(:reload)
a.all_users.count.should == a.user_count
a.user_count.should == 1
course_with_teacher
@teacher.update_account_associations
a.all_users.count.should == a.user_count(:reload)
a.all_users.count.should == a.user_count
a.user_count.should == 2
a2 = a.sub_accounts.create!
course_with_teacher(:account => a2)
@teacher.update_account_associations
a.all_users.count.should == a.user_count(:reload)
a.all_users.count.should == a.user_count
a.user_count.should == 3
user_with_pseudonym
a.all_users.count.should == a.user_count(:reload)
a.all_users.count.should == a.user_count
a.user_count.should == 4
end

6
spec/models/course_spec.rb
View File

@ -2708,7 +2708,7 @@ describe Course, "conclusions" do
enrollment.save!
@course.reload
@user.reload
@user.cached_current_enrollments(:reload)
@user.cached_current_enrollments
enrollment.reload.state.should == :active
enrollment.state_based_on_date.should == :completed
@ -2722,7 +2722,7 @@ describe Course, "conclusions" do
enrollment.save!
@course.reload
@user.reload
@user.cached_current_enrollments(:reload)
@user.cached_current_enrollments
enrollment.state.should == :completed
enrollment.state_based_on_date.should == :completed
@ -2734,7 +2734,7 @@ describe Course, "conclusions" do
@course.reload
@course.complete!
@user.reload
@user.cached_current_enrollments(:reload)
@user.cached_current_enrollments
enrollment.reload
enrollment.state.should == :completed
enrollment.state_based_on_date.should == :completed

2
spec/models/discussion_topic_spec.rb
View File

@ -614,7 +614,7 @@ describe DiscussionTopic do
ct.context.add_user(@student)
ct.user_can_see_posts?(@student).should be_false
ct.reply_from(user: @student, text: 'ohai')
ct.user_ids_who_have_posted_and_admins(true) # clear the memoization
ct.user_ids_who_have_posted_and_admins
ct.user_can_see_posts?(@student).should be_true
end
end

4
spec/models/enrollment_spec.rb
View File

@ -1171,7 +1171,7 @@ describe Enrollment do
@enrollment.reject!
# have to get the new updated_at
@user.reload
@user.cached_current_enrollments(true).should == []
@user.cached_current_enrollments.should == []
end
end
@ -1482,7 +1482,7 @@ describe Enrollment do
@user.cached_current_enrollments.should == [ @enrollment ]
@enrollment.conclude
@user.reload
@user.cached_current_enrollments(true).should == []
@user.cached_current_enrollments.should == []
end
end
end

2
spec/models/user_spec.rb
View File

@ -153,7 +153,6 @@ describe User do
google_docs_collaboration_model(:user_id => @user.id)
@user.recent_stream_items.size.should == 1
StreamItem.delete_all
@user.unmemoize_all
@user.recent_stream_items.size.should == 0
end
@ -2022,7 +2021,6 @@ describe User do
# grade one submission for one assignment; these numbers don't change
@course1.assignments.first.grade_student(@studentA, :grade => "1")
@teacher = User.find(@teacher.id) # use a new instance, since these are memoized
@teacher.assignments_needing_grading.size.should eql(2)
@teacher.assignments_needing_grading.should be_include(@course1.assignments.first)
@teacher.assignments_needing_grading.should be_include(@course2.assignments.first)

2
spec/selenium/helpers/gradebook2_common.rb
View File

@ -109,7 +109,7 @@ def conclude_and_unconclude_course
#conclude course
@course.complete!
@user.reload
@user.cached_current_enrollments(:reload)
@user.cached_current_enrollments
@enrollment.reload
#un-conclude course