preload "can_unpublish" values for modules

should improve performance when viewing as a course admin

test plan:
* modules index should work as before

refs #CNVS-21476 #CNVS-21317

Change-Id: I00ee7602a48bc78538e8034d562781059f6fabb8
Reviewed-on: https://gerrit.instructure.com/57276
Tested-by: Jenkins
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>
QA-Review: Jahnavi Yetukuri <jyetukuri@instructure.com>
Product-Review: James Williams  <jamesw@instructure.com>

app/controllers/assignments_api_controller.rb

@@ -596,6 +596,10 @@ class AssignmentsApiController < ApplicationController
       needs_grading_by_section_param = params[:needs_grading_count_by_section] || false
       needs_grading_count_by_section = value_to_boolean(needs_grading_by_section_param)
 
+      if @context.grants_right?(@current_user, :manage_assignments)
+        Assignment.preload_can_unpublish(assignments)
+      end
+
       hashes = []
       TempCache.enable do
         hashes = assignments.map do |assignment|

app/controllers/quizzes/quizzes_controller.rb

@@ -83,6 +83,9 @@ class Quizzes::QuizzesController < ApplicationController
       quizzes.map(&:id), # invalidate on add/delete of quizzes
       quizzes.map(&:updated_at).sort.last # invalidate on modifications
     ].cache_key) do
