Skip to content

Commit

Permalink
Refactor invocation of delete and ignore into document
Browse files Browse the repository at this point in the history
This removes two unnecessary abstractions, with the final one still to
come.
  • Loading branch information
csutter committed Nov 3, 2023
1 parent b0febc8 commit f6452bd
Show file tree
Hide file tree
Showing 8 changed files with 93 additions and 138 deletions.
8 changes: 0 additions & 8 deletions app/models/publishing_api_action/ignore.rb

This file was deleted.

8 changes: 0 additions & 8 deletions app/models/publishing_api_action/unpublish.rb

This file was deleted.

28 changes: 19 additions & 9 deletions app/models/publishing_api_document.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 4 additions & 0 deletions config/document_type_ignorelist.yml
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,7 @@ shared:
- world_location_news
- worldwide_office
- worldwide_organisation

test:
- test_ignored_type
- !ruby/regexp /^another_test_ignored_type/
3 changes: 3 additions & 0 deletions config/document_type_ignorelist_path_overrides.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,6 @@ shared:
- /help # special_route
- /help/cookies # special_route
- /find-local-council # special_route

test:
- /test_ignored_path_override
36 changes: 0 additions & 36 deletions spec/models/publishing_api_action/ignore_spec.rb

This file was deleted.

36 changes: 0 additions & 36 deletions spec/models/publishing_api_action/unpublish_spec.rb

This file was deleted.

108 changes: 67 additions & 41 deletions spec/models/publishing_api_document_spec.rb
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit f6452bd

Please sign in to comment.