Commit 3a865ed2 authored by Douwe Maan's avatar Douwe Maan

Merge branch '21211-comment-on-diff-partially-broken-after-updating-to-8-11' into 'master'

Fix diff comments on legacy MRs

## What does this MR do?

Allow diff commenting on MRs without complete diff refs. (Commenting would work before, but the JSON response would fail, so you'd only see your comment after a refresh.)

## Are there points in the code the reviewer needs to double check?

Is this really all I needed to do? I feel like there must be more to it 😊

## Why was this MR needed?

It's a regression!

## What are the relevant issue numbers?

Closes #21211.

## Does this MR meet the acceptance criteria?

- [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
- Tests
  - [x] Added for this feature/bug
  - [x] All builds are passing
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)

See merge request !6029
parents b2e23e33 9b57ad38
...@@ -49,6 +49,9 @@ v 8.11.4 (unreleased) ...@@ -49,6 +49,9 @@ v 8.11.4 (unreleased)
v 8.11.4 (unreleased) v 8.11.4 (unreleased)
- Fix resolving conflicts on forks - Fix resolving conflicts on forks
v 8.11.4 (unreleased)
- Fix diff commenting on merge requests created prior to 8.10
v 8.11.3 (unreleased) v 8.11.3 (unreleased)
- Do not enforce using hash with hidden key in CI configuration. !6079 - Do not enforce using hash with hidden key in CI configuration. !6079
- Allow system info page to handle case where info is unavailable - Allow system info page to handle case where info is unavailable
......
...@@ -28,4 +28,8 @@ module NoteOnDiff ...@@ -28,4 +28,8 @@ module NoteOnDiff
def can_be_award_emoji? def can_be_award_emoji?
false false
end end
def to_discussion
Discussion.new([self])
end
end end
...@@ -107,10 +107,6 @@ class DiffNote < Note ...@@ -107,10 +107,6 @@ class DiffNote < Note
self.noteable.find_diff_discussion(self.discussion_id) self.noteable.find_diff_discussion(self.discussion_id)
end end
def to_discussion
Discussion.new([self])
end
private private
def supported? def supported?
......
...@@ -147,6 +147,37 @@ feature 'Diff notes', js: true, feature: true do ...@@ -147,6 +147,37 @@ feature 'Diff notes', js: true, feature: true do
end end
end end
context 'when the MR only supports legacy diff notes' do
before do
@merge_request.merge_request_diff.update_attributes(start_commit_sha: nil)
visit diffs_namespace_project_merge_request_path(@project.namespace, @project, @merge_request, view: 'inline')
end
context 'with a new line' do
it 'should allow commenting' do
should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"]'))
end
end
context 'with an old line' do
it 'should allow commenting' do
should_allow_commenting(find('[id="6eb14e00385d2fb284765eb1cd8d420d33d63fc9_22_22"]'))
end
end
context 'with an unchanged line' do
it 'should allow commenting' do
should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_7_7"]'))
end
end
context 'with a match line' do
it 'should not allow commenting' do
should_not_allow_commenting(find('.match', match: :first))
end
end
end
def should_allow_commenting(line_holder, diff_side = nil) def should_allow_commenting(line_holder, diff_side = nil)
line = get_line_components(line_holder, diff_side) line = get_line_components(line_holder, diff_side)
line[:content].hover line[:content].hover
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment