Replace Discussion#new_discussion? with more reliable Note#start_of_discussion?
What does this MR do?
This MR adds a new Note#start_of_discussion?
method to replace the existing Discussion#new_discussion?
method.
@engwan identified a race condition with how we were using Discussion#new_discussion?
in !19990 (comment 244381523).
Discussion#new_discussion?
checks for whether a discussion is new like this:
def new_discussion?
notes.length == 1
end
The race condition is:
- A new discussion is created.
- A reply to the discussion is quickly posted.
- When a worker is run,
#new_discussion?
and would now return false because theDiscussion
has 2 notes.
As he suggested:
I think we should remove
Discussion#new_discussion?
and instead have something likeNote#start_of_discussion?
which checks if the current note is the first note of a discussion.
This method replaces Discussion#new_discussion?
with a more reliable
method of checking if a note is the start of a discussion, and updates
its use in the codebase.
In his review, @oswaldo identified a method with the same functionality
existed in DiffNote#discussion_first_note?
already. This MR removes
that method, as DiffNote inherits from Note.
Issue: #36329 (closed)
Does this MR meet the acceptance criteria?
Conformity
- [-] Changelog entry
- [-] Documentation (if required)
-
Code review guidelines -
Merge request performance guidelines -
Style guides - [-] Database guides
- [-] Separation of EE specific content