[Vouchers] Implement bare minimum checkout

[lin-d-hop]

Description

The bare minimum frontend implements the initial logic to use a voucher. These vouchers will be vastly simplified from the final designs

On the checkout flow:

• the voucher code box is only displayed if the enterprise has vouchers
• 'Apply Voucher' button and voucher code box are displayed in checkout step 2
• No need to be able to 'reset code' at checkout step 2
• Include the vouchers aspects of the order summary on checkout step 3
• Not necessary to include the voucher information on the Payment Method section of checkout step 3

Acceptance Criteria & Tests

1. Create voucher codes in the backoffice
2. Apply to a checkout flow and see the discount applied
3. If the order total goes below zero when the code is applied it should stay at zero
4. Disregard (at this stage) any case that additional negative fees apply to the order
5. Voucher codes should be able to be used unlimited number of times
6. Voucher codes should apply to any shoppers
7. Voucher codes should not apply in enterprises that have not enabled them. If the enterprise has enabled no voucher code the voucher code box should not be visible at checkout
8. If the voucher code covers the cost of the whole order, just display the full range of payment methods, no clever logic at this stage.
9. Unit tests as standard
10. Translations as standard.

[mkllnk]

Tax calculation

Vouchers discount the order total after applying tax. And the tax has to be reduced proportionally. After discussing several options of how to store the applied discount, we settled on the Spree::Adjustment with either included (negative) tax or an additional tax adjustment for prices excluding tax.

Here are four examples of how to calculate and store a voucher's discount value.

Tax included in price

An order has items worth $100. Some of these items include tax which totals to $5. The total including tax is $100.

Now we apply a 10% voucher which results in an adjustment of minus $10. The order total is reduced to $90 and the tax has to be reduced to $4.50. So we create an Adjustment with a value of -10 and included tax of -0.5. Now that included tax is calculated by the voucher calculator. It's not a tax rate applied to the adjustment but a note of reduced tax stored within the discount adjustment.

Spree::Adjustment.new(
  amount: -1 * voucher_rate * order_total, # -10% * $100 = 10%
  included_tax: -1 * voucher_rate * included_tax_total, # -10% * $5
)

If we apply an absolute $10 discount then we also have an adjustment of minus $10. The order total is reduced to $90 and the tax has also to be reduced to $4.50. But in order to calculate the tax, we have to work out the proportion first.

voucher_rate = voucher_amount / order_total # $10 / $100 = 10%
Spree::Adjustment.new(
  amount: -1 * voucher_amount, # -$10
  included_tax: -1 * voucher_rate * included_tax_total, # -10% * $5
)

Tax on top of price

An order has items worth $100 again but the tax is calculated on top. The additional tax amounts to $5 and is stored in tax adjustments. The order total with tax is $105.

When we apply a 10% voucher it results in an adjustment of minus $10 on the items and a tax adjustment of minus $0.50. So the total discount value is $10.50.

Spree::Adjustment.new(
  amount: -1 * voucher_rate * (order_total - additional_tax_total), # -10% * $100
  included_tax: 0, # when no included tax present, calculation same as above
)

Spree::Adjustment.new(
  amount: -1 * voucher_rate * additional_tax_total, # -10% * $5
)

And finally, when we apply a $10 discount to this order then the total adjustment value is only $10. The order total will be reduced from $105 to $95. The discount rate has to be calculated and the discounted tax has to be stored in a separate adjustment again.

voucher_rate = voucher_amount / order_total # $10 / $105 = 9.5238095 %

Spree::Adjustment.new(
  amount: -1 * voucher_rate * (order_total - additional_tax_total), # - 9.5238095% * $100
  included_tax: 0, # when no included tax present, calculation same as above
)

Spree::Adjustment.new(
  amount: -1 * voucher_rate * additional_tax_total, # -9.5238095% * $5
)

Mixed tax

The system can combine tax included and excluded. Some items have tax and others don't and they can be multiple tax rates. Some taxes like GST also apply to service fees which we store as adjustments. So the simplified examples above need to be combined. The included tax is always calculated but may be zero. And the additional tax adjustment only needs to be created when we have any additional tax on the order.

Implementation notes

