Merge pull request #97 from alphagov/doc-refac

Refactor invocation of delete and ignore into document

app/models/publishing_api_document.rb

@@ -8,30 +8,40 @@ class PublishingApiDocument
   # behaviour of the existing search. This may change in the future.
   PERMITTED_LOCALES = %w[en].freeze
 
-  def initialize(document_hash)
+  attr_reader :content_id, :payload_version
+
+  def initialize(
+    document_hash,
+    put_service: DiscoveryEngine::Put.new,
+    delete_service: DiscoveryEngine::Delete.new
+  )
     @document_hash = document_hash
 
-    @document_type = document_hash[:document_type]
+    @document_type = document_hash.fetch(:document_type)
+    @content_id = document_hash.fetch(:content_id)
     @base_path = document_hash[:base_path]
     @external_link = document_hash.dig(:details, :url)
     @locale = document_hash[:locale]
+    @payload_version = document_hash[:payload_version]
+
+    @put_service = put_service
+    @delete_service = delete_service
   end
 
-  def action
+  def synchronize
     if unpublish?
-      PublishingApiAction::Unpublish.new(document_hash)
+      delete_service.call(content_id, payload_version:)
     elsif ignore?
-      PublishingApiAction::Ignore.new(document_hash)
+      Rails.logger.info("Ignoring document '#{content_id}'")
     else
-      PublishingApiAction::Publish.new(document_hash)
+      PublishingApiAction::Publish.new(document_hash).synchronize(service: put_service)
     end
   end
 
-  delegate :synchronize, to: :action
-
 private
 
-  attr_reader :document_hash, :document_type, :base_path, :external_link, :locale
+  attr_reader :document_hash, :document_type, :base_path, :external_link, :locale,
+              :put_service, :delete_service
 
   def unpublish?
     UNPUBLISH_DOCUMENT_TYPES.include?(document_type)

config/document_type_ignorelist.yml

@@ -33,3 +33,7 @@ shared:
   - world_location_news
   - worldwide_office
   - worldwide_organisation
+
+test:
+  - test_ignored_type
+  - !ruby/regexp /^another_test_ignored_type/

config/document_type_ignorelist_path_overrides.yml

@@ -8,3 +8,6 @@ shared:
 - /help # special_route
 - /help/cookies # special_route
 - /find-local-council # special_route
+
+test:
+- /test_ignored_path_override

spec/models/publishing_api_document_spec.rb

@@ -1,93 +1,119 @@
 RSpec.describe PublishingApiDocument do
-  describe "#action" do
-    subject(:document) { described_class.new(document_hash).action }
-
-    let(:document_hash) do
-      {
-        document_type:,
-        base_path:,
-        details: { url: },
-        locale:,
-      }
+  subject(:document) do
+    described_class.new(
+      document_hash,
+      put_service:,
+      delete_service:,
+    )
+  end
+
+  let(:put_service) { double(:put_service, call: nil) }
+  let(:delete_service) { double(:delete_service, call: nil) }
+
+  let(:document_hash) do
+    {
+      content_id: "content-id",
+      document_type:,
+      base_path:,
+      details: { url: },
+      locale:,
+      payload_version: 42,
+    }
+  end
+  let(:base_path) { "/base-path" }
+  let(:url) { nil }
+  let(:locale) { "en" }
+
+  describe "#synchronize" do
+    before do
+      allow(Rails.logger).to receive(:info)
+
+      document.synchronize
     end
-    let(:base_path) { "/base-path" }
-    let(:url) { nil }
-    let(:locale) { "en" }
 
     %w[gone redirect substitute vanish].each do |document_type|
       context "when the document type is #{document_type}" do
         let(:document_type) { document_type }
 
-        it { is_expected.to be_a(PublishingApiAction::Unpublish) }
+        it "calls the delete service" do
+          expect(delete_service).to have_received(:call).with("content-id", payload_version: 42)
+        end
       end
     end
 
     context "when the document type is on the ignore list as a string" do
