check workflow safety before executing
closes FOO-1314 flag=none TEST PLAN: 1) try to publish a content tag in with an assignment in a state like "duplicating" 2) you should get a 4xx and no sentry error Change-Id: I8654402e4b4a7da0fdce6a2e471669449821b0aa Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/254829 Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com> Reviewed-by: Michael Ziwisky <mziwisky@instructure.com> QA-Review: Ethan Vizitei <evizitei@instructure.com> Product-Review: Ethan Vizitei <evizitei@instructure.com>
This commit is contained in:
parent
1d16c2e773
commit
43f542fbbb
|
@ -491,7 +491,11 @@ class ContextModuleItemsApiController < ApplicationController
|
||||||
|
|
||||||
if params[:module_item].has_key?(:published)
|
if params[:module_item].has_key?(:published)
|
||||||
if value_to_boolean(params[:module_item][:published])
|
if value_to_boolean(params[:module_item][:published])
|
||||||
@tag.publish
|
if module_item_publishable?(@tag)
|
||||||
|
@tag.publish
|
||||||
|
else
|
||||||
|
return render json: {message: "item can't be published"}, status: :unprocessable_entity
|
||||||
|
end
|
||||||
else
|
else
|
||||||
if module_item_unpublishable?(@tag)
|
if module_item_unpublishable?(@tag)
|
||||||
@tag.unpublish
|
@tag.unpublish
|
||||||
|
|
|
@ -94,7 +94,8 @@ module ContextModulesHelper
|
||||||
end
|
end
|
||||||
|
|
||||||
def module_item_publishable?(item)
|
def module_item_publishable?(item)
|
||||||
true
|
return true if item.nil? || !item.content || !item.content.respond_to?(:can_publish?)
|
||||||
|
item.content.can_publish?
|
||||||
end
|
end
|
||||||
|
|
||||||
def prerequisite_list(prerequisites)
|
def prerequisite_list(prerequisites)
|
||||||
|
|
|
@ -3133,6 +3133,11 @@ class Assignment < ActiveRecord::Base
|
||||||
Assignments::NeedsGradingCountQuery.new(self).manual_count
|
Assignments::NeedsGradingCountQuery.new(self).manual_count
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def can_publish?
|
||||||
|
return true if new_record?
|
||||||
|
['unpublished', 'published'].include?(workflow_state)
|
||||||
|
end
|
||||||
|
|
||||||
def can_unpublish?
|
def can_unpublish?
|
||||||
return true if new_record?
|
return true if new_record?
|
||||||
return @can_unpublish unless @can_unpublish.nil?
|
return @can_unpublish unless @can_unpublish.nil?
|
||||||
|
|
|
@ -56,6 +56,39 @@ describe ContextModulesHelper do
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
describe "module_item_publishable?" do
|
||||||
|
it "should return true for an itemless item like a subheader" do
|
||||||
|
item = t_module.add_item(type: 'context_module_sub_header')
|
||||||
|
expect(module_item_publishable?(item)).to be_truthy
|
||||||
|
end
|
||||||
|
|
||||||
|
it "returns true for an item that doesn't respond to can_publish?" do
|
||||||
|
tag = t_module.content_tags.build
|
||||||
|
tag.tag_type = 'context_module'
|
||||||
|
tag.content = Thumbnail.new
|
||||||
|
expect(module_item_publishable?(tag)).to be_truthy
|
||||||
|
end
|
||||||
|
|
||||||
|
it "returns the content's can_publish?" do
|
||||||
|
assignment = t_course.assignments.create(workflow_state: 'unpublished')
|
||||||
|
student_in_course(:course => t_course)
|
||||||
|
item = t_module.add_item(type: 'assignment', id: assignment.id)
|
||||||
|
expect(assignment.can_publish?).to be_truthy
|
||||||
|
expect(module_item_publishable?(item)).to be_truthy
|
||||||
|
assignment.publish!
|
||||||
|
item.content.reload
|
||||||
|
item.reload
|
||||||
|
expect(assignment.can_publish?).to be_truthy
|
||||||
|
expect(module_item_publishable?(item)).to be_truthy
|
||||||
|
assignment.workflow_state = 'duplicating'
|
||||||
|
assignment.save!
|
||||||
|
item.content.reload
|
||||||
|
item.reload
|
||||||
|
expect(assignment.can_publish?).to be_falsey
|
||||||
|
expect(module_item_publishable?(item)).to be_falsey
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
describe "module_item_translated_content_type" do
|
describe "module_item_translated_content_type" do
|
||||||
it 'returns "" for nil item' do
|
it 'returns "" for nil item' do
|
||||||
expect(module_item_translated_content_type(nil)).to eq ''
|
expect(module_item_translated_content_type(nil)).to eq ''
|
||||||
|
|
Loading…
Reference in New Issue