Skip to content

Rename ChatStrategy unread option

Tomas Vik requested to merge rename-unread-option into develop

This MR clarifies what the unread option means during serializing a chat message.

As the new comment states:

/**
 * `overrideUnreadTo` can have 3 states:
 *     - `undefined` - either the default value or unreadItemStrategy is used
 *     - `true` - all chat items are going to have  `unread: true`
 *     - `false` - all chat items are going to have `unread: false`
 */

The overrideUnreadTo (formerly unread) option, when present, hardcodes the unread attribute on every serialized message to its boolean value.

Note: This is at least third time I was wondering what it does and when I understood it I wanted to capture it in code

Best way to review is going to be a commit by commit because it goes from the serializer all the way to renderers.

graph LR;
A[ChatStrategy] --> B[serializeChatsForTroupe]
B --> C[renderChat]

Future refactoring

  • this MR is preparing the ground for using unread: false as a default value in the serializer. That will remove the need for all the "not logged in" routes to use this attribute and most likely the attribute can be then removed altogether.

Testing strategy

  1. validate that read messages in normal chat are not highlighted in any way
  2. validate that unread messages in the normal chat get highlighted as unread
  3. messages in the archive are not highlighted as unread (more of a sanity check because you cant check today's archive https://gitlab.com/gitlab-org/gitter/webapp/issues/2349 where there would be most likely unread messages)
  4. check the ~embed route (create messages with one user and then visit the <roomUrl>/~embed with another user) logged in user should see notifications, an anonymous user shouldn't see any
Edited by Eric Eastwood

Merge request reports