From 1b4666fc6a33315d9f5c788a2f241e6400700522 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Tue, 29 Jan 2019 19:57:46 +0000 Subject: [PATCH 1/2] Add checkbox to cache settings to disable products cache --- .../admin/cache_settings_controller.rb | 9 +++- .../spree/app_configuration_decorator.rb | 3 ++ .../add_caching.html.haml.deface | 2 +- .../{show.html.haml => edit.html.haml} | 13 +++++ config/locales/en.yml | 15 +++--- .../cached_products_renderer.rb | 2 + .../cached_products_renderer_spec.rb | 54 ++++++++++++++++--- 7 files changed, 80 insertions(+), 18 deletions(-) rename app/views/admin/cache_settings/{show.html.haml => edit.html.haml} (52%) diff --git a/app/controllers/admin/cache_settings_controller.rb b/app/controllers/admin/cache_settings_controller.rb index 138bbc7b18..1074f13e9f 100644 --- a/app/controllers/admin/cache_settings_controller.rb +++ b/app/controllers/admin/cache_settings_controller.rb @@ -1,8 +1,7 @@ require 'open_food_network/products_cache_integrity_checker' class Admin::CacheSettingsController < Spree::Admin::BaseController - - def show + def edit @results = Exchange.cachable.map do |exchange| checker = OpenFoodNetwork::ProductsCacheIntegrityChecker.new(exchange.receiver, exchange.order_cycle) @@ -10,4 +9,10 @@ class Admin::CacheSettingsController < Spree::Admin::BaseController end end + def update + Spree::Config.set(params[:preferences]) + respond_to do |format| + format.html { redirect_to main_app.edit_admin_cache_settings_path } + end + end end diff --git a/app/models/spree/app_configuration_decorator.rb b/app/models/spree/app_configuration_decorator.rb index 992c39abe1..e909a0c69b 100644 --- a/app/models/spree/app_configuration_decorator.rb +++ b/app/models/spree/app_configuration_decorator.rb @@ -52,4 +52,7 @@ Spree::AppConfiguration.class_eval do # Number localization preference :enable_localized_number?, :boolean, default: false + + # Enable cache + preference :enable_products_cache?, :boolean, default: true end diff --git a/app/overrides/spree/admin/shared/_configuration_menu/add_caching.html.haml.deface b/app/overrides/spree/admin/shared/_configuration_menu/add_caching.html.haml.deface index c320e072c0..77c808315b 100644 --- a/app/overrides/spree/admin/shared/_configuration_menu/add_caching.html.haml.deface +++ b/app/overrides/spree/admin/shared/_configuration_menu/add_caching.html.haml.deface @@ -1,4 +1,4 @@ / insert_bottom "[data-hook='admin_configurations_sidebar_menu']" %li - = link_to t('admin.cache_settings.show.title'), main_app.admin_cache_settings_path + = link_to t('admin.cache_settings.edit.title'), main_app.edit_admin_cache_settings_path diff --git a/app/views/admin/cache_settings/show.html.haml b/app/views/admin/cache_settings/edit.html.haml similarity index 52% rename from app/views/admin/cache_settings/show.html.haml rename to app/views/admin/cache_settings/edit.html.haml index cecef216a3..222910be94 100644 --- a/app/views/admin/cache_settings/show.html.haml +++ b/app/views/admin/cache_settings/edit.html.haml @@ -1,6 +1,19 @@ - content_for :page_title do = t(:cache_settings) += form_tag main_app.admin_cache_settings_path, :method => :put do + .field + = hidden_field_tag 'preferences[enable_products_cache?]', '0' + = check_box_tag 'preferences[enable_products_cache?]', '1', Spree::Config[:enable_products_cache?] + = label_tag nil, t('.enable_products_cache') + .form-buttons + = button t(:update), 'icon-refresh' + +%br +%br + +%h4= t(:cache_state) +%br %table.index %thead %tr diff --git a/config/locales/en.yml b/config/locales/en.yml index f0c72f289a..ebebe97314 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -389,13 +389,14 @@ en: total_monthly_bill_incl_tax_tip: "The example total monthly bill with tax included, given the settings and the turnover provided." cache_settings: - show: - title: Caching - distributor: Distributor - order_cycle: Order Cycle - status: Status - diff: Diff - error: Error + edit: + title: "Caching" + distributor: "Distributor" + order_cycle: "Order Cycle" + status: "Status" + diff: "Diff" + error: "Error" + enable_products_cache: "Enable Products Cache?" invoice_settings: edit: diff --git a/lib/open_food_network/cached_products_renderer.rb b/lib/open_food_network/cached_products_renderer.rb index 733455ed73..2a3a690059 100644 --- a/lib/open_food_network/cached_products_renderer.rb +++ b/lib/open_food_network/cached_products_renderer.rb @@ -27,6 +27,8 @@ module OpenFoodNetwork private def cached_products_json + return uncached_products_json unless Spree::Config[:enable_products_cache?] + if Rails.env.production? || Rails.env.staging? Rails.cache.fetch("products-json-#{@distributor.id}-#{@order_cycle.id}") do begin diff --git a/spec/lib/open_food_network/cached_products_renderer_spec.rb b/spec/lib/open_food_network/cached_products_renderer_spec.rb index 8a3e72a775..57eead8832 100644 --- a/spec/lib/open_food_network/cached_products_renderer_spec.rb +++ b/spec/lib/open_food_network/cached_products_renderer_spec.rb @@ -7,19 +7,57 @@ module OpenFoodNetwork let(:order_cycle) { double(:order_cycle, id: 456) } let(:cpr) { CachedProductsRenderer.new(distributor, order_cycle) } + # keeps global state unchanged + around do |example| + original_config = Spree::Config[:enable_products_cache?] + example.run + Spree::Config[:enable_products_cache?] = original_config + end + describe "#products_json" do - context "when in testing / development" do - let(:products_renderer) do - double(ProductsRenderer, products_json: 'uncached products') - end + let(:products_renderer) do + double(ProductsRenderer, products_json: 'uncached products') + end + before do + allow(ProductsRenderer) + .to receive(:new) + .with(distributor, order_cycle) { products_renderer } + end + + context "products cache toggle" do before do - allow(ProductsRenderer) - .to receive(:new) - .with(distributor, order_cycle) { products_renderer } + allow(Rails.env).to receive(:production?) { true } + Rails.cache.write "products-json-#{distributor.id}-#{order_cycle.id}", 'products' end - it "returns uncaches products JSON" do + context "disabled" do + before do + Spree::Config[:enable_products_cache?] = false + end + + it "returns uncached products JSON" do + expect(cpr.products_json).to eq 'uncached products' + end + end + + context "enabled" do + before do + Spree::Config[:enable_products_cache?] = true + end + + it "returns the cached JSON" do + expect(cpr.products_json).to eq 'products' + end + end + end + + context "when in testing / development" do + before do + allow(Rails.env).to receive(:production?) { false } + end + + it "returns uncached products JSON" do expect(cpr.products_json).to eq 'uncached products' end end From acc99fc9bbe73c9715b5135b95830e027621a1b9 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Thu, 7 Feb 2019 20:27:51 +0000 Subject: [PATCH 2/2] Improve readability of cached_product_renderer and respective spec --- .../cached_products_renderer.rb | 20 ++++++++-------- .../cached_products_renderer_spec.rb | 24 +++++++++---------- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/lib/open_food_network/cached_products_renderer.rb b/lib/open_food_network/cached_products_renderer.rb index 2a3a690059..abc1f45713 100644 --- a/lib/open_food_network/cached_products_renderer.rb +++ b/lib/open_food_network/cached_products_renderer.rb @@ -27,21 +27,21 @@ module OpenFoodNetwork private def cached_products_json - return uncached_products_json unless Spree::Config[:enable_products_cache?] + return uncached_products_json unless use_cached_products? - if Rails.env.production? || Rails.env.staging? - Rails.cache.fetch("products-json-#{@distributor.id}-#{@order_cycle.id}") do - begin - uncached_products_json - rescue ProductsRenderer::NoProducts - nil - end + Rails.cache.fetch("products-json-#{@distributor.id}-#{@order_cycle.id}") do + begin + uncached_products_json + rescue ProductsRenderer::NoProducts + nil end - else - uncached_products_json end end + def use_cached_products? + Spree::Config[:enable_products_cache?] && (Rails.env.production? || Rails.env.staging?) + end + def uncached_products_json ProductsRenderer.new(@distributor, @order_cycle).products_json end diff --git a/spec/lib/open_food_network/cached_products_renderer_spec.rb b/spec/lib/open_food_network/cached_products_renderer_spec.rb index 57eead8832..6bff04dad2 100644 --- a/spec/lib/open_food_network/cached_products_renderer_spec.rb +++ b/spec/lib/open_food_network/cached_products_renderer_spec.rb @@ -5,7 +5,7 @@ module OpenFoodNetwork describe CachedProductsRenderer do let(:distributor) { double(:distributor, id: 123) } let(:order_cycle) { double(:order_cycle, id: 456) } - let(:cpr) { CachedProductsRenderer.new(distributor, order_cycle) } + let(:cached_products_renderer) { CachedProductsRenderer.new(distributor, order_cycle) } # keeps global state unchanged around do |example| @@ -37,7 +37,7 @@ module OpenFoodNetwork end it "returns uncached products JSON" do - expect(cpr.products_json).to eq 'uncached products' + expect(cached_products_renderer.products_json).to eq 'uncached products' end end @@ -47,7 +47,7 @@ module OpenFoodNetwork end it "returns the cached JSON" do - expect(cpr.products_json).to eq 'products' + expect(cached_products_renderer.products_json).to eq 'products' end end end @@ -58,7 +58,7 @@ module OpenFoodNetwork end it "returns uncached products JSON" do - expect(cpr.products_json).to eq 'uncached products' + expect(cached_products_renderer.products_json).to eq 'uncached products' end end @@ -68,10 +68,10 @@ module OpenFoodNetwork end describe "when the distribution is not set" do - let(:cpr) { CachedProductsRenderer.new(nil, nil) } + let(:cached_products_renderer) { CachedProductsRenderer.new(nil, nil) } it "raises an exception and returns no products" do - expect { cpr.products_json }.to raise_error CachedProductsRenderer::NoProducts + expect { cached_products_renderer.products_json }.to raise_error CachedProductsRenderer::NoProducts end end @@ -81,12 +81,12 @@ module OpenFoodNetwork end it "returns the cached JSON" do - expect(cpr.products_json).to eq 'products' + expect(cached_products_renderer.products_json).to eq 'products' end it "raises an exception when there are no products" do Rails.cache.write "products-json-#{distributor.id}-#{order_cycle.id}", nil - expect { cpr.products_json }.to raise_error CachedProductsRenderer::NoProducts + expect { cached_products_renderer.products_json }.to raise_error CachedProductsRenderer::NoProducts end end @@ -108,11 +108,11 @@ module OpenFoodNetwork describe "when there are products" do it "returns products as JSON" do - expect(cpr.products_json).to eq 'fresh products' + expect(cached_products_renderer.products_json).to eq 'fresh products' end it "caches the JSON" do - cpr.products_json + cached_products_renderer.products_json expect(cached_json).to eq 'fresh products' end end @@ -129,11 +129,11 @@ module OpenFoodNetwork end it "raises an error" do - expect { cpr.products_json }.to raise_error CachedProductsRenderer::NoProducts + expect { cached_products_renderer.products_json }.to raise_error CachedProductsRenderer::NoProducts end it "caches the products as nil" do - expect { cpr.products_json }.to raise_error CachedProductsRenderer::NoProducts + expect { cached_products_renderer.products_json }.to raise_error CachedProductsRenderer::NoProducts expect(cache_present).to be expect(cached_json).to be_nil end