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.

Sign up for GitHub

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

Jump to bottom

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

Closed
carlossierra311 opened this issue Sep 14, 2023 · 4 comments
Closed

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

carlossierra311 opened this issue Sep 14, 2023 · 4 comments
Assignees
@ssddanbrown
Labels
🐛 Bug 🚀 Priority
Milestone
v23.08.3

Comments

@carlossierra311
Copy link

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
@ssddanbrown ssddanbrown added this to the v23.08.3 milestone Sep 14, 2023
@ssddanbrown ssddanbrown self-assigned this Sep 14, 2023
@ssddanbrown
Copy link
Member

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 ssddanbrown changed the title Notifications not being sent for replies to comments made by non-owners/maintainers Comment reply notifications often not being sent to the correct/expected user Sep 14, 2023
@carlossierra311
Copy link
Author

Thank you @ssddanbrown.

ssddanbrown added a commit that referenced this issue Sep 15, 2023
@ssddanbrown
Comments: Fixed wrong identification of parent comment
45b8d6c 
Would cause comment reply notifications to not be sent to expected user.
Updated test to cover problem case.

For #4548
@ssddanbrown
Copy link
Member

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

@carlossierra311
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ssddanbrown ssddanbrown

Labels
🐛 Bug 🚀 Priority
Milestone
v23.08.3
Development

No branches or pull requests
2 participants
@ssddanbrown @carlossierra311