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

If alwaysSyncPlatformPricesOver = true, - product.special_price always 0 #2940

Closed
2 of 5 tasks
Devors opened this issue May 23, 2019 · 4 comments
Closed
2 of 5 tasks
Labels
3: Medium complexity bug Bug reports QA approved after merge Testers will add this label after positive check on merged changes
Milestone

Comments

@Devors
Copy link

Devors commented May 23, 2019

Current behavior

reset product.special_price in both cases

Expected behavior

do not reset if product.priceInclTax < product.originalPriceInclTax

Repository

https://github.com/DivanteLtd/vue-storefront/blob/v1.9.0/core/modules/catalog/helpers/index.ts#L158

Can you handle fixing this bug by yourself?

By removing else case?

  • YES
  • NO

Which Release Cycle state this refers to? Info for developer.

Pick one option.

  • This is a bug report for test version on https://test.storefrontcloud.io - In this case Developer should create branch from develop branch and create Pull Request 2. Feature / Improvement back to develop.
  • This is a bug report for current Release Candidate version on https://next.storefrontcloud.io - In this case Developer should create branch from release branch and create Pull Request 3. Stabilisation fix back to release.
  • This is a bug report for current Stable version on https://demo.storefrontcloud.io and should be placed in next stable version hotfix - In this case Developer should create branch from hotfix or master branch and create Pull Request 4. Hotfix back to hotfix.
@Devors Devors added the bug Bug reports label May 23, 2019
@pkarw
Copy link
Collaborator

pkarw commented May 24, 2019

I belive this is the price that cames from render-list Magneto2 endpoint. Any ideas how to solve it?

Debugging starting point: taxcalc.ts

@janmyszkier
Copy link
Contributor

janmyszkier commented May 24, 2019

🤔 the code that @Devors points to seems to be a mess. Let's break this

if (product.priceInclTax >= product.originalPriceInclTax) {
    product.specialPriceInclTax = 0
    product.special_price = 0
  } else {
    product.special_price = 0 // the same price as original; it's not a promotion
  }

into few points....

  1. the first bad thing is that the comment in else is misleading, says same price as original, but the IF was = so else clearly takes care of NOT SAME case

  2. Considering product.priceInclTax is in fact final_price (https://github.com/DivanteLtd/vue-storefront/blob/v1.9.0/core/modules/catalog/helpers/index.ts#L142)
    and originalPriceInclTax is in fact regular_price (https://github.com/DivanteLtd/vue-storefront/blob/v1.9.0/core/modules/catalog/helpers/index.ts#L143) I'd love to know how on earth we woul ever get a promotion that makes the final_price (which is AFTER applying promotions) higher than regular_price

  3. If we are only interested in camelcased values (so final_price equivalent) and it was decided to reset special_price in every case (in BOTH if and else) then why we are checking it in Product.vue
    ?

My guess:
Since VueSF does not support anything except coupons (which is supported only in cart endpoint anyway and is not reflected on product/category pages) we should only simplify this whole if/else into

if (product.priceInclTax < product.specialPriceInclTax) {
    product.specialPriceInclTax = 0
    product.special_price = 0
  } 

because it's the only case when the price was SOMEHOW lowered by some rule, but lowering the price is not caused by the special_price setting by magento. THIS should not show the product was discounted. this way Product.vue would still be correct when looking at special_price.

@pkarw
Copy link
Collaborator

pkarw commented May 24, 2019

@janmyszkier, @Devors this condition seems like a logic error. We probably should simplify to:

if (product.priceInclTax >= product.originalPriceInclTax) {
    product.specialPriceInclTax = 0
    product.special_price = 0
  }

priceInclTax = final_price

This condition means like: if the final price isn't lower than the original price - there is no special_price

Please test it and if it fixes the issue please provide us with a PR

@pkarw pkarw added this to the 1.11.0-rc.1 milestone Jun 10, 2019
pkarw added a commit that referenced this issue Jun 11, 2019
I've simplified the condition - currently there was an logick-error in which when the final_price < regular_price the special_price got zeroed. Which makes totally no sense :)
@pkarw pkarw mentioned this issue Jun 11, 2019
5 tasks
@pkarw
Copy link
Collaborator

pkarw commented Jun 11, 2019

Fixed in the #3043

@pkarw pkarw closed this as completed Jun 11, 2019
@patzick patzick added the QA - Ready for tests This is notification for testers, that improvement is ready to be tested and verified. label Jun 12, 2019
@pkarw pkarw modified the milestones: 1.11.0-rc.1, 1.10.0-rc.1 Jun 19, 2019
@alinadivante alinadivante added QA approved after merge Testers will add this label after positive check on merged changes and removed QA - Ready for tests This is notification for testers, that improvement is ready to be tested and verified. labels Jun 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3: Medium complexity bug Bug reports QA approved after merge Testers will add this label after positive check on merged changes
Projects
None yet
Development

No branches or pull requests

5 participants