Skip to content

Commit a60bd5f

Browse files
committed
add more caching + performance improvements
refs #CNVS-21317 Change-Id: Id9bbb04a92abd835af69ba5ec0c21ef758a8a324 Reviewed-on: https://gerrit.instructure.com/61146 Reviewed-by: Dan Minkevitch <dan@instructure.com> Reviewed-by: Jeremy Stanley <jeremy@instructure.com> Product-Review: James Williams <jamesw@instructure.com> QA-Review: James Williams <jamesw@instructure.com> Tested-by: Jenkins
1 parent 2a9ece9 commit a60bd5f

6 files changed

Lines changed: 56 additions & 41 deletions

File tree

app/helpers/context_modules_helper.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ def preload_can_unpublish(context, modules)
5454
def module_item_publishable_id(item)
5555
if item.nil?
5656
''
57-
elsif (item.content_type_class == 'wiki_page')
57+
elsif (item.content_type == 'WikiPage')
5858
"page_id:#{item.content.id}"
5959
else
6060
(item.content && item.content.respond_to?(:published?) ? item.content.id : item.id)

app/models/content_tag.rb

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,16 @@ def initialize( alignment )
2424
@alignment = alignment
2525
end
2626
end
27+
28+
TABLED_CONTENT_TYPES = ['Attachment', 'Assignment', 'WikiPage', 'Quizzes::Quiz', 'LearningOutcome', 'DiscussionTopic',
29+
'Rubric', 'ContextExternalTool', 'LearningOutcomeGroup', 'AssessmentQuestionBank', 'LiveAssessments::Assessment', 'Lti::MessageHandler'].freeze
30+
TABLELESS_CONTENT_TYPES = ['ContextModuleSubHeader', 'ExternalUrl'].freeze
31+
CONTENT_TYPES = (TABLED_CONTENT_TYPES + TABLELESS_CONTENT_TYPES).freeze
32+
2733
include Workflow
2834
include SearchTermHelper
2935
belongs_to :content, :polymorphic => true
30-
validates_inclusion_of :content_type, :allow_nil => true, :in => ['Attachment', 'Assignment', 'WikiPage',
31-
'ContextModuleSubHeader', 'Quizzes::Quiz', 'ExternalUrl', 'LearningOutcome', 'DiscussionTopic',
32-
'Rubric', 'ContextExternalTool', 'LearningOutcomeGroup', 'AssessmentQuestionBank', 'LiveAssessments::Assessment', 'Lti::MessageHandler']
36+
validates_inclusion_of :content_type, :allow_nil => true, :in => CONTENT_TYPES
3337
belongs_to :context, :polymorphic => true
3438
validates_inclusion_of :context_type, :allow_nil => true, :in => ['Course', 'LearningOutcomeGroup',
3539
'Assignment', 'Account', 'Quizzes::Quiz']
@@ -167,8 +171,9 @@ def scoreable?
167171

168172
def graded?
169173
return true if self.content_type == 'Assignment'
170-
return false unless self.content_type.constantize.column_names.include?('assignment_id') #.new.respond_to?(:assignment_id)
171-
return !content.assignment_id.nil? rescue false
174+
return false unless self.can_have_assignment?
175+
176+
return content && !content.assignment_id.nil?
172177
end
173178

174179
def content_type_class
@@ -209,9 +214,7 @@ def assignment
209214

210215
alias_method :old_content, :content
211216
def content
212-
#self.content_type = 'Quizzes::Quiz' if self.content_type == 'Quiz'
213-
klass = self.content_type.classify.constantize rescue nil
214-
klass.respond_to?("tableless?") && klass.tableless? ? nil : old_content
217+
TABLELESS_CONTENT_TYPES.include?(self.content_type) ? nil : old_content
215218
end
216219

217220
def content_or_self

app/models/discussion_topic.rb

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1028,27 +1028,29 @@ def available_for?(user, opts = {})
10281028
#
10291029
# Returns a boolean.
10301030
def visible_for?(user = nil)
1031-
# user is the topic's author
1032-
return true if user == self.user
1031+
RequestCache.cache('discussion_visible_for', self, user) do
1032+
# user is the topic's author
1033+
return true if user == self.user
10331034

1034-
# user is an admin in the context (teacher/ta/designer) OR
1035-
# user is an account admin with appropriate permission
1036-
return true if context.grants_any_right?(user, :manage, :read_course_content)
1035+
# user is an admin in the context (teacher/ta/designer) OR
1036+
# user is an account admin with appropriate permission
1037+
return true if context.grants_any_right?(user, :manage, :read_course_content)
10371038

