Commit Graph

11640 Commits

Author SHA1 Message Date
Matt-Yorkley
0f5af2d9f1 Merge pull request #6927 from Matt-Yorkley/adjustments-payment-fee
[Adjustments] Payment fee adjustment
2021-03-25 17:40:53 +01:00
Andy Brett
b68476cdcb Merge pull request #7212 from coopdevs/remove-n+1-on-admin-customers
Fix multiple N+1s on /admin/customers.json
2021-03-25 09:29:43 -07:00
Matt-Yorkley
cbd7c9f4c0 Update adjustments controller collection scope 2021-03-25 15:02:02 +00:00
Pau Pérez Fabregat
b12293d1fb Merge pull request #6991 from jibees/6494-show-unit-price-in-admin/product-edit-form
Show unit price in admin/product edit form
2021-03-25 15:59:09 +01:00
Pau Pérez Fabregat
56d2db8c58 Merge pull request #7194 from Matt-Yorkley/deprecations-validates-id
Deprecations: validation definitions
2021-03-25 12:39:33 +01:00
Pau Pérez Fabregat
914e6d5f21 Merge pull request #7196 from Matt-Yorkley/deprecations-uniq
Deprecations: certain uses of #uniq
2021-03-25 12:12:36 +01:00
Pau Perez
ead1ab31b4 Fix multiple N+1 on /admin/customers.json
This (should) considerably improve traces like
https://app.datadoghq.com/apm/trace/917632173599137280?spanID=3163385094622710144&env=production&sort=time&colorBy=service&spanViewType=metadata&graphType=flamegraph&shouldShowLegend=true
by fixing the following 3 N+1s

```
user: root
GET /admin/customers.json?enterprise_id=4
USE eager loading detected
  Customer => [:enterprise]
  Add to your query: .includes([:enterprise])
Call stack
  /usr/src/app/app/serializers/api/admin/customer_with_calculated_balance_serializer.rb:24:in `balance_value'
  /usr/src/app/app/serializers/api/admin/customer_with_calculated_balance_serializer.rb:9:in `balance'
  /usr/src/app/app/controllers/admin/customers_controller.rb:20:in `block (2 levels) in index'
  /usr/src/app/app/controllers/admin/customers_controller.rb:17:in `index'

user: root
GET /admin/customers.json?enterprise_id=4
USE eager loading detected
  Spree::Address => [:state]
  Add to your query: .includes([:state])
Call stack
  /usr/src/app/app/serializers/api/address_serializer.rb:14:in `state_name'
  /usr/src/app/app/controllers/admin/customers_controller.rb:20:in `block (2 levels) in index'
  /usr/src/app/app/controllers/admin/customers_controller.rb:17:in `index'

user: root
GET /admin/customers.json?enterprise_id=4
USE eager loading detected
  Spree::Address => [:country]
  Add to your query: .includes([:country])
Call stack
  /usr/src/app/app/serializers/api/address_serializer.rb:10:in `country_name'
  /usr/src/app/app/controllers/admin/customers_controller.rb:20:in `block (2 levels) in index'
  /usr/src/app/app/controllers/admin/customers_controller.rb:17:in `index'
```

This popped up after improving the balances calculation. Now, that it's
fast, it's clear that are more performance problems on that endpoint.
We'll see if there are any left after this.
2021-03-25 11:19:00 +01:00
Matt-Yorkley
ba3984104b Replace deprecated #alias_method_chain in Spree::Product 2021-03-24 17:51:22 +00:00
Matt-Yorkley
26ed44412f Update #validates definitions
Where the presence of an object is being validated and that object comes from an *association*, we should use `validates :object, presence: true` instead of `validates :object_id, presence: true`.

