From b219df063b604b4aee49add5b5744358f52f2c22 Mon Sep 17 00:00:00 2001 From: Spencer Olson Date: Tue, 28 Mar 2017 11:04:33 -0500 Subject: [PATCH] set boolean values to false before saving if nil closes CNVS-35932 closes CNVS-35931 test plan: 1. Create an announcement. 2. Go to the announcements index page at /courses/:course_id/announcements. 3. Click the settings cog and select 'Close for Comments'. 4. Verify there is no error. 5. Refresh the page and verify the announcement has been closed for comments (there should be a lock icon to the left of the settings cog). Change-Id: Ide9559622460e59dc089291345b4c1da8f0e12d2 Reviewed-on: https://gerrit.instructure.com/106609 Reviewed-by: Neil Gupta Tested-by: Jenkins Reviewed-by: Jeremy Neander QA-Review: Anju Reddy Product-Review: Neil Gupta --- app/models/assignment.rb | 7 ++ app/models/discussion_topic.rb | 6 ++ app/models/quizzes/quiz.rb | 8 ++ spec/models/assignment_spec.rb | 115 +++++++++++++++++++++++++++ spec/models/discussion_topic_spec.rb | 68 ++++++++++++++++ spec/models/quizzes/quiz_spec.rb | 95 ++++++++++++++++++++++ 6 files changed, 299 insertions(+) diff --git a/app/models/assignment.rb b/app/models/assignment.rb index 5bdb4dec732..fe9a41257a4 100644 --- a/app/models/assignment.rb +++ b/app/models/assignment.rb @@ -521,6 +521,13 @@ class Assignment < ActiveRecord::Base # have to use peer_reviews_due_at here because it's the column name self.peer_reviews_assigned = false if peer_reviews_due_at_changed? self.points_possible = nil unless self.graded? + [ + :all_day, :could_be_locked, :grade_group_students_individually, + :anonymous_peer_reviews, :turnitin_enabled, :vericite_enabled, + :moderated_grading, :omit_from_final_grade, :freeze_on_copy, + :copied, :only_visible_to_overrides, :post_to_sis, :peer_reviews_assigned, + :peer_reviews, :automatic_peer_reviews, :muted, :intra_group_peer_reviews + ].each { |attr| self[attr] = false if self[attr].nil? } end protected :default_values diff --git a/app/models/discussion_topic.rb b/app/models/discussion_topic.rb index d902ffa8ed7..56a0ee1958b 100644 --- a/app/models/discussion_topic.rb +++ b/app/models/discussion_topic.rb @@ -120,6 +120,12 @@ class DiscussionTopic < ActiveRecord::Base if self.has_group_category? self.subtopics_refreshed_at ||= Time.parse("Jan 1 2000") end + + [ + :could_be_locked, :podcast_enabled, :podcast_has_student_posts, + :require_initial_post, :pinned, :locked, :allow_rating, + :only_graders_can_rate, :sort_by_rating + ].each { |attr| self[attr] = false if self[attr].nil? } end protected :default_values diff --git a/app/models/quizzes/quiz.rb b/app/models/quizzes/quiz.rb index 009ed313b1c..60c2a25e311 100644 --- a/app/models/quizzes/quiz.rb +++ b/app/models/quizzes/quiz.rb @@ -139,6 +139,14 @@ class Quizzes::Quiz < ActiveRecord::Base self.question_count = self.question_count(true) @update_existing_submissions = true if self.for_assignment? && self.quiz_type_changed? @stored_questions = nil + + [ + :shuffle_answers, :could_be_locked, :anonymous_submissions, + :require_lockdown_browser, :require_lockdown_browser_for_results, + :one_question_at_a_time, :cant_go_back, :require_lockdown_browser_monitor, + :only_visible_to_overrides, :one_time_results, :show_correct_answers_last_attempt + ].each { |attr| self[attr] = false if self[attr].nil? } + self[:show_correct_answers] = true if self[:show_correct_answers].nil? end # quizzes differ from other publishable objects in that they require we diff --git a/spec/models/assignment_spec.rb b/spec/models/assignment_spec.rb index 7d6444c9f61..83ee893035b 100644 --- a/spec/models/assignment_spec.rb +++ b/spec/models/assignment_spec.rb @@ -96,6 +96,121 @@ describe Assignment do expect(@assignment.errors[:grading_type]).not_to be_nil end + describe "default values for boolean attributes" do + before(:once) do + @assignment = @course.assignments.create! + end + + let(:values) do + Assignment.where(id: @assignment).pluck( + :could_be_locked, + :grade_group_students_individually, + :anonymous_peer_reviews, + :turnitin_enabled, + :vericite_enabled, + :moderated_grading, + :omit_from_final_grade, + :freeze_on_copy, + :copied, + :only_visible_to_overrides, + :post_to_sis, + :peer_reviews_assigned, + :peer_reviews, + :automatic_peer_reviews, + :muted, + :intra_group_peer_reviews + ).first + end + + it "saves boolean attributes as false if they are set to nil" do + @assignment.update!( + could_be_locked: nil, + grade_group_students_individually: nil, + anonymous_peer_reviews: nil, + turnitin_enabled: nil, + vericite_enabled: nil, + moderated_grading: nil, + omit_from_final_grade: nil, + freeze_on_copy: nil, + copied: nil, + only_visible_to_overrides: nil, + post_to_sis: nil, + peer_reviews_assigned: nil, + peer_reviews: nil, + automatic_peer_reviews: nil, + muted: nil, + intra_group_peer_reviews: nil + ) + + expect(values).to eq([false] * values.length) + end + + it "saves boolean attributes as false if they are set to false" do + @assignment.update!( + could_be_locked: false, + grade_group_students_individually: false, + anonymous_peer_reviews: false, + turnitin_enabled: false, + vericite_enabled: false, + moderated_grading: false, + omit_from_final_grade: false, + freeze_on_copy: false, + copied: false, + only_visible_to_overrides: false, + post_to_sis: false, + peer_reviews_assigned: false, + peer_reviews: false, + automatic_peer_reviews: false, + muted: false, + intra_group_peer_reviews: false + ) + + expect(values).to eq([false] * values.length) + end + + it "saves boolean attributes as true if they are set to true" do + # exluding the moderated_grading attribute because it cannot be + # true when peer_reviews is true + @assignment.update!( + could_be_locked: true, + grade_group_students_individually: true, + anonymous_peer_reviews: true, + turnitin_enabled: true, + vericite_enabled: true, + omit_from_final_grade: true, + freeze_on_copy: true, + copied: true, + only_visible_to_overrides: true, + post_to_sis: true, + peer_reviews_assigned: true, + peer_reviews: true, + automatic_peer_reviews: true, + muted: true, + intra_group_peer_reviews: true + ) + + values = Assignment.where(id: @assignment).pluck( + :could_be_locked, + :grade_group_students_individually, + :anonymous_peer_reviews, + :turnitin_enabled, + :vericite_enabled, + :omit_from_final_grade, + :freeze_on_copy, + :copied, + :only_visible_to_overrides, + :post_to_sis, + :peer_reviews_assigned, + :peer_reviews, + :automatic_peer_reviews, + :muted, + :intra_group_peer_reviews + ).first + + expect(values).to eq([true] * values.length) + end + end + describe '#tool_settings_tool=' do let(:stub_response){ double(code: 200, body: {}.to_json, parsed_response: {'Id' => 'test-id'}, ok?: true) } let(:subscription_helper){ class_double(Lti::AssignmentSubscriptionsHelper).as_stubbed_const } diff --git a/spec/models/discussion_topic_spec.rb b/spec/models/discussion_topic_spec.rb index 7b599734009..42f865e0039 100644 --- a/spec/models/discussion_topic_spec.rb +++ b/spec/models/discussion_topic_spec.rb @@ -24,6 +24,74 @@ describe DiscussionTopic do student_in_course(:active_all => true) end + describe "default values for boolean attributes" do + before(:once) do + @topic = @course.discussion_topics.create! + end + + let(:values) do + DiscussionTopic.where(id: @topic).pluck( + :could_be_locked, + :podcast_enabled, + :podcast_has_student_posts, + :require_initial_post, + :pinned, + :locked, + :allow_rating, + :only_graders_can_rate, + :sort_by_rating + ).first + end + + it "saves boolean attributes as false if they are set to nil" do + @topic.update!( + could_be_locked: nil, + podcast_enabled: nil, + podcast_has_student_posts: nil, + require_initial_post: nil, + pinned: nil, + locked: nil, + allow_rating: nil, + only_graders_can_rate: nil, + sort_by_rating: nil + ) + + expect(values).to eq([false] * values.length) + end + + it "saves boolean attributes as false if they are set to false" do + @topic.update!( + could_be_locked: false, + podcast_enabled: false, + podcast_has_student_posts: false, + require_initial_post: false, + pinned: false, + locked: false, + allow_rating: false, + only_graders_can_rate: false, + sort_by_rating: false + ) + + expect(values).to eq([false] * values.length) + end + + it "saves boolean attributes as true if they are set to true" do + @topic.update!( + could_be_locked: true, + podcast_enabled: true, + podcast_has_student_posts: true, + require_initial_post: true, + pinned: true, + locked: true, + allow_rating: true, + only_graders_can_rate: true, + sort_by_rating: true + ) + + expect(values).to eq([true] * values.length) + end + end + it "should santize message" do @course.discussion_topics.create!(:message => "only this should stay") expect(@course.discussion_topics.first.message).to eql("only this should stay") diff --git a/spec/models/quizzes/quiz_spec.rb b/spec/models/quizzes/quiz_spec.rb index 74ec3c6c40f..e7da5bfc3f6 100644 --- a/spec/models/quizzes/quiz_spec.rb +++ b/spec/models/quizzes/quiz_spec.rb @@ -25,6 +25,101 @@ describe Quizzes::Quiz do course_factory end + describe "default values for boolean attributes" do + before(:once) do + @quiz = @course.quizzes.create!(title: "hello") + end + + let(:default_false_values) do + Quizzes::Quiz.where(id: @quiz).pluck( + :shuffle_answers, + :could_be_locked, + :anonymous_submissions, + :require_lockdown_browser, + :require_lockdown_browser_for_results, + :one_question_at_a_time, + :cant_go_back, + :require_lockdown_browser_monitor, + :only_visible_to_overrides, + :one_time_results, + :show_correct_answers_last_attempt + ).first + end + + it "saves boolean attributes as false if they are set to nil" do + @quiz.update!( + shuffle_answers: nil, + could_be_locked: nil, + anonymous_submissions: nil, + require_lockdown_browser: nil, + require_lockdown_browser_for_results: nil, + one_question_at_a_time: nil, + cant_go_back: nil, + require_lockdown_browser_monitor: nil, + only_visible_to_overrides: nil, + one_time_results: nil, + show_correct_answers_last_attempt: nil + ) + + expect(default_false_values).to eq([false] * default_false_values.length) + end + + it "saves show_correct_answers as true if it is set to nil" do + @quiz.update!(show_correct_answers: nil) + expect(@quiz.show_correct_answers).to eq(true) + end + + it "saves boolean attributes as false if they are set to false" do + @quiz.update!( + shuffle_answers: false, + could_be_locked: false, + anonymous_submissions: false, + require_lockdown_browser: false, + require_lockdown_browser_for_results: false, + one_question_at_a_time: false, + cant_go_back: false, + require_lockdown_browser_monitor: false, + only_visible_to_overrides: false, + one_time_results: false, + show_correct_answers_last_attempt: false + ) + + expect(default_false_values).to eq([false] * default_false_values.length) + end + + it "saves show_correct_answers as false if it is set to false" do + @quiz.update!(show_correct_answers: false) + expect(@quiz.show_correct_answers).to eq(false) + end + + it "saves boolean attributes as true if they are set to true" do + Quizzes::Quiz.stubs(:lockdown_browser_plugin_enabled?).returns(true) + @quiz.update!( + shuffle_answers: true, + could_be_locked: true, + anonymous_submissions: true, + require_lockdown_browser: true, + require_lockdown_browser_for_results: true, + one_question_at_a_time: true, + cant_go_back: true, + require_lockdown_browser_monitor: true, + only_visible_to_overrides: true, + one_time_results: true, + show_correct_answers_last_attempt: true, + # if allowed_attempts is <= 1, show_correct_answers_last_attempt + # cannot be set to true + allowed_attempts: 2 + ) + + expect(default_false_values).to eq([true] * default_false_values.length) + end + + it "saves show_correct_answers as true if it is set to true" do + @quiz.update!(show_correct_answers: true) + expect(@quiz.show_correct_answers).to eq(true) + end + end + describe ".mark_quiz_edited" do it "should mark a quiz as having unpublished changes" do quiz = nil