Adjustments have an originator which tells us what kind of adjustment it is. So if the originator is a tax rate then it's a tax adjustment. If the originator is an enterprise fee then it's the applied fee. The adjustments we create here originate from the voucher. But we have two different types here, the normal discount adjustment and the tax discount adjustment. The best idea I have so far is to create a VoucherTax < Voucher model. The normal adjustments can have the Voucher as originator and the tax adjustment can have the VoucherTax as originator but they actually point to the same row in the vouchers table. Rails just loads different classes and we can change the Adjustment.tax scope to include that type. Slightly hacky but in line with the current model and default Rails behaviour.

I'm also quite certain that we can't re-use any of the existing calculators because they are based on an order total or line items. But the voucher calculation has to calculate the tax adjustment at the same time and therefore we need a new calculator. Or, it may be easier to do the calculations in the Voucher and VoucherTax models. I don't know. I didn't look that far into the code.

On another note, in the calculation examples above I used terms like additional_tax_total which is a field on the order model. While that seems to make sense to me, we need to double-check that those fields contain what I assumed here and that they are updated. For example, most parts of the code query order.adjustments.tax.sum(:amount) instead of looking at the field on the order, I think. So I don't know if it's up-to-date. But hopefully you get the meaning of the values above and find the right code to get those numbers.

[rioug]

I added a dependency on the backend issue. I am not sure that's the correct way to go about it, but I wanted to flag that #10431 will need to be merged before we can resolve this.

[mkllnk]

I added a dependency on the backend issue. I am not sure that's the correct way to go about it, but I wanted to flag that #10431 will need to be merged before we can resolve this.

Good point. I usually add the dependency on a pull request which depends on another pull request. You can work on the issue, right? It's just that you have to rebase your work later once the first PR is merged. But that's just how I started using it and I've never discussed that with the team. What do you think?

[rioug]

Good point. I usually add the dependency on a pull request which depends on another pull request.

Ah yeah I recall seeing that somewhere, I think that makes more sense.

Yeah I can work on the issue, I just branched of the backend branch. Although I have never done it, I am pretty sure I can rebase the branch to branch off master once the backend branch is merged (that's a lot of branch in there 😆 )

[mkllnk]

I am pretty sure I can rebase the branch to branch off master once the backend branch is merged

Yes, if the commits in the backend branch stay the same, you can simply git rebase master and it will ignore duplicate commits. Done. If the commits change then it's better to git rebase master -i and remove the old commits from the history to avoid conflicts.

[rioug]

@lin-d-hop @kirstenalarsen
Currently the code assumes that an order has either tax included in price or tax excluded from the price. @mkllnk commented on the PR :

I think it's possible that an order has items including tax and items excluding tax and even items having both if they are subject to two tax categories. I think that we need to find all items for each type of tax calculation and then create adjustments for whichever kind is present. It means that we can't use the existing totals and have to add it up ourselves. Or am I overcomplicating it?

Do we need to take into account potential scenario when an order has both tax included and tax excluded from the price ?

[kirstenalarsen]

Hmmm . . I am not sure about this - I would hesitate to question @mkllnk but I think that tax-ex or tax-incl is set at the instance level and therefore there can only be one 'kind' within an order. I will ask @tschumilas and @lauriewayne because they are the tax gurus!!

[mkllnk]

I think that tax-ex or tax-incl is set at the instance level and therefore there can only be one 'kind' within an order.

No, it's set per tax rate. So you can have GST inclusive and an alcohol tax exclusive. I doubt that a government has done such a thing but I think it would be good to have consistently correct code. But we can leave it until someone encounters that problem or a tester gets confused.

[kirstenalarsen]

wow, is that a thing that changed at some point? Or has always been that way? How little I know :)

[kirstenalarsen]

I think if we are going to leave it we need to document it (in the code?) as well as elsewhere (user guide), that we are assuming this doesn't happen and not overcomplicating the code by allowing for it . . and I agree we should implement without covering this as it sounds very complicated

[tschumilas]

I just deleted an earlier post here - in case you saw it and were confused... Because I was certainly confused before my coffee this morning.

It IS indeed possible to set one tax rate as tax inclusive and another as tax exclusive, and hence have one product with tax inclusive pricing and another with tax exclusive pricing both inclusive and exclusive pricing in the same shop and hence on a given order. (I had no idea!)