This does not apply in the same way to validations on uniqueness of certain attributes, such as `validates :object_id, uniqueness...`
2021-03-24 15:43:09 +00:00
Matt-Yorkley
f1e4612c05 Update deprecated uses of #uniq
Enumerable#uniq is fine (eg calling #uniq on an Array object), but now using #uniq on an ActiveRecord::Relation is deprecated in favour of #distinct (which modifies the query itself, as opposed to iterating over the results of the query).
2021-03-24 11:42:24 +00:00
Andy Brett
1cb9504a2c Merge pull request #7187 from luisramos0/uniq
[Rails 5.2] Relation#Uniq is deprecated, distinct is recommended instead
2021-03-23 20:52:47 -07:00
Andy Brett
28ad3a5f1d Merge pull request #7186 from luisramos0/tax_rate_validation_fix
[Rails 5.2] Fix tax_category validation to work in rails 5.2
2021-03-23 20:52:13 -07:00
Luis Ramos
b3d10cfb60 Fix validation of inventory item 2021-03-23 21:22:03 +00:00
Luis Ramos
c2364b3528 Relation#Uniq is deprecated, distinct is recommended instead 2021-03-23 20:57:58 +00:00
Luis Ramos
708c99fe9b Fix validation to work in rails 5.2 2021-03-23 20:33:04 +00:00
Matt-Yorkley
5840b0e33c Adapt adjustment interface for payment's adjustment being singular
Payments only have one adjustment, all other adjustable objects have adjustments (plural).
2021-03-23 18:19:37 +00:00
Matt-Yorkley
5d1d72b36b Update Admin::OrdersHelper#order_adjustments_for_display 2021-03-23 18:19:37 +00:00
Matt-Yorkley
21e0c36f4f Update order totals after deleting shipments and payments 2021-03-23 18:19:06 +00:00
Matt-Yorkley
242ccc4fb3 Update OrderAdjustmentsFetcher 2021-03-23 18:18:36 +00:00
Matt-Yorkley
a0e6b64e98 Update Order serializer 2021-03-23 18:17:50 +00:00
Matt-Yorkley
a42651d543 Update Payment fee adjustment 2021-03-23 18:17:50 +00:00
Matt-Yorkley
525fef14b1 Merge pull request #6974 from Matt-Yorkley/adjustments-line-item-fees
[Adjustments] Update line item enteprise fees
2021-03-23 19:06:02 +01:00
Andy Brett
b539015647 Merge pull request #7025 from Matt-Yorkley/adjustments-order-taxes
[Adjustments] Simplify order tax adjustments
2021-03-23 09:37:44 -07:00
Pau Pérez Fabregat
393ea92dc7 Merge pull request #7177 from coopdevs/fix-balances-order-cycle-report-regression
Fix balances order cycle report regression
2021-03-23 10:08:11 +01:00
Pau Perez
199d48123b Fully qualify table columns in query object
This fixes the error UK's is experiencing:

```
PG::AmbiguousColumn: ERROR: column reference "state" is ambiguous LINE
1: SELECT DISTINCT spree_orders.*, CASE WHEN state IN ('cancele...
^ : SELECT DISTINCT spree_orders.*, CASE WHEN state IN ('canceled',
'returned') THEN payment_total WHEN state IS NOT NULL THEN payment_total
- total ELSE 0 END AS balance_value, spree_orders.* FROM "spree_orders"
INNER JOIN "spree_shipments"
```

See
https://app.bugsnag.com/yaycode/openfoodnetwork-uk/errors/6058c45989d37300079e8312?event_id=6058ccd30075af73bcb20000&i=sk&m=nw.
2021-03-23 09:37:47 +01:00
Andy Brett
e8f00f3d8c Merge branch 'master' into rails52commits 2021-03-22 17:43:34 -07:00
Andy Brett
aab1a5e5d5 Merge pull request #7147 from Matt-Yorkley/deprecations-2
Deprecations 2
2021-03-22 17:28:11 -07:00
Luis Ramos
13b8b3d4a5 Fix rubocop issues 2021-03-22 23:24:01 +00:00
Luis Ramos
3853aed718 Make validator work in rails 5 and rails 5.2 by adding a proc 2021-03-22 22:45:04 +00:00
Andy Brett
dbe06e206c Merge pull request #7048 from andrewpbrett/fix-sub-manifests
Verify line item when placing sub order (Fix #6680)
2021-03-22 10:24:20 -07:00
Matt-Yorkley
3e9d1bfe1f Add defensive more code in not_hidden_for scope 2021-03-22 14:52:42 +00:00
Andy Brett
40be8e2874 Merge pull request #7157 from Matt-Yorkley/dead-code-billing-names
DCOTW: Order#billing_firstname and Order#billing_lastname
2021-03-19 12:12:03 -07:00
Matt-Yorkley
77b0e2d23f Update has_many_through on Order
Fixes:

384) Spree::OrderMailer basic behaviour doesn't aggressively escape double quotes in confirmation body
       Failure/Error: adjustments = adjustments.to_a + order.shipment_adjustments.to_a

       ActionView::Template::Error:
         Cannot have a has_many :through association 'Spree::Order#shipment_adjustments' which goes through 'Spree::Order#shipments' before the through association is defined.
       # ./app/helpers/checkout_helper.rb:10:in `checkout_adjustments_for'
       # ./app/views/spree/order_mailer/_order_summary.html.haml:43:in `_app_views_spree_order_mailer__order_summary_html_haml__2911251238692323485_70331958934800'
       # ./app/views/spree/order_mailer/confirm_email_for_customer.html.haml:23:in `_app_views_spree_order_mailer_confirm_email_for_customer_html_haml__3734564010704881256_70331959518520'
       # ./app/mailers/spree/order_mailer.rb:35:in `block in confirm_email_for_customer'
       # ./app/mailers/spree/order_mailer.rb:33:in `confirm_email_for_customer'
       # ./spec/mailers/order_mailer_spec.rb:20:in `block (3 levels) in <top (required)>'
       # ------------------
       # --- Caused by: ---
       # ActiveRecord::HasManyThroughOrderError:
       #   Cannot have a has_many :through association 'Spree::Order#shipment_adjustments' which goes through 'Spree::Order#shipments' before the through association is defined.
       #   ./app/helpers/checkout_helper.rb:10:in `checkout_adjustments_for'
