Prevent publishing/scheduling 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 and Edition Scheduler services now collect each tab's failure reasons (e.g. "Social media accounts tab is invalid") so publishers see useful messages rather than a generic one.

Consequently, the Draft Edition Updater service 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

@@ -10,6 +10,7 @@ def failure_reasons
     reasons << "This edition is invalid: #{edition.errors.full_messages.to_sentence}" unless edition.valid?(:publish)
     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?
+    reasons.concat(invalid_tab_reasons) if edition.is_a?(StandardEdition)
 
     @failure_reasons = reasons
   end

app/services/edition_scheduler.rb

@@ -24,6 +24,7 @@ def failure_reasons
     elsif scheduled_publication_is_not_within_cache_limit?
       reasons << "Scheduled publication date must be at least #{Whitehall.default_cache_max_age / 60} minutes from now"
     end
+    reasons.concat(invalid_tab_reasons) if edition.is_a?(StandardEdition)
     @failure_reasons = reasons
   end
 

app/services/edition_service.rb

@@ -67,4 +67,14 @@ def prepare_edition
   def fire_transition!
     edition.public_send("#{verb}!")
   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
 end

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))

test/unit/app/services/edition_scheduler_test.rb

@@ -61,4 +61,54 @@ class EditionSchedulerTest < ActiveSupport::TestCase
       assert_match %r{Scheduled publication date must be at least 15 minutes from now}, scheduler.failure_reason
     end
   end
+
+  test "#perform! with an invalid form tab on a standard edition cannot be scheduled" 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
+
+    scheduler = EditionScheduler.new(edition)
+
+    assert_not scheduler.perform!
+    assert_not edition.scheduled?
+    assert_includes scheduler.failure_reasons, "Extra tab tab is invalid"
+  end
 end