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.

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

Fix implementation of stream notifications without Rocketbot #4016

Closed
timkinnane opened this issue Aug 16, 2016 · 4 comments
Closed

Fix implementation of stream notifications without Rocketbot #4016

timkinnane opened this issue Aug 16, 2016 · 4 comments

Comments

@timkinnane
Copy link
Contributor

timkinnane commented Aug 16, 2016

0.37.0

Related #3869

Notifications are pretty weird, if I understand correctly...

Notifications.notifyUser (with second param as 'message') emits a notification message event directed at a user (usually the current user). It accepts the message object (usually defined with a random user ID, with a localized string as the message content).

Then RoomManager watches for RocketChat.Notifications.onUser event and inserts a new message into the stream, used to give feedback to user within the current channel, without actually adding that message to the channel (as it shouldn't appear to all).

The message has the username attribute assigned as 'rocketbot' before its inserted into the stream.

This method is used for various message error notices e.g. User submits message when muted or tries to mention @all but doesn't have mention-all permission.

Problems:

  • Passing the random ID for a notification message seems redundant, since it's being replaced and also seems bad practice to be assigning system messages to random IDs (even if temporarily).
  • Sometimes the notifications functionality is bypassed, with packages directly calling the same code, adding messages to the stream with rocketbot as username. e.g. chatMessages.coffee No_such_command error.
  • Rocket**.Cat** is a user in most instances (maybe it once was rocketbot), but this has branding issues as detailed in Muted submission response inconsistent with system notices and translations #3869. This name should be configurable, either use the instance title or a localized string like "system".
  • When you click on the rocketbot name it brings up a user panel, but it's not a real user, so none of the controls work. You get 400 errors if you try to click them. e.g. Try muting rocketbot. Notifications should be rendered differently, so the name is not clickable or relate to normal user functions.
@timkinnane
Copy link
Contributor Author

Worth noting that notifications do not persist in the room, because they're added directly to the client stream, not the server. They look like messages and have all the UI controls of a normal message, but don't act like messages. They don't need a user icon or time stamp or message controls. It's a subtle but confusing user experience, they should probably be replaced altogether by a more appropriately designed component.

screenshot 2016-08-16 18 28 44

@timkinnane
Copy link
Contributor Author

FWIW, I went overboard with the detail because I intend to come back here one day and submit a fix.

@timkinnane
Copy link
Contributor Author

#3498 and #3418 are related, but these are messages to the channel instead. There may be a common solution.

@timkinnane
Copy link
Contributor Author

Discuss further on #10456

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant