Merge pull request #36 from alphagov/doctype

Simplify document type triage logic

lib/publishing_event_pipeline/document.rb

@@ -1,9 +1,15 @@
 module PublishingEventPipeline
   module Document
+    # When a document is unpublished in the source system, its document type changes to one of
+    # these values. While semantically different for other systems, we only need to know that they
+    # imply removal from search.
+    UNPUBLISH_DOCUMENT_TYPES = %w[gone redirect substitute vanish].freeze
+
     # Factory method returning a Document instance of an appropriate concrete type for the given
     # document hash.
     def self.for(document_hash)
-      if Unpublish.handles?(document_hash)
+      case document_hash["document_type"]
+      when *UNPUBLISH_DOCUMENT_TYPES
         Unpublish.new(document_hash)
       else
         Publish.new(document_hash)

lib/publishing_event_pipeline/document/unpublish.rb

@@ -1,16 +1,6 @@
 module PublishingEventPipeline
   module Document
     class Unpublish < Base
-      # When a document is unpublished in the source system, its document type changes to one of
-      # these values. While semantically different for other systems, we only need to know that they
-      # imply removal from search.
-      UNPUBLISH_DOCUMENT_TYPES = %w[gone redirect substitute vanish].freeze
-
-      # Returns whether this class can handle the given document hash.
-      def self.handles?(document_hash)
-        UNPUBLISH_DOCUMENT_TYPES.include?(document_hash.fetch("document_type"))
-      end
-
       # Synchronize the document to the given repository (i.e. delete it from the repository).
       def synchronize_to(repository)
         repository.delete(content_id, payload_version:)

spec/lib/publishing_event_pipeline/document/unpublish_spec.rb

@@ -16,24 +16,6 @@
     }
   end
 
-  describe ".handles?" do
-    subject(:handles) { described_class.handles?(document_hash) }
-
-    %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(true) }
-      end
-    end
-
-    context "when the document type is not one of the unpublish document types" do
-      let(:document_type) { "anything-else" }
-
-      it { is_expected.to be(false) }
-    end
-  end
-
   describe "#content_id" do
     it "returns the content_id from the document hash" do
       expect(document.content_id).to eq(content_id)

spec/lib/publishing_event_pipeline/document_spec.rb

@@ -2,28 +2,20 @@
   describe ".for" do
     subject(:document) { described_class.for(document_hash) }
 
-    let(:document_hash) { double }
+    let(:document_hash) { { "document_type" => document_type } }
 
-    context "when the document is handled by Unpublish" do
-      before do
-        allow(PublishingEventPipeline::Document::Unpublish)
-          .to receive(:handles?).with(document_hash).and_return(true)
-      end
+    %w[gone redirect substitute vanish].each do |document_type|
+      context "when the document type is #{document_type}" do
+        let(:document_type) { document_type }
 
-      it "returns an Unpublish document" do
-        expect(document).to be_a(PublishingEventPipeline::Document::Unpublish)
+        it { is_expected.to be_a(PublishingEventPipeline::Document::Unpublish) }
       end
     end
 
-    context "when the document is not handled by Unpublish" do
-      before do
-        allow(PublishingEventPipeline::Document::Unpublish)
-          .to receive(:handles?).with(document_hash).and_return(false)
-      end
+    context "when the document type is not one of the unpublish document types" do
+      let(:document_type) { "anything-else" }
 
-      it "returns a Publish document" do
-        expect(document).to be_a(PublishingEventPipeline::Document::Publish)
-      end
+      it { is_expected.to be_a(PublishingEventPipeline::Document::Publish) }
     end
   end
 end