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.

Sign up for GitHub

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

Jump to bottom

description_plaintext blocks are improperly concatenated #7600

Merged
merged 5 commits into from Jul 9, 2021
Merged

description_plaintext blocks are improperly concatenated #7600

merged 5 commits into from Jul 9, 2021

Conversation

daltonsena
Copy link
Contributor

Signed-off-by: Dalton daltonsena@users.noreply.github.com

fix issue #7599

Thanks guys for the wonderful work!!! Saleor is a great project and I learn a lot from him.

Impact

  • New migrations
  • New/Updated API fields or mutations
  • Deprecated API fields or mutations
  • Removed API types, fields, or mutations
  • Documentation needs to be updated

Pull Request Checklist

  • Privileged queries and mutations are guarded by proper permission checks
  • Database queries are optimized and the number of queries is constant
  • Database migration files are up to date
  • The changes are tested
  • GraphQL schema and type definitions are up to date
  • Changes are mentioned in the changelog

@daltonsena
fix: fixes issue #7599
da31ad6 
Signed-off-by: Dalton <daltonsena@users.noreply.github.com>
@CLAassistant
Copy link

CLAassistant commented Jul 8, 2021 •
edited

CLA assistant check
All committers have signed the CLA.

@maarcingebala
Copy link
Member

@daltonsena Good catch! I checked the example from the issue locally and can confirm the bug. Would you mind fixing the test test_clean_editor_js_for_list that is now failing after your changes?

patrys
patrys reviewed Jul 8, 2021
View reviewed changes
Copy link
Member

@patrys patrys left a comment

Choose a reason for hiding this comment

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

Wouldn't it be more efficient to add the results to a list and then " ".join(...) it?

@daltonsena
- changing clean_editor_js to add the results to a list
b73595d 
- fixing test_clean_editor_js_for_list

Signed-off-by: Dalton <daltonsena@users.noreply.github.com>
@daltonsena
Copy link
Contributor Author

@daltonsena Good catch! I checked the example from the issue locally and can confirm the bug. Would you mind fixing the test test_clean_editor_js_for_list that is now failing after your changes?

@maarcingebala I had forgotten about the tests, sorry. I took the chance and changed the behaviour to a list/join.

@db-queries
Copy link

db-queries bot commented Jul 8, 2021 •
edited

Here is the report for 7092db2 (daltonsena:bugfix/clean-editor-js-unwanted-concatenation)
Base comparison is 75c3418.

No differences were found. 

# saleor.graphql.accountbenchmark account
  test name                                                  	left count 	right count	duplicate count
  -----------------------------------------------------------	-----------	-----------	---------------
  customers query                                            	         48	         48	              0
  delete staff members                                       	         32	         32	              0
  query staff user                                           	         18	         18	              3
  staff create                                               	         23	         23	              3
  staff update groups and permissions                        	         32	         32	              4

# saleor.graphql.accountbenchmark permission group
  test name                                                  	left count 	right count	duplicate count
  -----------------------------------------------------------	-----------	-----------	---------------
  permission group create                                    	         19	         19	              2
  permission group delete                                    	         20	         20	              3
  permission group query                                     	          7	          7	              0
  permission group update                                    	         33	         33	              1
  permission group update remove users with manage staff     	         27	         27	              3

# saleor.graphql.attributebenchmark attribute
  test name                                                  	left count 	right count	duplicate count
  -----------------------------------------------------------	-----------	-----------	---------------
  attribute translation                                      	          5	          5	              0
  attribute value translation                                	         26	         26	              0
  query attribute                                            	          6	          6	              0
  query attributes                                           	          8	          8	              0

# saleor.graphql.checkoutbenchmark checkout mutations
  test name                                                  	left count 	right count	duplicate count
  -----------------------------------------------------------	-----------	-----------	---------------
  add billing address to checkout                            	         51	         51	              5
  add checkout lines                                         	         49	         49	              8
  add shipping to checkout                                   	         64	         64	              9
  checkout email update                                      	         25	         25	              0
  checkout payment charge                                    	         42	         42	             16
  checkout shipping address update                           	         69	         69	             13
  checkout voucher code                                      	         70	         70	             12
  complete checkout                                          	        109	        109	             13
  complete checkout with single line                         	        111	        111	             13
  create checkout                                            	         73	         73	             12
  customer complete checkout                                 	        155	        155	             42
  update checkout lines                                      	         51	         51	              8

