Fix ID collision in comment issue banners when resolving issues.

Review Request #12437 — Created July 5, 2022 and submitted

chipx86
Review Board
release-5.0.x
reviewboard

If a review request contains multiple comments of different types but
with the same IDs (such as a General Comment and a Diff Comment both
with ID 2), closing one of the comments would result in the other
comment appearing to close as well. In fact, the comment did not close,
it merely appeared to.

This was due to a bad check in CommentIssueBarView. It listened for
state updates and assumed that if the IDs matched, that was good enough.
It also needed to consider the comment type.

In order to do that, we first needed a way to represent the types. We
had hard-coded strings like 'diff_comments', but nothing formal, and
no way to get to it from the event handler.

There's now a CommentIssueManager.CommentTypes mapping of type
constants to values. The appropriate value is determined and then
provided when triggering issueStatusChanged. This allows the event
handler to perform the proper match.

Unit tests have been updated to check this for each supported comment
type.

In the future, we'll want to consider rethinking some of the way this
works, moving toward a model where we work directly with the comment
objects (ensuring there's only one object per comment) rather than
a more broad listener approach (which requires each handler to
perform these checks).

Unit tests pass.

Reproduced the original problem in a new database, and verified that
this fixes the state and UI collision.

Summary
Fix ID collision in comment issue banners when resolving issues.
Description From Last Updated

Expected '===' and instead saw '=='. Column: 27 Error code: W116

reviewbotreviewbot

A constructor name should start with an uppercase letter. Column: 41 Error code: W055

reviewbotreviewbot

A constructor name should start with an uppercase letter. Column: 45 Error code: W055

reviewbotreviewbot

A constructor name should start with an uppercase letter. Column: 45 Error code: W055

reviewbotreviewbot

Could fix the typo here, "dispaly" -> "display"

maubinmaubin
Checks run (1 failed, 1 succeeded)
flake8 passed.
JSHint failed.

JSHint

chipx86
maubin
  1. 
      
  2. Could fix the typo here, "dispaly" -> "display"

  3. 
      
chipx86
maubin
  1. Ship It!
  2. 
      
david
  1. Ship It!
  2. 
      
chipx86
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-5.0.x (4b04007)
Loading...