wip2

app/models/concerns/edition/publishing.rb

@@ -40,7 +40,11 @@ def published_version
   def change_note_required?
     return false if new_record?
 
-    pre_publication? && previous_edition.present?
+    states_that_require_change_note_check.include?(state.to_s) && previous_edition.present?
+  end
+
+  def states_that_require_change_note_check
+    Flipflop.remove_draft_change_note_validation? ? (Edition::PRE_PUBLICATION_STATES - %w[draft]) : Edition::PRE_PUBLICATION_STATES
   end
 
   def change_note_present!

config/features.rb

@@ -26,4 +26,5 @@
   feature :govspeak_visual_editor, description: "Enables a visual editor for Govspeak fields", default: false
   feature :override_government, description: "Enables GDS Editors and Admins to override the government associated with a document", default: false
   feature :show_link_to_content_block_manager, description: "Shows link to Content Block Manager from Whitehall editor", default: Whitehall.integration_or_staging?
+  feature :remove_draft_change_note_validation, description: "Remove the change note validation for editions in draft state", default: false
 end

test/integration/attachment_replacement_integration_test.rb

@@ -97,10 +97,10 @@ class AttachmentReplacementIntegrationTest < ActionDispatch::IntegrationTest
 
       context "when new draft is created and attachment is replaced twice" do
         before do
-
           [
-           { id: replacement_asset_manager_id, filename: replacement_filename },
-           { id: double_replacement_asset_manager_id, filename: double_replacement_filename }].each do |item|
+            { id: replacement_asset_manager_id, filename: replacement_filename },
+            { id: double_replacement_asset_manager_id, filename: double_replacement_filename },
+          ].each do |item|
             Services.asset_manager.expects(:create_asset).with { |args|
               args[:file].path =~ /#{item[:filename]}/
             }.returns("id" => "http://asset-manager/assets/#{item[:id]}", "name" => item[:filename])
@@ -110,11 +110,10 @@ class AttachmentReplacementIntegrationTest < ActionDispatch::IntegrationTest
           Sidekiq::Job.clear_all
         end
 
-        it "updates replacement_id for attachment in Asset Manager" do
+        it "when remove_draft_change_note_validation is true, without pre-saving the edition - runs a set of 3 draft & replacement updates for attachment in Asset Manager" do
+          feature_flags.switch!(:remove_draft_change_note_validation, true)
           visit admin_news_article_path(edition)
           click_button "Create new edition"
-          # choose "No – it’s a minor edit that does not change the meaning"
-          # click_button "Save"
 
           AssetManagerCreateAssetWorker.drain
           PublishingApiDraftUpdateWorker.drain
@@ -139,6 +138,54 @@ class AttachmentReplacementIntegrationTest < ActionDispatch::IntegrationTest
           PublishingApiDraftUpdateWorker.drain
           AssetManagerAttachmentMetadataWorker.drain
 
+          Services.asset_manager.expects(:update_asset)
+                  .times(3)
+                  .with(replacement_asset_manager_id, { "replacement_id" => double_replacement_asset_manager_id })
+          Services.asset_manager.expects(:update_asset)
+                  .times(3)
+                  .with(double_replacement_asset_manager_id, has_entry({ "draft" => true }))
+
+          within page.find("li", text: replacement_filename) do
+            click_link "Edit attachment"
+          end
+          attach_file "Replace file", path_to_attachment(double_replacement_filename)
+          click_button "Save"
+          assert_text "Attachment 'attachment-title' updated"
+
+          AssetManagerCreateAssetWorker.drain
+          PublishingApiDraftUpdateWorker.drain
+          AssetManagerAttachmentMetadataWorker.drain
+        end
+
+        it "when remove_draft_change_note_validation is false, when pre-saving the edition - runs a set of 3 draft & replacement updates for attachment in Asset Manager" do
+          feature_flags.switch!(:remove_draft_change_note_validation, false)
+          visit admin_news_article_path(edition)
+          click_button "Create new edition"
+          choose "No – it’s a minor edit that does not change the meaning"
+          click_button "Save"
+
+          AssetManagerCreateAssetWorker.drain
+          PublishingApiDraftUpdateWorker.drain
+          AssetManagerAttachmentMetadataWorker.drain
+
+          Services.asset_manager.expects(:update_asset)
+                  .times(3)
+                  .with(asset_manager_id, { "replacement_id" => replacement_asset_manager_id })
+          Services.asset_manager.expects(:update_asset)
+                  .times(3)
+                  .with(replacement_asset_manager_id, has_entry({ "draft" => true }))
+
+          click_link "Attachments 1"
+          within page.find("li", text: filename) do
+            click_link "Edit attachment"
+          end
+          attach_file "Replace file", path_to_attachment(replacement_filename)
+          click_button "Save"
+          assert_text "Attachment 'attachment-title' updated"
+
+          AssetManagerCreateAssetWorker.drain
+          PublishingApiDraftUpdateWorker.drain
+          AssetManagerAttachmentMetadataWorker.drain
 
           Services.asset_manager.expects(:update_asset)
                   .times(3)
