Notify users mentioned in chat messages #460
Conversation
danxuliu
added
2. developing
enhancement
feature: chat 💬
lib/Chat/Notifier.php
Outdated
| * has no password, or if it has a password and the user is a participant | ||
| * already. | ||
| + | ||
| + @param string userId |
|
Btw when is the PR for the chat UI coming? We need to merge stuff today............ |
|
I can take care of the notifications afterwards especially about the "hide when you are active" |
danxuliu
force-pushed
the
notify-users-mentioned-in-chat-messages
branch
from
7bafac4 to
59f7687
Compare
Today? :-S I thought the limit was tomorrow... Well, then the PR for the chat UI will have to come today.
Great, thanks! |
nickvergessen
force-pushed
the
notify-users-mentioned-in-chat-messages
branch
from
59f7687 to
bef6d2a
Compare
|
While rebasing and looking into the stuff, I stumbled over a few things:
|
|
With the last commit I resolved 1. and also the issue with "dont mention while active" |
Well, I think that users should be notified when mentioned on a public room that they are not part of (that is why I made it that way :-P ). But I do not have a strong opinion on that, and you already removed it... :-P
I kept mentions by deleted users just for consistency with the Comments app. Anyway I do not have a strong opinion on this either, so feel free to drop them if you want.
I have been thinking about not notifying a user when she is active and I am not sure about it. If a user is mentioned in an inactive room she will get a notification because she did not read the mention. I think that if a user is mentioned in an active room she should get the notification too until she actually read the mention. It would be good if Talk could intercept the Notification app of its own tab/window to prevent it from showing notifications for chat mentions (just prevent it from showing, without discarding them). This would make possible to provide notifications better integrated with the Talk UI (for example, by adding a mention counter to the room in the room list) while not interfering with other open Nextcloud tabs/windows. @nickvergessen Is there any kind of support for that in the Notifications app? Anyway, yes, currently if a user is active in a room she will immediately receive the messages, and thus the notification for the mention will be removed, because there is no support yet in the frontend nor the backend for unread messages. But I am working on that :-) |
|
Just to clarify, because there seem to be a misunderstanding: |
No, that is exactly what I think that should not be done. I mean, if you are in room A and a user in room A mentions you you should be notified like in the other cases; only once you actually read the mention the notification should be removed. An example: In that scenario the user should have been notified about the mention even if she was active in the room. |
|
The problem with this solution is, that you still get a push notification etc. So even though its immediatelly invalidated, you get noise. |
When a user is mentioned in a chat message she will now receive a
notification about that. If a chat room is deleted all the pending
notifications will be also removed.
The rich subject of the notifications use "call" instead of "room" or
"chat" due to "call" being already available in
"lib/public/RichObjectStrings/Definitions.php" (in Nextcloud server),
although the Notifications app does not handle it in any special way
though ("apps/notifications/js/richObjectStringParser.js").
Note that, unlike with the current code for invitation notifications, no
checks are made on whether "call" is available or not, as the chat is
available only from Nextcloud 13 and onwards.
Currently every user mentioned in a chat message is notified, but this
will be tightened in later commits (for example, if a user is mentioned
in a chat room to which she is not able to join).
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
The chat message can be much longer than the maximum length allowed for notification messages, so the notification message is trimmed if needed. When the notification is being prepared to be displayed it is taken into account if the message was trimmed, and in that case an ellipsis is added at the start and/or the end (depending on the case) of the message. Note that the original notification message does not contain the ellipsis characters, as depending on the language a whitespace should be added or not before and after an ellipsis, and the language in which the notification will be displayed is only known at the time of displaying it, but not when it is saved. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Now mentioned users are notified only if they are already participants of a private chat (a one to one chat, a group chat, or a password protected public chat), or if the message was written in a public chat. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
Also removes notifications for public rooms where the user was not invited. Signed-off-by: Joas Schilling <coding@schilljs.com>
nickvergessen
force-pushed
the
notify-users-mentioned-in-chat-messages
branch
from
e615d17 to
18a9456
Compare
|
Implemented this way now. Also marking mention notifications read when you pull the messages of that room now. |
…ad the chat Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
nickvergessen
force-pushed
the
notify-users-mentioned-in-chat-messages
branch
from
18a9456 to
6346c5a
Compare
…ages of the room Signed-off-by: Joas Schilling <coding@schilljs.com>
nickvergessen
force-pushed
the
notify-users-mentioned-in-chat-messages
branch
from
6346c5a to
335ad72
Compare
This pull request adds notifications for users mentioned in a chat; the notification links to the Talk room, and includes an ellipsized version of the original message centered around the mention (as the length of the notification message can be at most 64 bytes).
The rich subject of the notifications use
callinstead ofroomorchatdue tocallbeing already available in lib/public/RichObjectStrings/Definitions.php (in Nextcloud server). Unlike with the current code for invitation notifications, no checks are made on whethercallis available or not, as the chat will be available only from Nextcloud 13 and onwards (I think so :-P ).A mentioned user is notified only if she is able to participate in the room in which she was mentioned, that is, if she is already a participant of the room if the message was written in a private chat (a one to one chat, a group chat, or a password protected public chat), or in any case if the message was written in a public chat. When a room is deleted all the pending notifications for that room are also removed.
Still missing is not sending a notification to a user when she is mentioned in the chat room in which she is currently active. It is related to #459, but I will need some guidance here @nickvergessen @Ivansss (and if you want to implement it yourselves please feel free to do it ;-) ).
Finally, there are no integration tests yet, although they can be added at a later time.