# saleor.graphql.checkoutbenchmark homepage
  test name                                                  	left count 	right count	duplicate count
  -----------------------------------------------------------	-----------	-----------	---------------
  user checkout details                                      	         41	         41	              2

# saleor.graphql.discountbenchmark sales
  test name                                                  	left count 	right count	duplicate count
  -----------------------------------------------------------	-----------	-----------	---------------
  sales query with channel slug                              	         16	         16	              0
  sales query withot channel slug                            	         15	         15	              0

# saleor.graphql.discountbenchmark vouchers
  test name                                                  	left count 	right count	duplicate count
  -----------------------------------------------------------	-----------	-----------	---------------
  vouchers query with channel slug                           	         16	         16	              0
  vouchers query withot channel slug                         	         15	         15	              0

# saleor.graphql.orderbenchmark fulfillment refund and return products
  test name                                                  	left count 	right count	duplicate count
  -----------------------------------------------------------	-----------	-----------	---------------
  fulfillment refund products order lines                    	         53	         53	              2
  fulfillment return products order lines                    	        101	        101	             14

# saleor.graphql.orderbenchmark order
  test name                                                  	left count 	right count	duplicate count
  -----------------------------------------------------------	-----------	-----------	---------------
  staff multiple draft orders                                	        402	        402	            308
  staff multiple orders                                      	        152	        152	             76
  staff order details                                        	         46	         46	              7
  user order details                                         	         42	         42	              7

# saleor.graphql.pagebenchmark page type
  test name                                                  	left count 	right count	duplicate count
  -----------------------------------------------------------	-----------	-----------	---------------
  query page type                                            	         19	         19	              7
  query page types                                           	         25	         25	             12

# saleor.graphql.productbenchmark category
  test name                                                  	left count 	right count	duplicate count
  -----------------------------------------------------------	-----------	-----------	---------------
  categories children                                        	          3	          3	              0
  category delete                                            	        108	        108	             27
  category view                                              	         27	         27	              1

# saleor.graphql.productbenchmark collection
  test name                                                  	left count 	right count	duplicate count
  -----------------------------------------------------------	-----------	-----------	---------------
  collection add products                                    	         44	         44	             10
  collection bulk delete                                     	         51	         51	             10
  collection view                                            	          6	          6	              0
  create collection                                          	         47	         47	             12
  delete collection                                          	         46	         46	             11
  remove products from collection                            	         41	         41	             10
  retrieve collection channel listings                       	          4	          4	              0

# saleor.graphql.productbenchmark homepage
  test name                                                  	left count 	right count	duplicate count
  -----------------------------------------------------------	-----------	-----------	---------------
  report product sales                                       	         10	         10	              3
  retrieve product list                                      	          4	          4	              0

# saleor.graphql.productbenchmark product
  test name                                                  	left count 	right count	duplicate count
  -----------------------------------------------------------	-----------	-----------	---------------
  filter products by attributes                              	          9	          9	              0
  filter products by boolean attributes                      	         19	         19	              0
  filter products by numeric attributes                      	         18	         18	              0
  product create                                             	         70	         70	              3
  product details                                            	         31	         31	              0
  product translations                                       	          5	          5	              0
  retrieve channel listings                                  	         19	         19	              0
  retrieve product attributes                                	          9	          9	              0
  retrieve product images                                    	          4	          4	              0
  retrieve product media                                     	          4	          4	              0
  retrive products with product types and attributes         	          7	          7	              0
  update product                                             	         46	         46	              4

# saleor.graphql.productbenchmark product variant channel listing update
  test name                                                  	left count 	right count	duplicate count
  -----------------------------------------------------------	-----------	-----------	---------------
  variant channel listing update                             	         50	         50	              5

# saleor.graphql.productbenchmark variant
  test name                                                  	left count 	right count	duplicate count
  -----------------------------------------------------------	-----------	-----------	---------------
  product variant bulk create                                	         63	         63	              2
  product variant create                                     	         75	         75	              6
  retrieve variant list                                      	         25	         25	              0
  update product variant                                     	         79	         79	             12

