Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP][RFC][Cart] Cart taxes scenarios #3941

Closed
wants to merge 5 commits into from

Conversation

Zales0123
Copy link
Member

Q A
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Fixed tickets
License MIT
Doc PR

According to awesome work of @Arminek with behat scenarios here and upcoming changes with taxes here, there are some new scenarios for dealing with taxes in cart.
Of course, they could be green after merging whole taxes-related stuff. They are not implemented yet, I'm just wondering about community feedback about theirs style, have I properly caught concept of new scenarios ;)

@pjedrzejewski pjedrzejewski added Enhancement Minor issues and PRs improving the current solutions (optimizations, typo fixes, etc.). RFC Discussions about potential changes or new features. labels Jan 21, 2016
@Zales0123 Zales0123 changed the title [WIP][Cart] Cart taxes scenarios [WIP][RFC][Cart] Cart taxes scenarios Jan 21, 2016
Background:
Given that store is operating on the France channel
And default currency is "EUR"
And tax rate "EU VAT" with 23% rate belongs to "Taxable Goods" category for "EU" zone
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about: And store has "EU VAT" tax rate of 23% for "Taxable Goods" in "EU" zone
I'd use a different example here, Taxable Goods is too general, let's say it is food category or something like WDYT?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 I was wondering about sth conciser, but didn't know how short can it be 😄 And 👍 for more specific category

@pjedrzejewski
Copy link
Member

We should be more explicit about the zone, I'd be fine with steps like:

And there is "EU" zone containing all members of European Union

I know that first step is assuming we are operating a single country store, but in these scenarios we should be more explicit so something like:

Given that store is operating via single channel of sales
And it is shipping to France, Spain and Germany
And there is "EU" zone containing all members of European Union

I think our scenario will evolve and I am just throwing random ideas here, we will improve them step by step, but we need to get rid of this old, anti-BDD style. ;)

And catalog has a product "PHP T-Shirt" priced at €100.00
And "PHP T-Shirt" tax category is "Clothes"
And catalog has a product "Symfony Mug" priced at €50.00
And "Symfony T-Shirt" tax category is "Mugs"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should be "Symfony Mug"

@peteward
Copy link

Great to see these 👍

I suggest you add some scenarios that look at rounding and the calculation of tax at item / unit level as discussed by myself and @michalmarcinkowski in #3868.

Even if the default stance of Sylius doesn't agree with me, I think having scenarios that cover this are really important, and can be extended when multiple calculation strategies are implemented

@@ -0,0 +1,30 @@
@ui-cart
Feature: Cart with multiple tax rates
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feature: Apply correct tax for items with different tax rates

@pamil
Copy link
Contributor

pamil commented Jan 22, 2016

From the implementation point of view, implementing step like this may be tough:

And "DHL" tax category is "Clothes" # DHL = Shipping method
And "Banana" tax category is "Fruits" # Banana = Product

And default currency is "EUR"
And there is user "[email protected]" identified by "password123"
And catalog has a product "PHP T-Shirt" priced at €100.00
And store has "DHL" shipping method with "€10.00" fee
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should decide whether we write prices as €0.50 or "€3.00".

Scenario: Proper taxes before addressing
Given I added product "PHP T-Shirt" to the cart
Then my cart taxes should be "€23.00"
And my cart total should be "€123.00"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO it's more natural to swap these two:

Then my cart total should be ...
And my cart taxes should be

WDYT?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remember to follow:

GIVEN -> WHEN -> THEN

This should be:

When I add product "PHP T-Shirt" to the cart
Then my cart taxes should be "€23.00"
And my cart total should be "€123.00"

Anyway, we are clearly missing some context here, because we don't know anything about the user or the zone he falls into. We probably need to say: And default tax zone is "EU" in the background.

@pjedrzejewski
Copy link
Member

I left bunch of comments but I stopped to avoid creating a mess in the diff comments section, so please apply these and other suggestion and we will iterate again. Not an easy task but I think we are very close! 👍

@Zales0123
Copy link
Member Author

@pjedrzejewski @michalmarcinkowski @peteward @pamil Ok 😄
I've implemented part of your comments, another one will be soon. Of course I could've missed something, co feel free to point them again.
Some thoughts:

  1. I fixed this comment by specifying tax category target (product "PHP T-Shirt" belongs to.../shipping method "DHL" belongs to..), I think it's clear enough
  2. I think we should stay with quotation marks for prices (€10.00 -> "€10.00") ;)
  3. 👍 for avoiding "catalog" word, I just cannot decide which one is better: And there is a product "PHP T-Shirt" priced at... or And store has a product "PHP T-Shirt" priced at..., WDYT?

@michalmarcinkowski
Copy link
Contributor

@peteward we definitely are going to have some scenarios for rounding the tax at item / unit level.

@Zales0123 as for 3. IMO And store has a product...

@Zales0123 Zales0123 force-pushed the cart-taxes-scenarios branch from d8180c2 to ac7f485 Compare January 22, 2016 11:54
And I am logged in customer

Scenario: Displaying correct tax before specifying shipping address
When I added product "PHP T-Shirt" to the cart
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added suggests it is a pre condition, I think it should be action (When) -> add product

And product "PHP T-Shirt" belongs to "Clothes" tax category
And store has a product "Symfony Mug" priced at "€50.00"
And product "Symfony Mug" belongs to "Mugs" tax category
And store ships everything for free
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Am I right, that shipment cost is calculated based on zone, not on item? Shouldn't we change this step to And store ships everywhere for free?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually we ship only to the countries/zones we specified not everywhere in the world, so everything is probably more accurate than everywhere, WDYT?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still found everywhere much more accurate. Firstly, as I wrote before, because shipping cost is related to zone, not to item. Secondly, because in this scenario we are delivering items everywhere in the world, actually. But to me more precise, WDYT about And all store shipments are free of charge?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Actually the shipping cost can be per item, there are e.g. per item and volume based calculators :)
  2. The step is implemented in a way that right now it takes the 'current' zone, so the shipping method is available in one zone, not everywhere.

The most appropriate step will be:
And the store ships everything for free everywhere - this step could get all zones and create free shipping method for every available zone.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And the store ships everything for free to every possible location

Still not perfect, because it is not really everywhere, it is only to zones defined, which can be 2 countries. :D

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pjedrzejewski looks like the most self describing 👍

@michalmarcinkowski
Copy link
Contributor

@Zales0123 I suppose this can be closed?

@Zales0123
Copy link
Member Author

@michalmarcinkowski I think yes, as we're heading to the end of this scenarios implementation ;)

@Zales0123 Zales0123 closed this Feb 10, 2016
@Zales0123 Zales0123 deleted the cart-taxes-scenarios branch October 6, 2016 20:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Minor issues and PRs improving the current solutions (optimizations, typo fixes, etc.). RFC Discussions about potential changes or new features.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants