From bc34d04c31b4e280ce23c35cb764039eb6dfb1e1 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Wed, 6 Mar 2019 15:57:18 +0100
Subject: [PATCH 1/3] Do not retry when refreshing cache on deleted OC

---
 app/jobs/refresh_products_cache_job.rb       |  2 ++
 spec/jobs/refresh_products_cache_job_spec.rb | 30 +++++++++++++++++---
 2 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/app/jobs/refresh_products_cache_job.rb b/app/jobs/refresh_products_cache_job.rb
index 2662237dc1..b08148b280 100644
--- a/app/jobs/refresh_products_cache_job.rb
+++ b/app/jobs/refresh_products_cache_job.rb
@@ -3,6 +3,8 @@ require 'open_food_network/products_renderer'
 RefreshProductsCacheJob = Struct.new(:distributor_id, :order_cycle_id) do
   def perform
     Rails.cache.write(key, products_json)
+  rescue ActiveRecord::RecordNotFound
+    true
   end
 
   private
diff --git a/spec/jobs/refresh_products_cache_job_spec.rb b/spec/jobs/refresh_products_cache_job_spec.rb
index d20efe5fad..c1db2f2732 100644
--- a/spec/jobs/refresh_products_cache_job_spec.rb
+++ b/spec/jobs/refresh_products_cache_job_spec.rb
@@ -5,12 +5,34 @@ describe RefreshProductsCacheJob do
   let(:distributor) { create(:distributor_enterprise) }
   let(:order_cycle) { create(:simple_order_cycle) }
 
-  it "renders products and writes them to cache" do
-    RefreshProductsCacheJob.any_instance.stub(:products_json) { 'products' }
+  context 'when the enterprise and the order cycle exist' do
+    it "renders products and writes them to cache" do
+      RefreshProductsCacheJob.any_instance.stub(:products_json) { 'products' }
 
-    run_job RefreshProductsCacheJob.new distributor.id, order_cycle.id
+      run_job RefreshProductsCacheJob.new distributor.id, order_cycle.id
 
-    expect(Rails.cache.read("products-json-#{distributor.id}-#{order_cycle.id}")).to eq 'products'
+      expect(Rails.cache.read("products-json-#{distributor.id}-#{order_cycle.id}")).to eq 'products'
+    end
+  end
+
+  context 'when the order cycle does not exist' do
+    before do
+      allow(OrderCycle)
+        .to receive(:find)
+        .with(order_cycle.id)
+        .and_raise(ActiveRecord::RecordNotFound)
+    end
+
+    it 'does not raise' do
+      expect {
+        run_job RefreshProductsCacheJob.new(distributor.id, order_cycle.id)
+      }.not_to raise_error(/ActiveRecord::RecordNotFound/)
+    end
+
+    it 'returns true' do
+      refresh_products_cache_job = RefreshProductsCacheJob.new(distributor.id, order_cycle.id)
+      expect(refresh_products_cache_job.perform).to eq(true)
+    end
   end
 
   describe "fetching products JSON" do

From b9636b975a74131729b95a70b08547709cb8ee64 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Wed, 6 Mar 2019 16:07:22 +0100
Subject: [PATCH 2/3] Refactor test to stop using .any_instance

Although might be useful in very particular cases its use is discourage
by RSpec itself. See https://relishapp.com/rspec/rspec-mocks/docs/working-with-legacy-code/any-instance
---
 spec/jobs/refresh_products_cache_job_spec.rb | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/spec/jobs/refresh_products_cache_job_spec.rb b/spec/jobs/refresh_products_cache_job_spec.rb
index c1db2f2732..c9476891b7 100644
--- a/spec/jobs/refresh_products_cache_job_spec.rb
+++ b/spec/jobs/refresh_products_cache_job_spec.rb
@@ -6,11 +6,13 @@ describe RefreshProductsCacheJob do
   let(:order_cycle) { create(:simple_order_cycle) }
 
   context 'when the enterprise and the order cycle exist' do
-    it "renders products and writes them to cache" do
-      RefreshProductsCacheJob.any_instance.stub(:products_json) { 'products' }
+    before do
+      refresh_products_cache_job = instance_double(OpenFoodNetwork::ProductsRenderer, products_json: 'products')
+      allow(OpenFoodNetwork::ProductsRenderer).to receive(:new).with(distributor, order_cycle) { refresh_products_cache_job }
+    end
 
+    it 'renders products and writes them to cache' do
       run_job RefreshProductsCacheJob.new distributor.id, order_cycle.id
-
       expect(Rails.cache.read("products-json-#{distributor.id}-#{order_cycle.id}")).to eq 'products'
     end
   end

From e60437c6a245311aac23f23ac1878e066a5c1f58 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Wed, 6 Mar 2019 16:08:34 +0100
Subject: [PATCH 3/3] Refactor test to execute public API instead

---
 spec/jobs/refresh_products_cache_job_spec.rb | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/spec/jobs/refresh_products_cache_job_spec.rb b/spec/jobs/refresh_products_cache_job_spec.rb
index c9476891b7..da62861717 100644
--- a/spec/jobs/refresh_products_cache_job_spec.rb
+++ b/spec/jobs/refresh_products_cache_job_spec.rb
@@ -38,12 +38,16 @@ describe RefreshProductsCacheJob do
   end
 
   describe "fetching products JSON" do
-    let(:job) { RefreshProductsCacheJob.new distributor.id, order_cycle.id }
-    let(:pr) { double(:products_renderer, products_json: nil) }
+    let(:job) { RefreshProductsCacheJob.new(distributor.id, order_cycle.id) }
+    let(:products_renderer) { instance_double(OpenFoodNetwork::ProductsRenderer, products_json: nil) }
+
+    before do
+      allow(OpenFoodNetwork::ProductsRenderer).to receive(:new).with(distributor, order_cycle) { products_renderer }
+    end
 
     it "fetches products JSON" do
-      expect(OpenFoodNetwork::ProductsRenderer).to receive(:new).with(distributor, order_cycle) { pr }
-      job.send(:products_json)
+      job.perform
+      expect(OpenFoodNetwork::ProductsRenderer).to have_received(:new).with(distributor, order_cycle) { products_renderer }
     end
   end
 end