-
Notifications
You must be signed in to change notification settings - Fork 83
IBX-10675: Document Ibexa Messenger and discount re-indexing in the background #2909
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
Conversation
code_samples/background_tasks/src/MessageHandler/SomeHandler.php
Outdated
Show resolved
Hide resolved
Thank you @konradoboza for your comments |
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.
Few links to add, some precisions about the code, but over all very good.
code_samples/background_tasks/src/Dispatcher/SomeClassThatSchedulesExecutionInTheBackground.php
Show resolved
Hide resolved
Co-authored-by: Adrien Dupuis <[email protected]>
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 love it already, thank you for preparing this! 🙇
Added a couple of suggestions, but the overall shape is great
One sugggestion that does not apply to a particular line:
- For v5, everyone using Commerce should set this up (or at least know that it needs to be set up). I'd mention it in the post install steps: https://github.com/ibexa/documentation-developer/blob/5.0/docs/getting_started/install_ibexa_dxp.md#post-installation-steps
- For 4.6, it's the same but when installing the Discounts LTS update (I assume you'll adjust it when cherry-picking to 4.6)
On a different note, @Steveb-p what do you think about adding some (commented out) configuration for Ibexa Cloud to make setting up Messenger easier? I mean https://github.com/ibexa/post-install/blob/main/resources/platformsh/ibexa-commerce/5.0/.platform.app.yaml
code_samples/background_tasks/src/Dispatcher/SomeClassThatSchedulesExecutionInTheBackground.php
Outdated
Show resolved
Hide resolved
|
||
use App\Message\SomeMessage; | ||
|
||
final class SomeHandler |
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.
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.
This is only true for Symfony 5.x. On Symfony 6 and above, that interface does not exist.
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.
Ok, so this is ok for v5, but the interface should be added when backporting to 4.6?
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.
On 4.6 MessageHandlerInterface
is indeed required. I don't remember if you can create handlers without it, or just tag the service properly. I think it's mostly used to mark service, but unsure.
code_samples/ change report
|
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.
Thank you!
…ackground (#2909) * API changes documented * Add messenger description * Modify configure_discounts.md * Add dispatchers to discount events * Mention discount re-indexing in post install steps --------- Co-authored-by: dabrt <[email protected]> Co-authored-by: Adrien Dupuis <[email protected]> Co-authored-by: Marek Nocoń <[email protected]>
Document Ibexa Messenger and discount re-indexing in the background
Checklist