Fix modules ignoring page assignment override behavior

Filters pages from set of assignments rendered in modal
Filters pages from displayed content tags according to differentiated
assignments behavior

refs CYOE-263

Test Plan:
1. Create a wiki page, via pages index or modules + button
2. Ensure page can be added to module and is visible to students
3. Check edit page, "mastery paths content"
4. Ensure the page is no longer visible to students in modules, until
   they are assigned it via an override.
5. Open the "add item to module" modal. Ensure the page does not appear
   in the assignments section, only under pages.

Change-Id: I822d53dc562476ef422f4a3ee6004e0d1e132246
Reviewed-on: https://gerrit.instructure.com/87418
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>
Tested-by: Jenkins
QA-Review: Jahnavi Yetukuri <jyetukuri@instructure.com>
Product-Review: Christian Prescott <cprescott@instructure.com>

app/controllers/context_modules_api_controller.rb

@@ -380,6 +380,7 @@ class ContextModulesApiController < ApplicationController
 
         opts[:assignment_visibilities] = AssignmentStudentVisibility.visible_assignment_ids_for_user(user_ids, @context.id)
         opts[:discussion_visibilities] = DiscussionTopic.visible_to_students_in_course_with_da(user_ids, @context.id).pluck(:id)
+        opts[:page_visibilities] = WikiPage.visible_to_students_in_course_with_da(user_ids, @context.id).pluck(:id)
         opts[:quiz_visibilities] = Quizzes::Quiz.visible_to_students_in_course_with_da(user_ids,@context.id).pluck(:quiz_id)
       end
 

app/models/assignment.rb

@@ -1711,7 +1711,7 @@ class Assignment < ActiveRecord::Base
 
   scope :include_submittables, -> { preload(:quiz, :discussion_topic, :wiki_page) }
 
-  scope :no_graded_quizzes_or_topics, -> { where("submission_types NOT IN ('online_quiz', 'discussion_topic')") }
+  scope :no_submittables, -> { where.not(submission_types: %w(online_quiz discussion_topic wiki_page)) }
 
   scope :with_submissions, -> { preload(:submissions) }
 

app/models/context_module.rb