+      if can_manage
+        Quizzes::Quiz.preload_can_unpublish(quizzes)
+      end
       quizzes.each_with_object({}) do |quiz, quiz_user_permissions|
         quiz_user_permissions[quiz.id] = {
           can_update: can_manage,

app/helpers/context_modules_helper.rb

@@ -37,6 +37,20 @@ module ContextModulesHelper
     cache_key
   end
 
+  def preload_can_unpublish(context, modules)
+    items = modules.map(&:content_tags).flatten.map(&:content)
+    asmnts = items.select{|item| item.is_a?(Assignment)}
+    topics = items.select{|item| item.is_a?(DiscussionTopic)}
+    quizzes = items.select{|item| item.is_a?(Quizzes::Quiz)}
+    wiki_pages = items.select{|item| item.is_a?(WikiPage)}
+
+    assmnt_ids_with_subs = Assignment.assignment_ids_with_submissions(context.assignments.pluck(:id))
+    Assignment.preload_can_unpublish(asmnts, assmnt_ids_with_subs)
+    DiscussionTopic.preload_can_unpublish(context, topics, assmnt_ids_with_subs)
+    Quizzes::Quiz.preload_can_unpublish(quizzes, assmnt_ids_with_subs)
+    WikiPage.preload_can_unpublish(context, wiki_pages)
+  end
+
   def module_item_publishable_id(item)
     if item.nil?
       ''

app/models/assignment.rb

@@ -1961,7 +1961,19 @@ class Assignment < ActiveRecord::Base
 
   def can_unpublish?
     return true if new_record?
-    !has_student_submissions?
+    return @can_unpublish unless @can_unpublish.nil?
+    @can_unpublish = !has_student_submissions?
+  end
+  attr_writer :can_unpublish
+
+  def self.preload_can_unpublish(assignments, assmnt_ids_with_subs=nil)
+    return unless assignments.any?
+    assmnt_ids_with_subs ||= assignment_ids_with_submissions(assignments.map(&:id))
+    assignments.each{|a| a.can_unpublish = !(assmnt_ids_with_subs.include?(a.id)) }
+  end
+
+  def self.assignment_ids_with_submissions(assignment_ids)
+    Submission.having_submission.where(:assignment_id => assignment_ids).uniq.pluck(:assignment_id)
   end
 
   # override so validations are called

app/models/discussion_topic.rb

@@ -635,14 +635,38 @@ class DiscussionTopic < ActiveRecord::Base
   end
 
   def can_unpublish?(opts={})
-    if self.assignment
-      !self.assignment.has_student_submissions?
-    else
-      student_ids = opts[:student_ids] || self.context.all_real_students.pluck(:id)
-      if self.for_group_discussion?
-        !(self.child_topics.any? { |child| child.discussion_entries.active.where(:user_id => student_ids).exists? })
+    return @can_unpublish unless @can_unpublish.nil?
+
+    @can_unpublish = begin
+      if self.assignment
+        !self.assignment.has_student_submissions?
       else
-        !self.discussion_entries.active.where(:user_id => student_ids).exists?
+        student_ids = opts[:student_ids] || self.context.all_real_students.pluck(:id)
+        if self.for_group_discussion?
+          !(self.child_topics.any? { |child| child.discussion_entries.active.where(:user_id => student_ids).exists? })
+        else
+          !self.discussion_entries.active.where(:user_id => student_ids).exists?
+        end
+      end
+    end
+  end
+  attr_writer :can_unpublish
+
+  def self.preload_can_unpublish(context, topics, assmnt_ids_with_subs=nil)
+    return unless topics.any?
+    assmnt_ids_with_subs ||= Assignment.assignment_ids_with_submissions(topics.map(&:assignment_id).compact)
+
+    student_ids = context.all_real_students.pluck(:id)
+    topic_ids_with_entries = DiscussionEntry.active.where(:discussion_topic_id => topics.map(&:id)).
+      where(:user_id => student_ids).uniq.pluck(:discussion_topic_id)
+    topic_ids_with_entries += DiscussionTopic.where("root_topic_id IS NOT NULL").
+      where(:id => topic_ids_with_entries).uniq.pluck(:root_topic_id)
+
+    topics.each do |topic|
+      if topic.assignment_id
+        topic.can_unpublish = !(assmnt_ids_with_subs.include?(topic.assignment_id))
+      else
+        topic.can_unpublish = !(topic_ids_with_entries.include?(topic.id))
       end
     end
   end

app/models/quizzes/quiz.rb

@@ -1139,8 +1139,22 @@ class Quizzes::Quiz < ActiveRecord::Base
 
   def can_unpublish?
     return true if new_record?
-    !has_student_submissions? &&
-      (assignment.blank? || assignment.can_unpublish?)
+    return @can_unpublish unless @can_unpublish.nil?
+    @can_unpublish = !has_student_submissions? && (assignment.blank? || assignment.can_unpublish?)
+  end
+  attr_writer :can_unpublish
+
+  def self.preload_can_unpublish(quizzes, assmnt_ids_with_subs=nil)
+    return unless quizzes.any?
+    assmnt_ids_with_subs ||= Assignment.assignment_ids_with_submissions(quizzes.map(&:assignment_id).compact)
+
+    quiz_ids_with_subs = Quizzes::QuizSubmission.where(:quiz_id => quizzes.map(&:id)).
+      not_settings_only.where("user_id IS NOT NULL").uniq.pluck(:quiz_id)
+
+    quizzes.each do |quiz|
+      quiz.can_unpublish = !(quiz_ids_with_subs.include?(quiz.id)) &&
+        (quiz.assignment_id.nil? || !assmnt_ids_with_subs.include?(quiz.assignment_id))
+    end
   end
 
   alias_method :unpublishable?, :can_unpublish?

app/models/wiki_page.rb

@@ -89,8 +89,8 @@ class WikiPage < ActiveRecord::Base
 
   def self.title_order_by_clause
     best_unicode_collation_key('wiki_pages.title')
-  end  
-  
+  end
+
   def ensure_unique_url
     url_attribute = self.class.url_attribute
     base_url = self.send(url_attribute)
@@ -357,7 +357,7 @@ class WikiPage < ActiveRecord::Base
       entry.updated   = self.updated_at
       entry.published = self.created_at
       entry.id        = "tag:#{HostUrl.default_host},#{self.created_at.strftime("%Y-%m-%d")}:/wiki_pages/#{self.feed_code}_#{self.updated_at.strftime("%Y-%m-%d")}"
-      entry.links    << Atom::Link.new(:rel => 'alternate', 
+      entry.links    << Atom::Link.new(:rel => 'alternate',
                                     :href => "http://#{HostUrl.context_host(context)}/#{self.context.class.to_s.downcase.pluralize}/#{self.context.id}/pages/#{self.url}")
       entry.content   = Atom::Content::Html.new(self.body || t('defaults.no_content', "no content"))
     end
@@ -388,7 +388,15 @@ class WikiPage < ActiveRecord::Base
   end
 
   def can_unpublish?
-    !is_front_page?
+    return @can_unpublish unless @can_unpublish.nil?
+    @can_unpublish = !is_front_page?
+  end
+  attr_writer :can_unpublish
+
+  def self.preload_can_unpublish(context, wiki_pages)
+    return unless wiki_pages.any?
+    front_page_url = context.wiki.get_front_page_url
+    wiki_pages.each{|wp| wp.can_unpublish = !(wp.url == front_page_url)}
   end
 
   def initialize_wiki_page(user)

app/views/context_modules/_content_next.html.erb

@@ -70,6 +70,7 @@ TEXT
     <% cache_key = add_menu_tools_to_cache_key(cache_key) %>
     <% cache(cache_key) do %>
       <% ActiveRecord::Associations::Preloader.new(@modules, :content_tags => :content).run %>
+      <% preload_can_unpublish(@context, @modules) %>
       <% @modules.each do |m| %>
         <%= render :partial => 'context_modules/context_module_next', :object => m, :locals => {:editable => editable } %>
       <% end %>
@@ -251,4 +252,4 @@ TEXT
   </div>
 </div>
 
-<%= render :partial => "context_modules/keyboard_navigation" %>
+<%= render :partial => "context_modules/keyboard_navigation" %>

lib/api/v1/assignment.rb

@@ -212,7 +212,9 @@ module Api::V1::Assignment
     end
 
     hash['published'] = ! assignment.unpublished?
-    hash['unpublishable'] = assignment.can_unpublish?
+    if assignment.grants_right?(user, :update)
+      hash['unpublishable'] = assignment.can_unpublish?
+    end
 
     if opts[:differentiated_assignments_enabled] || (opts[:differentiated_assignments_enabled] != false && assignment.context.feature_enabled?(:differentiated_assignments))
       hash['only_visible_to_overrides'] = value_to_boolean(assignment.only_visible_to_overrides)

spec/helpers/context_modules_helper_spec.rb

@@ -48,6 +48,7 @@ describe ContextModulesHelper do
       student_in_course(:course => t_course)
       item = t_module.add_item(type: 'discussion_topic', id: topic.id)
       expect(module_item_unpublishable?(item)).to be_truthy
+      item.reload
       topic.discussion_entries.create!(:user => @student)
       expect(module_item_unpublishable?(item)).to be_falsey
     end