From c0ba00ac048a79669cf3ce673ef346c68f8be594 Mon Sep 17 00:00:00 2001 From: Bruce Bolt Date: Thu, 1 Aug 2024 10:35:09 +0100 Subject: [PATCH] Allow `InContentStoreValidator` to accept multiple schema names This validator checks whether a piece of content in Content Store matches a given schema name. Sometimes we may wish to accept multiple names, so updating the validator to accept an array. --- app/models/publishing_api_redirected_manual.rb | 2 +- .../publishing_api_redirected_section.rb | 2 +- app/models/publishing_api_removed_manual.rb | 2 +- app/models/publishing_api_removed_section.rb | 2 +- app/validators/in_content_store_validator.rb | 16 ++++++++-------- .../in_content_store_validator_spec.rb | 18 +++++++++--------- 6 files changed, 21 insertions(+), 21 deletions(-) diff --git a/app/models/publishing_api_redirected_manual.rb b/app/models/publishing_api_redirected_manual.rb index 024b7777..13758e98 100644 --- a/app/models/publishing_api_redirected_manual.rb +++ b/app/models/publishing_api_redirected_manual.rb @@ -6,7 +6,7 @@ class PublishingAPIRedirectedManual validates :manual_slug, :destination_manual_slug, format: { with: ValidSlug::PATTERN, message: "should match the pattern: #{ValidSlug::PATTERN}" } validates_with InContentStoreValidator, - schema_name: MANUAL_SCHEMA_NAME, + schema_names: [MANUAL_SCHEMA_NAME], content_store: Services.content_store, unless: lambda { errors[:manual_slug].present? || errors[:destination_manual_slug].present? diff --git a/app/models/publishing_api_redirected_section.rb b/app/models/publishing_api_redirected_section.rb index 3b602c79..a29eecb1 100644 --- a/app/models/publishing_api_redirected_section.rb +++ b/app/models/publishing_api_redirected_section.rb @@ -7,7 +7,7 @@ class PublishingAPIRedirectedSection validates :manual_slug, :section_slug, :destination_manual_slug, format: { with: ValidSlug::PATTERN, message: "should match the pattern: #{ValidSlug::PATTERN}" } validates :destination_section_slug, format: { with: ValidSlug::PATTERN, message: "should match the pattern: #{ValidSlug::PATTERN}" }, allow_nil: true validates_with InContentStoreValidator, - schema_name: SECTION_SCHEMA_NAME, + schema_names: [SECTION_SCHEMA_NAME], content_store: Services.content_store, unless: lambda { errors[:manual_slug].present? || diff --git a/app/models/publishing_api_removed_manual.rb b/app/models/publishing_api_removed_manual.rb index d2d2c693..4e8217dd 100644 --- a/app/models/publishing_api_removed_manual.rb +++ b/app/models/publishing_api_removed_manual.rb @@ -6,7 +6,7 @@ class PublishingAPIRemovedManual validates :slug, format: { with: ValidSlug::PATTERN, message: "should match the pattern: #{ValidSlug::PATTERN}" } validates_with InContentStoreValidator, - schema_name: MANUAL_SCHEMA_NAME, + schema_names: [MANUAL_SCHEMA_NAME], content_store: Services.content_store, unless: -> { errors[:slug].present? } diff --git a/app/models/publishing_api_removed_section.rb b/app/models/publishing_api_removed_section.rb index 22c5841a..a2dc0744 100644 --- a/app/models/publishing_api_removed_section.rb +++ b/app/models/publishing_api_removed_section.rb @@ -6,7 +6,7 @@ class PublishingAPIRemovedSection validates :manual_slug, :section_slug, format: { with: ValidSlug::PATTERN, message: "should match the pattern: #{ValidSlug::PATTERN}" } validates_with InContentStoreValidator, - schema_name: SECTION_SCHEMA_NAME, + schema_names: [SECTION_SCHEMA_NAME], content_store: Services.content_store, unless: -> { errors[:manual_slug].present? || errors[:section_slug].present? } diff --git a/app/validators/in_content_store_validator.rb b/app/validators/in_content_store_validator.rb index eb8f13cf..21fc6f08 100644 --- a/app/validators/in_content_store_validator.rb +++ b/app/validators/in_content_store_validator.rb @@ -1,19 +1,19 @@ class InContentStoreValidator < ActiveModel::Validator - attr_reader :schema_name, :content_store + attr_reader :schema_names, :content_store def initialize(options = {}) super - raise "Must provide schema_name and content_store options to the validator" unless options[:schema_name] && options[:content_store] - raise 'Can\'t provide "gone" as a schema_name to the validator' if options[:schema_name] == "gone" + raise "Must provide schema_names and content_store options to the validator" unless options[:schema_names] && options[:content_store] + raise 'Can\'t provide "gone" as schema_names to the validator' if options[:schema_names].include?("gone") - @schema_name = options[:schema_name] + @schema_names = options[:schema_names] @content_store = options[:content_store] end def validate(record) content_item = fetch_content_item(record) - if content_item["schema_name"] != schema_name - record.errors.add(:base, wrong_schema_name_message(record, content_item)) + unless schema_names.include?(content_item["schema_name"]) + record.errors.add(:base, wrong_schema_names_message(record, content_item)) end rescue GdsApi::HTTPNotFound record.errors.add(:base, missing_message(record, content_item)) @@ -35,7 +35,7 @@ def gone_message(_record, _content_item) 'Exists in the content store, but is already "gone"' end - def wrong_schema_name_message(_record, content_item) - %{Exists in the content store, but is not a "#{schema_name}" schema (it's a "#{content_item['schema_name']}" schema)} + def wrong_schema_names_message(_record, content_item) + %{Exists in the content store, but is not a "#{schema_names.join(',')}" schema (it's a "#{content_item['schema_name']}" schema)} end end diff --git a/spec/validators/in_content_store_validator_spec.rb b/spec/validators/in_content_store_validator_spec.rb index a78a595e..76e803dc 100644 --- a/spec/validators/in_content_store_validator_spec.rb +++ b/spec/validators/in_content_store_validator_spec.rb @@ -7,11 +7,11 @@ subject do InContentStoreValidator.new( content_store:, - schema_name:, + schema_names:, ) end let(:content_store) { Services.content_store } - let(:schema_name) { MANUAL_SCHEMA_NAME } + let(:schema_names) { [MANUAL_SCHEMA_NAME, "redirect"] } let(:manual) { PublishingAPIManual.new(slug, attributes) } let(:slug) { "some-slug" } @@ -33,7 +33,7 @@ let(:content_item_schema_name) { "invalid-schema-name" } it "returns an error" do expect(subject.validate(manual).attribute).to eq(:base) - expect(subject.validate(manual).type).to eq("Exists in the content store, but is not a \"#{MANUAL_SCHEMA_NAME}\" schema (it's a \"invalid-schema-name\" schema)") + expect(subject.validate(manual).type).to eq("Exists in the content store, but is not a \"#{MANUAL_SCHEMA_NAME},redirect\" schema (it's a \"invalid-schema-name\" schema)") end end @@ -60,10 +60,10 @@ end context "without a value for schema_name" do - let(:schema_name) { nil } + let(:schema_names) { nil } it "raises an error" do - expect { subject }.to raise_error(RuntimeError, "Must provide schema_name and content_store options to the validator") + expect { subject }.to raise_error(RuntimeError, "Must provide schema_names and content_store options to the validator") end end @@ -71,15 +71,15 @@ let(:content_store) { nil } it "raises an error" do - expect { subject }.to raise_error(RuntimeError, "Must provide schema_name and content_store options to the validator") + expect { subject }.to raise_error(RuntimeError, "Must provide schema_names and content_store options to the validator") end end - context "with 'gone' as the schema_name" do - let(:schema_name) { "gone" } + context "with 'gone' as one of the schema_names" do + let(:schema_names) { %w[gone] } it "raises an error" do - expect { subject }.to raise_error(RuntimeError, "Can't provide \"gone\" as a schema_name to the validator") + expect { subject }.to raise_error(RuntimeError, "Can't provide \"gone\" as schema_names to the validator") end end end