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

Drop JSONB mapping of attributes for M2M relations #4663

Merged
merged 7 commits into from
Sep 10, 2019

Conversation

NyanKiyoshi
Copy link
Member

@NyanKiyoshi NyanKiyoshi commented Aug 20, 2019

Closes #4643, closes #4674, fixes #4685.

  • The migrations are done
  • The migrations are automatically tested against all cases
  • Dashboard 1.0 support
  • GraphQL: mutations
  • GraphQL filtering
  • GraphQL tests are updated adequately
  • GraphQL test cases checklist
    • Existing attribute value doesn't create a new one
    • Attributes flagged as required returns a validation error if no value supplied
    • Passing a non existing attribute ID and slug
    • When no slug or ID was provided in input, raises a validation error (deprecated)
    • The deprecated field SelectAttribute.value is tested
    • Blank value triggers a validation error
    • All variant attributes are required
  • The old attribute logic in SEO is updated
  • Storefront 1.0 support
  • unit tested?
  • Storefront 1.0 filtering support
  • unit tested?
  • Populatedb is updated to assign attributes and values to the products/variants

Screenshots

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.

@NyanKiyoshi NyanKiyoshi added core attributes Issues related to improving the attributes/product types behavior labels Aug 20, 2019
@NyanKiyoshi NyanKiyoshi self-assigned this Aug 20, 2019
@NyanKiyoshi NyanKiyoshi force-pushed the product/attributes/mapping branch 2 times, most recently from b76d72f to 793c941 Compare August 20, 2019 11:41
@NyanKiyoshi NyanKiyoshi force-pushed the product/attributes/mapping branch 7 times, most recently from cbfe8e9 to 90e5a65 Compare August 27, 2019 12:37
@NyanKiyoshi NyanKiyoshi marked this pull request as ready for review August 27, 2019 14:05
Copy link

django-queries commented Aug 27, 2019

Here is the report for 0e15b38 (NyanKiyoshi/saleor @ product/attributes/mapping)
Base comparison is c7c1416.

**Found 2 differences!** (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                     	         13	         15	              3

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

@codecov

This comment has been minimized.

saleor/product/utils/attributes.py Show resolved Hide resolved
saleor/graphql/core/utils/__init__.py Outdated Show resolved Hide resolved
saleor/graphql/product/mutations/products.py Outdated Show resolved Hide resolved
saleor/graphql/product/mutations/products.py Outdated Show resolved Hide resolved
saleor/graphql/core/mutations.py Outdated Show resolved Hide resolved
saleor/graphql/core/mutations.py Outdated Show resolved Hide resolved
saleor/graphql/core/mutations.py Outdated Show resolved Hide resolved
saleor/dashboard/product/forms.py Outdated Show resolved Hide resolved
saleor/graphql/product/mutations/products.py Show resolved Hide resolved
saleor/graphql/product/mutations/products.py Show resolved Hide resolved
saleor/graphql/product/types/attributes.py Outdated Show resolved Hide resolved
@codecov-io

This comment has been minimized.

@codecov-io
Copy link

codecov-io commented Sep 10, 2019

Codecov Report

Merging #4663 into master will decrease coverage by 0.04%.
The diff coverage is 93.61%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4663      +/-   ##
==========================================
- Coverage   91.72%   91.67%   -0.05%     
==========================================
  Files         313      313              
  Lines       18744    18852     +108     
  Branches     1849     1851       +2     
==========================================
+ Hits        17192    17283      +91     
- Misses       1040     1049       +9     
- Partials      512      520       +8
Impacted Files Coverage Δ
saleor/graphql/product/mutations/attributes.py 96.93% <ø> (ø) ⬆️
saleor/graphql/payment/mutations.py 96% <ø> (ø) ⬆️
saleor/product/utils/__init__.py 95.29% <ø> (ø) ⬆️
saleor/dashboard/product/forms.py 92.55% <100%> (+0.54%) ⬆️
saleor/product/utils/variants_picker.py 95.08% <100%> (+0.16%) ⬆️
saleor/graphql/product/types/attributes.py 98.13% <100%> (+0.07%) ⬆️
saleor/page/models.py 85.71% <100%> (ø) ⬆️
saleor/core/db/fields.py 100% <100%> (ø)
saleor/dashboard/product/views.py 93.4% <100%> (ø) ⬆️
saleor/product/tasks.py 100% <100%> (ø) ⬆️
... and 13 more

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 c7c1416...0e15b38. Read the comment docs.

@maarcingebala maarcingebala merged commit fef6eef into saleor:master Sep 10, 2019
@NyanKiyoshi NyanKiyoshi deleted the product/attributes/mapping branch September 10, 2019 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
attributes Issues related to improving the attributes/product types behavior
Projects
None yet
5 participants