Merge pull request #1940 from coopdevs/improve-readability-shop

Improve readability of shop's code

app/helpers/injection_helper.rb

@@ -87,10 +87,12 @@ module InjectionHelper
 
   def inject_json_ams(name, data, serializer, opts = {})
     if data.is_a?(Array)
-      json = ActiveModel::ArraySerializer.new(data, {each_serializer: serializer}.merge(opts)).to_json
-    else
-      json = serializer.new(data, opts).to_json
+      opts = { each_serializer: serializer }.merge(opts)
+      serializer = ActiveModel::ArraySerializer
     end
+
+    serializer_instance = serializer.new(data, opts)
+    json = serializer_instance.to_json
     render partial: "json/injection_ams", locals: {name: name, json: json}
   end
 

app/views/shop/products/_form.html.haml

@@ -33,13 +33,6 @@
         = render "shop/products/summary"
         %shop-variant{variant: 'variant', "ng-repeat" => "variant in product.variants | orderBy: ['name_to_display','unit_value'] track by variant.id", "id" => "variant-{{ variant.id }}", "ng-class" => "{'out-of-stock': !variant.on_demand && variant.count_on_hand == 0}"}
 
-      -# Load more button, which can be used to initiate infinite scrolling.
-      -# %product{ "ng-hide" => "Products.loading || !infiniteDisabled || limit >= filteredProducts.length" }
-      -#   .row.summary
-      -#     .small-12.columns.text-center
-      -#       %a.button{ ng: { click: 'infiniteDisabled = false; incrementLimit();' } }
-      -#         Load More Products
-
       %product{"ng-show" => "Products.loading"}
         .row.summary
           .small-12.columns.text-center

lib/open_food_network/cached_products_renderer.rb

@@ -26,17 +26,9 @@ module OpenFoodNetwork
 
     private
 
-    def log_warning
-      if Rails.env.production? || Rails.env.staging?
-        Bugsnag.notify RuntimeError.new("Live server MISS on products cache for distributor: #{@distributor.id}, order cycle: #{@order_cycle.id}")
-      end
-    end
-
     def cached_products_json
       if Rails.env.production? || Rails.env.staging?
         Rails.cache.fetch("products-json-#{@distributor.id}-#{@order_cycle.id}") do
-          log_warning
-
           begin
             uncached_products_json
           rescue ProductsRenderer::NoProducts

spec/lib/open_food_network/cached_products_renderer_spec.rb

@@ -1,6 +1,5 @@
 require 'spec_helper'
 require 'open_food_network/cached_products_renderer'
-require 'open_food_network/products_renderer'
 
 module OpenFoodNetwork
   describe CachedProductsRenderer do
@@ -8,10 +7,16 @@ module OpenFoodNetwork
     let(:order_cycle) { double(:order_cycle, id: 456) }
     let(:cpr) { CachedProductsRenderer.new(distributor, order_cycle) }
 
-    describe "fetching cached products JSON" do
+    describe "#products_json" do
       context "when in testing / development" do
+        let(:products_renderer) do
+          double(ProductsRenderer, products_json: 'uncached products')
+        end
+
         before do
-          allow(cpr).to receive(:uncached_products_json) { "uncached products" }
+          allow(ProductsRenderer)
+            .to receive(:new)
+            .with(distributor, order_cycle) { products_renderer }
         end
 
         it "returns uncaches products JSON" do
@@ -51,10 +56,16 @@ module OpenFoodNetwork
           let(:cache_key) { "products-json-#{distributor.id}-#{order_cycle.id}" }
           let(:cached_json) { Rails.cache.read(cache_key) }
           let(:cache_present) { Rails.cache.exist?(cache_key) }
+          let(:products_renderer) do
+            double(ProductsRenderer, products_json: 'fresh products')
+          end
 
           before do
             Rails.cache.delete(cache_key)
-            cpr.stub(:uncached_products_json) { 'fresh products' }
+
+            allow(ProductsRenderer)
+              .to receive(:new)
+              .with(distributor, order_cycle) { products_renderer }
           end
 
           describe "when there are products" do
@@ -66,15 +77,18 @@ module OpenFoodNetwork
               cpr.products_json
               expect(cached_json).to eq 'fresh products'
             end
-
-            it "logs a warning" do
-              cpr.should_receive :log_warning
-              cpr.products_json
-            end
           end
 
           describe "when there are no products" do
-            before { cpr.stub(:uncached_products_json).and_raise ProductsRenderer::NoProducts }
+            let(:products_renderer) { double(ProductsRenderer) }
+
+            before do
+              allow(products_renderer).to receive(:products_json).and_raise ProductsRenderer::NoProducts
+
+              allow(ProductsRenderer)
+                .to receive(:new)
+                .with(distributor, order_cycle) { products_renderer }
+            end
 
             it "raises an error" do
               expect { cpr.products_json }.to raise_error CachedProductsRenderer::NoProducts
@@ -85,46 +99,9 @@ module OpenFoodNetwork
               expect(cache_present).to be
               expect(cached_json).to be_nil
             end
-
-            it "logs a warning" do
-              cpr.should_receive :log_warning
-              expect { cpr.products_json }.to raise_error CachedProductsRenderer::NoProducts
-            end
           end
         end
       end
     end
-
-    describe "logging a warning" do
-      it "logs a warning when in production" do
-        Rails.env.stub(:production?) { true }
-        expect(Bugsnag).to receive(:notify)
-        cpr.send(:log_warning)
-      end
-
-      it "logs a warning when in staging" do
-        Rails.env.stub(:production?) { false }
-        Rails.env.stub(:staging?) { true }
-        expect(Bugsnag).to receive(:notify)
-        cpr.send(:log_warning)
-      end
-
-      it "does not log a warning in development or test" do
-        expect(Bugsnag).to receive(:notify).never
-        cpr.send(:log_warning)
-      end
-    end
-
-    describe "fetching uncached products from ProductsRenderer" do
-      let(:pr) { double(:products_renderer, products_json: 'uncached products') }
-
-      before do
-        ProductsRenderer.stub(:new) { pr }
-      end
-
-      it "returns the uncached products" do
-        expect(cpr.send(:uncached_products_json)).to eq 'uncached products'
-      end
-    end
   end
 end