2021-03-18 21:56:37 +00:00
Matt-Yorkley
d5443cf489 Update has_many_through on ShippingMethod
Fixes:

259) Spree::ShippingMethod#shipments can gather all the related shipments
       Failure/Error: expect(shipping_method.shipments).to include(shipment)

       ActiveRecord::HasManyThroughOrderError:
         Cannot have a has_many :through association 'Spree::ShippingMethod#shipments' which goes through 'Spree::ShippingMethod#shipping_rates' before the through association is defined.
       # ./spec/models/spree/shipping_method_spec.rb:190:in `block (3 levels) in <module:Spree>'
2021-03-18 21:56:37 +00:00
Matt-Yorkley
6132f4bf18 Don't pass string to conditional in validation 2021-03-18 21:56:37 +00:00
Matt-Yorkley
a7fd64d7bf Update has_many_through on Schedule 2021-03-18 21:53:00 +00:00
Matt-Yorkley
eb5c37daca Change order of declaration for has_many_through association with it's parent
Fixes:

292) OrderManagement::Subscriptions::ProxyOrderSyncer#sync! when the subscription is not persisted and the schedule includes upcoming oc that closes after ends_at does not create a new proxy order for that oc
       Failure/Error: order_cycle.schedules << schedule

       ActiveRecord::HasManyThroughOrderError:
         Cannot have a has_many :through association 'OrderCycle#schedules' which goes through 'OrderCycle#order_cycle_schedules' before the through association is defined.
       # ./spec/factories.rb:29:in `block (4 levels) in <top (required)>'
       # ./spec/factories.rb:28:in `each'
       # ./spec/factories.rb:28:in `block (3 levels) in <top (required)>'
       # ./engines/order_management/spec/services/order_management/subscriptions/proxy_order_syncer_spec.rb:59:in `block (4 levels) in <module:Subscriptions>'
       # ./engines/order_management/spec/services/order_management/subscriptions/proxy_order_syncer_spec.rb:65:in `block (4 levels) in <module:Subscriptions>'
       # ./engines/order_management/spec/services/order_management/subscriptions/proxy_order_syncer_spec.rb:133:in `block (6 levels) in <module:Subscriptions>'
       # ./engines/order_management/spec/services/order_management/subscriptions/proxy_order_syncer_spec.rb:133:in `block (5 levels) in <module:Subscriptions>'
2021-03-18 21:53:00 +00:00
Matt-Yorkley
ee0da87681 Fix touch call to unsaved object
Failure/Error: enterprise.andand.touch

       ActiveRecord::ActiveRecordError:
         cannot touch on a new or destroyed record object. Consider using persisted?, new_record?, or destroyed? before touching
       # ./app/models/spree/address.rb:155:in `touch_enterprise'
       # ./spec/factories/product_factory.rb:12:in `block (3 levels) in <top (required)>'
       # ./spec/factories/variant_factory.rb:26:in `block (4 levels) in <top (required)>'
       # ./spec/models/spree/variant_spec.rb:9:in `block (2 levels) in <module:Spree>'
2021-03-18 21:53:00 +00:00
Matt-Yorkley
bdbeb2feb8 Require missing dependency in VariantOverride 2021-03-18 21:53:00 +00:00
Matt-Yorkley
479c1ae6e6 Merge pull request #7062 from mkllnk/6327-terms-of-service
Show Terms of Service at checkout if required
2021-03-18 19:16:06 +01:00
Andy Brett
afa7269f48 Merge branch 'master' into render_plain 2021-03-18 10:53:13 -07:00
Matt-Yorkley
2e20078c05 Delete dead code Order#billing_firstname and Order#billing_lastname
🔥
2021-03-18 15:54:40 +00:00
Andy Brett
97a63edf50 Merge pull request #7096 from luisramos0/rails-5-2-helper
Rails 5.2 add T&Cs helper
2021-03-18 08:16:12 -07:00
Pau Pérez Fabregat
a1af16ae72 Merge pull request #7116 from coopdevs/increase-invoice-renderer-coverage
Better test InvoiceRenderer
2021-03-18 15:54:43 +01:00
Pau Pérez Fabregat
71b902d9d2 Merge pull request #6980 from drummer83/clean-footer
Cleaner and consistent design of front-end footer
2021-03-18 15:53:09 +01:00
Matt-Yorkley
49f1e44999 Simplify Order#total_tax to cover all taxes from all sources 2021-03-18 14:05:29 +00:00
Matt-Yorkley
9f23bb2203 Update SubscriptionPlacementJob fee deleting 2021-03-18 13:18:20 +00:00
Matt-Yorkley
cafe1b5f1c Update order totals after creating fees
This was being triggered by a callback in Spree::Adjustments before, but now that the adjustable is not the order, it does not get triggered by fees being added to line items...
2021-03-18 13:18:20 +00:00
Matt-Yorkley
526794b32e Refactor CheckoutHelper 2021-03-18 13:18:20 +00:00
Matt-Yorkley
4ee0271756 Loosen scope in order enterprise fees tax total method 2021-03-18 13:18:20 +00:00