Merge pull request #983 from alphagov/remove-redirected-manual

Allow previously redirected manuals to be removed

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?

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? ||

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, "redirect"],
                  content_store: Services.content_store,
                  unless: -> { errors[:slug].present? }
 

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, "redirect"],
                  content_store: Services.content_store,
                  unless: -> { errors[:manual_slug].present? || errors[:section_slug].present? }
 

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

spec/models/publishing_api_removed_manual_spec.rb

@@ -34,6 +34,12 @@
         expect(subject).to be_valid
       end
 
+      it 'is valid when the slugs represent a "redirect" piece of content' do
+        content_item = hmrc_manual_content_item_for_base_path(manual_path).merge("schema_name" => "redirect")
+        stub_content_store_has_item(manual_path, content_item)
+        expect(subject).to be_valid
+      end
+
       it "is invalid when the slug represents a piece of content with any other schema_name" do
         stub_content_store_has_item(manual_path)
         expect(subject).not_to be_valid

spec/models/publishing_api_removed_section_spec.rb

@@ -59,6 +59,12 @@
         expect(subject).to be_valid
       end
 
+      it 'is valid when the slugs represent a "redirect" piece of content' do
+        content_item = hmrc_manual_section_content_item_for_base_path(section_path).merge("schema_name" => "redirect")
+        stub_content_store_has_item(section_path, content_item)
+        expect(subject).to be_valid
+      end
+
       it "is invalid when the slugs represents a piece of content with any other schema_name" do
         stub_content_store_has_item(section_path)
         expect(subject).not_to be_valid

spec/validators/in_content_store_validator_spec.rb

@@ -0,0 +1,85 @@
+require "rails_helper"
+require "gds_api/test_helpers/content_store"
+
+describe InContentStoreValidator do
+  include GdsApi::TestHelpers::ContentStore
+
+  subject do
+    InContentStoreValidator.new(
+      content_store:,
+      schema_names:,
+    )
+  end
+  let(:content_store) { Services.content_store }
+  let(:schema_names) { [MANUAL_SCHEMA_NAME, "redirect"] }
+
+  let(:manual) { PublishingAPIManual.new(slug, attributes) }
+  let(:slug) { "some-slug" }
+  let(:attributes) { valid_manual }
+  let(:content_item_schema_name) { MANUAL_SCHEMA_NAME }
+
+  before do
+    stub_content_store_has_item("/hmrc-internal-manuals/#{slug}",
+                                content_item_for_base_path("/hmrc-internal-manuals/#{slug}").merge(schema_name: content_item_schema_name))
+  end
+
+  context "with a matching schema name" do
+    it "returns no errors" do
+      expect(subject.validate(manual)).to be_nil
+    end
+  end
+
+  context "with an invalid schema name" do
+    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},redirect\" schema (it's a \"invalid-schema-name\" schema)")
+    end
+  end
+
+  context "with a content item that does not exist" do
+    before do
+      stub_content_store_does_not_have_item("/hmrc-internal-manuals/#{slug}")
+    end
+
+    it "returns an error" do
+      expect(subject.validate(manual).attribute).to eq(:base)
+      expect(subject.validate(manual).type).to eq("Is not a manual in the content store")
+    end
+  end
+
+  context "with a content item that is gone" do
+    before do
+      stub_content_store_has_gone_item("/hmrc-internal-manuals/#{slug}")
+    end
+
+    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 already \"gone\"")
+    end
+  end
+
+  context "without a value for schema_name" do
+    let(:schema_names) { nil }
+
+    it "raises an error" do
+      expect { subject }.to raise_error(RuntimeError, "Must provide schema_names and content_store options to the validator")
+    end
+  end
+
+  context "without a value for content_store" do
+    let(:content_store) { nil }
+
+    it "raises an error" do
+      expect { subject }.to raise_error(RuntimeError, "Must provide schema_names and content_store options to the validator")
+    end
+  end
+
+  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 schema_names to the validator")
+    end
+  end
+end