1038-
# assignment exists and isnt assigned to user (differentiated assignments)
1039-
if for_assignment? && !self.assignment.visible_to_user?(user)
1040-
return false
1041-
end
1039+
# assignment exists and isnt assigned to user (differentiated assignments)
1040+
if for_assignment? && !self.assignment.visible_to_user?(user)
1041+
return false
1042+
end
10421043

1043-
# topic is not published
1044-
if !published?
1045-
false
1046-
elsif is_announcement && unlock_at = available_from_for(user)
1047-
# unlock date exists and has passed
1048-
unlock_at < Time.now.utc
1049-
# everything else
1050-
else
1051-
true
1044+
# topic is not published
1045+
if !published?
1046+
false
1047+
elsif is_announcement && unlock_at = available_from_for(user)
1048+
# unlock date exists and has passed
1049+
unlock_at < Time.now.utc
1050+
# everything else
1051+
else
1052+
true
1053+
end
10521054
end
10531055
end
10541056

app/models/enrollment.rb

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -241,14 +241,17 @@ def update_needs_grading_count
241241

242242

243243
def self.readable_types
244-
{
245-
'TeacherEnrollment' => t('#enrollment.roles.teacher', "Teacher"),
246-
'TaEnrollment' => t('#enrollment.roles.ta', "TA"),
247-
'DesignerEnrollment' => t('#enrollment.roles.designer', "Designer"),
248-
'StudentEnrollment' => t('#enrollment.roles.student', "Student"),
249-
'StudentViewEnrollment' => t('#enrollment.roles.student', "Student"),
250-
'ObserverEnrollment' => t('#enrollment.roles.observer', "Observer")
251-
}
244+
# with enough use, even translations can add up
245+
RequestCache.cache('enrollment_readable_types') do
246+
{
247+
'TeacherEnrollment' => t('#enrollment.roles.teacher', "Teacher"),
248+
'TaEnrollment' => t('#enrollment.roles.ta', "TA"),
249+
'DesignerEnrollment' => t('#enrollment.roles.designer', "Designer"),
250+
'StudentEnrollment' => t('#enrollment.roles.student', "Student"),
251+
'StudentViewEnrollment' => t('#enrollment.roles.student', "Student"),
252+
'ObserverEnrollment' => t('#enrollment.roles.observer', "Observer")
253+
}
254+
end
252255
end
253256

254257
def self.readable_type(type)

app/models/user.rb

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -236,8 +236,10 @@ def page_views(options={})
236236
}
237237

238238
def assignment_and_quiz_visibilities(context)
239-
{assignment_ids: DifferentiableAssignment.scope_filter(context.assignments, self, context).pluck(:id),
240-
quiz_ids: DifferentiableAssignment.scope_filter(context.quizzes, self, context).pluck(:id)}
239+
RequestCache.cache("assignment_and_quiz_visibilities", self, context) do
240+
{assignment_ids: DifferentiableAssignment.scope_filter(context.assignments, self, context).pluck(:id),
241+
quiz_ids: DifferentiableAssignment.scope_filter(context.quizzes, self, context).pluck(:id)}
242+
end
241243
end
242244

243245
def self.order_by_sortable_name(options = {})

lib/differentiable_assignment.rb

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,10 +58,15 @@ def self.scope_filter(scope, user, context, opts={})
5858

5959
def self.teacher_or_public_user?(user, context, opts)
6060
return true if opts[:is_teacher] == true
61-
return true if !context.includes_user?(user)
62-
permissions_implying_visibility = [:read_as_admin, :manage_grades, :manage_assignments]
63-
permissions_implying_visibility << :manage_content if context.is_a?(Course)
64-
context.grants_any_right?(user, *permissions_implying_visibility)
61+
RequestCache.cache('teacher_or_public_user', user, context) do
62+
if !context.includes_user?(user)
63+
true
64+
else
65+
permissions_implying_visibility = [:read_as_admin, :manage_grades, :manage_assignments]
66+
permissions_implying_visibility << :manage_content if context.is_a?(Course)
67+
context.grants_any_right?(user, *permissions_implying_visibility)
68+
end
69+
end
6570
end
6671

6772
def self.user_not_observer?(user, context, opts)

0 commit comments

Comments
 (0)