From 8893003b1e4d83de93e1b8cba98dbc526ed2f26c Mon Sep 17 00:00:00 2001 From: Michael Nomitch Date: Mon, 22 Sep 2014 14:19:57 -0500 Subject: [PATCH] DA - quiz messaging fixes CNVS-15716 test plan: - with DA on - make a sel_rel'd quiz - on the quiz show page, click message_students_who - students without visibility should not show up - send a message - make sure only students with visibility get it - on the show page, click "Show Student Quiz Results" in the gear menu - students without visibility should not show up - with DA off - repeat the above steps but ensure all students show up/get the message Change-Id: I202599a9a0c26430075c2056b553c03a908b0dfd Reviewed-on: https://gerrit.instructure.com/41545 Reviewed-by: Cameron Sutter Tested-by: Jenkins QA-Review: Amber Taniuchi QA-Review: Sean Lewis Product-Review: Mike Nomitch --- .../quiz_submission_users_controller.rb | 2 +- app/controllers/quizzes/quizzes_controller.rb | 7 +++- app/models/quizzes/quiz_user_finder.rb | 14 +++++-- app/models/user.rb | 7 ++++ .../v1/quizzes/quiz_submission_users_spec.rb | 20 ++++++++++ .../quizzes/quizzes_controller_spec.rb | 40 +++++++++++++++++++ spec/models/quizzes/quiz_user_finder_spec.rb | 15 +++++++ 7 files changed, 100 insertions(+), 5 deletions(-) diff --git a/app/controllers/quizzes/quiz_submission_users_controller.rb b/app/controllers/quizzes/quiz_submission_users_controller.rb index 5c0c9a6d3cc..6990fb601d6 100644 --- a/app/controllers/quizzes/quiz_submission_users_controller.rb +++ b/app/controllers/quizzes/quiz_submission_users_controller.rb @@ -108,7 +108,7 @@ module Quizzes if submitted_param? submitted? ? submitted_users : unsubmitted_users else - user_finder.all_students + user_finder.all_students_with_visibility end end diff --git a/app/controllers/quizzes/quizzes_controller.rb b/app/controllers/quizzes/quizzes_controller.rb index f99c81e9408..8e8a72466c2 100644 --- a/app/controllers/quizzes/quizzes_controller.rb +++ b/app/controllers/quizzes/quizzes_controller.rb @@ -564,7 +564,12 @@ class Quizzes::QuizzesController < ApplicationController def managed_quiz_data extend Api::V1::User if authorized_action(@quiz, @current_user, [:grade, :read_statistics]) - students = @context.students_visible_to(@current_user).order_by_sortable_name.to_a.uniq + student_scope = @context.students_visible_to(@current_user) + if @quiz.differentiated_assignments_applies? + student_scope = student_scope.able_to_see_quiz_in_course_with_da(@quiz.id, @context.id) + end + students = student_scope.order_by_sortable_name.to_a.uniq + @submissions_from_users = @quiz.quiz_submissions.for_user_ids(students.map(&:id)).not_settings_only.all @submissions_from_users = Hash[@submissions_from_users.map { |s| [s.user_id,s] }] diff --git a/app/models/quizzes/quiz_user_finder.rb b/app/models/quizzes/quiz_user_finder.rb index 9cf68483e3f..3d3b5e7ac90 100644 --- a/app/models/quizzes/quiz_user_finder.rb +++ b/app/models/quizzes/quiz_user_finder.rb @@ -20,7 +20,7 @@ module Quizzes extend Forwardable attr_reader :quiz, :user - def_delegators :@quiz, :context, :quiz_submissions + def_delegators :@quiz, :context, :quiz_submissions, :differentiated_assignments_applies? def initialize(quiz, user) @quiz = quiz @@ -28,17 +28,25 @@ module Quizzes end def submitted_students - all_students.where(id: non_preview_user_ids) + all_students_with_visibility.where(id: non_preview_user_ids) end def unsubmitted_students - all_students.where('users.id NOT IN (?)', non_preview_user_ids) + all_students_with_visibility.where('users.id NOT IN (?)', non_preview_user_ids) end def all_students context.students_visible_to(user).order_by_sortable_name.group('users.id') end + def all_students_with_visibility + if differentiated_assignments_applies? + all_students.able_to_see_quiz_in_course_with_da(@quiz.id, context.id) + else + all_students + end + end + def non_preview_quiz_submissions # This could optionally check for temporary_user_code<>NULL, but # that's not indexed and we're checking user_id anyway in the queries above. diff --git a/app/models/user.rb b/app/models/user.rb index ca84e5a06a5..1385f844e72 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -110,6 +110,7 @@ class User < ActiveRecord::Base has_many :active_assignments, :as => :context, :class_name => 'Assignment', :conditions => ['assignments.workflow_state != ?', 'deleted'] has_many :all_attachments, :as => 'context', :class_name => 'Attachment' has_many :assignment_student_visibilities + has_many :quiz_student_visibilities, :class_name => 'Quizzes::QuizStudentVisibility' has_many :folders, :as => 'context', :order => 'folders.name' has_many :active_folders, :class_name => 'Folder', :as => :context, :conditions => ['folders.workflow_state != ?', 'deleted'], :order => 'folders.name' has_many :active_folders_with_sub_folders, :class_name => 'Folder', :as => :context, :include => [:active_sub_folders], :conditions => ['folders.workflow_state != ?', 'deleted'], :order => 'folders.name' @@ -210,6 +211,12 @@ class User < ActiveRecord::Base where(:assignment_student_visibilities => { :assignment_id => assignment_id, :course_id => course_id }) } + # NOTE: only use for courses with differentiated assignments on + scope :able_to_see_quiz_in_course_with_da, lambda {|quiz_id, course_id| + joins(:quiz_student_visibilities). + where(:quiz_student_visibilities => { :quiz_id => quiz_id, :course_id => course_id }) + } + def assignment_and_quiz_visibilities(opts = {}) opts = {user_id: self.id}.merge(opts) {assignment_ids: AssignmentStudentVisibility.where(opts).order('assignment_id desc').pluck(:assignment_id), diff --git a/spec/apis/v1/quizzes/quiz_submission_users_spec.rb b/spec/apis/v1/quizzes/quiz_submission_users_spec.rb index b7691a24e58..31105bdf1d7 100644 --- a/spec/apis/v1/quizzes/quiz_submission_users_spec.rb +++ b/spec/apis/v1/quizzes/quiz_submission_users_spec.rb @@ -123,5 +123,25 @@ describe Quizzes::QuizSubmissionUsersController, type: :request do json['quiz_submissions'].first.with_indifferent_access[:id].should == @quiz_submission.id.to_s json['quiz_submissions'].length.should == 1 end + + context "differentiated_assignments" do + it "only returns submissions of students with visibility" do + @quiz.only_visible_to_overrides = true + @quiz.save! + @course.enable_feature!(:differentiated_assignments) + + json = get_submitted_users(submitted: false) + response.should be_success + user_ids = json['users'].map { |h| h['id'] } + user_ids.should_not include @student2.id.to_s + + create_section_override_for_quiz(@quiz, {course_section: @student2.current_enrollments.first.course_section}) + + json = get_submitted_users(submitted: false) + response.should be_success + user_ids = json['users'].map { |h| h['id'] } + user_ids.should include @student2.id.to_s + end + end end end diff --git a/spec/controllers/quizzes/quizzes_controller_spec.rb b/spec/controllers/quizzes/quizzes_controller_spec.rb index 87537610604..0239022455e 100644 --- a/spec/controllers/quizzes/quizzes_controller_spec.rb +++ b/spec/controllers/quizzes/quizzes_controller_spec.rb @@ -520,6 +520,46 @@ describe Quizzes::QuizzesController do assigns[:submissions_from_logged_out].should be_empty assigns[:submitted_students].should be_empty end + + context "differentiated_assignments" do + it "only returns submissions and users when there is visibility" do + user_session(@teacher) + + @user1 = user_with_pseudonym(:active_all => true, :name => 'Student1', :username => 'student1@instructure.com') + @course.enroll_student(@user1) + @course.enable_feature!(:differentiated_assignments) + + questions = [{:question_data => { :name => "test 1" }}] + + @assignment = @course.assignments.create(:title => "Test Assignment") + @assignment.workflow_state = "available" + @assignment.submission_types = "online_quiz" + @assignment.save + @quiz = Quizzes::Quiz.find_by_assignment_id(@assignment.id) + @quiz.anonymous_submissions = true + @quiz.quiz_type = "survey" + + @questions = questions.map { |q| @quiz.quiz_questions.create!(q) } + @quiz.generate_quiz_data + @quiz.only_visible_to_overrides = true + @quiz.save! + + @quiz_submission = @quiz.generate_submission(@user1) + @quiz_submission.mark_completed + + get 'managed_quiz_data', :course_id => @course.id, :quiz_id => @quiz.id + + assigns[:submissions_from_users][@quiz_submission.user_id].should == nil + assigns[:submitted_students].should == [] + + create_section_override_for_quiz(@quiz, {course_section: @user1.enrollments.first.course_section}) + + get 'managed_quiz_data', :course_id => @course.id, :quiz_id => @quiz.id + + assigns[:submissions_from_users][@quiz_submission.user_id].should == @quiz_submission + assigns[:submitted_students].should == [@user1] + end + end end describe "GET 'moderate'" do diff --git a/spec/models/quizzes/quiz_user_finder_spec.rb b/spec/models/quizzes/quiz_user_finder_spec.rb index 655282e4f05..903f21124cb 100644 --- a/spec/models/quizzes/quiz_user_finder_spec.rb +++ b/spec/models/quizzes/quiz_user_finder_spec.rb @@ -64,4 +64,19 @@ describe Quizzes::QuizUserFinder do @finder.all_students.should =~ students end + context "differentiated_assignments" do + before{@quiz.only_visible_to_overrides = true;@quiz.save!} + it "(#all_students_with_visibility) filters students if DA is on" do + @course.enable_feature!(:differentiated_assignments) + @finder.unsubmitted_students.should_not include(@unsubmitted_student) + create_section_override_for_quiz(@quiz, {course_section: @unsubmitted_student.current_enrollments.first.course_section}) + @finder.unsubmitted_students.should include(@unsubmitted_student) + end + + it "(#all_students_with_visibility) returns all_students if DA is off" do + @course.disable_feature!(:differentiated_assignments) + @finder.unsubmitted_students.should include(@unsubmitted_student) + end + end + end