From b5abfbf20f1bdb935cea27ac17a9a76be7e3828d Mon Sep 17 00:00:00 2001 From: Felix Marcus Millne Date: Tue, 8 Aug 2023 09:39:08 +0100 Subject: [PATCH 1/4] fix(workflow) update artefact before allowing edition state to change to published Seeing several issues where an edition has gotten into an incorrect state because of errors raised _after_publish_ breaking, but once the state has been moved to "published" then it is unable to rerun that code without manually changing the status. --- app/models/workflow.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/workflow.rb b/app/models/workflow.rb index 9616ba4e6..077a4b478 100644 --- a/app/models/workflow.rb +++ b/app/models/workflow.rb @@ -31,7 +31,7 @@ class CannotDeletePublishedPublication < RuntimeError; end edition.reviewer = nil end - after_transition on: :publish do |edition, _transition| + before_transition on: :publish do |edition, _transition| edition.was_published end From 2e98cc7cdd542bd3e28c97256595f6cb6f9e52b5 Mon Sep 17 00:00:00 2001 From: Felix Marcus Millne Date: Mon, 21 Aug 2023 13:07:15 +0100 Subject: [PATCH 2/4] test(workflow) assert that edition state is still ready when publish transition fails --- test/models/workflow_test.rb | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/test/models/workflow_test.rb b/test/models/workflow_test.rb index 449b387fe..8a0631c3d 100644 --- a/test/models/workflow_test.rb +++ b/test/models/workflow_test.rb @@ -1,4 +1,5 @@ require "test_helper" +require "minitest/autorun" class WorkflowTest < ActiveSupport::TestCase def setup @@ -521,4 +522,19 @@ def template_user_and_published_transaction assert_equal "schedule_for_publishing", edition.actions.last.request_type end end + + context "#publish" do + setup do + @user = FactoryBot.create(:user, :govuk_editor) + end + + should "not be in published state if the transition fails" do + edition = FactoryBot.create(:edition, state: "ready") + raise_exception = -> { raise ArgumentError.new } + edition.stub :was_published, raise_exception do + assert_raises(ArgumentError) { publish(@user, edition, "this should fail") } + end + assert_equal "ready", edition.state + end + end end From b331c9d372bf0e5d3ac9b5fa98800a65e5af02b6 Mon Sep 17 00:00:00 2001 From: Felix Marcus Millne Date: Mon, 21 Aug 2023 13:10:06 +0100 Subject: [PATCH 3/4] style(workflow_test) use generic error instead of ArgumentError --- test/models/workflow_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/models/workflow_test.rb b/test/models/workflow_test.rb index 8a0631c3d..7f905fd45 100644 --- a/test/models/workflow_test.rb +++ b/test/models/workflow_test.rb @@ -530,7 +530,7 @@ def template_user_and_published_transaction should "not be in published state if the transition fails" do edition = FactoryBot.create(:edition, state: "ready") - raise_exception = -> { raise ArgumentError.new } + raise_exception = -> { raise "something went wrong with the publish transition" } edition.stub :was_published, raise_exception do assert_raises(ArgumentError) { publish(@user, edition, "this should fail") } end From 6394162ac507208ca39ec93fa14e3a681f554f17 Mon Sep 17 00:00:00 2001 From: Felix Marcus Millne Date: Mon, 21 Aug 2023 13:42:58 +0100 Subject: [PATCH 4/4] test (workflow) correct test assertion error type --- test/models/workflow_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/models/workflow_test.rb b/test/models/workflow_test.rb index 7f905fd45..49034021b 100644 --- a/test/models/workflow_test.rb +++ b/test/models/workflow_test.rb @@ -532,7 +532,7 @@ def template_user_and_published_transaction edition = FactoryBot.create(:edition, state: "ready") raise_exception = -> { raise "something went wrong with the publish transition" } edition.stub :was_published, raise_exception do - assert_raises(ArgumentError) { publish(@user, edition, "this should fail") } + assert_raises(RuntimeError) { publish(@user, edition, "this should fail") } end assert_equal "ready", edition.state end