@@ -369,11 +369,13 @@ class ContextModule < ActiveRecord::Base
     filter = Proc.new{|tags, user_ids, course_id, opts|
       visible_assignments = opts[:assignment_visibilities] || assignment_visibilities_for_users(user_ids)
       visible_discussions = opts[:discussion_visibilities] || discussion_visibilities_for_users(user_ids)
+      visible_pages = opts[:page_visibilities] || page_visibilities_for_users(user_ids)
       visible_quizzes = opts[:quiz_visibilities] || quiz_visibilities_for_users(user_ids)
       tags.select{|tag|
         case tag.content_type;
         when 'Assignment'; visible_assignments.include?(tag.content_id);
         when 'DiscussionTopic'; visible_discussions.include?(tag.content_id);
+        when 'WikiPage'; visible_pages.include?(tag.content_id);
         when *Quizzes::Quiz.class_names; visible_quizzes.include?(tag.content_id);
         else; true; end
       }
@@ -694,6 +696,11 @@ class ContextModule < ActiveRecord::Base
     user_ids.flat_map{|id| discussion_visibilities_by_user[id]}
   end
 
+  def page_visibilities_for_users(user_ids)
+    page_visibilities_by_user = @page_visibilities_by_user || WikiPage.visible_ids_by_user(user_id: user_ids, course_id: [context.id])
+    user_ids.flat_map{|id| page_visibilities_by_user[id]}
+  end
+
   def quiz_visibilities_for_users(user_ids)
     quiz_visibilities_by_user = @quiz_visibilities_by_user || Quizzes::QuizStudentVisibility.visible_quiz_ids_in_course_by_user(user_id: user_ids, course_id: [context.id])
     user_ids.flat_map{|id| quiz_visibilities_by_user[id]}

app/models/discussion_topic.rb

@@ -494,30 +494,6 @@ class DiscussionTopic < ActiveRecord::Base
   alias_attribute :unlock_at, :delayed_post_at
   alias_attribute :available_until, :lock_at
 
-  def self.visible_ids_by_user(opts)
-    # pluck id, assignment_id, and user_id from discussions joined with the SQL view
-    plucked_visibilities = pluck_discussion_visibilities(opts).group_by{|r| r["user_id"]}
-    # discussions without an assignment are visible to all, so add them into every students hash at the end
-    ids_of_discussions_visible_to_all = self.without_assignment_in_course(opts[:course_id]).pluck(:id)
-    # format to be hash of user_id's with array of discussion_ids: {1 => [2,3,4], 2 => [2,4]}
-    opts[:user_id].reduce({}) do |vis_hash, student_id|
-      vis_hash[student_id] = begin
-        ids_from_pluck = (plucked_visibilities[student_id.to_s] || []).map{|r| r["id"]}
-        ids_from_pluck.concat(ids_of_discussions_visible_to_all).map(&:to_i)
-      end
-      vis_hash
-    end
-  end
-
-  def self.pluck_discussion_visibilities(opts)
-    # once on Rails 4 change this to a multi-column pluck
-    # and clean up reformatting in visible_ids_by_user
-    connection.select_all(
-      self.joins_assignment_student_visibilities(opts[:user_id],opts[:course_id]).
-        select(["discussion_topics.id", "discussion_topics.assignment_id", "assignment_student_visibilities.user_id"])
-    )
-  end
-
   def should_lock_yet
     # not assignment or vdd aware! only use this to check the topic's own field!
     # you should be checking other lock statuses in addition to this one

app/views/shared/_select_content_dialog.html.erb

@@ -231,7 +231,7 @@
           <% end %>
           <% @context.assignment_groups.active.include_active_assignments.each do |group| %>
             <optgroup label="<%= group.name %>">
-            <% group.active_assignments.no_graded_quizzes_or_topics.limit(200).each do |assignment| %>
+            <% group.active_assignments.no_submittables.limit(200).each do |assignment| %>
               <option value="<%= assignment.id %>"><%= assignment.title %></option>
             <% end %>
             </optgroup>

lib/api/v1/context_module.rb

@@ -37,7 +37,15 @@ module Api::V1::ContextModule
     end
     has_update_rights = context_module.grants_right?(current_user, :update)
     hash['published'] = context_module.active? if has_update_rights
-    tags = context_module.content_tags_visible_to(@current_user, assignment_visibilities: opts[:assignment_visibilities], discussion_visibilities: opts[:discussion_visibilities], quiz_visibilities: opts[:quiz_visibilities], observed_student_ids: opts[:observed_student_ids])
+    tags = context_module.content_tags_visible_to(@current_user,
+      opts.slice(
+        :assignment_visibilities,
+        :discussion_visibilities,
+        :page_visibilities,
+        :quiz_visibilities,
+        :observed_student_ids
+      )
+    )
     count = tags.count
     hash['items_count'] = count
     hash['items_url'] = polymorphic_url([:api_v1, context_module.context, context_module, :items])

lib/cc/assignment_resources.rb

@@ -20,7 +20,7 @@ module CC
 
     def add_assignments
       Assignments::ScopedToUser.new(@course, @user).scope.
-        no_graded_quizzes_or_topics.each do |assignment|
+        no_submittables.each do |assignment|
         next unless export_object?(assignment)
 
         title = assignment.title || I18n.t('course_exports.unknown_titles.assignment', "Unknown assignment")

lib/submittable.rb

@@ -17,6 +17,31 @@ module Submittable
       klass.joins(:assignment_student_visibilities)
         .where(assignment_student_visibilities: { user_id: user_ids, course_id: course_ids })
     }
+
+    klass.extend ClassMethods
+  end
+
+  module ClassMethods
+    def visible_ids_by_user(opts)
+      # pluck id, assignment_id, and user_id from items joined with the SQL view
+      plucked_visibilities = pluck_visibilities(opts).group_by{|_, _, user_id| user_id}
+      # items without an assignment are visible to all, so add them into every students hash at the end
+      ids_visible_to_all = self.without_assignment_in_course(opts[:course_id]).pluck(:id)
+      # build map of user_ids to array of item ids {1 => [2,3,4], 2 => [2,4]}
+      opts[:user_id].reduce({}) do |vis_hash, student_id|
+        vis_hash[student_id] = begin
+          ids_from_pluck = (plucked_visibilities[student_id] || []).map{|id, _ ,_| id}
+          ids_from_pluck.concat(ids_visible_to_all)
+        end
+        vis_hash
+      end
+    end
+
+    def pluck_visibilities(opts)
+      name = self.name.underscore.pluralize
+      self.joins_assignment_student_visibilities(opts[:user_id],opts[:course_id]).
+        pluck("#{name}.id", "#{name}.assignment_id", "assignment_student_visibilities.user_id")
+    end
   end
 
   def sync_assignment