That said - in Canada we would never have a situation where both tax inclusive and tax exclusive prices occur in a shop or on an order. We never use tax inclusive pricing at all. (Well maybe in obscur situations I don't know about - but certainly not in retail situations.) All our shops/orders will have prices exclusive of tax, and tax calculated at checkout on top of the price in the store. (We have to do this, because the tax rate that is fetched and applied to a product, is given by the shopper's address. We call this 'point of supply'. So maybe that simplifies the voucher-tax calulation from our point of view?

I can't speak for the US ..... @lauriewayne ?

[lauriewayne]

Same in the US @tschumilas - it's virtually unheard-of and even considered a little shady in some cases to not have tax as a separate line item. There are places that don't tax food (or don't have any sales tax at all) but in other places people expect to see the tax explicitly noted (usually including the rate).
(I thought your deleted post was helpful in thinking through it btw - thank you for "showing your work" there!)

[sigmundpetersen]

I guess you have found it but there's a chapter on this in the super admin guide.

Included in Price: If product prices should display to the enterprise manager and to the customer as the ‘tax inclusive price’, click this tick box. If customers and enterprise managers prefer to see the ‘tax exclusive price’ and then see the tax added on at checkout, deselect this option. Note: tax inclusivity/exclusivity is defined at the instance level. Users can’t choose individually whether their shops display tax inclusive or exclusive prices.

I don't know if it was more confusing or clarifying for me, but maybe it will help getting a better understanding for us all 🙂

[lauriewayne]

Adding that in the US, discounts, vouchers, and gift certificates are normally treated like cash for tax purposes - so in the example of the $100 purchase with a 10% voucher and a tax rate of 5%, the tax should be on the original $100 amount even if the customer price is $90 (sometimes they call that "tax applied to original price").

[mkllnk]

the tax should be on the original $100 amount even if the customer price is $90 (sometimes they call that "tax applied to original price").

😱 That's different to our scenario before. When we asked for feedback, you asked Kent and he said:

It makes sense to the best of my understanding of how it would be applied, and looks like it should work okay.

So, my question now is: do we need to apply tax to the original price in some cases or is that optional? It could just be that some people do their accounting differently and it all comes out in a wash at the end of the financial year. If it's required, but only sometimes, then we need to find a way to distinguish the cases or add an option for that. It's something that would come in a later iteration, I think. It's too much work to include in the minimal product.

[mkllnk]

My view on the impact of mixing inclusive and exclusive tax on our implementation: I don't think it's that hard to implement. We just need to calculate a couple of totals and calculate both on each order instead of if .. else. Do you agree, @rioug?

[rioug]

That sounds fine in theory, although I have a feeling it's not that easy. I'll try come up with an example later on today to see if that works as expected.

[kirstenalarsen]

I think we need to put it through as is and make amendments / complexities later considerations, possibly with funding contributed from other instances to deal with these cases. We need the basic voucher shipped and in use

…

On Fri, 14 Apr 2023 at 10:01, Maikel ***@***.***> wrote: the tax should be on the original $100 amount even if the customer price is $90 (sometimes they call that "tax applied to original price"). 😱 That's different to our scenario before. When we asked for feedback, you asked Kent and he said: It makes sense to the best of my understanding of how it would be applied, and looks like it should work okay. So, my question now is: do we need to apply tax to the original price in some cases or is that optional? It could just be that some people do their accounting differently and it all comes out in a wash at the end of the financial year. If it's required, but only sometimes, then we need to find a way to distinguish the cases or add an option for that. It's something that would come in a later iteration, I think. It's too much work to include in the minimal product. — Reply to this email directly, view it on GitHub <#10432 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAWGXSBOPCV4DPDX76OHR3TXBCHU3ANCNFSM6AAAAAAU26OSWQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[lauriewayne]

Yes let's get it done and the US can either be an edge case that we can muster the resources to take care of later or as long as we know the logic we can adapt. Sorry to elicit the scary face 🤗

[rioug]

Thanks everyone for the feedback. I added a comment in the code that for now we assume that it is either all tax included in the price or all taxes excluded from the price. We can tackle the complexity if/when the problem arises.

[rioug]

A few notes:

• I used the existing colours for vouchers (teal ones) which are slightly different from the ones in the design
• There is a warning if the voucher is going to cover more than the order amount. This is not always true, at the time when the warning is displayed not all fess have been applied, ie any payment related fees. So it is possible to have a scenario where the warning is displayed but the voucher won't cover all the order amount once all fees have been applied.
• as discussed above, for now we assume that taxes are either all included in the price, or all excluded from the price.