diff --git a/app/models/publishing_api_action/ignore.rb b/app/models/publishing_api_action/ignore.rb deleted file mode 100644 index 01f2b24..0000000 --- a/app/models/publishing_api_action/ignore.rb +++ /dev/null @@ -1,8 +0,0 @@ -module PublishingApiAction - class Ignore < Base - # Synchonisation is a no-op for ignored documents - def synchronize(*) - Rails.logger.info("Ignoring document #{content_id} for synchronisation") - end - end -end diff --git a/app/models/publishing_api_action/unpublish.rb b/app/models/publishing_api_action/unpublish.rb deleted file mode 100644 index 39e4548..0000000 --- a/app/models/publishing_api_action/unpublish.rb +++ /dev/null @@ -1,8 +0,0 @@ -module PublishingApiAction - class Unpublish < Base - # Synchronize the document to the given service (i.e. delete it remotely). - def synchronize(service: DiscoveryEngine::Delete.new) - service.call(content_id, payload_version:) - end - end -end diff --git a/app/models/publishing_api_document.rb b/app/models/publishing_api_document.rb index e297144..917935d 100644 --- a/app/models/publishing_api_document.rb +++ b/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) diff --git a/config/document_type_ignorelist.yml b/config/document_type_ignorelist.yml index 1b38e30..64d89da 100644 --- a/config/document_type_ignorelist.yml +++ b/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/ diff --git a/config/document_type_ignorelist_path_overrides.yml b/config/document_type_ignorelist_path_overrides.yml index 62b9ebd..8a3ee94 100644 --- a/config/document_type_ignorelist_path_overrides.yml +++ b/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 diff --git a/spec/models/publishing_api_action/ignore_spec.rb b/spec/models/publishing_api_action/ignore_spec.rb deleted file mode 100644 index 16dc512..0000000 --- a/spec/models/publishing_api_action/ignore_spec.rb +++ /dev/null @@ -1,36 +0,0 @@ -RSpec.describe PublishingApiAction::Ignore do - subject(:document) { described_class.new(document_hash) } - - let(:content_id) { "123" } - let(:payload_version) { "1" } - let(:document_type) { "ignored" } - let(:document_hash) do - { - content_id:, - payload_version:, - document_type:, - } - end - - describe "#content_id" do - it "returns the content_id from the document hash" do - expect(document.content_id).to eq(content_id) - end - end - - describe "#payload_version" do - it "returns the payload_version from the document hash" do - expect(document.payload_version).to eq(1) - end - end - - describe "#synchronize" do - let(:service) { double("A service", call: nil) } - - it "does not call the service" do - document.synchronize(service:) - - expect(service).not_to have_received(:call) - end - end -end diff --git a/spec/models/publishing_api_action/unpublish_spec.rb b/spec/models/publishing_api_action/unpublish_spec.rb deleted file mode 100644 index aa79b1f..0000000 --- a/spec/models/publishing_api_action/unpublish_spec.rb +++ /dev/null @@ -1,36 +0,0 @@ -RSpec.describe PublishingApiAction::Unpublish do - subject(:document) { described_class.new(document_hash) } - - let(:content_id) { "123" } - let(:payload_version) { "1" } - let(:document_type) { "gone" } - let(:document_hash) do - { - content_id:, - payload_version:, - document_type:, - } - end - - describe "#content_id" do - it "returns the content_id from the document hash" do - expect(document.content_id).to eq(content_id) - end - end - - describe "#payload_version" do - it "returns the payload_version from the document hash" do - expect(document.payload_version).to eq(1) - end - end - - describe "#synchronize" do - let(:service) { double("Delete service", call: nil) } - - it "synchronises using a services" do - document.synchronize(service:) - - expect(service).to have_received(:call).with(content_id, payload_version: 1) - end - end -end diff --git a/spec/models/publishing_api_document_spec.rb b/spec/models/publishing_api_document_spec.rb index 576b889..1bc7434 100644 --- a/spec/models/publishing_api_document_spec.rb +++ b/spec/models/publishing_api_document_spec.rb @@ -1,72 +1,92 @@ 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 @@ -74,20 +94,26 @@ 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