Skip to content

Resolve "Existing notes on merge requests aren't displayed on merge (head) version"

What does this MR do?

The backend added support for multi-line comments by added the property line_range to discussions. Due to inconsistencies in how the frontend and backend build this data line_range will not be available to both "sides" of the discussion comparison. For our purposes, we simply need to omit line_range from the comparison.

This will no longer be needed if the backend gets updated as per @robotmay's suggestion in #213855 (closed)

Review/Maintainer note

This MR fixes the "downstream" problem of the comparison not working, and not the "upstream" problem of the objects actually being inequal. Which is to say, I am not adding/removing line_range from the objects directly, only omitting them from the comparison.

I chose this approach for two reasons:

  1. This code is very complicated and changes the underlying objects is rife with possibilities of breaking edge cases. Fixing the downstream issue is much simpler and much less likely to break something.
  2. This should only be a temporary fix. If/when the backend code is updated this code will no longer be needed so let's keep it as simple as possible.

Screenshots

n/a

Does this MR meet the acceptance criteria?

Conformity

Related to #213855 (closed) Related to #213010 (closed)

Edited by Justin Boyson

Merge request reports