diff --git a/app/controllers/context_modules_api_controller.rb b/app/controllers/context_modules_api_controller.rb index 8a5934dc77d..eb9d5e9b88b 100644 --- a/app/controllers/context_modules_api_controller.rb +++ b/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 diff --git a/app/models/assignment.rb b/app/models/assignment.rb index 4d945c2b1b9..fc50c53c4f0 100644 --- a/app/models/assignment.rb +++ b/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) } diff --git a/app/models/context_module.rb b/app/models/context_module.rb index 1a2ed5b30c8..f5faf27382c 100644 --- a/app/models/context_module.rb +++ b/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]} diff --git a/app/models/discussion_topic.rb b/app/models/discussion_topic.rb index 82d803ac1eb..e8179713d3e 100644 --- a/app/models/discussion_topic.rb +++ b/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 diff --git a/app/views/shared/_select_content_dialog.html.erb b/app/views/shared/_select_content_dialog.html.erb index 1680856a7f7..c98e340f693 100644 --- a/app/views/shared/_select_content_dialog.html.erb +++ b/app/views/shared/_select_content_dialog.html.erb @@ -231,7 +231,7 @@ <% end %> <% @context.assignment_groups.active.include_active_assignments.each do |group| %> - <% group.active_assignments.no_graded_quizzes_or_topics.limit(200).each do |assignment| %> + <% group.active_assignments.no_submittables.limit(200).each do |assignment| %> <% end %> diff --git a/lib/api/v1/context_module.rb b/lib/api/v1/context_module.rb index 42c52448cbe..79c058a432d 100644 --- a/lib/api/v1/context_module.rb +++ b/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]) diff --git a/lib/cc/assignment_resources.rb b/lib/cc/assignment_resources.rb index 40656ae6c98..721cbc9b281 100644 --- a/lib/cc/assignment_resources.rb +++ b/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") diff --git a/lib/submittable.rb b/lib/submittable.rb index f2c93184175..3431cc49d2f 100644 --- a/lib/submittable.rb +++ b/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 diff --git a/spec/factories/wiki_page_factory.rb b/spec/factories/wiki_page_factory.rb index e1f063b21a9..758bf0626c2 100644 --- a/spec/factories/wiki_page_factory.rb +++ b/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 diff --git a/spec/integration/context_module_spec.rb b/spec/integration/context_module_spec.rb index cb2d3bd40dd..8402fdce8eb 100644 --- a/spec/integration/context_module_spec.rb +++ b/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 diff --git a/spec/lib/submittable_spec.rb b/spec/lib/submittable_spec.rb new file mode 100644 index 00000000000..cb560d233e3 --- /dev/null +++ b/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 diff --git a/spec/models/context_module_spec.rb b/spec/models/context_module_spec.rb index 5f3958e7881..17759545bbb 100644 --- a/spec/models/context_module_spec.rb +++ b/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, diff --git a/spec/models/discussion_topic_spec.rb b/spec/models/discussion_topic_spec.rb index 511ee60cff4..0d1d5b02cd8 100644 --- a/spec/models/discussion_topic_spec.rb +++ b/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