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

Handle tax rule for multishop #36387

Merged
merged 3 commits into from
Jun 28, 2024
Merged

Handle tax rule for multishop #36387

merged 3 commits into from
Jun 28, 2024

Conversation

tleon
Copy link
Contributor

@tleon tleon commented Jun 19, 2024 •
edited
Loading

Questions Answers
Branch? develop
Description? Handle ShopConstraint and get tax rule out of carrier command and tests
Type? improvement
Category? BO
BC breaks? yes
Deprecations? yes
How to test? CI ok
UI Tests Please run UI tests and paste here the link to the run. Read this page to know why and how to use this tool.
Fixed issue or discussion? Fixes #35926
Sponsor company PrestaShop SA

@tleon tleon self-assigned this Jun 19, 2024
@tleon tleon force-pushed the Issue-35926-add-update-multistore branch 2 times, most recently from 1a2e6ee to 1140e3b Compare June 20, 2024 14:53
@tleon tleon changed the title chore(bo): handle tax rule for multishop Handle tax rule for multishop Jun 20, 2024
@tleon tleon marked this pull request as ready for review June 20, 2024 14:57
@tleon tleon requested a review from a team as a code owner June 20, 2024 14:57
@tleon tleon force-pushed the Issue-35926-add-update-multistore branch 2 times, most recently from 2fac88e to 3c661a6 Compare June 21, 2024 08:05
boherm
boherm requested changes Jun 21, 2024
View reviewed changes
Copy link
Member

@boherm boherm left a comment

Choose a reason for hiding this comment

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

It's look promising! Just some few things 😉
We can discuss about it if you need.

src/Adapter/Carrier/CommandHandler/AddCarrierHandler.php Outdated Show resolved Hide resolved
src/Adapter/Carrier/CommandHandler/SetCarrierTaxRuleGroupHandler.php Outdated Show resolved Hide resolved
src/Core/Domain/Carrier/Command/SetCarrierTaxRuleGroupCommand.php Outdated Show resolved Hide resolved
.../Integration/Behaviour/Features/Context/Domain/Carrier/CarrierTaxRuleGroupFeatureContext.php Outdated Show resolved Hide resolved
src/Adapter/Carrier/CommandHandler/SetCarrierTaxRuleGroupHandler.php Outdated Show resolved Hide resolved
tests/Integration/Behaviour/Features/Scenario/Carrier/carrier_tax_rule_group_management.feature Show resolved Hide resolved
@ps-jarvis ps-jarvis added the Waiting for author Status: action required, waiting for author feedback label Jun 21, 2024
@tleon tleon closed this Jun 21, 2024
@tleon tleon reopened this Jun 21, 2024
@tleon tleon force-pushed the Issue-35926-add-update-multistore branch from 3c661a6 to 82714d9 Compare June 24, 2024 15:36
@tleon tleon requested review from jolelievre and boherm June 24, 2024 17:00
@jolelievre jolelievre force-pushed the Issue-35926-add-update-multistore branch 3 times, most recently from adedaeb to 91ca887 Compare June 25, 2024 15:23
jolelievre
jolelievre previously approved these changes Jun 25, 2024
View reviewed changes
@jolelievre jolelievre force-pushed the Issue-35926-add-update-multistore branch from 91ca887 to 3096f63 Compare June 26, 2024 07:14
@jolelievre jolelievre force-pushed the Issue-35926-add-update-multistore branch 2 times, most recently from 51d6ef4 to 3889ea7 Compare June 26, 2024 08:18
@jolelievre jolelievre removed the Waiting for author Status: action required, waiting for author feedback label Jun 26, 2024
@jolelievre jolelievre force-pushed the Issue-35926-add-update-multistore branch from dc41f8e to 05f9aaf Compare June 26, 2024 12:11
@jolelievre jolelievre force-pushed the Issue-35926-add-update-multistore branch 3 times, most recently from 1d7ce7b to c1c8aad Compare June 26, 2024 14:44
@jolelievre jolelievre force-pushed the Issue-35926-add-update-multistore branch from c1c8aad to b778076 Compare June 26, 2024 14:52
@jolelievre jolelievre force-pushed the Issue-35926-add-update-multistore branch from b778076 to 154dafe Compare June 26, 2024 14:56
boherm
boherm reviewed Jun 27, 2024
View reviewed changes
src/Adapter/Carrier/CommandHandler/SetCarrierTaxRuleGroupHandler.php

$carrier = $this->carrierRepository->get($command->getCarrierId());

$newCarrier = $this->carrierRepository->updateInNewVersion($command->getCarrierId(), $carrier);
Copy link
Member

Choose a reason for hiding this comment

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

This way will change soon ;)

boherm
boherm approved these changes Jun 27, 2024
View reviewed changes
Copy link
Member

@boherm boherm left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @tleon!

@ps-jarvis ps-jarvis added the Waiting for QA Status: action required, waiting for test feedback label Jun 27, 2024
@jolelievre jolelievre removed the Waiting for QA Status: action required, waiting for test feedback label Jun 27, 2024
@jolelievre
Copy link
Contributor

Thanks @tleon

@jolelievre
Copy link
Contributor

Rock and roll 🤘

@jolelievre jolelievre merged commit 6a3c869 into PrestaShop:develop Jun 28, 2024
42 checks passed
@ps-jarvis
Copy link

PR merged, well done!

Message to @PrestaShop/committers: do not forget to milestone it before the merge.

@kpodemski kpodemski added this to the 9.0.0 milestone Jun 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jolelievre jolelievre jolelievre approved these changes

@boherm boherm boherm approved these changes

Assignees

@tleon tleon

Labels
None yet
Projects
PR Dashboard
Archived in project
Milestone
9.0.0
Development

Successfully merging this pull request may close these issues.

[CQRS] Add/Update multistore
5 participants
@tleon @jolelievre @ps-jarvis @boherm @kpodemski