Skip to content

Replace Discussion#new_discussion? with more reliable Note#start_of_discussion?

Luke Duncalfe requested to merge 36329-discussion-start-of-discussion into master

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 the Discussion has 2 notes.

As he suggested:

I think we should remove Discussion#new_discussion? and instead have something like Note#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

Availability and Testing

Edited by Luke Duncalfe

Merge request reports