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
Refactor mailing #12050
Refactor mailing #12050
Conversation
SirDomin
commented
Nov 17, 2020
Q | A |
---|---|
Branch? | master |
Bug fix? | no |
New feature? | no |
BC breaks? | no |
Deprecations? | not sure |
Related tickets | refactoring the way we handle emails implemented in #11754 |
License | MIT |
src/Sylius/Bundle/ApiBundle/spec/CommandHandler/Checkout/CompleteOrderHandlerSpec.php
Outdated
Show resolved
Hide resolved
|
||
final class SendOrderConfirmationHandlerSpec extends ObjectBehavior | ||
{ | ||
function let( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can be in one line
src/Sylius/Bundle/ApiBundle/EventListener/OrderCompleteListener.php
Outdated
Show resolved
Hide resolved
@@ -66,7 +68,7 @@ public function __invoke(CompleteOrder $completeOrder): OrderInterface | |||
|
|||
$stateMachine->apply(OrderCheckoutTransitions::TRANSITION_COMPLETE); | |||
|
|||
$this->emailManager->sendConfirmationEmail($cart); | |||
$this->messageBus->dispatch(new OrderCompletedEvent($orderTokenValue), [new DispatchAfterCurrentBusStamp()]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be good to add comment why you added that stamp here.
src/Sylius/Bundle/ApiBundle/CommandHandler/Checkout/CompleteOrderHandler.php
Outdated
Show resolved
Hide resolved
src/Sylius/Bundle/ApiBundle/CommandHandler/Checkout/CompleteOrderHandler.php
Outdated
Show resolved
Hide resolved
src/Sylius/Bundle/ApiBundle/EventListener/OrderCompleteListener.php
Outdated
Show resolved
Hide resolved
src/Sylius/Bundle/ApiBundle/EventListener/OrderCompleteListener.php
Outdated
Show resolved
Hide resolved
7e8473d
to
546bafc
Compare
546bafc
to
da13fb2
Compare
src/Sylius/Bundle/ApiBundle/Resources/config/services/command_handlers.xml
Outdated
Show resolved
Hide resolved
src/Sylius/Bundle/ApiBundle/EventHandler/OrderCompletedHandler.php
Outdated
Show resolved
Hide resolved
src/Sylius/Bundle/ApiBundle/spec/CommandHandler/Checkout/CompleteOrderHandlerSpec.php
Outdated
Show resolved
Hide resolved
src/Sylius/Bundle/ApiBundle/spec/CommandHandler/Checkout/CompleteOrderHandlerSpec.php
Outdated
Show resolved
Hide resolved
$emailManager->sendConfirmationEmail($order)->shouldBeCalled(); | ||
$orderCompleted = new OrderCompleted('COMPLETED_ORDER_TOKEN'); | ||
|
||
$eventBus->dispatch($orderCompleted, [new DispatchAfterCurrentBusStamp()]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm surprised, that it works. I would expect, that the second argument makes a problem with matching to the actual value.
src/Sylius/Bundle/ApiBundle/Resources/config/services/event_handlers.xml
Outdated
Show resolved
Hide resolved
e866c74
to
b7bbf2c
Compare
b7bbf2c
to
b10b272
Compare
Thanks, @SirDomin! 🥇 |
This PR was merged into the 1.9-dev branch. Discussion ---------- | Q | A | --------------- | ----- | Branch? | master | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Related tickets | #12050 | License | MIT <!-- - Bug fixes must be submitted against the 1.7 or 1.8 branch (the lowest possible) - Features and deprecations must be submitted against the master branch - Make sure that the correct base branch is set To be sure you are not breaking any Backward Compatibilities, check the documentation: https://docs.sylius.com/en/latest/book/organization/backward-compatibility-promise.html --> Commits ------- 02ca9e6 adr how to send email e537bd4 pr-fix
<argument type="service" id="sylius_event.bus" /> | ||
<tag name="messenger.message_handler" bus="sylius_default.bus" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering about naming. Maybe we should have two busses:
- sylius.command_bus
- sylius.event_bus
and try to alias sylius_default.bus
to sylius.command_bus
if it is possilbe