diff --git a/app/adapters/organisations_adapter.rb b/app/adapters/organisations_adapter.rb index eeecf9eac..c1d3b3575 100644 --- a/app/adapters/organisations_adapter.rb +++ b/app/adapters/organisations_adapter.rb @@ -1,9 +1,6 @@ class OrganisationsAdapter - def initialize - @cache = {} - end - - def find(slug) + def self.find(slug) + @cache ||= {} @cache.fetch(slug) do response = Services.organisations.organisation(slug) organisation = Organisation.new( diff --git a/app/adapters/publishing_adapter.rb b/app/adapters/publishing_adapter.rb index 8b3fe3141..5996304ee 100644 --- a/app/adapters/publishing_adapter.rb +++ b/app/adapters/publishing_adapter.rb @@ -1,7 +1,7 @@ require "securerandom" class PublishingAdapter - def save_draft(manual, republish: false, include_sections: true, include_links: true) + def self.save_draft(manual, republish: false, include_sections: true, include_links: true) save_manual(manual, republish:, include_links:) if include_sections @@ -11,7 +11,7 @@ def save_draft(manual, republish: false, include_sections: true, include_links: end end - def unpublish_and_redirect_manual_and_sections(manual, redirect:, discard_drafts:) + def self.unpublish_and_redirect_manual_and_sections(manual, redirect:, discard_drafts:) Services.publishing_api.unpublish( manual.id, type: "redirect", @@ -27,7 +27,7 @@ def unpublish_and_redirect_manual_and_sections(manual, redirect:, discard_drafts end end - def unpublish_section(section, redirect:, republish: false, discard_drafts: true) + def self.unpublish_section(section, redirect:, republish: false, discard_drafts: true) if !section.withdrawn? || republish begin Services.publishing_api.unpublish( @@ -40,7 +40,7 @@ def unpublish_section(section, redirect:, republish: false, discard_drafts: true end end - def unpublish(manual) + def self.unpublish(manual) Services.publishing_api.unpublish(manual.id, type: "gone") manual.sections.each do |section| @@ -48,7 +48,7 @@ def unpublish(manual) end end - def publish(manual, republish: false) + def self.publish(manual, republish: false) publish_manual(manual, republish:) manual.sections.each do |section| @@ -60,21 +60,21 @@ def publish(manual, republish: false) end end - def discard(manual) + def self.discard(manual) manual.sections.each do |section| discard_section(section) end Services.publishing_api.discard_draft(manual.id) end - def save_section(section, manual, republish: false, include_links: true) + def self.save_section(section, manual, republish: false, include_links: true) if section.needs_exporting? || republish save_section_links(section, manual) if include_links save_section_content(section, manual, republish:) end end - def redirect_section(section, to:) + def self.redirect_section(section, to:) Services.publishing_api.put_content( SecureRandom.uuid, document_type: "redirect", @@ -91,23 +91,17 @@ def redirect_section(section, to:) ) end - def discard_section(section) + def self.discard_section(section) Services.publishing_api.discard_draft(section.uuid) end -private - - def organisation_for(manual) - Adapters.organisations.find(manual.organisation_slug) - end - - def save_manual(manual, republish:, include_links:) + def self.save_manual(manual, republish:, include_links:) save_manual_links(manual) if include_links save_manual_content(manual, republish:) end - def save_manual_links(manual) - organisation = organisation_for(manual) + def self.save_manual_links(manual) + organisation = OrganisationsAdapter.find(manual.organisation_slug) Services.publishing_api.patch_links( manual.id, @@ -119,8 +113,8 @@ def save_manual_links(manual) ) end - def save_manual_content(manual, republish: false) - organisation = organisation_for(manual) + def self.save_manual_content(manual, republish: false) + organisation = OrganisationsAdapter.find(manual.organisation_slug) update_type = case version_type(republish) || manual.version_type when :new, :major @@ -213,12 +207,12 @@ def save_manual_content(manual, republish: false) Services.publishing_api.put_content(manual.id, attributes) end - def publish_manual(manual, republish:) + def self.publish_manual(manual, republish:) Services.publishing_api.publish(manual.id, update_type(republish)) end - def save_section_links(section, manual) - organisation = organisation_for(manual) + def self.save_section_links(section, manual) + organisation = OrganisationsAdapter.find(manual.organisation_slug) Services.publishing_api.patch_links( section.uuid, @@ -230,8 +224,8 @@ def save_section_links(section, manual) ) end - def save_section_content(section, manual, republish: false) - organisation = organisation_for(manual) + def self.save_section_content(section, manual, republish: false) + organisation = OrganisationsAdapter.find(manual.organisation_slug) update_type = case version_type(republish) || section.version_type when :new, :major @@ -306,18 +300,18 @@ def save_section_content(section, manual, republish: false) Services.publishing_api.put_content(section.uuid, attributes) end - def publish_section(section, republish:) + def self.publish_section(section, republish:) if section.needs_exporting? || republish Services.publishing_api.publish(section.uuid, update_type(republish)) section.mark_as_exported! unless republish end end - def update_type(republish) + def self.update_type(republish) republish ? GdsApiConstants::PublishingApi::REPUBLISH_UPDATE_TYPE : nil end - def version_type(republish) + def self.version_type(republish) republish ? :republish : nil end end diff --git a/app/lib/adapters.rb b/app/lib/adapters.rb deleted file mode 100644 index 123a5a1e8..000000000 --- a/app/lib/adapters.rb +++ /dev/null @@ -1,9 +0,0 @@ -module Adapters - def self.organisations - @organisations ||= OrganisationsAdapter.new - end - - def self.publishing - @publishing ||= PublishingAdapter.new - end -end diff --git a/app/lib/manual_relocator.rb b/app/lib/manual_relocator.rb index a88d5b8be..058c94245 100644 --- a/app/lib/manual_relocator.rb +++ b/app/lib/manual_relocator.rb @@ -183,7 +183,7 @@ def redraft_and_republish def send_draft(manual) logger.info "Sending a draft of manual #{manual.id} (version: #{manual.version_number}) and its sections" - Adapters.publishing.save_draft(manual, include_links: false, republish: true) + PublishingAdapter.save_draft(manual, include_links: false, republish: true) end def send_gone(section_uuid, slug) diff --git a/app/lib/manual_withdrawer.rb b/app/lib/manual_withdrawer.rb index 24b718bf9..928e3d4e6 100644 --- a/app/lib/manual_withdrawer.rb +++ b/app/lib/manual_withdrawer.rb @@ -19,7 +19,7 @@ def execute(manual_id) logger.error "FAILURE: #{message}" raise message end - rescue Manual::WithdrawService::ManualNotFoundError + rescue Manual::NotFoundError message = "Manual not found for manual_id `#{manual_id}`" warn "ERROR: #{message}" raise message diff --git a/app/lib/section_reslugger.rb b/app/lib/section_reslugger.rb index 1f3813822..550575b50 100644 --- a/app/lib/section_reslugger.rb +++ b/app/lib/section_reslugger.rb @@ -59,7 +59,7 @@ def validate_new_section_in_content_store end def redirect_section(section) - Adapters.publishing.redirect_section(section, to: "/#{new_section_slug}") + PublishingAdapter.redirect_section(section, to: "/#{new_section_slug}") end def update_slug diff --git a/app/lib/withdraw_and_redirect_manual.rb b/app/lib/withdraw_and_redirect_manual.rb index 6998e54ad..080d52f4a 100644 --- a/app/lib/withdraw_and_redirect_manual.rb +++ b/app/lib/withdraw_and_redirect_manual.rb @@ -14,7 +14,7 @@ def execute raise ManualNotPublishedError, manual.slug unless manual.can_withdraw? - Adapters.publishing.unpublish_and_redirect_manual_and_sections( + PublishingAdapter.unpublish_and_redirect_manual_and_sections( manual, redirect:, discard_drafts:, diff --git a/app/lib/withdraw_and_redirect_section.rb b/app/lib/withdraw_and_redirect_section.rb index 97eb59866..fab8a09e6 100644 --- a/app/lib/withdraw_and_redirect_section.rb +++ b/app/lib/withdraw_and_redirect_section.rb @@ -19,9 +19,9 @@ def execute raise SectionNotPublishedError, section.slug unless section.published? if discard_draft && section.draft? - Adapters.publishing.unpublish_section(section, redirect:, discard_drafts: true) + PublishingAdapter.unpublish_section(section, redirect:, discard_drafts: true) else - Adapters.publishing.unpublish_section(section, redirect:, discard_drafts: false) + PublishingAdapter.unpublish_section(section, redirect:, discard_drafts: false) end end diff --git a/app/services/manual/create_service.rb b/app/services/manual/create_service.rb index 31749a1de..29fb584ca 100644 --- a/app/services/manual/create_service.rb +++ b/app/services/manual/create_service.rb @@ -8,7 +8,7 @@ def call manual = Manual.new(attributes) if manual.valid? - Adapters.publishing.save_draft(manual) + PublishingAdapter.save_draft(manual) manual.save!(user) end diff --git a/app/services/manual/discard_draft_service.rb b/app/services/manual/discard_draft_service.rb index de100376d..f3a97d50b 100644 --- a/app/services/manual/discard_draft_service.rb +++ b/app/services/manual/discard_draft_service.rb @@ -11,7 +11,7 @@ def call Result.failure(manual) else begin - Adapters.publishing.discard(manual) + PublishingAdapter.discard(manual) rescue GdsApi::HTTPNotFound # this is fine, the manual has already been discarded from the # publishing API and the next line will clean it up in our DB diff --git a/app/services/manual/publish_service.rb b/app/services/manual/publish_service.rb index 7d6b89143..58e8131db 100644 --- a/app/services/manual/publish_service.rb +++ b/app/services/manual/publish_service.rb @@ -11,8 +11,8 @@ def call if version_number == manual.version_number manual.publish PublicationLogger.new.call(manual) - Adapters.publishing.save_draft(manual) - Adapters.publishing.publish(manual) + PublishingAdapter.save_draft(manual) + PublishingAdapter.publish(manual) manual.save!(user) else raise VersionMismatchError, diff --git a/app/services/manual/republish_service.rb b/app/services/manual/republish_service.rb index 5f677f06a..3ea68429b 100644 --- a/app/services/manual/republish_service.rb +++ b/app/services/manual/republish_service.rb @@ -6,12 +6,12 @@ def self.call(user:, manual_id:) draft_manual_version = manual_versions[:draft] if published_manual_version.present? - Adapters.publishing.save_draft(published_manual_version, republish: true) - Adapters.publishing.publish(published_manual_version, republish: true) + PublishingAdapter.save_draft(published_manual_version, republish: true) + PublishingAdapter.publish(published_manual_version, republish: true) end if draft_manual_version.present? - Adapters.publishing.save_draft(draft_manual_version, republish: true) + PublishingAdapter.save_draft(draft_manual_version, republish: true) end manual_versions diff --git a/app/services/manual/update_original_publication_date_service.rb b/app/services/manual/update_original_publication_date_service.rb index 2668775af..a87eea665 100644 --- a/app/services/manual/update_original_publication_date_service.rb +++ b/app/services/manual/update_original_publication_date_service.rb @@ -17,7 +17,7 @@ def call manual.save!(user) manual = Manual.find(manual_id, user) - Adapters.publishing.save_draft(manual) + PublishingAdapter.save_draft(manual) manual end diff --git a/app/services/manual/update_service.rb b/app/services/manual/update_service.rb index 521604038..d05270ed8 100644 --- a/app/services/manual/update_service.rb +++ b/app/services/manual/update_service.rb @@ -14,7 +14,7 @@ def call if manual.valid? manual.save!(user) reloaded_manual = Manual.find(manual.id, user) - Adapters.publishing.save_draft(reloaded_manual) + PublishingAdapter.save_draft(reloaded_manual) end manual diff --git a/app/services/manual/withdraw_service.rb b/app/services/manual/withdraw_service.rb index 1b3cd71e8..2f6e8b910 100644 --- a/app/services/manual/withdraw_service.rb +++ b/app/services/manual/withdraw_service.rb @@ -5,17 +5,12 @@ def initialize(user:, manual_id:) end def call - begin - manual = Manual.find(manual_id, user) - rescue KeyError => e - raise ManualNotFoundError, e - end - + manual = Manual.find(manual_id, user) manual.withdraw if manual.withdrawn? manual.save!(user) - Adapters.publishing.unpublish(manual) + PublishingAdapter.unpublish(manual) end manual @@ -24,6 +19,4 @@ def call private attr_reader :user, :manual_id - - class ManualNotFoundError < StandardError; end end diff --git a/app/services/section/create_service.rb b/app/services/section/create_service.rb index 0e5d8b4f0..128a3932b 100644 --- a/app/services/section/create_service.rb +++ b/app/services/section/create_service.rb @@ -11,8 +11,8 @@ def call if new_section.valid? manual.draft - Adapters.publishing.save_draft(manual, include_sections: false) - Adapters.publishing.save_section(new_section, manual) + PublishingAdapter.save_draft(manual, include_sections: false) + PublishingAdapter.save_section(new_section, manual) manual.save!(user) end diff --git a/app/services/section/remove_service.rb b/app/services/section/remove_service.rb index 1fae5bace..1b5ffd522 100644 --- a/app/services/section/remove_service.rb +++ b/app/services/section/remove_service.rb @@ -7,11 +7,7 @@ def initialize(user:, manual_id:, section_uuid:, attributes:) end def call - begin - manual = Manual.find(manual_id, user) - rescue KeyError - raise ManualNotFoundError, manual_id - end + manual = Manual.find(manual_id, user) section = manual.find_section(section_uuid) raise SectionNotFoundError, section_uuid if section.blank? @@ -24,7 +20,7 @@ def call # We need to capture the state of the section before assigning attributes. # The Section#assign_attributes method always creates a new draft section if # the latest edition is published. - # This causes Adapters.publishing.discard_section(section) to be called which + # This causes PublishingAdapter.discard_section(section) to be called which # blows up as there is no draft section in the Publishing API database. draft_section = section.draft? @@ -36,10 +32,10 @@ def call manual.remove_section(section_uuid) manual.save!(user) - Adapters.publishing.save_draft(manual, include_sections: false) + PublishingAdapter.save_draft(manual, include_sections: false) if draft_section - Adapters.publishing.discard_section(section) + PublishingAdapter.discard_section(section) end end @@ -50,7 +46,5 @@ def call attr_reader :user, :manual_id, :section_uuid, :attributes - class ManualNotFoundError < StandardError; end - class SectionNotFoundError < StandardError; end end diff --git a/app/services/section/reorder_service.rb b/app/services/section/reorder_service.rb index da8b351eb..9036add87 100644 --- a/app/services/section/reorder_service.rb +++ b/app/services/section/reorder_service.rb @@ -10,7 +10,7 @@ def call manual.draft manual.reorder_sections(section_order) manual.save!(user) - Adapters.publishing.save_draft(manual, include_sections: false) + PublishingAdapter.save_draft(manual, include_sections: false) [manual, manual.sections] end diff --git a/app/services/section/update_service.rb b/app/services/section/update_service.rb index 710d344d7..8dd7209dc 100644 --- a/app/services/section/update_service.rb +++ b/app/services/section/update_service.rb @@ -13,8 +13,8 @@ def call if section.valid? manual.draft - Adapters.publishing.save_draft(manual, include_sections: false) - Adapters.publishing.save_section(section, manual) + PublishingAdapter.save_draft(manual, include_sections: false) + PublishingAdapter.save_section(section, manual) manual.save!(user) end diff --git a/spec/adapters/organisations_adapter_spec.rb b/spec/adapters/organisations_adapter_spec.rb index cdc7f266a..fc2fcaa72 100644 --- a/spec/adapters/organisations_adapter_spec.rb +++ b/spec/adapters/organisations_adapter_spec.rb @@ -2,29 +2,25 @@ describe OrganisationsAdapter do let(:api) { double(:organisations_api) } + let(:response) do + { + "title" => "organisation-title", + "web_url" => "organisation-web-url", + "details" => { + "abbreviation" => "organisation-abbreviation", + "content_id" => "organisation-content-id", + }, + } + end before do allow(Services).to receive(:organisations).and_return(api) + allow(api).to receive(:organisation).with("slug").and_return(response) end describe "#find" do - let(:response) do - { - "title" => "organisation-title", - "web_url" => "organisation-web-url", - "details" => { - "abbreviation" => "organisation-abbreviation", - "content_id" => "organisation-content-id", - }, - } - end - - before do - allow(api).to receive(:organisation).with("slug").and_return(response) - end - it "returns an Organisation populated from the Organisations API" do - organisation = subject.find("slug") + organisation = OrganisationsAdapter.find("slug") expect(organisation.title).to eq("organisation-title") expect(organisation.abbreviation).to eq("organisation-abbreviation") @@ -33,8 +29,8 @@ end it "caches the result for a given slug from the Organisations API" do - subject.find("slug") - subject.find("slug") + OrganisationsAdapter.find("slug") + OrganisationsAdapter.find("slug") expect(api).to have_received(:organisation).with("slug").at_most(:once) end diff --git a/spec/adapters/publishing_adapter_spec.rb b/spec/adapters/publishing_adapter_spec.rb index f2c313ffd..29117deb5 100644 --- a/spec/adapters/publishing_adapter_spec.rb +++ b/spec/adapters/publishing_adapter_spec.rb @@ -20,7 +20,6 @@ let(:timestamp) { Time.zone.parse("2017-01-01 00:00:00") } let(:publishing_api) { double(:publishing_api) } - let(:organisations) { double(:organisations_adapter) } let(:manual_id) { "a55242ed-178f-4716-8bb3-5d4f82d38531" } @@ -72,7 +71,6 @@ before do allow(Services).to receive(:publishing_api).and_return(publishing_api) - allow(Adapters).to receive(:organisations).and_return(organisations) allow(PublicationLog).to receive(:change_notes_for).with("manual-slug") .and_return(publication_logs) @@ -85,7 +83,7 @@ allow(manual).to receive(:version_type).and_return(:new) allow(section).to receive(:needs_exporting?).and_return(true) - allow(organisations).to receive(:find).with("organisation-slug").and_return(organisation) + allow(OrganisationsAdapter).to receive(:find).with("organisation-slug").and_return(organisation) allow(publishing_api).to receive(:patch_links).with(anything, anything) allow(publishing_api).to receive(:put_content).with(anything, anything) end @@ -100,7 +98,7 @@ }, ) - subject.save_draft(manual) + PublishingAdapter.save_draft(manual) end it "saves links for manual to Publishing API with attributes which validate against links schema for manual" do @@ -111,7 +109,7 @@ ), ) - subject.save_draft(manual) + PublishingAdapter.save_draft(manual) end it "saves content for manual to Publishing API" do @@ -173,7 +171,7 @@ }, ) - subject.save_draft(manual) + PublishingAdapter.save_draft(manual) end it "saves links for all manual's sections to Publishing API" do @@ -186,7 +184,7 @@ }, ) - subject.save_draft(manual) + PublishingAdapter.save_draft(manual) end it "saves links for all manual's sections to Publishing API with attributes which validate against links schema for section" do @@ -197,7 +195,7 @@ ), ) - subject.save_draft(manual) + PublishingAdapter.save_draft(manual) end it "saves content for all manual's sections to Publishing API" do @@ -248,7 +246,7 @@ }, ) - subject.save_draft(manual) + PublishingAdapter.save_draft(manual) end context "when section does not need exporting" do @@ -262,7 +260,7 @@ anything, ) - subject.save_draft(manual) + PublishingAdapter.save_draft(manual) end it "does not save content for section to Publishing API" do @@ -271,7 +269,7 @@ anything, ) - subject.save_draft(manual) + PublishingAdapter.save_draft(manual) end context "and action is republish" do @@ -281,7 +279,7 @@ anything, ) - subject.save_draft(manual, republish: true) + PublishingAdapter.save_draft(manual, republish: true) end it "saves content for section to Publishing API" do @@ -290,7 +288,7 @@ anything, ) - subject.save_draft(manual, republish: true) + PublishingAdapter.save_draft(manual, republish: true) end end end @@ -309,7 +307,7 @@ ), ) - subject.save_draft(manual) + PublishingAdapter.save_draft(manual) end it "saves content for section to Publishing API with timestamps" do @@ -321,7 +319,7 @@ ), ) - subject.save_draft(manual) + PublishingAdapter.save_draft(manual) end context "but Manual#use_originally_published_at_for_public_timestamp? is false" do @@ -337,7 +335,7 @@ ), ) - subject.save_draft(manual) + PublishingAdapter.save_draft(manual) end it "saves content for section to Publishing API without public timestamp" do @@ -348,7 +346,7 @@ ), ) - subject.save_draft(manual) + PublishingAdapter.save_draft(manual) end end end @@ -364,7 +362,7 @@ ), ) - subject.save_draft(manual, republish: true) + PublishingAdapter.save_draft(manual, republish: true) end it "saves content for section to Publishing API with republish update_type" do @@ -376,7 +374,7 @@ ), ) - subject.save_draft(manual, republish: true) + PublishingAdapter.save_draft(manual, republish: true) end end end @@ -393,7 +391,7 @@ including(update_type: GdsApiConstants::PublishingApi::MAJOR_UPDATE_TYPE), ) - subject.save_draft(manual) + PublishingAdapter.save_draft(manual) end it "saves content for section to Publishing API with major update_type" do @@ -402,7 +400,7 @@ including(update_type: GdsApiConstants::PublishingApi::MAJOR_UPDATE_TYPE), ) - subject.save_draft(manual) + PublishingAdapter.save_draft(manual) end it_behaves_like "republishing overrides update_type and sets bulk_publishing" @@ -420,7 +418,7 @@ including(update_type: GdsApiConstants::PublishingApi::MINOR_UPDATE_TYPE), ) - subject.save_draft(manual) + PublishingAdapter.save_draft(manual) end it "saves content for section to Publishing API with minor update_type" do @@ -429,7 +427,7 @@ including(update_type: GdsApiConstants::PublishingApi::MINOR_UPDATE_TYPE), ) - subject.save_draft(manual) + PublishingAdapter.save_draft(manual) end it_behaves_like "republishing overrides update_type and sets bulk_publishing" @@ -447,7 +445,7 @@ including(update_type: GdsApiConstants::PublishingApi::MAJOR_UPDATE_TYPE), ) - subject.save_draft(manual) + PublishingAdapter.save_draft(manual) end it "saves content for section to Publishing API with major update_type" do @@ -456,7 +454,7 @@ including(update_type: GdsApiConstants::PublishingApi::MAJOR_UPDATE_TYPE), ) - subject.save_draft(manual) + PublishingAdapter.save_draft(manual) end it_behaves_like "republishing overrides update_type and sets bulk_publishing" @@ -507,7 +505,7 @@ ), ) - subject.save_draft(manual) + PublishingAdapter.save_draft(manual) end end @@ -543,7 +541,7 @@ )), ) - subject.save_draft(manual) + PublishingAdapter.save_draft(manual) end end end @@ -572,7 +570,7 @@ section.uuid, type: "redirect", discard_drafts: false, alternative_path: redirect ) - subject.unpublish_and_redirect_manual_and_sections( + PublishingAdapter.unpublish_and_redirect_manual_and_sections( manual, redirect:, discard_drafts: false ) end @@ -588,13 +586,13 @@ it "unpublishes manual via Publishing API" do expect(publishing_api).to receive(:unpublish).with(manual_id, type: "gone") - subject.unpublish(manual) + PublishingAdapter.unpublish(manual) end it "unpublishes all manual's sections via Publishing API" do expect(publishing_api).to receive(:unpublish).with(section_uuid, type: "gone") - subject.unpublish(manual) + PublishingAdapter.unpublish(manual) end end @@ -632,19 +630,19 @@ it "publishes manual to Publishing API" do expect(publishing_api).to receive(:publish).with(manual_id, nil) - subject.publish(manual) + PublishingAdapter.publish(manual) end it "publishes all manual's sections to Publishing API" do expect(publishing_api).to receive(:publish).with(section_uuid, nil) - subject.publish(manual) + PublishingAdapter.publish(manual) end it "marks all manual's sections as exported" do expect(section).to receive(:mark_as_exported!) - subject.publish(manual) + PublishingAdapter.publish(manual) end it "unpublishes all manual's removed sections via Publishing API" do @@ -655,13 +653,13 @@ discard_drafts: true, ) - subject.publish(manual) + PublishingAdapter.publish(manual) end it "withdraws & marks all manual's removed sections as exported" do expect(removed_section).to receive(:withdraw_and_mark_as_exported!) - subject.publish(manual) + PublishingAdapter.publish(manual) end context "when removed section is withdrawn" do @@ -675,7 +673,7 @@ anything, ) - subject.publish(manual) + PublishingAdapter.publish(manual) end end @@ -683,19 +681,19 @@ it "publishes manual to Publishing API with update type set to republish" do expect(publishing_api).to receive(:publish).with(manual_id, "republish") - subject.publish(manual, republish: true) + PublishingAdapter.publish(manual, republish: true) end it "publishes all manual's sections to Publishing API with update type set to republish" do expect(publishing_api).to receive(:publish).with(section_uuid, "republish") - subject.publish(manual, republish: true) + PublishingAdapter.publish(manual, republish: true) end it "does not mark all manual's sections as exported" do expect(section).not_to receive(:mark_as_exported!) - subject.publish(manual, republish: true) + PublishingAdapter.publish(manual, republish: true) end it "unpublishes all manual's removed sections via Publishing API" do @@ -704,13 +702,13 @@ anything, ) - subject.publish(manual, republish: true) + PublishingAdapter.publish(manual, republish: true) end it "does not mark all manual's removed sections as exported" do expect(removed_section).not_to receive(:withdraw_and_mark_as_exported!) - subject.publish(manual, republish: true) + PublishingAdapter.publish(manual, republish: true) end context "and removed section is withdrawn" do @@ -724,7 +722,7 @@ anything, ) - subject.publish(manual, republish: true) + PublishingAdapter.publish(manual, republish: true) end end end @@ -740,13 +738,13 @@ it "discards draft manual via Publishing API" do expect(publishing_api).to receive(:discard_draft).with(manual_id) - subject.discard(manual) + PublishingAdapter.discard(manual) end it "discards all manual's draft sections via Publishing API" do expect(publishing_api).to receive(:discard_draft).with(section_uuid) - subject.discard(manual) + PublishingAdapter.discard(manual) end end @@ -773,7 +771,7 @@ ], ) - subject.redirect_section(section, to: "/new-location") + PublishingAdapter.redirect_section(section, to: "/new-location") end it "redirects section via Publishing API with attributes which are valid according to redirect schema" do @@ -782,7 +780,7 @@ attributes_valid_according_to_schema("redirect"), ) - subject.redirect_section(manual, to: "/new-location") + PublishingAdapter.redirect_section(manual, to: "/new-location") end end @@ -790,7 +788,7 @@ it "discards draft section via Publishing API" do expect(publishing_api).to receive(:discard_draft).with(section_uuid) - subject.discard_section(section) + PublishingAdapter.discard_section(section) end end diff --git a/spec/factories.rb b/spec/factories.rb index 96c878d5d..721fb4c22 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -65,11 +65,12 @@ organisation_slug { "organisation_slug" } transient do + section_uuids { [] } state { "draft" } end after(:build) do |manual_record, evaluator| - manual_record.editions << FactoryBot.build(:manual_record_edition, state: evaluator.state) + manual_record.editions << FactoryBot.build(:manual_record_edition, state: evaluator.state, section_uuids: evaluator.section_uuids) end after(:create) do |manual_record| diff --git a/spec/lib/manual_withdrawer_spec.rb b/spec/lib/manual_withdrawer_spec.rb new file mode 100644 index 000000000..2772f8ad3 --- /dev/null +++ b/spec/lib/manual_withdrawer_spec.rb @@ -0,0 +1,15 @@ +require "spec_helper" + +describe ManualWithdrawer do + let(:logger) { double(:logger) } + + subject do + described_class.new(logger) + end + + it "raises error when Manual is not found" do + expect { + subject.execute("non-existant-id") + }.to raise_error(RuntimeError, "Manual not found for manual_id `non-existant-id`") + end +end diff --git a/spec/lib/withdraw_and_redirect_manual_spec.rb b/spec/lib/withdraw_and_redirect_manual_spec.rb index e7490023c..94757c617 100644 --- a/spec/lib/withdraw_and_redirect_manual_spec.rb +++ b/spec/lib/withdraw_and_redirect_manual_spec.rb @@ -11,7 +11,6 @@ let(:dry_run) { false } let(:discard_service) { double(:discard_service) } - let(:publishing_adapter) { double(:publishing_adapter) } subject do described_class.new( @@ -27,8 +26,7 @@ allow(Manual::DiscardDraftService).to receive(:new) { discard_service } allow(discard_service).to receive(:call) - allow(Adapters).to receive(:publishing) { publishing_adapter } - allow(publishing_adapter).to receive(:unpublish_and_redirect_manual_and_sections) + allow(PublishingAdapter).to receive(:unpublish_and_redirect_manual_and_sections) end it "withdraws the manual and unpublishes" do @@ -36,7 +34,7 @@ reloaded_manual = Manual.find(manual.id, user) expect(reloaded_manual.withdrawn?).to eq(true) - expect(publishing_adapter).to have_received(:unpublish_and_redirect_manual_and_sections) + expect(PublishingAdapter).to have_received(:unpublish_and_redirect_manual_and_sections) .with(instance_of(Manual), redirect:, discard_drafts:) @@ -59,7 +57,7 @@ reloaded_manual = Manual.find(manual.id, user) expect(reloaded_manual.withdrawn?).to eq(true) - expect(publishing_adapter).to have_received(:unpublish_and_redirect_manual_and_sections) + expect(PublishingAdapter).to have_received(:unpublish_and_redirect_manual_and_sections) .with(instance_of(Manual), redirect:, discard_drafts:) @@ -71,7 +69,7 @@ it "doesn't action the withdrawal" do subject.execute - expect(publishing_adapter).to_not have_received(:unpublish_and_redirect_manual_and_sections) + expect(PublishingAdapter).to_not have_received(:unpublish_and_redirect_manual_and_sections) end end end diff --git a/spec/lib/withdraw_and_redirect_section_spec.rb b/spec/lib/withdraw_and_redirect_section_spec.rb index 4995f2633..60e027416 100644 --- a/spec/lib/withdraw_and_redirect_section_spec.rb +++ b/spec/lib/withdraw_and_redirect_section_spec.rb @@ -4,7 +4,6 @@ let(:manual_record) { FactoryBot.create(:manual_record, :with_sections, state:) } let(:manual) { Manual.build_manual_for(manual_record) } let(:section) { manual.sections.last } - let(:publishing_adapter) { double(:publishing_adapter) } let(:redirect) { "/redirect/blah" } let(:discard_draft) { false } let(:state) { "published" } @@ -22,13 +21,12 @@ end before do - allow(Adapters).to receive(:publishing) { publishing_adapter } - allow(publishing_adapter).to receive(:unpublish_section) + allow(PublishingAdapter).to receive(:unpublish_section) end it "calls the publishing adapter to unpublish the section" do subject.execute - expect(publishing_adapter).to have_received(:unpublish_section) + expect(PublishingAdapter).to have_received(:unpublish_section) .with(instance_of(Section), redirect:, discard_drafts: discard_draft) @@ -51,7 +49,7 @@ manual.save!(User.gds_editor) subject.execute - expect(publishing_adapter).to have_received(:unpublish_section) + expect(PublishingAdapter).to have_received(:unpublish_section) .with(instance_of(Section), redirect:, discard_drafts: discard_draft) @@ -63,7 +61,7 @@ it "doesn't action the withdrawal" do subject.execute - expect(publishing_adapter).to_not have_received(:unpublish_section) + expect(PublishingAdapter).to_not have_received(:unpublish_section) end end end diff --git a/spec/services/manual/create_service_spec.rb b/spec/services/manual/create_service_spec.rb index a8bd61586..f7ca3ab41 100644 --- a/spec/services/manual/create_service_spec.rb +++ b/spec/services/manual/create_service_spec.rb @@ -2,8 +2,7 @@ RSpec.describe Manual::CreateService do let(:user) { double(:user) } - let(:manual) { double(:manual, valid?: nil, save!: nil) } - let(:publishing_api_adapter) { double(:publishing_api_adapter, save_draft: nil) } + let(:manual) { double(:manual, valid?: nil, save!: nil, organisation_slug: "org") } subject do described_class.new( @@ -14,8 +13,6 @@ before do allow(Manual).to receive(:new).and_return(manual) - allow(Adapters) - .to receive(:publishing).and_return(publishing_api_adapter) end context "when the manual is valid" do @@ -25,11 +22,7 @@ it "saves the manual" do expect(manual).to receive(:save!) - subject.call - end - - it "saves the manual to the Publishing API" do - expect(publishing_api_adapter).to receive(:save_draft).with(manual) + expect(PublishingAdapter).to receive(:save_draft).with(manual) subject.call end end @@ -39,21 +32,13 @@ before do allow(manual).to receive(:valid?).and_return(true) - allow(publishing_api_adapter) + allow(PublishingAdapter) .to receive(:save_draft).and_raise(gds_api_exception) end - it "raises the exception" do - expect { subject.call }.to raise_error(gds_api_exception) - end - - it "does not save the manual" do + it "raises the exception and does not save manual" do expect(manual).to_not receive(:save!) - begin - subject.call - rescue StandardError - gds_api_exception - end + expect { subject.call }.to raise_error(gds_api_exception) end end @@ -64,11 +49,7 @@ it "does not save the manual" do expect(manual).to_not receive(:save!) - subject.call - end - - it "does not save the manual to the Publishing API" do - expect(publishing_api_adapter).to_not receive(:save_draft).with(manual) + expect(PublishingAdapter).to_not receive(:save_draft).with(manual) subject.call end end diff --git a/spec/services/manual/discard_draft_service_spec.rb b/spec/services/manual/discard_draft_service_spec.rb index 95e44f66b..ebaeb1aa6 100644 --- a/spec/services/manual/discard_draft_service_spec.rb +++ b/spec/services/manual/discard_draft_service_spec.rb @@ -3,7 +3,6 @@ RSpec.describe Manual::DiscardDraftService do let(:manual_id) { double(:manual_id) } let(:manual) { double(:manual, id: manual_id, has_ever_been_published?: has_ever_been_published, destroy!: nil) } - let(:publishing_adapter) { double(:publishing_adapter) } let(:user) { double(:user) } subject do @@ -15,8 +14,7 @@ before do allow(Manual).to receive(:find) { manual } - allow(Adapters).to receive(:publishing) { publishing_adapter } - allow(publishing_adapter).to receive(:discard) + allow(PublishingAdapter).to receive(:discard) end context "when the manual has never been published" do @@ -29,7 +27,7 @@ it "discards the manual via the publishing-api" do subject.call - expect(publishing_adapter).to have_received(:discard).with(manual) + expect(PublishingAdapter).to have_received(:discard).with(manual) end it "destroys the manual in the local db" do @@ -48,7 +46,7 @@ it "does not discard the manual via the publishing-api" do subject.call - expect(publishing_adapter).not_to have_received(:discard).with(manual) + expect(PublishingAdapter).not_to have_received(:discard).with(manual) end it "does not destroy the manual in the local db" do diff --git a/spec/services/manual/publish_service_spec.rb b/spec/services/manual/publish_service_spec.rb index 876ef2908..0acef5661 100644 --- a/spec/services/manual/publish_service_spec.rb +++ b/spec/services/manual/publish_service_spec.rb @@ -5,7 +5,6 @@ let(:manual_id) { double(:manual_id) } let(:manual) { double(:manual, id: manual_id, version_number: 3) } let(:publication_logger) { double(:publication_logger) } - let(:publishing_adapter) { double(:publishing_adapter) } let(:user) { double(:user) } subject do @@ -21,10 +20,9 @@ allow(manual).to receive(:save!) allow(manual).to receive(:publish) allow(PublicationLogger).to receive(:new) { publication_logger } - allow(Adapters).to receive(:publishing) { publishing_adapter } allow(publication_logger).to receive(:call) - allow(publishing_adapter).to receive(:save_draft) - allow(publishing_adapter).to receive(:publish) + allow(PublishingAdapter).to receive(:save_draft) + allow(PublishingAdapter).to receive(:publish) end context "when the version number is up to date" do @@ -42,20 +40,20 @@ it "calls the publishing api draft exporter" do subject.call - expect(publishing_adapter).to have_received(:save_draft).with(manual) + expect(PublishingAdapter).to have_received(:save_draft).with(manual) end it "calls the new publishing api publisher" do subject.call - expect(publishing_adapter).to have_received(:publish).with(manual) + expect(PublishingAdapter).to have_received(:publish).with(manual) end it "makes the calls to the collaborators in the correct order" do subject.call expect(publication_logger).to have_received(:call).ordered - expect(publishing_adapter).to have_received(:save_draft).ordered - expect(publishing_adapter).to have_received(:publish).ordered + expect(PublishingAdapter).to have_received(:save_draft).ordered + expect(PublishingAdapter).to have_received(:publish).ordered end end diff --git a/spec/services/manual/republish_service_spec.rb b/spec/services/manual/republish_service_spec.rb index 571eed7c6..c2ec07e21 100644 --- a/spec/services/manual/republish_service_spec.rb +++ b/spec/services/manual/republish_service_spec.rb @@ -4,14 +4,12 @@ let(:manual_id) { double(:manual_id) } let(:published_manual_version) { double(:manual) } let(:draft_manual_version) { double(:manual) } - let(:publishing_adapter) { double(:publishing_adapter) } let(:manual) { double(:manual) } let(:user) { double(:user) } before do - allow(Adapters).to receive(:publishing) { publishing_adapter } - allow(publishing_adapter).to receive(:save_draft) - allow(publishing_adapter).to receive(:publish) + allow(PublishingAdapter).to receive(:save_draft) + allow(PublishingAdapter).to receive(:publish) allow(Manual).to receive(:find).with(manual_id, user) { manual } end @@ -26,17 +24,17 @@ it "calls the publishing api draft exporter" do described_class.call(user:, manual_id:) - expect(publishing_adapter).to have_received(:save_draft).with(published_manual_version, republish: true) + expect(PublishingAdapter).to have_received(:save_draft).with(published_manual_version, republish: true) end it "calls the new publishing api publisher" do described_class.call(user:, manual_id:) - expect(publishing_adapter).to have_received(:publish).with(published_manual_version, republish: true) + expect(PublishingAdapter).to have_received(:publish).with(published_manual_version, republish: true) end it "tells the draft listeners nothing" do described_class.call(user:, manual_id:) - expect(publishing_adapter).not_to have_received(:save_draft).with(draft_manual_version, republish: true) + expect(PublishingAdapter).not_to have_received(:save_draft).with(draft_manual_version, republish: true) end end @@ -51,13 +49,13 @@ it "tells the published listeners nothing" do described_class.call(user:, manual_id:) - expect(publishing_adapter).not_to have_received(:publish) - expect(publishing_adapter).not_to have_received(:save_draft).with(published_manual_version, republish: true) + expect(PublishingAdapter).not_to have_received(:publish) + expect(PublishingAdapter).not_to have_received(:save_draft).with(published_manual_version, republish: true) end it "tells the draft listeners to republish the draft version of the manual" do described_class.call(user:, manual_id:) - expect(publishing_adapter).to have_received(:save_draft).with(draft_manual_version, republish: true) + expect(PublishingAdapter).to have_received(:save_draft).with(draft_manual_version, republish: true) end end @@ -72,17 +70,17 @@ it "calls the publishing api draft exporter" do described_class.call(user:, manual_id:) - expect(publishing_adapter).to have_received(:save_draft).with(published_manual_version, republish: true) + expect(PublishingAdapter).to have_received(:save_draft).with(published_manual_version, republish: true) end it "calls the new publishing api publisher" do described_class.call(user:, manual_id:) - expect(publishing_adapter).to have_received(:publish).with(published_manual_version, republish: true) + expect(PublishingAdapter).to have_received(:publish).with(published_manual_version, republish: true) end it "tells the draft listeners to republish the draft version of the manual" do described_class.call(user:, manual_id:) - expect(publishing_adapter).to have_received(:save_draft).with(draft_manual_version, republish: true) + expect(PublishingAdapter).to have_received(:save_draft).with(draft_manual_version, republish: true) end end @@ -96,8 +94,8 @@ it "tells none of the listeners to do anything" do expect { described_class.call(user:, manual_id:) }.to raise_error arbitrary_exception - expect(publishing_adapter).not_to have_received(:save_draft) - expect(publishing_adapter).not_to have_received(:publish) + expect(PublishingAdapter).not_to have_received(:save_draft) + expect(PublishingAdapter).not_to have_received(:publish) end end @@ -112,8 +110,8 @@ it "tells none of the listeners to do anything" do described_class.call(user:, manual_id:) - expect(publishing_adapter).not_to have_received(:save_draft) - expect(publishing_adapter).not_to have_received(:publish) + expect(PublishingAdapter).not_to have_received(:save_draft) + expect(PublishingAdapter).not_to have_received(:publish) end end end diff --git a/spec/services/manual/update_original_publication_date_service_spec.rb b/spec/services/manual/update_original_publication_date_service_spec.rb index 17ba3df07..0232163b0 100644 --- a/spec/services/manual/update_original_publication_date_service_spec.rb +++ b/spec/services/manual/update_original_publication_date_service_spec.rb @@ -7,7 +7,6 @@ let(:section2) { double(:section, assign_attributes: nil) } let(:sections) { [section1, section2] } let(:originally_published_at) { 10.years.ago } - let(:publishing_adapter) { double(:publishing_adapter) } let(:user) { double(:user) } subject do @@ -27,8 +26,7 @@ allow(manual).to receive(:draft) allow(manual).to receive(:assign_attributes) allow(manual).to receive(:save!) - allow(Adapters).to receive(:publishing) { publishing_adapter } - allow(publishing_adapter).to receive(:save_draft) + allow(PublishingAdapter).to receive(:save_draft) end it "updates the manual with only the originally_published_at and use_originally_published_at_for_public_timestamp attribtues" do @@ -58,6 +56,6 @@ subject.call expect(manual).to have_received(:save!).with(user).ordered - expect(publishing_adapter).to have_received(:save_draft).with(manual).ordered + expect(PublishingAdapter).to have_received(:save_draft).with(manual).ordered end end diff --git a/spec/services/manual/update_service_spec.rb b/spec/services/manual/update_service_spec.rb index 4cb1335fa..407916d53 100644 --- a/spec/services/manual/update_service_spec.rb +++ b/spec/services/manual/update_service_spec.rb @@ -2,8 +2,7 @@ RSpec.describe Manual::UpdateService do let(:user) { double(:user) } - let(:manual) { instance_double(Manual, id: "1", draft: nil, assign_attributes: nil, save!: nil) } - let(:publishing_api_adapter) { double(:publishing_api_adapter, save_draft: nil) } + let(:manual) { instance_double(Manual, id: "1", draft: nil, assign_attributes: nil, save!: nil, organisation_slug: "org") } subject do described_class.new( @@ -15,25 +14,23 @@ before do allow(Manual).to receive(:find).and_return(manual) - allow(Adapters) - .to receive(:publishing).and_return(publishing_api_adapter) end it "does not allow saving of an invalid manual" do allow(manual).to receive(:valid?).and_return(false) + expect(PublishingAdapter).not_to receive(:save_draft) subject.call expect(manual).not_to have_received(:save!) - expect(publishing_api_adapter).not_to have_received(:save_draft) end it "allows saving of a valid manual" do allow(manual).to receive(:valid?).and_return(true) + expect(PublishingAdapter).to receive(:save_draft) subject.call expect(manual).to have_received(:save!) - expect(publishing_api_adapter).to have_received(:save_draft) end end diff --git a/spec/services/manual/withdraw_service_spec.rb b/spec/services/manual/withdraw_service_spec.rb new file mode 100644 index 000000000..f9d7fb0c3 --- /dev/null +++ b/spec/services/manual/withdraw_service_spec.rb @@ -0,0 +1,63 @@ +require "spec_helper" + +RSpec.describe Manual::WithdrawService do + let(:user) { User.gds_editor } + let(:state) { "published" } + let!(:manual_record) { FactoryBot.create(:manual_record, manual_id: "manual-id", state:) } + + subject do + described_class.new(user:, manual_id: manual_record.manual_id) + end + + it "raises error when Manual is not found" do + expect { + Manual::WithdrawService.new(user:, manual_id: "non-existant-id").call + }.to raise_error(Manual::NotFoundError, "Manual ID not found: non-existant-id") + end + + it "withdraws published manuals" do + expect(PublishingAdapter).to receive(:unpublish).with(have_attributes(id: manual_record.manual_id)).once + subject.call + expect(manual_record.reload.latest_edition.state).to eq("withdrawn") + end + + context "for already withdrawn manuals" do + let(:state) { "withdrawn" } + it "unpublishes" do + expect(PublishingAdapter).to receive(:unpublish).with(have_attributes(id: manual_record.manual_id)).once + subject.call + expect(manual_record.reload.latest_edition.state).to eq("withdrawn") + end + end + + context "for archived manuals" do + let(:state) { "archived" } + it "does not unpublish archived manuals" do + expect(PublishingAdapter).to_not receive(:unpublish) + subject.call + expect(manual_record.reload.latest_edition.state).to eq("archived") + end + end + + context "for draft only manuals" do + let(:state) { "draft" } + it "does not unpublish" do + expect(PublishingAdapter).to_not receive(:unpublish) + subject.call + expect(manual_record.reload.latest_edition.state).to eq("draft") + end + end + + context "for published with draft manuals" do + it "does not unpublish" do + Manual.build_manual_for(manual_record).draft.save!(user) + expect(PublishingAdapter).to_not receive(:unpublish) + + subject.call + + editions = manual_record.reload.editions + expect(editions[0].state).to eq("published") + expect(editions[1].state).to eq("draft") + end + end +end diff --git a/spec/services/section/create_service_spec.rb b/spec/services/section/create_service_spec.rb index bd783a049..25e0d454b 100644 --- a/spec/services/section/create_service_spec.rb +++ b/spec/services/section/create_service_spec.rb @@ -4,7 +4,6 @@ let(:user) { User.gds_editor } let(:manual) { Manual.new(title: "manual-title") } let(:section_attributes) { double(:section_attributes) } - let(:publishing_api_adapter) { double(:publishing_api) } let(:new_section) do Section.new(manual:, uuid: "uuid") end @@ -24,10 +23,8 @@ allow(manual) .to receive(:build_section) .and_return(new_section) - allow(Adapters).to receive(:publishing) - .and_return(publishing_api_adapter) - allow(publishing_api_adapter).to receive(:save_draft) - allow(publishing_api_adapter).to receive(:save_section) + allow(PublishingAdapter).to receive(:save_draft) + allow(PublishingAdapter).to receive(:save_section) allow(section_attributes).to receive(:merge).and_return({}) allow(user).to receive(:name).and_return("Mr Testy") end @@ -61,14 +58,14 @@ end it "saves the draft manual to the publishing api" do - expect(publishing_api_adapter) + expect(PublishingAdapter) .to receive(:save_draft).with(manual, include_sections: false) subject.call end it "saves the new section to the publishing api" do - expect(publishing_api_adapter) + expect(PublishingAdapter) .to receive(:save_section).with(new_section, manual) subject.call @@ -80,7 +77,7 @@ before do allow(new_section).to receive(:valid?).and_return(true) - allow(publishing_api_adapter) + allow(PublishingAdapter) .to receive(:save_draft) .and_raise(gds_api_exception) end @@ -110,7 +107,7 @@ end it "does not save the section to the publishing api" do - expect(publishing_api_adapter) + expect(PublishingAdapter) .to_not receive(:save_section) begin @@ -126,7 +123,7 @@ before do allow(new_section).to receive(:valid?).and_return(true) - allow(publishing_api_adapter) + allow(PublishingAdapter) .to receive(:save_section) .and_raise(gds_api_exception) end @@ -156,7 +153,7 @@ end it "saves the draft manual to the publishing api" do - expect(publishing_api_adapter) + expect(PublishingAdapter) .to receive(:save_draft).with(manual, include_sections: false) begin @@ -185,14 +182,14 @@ end it "saves the draft manual to the publishing api" do - expect(publishing_api_adapter) + expect(PublishingAdapter) .to_not receive(:save_draft) subject.call end it "saves the new section to the publishing api" do - expect(publishing_api_adapter) + expect(PublishingAdapter) .to_not receive(:save_section) subject.call diff --git a/spec/services/section/remove_service_spec.rb b/spec/services/section/remove_service_spec.rb index ac839891f..a862edb6b 100644 --- a/spec/services/section/remove_service_spec.rb +++ b/spec/services/section/remove_service_spec.rb @@ -1,122 +1,95 @@ require "spec_helper" RSpec.describe Section::RemoveService do - let(:section_uuid) { "123" } - let(:draft_section?) { true } - - let(:manual) do - double( - draft: nil, - remove_section: nil, - find_section: section, - ) - end - - let(:user) { double(:user) } + let(:state) { "published" } + let(:section_edition) { FactoryBot.create(:section_edition, state:) } + let(:manual_record) { FactoryBot.create(:manual_record, state:, section_uuids: [section_edition.section_uuid]) } + let(:change_note_params) { {} } + let(:user) { User.gds_editor } let(:service) do described_class.new( user:, - manual_id: "ABC", - section_uuid:, + manual_id: manual_record.manual_id, + section_uuid: section_edition.section_uuid, attributes: change_note_params, ) end - let(:publishing_adapter) { spy(PublishingAdapter) } before do - allow(Manual).to receive(:find).and_return(manual) - allow(manual).to receive(:save!) - allow(section).to receive(:draft?).and_return(draft_section?) - allow(Adapters).to receive(:publishing).and_return(publishing_adapter) + allow(OrganisationsAdapter).to receive(:find).with(manual_record.organisation_slug) end - context "with a section id that doesn't belong to the manual" do - let(:section) do - double(uuid: section_uuid) + context "with a non-existant manual" do + context "with a section id that doesn't belong to the manual" do + let(:service) do + described_class.new( + user:, + manual_id: "non-existant-id", + section_uuid: section_edition.section_uuid, + attributes: change_note_params, + ) + end + + it "raises a an error and does not remove the section" do + expect(PublishingAdapter).to_not receive(:save_draft) + expect(PublishingAdapter).to_not receive(:discard_section) + expect { + service.call + }.to raise_error(Manual::NotFoundError, "Manual ID not found: non-existant-id") + manual = Manual.find(manual_record.manual_id, user) + expect(manual.sections.map(&:uuid)).to eq([section_edition.section_uuid]) + expect(manual.removed_sections.map(&:uuid)).to eq([]) + expect(manual.state).to eq("published") + end end - let(:manual) do - double( - draft: nil, - sections: [], - remove_section: nil, - find_section: nil, + end + + context "with a section id that doesn't belong to the manual" do + let(:a_different_section) { FactoryBot.create(:section_edition) } + let(:service) do + described_class.new( + user:, + manual_id: manual_record.manual_id, + section_uuid: a_different_section.section_uuid, + attributes: change_note_params, ) end - let(:change_note_params) do - { - minor_update: "0", - change_note: "Make a change", - } - end - it "raises a SectionNotFoundError" do + it "raises a SectionNotFoundError and does not remove the section" do + expect(PublishingAdapter).to_not receive(:save_draft) + expect(PublishingAdapter).to_not receive(:discard_section) expect { service.call - }.to raise_error(described_class::SectionNotFoundError, section_uuid) - end - - context "when SectionNotFoundError is raised" do - before do - expect { service.call }.to raise_error(described_class::SectionNotFoundError) - end - - it "does not mark the manual as a draft" do - expect(manual).not_to have_received(:draft) - end - - it "does not export a manual" do - expect(publishing_adapter).not_to have_received(:save_draft) - end - - it "does not discard a section" do - expect(publishing_adapter).not_to have_received(:discard_section) - end + }.to raise_error(described_class::SectionNotFoundError, a_different_section.section_uuid) + manual = Manual.find(manual_record.manual_id, user) + expect(manual.sections.map(&:uuid)).to eq([section_edition.section_uuid]) + expect(manual.removed_sections.map(&:uuid)).to eq([]) + expect(manual.state).to eq("published") end end context "with invalid change_note params" do - let(:section) do - double( - uuid: section_uuid, - published?: true, - assign_attributes: nil, - valid?: false, - ) - end let(:change_note_params) do { - minor_update: "1", + minor_update: "0", change_note: "", } end - before do + it "does not remove the section, does not save change note, but also does not output any warnings" do + expect(PublishingAdapter).to_not receive(:save_draft) + expect(PublishingAdapter).to_not receive(:discard_section) service.call - end - - it "tries to save the change note to the section" do - expect(section).to have_received(:assign_attributes).with(change_note_params) - end - - it "does not removes the section" do - expect(manual).not_to have_received(:remove_section).with(section.uuid) - end - - it "does not mark the manual as a draft" do - expect(manual).not_to have_received(:draft) - end - - it "does not persists the manual" do - expect(manual).not_to have_received(:save!).with(user) - end - - it "does not export a manual" do - expect(publishing_adapter).not_to have_received(:save_draft) - end - - it "does not discard a section" do - expect(publishing_adapter).not_to have_received(:discard_section) + manual = Manual.find(manual_record.manual_id, user) + expect(manual.sections.map(&:uuid)).to eq([section_edition.section_uuid]) + expect(manual.removed_sections.map(&:uuid)).to eq([]) + expect(manual.state).to eq("published") + sections = SectionEdition.all_for_section(section_edition.section_uuid) + expect(sections.count).to eq(1) + expect(sections.first.state).to eq("published") + expect(sections.first.minor_update).to eq(nil) + expect(sections.first.change_note).to eq("New section added") end end @@ -128,94 +101,70 @@ } end - context "with a section that's previously been published" do - let(:section) do - double( - uuid: section_uuid, - published?: true, - assign_attributes: nil, - valid?: true, - ) - end - - before do + context "when section is published with no draft" do + it "removes the section, saves change notes as new draft, updates manual but does not discard section draft in publishing API" do + expect(PublishingAdapter).to receive(:save_draft).with(have_attributes(id: manual_record.manual_id), include_sections: false) + expect(PublishingAdapter).to_not receive(:discard_section) service.call - end - - it "saves the change note to the section" do - expect(section).to have_received(:assign_attributes).with(change_note_params) - end - - it "removes the section" do - expect(manual).to have_received(:remove_section).with(section.uuid) - end - - it "marks the manual as a draft" do - expect(manual).to have_received(:draft) - end - - it "persists the manual" do - expect(manual).to have_received(:save!).with(user) - end - - it "exports a manual" do - expect(publishing_adapter).to have_received(:save_draft).with(manual, include_sections: false) - end - - it "discards a section" do - expect(publishing_adapter).to have_received(:discard_section).with(section) + manual = Manual.find(manual_record.manual_id, user) + expect(manual.sections.map(&:uuid)).to eq([]) + expect(manual.removed_sections.map(&:uuid)).to eq([section_edition.section_uuid]) + expect(manual.state).to eq("draft") + sections = SectionEdition.all_for_section(section_edition.section_uuid) + expect(sections.count).to eq(2) + expect(sections.first.state).to eq("published") + expect(sections.first.minor_update).to eq(nil) + expect(sections.first.change_note).to eq("New section added") + expect(sections.second.state).to eq("draft") + expect(sections.second.minor_update).to eq(false) + expect(sections.second.change_note).to eq("Make a change") + end + end + + context "with a section published with draft" do + let(:section_edition) { FactoryBot.create(:section_edition, state: "draft", version_number: 2) } + let!(:previous_published_section_edition) { FactoryBot.create(:section_edition, state: "published", version_number: 1, section_uuid: section_edition.section_uuid) } + + it "removes the section, updates change notes to draft, updates manual and discards section drafts in publishing API" do + expect(PublishingAdapter).to receive(:save_draft).with(have_attributes(id: manual_record.manual_id), include_sections: false) + expect(PublishingAdapter).to receive(:discard_section).with(have_attributes(uuid: section_edition.section_uuid)) + service.call + manual = Manual.find(manual_record.manual_id, user) + expect(manual.sections.map(&:uuid)).to eq([]) + expect(manual.removed_sections.map(&:uuid)).to eq([section_edition.section_uuid]) + expect(manual.state).to eq("draft") + sections = SectionEdition.all_for_section(section_edition.section_uuid) + expect(sections.count).to eq(2) + expect(sections.first.state).to eq("draft") + expect(sections.first.minor_update).to eq(false) + expect(sections.first.change_note).to eq("Make a change") + expect(sections.second.state).to eq("published") + expect(sections.second.minor_update).to eq(nil) + expect(sections.second.change_note).to eq("New section added") end end context "with a section that's never been published" do - let(:section) do - double( - uuid: section_uuid, - published?: false, - assign_attributes: nil, - valid?: true, - ) - end + let(:state) { "draft" } - before do + it "removes the section, updates change notes to draft, updates manual and discards section drafts in publishing API" do + expect(PublishingAdapter).to receive(:save_draft).with(have_attributes(id: manual_record.manual_id), include_sections: false) + expect(PublishingAdapter).to receive(:discard_section).with(have_attributes(uuid: section_edition.section_uuid)) service.call - end - - it "saves the change note to the section" do - expect(section).to have_received(:assign_attributes).with(minor_update: "0", change_note: "Make a change") - end - - it "removes the section" do - expect(manual).to have_received(:remove_section).with(section_uuid) - end - - it "marks the manual as a draft" do - # NOTE: this isn't neccesary really, but we do it to be consistent - expect(manual).to have_received(:draft) - end - - it "persists the manual" do - expect(manual).to have_received(:save!).with(user) - end - - it "exports a manual" do - expect(publishing_adapter).to have_received(:save_draft).with(manual, include_sections: false) - end - - it "discards a section" do - expect(publishing_adapter).to have_received(:discard_section).with(section) + manual = Manual.find(manual_record.manual_id, user) + expect(manual.sections.map(&:uuid)).to eq([]) + expect(manual.removed_sections.map(&:uuid)).to eq([section_edition.section_uuid]) + expect(manual.state).to eq("draft") + sections = SectionEdition.all_for_section(section_edition.section_uuid) + expect(sections.count).to eq(1) + expect(sections.first.state).to eq("draft") + expect(sections.first.minor_update).to eq(false) + expect(sections.first.change_note).to eq("Make a change") end end context "with extra section params" do - let(:section) do - double( - uuid: section_uuid, - published?: true, - assign_attributes: nil, - valid?: true, - ) - end + let(:state) { "draft" } let(:change_note_params) do { minor_update: "0", @@ -224,32 +173,20 @@ } end - before do + it "operates as usual and ignores any other parameters" do + expect(PublishingAdapter).to receive(:save_draft).with(have_attributes(id: manual_record.manual_id), include_sections: false) + expect(PublishingAdapter).to receive(:discard_section).with(have_attributes(uuid: section_edition.section_uuid)) service.call - end - - it "only saves the change note params to the section ignoring others" do - expect(section).to have_received(:assign_attributes).with(change_note_params.slice(:change_note, :minor_update)) - end - end - - context "when section is not a draft" do - let(:draft_section?) { false } - let(:section) do - double( - uuid: section_uuid, - published?: true, - assign_attributes: nil, - valid?: false, - ) - end - - before do - service.call - end - - it "doesn't try to discard draft from Publishing API" do - expect(publishing_adapter).to_not have_received(:discard_section) + manual = Manual.find(manual_record.manual_id, user) + expect(manual.sections.map(&:uuid)).to eq([]) + expect(manual.removed_sections.map(&:uuid)).to eq([section_edition.section_uuid]) + expect(manual.state).to eq("draft") + sections = SectionEdition.all_for_section(section_edition.section_uuid) + expect(sections.count).to eq(1) + expect(sections.first.state).to eq("draft") + expect(sections.first.minor_update).to eq(false) + expect(sections.first.change_note).to eq("Make a change") + expect(sections.first.title).to eq(section_edition.title) end end end diff --git a/spec/services/section/update_service_spec.rb b/spec/services/section/update_service_spec.rb index 39848b1dc..74f7e92d4 100644 --- a/spec/services/section/update_service_spec.rb +++ b/spec/services/section/update_service_spec.rb @@ -4,7 +4,6 @@ let(:user) { User.gds_editor } let(:manual) { Manual.new(title: "manual-title") } let(:section_uuid) { "section-uuid" } - let(:publishing_api_adapter) { double(:publishing_api) } let(:section) do Section.new(manual:, uuid: section_uuid) end @@ -25,10 +24,8 @@ allow(Manual) .to receive(:find).with(manual.id, user) .and_return(manual) - allow(Adapters).to receive(:publishing) - .and_return(publishing_api_adapter) - allow(publishing_api_adapter).to receive(:save_draft) - allow(publishing_api_adapter).to receive(:save_section) + allow(PublishingAdapter).to receive(:save_draft) + allow(PublishingAdapter).to receive(:save_section) end context "when the new section is valid" do @@ -60,14 +57,14 @@ end it "saves the draft manual to the publishing api" do - expect(publishing_api_adapter) + expect(PublishingAdapter) .to receive(:save_draft).with(manual, include_sections: false) subject.call end it "saves the new section to the publishing api" do - expect(publishing_api_adapter) + expect(PublishingAdapter) .to receive(:save_section).with(section, manual) subject.call @@ -79,7 +76,7 @@ before do allow(section).to receive(:valid?).and_return(true) - allow(publishing_api_adapter) + allow(PublishingAdapter) .to receive(:save_draft).and_raise(gds_api_exception) end @@ -108,7 +105,7 @@ end it "does not save the section to the publishing api" do - expect(publishing_api_adapter) + expect(PublishingAdapter) .to_not receive(:save_section).with(section, manual) begin @@ -124,7 +121,7 @@ before do allow(section).to receive(:valid?).and_return(true) - allow(publishing_api_adapter) + allow(PublishingAdapter) .to receive(:save_section).and_raise(gds_api_exception) end @@ -153,7 +150,7 @@ end it "saves the draft manual to the publishing api" do - expect(publishing_api_adapter) + expect(PublishingAdapter) .to receive(:save_draft).with(manual, include_sections: false) begin @@ -182,14 +179,14 @@ end it "saves the draft manual to the publishing api" do - expect(publishing_api_adapter) + expect(PublishingAdapter) .to_not receive(:save_draft) subject.call end it "saves the new section to the publishing api" do - expect(publishing_api_adapter) + expect(PublishingAdapter) .to_not receive(:save_section) subject.call