Comment reply notifications often not being sent to the correct/expected user #4214

Closed
opened 2026-02-05 08:15:41 +03:00 by OVERLORD · 4 comments
Owner

Originally created by @carlossierra311 on GitHub (Sep 14, 2023).

Originally assigned to: @ssddanbrown on GitHub.

Describe the Bug

Notifications are not being sent to the original commenter when a reply is made to their comments on pages not owned or maintained by them.

Btw, splendid work on the notification template: its clean, clear and informative. Well done!

Steps to Reproduce

  1. Make a comment in a page not owned, nor maintained, by the user.
  2. Ask another user to reply to that comment.
  3. Wait for the notification to (not) arrive ;)

Expected Behaviour

The notification of a reply to their previous comment should be sent to the user when they have this type of notification enabled in their settings.

Screenshots or Additional Context

I have domain restriction configured for my BookStack implementation, so that only users from my company can register. But the account I am using to test the new notifications is a Hotmail account (which is obviously not my company's domain). I don´t know if that´s relevant, but wanted to mention it, just in case.

Browser Details

Google Chrome 116.0.5845.180 (Build oficial) (64 bits)

Exact BookStack Version

v23.08.2

Originally created by @carlossierra311 on GitHub (Sep 14, 2023). Originally assigned to: @ssddanbrown on GitHub. ### Describe the Bug Notifications are not being sent to the original commenter when a reply is made to their comments on pages not owned or maintained by them. Btw, splendid work on the notification template: its clean, clear and informative. Well done! ### Steps to Reproduce 1. Make a comment in a page not owned, nor maintained, by the user. 2. Ask another user to reply to that comment. 3. Wait for the notification to (not) arrive ;) ### Expected Behaviour The notification of a reply to their previous comment should be sent to the user when they have this type of notification enabled in their settings. ### Screenshots or Additional Context I have domain restriction configured for my BookStack implementation, so that only users from my company can register. But the account I am using to test the new notifications is a Hotmail account (which is obviously not my company's domain). I don´t know if that´s relevant, but wanted to mention it, just in case. ### Browser Details Google Chrome 116.0.5845.180 (Build oficial) (64 bits) ### Exact BookStack Version v23.08.2
OVERLORD added the 🐛 Bug🚀 Priority labels 2026-02-05 08:15:41 +03:00
Author
Owner

@ssddanbrown commented on GitHub (Sep 15, 2023):

Thanks for raising @carlossierra311.

Looking into this, I don't think this is anything specific to who the owner/maintainer is, but a misidentification of the parent comment (being replied to) that will instead often look to an early comment, so a first reply on a page will consider the first comment ever created in the system. This could create awkward behaviour like that experienced.

I've marked this as a priority to address in a patch release, which I'll probably look to put together tomorrow.
Code link for my own reference.

Btw, splendid work on the notification template: its clean, clear and informative. Well done!

Thanks!

@ssddanbrown commented on GitHub (Sep 15, 2023): Thanks for raising @carlossierra311. Looking into this, I don't think this is anything specific to who the owner/maintainer is, but a misidentification of the parent comment (being replied to) that will instead often look to an early comment, so a first reply on a page will consider the first comment _ever created_ in the system. This could create awkward behaviour like that experienced. I've marked this as a priority to address in a patch release, which I'll probably look to put together tomorrow. [Code link](https://github.com/BookStackApp/BookStack/blob/615741af9d36c725299cc8fc9c0ee012bd3d5759/app/Activity/Models/Comment.php#L41) for my own reference. > Btw, splendid work on the notification template: its clean, clear and informative. Well done! Thanks!
Author
Owner

@carlossierra311 commented on GitHub (Sep 15, 2023):

Thank you @ssddanbrown.

@carlossierra311 commented on GitHub (Sep 15, 2023): Thank you @ssddanbrown.
Author
Owner

@ssddanbrown commented on GitHub (Sep 15, 2023):

Addressed in 45b8d6cd0c, to be part of a very soon patch release.
Thanks again @carlossierra311 for reporting!

@ssddanbrown commented on GitHub (Sep 15, 2023): Addressed in 45b8d6cd0c214118a3745050b64805be75cec9d8, to be part of a very soon patch release. Thanks again @carlossierra311 for reporting!
Author
Owner

@carlossierra311 commented on GitHub (Sep 16, 2023):

I can confirm this is working now as expected. Thank you @ssddanbrown. Great work!!!

@carlossierra311 commented on GitHub (Sep 16, 2023): I can confirm this is working now as expected. Thank you @ssddanbrown. Great work!!!
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/BookStack#4214