@@ -158,6 +205,55 @@ class AttachmentReplacementIntegrationTest < ActionDispatch::IntegrationTest
           PublishingApiDraftUpdateWorker.drain
           AssetManagerAttachmentMetadataWorker.drain
         end
+
+        it "when remove_draft_change_note_validation is false, without pre-saving the edition - runs only 1 set of updates for attachment in Asset Manager" do
+          # TODO: - to show the difference between the scenarios we'd ideally have 0 update and 3 updates respectively in the two flagged contexts
+
+          feature_flags.switch!(:remove_draft_change_note_validation, false)
+          visit admin_news_article_path(edition)
+          click_button "Create new edition"
+
+          AssetManagerCreateAssetWorker.drain
+          PublishingApiDraftUpdateWorker.drain
+          AssetManagerAttachmentMetadataWorker.drain
+
+          Services.asset_manager.expects(:update_asset)
+                  .times(1)
+                  .with(asset_manager_id, { "replacement_id" => replacement_asset_manager_id })
+          Services.asset_manager.expects(:update_asset)
+                  .times(1)
+                  .with(replacement_asset_manager_id, has_entry({ "draft" => true }))
+
+          click_link "Attachments 1"
+          within page.find("li", text: filename) do
+            click_link "Edit attachment"
+          end
+          attach_file "Replace file", path_to_attachment(replacement_filename)
+          click_button "Save"
+          assert_text "Attachment 'attachment-title' updated"
+
+          AssetManagerCreateAssetWorker.drain
+          PublishingApiDraftUpdateWorker.drain
+          AssetManagerAttachmentMetadataWorker.drain
+
+          Services.asset_manager.expects(:update_asset)
+                  .times(1)
+                  .with(replacement_asset_manager_id, { "replacement_id" => double_replacement_asset_manager_id })
+          Services.asset_manager.expects(:update_asset)
+                  .times(1)
+                  .with(double_replacement_asset_manager_id, has_entry({ "draft" => true }))
+
+          within page.find("li", text: replacement_filename) do
+            click_link "Edit attachment"
+          end
+          attach_file "Replace file", path_to_attachment(double_replacement_filename)
+          click_button "Save"
+          assert_text "Attachment 'attachment-title' updated"
+
+          AssetManagerCreateAssetWorker.drain
+          PublishingApiDraftUpdateWorker.drain
+          AssetManagerAttachmentMetadataWorker.drain
+        end
       end
     end
 

test/unit/app/models/edition/publishing_test.rb

@@ -7,7 +7,15 @@ class Edition::PublishingChangeNoteTest < ActiveSupport::TestCase
     assert edition.valid?
   end
 
-  test "a draft is invalid without change note once saved if a published edition already exists" do
+  test "when remove_draft_change_note_validation is true - a draft is valid without change note once saved if a published edition already exists" do
+    feature_flags.switch!(:remove_draft_change_note_validation, true)
+    published_edition = create(:published_edition)
+    edition = create(:draft_edition, change_note: nil, minor_change: false, document: published_edition.document)
+    assert edition.valid?
+  end
+
+  test "when remove_draft_change_note_validation is false - a draft is invalid without change note once saved if a published edition already exists" do
+    feature_flags.switch!(:remove_draft_change_note_validation, false)
     published_edition = create(:published_edition)
     edition = create(:draft_edition, change_note: nil, minor_change: false, document: published_edition.document)
     assert_not edition.valid?