Compare commits

...

29 Commits

Author SHA1 Message Date
Luis Ramos
3d94ce39dd Merge pull request #5793 from luisramos0/payment_fees
Make charges update method update the first pending payment
2020-07-20 21:36:19 +01:00
Luis Ramos
62a3b6b720 Merge pull request #5406 from kristinalim/fix/5300-optimistic_locking_in_stock_items
5300 Avoid race conditions in Spree::StockItem
2020-07-17 22:24:59 +01:00
Luis Ramos
152e432f78 Merge pull request #5749 from mbudm/issue/1253
Ensure the hero image doesn't pixelate on hamburger menu open
2020-07-17 22:15:46 +01:00
Pau Pérez Fabregat
89906f581d Merge pull request #5778 from openfoodfoundation/transifex
Transifex
2020-07-17 18:37:22 +02:00
Transifex-Openfoodnetwork
f31a1ff59c Updating translations for config/locales/en_GB.yml 2020-07-17 04:10:57 +10:00
Steve Roberts
dbc7632c4e Add inline comment to explain two height properties 2020-07-15 09:52:04 +10:00
Steve Roberts
c4d7899a99 Use vh units for new browsers and align tagline bg to top. 2020-07-14 19:26:12 +10:00
Steve Roberts
60870a1215 Fix linting errors 2020-07-14 12:58:48 +10:00
Steve Roberts
63a080266e Merge branch 'master' of https://github.com/openfoodfoundation/openfoodnetwork into issue/1253 2020-07-14 12:45:14 +10:00
Steve Roberts
2f562809c0 Ensure the hero image doesn't attempt to use the full height of all page content
Not sure exactly why this happens, but when the mobile nav is opened the hero image at #tagline:before uses the height of the full window - often around 4000px. Adding max-height of 100% to the nearest safe parent prevents this behaviour.
2020-07-08 21:31:52 +10:00
Luis Ramos
ba50491c6d Restructure the spec a little 2020-06-24 16:16:58 +01:00
Luis Ramos
34207fc20f Bring changes to stock_item from spree 2.1, the previous version was from spree 2.0.4 2020-06-24 16:16:58 +01:00
Luis Ramos
e12e50aa84 Move rubocop exception to rubocop todo 2020-06-24 16:16:58 +01:00
Kristina Lim
20fd3c2642 Reset negative count on hand in existing non backorderable stock items 2020-06-24 16:16:58 +01:00
Kristina Lim
4694f1b21a Require count on hand in non backorderable StockItem to be positive or zero
Fix setting of count on hand in line item specs
2020-06-24 16:16:45 +01:00
Kristina Lim
e53913756c Add lock_version to Spree::StockItem 2020-06-24 16:15:37 +01:00
Kristina Lim
774b3720d5 Update stock item count on hand in Spree core specs 2020-06-24 16:15:09 +01:00
Kristina Lim
13ecf0ec73 Update specs for StockItem with transpec 2020-06-24 16:15:09 +01:00
Kristina Lim
fb20f220c0 Use break instead of return in StockItem#process_backorders
We are not using the return value of this method anywhere.
2020-06-24 16:15:09 +01:00
Kristina Lim
0a1cb71ee4 Ignore Rails/UniqueValidationWithoutIndex for unique index of StockItem#stock_location 2020-06-24 16:15:09 +01:00
Kristina Lim
bc530b92b5 Address violation of Rubocop Rails/Validation: 2020-06-24 16:15:09 +01:00
Kristina Lim
2acf61fd0f Address violation of Rubocop Rails/Delegate 2020-06-24 16:15:09 +01:00
Kristina Lim
1e8543dfe7 Address violation of Rubocop Rails/ReadWriteAttribute 2020-06-24 16:15:09 +01:00
Kristina Lim
22c0693beb Address violation of Rubocop Style/NumericPredicate 2020-06-24 16:15:09 +01:00
Kristina Lim
d1725014c4 Auto-correct violationso of Rubocop Layout/* 2020-06-24 16:15:09 +01:00
Kristina Lim
0fd66f9a55 Auto-correct violationso of Rubocop Style/* 2020-06-24 16:15:09 +01:00
Kristina Lim
b783118700 Auto-correct violationso of Rubocop Style/RedundantSelf 2020-06-24 16:15:09 +01:00
Kristina Lim
84d973d383 Specify RSpec.describe in StockItem spec file 2020-06-24 16:15:09 +01:00
Kristina Lim
0e711832fd Bring Spree::StockItem code from spree_core into the app 2020-06-24 16:15:09 +01:00
10 changed files with 221 additions and 18 deletions

View File

@@ -363,6 +363,13 @@ Rails/UniqueValidationWithoutIndex:
- 'app/models/customer.rb'
- 'app/models/exchange.rb'
# Offense count: 2
# Configuration parameters: Include.
# Include: app/models/**/*.rb
Rails/UniqueValidationWithoutIndex:
Exclude:
- 'app/models/spree/stock_item.rb'
# Offense count: 1
# Configuration parameters: Environments.
# Environments: development, test, production

