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

Add a note about payment-related events #15290

Merged
merged 1 commit into from Sep 14, 2023
Merged

Conversation

jakubtobiasz
Copy link
Member

@jakubtobiasz jakubtobiasz commented Sep 12, 2023

Q A
Branch? 1.12
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Related tickets closes #12541
License MIT

We agreed there is no good way to resolve that issue in 1.x version, so we decided to make the docs as clear as it's possible in that matter.

CleanShot 2023-09-13 at 04 49 57

P.S. I've created a task for considering how to fix this in later Sylius versions :).

@jakubtobiasz jakubtobiasz added the Documentation Documentation related issues and PRs - requests, fixes, proposals. label Sep 12, 2023
@jakubtobiasz jakubtobiasz requested a review from a team as a code owner September 12, 2023 13:05
@github-actions
Copy link

github-actions bot commented Sep 12, 2023

Bunnyshell Preview Environment deployed

It will be automatically stopped in 4 hours.

Use the command /bns:start to start it if it's stopped.

Component Endpoints
mailhog https://mailhog-elxvvv.bunnyenv.com/
php https://store-elxvvv.bunnyenv.com/

Available commands:

  • /bns:stop to stop the environment
  • /bns:deploy to redeploy the environment
  • /bns:delete to remove the environment

@diimpp
Copy link
Member

diimpp commented Sep 12, 2023

  1. are only dispatched by the ResourceController phrase suggest there is implication, but new documentation reader wouldn't know which one.
  2. sylius.payment.pre/post_complete correlates to sylius_payment SM with completed state, while people most likely intrested in sylius_order_payment SM paid state.

I couldn't phrase it better, but my take on the message is to split it into two alerts:
Warning

These events are not universal way to listen for a completed payment (Dispatched by ResourceController at specific places like payment complete for admin ui, but not at the time of payment by a customer for a shop ui), consider adding :doc:callback </customization/state_machine> for a "sylius_payment" state machine with completed state.

Tip

If you looking for order payment paid event, consider adding :doc:callback </customization/state_machine> for a "sylius_order_payment" state machine with paid state.

@jakubtobiasz
Copy link
Member Author

Thanks for your perspective @diimpp! Now it should sound better (I guess :D).

Prometee
Prometee previously approved these changes Sep 13, 2023
docs/book/orders/payments.rst Outdated Show resolved Hide resolved
Prometee
Prometee previously approved these changes Sep 13, 2023
NoResponseMate
NoResponseMate previously approved these changes Sep 14, 2023
GSadee
GSadee previously approved these changes Sep 14, 2023
docs/book/orders/payments.rst Outdated Show resolved Hide resolved
@GSadee GSadee merged commit 875171e into Sylius:1.12 Sep 14, 2023
6 checks passed
@GSadee
Copy link
Member

GSadee commented Sep 14, 2023

Thank you, Jacob! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Documentation related issues and PRs - requests, fixes, proposals.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants