Draft-State Quizzes - "Save It Now" should work
Closes CNVS-13730 This patch enables the (re)publishing functionality in DS mode. The issue was that the internals were relying on "workflow_state_changed?" to be truthy in order to re-publish the quiz, which is not the usual case in DS where the quiz's WState is always "available" and doesn't really change between authoring updates. A side change: Canvas::DraftStateValidations is now covered and responsible for adding the required validations to classes that mix it in. TEST PLAN ---- ---- - create a quiz with draft-state on - add a question - save the quiz, publish it - open a tab on the quiz show page and keep it - in another tab, edit the quiz, modify the question, hit "Update Question" and switch back to the show quiz tab - refresh the page, verify you get the "Save it Now" button - push the button - refresh again - verify that the button no longer re-appears, and that your changes were actually published (you can do this by taking the quiz as a student) Change-Id: Idf3b4302b23d6abc483b80796fee753b0e6fba85 Reviewed-on: https://gerrit.instructure.com/37303 Reviewed-by: Derek DeVries <ddevries@instructure.com> Tested-by: Jenkins <jenkins@instructure.com> QA-Review: Caleb Guanzon <cguanzon@instructure.com> Product-Review: Ahmad Amireh <ahmad@instructure.com>
This commit is contained in:
parent
86020c7863
commit
29bd44354a
|
@ -75,7 +75,6 @@ class Assignment < ActiveRecord::Base
|
||||||
|
|
||||||
has_one :external_tool_tag, :class_name => 'ContentTag', :as => :context, :dependent => :destroy
|
has_one :external_tool_tag, :class_name => 'ContentTag', :as => :context, :dependent => :destroy
|
||||||
validates_associated :external_tool_tag, :if => :external_tool?
|
validates_associated :external_tool_tag, :if => :external_tool?
|
||||||
validate :validate_draft_state_change, :if => :workflow_state_changed?
|
|
||||||
validate :group_category_changes_ok?
|
validate :group_category_changes_ok?
|
||||||
validate :positive_points_possible?
|
validate :positive_points_possible?
|
||||||
|
|
||||||
|
|
|
@ -73,7 +73,6 @@ class Quizzes::Quiz < ActiveRecord::Base
|
||||||
validate :validate_quiz_type, :if => :quiz_type_changed?
|
validate :validate_quiz_type, :if => :quiz_type_changed?
|
||||||
validate :validate_ip_filter, :if => :ip_filter_changed?
|
validate :validate_ip_filter, :if => :ip_filter_changed?
|
||||||
validate :validate_hide_results, :if => :hide_results_changed?
|
validate :validate_hide_results, :if => :hide_results_changed?
|
||||||
validate :validate_draft_state_change, :if => :workflow_state_changed?
|
|
||||||
validate :validate_correct_answer_visibility, :if => lambda { |quiz|
|
validate :validate_correct_answer_visibility, :if => lambda { |quiz|
|
||||||
quiz.show_correct_answers_at_changed? ||
|
quiz.show_correct_answers_at_changed? ||
|
||||||
quiz.hide_correct_answers_at_changed?
|
quiz.hide_correct_answers_at_changed?
|
||||||
|
@ -81,7 +80,7 @@ class Quizzes::Quiz < ActiveRecord::Base
|
||||||
sanitize_field :description, CanvasSanitize::SANITIZE
|
sanitize_field :description, CanvasSanitize::SANITIZE
|
||||||
copy_authorized_links(:description) { [self.context, nil] }
|
copy_authorized_links(:description) { [self.context, nil] }
|
||||||
|
|
||||||
before_save :generate_quiz_data_on_publish
|
before_save :generate_quiz_data_on_publish, :if => :needs_republish?
|
||||||
before_save :build_assignment
|
before_save :build_assignment
|
||||||
before_save :set_defaults
|
before_save :set_defaults
|
||||||
before_save :flag_columns_that_need_republish
|
before_save :flag_columns_that_need_republish
|
||||||
|
@ -149,25 +148,26 @@ class Quizzes::Quiz < ActiveRecord::Base
|
||||||
# generate quiz data and update time when we publish. This method makes it
|
# generate quiz data and update time when we publish. This method makes it
|
||||||
# harder to mess up (like someone setting using workflow_state directly)
|
# harder to mess up (like someone setting using workflow_state directly)
|
||||||
def generate_quiz_data_on_publish
|
def generate_quiz_data_on_publish
|
||||||
# when draft state is turned on permanently, remove this conditional, the
|
|
||||||
# @publishing ivar from publish!, and change the filter to:
|
|
||||||
#
|
|
||||||
# before_save :generate_quiz_data_on_publish, :if => :workflow_state_changed?
|
|
||||||
#
|
|
||||||
if context.feature_enabled?(:draft_state)
|
|
||||||
return unless workflow_state_changed?
|
|
||||||
|
|
||||||
# pre-draft state we need ability to republish things. Since workflow_state
|
|
||||||
# is stays available, we need to flag when we're forcing to publish!
|
|
||||||
else
|
|
||||||
return unless @publishing
|
|
||||||
end
|
|
||||||
|
|
||||||
if workflow_state == 'available'
|
if workflow_state == 'available'
|
||||||
self.generate_quiz_data
|
self.generate_quiz_data
|
||||||
self.published_at = Time.zone.now
|
self.published_at = Time.zone.now
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
private :generate_quiz_data_on_publish
|
||||||
|
|
||||||
|
# @return [Boolean] Whether the quiz has unsaved changes due for a republish.
|
||||||
|
def needs_republish?
|
||||||
|
# TODO: remove this conditional and the non-DS scenario once Draft State is
|
||||||
|
# permanently turned on
|
||||||
|
if context.feature_enabled?(:draft_state)
|
||||||
|
return true if @publishing || workflow_state_changed?
|
||||||
|
|
||||||
|
# pre-draft state we need ability to republish things. Since workflow_state
|
||||||
|
# stays available, we need to flag when we're forcing to publish!
|
||||||
|
else
|
||||||
|
return true if @publishing
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
# some attributes require us to republish for non-draft state
|
# some attributes require us to republish for non-draft state
|
||||||
# We can safely remove this when draft state is permanent
|
# We can safely remove this when draft state is permanent
|
||||||
|
|
|
@ -1,5 +1,11 @@
|
||||||
module Canvas
|
module Canvas
|
||||||
module DraftStateValidations
|
module DraftStateValidations
|
||||||
|
def self.included(base)
|
||||||
|
base.class_eval do
|
||||||
|
validate :validate_draft_state_change, :if => :workflow_state_changed?
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
def validate_draft_state_change
|
def validate_draft_state_change
|
||||||
old_draft_state, new_draft_state = self.changes['workflow_state']
|
old_draft_state, new_draft_state = self.changes['workflow_state']
|
||||||
return if old_draft_state == new_draft_state
|
return if old_draft_state == new_draft_state
|
||||||
|
|
|
@ -0,0 +1,40 @@
|
||||||
|
#
|
||||||
|
# Copyright (C) 2011 Instructure, Inc.
|
||||||
|
#
|
||||||
|
# This file is part of Canvas.
|
||||||
|
#
|
||||||
|
# Canvas is free software: you can redistribute it and/or modify it under
|
||||||
|
# the terms of the GNU Affero General Public License as published by the Free
|
||||||
|
# Software Foundation, version 3 of the License.
|
||||||
|
#
|
||||||
|
# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
|
||||||
|
# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
|
||||||
|
# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
|
||||||
|
# details.
|
||||||
|
#
|
||||||
|
# You should have received a copy of the GNU Affero General Public License along
|
||||||
|
# with this program. If not, see <http://www.gnu.org/licenses/>.
|
||||||
|
#
|
||||||
|
|
||||||
|
require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb')
|
||||||
|
|
||||||
|
describe Canvas::DraftStateValidations do
|
||||||
|
shared_examples_for "Canvas::DraftStateValidations" do
|
||||||
|
describe ":validate_draft_state_change" do
|
||||||
|
it "should work" do
|
||||||
|
subject.workflow_state = 'unpublished'
|
||||||
|
subject.stubs(has_student_submissions?: true)
|
||||||
|
subject.stubs(workflow_state_changed?: true)
|
||||||
|
subject.stubs({
|
||||||
|
changes: { 'workflow_state' => [ 'published', 'unpublished' ] }
|
||||||
|
})
|
||||||
|
subject.save
|
||||||
|
|
||||||
|
subject.errors[:workflow_state].should be_present
|
||||||
|
subject.errors[:workflow_state][0].to_s.should match(
|
||||||
|
%r{can't unpublish if there are student submissions}i
|
||||||
|
)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
|
@ -17,6 +17,7 @@
|
||||||
#
|
#
|
||||||
|
|
||||||
require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb')
|
require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb')
|
||||||
|
require File.expand_path(File.dirname(__FILE__) + '/../../lib/canvas/draft_state_validations_spec.rb')
|
||||||
|
|
||||||
describe Quizzes::Quiz do
|
describe Quizzes::Quiz do
|
||||||
before :once do
|
before :once do
|
||||||
|
@ -103,6 +104,8 @@ describe Quizzes::Quiz do
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
it_should_behave_like 'Canvas::DraftStateValidations'
|
||||||
|
|
||||||
it "should flag as edited if shuffle answers changes to off" do
|
it "should flag as edited if shuffle answers changes to off" do
|
||||||
q = @course.quizzes.create!(:title => "new quiz")
|
q = @course.quizzes.create!(:title => "new quiz")
|
||||||
q.quiz_questions.create!
|
q.quiz_questions.create!
|
||||||
|
@ -1171,6 +1174,37 @@ describe Quizzes::Quiz do
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
describe '#needs_republish?' do
|
||||||
|
subject { @course.quizzes.create!(title: 'Test Quiz') }
|
||||||
|
|
||||||
|
it 'should be true if publish! was manually called' do
|
||||||
|
subject.needs_republish?.should be_false
|
||||||
|
|
||||||
|
# intercepting the call to save! and running our expectations there
|
||||||
|
# because by the time it's saved, #needs_republish? will be reset
|
||||||
|
subject.expects(:save!).with { |*args|
|
||||||
|
subject.needs_republish?.should be_true
|
||||||
|
true
|
||||||
|
}
|
||||||
|
|
||||||
|
subject.publish!
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'with draft-state' do
|
||||||
|
before do
|
||||||
|
subject.context.root_account.enable_feature!(:draft_state)
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'should be true if the workflow_state has changed' do
|
||||||
|
subject.workflow_state = 'deleted'
|
||||||
|
subject.save!
|
||||||
|
subject.reload
|
||||||
|
subject.workflow_state = 'available'
|
||||||
|
subject.needs_republish?.should be_true
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
describe "#current_regrade" do
|
describe "#current_regrade" do
|
||||||
|
|
||||||
before(:once) { @quiz = @course.quizzes.create! title: 'Test Quiz' }
|
before(:once) { @quiz = @course.quizzes.create! title: 'Test Quiz' }
|
||||||
|
|
Loading…
Reference in New Issue