Prevent publishing a standard edition with invalid tab form

We have agreed to only send a globally valid edition to Publishing API. In that sense, the Edition Publisher now collects each tab's failure reasons (e.g. "Social media accounts tab is invalid") so publishers see useful messages rather than a generic one.

Also, the Draft Edition Updater skips pushing to Publishing API if any tab is invalid.

Author: eYinka

app/services/draft_edition_updater.rb

@@ -1,7 +1,7 @@
 class DraftEditionUpdater < EditionService
   def perform!
     if can_perform?
-      update_publishing_api!
+      update_publishing_api! if can_push_draft?
       notify!
       true
     end
@@ -30,4 +30,12 @@ def should_check_current_user_will_retain_access?
   def access_limit_excludes_current_user?
     edition.organisation_association_enabled? && edition.edition_organisations.map(&:organisation_id).exclude?(@options[:current_user].organisation.id)
   end
+
+  def can_push_draft?
+    return true unless edition.is_a?(StandardEdition)
+
+    edition.type_instance.form_keys.all? do |tab_key|
+      StandardEdition::TabForm.new(edition, tab_key).valid?
+    end
+  end
 end

app/services/edition_publisher.rb

@@ -8,6 +8,7 @@ def failure_reasons
 
     reasons = []
     reasons << "This edition is invalid: #{edition.errors.full_messages.to_sentence}" unless edition.valid?(:publish)
+    reasons.concat(invalid_tab_reasons) if edition.is_a?(StandardEdition)
     reasons << "An edition that is #{edition.current_state} cannot be #{past_participle}" unless can_transition?
     reasons << "Scheduled editions cannot be published. This edition is scheduled for publication on #{edition.scheduled_publication}" if scheduled_for_publication?
 
@@ -38,6 +39,16 @@ def fire_transition!
     delete_unpublishing!
   end
 
+  def invalid_tab_reasons
+    edition.type_instance.form_keys.filter_map do |tab_key|
+      tab_form = StandardEdition::TabForm.new(edition, tab_key)
+      unless tab_form.valid?(:publish)
+        tab_label = edition.type_instance.form(tab_key)["label"] || tab_key.humanize
+        "#{tab_label} tab is invalid"
+      end
+    end
+  end
+
   def editions_to_supersede
     edition.document.editions
       .where(state: %i[published unpublished])

test/unit/app/services/draft_edition_updater_test.rb

@@ -47,4 +47,15 @@ class DraftEditionUpdaterTest < ActiveSupport::TestCase
 
     updater.perform!
   end
+
+  test "#perform! skips update_publishing_api! when a draft edition is invalid" do
+    edition = create(:draft_edition)
+    edition.freeze
+    updater = DraftEditionUpdater.new(edition)
+    updater.stubs(:can_push_draft?).returns(false)
+    updater.expects(:update_publishing_api!).never
+    updater.expects(:notify!).once
+
+    updater.perform!
+  end
 end

test/unit/app/services/edition_publisher_test.rb

@@ -51,6 +51,56 @@ class EditionPublisherTest < ActiveSupport::TestCase
     assert_equal "This edition is invalid: Title cannot be blank", publisher.failure_reason
   end
 
+  test "#perform! with an invalid form tab on a standard edition refuses to publish" do
+    configurable_document_type = build_configurable_document_type("test_type", {
+      "forms" => {
+        "documents" => {
+          "fields" => {
+            "body" => {
+              "title" => "Body",
+              "block" => "govspeak",
+              "attribute_path" => %w[block_content body],
+            },
+          },
+        },
+        "extra_tab" => {
+          "dynamic" => true,
+          "label" => "Extra tab",
+          "fields" => {
+            "sidebar" => {
+              "title" => "Sidebar",
+              "block" => "govspeak",
+              "attribute_path" => %w[block_content sidebar],
+            },
+          },
+        },
+      },
+      "schema" => {
+        "attributes" => {
+          "body" => { "type" => "string" },
+          "sidebar" => { "type" => "string" },
+        },
+        "validations" => {
+          "presence" => { "attributes" => %w[body sidebar] },
+        },
+      },
+    })
+    ConfigurableDocumentType.setup_test_types(configurable_document_type)
+
+    edition = create(:submitted_standard_edition, :with_organisations,
+                     configurable_document_type: "test_type",
+                     title: "Title", summary: "Summary",
+                     block_content: { body: "Content", sidebar: "Sidebar content" })
+    edition.translations.update_all(block_content: { body: "New content", sidebar: "" })
+    edition.reload
+
+    publisher = EditionPublisher.new(edition)
+
+    assert_not publisher.perform!
+    assert_not edition.published?
+    assert_includes publisher.failure_reasons, "Extra tab tab is invalid"
+  end
+
   test "#perform! with a re-editioned document updates the version numbers" do
     published_edition = create(:published_edition, major_change_published_at: 1.week.ago)
     edition = published_edition.create_draft(create(:writer))