spec/factories/wiki_page_factory.rb

@@ -24,13 +24,13 @@ def wiki_page_model(opts={})
 end
 
 def wiki_page_assignment_model(opts={})
-  page = opts.delete(:wiki_page) || wiki_page_model(opts)
-  assignment_model(
-    course: page.course,
-    wiki_page: page,
+  @page = opts.delete(:wiki_page) || wiki_page_model(opts)
+  assignment_model({
+    course: @page.course,
+    wiki_page: @page,
     submission_types: 'wiki_page',
     title: 'Content Page Assignment'
-  )
+  }.merge(opts))
 end
 
 def valid_wiki_page_attributes

spec/integration/context_module_spec.rb

@@ -275,10 +275,12 @@ describe ContextModule do
     it "should load visibilities for each model" do
       AssignmentStudentVisibility.expects(:visible_assignment_ids_in_course_by_user).returns({}).once
       DiscussionTopic.expects(:visible_ids_by_user).returns({}).once
+      WikiPage.expects(:visible_ids_by_user).returns({}).once
       Quizzes::QuizStudentVisibility.expects(:visible_quiz_ids_in_course_by_user).returns({}).once
       course_module
       @module.assignment_visibilities_for_users([2])
       @module.discussion_visibilities_for_users([2])
+      @module.page_visibilities_for_users([2])
       @module.quiz_visibilities_for_users([2])
     end
   end

spec/lib/submittable_spec.rb

@@ -0,0 +1,80 @@
+require File.expand_path(File.dirname(__FILE__) + '/../spec_helper.rb')
+
+shared_examples_for "submittable" do
+  describe "visible_ids_by_user" do
+    before :once do
+      @course = course(active_course: true)
+
+      @item_without_assignment = submittable_without_assignment
+      @item_with_assignment_and_only_vis, @assignment = submittable_and_assignment(only_visible_to_overrides: true)
+      @item_with_assignment_and_visible_to_all, @assignment2 = submittable_and_assignment(only_visible_to_overrides: false)
+      @item_with_override_for_section_with_no_students, @assignment3 = submittable_and_assignment(only_visible_to_overrides: true)
+      @item_with_no_override, @assignment4 = submittable_and_assignment(only_visible_to_overrides: true)
+
+      @course_section = @course.course_sections.create
+      @student1, @student2, @student3 = create_users(3, return_type: :record)
+      @course.enroll_student(@student2, enrollment_state: 'active')
+      @section = @course.course_sections.create!(name: "test section")
+      @section2 = @course.course_sections.create!(name: "second test section")
+      student_in_section(@section, user: @student1)
+      create_section_override_for_assignment(@assignment, {course_section: @section})
+      create_section_override_for_assignment(@assignment3, {course_section: @section2})
+      @course.reload
+      @vis_hash = submittable_class.visible_ids_by_user(course_id: @course.id, user_id: [@student1, @student2, @student3].map(&:id))
+    end
+
+    it "should return both topics for a student with an override" do
+      expect(@vis_hash[@student1.id].sort).to eq [
+        @item_without_assignment.id,
+        @item_with_assignment_and_only_vis.id,
+        @item_with_assignment_and_visible_to_all.id
+      ].sort
+    end
+
+    it "should not return differentiated topics to a student with no overrides" do
+      expect(@vis_hash[@student2.id].sort).to eq [
+        @item_without_assignment.id,
+        @item_with_assignment_and_visible_to_all.id
+      ].sort
+    end
+  end
+end
+
+describe DiscussionTopic do
+  let(:submittable_class) { DiscussionTopic }
+
+  include_examples "submittable" do
+    def submittable_without_assignment
+      discussion_topic_model(user: @teacher)
+    end
+
+    def submittable_and_assignment(opts = {})
+      assignment = @course.assignments.create!({
+        title: "some discussion assignment",
+        submission_types: 'discussion_topic'
+      }.merge(opts))
+      [assignment.discussion_topic, assignment]
+    end
+  end
+end
+
+describe WikiPage do
+  let(:submittable_class) { WikiPage }
+
+  include_examples "submittable" do
+    def submittable_without_assignment
+      wiki_page_model(course: @course)
+    end
+
+    def submittable_and_assignment(opts = {})
+      assignment = @course.assignments.create!({
+        title: "glorious page assignment",
+        submission_types: 'wiki_page'
+      }.merge(opts))
+      page = submittable_without_assignment
+      page.assignment_id = assignment.id
+      page.save!
+      [page, assignment]
+    end
+  end
+end

