Merge request diff notes are marked as "outdated" after a push when the line in question actually persisted
This also causes issues where commenting on an existing diff note discussion after a new push will create a new discussion, instead of adding onto the previous one.
Example: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/6425#note_17066906, and the continued discussion on the same line on https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/6425#note_17147374
What happens is that when the MR is updated, every DiffNote
should have its position
updated to correspond to the new locations of the lines in the new MR diff, which happens here: https://gitlab.com/gitlab-org/gitlab-ce/blob/master/app/models/merge_request.rb#L823. When that position
is not updated, GitLab takes that to mean the line the note was on could not be found in the new diffs, and will display it as "outdated".
The problem may be in http://gitlab.com/gitlab-org/gitlab-ce/blob/master/app/models/merge_request.rb#L377, which tries to get the DiffRefs for the new MergeRequestDiff (corresponding to the new push).
It assumes that self.diff_refs
will return the new diff refs immediately after create_merge_request_diff
is called. self.diff_refs
calls diff_start_commit
and diff_base_commit
, which are delegated to merge_request_diff
, which returns the last MergeRequestDiff
of the MR, but is memoized by ActiveRecord, which means that it will return still return the cached second-to-last MergeRequestDiff
after create_merge_request_diff
.
We should somehow reset the memoized merge_request_diff
, or get the new MergeRequestDiff
explicitly (as a return value from create_mege_request_diff
?).
One issue: resetting the memoization should already be happening with the merge_request_diff(true)
call on https://gitlab.com/gitlab-org/gitlab-ce/blob/master/app/models/merge_request.rb#L362, so perhaps my analysis is totally incorrect.