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

Fixed display of tax rate (forms & details) + update of product tax rate #4780

Conversation

NyanKiyoshi
Copy link
Member

Closes #4635

  • No longer display the tax rate field if there is no tax support
  • User is now able to change a product's tax rate
  • User is now able to see the tax rate of in the product details page

Pull Request Checklist

  1. Privileged views and APIs are guarded by proper permission checks.
  2. All visible strings are translated with proper context.
  3. All data-formatting is locale-aware (dates, numbers, and so on).
  4. Database queries are optimized and the number of queries is constant.
  5. Database migration files are up to date.
  6. The changes are tested.
  7. GraphQL schema and type definitions are up to date.
  8. Changes are mentioned in the changelog.

Copy link

django-queries commented Sep 28, 2019

Here is the report for 7ce128e (NyanKiyoshi/saleor @ fix/dashboard-1.0/products/tax-update-and-details)
Base comparison is 65c819f.

No differences were found. (click me)

# api.benchmark checkout
  test name                                  	left count 	right count	duplicate count
  -------------------------------------------	-----------	-----------	---------------
  add billing address to checkout            	         34	         34	             20
  add shipping to checkout                   	          7	          7	              0
  checkout payment charge                    	         14	         14	              0
  complete checkout                          	          6	          6	              0
  create checkout                            	         48	         48	             24

# api.benchmark homepage
  test name                                  	left count 	right count	duplicate count
  -------------------------------------------	-----------	-----------	---------------
  retrieve main menu                         	          5	          5	              0
  retrieve product list                      	          4	          4	              0
  retrieve secondary menu                    	          5	          5	              0
  retrieve shop                              	          2	          2	              0

# api.benchmark product
  test name                                  	left count 	right count	duplicate count
  -------------------------------------------	-----------	-----------	---------------
  product details                            	         15	         15	              3
  retrieve product attributes                	         13	         13	              2

# api.benchmark variant
  test name                                  	left count 	right count	duplicate count
  -------------------------------------------	-----------	-----------	---------------
  retrieve variant list                      	         18	         18	              8

# api product sorting attributes
  test name                                  	left count 	right count	duplicate count
  -------------------------------------------	-----------	-----------	---------------
  sort product not having attribute data     	         21	         21	              0

saleor/dashboard/product/forms.py Outdated Show resolved Hide resolved
saleor/dashboard/product/forms.py Outdated Show resolved Hide resolved
from ...product.models import Product


def get_product_tax_rate(product: Product, manager: ExtensionsManager = None) -> str:
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we use the same logic in the API as well? If this was the case, I think we could have this logic in the manager's get_tax_code_from_object_meta method. @korycins what do you think?

Copy link
Member Author

Choose a reason for hiding this comment

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

In the API we only retrieve the value once (resolver). I am not sure if there would be any other use case. That's why I put it into the dashboard utilities for now, so it can be removed safely with the dashboard.

Copy link
Member

Choose a reason for hiding this comment

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

In the dashboard, we're getting the tax rate from product and if it's unknown, then we're getting it from the product type. Don't we have to use the same logic over there as well? If not then fine, but I thought it should be reused.

templates/dashboard/product/form.html Outdated Show resolved Hide resolved
tests/dashboard/test_product.py Outdated Show resolved Hide resolved
NyanKiyoshi and others added 2 commits October 2, 2019 16:23
- No longer display the tax rate field if there is no tax support
- User is now able to change a product's tax rate
- User is now able to see the tax rate of in the product details page
@NyanKiyoshi NyanKiyoshi force-pushed the fix/dashboard-1.0/products/tax-update-and-details branch from 4f18e91 to 7ce128e Compare October 2, 2019 14:23
@codecov
Copy link

codecov bot commented Oct 2, 2019

Codecov Report

Merging #4780 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4780      +/-   ##
==========================================
+ Coverage   91.24%   91.24%   +<.01%     
==========================================
  Files         343      344       +1     
  Lines       20602    20616      +14     
  Branches     1954     1956       +2     
==========================================
+ Hits        18798    18812      +14     
  Misses       1266     1266              
  Partials      538      538
Impacted Files Coverage Δ
saleor/dashboard/product/forms.py 92.69% <100%> (+0.14%) ⬆️
saleor/dashboard/product/views.py 93.43% <100%> (+0.01%) ⬆️
saleor/dashboard/product/utils.py 100% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 65c819f...7ce128e. Read the comment docs.

@maarcingebala maarcingebala merged commit 6cf7843 into saleor:master Oct 2, 2019
@NyanKiyoshi NyanKiyoshi deleted the fix/dashboard-1.0/products/tax-update-and-details branch October 2, 2019 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Product tax rates are broken in dashboard 1.0
4 participants