Skip to content

Add or Remove attention request instead of toggling it

For #357349 (closed)

Why

First, two facts about the previous implementation of attention requests:

  1. Adding/removing attention requests uses a toggle based on the known front end state at the time of interaction
  2. When a user does one of the actions that automatically manages attention requests, the attention display isn't aware of those changes until a refresh.

Because of these two facts, it's possible to have the front end in the opposite state as the back end. Because the front end behavior is a toggle, an action on the front end that appears to be "remove the attention request for myself" actually results in "toggle the attention request status for me" which results in adding the attention request because the back end has already been switched to "no attention request present" automatically.

What

Following on from a back end change, this MR swaps the front end implementation to use an "explicit" or "forced" toggle, like the second signature of DOMTokenList.toggle().

While the interface that most components use to integrate with this feature still reads as a toggle, there's now a forced "direction" that is used to determine what to "toggle" to.

This is determined - exclusively - as "the opposite of whatever we think it is right now."

This way, if the UI is showing that a user has an attention request, and they click the icon to remove it, it guarantees that a request is made to remove the attention request, and vice versa.

Fundamentally it "doesn't matter" what the backend has stored; all that matters is that the user expectation is met when they click the attention request control.

Caveat

While this is the iterative step forward on this feature to resolve the buggy behavior, I don't believe this solves the foundational problem.

This ensures the UX state isn't misinterpreted, leading to unmet user expectations, but the problem is that attention request state changes that happen automatically are not communicated to other parts of the application that need to display that new state.

Testing

This assumes the new changes in this branch. You can follow the same steps on master to see how the previous implementation causes a mismatch between user expectations and what really happens.

  1. Using the Rails console: Feature.enable( :mr_attention_requests )
  2. Create an MR
  3. Assign one user as the assignee, and another user as the reviewer
  4. Ensure you are signed in as one of those two users
  5. Set up the attention requests so that your own user has their attention requested (solid color) and the other user does not have their attention requested
  6. Using a quick command, request the attention of the other user: /attention @other_user
    • Note that if you're on the Overview tab, you'll see the automated removal of your own attention as a System note after a short polling interval (when you request someone else's attention, it removes the request for yours).
  7. The UI will continue to show your own attention still being requested (solid color)
  8. Click the attention request chevron to remove the request for your own attention.
    • You should see a toast message that indicates that you've removed the request for your own attention.
  9. Critically, note that GitLab does not re-add the request for your own attention.
Edited by Thomas Randolph

Merge request reports

Loading