Bug fix - Select the correct users to notify for subscribed posts #3442
Conversation
This file contains 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
|
Hey! Thanks for the PR! While this does solve the bug, it is a change in behavior, I believe doing this would send reply notifications not just once but every time there is a new post reply. The initial behavior was to just send a notification on the first reply only. Ref: #3443 Need to look into this a bit further 🤔 |
|
@OwenMelbz thanks for the pull request. After some further research it seemed your changes weren't the right one. We have merged a fix and will deploy this as soon as possible as a patch. Although your contribution wasn't merged, we are still very appreciative of the initial research and assistance in trying to resolve the issue. Thank you! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Sorry, I don't have time to do a full tested and reviewed PR feedback form to due our project deadlines.
However this has been requested via: https://discuss.flarum.org/d/30928-subscription-notifications-not-firing
The issue was that the notification would only get dispatched to users who's last read number was the same as the latest post.
However we need to send the notification to all users who have a read number LESS than the current post to make sure they are notified.
This PR hopes to solve that.