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

issue/6820 - Adjustments → Discounts & Tax Rates table #6822

Merged
merged 5 commits into from Aug 14, 2018

Conversation

Projects
None yet
2 participants
@sunnyratilal
Member

sunnyratilal commented Aug 1, 2018

See #6820.

Proposed Changes:

  1. Change adjustments to discounts migrating all discounts.
  2. Introduce a tax rates table.
  3. Migrate tax rates to edd_tax_rates table.
  4. Introduce EDD\Tax_Rates\Tax_Rate object along with helper functions.

@sunnyratilal sunnyratilal requested a review from cklosowski Aug 1, 2018

@sunnyratilal sunnyratilal changed the title from issue/6820: Adjustments → Discounts & Tax Rates table to issue/6820 - Adjustments → Discounts & Tax Rates table Aug 1, 2018

@sunnyratilal sunnyratilal requested a review from JJJ Aug 1, 2018

@cklosowski

This comment has been minimized.

Show comment
Hide comment
@cklosowski

cklosowski Aug 7, 2018

Member

@sunnyratilal The default sort seems to be 'DESC', which is a weird experience when adding tax rates. As I add a tax rate to the bottom of the 'Rates' list in the UI, when I save, the order is reverse from when I add them. We should try and keep the order the same as the UI, which means it would default to an 'ASC' tax order, which makes sense from the perspective of applying tax rates anyway, You want to target more specific first, then get less specific.

Member

cklosowski commented Aug 7, 2018

@sunnyratilal The default sort seems to be 'DESC', which is a weird experience when adding tax rates. As I add a tax rate to the bottom of the 'Rates' list in the UI, when I save, the order is reverse from when I add them. We should try and keep the order the same as the UI, which means it would default to an 'ASC' tax order, which makes sense from the perspective of applying tax rates anyway, You want to target more specific first, then get less specific.

@sunnyratilal sunnyratilal merged commit d0a9f08 into release/3.0 Aug 14, 2018

1 check was pending

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details

@sunnyratilal sunnyratilal deleted the isuse/6820 branch Aug 14, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment