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

#14302 : Duplicate Product in BO with several taxes (multishop) #14308

Merged
merged 7 commits into from Jul 9, 2019

Conversation

@202-ecommerce
Copy link
Contributor

commented Jun 20, 2019

Questions Answers
Branch? develop
Description? A product duplicated from all shop context set the default shop's tax for all shops. It should set the same tax than the original product (the tax corresponding to the shop).
Type? bug fix
Category? BO
BC breaks? no
Deprecations? no
Fixed ticket? Fixes #14302
How to test? Steps to reproduce the behavior:
  1. Set a multishop with 2 shops (or more) and several taxe rule. For instance:
    • shop FR with Tax rate FR 20%
    • shop CH with Tax rate CH 10%
  2. Create a product in context all shop
  3. On each shop set a specific tax rate
  4. Finally on products board, select the previous product and click on duplicate product on all shop context.

This change is Reviewable

202-ecommerce added some commits Jun 20, 2019

@202-ecommerce 202-ecommerce requested a review from PrestaShop/prestashop-core-developers as a code owner Jun 20, 2019

classes/Product.php Outdated Show resolved Hide resolved

202-ecommerce added some commits Jun 25, 2019

Update Product.php
Remove spaces for PrettyCI
Update Product.php for PrettyCI
Remove spaces again for PrettyCI
@202-ecommerce

This comment has been minimized.

Copy link
Contributor Author

commented Jun 26, 2019

@PierreRambaud

We finally chose the third way (deleting the duplicated product if the product duplication process returns false).

What do you think about this solution ?

@matks

This comment has been minimized.

Copy link
Contributor

commented Jul 3, 2019

@PierreRambaud

We finally chose the third way (deleting the duplicated product if the product duplication process returns false).

What do you think about this solution ?

So if Db::getInstance()->update(///) returns false, this will delete the product and "cancel" the duplication (note: I hope it works well and the cancel impact all shops 😛 )

What happens if the tax rate was already the good one ? Wouldn't it cancel the duplication process ?

@PierreRambaud

This comment has been minimized.

Copy link
Contributor

commented Jul 3, 2019

@matks As I understand, if the tax rate was already the good one, nothing happened, the Db::getInstance()->update will only returns false in case of failure, so it continues.

I think it's a good idea to delete the product if something's going wrong, but (I'm sorry), what happened if $product->add() failed and returns false, you'll be able to use $product->delete()?

@202-ecommerce

This comment has been minimized.

Copy link
Contributor Author

commented Jul 4, 2019

@matks It won't change anything for the update and cancel the product for all shops.
The duplicated product will be deleted after its creation only if it exists since our last commit.

Thanks to @PierreRambaud for the approval.

@sarahdib sarahdib added QA ✔️ and removed waiting for QA labels Jul 8, 2019

@sarahdib sarahdib added this to the 1.7.7.0 milestone Jul 8, 2019

@matks

This comment has been minimized.

Copy link
Contributor

commented Jul 9, 2019

Thank you @202-ecommerce, another wonderful PR 😄 about multishop

@matks matks merged commit 6993b77 into PrestaShop:develop Jul 9, 2019

2 checks passed

PrettyCI Code formatting
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.