Change calculation payment_total #3

kennyadsl · 2016-10-14T15:54:18Z

Internal review

In Spree::OrderUpdater the update_payment_total subs from payments the
refunds. I change this behavior and now the payment_total is composed
with only payments completed.

rel solidusio#1475

DEPRECATION WARNING: Passing a class as a value in an Active Record query is deprecated and will be removed. Pass a string instead. (called from block in <class:PromotionAction> at ...

The taxon products API which is used to manage display order falls down under its own weight in any sort of reasonable data set. In order to simplify that, I have added the ability to customize the return data from this controller. In a simple case, the results will be the same as they are right now, however you can pass `simple: 1` to the taxon/products API request and it will return significantly less data. Additionally, this updates the backend taxons.js.coffee to use this simple view for all it needs. This reduces the amount of data that needs to be queried and returned in order to make this fast. Previously I was seeing 20+ second response times for a small number of products in a complicated taxon structure. This reduces that down to the 100ms range.

Add the ability to get simple results from taxon API

kennyadsl

I'd also change the commit message to:

Fix how orders payment total is calculated 

In Spree::OrderUpdater the `update_payment_total` was incorrectly 
removing  the refunds total from each order payment. This seems 
incorrect because they are separate things. Refund total has to be 
taken away into the outstanding_balance calculation instead.

kennyadsl · 2016-10-14T16:33:06Z

core/app/models/spree/order.rb

@@ -355,7 +355,7 @@ def create_tax_charge!
    def outstanding_balance
      # If reimbursement has happened add it back to total to prevent balance_due payment state


I think these two comment lines do not apply anymore since we are not adding back any refund (but we are removing it). I'd probably remove them at all since the code is self-explanatory now.

kennyadsl · 2016-10-14T16:34:28Z

core/app/models/spree/order_updater.rb

@@ -73,7 +73,7 @@ def update_shipments
    end

    def update_payment_total
-      order.payment_total = payments.completed.includes(:refunds).map { |payment| payment.amount - payment.refunds.sum(:amount) }.sum
+      order.payment_total = payments.completed.map { |payment| payment.amount }.sum


order.payment_total = payments.completed.sum(&:amount)

kennyadsl · 2016-10-14T16:39:05Z

core/spec/models/spree/order/payment_spec.rb

@@ -185,15 +213,17 @@ module Spree
        context "for canceled orders" do
          before { order.update_attributes(state: 'canceled') }

-          it "it should be a negative amount incorporating reimbursements" do
+          it "it should be a zero amount incorporating reimbursements" do


this specs is saying that it should be a zero amount but you expect -10. Probably the description is wrong?

@prototype

Previously, @prototype was conditionally (if the corresponding ID as included in the params) in a before create callback on the product. This functionality has be moved out into a Prototypes extension.

In the bootstrap refactoring, the argument "wrapper_class" given to rendering the `rebuild_vat_prices_checkbox` was lost, leading to method not founds for stores with VAT. This commit changes the reference from `wrapper_class` to `local_assigns[:wrapper_class]` so that a) the partial does not error out when rendered without it and b) it is immediately apparent to the reader that `wrapper_class` is not a helper.

…nsion Prototypes move to extension

Fix deprecations from PromotionAction.of_type

Change new order button text on users edit page

…ass-error Do not require 'wrapper_class' var in price checkbox partial

kennyadsl

just two small things

kennyadsl · 2016-10-19T08:51:51Z

core/spec/models/spree/order/payment_spec.rb

      it "should return negative amount when payment_total is greater than total" do
        order.total = 8.20
        order.payment_total = 10.20
        expect(order.outstanding_balance).to be_within(0.001).of(-2.00)
      end

+      let(:amount_payed) { 20.0 }
+      let(:reimbursement) { create(:reimbursement) }


@DanielePalombo can this be removed?

kennyadsl · 2016-10-19T08:52:17Z

core/spec/models/spree/order/payment_spec.rb

      context "with reimburesements on the order" do
-        let(:amount) { 10.0 }
+        let(:amount_payed) { 20.0 }


paid everywhere 😄

In Spree::OrderUpdater the `update_payment_total` was incorrectly removing the refunds total from each order payment. This seems incorrect because they are separate things. Refund total has to be taken away into the outstanding_balance calculation instead. rel solidusio#1475

kennyadsl · 2016-10-19T12:13:29Z

review done, opened on solidus repo: solidusio#1536!

Implement voiding transactions

kennyadsl and others added 4 commits October 9, 2016 20:54

Change new order button text on users edit page

e2f98ba

Fix deprecations from PromotionAction.of_type

0960b93