spec/models/context_module_spec.rb

@@ -1100,6 +1100,14 @@ describe ContextModule do
         expect(@module.content_tags_visible_to(@student_1).map(&:content).include?(@topic)).to be_truthy
         expect(@module.content_tags_visible_to(@student_2).map(&:content).include?(@topic)).to be_falsey
       end
+      it "should filter differentiated pages" do
+        @page_assignment = wiki_page_assignment_model(course: @course, only_visible_to_overrides: true)
+        create_section_override_for_assignment(@page_assignment, {course_section: @overriden_section})
+        @module.add_item({id: @page.id, type: 'wiki_page'})
+        expect(@module.content_tags_visible_to(@teacher).map(&:content).include?(@page)).to be_truthy
+        expect(@module.content_tags_visible_to(@student_1).map(&:content).include?(@page)).to be_truthy
+        expect(@module.content_tags_visible_to(@student_2).map(&:content).include?(@page)).to be_falsey
+      end
       it "should filter differentiated quizzes" do
         @quiz = Quizzes::Quiz.create!({
           context: @course,

spec/models/discussion_topic_spec.rb

@@ -312,47 +312,6 @@ describe DiscussionTopic do
     end
   end
 
-  def discussion_with_assignment(opts={})
-    assignment = @course.assignments.create!(:title => "some discussion assignment", only_visible_to_overrides: !!opts[:only_visible_to_overrides])
-    assignment.submission_types = 'discussion_topic'
-    assignment.save!
-    topic = assignment.discussion_topic
-    topic.save!
-    [topic, assignment]
-  end
-
-  context "visible_ids_by_user" do
-    before :once do
-      @course = course(:active_course => true)
-      discussion_topic_model(:user => @teacher)
-      @topic_without_assignment = @topic
-      @course_section = @course.course_sections.create
-      @student1, @student2, @student3 = create_users(3, return_type: :record)
-
-      @topic_with_assignment_and_only_vis, @assignment = discussion_with_assignment(only_visible_to_overrides: true)
-      @topic_with_assignment_and_visible_to_all, @assignment2 = discussion_with_assignment(only_visible_to_overrides: false)
-      @topic_with_override_for_section_with_no_students, @assignment3 = discussion_with_assignment(only_visible_to_overrides: true)
-      @topic_with_no_override, @assignment4 = discussion_with_assignment(only_visible_to_overrides: true)
-
-      @course.enroll_student(@student2, :enrollment_state => 'active')
-      @section = @course.course_sections.create!(name: "test section")
-      @section2 = @course.course_sections.create!(name: "second test section")
-      student_in_section(@section, user: @student1)
-      create_section_override_for_assignment(@assignment, {course_section: @section})
-      create_section_override_for_assignment(@assignment3, {course_section: @section2})
-      @course.reload
-      @vis_hash = DiscussionTopic.visible_ids_by_user(course_id: @course.id, user_id: [@student1, @student2, @student3].map(&:id))
-    end
-
-    it "should return both topics for a student with an override" do
-      expect(@vis_hash[@student1.id].sort).to eq [@topic_without_assignment.id, @topic_with_assignment_and_only_vis.id, @topic_with_assignment_and_visible_to_all.id].sort
-    end
-
-    it "should not return differentiated topics to a student with no overrides" do
-      expect(@vis_hash[@student2.id].sort).to eq [@topic_without_assignment.id, @topic_with_assignment_and_visible_to_all.id].sort
-    end
-  end
-
   describe "allow_student_discussion_topics setting" do
 
     before(:once) do