-      let(:document_type) { "ignored" }
+      let(:document_type) { "test_ignored_type" } # see test section in YAML config
 
-      before do
-        allow(Rails.configuration).to receive(:document_type_ignorelist).and_return(%w[ignored])
+      it "does not publish the document and logs a message" do
+        expect(put_service).not_to have_received(:call)
+        expect(Rails.logger).to have_received(:info).with("Ignoring document 'content-id'")
       end
-
-      it { is_expected.to be_a(PublishingApiAction::Ignore) }
     end
 
     context "when the document type is on the ignore list as a pattern" do
-      let(:document_type) { "ignored_thing" }
+      let(:document_type) { "another_test_ignored_type_foo" } # see test section in YAML config
 
-      before do
-        allow(Rails.configuration).to receive(:document_type_ignorelist).and_return([/^ignored_/])
+      it "does not publish the document and logs a message" do
+        expect(put_service).not_to have_received(:call)
+        expect(Rails.logger).to have_received(:info).with("Ignoring document 'content-id'")
       end
-
-      it { is_expected.to be_a(PublishingApiAction::Ignore) }
     end
 
-    context "when the document type is on the ignore list but the path is excluded" do
-      let(:document_type) { "ignored" }
+    context "when the document doesn't have a base path or a details.url" do
+      let(:document_type) { "internal" }
+      let(:base_path) { nil }
+      let(:url) { nil }
 
-      before do
-        allow(Rails.configuration).to receive(:document_type_ignorelist).and_return(%w[ignored])
-        allow(Rails.configuration).to receive(:document_type_ignorelist_path_overrides)
-          .and_return(%w[/base-path])
+      it "does not publish the document and logs a message" do
+        expect(put_service).not_to have_received(:call)
+        expect(Rails.logger).to have_received(:info).with("Ignoring document 'content-id'")
       end
-
-      it { is_expected.to be_a(PublishingApiAction::Publish) }
     end
 
     context "when the document doesn't have an English locale" do
       let(:document_type) { "dokument" }
       let(:locale) { "de" }
 
-      it { is_expected.to be_a(PublishingApiAction::Ignore) }
+      it "does not publish the document and logs a message" do
+        expect(put_service).not_to have_received(:call)
+        expect(Rails.logger).to have_received(:info).with("Ignoring document 'content-id'")
+      end
     end
 
-    context "when the document doesn't have a base path or a details.url" do
-      let(:document_type) { "internal" }
-      let(:base_path) { nil }
-      let(:url) { nil }
+    context "when the document type is on the ignore list but the path is excluded" do
+      let(:document_type) { "test_ignored_type" } # see test section in YAML config
+      let(:base_path) { "/test_ignored_path_override" } # see test section in YAML config
 
-      it { is_expected.to be_a(PublishingApiAction::Ignore) }
+      it "calls the put service" do
+        expect(put_service).to have_received(:call)
+      end
     end
 
     context "when the document doesn't have a base path but does have a url" do
       let(:document_type) { "external_content" }
       let(:base_path) { nil }
       let(:url) { "https://www.example.com" }
 
-      it { is_expected.to be_a(PublishingApiAction::Publish) }
+      it "calls the put service" do
+        expect(put_service).to have_received(:call)
+      end
     end
 
     context "when the document has a blank locale but otherwise should be added" do
       let(:document_type) { "stuff" }
       let(:locale) { nil }
 
-      it { is_expected.to be_a(PublishingApiAction::Publish) }
+      it "calls the put service" do
+        expect(put_service).to have_received(:call)
+      end
     end
 
     context "when the document type is anything else" do
       let(:document_type) { "anything-else" }
 
-      it { is_expected.to be_a(PublishingApiAction::Publish) }
+      it "calls the put service" do
+        expect(put_service).to have_received(:call)
+      end
     end
   end
 end