DEPRECATION WARNING: Passing a class as a value in an Active Record query is deprecated and will be removed. Pass a string instead. (called from block in <class:PromotionAction> at ...

Merge pull request solidusio#1331 from gmacdougall/simple-taxon-show-api

721ae1a

Add the ability to get simple results from taxon API

kennyadsl commented Oct 14, 2016

View reviewed changes

Graeme Nathan added 25 commits October 14, 2016 10:18

Remove prototypes controller

4a4e602

Remove prototype views

d3ed7a7

Remove prototype-creation feature spec

8f7f432

Remove @prototype assignment from controller

f29476d

Previously, @prototype was conditionally (if the corresponding ID as included in the params) in a before create callback on the product. This functionality has be moved out into a Prototypes extension.

Remove prototype routes

b2c0680

Remove prototypes from product sub tabs

722863c

Remove prototypes button from toolbar

5434a7f

Remove prototype from form

bb83bc7

Remove prototype for ajax

1ad2198

Remove prototype from new product page

5b5e4c3

Remove prototype clearfix

c7ee8b3

Remove test for prototypes tab on homepage

da8f769

Remove prototype sections of spec

a39a9bd

Delete prototype model

cdc51e1

Remove prototype models and associations

cb83680

Remove callback and accessor from product model

82865eb

Remove relations from property model

2e795e8

Delete property prototype model

3e81f93

Delete prototype taxon

b0c4383

Remove prototype relations from taxon

54faba9

Remove prototype translations

64b8824

Remove prototype from product display permission set

98f92d1

Remove prototype from product management permission set

27b43a9

Remove prototype from PermittedAttributes

a490cb3

Remove prototype factory

923a726

Graeme Nathan and others added 11 commits October 14, 2016 10:18

Remove prototype factory spec

f687a00

Remove prototype from product display spec

46a2d3e

Remove prototype from product management permission set spec

c35c7d4

Remove prototype-contexts from product model spec

25bbec4

Remove prototype shared contexts

0ef151c

Remove sample prototypes

95cd0f1

Update CHANGELOG

4ffd596

Merge pull request solidusio#1517 from gevann/prototypes-move-to-exte…

bac074a

…nsion Prototypes move to extension

Merge pull request solidusio#1522 from jhawthorn/fix_of_type_deprecation

1256569

Fix deprecations from PromotionAction.of_type

Merge pull request solidusio#1488 from nebulab/improve-button-text

ebda5c8

Change new order button text on users edit page

DanielePalombo force-pushed the update-credits-owed branch from 45de5ad to 2e824cd Compare October 18, 2016 16:53

Merge pull request solidusio#1528 from mamhoff/fix-missing-wrapper-cl…

d3e3274

…ass-error Do not require 'wrapper_class' var in price checkbox partial

kennyadsl commented Oct 19, 2016

View reviewed changes

DanielePalombo force-pushed the update-credits-owed branch from 2e824cd to 94e6e54 Compare October 19, 2016 11:27

DanielePalombo force-pushed the update-credits-owed branch from 94e6e54 to c851404 Compare October 19, 2016 11:46

kennyadsl closed this Oct 19, 2016

spaghetticode pushed a commit that referenced this pull request Mar 26, 2021

Merge pull request #3 from adammathys/void

69ba1b0

Implement voiding transactions

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change calculation payment_total #3

Change calculation payment_total #3

kennyadsl commented Oct 14, 2016

kennyadsl left a comment •

edited

Loading

kennyadsl Oct 14, 2016

kennyadsl Oct 14, 2016

kennyadsl Oct 14, 2016

kennyadsl left a comment

kennyadsl Oct 19, 2016

kennyadsl Oct 19, 2016

kennyadsl commented Oct 19, 2016

		@@ -355,7 +355,7 @@ def create_tax_charge!
		def outstanding_balance
		# If reimbursement has happened add it back to total to prevent balance_due payment state

Change calculation payment_total #3

Change calculation payment_total #3

Conversation

kennyadsl commented Oct 14, 2016

Internal review

kennyadsl left a comment • edited Loading

Choose a reason for hiding this comment

kennyadsl Oct 14, 2016

Choose a reason for hiding this comment

kennyadsl Oct 14, 2016

Choose a reason for hiding this comment

kennyadsl Oct 14, 2016

Choose a reason for hiding this comment

kennyadsl left a comment

Choose a reason for hiding this comment

kennyadsl Oct 19, 2016

Choose a reason for hiding this comment

kennyadsl Oct 19, 2016

Choose a reason for hiding this comment

kennyadsl commented Oct 19, 2016

kennyadsl left a comment •

edited

Loading