Clarify that the notifications config key has no effect (#2359) - #43
Open
wakqasahmed wants to merge 1 commit into
Open
Clarify that the notifications config key has no effect (#2359)#43wakqasahmed wants to merge 1 commit into
wakqasahmed wants to merge 1 commit into
Conversation
The Order Notifications section described mailers and notifications as if both were dispatched the same way, but only mailers are wired up anywhere in Lunar core - the notifications array is read nowhere. Fixes #2359.
Author
|
Hi @glennjacobs — this one seems to have no reviewer assigned, about 4 days in with CI passing. Happy to be pointed at whoever's best placed to look. |
Collaborator
Please stop chasing PRs. This is a last warning. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The Order Notifications section reads as if
mailersandnotificationsboth do something when an order's status changes, but onlymailersis actually wired up anywhere in Lunar core.notificationsis present in the default config stub and nothing ever reads it — checked across the wholelunarphp/lunarrepo, not just the order-status code. Closes lunarphp/lunar#2359.Added a warning noting this and pointing at the
Ordermodel'supdatedevent withwasChanged('status')as the way to hook a real notification in yourself today.The same wording exists in
0.x/reference/orders.mdxtoo — left that alone since 0.x isn't the target of the reported issue, happy to open a follow-up if useful.