# saleor.graphql.productbenchmark variant stocks
  test name                                                  	left count 	right count	duplicate count
  -----------------------------------------------------------	-----------	-----------	---------------
  product variants stocks create                             	         22	         22	              5
  product variants stocks delete                             	         19	         19	              5
  product variants stocks update                             	         27	         27	              5
  query product variants stocks                              	         14	         14	              4

# saleor.graphql.producttest product sorting attributes
  test name                                                  	left count 	right count	duplicate count
  -----------------------------------------------------------	-----------	-----------	---------------
  sort product not having attribute data                     	         23	         23	              0

# saleor.graphql.shippingbenchmark shipping methods
  test name                                                  	left count 	right count	duplicate count
  -----------------------------------------------------------	-----------	-----------	---------------
  vouchers query with channel slug                           	         10	         10	              0
  vouchers query without channel slug                        	          9	          9	              0

# saleor.graphql.shopbenchmark homepage
  test name                                                  	left count 	right count	duplicate count
  -----------------------------------------------------------	-----------	-----------	---------------
  retrieve shop                                              	          6	          6	              0

@codecov
Copy link

codecov bot commented Jul 8, 2021 •
edited

Codecov Report

Merging #7600 (7092db2) into master (75c3418) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #7600   +/-   ##
=======================================
  Coverage   93.33%   93.33%           
=======================================
  Files         462      462           
  Lines       34925    34941   +16     
  Branches     3602     3604    +2     
=======================================
+ Hits        32596    32611   +15     
  Misses       1614     1614           
- Partials      715      716    +1
Impacted Files Coverage Δ
saleor/core/utils/editorjs.py 92.45% <100.00%> (ø)
saleor/graphql/order/mutations/discount_order.py 97.39% <0.00%> (-0.08%) ⬇️
saleor/graphql/app/mutations.py 96.81% <0.00%> (-0.02%) ⬇️
saleor/graphql/tests/fixtures.py 98.41% <0.00%> (-0.02%) ⬇️
saleor/app/installation_utils.py 100.00% <0.00%> (ø)
saleor/tests/fixtures.py 96.84% <0.00%> (+<0.01%) ⬆️
saleor/graphql/middleware.py 61.97% <0.00%> (+1.36%) ⬆️
saleor/app/models.py 90.80% <0.00%> (+2.39%) ⬆️

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 75c3418...7092db2. Read the comment docs.

@daltonsena
removing commented unused code
5fd93af 
Signed-off-by: Dalton <daltonsena@users.noreply.github.com>
@daltonsena
Copy link
Contributor Author

Wouldn't it be more efficient to add the results to a list and then " ".join(...) it?

@patrys I agree!

@daltonsena
chore: renaming variable "string" to "plain_text_list" for better
5f79988 
understanding
Signed-off-by: Dalton <daltonsena@users.noreply.github.com>
@daltonsena
add new test_clean_editor_js_for_complex_description for editorjs
7092db2 
Signed-off-by: Dalton <daltonsena@users.noreply.github.com>
@daltonsena
Copy link
Contributor Author

@patrys Hi! I made three new modifications in this PR.

  1. Added a new case test for editorjs named "test_clean_editor_js_for_complex_description" that will check a description with two paragraphs and two list elements
  2. Changed the variable which was called "string" to "plain_text_list"
  3. Now we use a list (plain_text_list var) to add the results and then " ".join(...) it as a return of the method (as you suggested)

@daltonsena daltonsena requested a review from patrys July 9, 2021 01:45
@maarcingebala maarcingebala requested a review from a team July 9, 2021 07:27
@IKarbowiak IKarbowiak requested a review from a team July 9, 2021 07:46
@maarcingebala maarcingebala merged commit 804ebc5 into saleor:master Jul 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@patrys patrys patrys approved these changes

@maarcingebala maarcingebala maarcingebala approved these changes

@tomaszszymanski129 tomaszszymanski129 tomaszszymanski129 approved these changes

@IKarbowiak IKarbowiak IKarbowiak approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@daltonsena @CLAassistant @maarcingebala @patrys @tomaszszymanski129 @IKarbowiak