mirror of
https://github.com/openfoodfoundation/openfoodnetwork
synced 2026-03-01 02:03:22 +00:00
Wrap commonly-repeated calls to Spree::Config to reduce unnecessary cache reads
These config values are relatively static but in some cases they can be called many times in the same request (like rendering a report or a large list of line_items in BOM). These values will now only get fetched from Redis/Postgres once at most per request/job.
This commit is contained in:
@@ -162,7 +162,7 @@ module Admin
|
||||
def admin_inject_available_units
|
||||
admin_inject_json "admin.products",
|
||||
"availableUnits",
|
||||
Spree::Config.available_units
|
||||
CurrentConfig.get(:available_units)
|
||||
end
|
||||
|
||||
def admin_inject_json(ng_module, name, data)
|
||||
|
||||
19
app/models/current_config.rb
Normal file
19
app/models/current_config.rb
Normal file
@@ -0,0 +1,19 @@
|
||||
# frozen_string_literal: true
|
||||
|
||||
# Wraps repeatedly-called configs in a CurrentAttributes object so they only get fetched once
|
||||
# per request at most, eg: CurrentConfig.get(:available_units) for Spree::Config[:available_units]
|
||||
|
||||
class CurrentConfig < ActiveSupport::CurrentAttributes
|
||||
attribute :display_currency, :hide_cents, :currency_decimal_mark,
|
||||
:currency_thousands_separator, :currency_symbol_position, :available_units
|
||||
|
||||
def get(config_key)
|
||||
return public_send(config_key) unless public_send(config_key).nil?
|
||||
|
||||
public_send("#{config_key}=", Spree::Config.public_send(config_key))
|
||||
end
|
||||
|
||||
def currency
|
||||
ENV.fetch("CURRENCY")
|
||||
end
|
||||
end
|
||||
@@ -120,7 +120,7 @@ module Spree
|
||||
end
|
||||
|
||||
def currency
|
||||
adjustable ? adjustable.currency : Spree::Config[:currency]
|
||||
adjustable ? adjustable.currency : CurrentConfig.get(:currency)
|
||||
end
|
||||
|
||||
def display_amount
|
||||
|
||||
@@ -186,7 +186,7 @@ module Spree
|
||||
end
|
||||
|
||||
def currency
|
||||
self[:currency] || Spree::Config[:currency]
|
||||
self[:currency] || CurrentConfig.get(:currency)
|
||||
end
|
||||
|
||||
def display_item_total
|
||||
@@ -689,7 +689,7 @@ module Spree
|
||||
end
|
||||
|
||||
def set_currency
|
||||
self.currency = Spree::Config[:currency] if self[:currency].nil?
|
||||
self.currency = CurrentConfig.get(:currency) if self[:currency].nil?
|
||||
end
|
||||
|
||||
def using_guest_checkout?
|
||||
|
||||
@@ -37,7 +37,7 @@ module Spree
|
||||
def check_price
|
||||
return unless currency.nil?
|
||||
|
||||
self.currency = Spree::Config[:currency]
|
||||
self.currency = CurrentConfig.get(:currency)
|
||||
end
|
||||
|
||||
# strips all non-price-like characters from the price, taking into account locale settings
|
||||
|
||||
@@ -29,7 +29,7 @@ module Spree
|
||||
end
|
||||
|
||||
def currency
|
||||
order.nil? ? Spree::Config[:currency] : order.currency
|
||||
order.nil? ? CurrentConfig.get(:currency) : order.currency
|
||||
end
|
||||
|
||||
def display_amount
|
||||
|
||||
@@ -163,7 +163,7 @@ module Spree
|
||||
end
|
||||
|
||||
def currency
|
||||
order ? order.currency : Spree::Config[:currency]
|
||||
order ? order.currency : CurrentConfig.get(:currency)
|
||||
end
|
||||
|
||||
def display_cost
|
||||
|
||||
@@ -42,7 +42,7 @@ module Spree
|
||||
accepts_nested_attributes_for :images
|
||||
|
||||
has_one :default_price,
|
||||
-> { with_deleted.where(currency: Spree::Config[:currency]) },
|
||||
-> { with_deleted.where(currency: CurrentConfig.get(:currency)) },
|
||||
class_name: 'Spree::Price',
|
||||
dependent: :destroy
|
||||
has_many :prices,
|
||||
@@ -162,7 +162,7 @@ module Spree
|
||||
where("spree_variants.id in (?)", joins(:prices).
|
||||
where(deleted_at: nil).
|
||||
where('spree_prices.currency' =>
|
||||
currency || Spree::Config[:currency]).
|
||||
currency || CurrentConfig.get(:currency)).
|
||||
where.not(spree_prices: { amount: nil }).
|
||||
select("spree_variants.id"))
|
||||
end
|
||||
@@ -211,7 +211,7 @@ module Spree
|
||||
def check_currency
|
||||
return unless currency.nil?
|
||||
|
||||
self.currency = Spree::Config[:currency]
|
||||
self.currency = CurrentConfig.get(:currency)
|
||||
end
|
||||
|
||||
def save_default_price
|
||||
@@ -223,7 +223,7 @@ module Spree
|
||||
end
|
||||
|
||||
def set_cost_currency
|
||||
self.cost_currency = Spree::Config[:currency] if cost_currency.blank?
|
||||
self.cost_currency = CurrentConfig.get(:currency) if cost_currency.blank?
|
||||
end
|
||||
|
||||
def create_stock_items
|
||||
|
||||
@@ -12,7 +12,7 @@ module Api
|
||||
delegate :balance_value, to: :object
|
||||
|
||||
def balance
|
||||
Spree::Money.new(balance_value, currency: Spree::Config[:currency]).to_s
|
||||
Spree::Money.new(balance_value, currency: CurrentConfig.get(:currency)).to_s
|
||||
end
|
||||
|
||||
def balance_status
|
||||
|
||||
@@ -4,22 +4,22 @@ class Api::CurrencyConfigSerializer < ActiveModel::Serializer
|
||||
attributes :currency, :display_currency, :symbol, :symbol_position, :hide_cents
|
||||
|
||||
def currency
|
||||
Spree::Config[:currency]
|
||||
CurrentConfig.get(:currency)
|
||||
end
|
||||
|
||||
def display_currency
|
||||
Spree::Config[:display_currency]
|
||||
CurrentConfig.get(:display_currency)
|
||||
end
|
||||
|
||||
def symbol
|
||||
::Money.new(1, Spree::Config[:currency]).symbol
|
||||
::Money.new(1, CurrentConfig.get(:currency)).symbol
|
||||
end
|
||||
|
||||
def symbol_position
|
||||
Spree::Config[:currency_symbol_position]
|
||||
CurrentConfig.get(:currency_symbol_position)
|
||||
end
|
||||
|
||||
def hide_cents
|
||||
Spree::Config[:hide_cents]
|
||||
CurrentConfig.get(:hide_cents)
|
||||
end
|
||||
end
|
||||
|
||||
@@ -40,7 +40,7 @@ class WeightsAndMeasures
|
||||
end
|
||||
|
||||
def self.available_units
|
||||
Spree::Config.available_units.split(",")
|
||||
CurrentConfig.get(:available_units).split(",")
|
||||
end
|
||||
|
||||
def self.available_units_sorted
|
||||
|
||||
@@ -79,7 +79,7 @@
|
||||
%fieldset.available_units.no-border-bottom
|
||||
%legend{:align => "center"}= t('admin.available_units')
|
||||
.field
|
||||
- available_units = Spree::Config[:available_units].split(",")
|
||||
- available_units = CurrentConfig.get(:available_units).split(",")
|
||||
- all_units.each do |unit|
|
||||
- selected = available_units.include?(unit)
|
||||
= preference_field_tag("available_units[#{unit}]", selected, { type: :boolean, selected: selected })
|
||||
|
||||
@@ -18,8 +18,8 @@
|
||||
%small
|
||||
%em= Spree.t(:original_amount, scope: 'paypal', amount: @payment.display_amount)
|
||||
%br/
|
||||
- symbol = ::Money.new(1, Spree::Config[:currency]).symbol
|
||||
- if Spree::Config[:currency_symbol_position] == "before"
|
||||
- symbol = ::Money.new(1, CurrentConfig.get(:currency)).symbol
|
||||
- if CurrentConfig.get(:currency_symbol_position) == "before"
|
||||
= symbol
|
||||
= text_field_tag 'refund_amount', @payment.amount
|
||||
- else
|
||||
|
||||
@@ -189,7 +189,7 @@ module Reporting
|
||||
'',
|
||||
'',
|
||||
'',
|
||||
Spree::Config.currency,
|
||||
CurrentConfig.get(:currency),
|
||||
'',
|
||||
order.paid? ? I18n.t(:y) : I18n.t(:n)]
|
||||
end
|
||||
|
||||
@@ -79,7 +79,7 @@ module Spree
|
||||
end
|
||||
|
||||
def current_currency
|
||||
Spree::Config[:currency]
|
||||
CurrentConfig.get(:currency)
|
||||
end
|
||||
|
||||
def ip_address
|
||||
|
||||
@@ -9,7 +9,7 @@ module Spree
|
||||
delegate :cents, to: :money
|
||||
|
||||
def initialize(amount, options = {})
|
||||
@money = ::Monetize.parse([amount, options[:currency] || Spree::Config[:currency]].join)
|
||||
@money = ::Monetize.parse([amount, options[:currency] || CurrentConfig.get(:currency)].join)
|
||||
|
||||
if options.key?(:symbol_position)
|
||||
options[:format] = position_to_format(options.delete(:symbol_position))
|
||||
@@ -20,7 +20,7 @@ module Spree
|
||||
|
||||
# Return the currency symbol (on its own) for the current default currency
|
||||
def self.currency_symbol
|
||||
::Money.new(0, Spree::Config[:currency]).symbol
|
||||
::Money.new(0, CurrentConfig.get(:currency)).symbol
|
||||
end
|
||||
|
||||
def to_s
|
||||
@@ -44,11 +44,11 @@ module Spree
|
||||
|
||||
def defaults
|
||||
{
|
||||
with_currency: Spree::Config[:display_currency],
|
||||
no_cents: Spree::Config[:hide_cents],
|
||||
decimal_mark: Spree::Config[:currency_decimal_mark],
|
||||
thousands_separator: Spree::Config[:currency_thousands_separator],
|
||||
format: position_to_format(Spree::Config[:currency_symbol_position])
|
||||
with_currency: CurrentConfig.get(:display_currency),
|
||||
no_cents: CurrentConfig.get(:hide_cents),
|
||||
decimal_mark: CurrentConfig.get(:currency_decimal_mark),
|
||||
thousands_separator: CurrentConfig.get(:currency_thousands_separator),
|
||||
format: position_to_format(CurrentConfig.get(:currency_symbol_position))
|
||||
}
|
||||
end
|
||||
|
||||
|
||||
@@ -194,6 +194,7 @@ RSpec.configure do |config|
|
||||
spree_config.currency = currency
|
||||
spree_config.shipping_instructions = true
|
||||
end
|
||||
CurrentConfig.clear_all
|
||||
end
|
||||
|
||||
# Don't validate our invalid test data with expensive network requests.
|
||||
|
||||
@@ -24,6 +24,11 @@ describe Spree::Money do
|
||||
end
|
||||
|
||||
context "with currency" do
|
||||
before do
|
||||
allow(ENV).to receive(:fetch).and_call_original
|
||||
allow(ENV).to receive(:fetch).with("CURRENCY").and_return("USD")
|
||||
end
|
||||
|
||||
it "passed in option" do
|
||||
money = Spree::Money.new(10, with_currency: true, html_wrap: false)
|
||||
expect(money.to_s).to eq("$10.00 USD")
|
||||
@@ -96,6 +101,9 @@ describe Spree::Money do
|
||||
config.currency_symbol_position = :after
|
||||
config.display_currency = false
|
||||
end
|
||||
|
||||
allow(ENV).to receive(:fetch).and_call_original
|
||||
allow(ENV).to receive(:fetch).with("CURRENCY").and_return("EUR")
|
||||
end
|
||||
|
||||
# Regression test for Spree #2634
|
||||
|
||||
Reference in New Issue
Block a user