View File

@@ -17,10 +17,12 @@
position: fixed;
left: 0;
right: 0;
bottom: 0;
top: 0;
z-index: -1;
width: 100%;
height: 100%;
// Use vh units for new browsers - fixed issue 1253
height: 100vh;
}
h1 {

View File

@@ -0,0 +1,58 @@
# frozen_string_literal: true
module Spree
class StockItem < ActiveRecord::Base
acts_as_paranoid
belongs_to :stock_location, class_name: 'Spree::StockLocation'
belongs_to :variant, class_name: 'Spree::Variant'
has_many :stock_movements, dependent: :destroy
validates :stock_location, :variant, presence: true
validates :variant_id, uniqueness: { scope: [:stock_location_id, :deleted_at] }
validates :count_on_hand, numericality: { greater_than_or_equal_to: 0, unless: :backorderable? }
delegate :weight, to: :variant
delegate :name, to: :variant, prefix: true
def backordered_inventory_units
Spree::InventoryUnit.backordered_for_stock_item(self)
end
def adjust_count_on_hand(value)
with_lock do
self.count_on_hand = count_on_hand + value
process_backorders if in_stock?
save!
end
end
def in_stock?
count_on_hand.positive?
end
# Tells whether it's available to be included in a shipment
def available?
in_stock? || backorderable?
end
def variant
Spree::Variant.unscoped { super }
end
private
def count_on_hand=(value)
self[:count_on_hand] = value
end
def process_backorders
backordered_inventory_units.each do |unit|
break unless in_stock?
unit.fill_backorder
end
end
end
end

View File

@@ -182,6 +182,7 @@ en_GB:
explainer: Automatic processing of these orders failed for an unknown reason. This should not occur, please contact us if you are seeing this.
home: "OFN"
title: "Open Food Network"
welcome_to: "Welcome to"
site_meta_description: "The Open Food Network software platform allows farmers to sell produce online, at a price that works for them. It has been built specifically for selling food so it can handle tricky measures or stock levels that only food has - a dozen eggs, a bunch of parsley, a whole chicken that varies in weight…"
search_by_name: Search by name, town, county or postcode...
producers_join: UK producers are now welcome to join Open Food Network UK.
@@ -1713,6 +1714,7 @@ en_GB:
remember_me: Remember Me
are_you_sure: "Are you sure?"
orders_open: "Orders open"
closing: "Closing"
going_back_to_home_page: "Taking you back to the home page"
creating: Creating
updating: Updating

View File

@@ -0,0 +1,5 @@
class AddLockVersionToStockItems < ActiveRecord::Migration
def change
add_column :spree_stock_items, :lock_version, :integer, default: 0
end
end

View File

@@ -0,0 +1,19 @@
# frozen_string_literal: true
class ResetNegativeNonbackorderableCountOnHandInStockItems < ActiveRecord::Migration
module Spree
class StockItem < ActiveRecord::Base
self.table_name = "spree_stock_items"
end
end
def up
Spree::StockItem.where(backorderable: false)
.where("count_on_hand < 0")
.update_all(count_on_hand: 0)
end
def down
raise ActiveRecord::IrreversibleMigration
end
end

View File

@@ -901,6 +901,7 @@ ActiveRecord::Schema.define(version: 20200702112157) do
t.datetime "updated_at", null: false
t.boolean "backorderable", default: false
t.datetime "deleted_at"
t.integer "lock_version", default: 0
end
add_index "spree_stock_items", ["stock_location_id", "variant_id"], name: "stock_item_by_loc_and_var_id", using: :btree

View File

@@ -1,3 +1,5 @@
# frozen_string_literal: true
require 'spec_helper'
# This is the first example of testing concurrency in the Open Food Network.
@@ -15,6 +17,20 @@ describe CheckoutController, concurrency: true, type: :controller do
let(:payment_method) { create(:payment_method, distributors: [distributor]) }
let(:breakpoint) { Mutex.new }
let(:address_params) { address.attributes.except("id") }
let(:order_params) {
{
"payments_attributes" => [
{
"payment_method_id" => payment_method.id,
"amount" => order.total
}
],
"bill_address_attributes" => address_params,
"ship_address_attributes" => address_params,
}
}
before do
# Create a valid order ready for checkout:
create(:shipping_method, distributors: [distributor])
@@ -26,7 +42,9 @@ describe CheckoutController, concurrency: true, type: :controller do
allow(controller).to receive(:spree_current_user).and_return(order.user)
allow(controller).to receive(:current_distributor).and_return(order.distributor)
allow(controller).to receive(:current_order_cycle).and_return(order.order_cycle)
end
it "handles two concurrent orders successfully" do
# New threads start running straight away. The breakpoint is after loading
# the order and before advancing the order's state and making payments.
breakpoint.lock
@@ -36,21 +54,6 @@ describe CheckoutController, concurrency: true, type: :controller do
# I did not find out how to call the original code otherwise.
ActiveSupport::Notifications.instrument("spree.checkout.update")
end
end
it "waits for concurrent checkouts" do
# Basic data the user submits during checkout:
address_params = address.attributes.except("id")
order_params = {
"payments_attributes" => [
{
"payment_method_id" => payment_method.id,
"amount" => order.total
}
],
"bill_address_attributes" => address_params,
"ship_address_attributes" => address_params,
}
# Starting two checkout threads. The controller code will determine if
# these two threads are synchronised correctly or run into a race condition.

View File

@@ -97,7 +97,7 @@ module Spree
end
it "caps at zero when stock is negative" do
v.update! on_hand: -2
v.__send__(:stock_item).update_column(:count_on_hand, -2)
li.cap_quantity_at_stock!
expect(li.reload.quantity).to eq 0
end
@@ -123,7 +123,7 @@ module Spree
before { vo.update(count_on_hand: -3) }
it "caps at zero" do
v.update(on_hand: -2)
v.__send__(:stock_item).update_column(:count_on_hand, -2)
li.cap_quantity_at_stock!
expect(li.reload.quantity).to eq 0
end

View File

@@ -0,0 +1,106 @@
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Spree::StockItem do
let(:stock_location) { create(:stock_location_with_items) }
subject { stock_location.stock_items.order(:id).first }
describe "validation" do
let(:stock_item) { stock_location.stock_items.first }
it "requires count_on_hand to be positive if not backorderable" do
stock_item.backorderable = false
stock_item.__send__(:count_on_hand=, 1)
expect(stock_item.valid?).to eq(true)
stock_item.__send__(:count_on_hand=, 0)
expect(stock_item.valid?).to eq(true)
stock_item.__send__(:count_on_hand=, -1)
expect(stock_item.valid?).to eq(false)
end
it "allows count_on_hand to be negative if backorderable" do
stock_item.backorderable = true
stock_item.__send__(:count_on_hand=, 1)
expect(stock_item.valid?).to eq(true)
stock_item.__send__(:count_on_hand=, -1)
expect(stock_item.valid?).to eq(true)
end
end
it 'maintains the count on hand for a variant' do
expect(subject.count_on_hand).to eq 15
end
it "can return the stock item's variant's name" do
expect(subject.variant_name).to eq(subject.variant.name)
end
context "available to be included in shipment" do
context "has stock" do
it { expect(subject).to be_available }
end
context "backorderable" do
before { subject.backorderable = true }
it { expect(subject).to be_available }
end
context "no stock and not backorderable" do
before do
subject.backorderable = false
allow(subject).to receive_messages(count_on_hand: 0)
end
it { expect(subject).not_to be_available }
end
end
context "adjust count_on_hand" do
let!(:current_on_hand) { subject.count_on_hand }
it 'is updated pessimistically' do
copy = Spree::StockItem.find(subject.id)
subject.adjust_count_on_hand(5)
expect(subject.count_on_hand).to eq(current_on_hand + 5)
expect(copy.count_on_hand).to eq(current_on_hand)
copy.adjust_count_on_hand(5)
expect(copy.count_on_hand).to eq(current_on_hand + 10)
end
context "item out of stock (by two items)" do
let(:inventory_unit) { double('InventoryUnit') }
let(:inventory_unit_2) { double('InventoryUnit2') }
before do
allow(subject).to receive(:backorderable?).and_return(true)
subject.adjust_count_on_hand(- (current_on_hand + 2))
end
it "doesn't process backorders" do
expect(subject).not_to receive(:backordered_inventory_units)
subject.adjust_count_on_hand(1)
end
context "adds new items" do
before { allow(subject).to receive_messages(backordered_inventory_units: [inventory_unit, inventory_unit_2]) }
it "fills existing backorders" do
expect(inventory_unit).to receive(:fill_backorder)
expect(inventory_unit_2).to receive(:fill_backorder)
subject.adjust_count_on_hand(3)
expect(subject.count_on_hand).to eq(1)
end
end
end
end
end