Commit 2a7da96a0d7fb71a1371ce2d97157a2dd8a766f2
Exists in
spb-stable
and in
3 other branches
Merge branch 'toggle_diff_comments' of https://github.com/jacob-carlborg/gitlabh…
…q into jacob-carlborg-toggle_diff_comments Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> Conflicts: app/assets/stylesheets/sections/commits.scss app/assets/stylesheets/sections/notes.scss app/views/projects/commits/_diffs.html.haml features/steps/project/merge_requests.rb
Showing
6 changed files
with
120 additions
and
4 deletions
Show diff stats
app/assets/stylesheets/sections/notes.scss
app/views/projects/commits/_diffs.html.haml
| @@ -44,7 +44,7 @@ | @@ -44,7 +44,7 @@ | ||
| 44 | - file = project.repository.blob_at(@commit.id, diff.new_path) | 44 | - file = project.repository.blob_at(@commit.id, diff.new_path) |
| 45 | - file = project.repository.blob_at(@commit.parent_id, diff.old_path) unless file | 45 | - file = project.repository.blob_at(@commit.parent_id, diff.old_path) unless file |
| 46 | - next unless file | 46 | - next unless file |
| 47 | - .diff-file{id: "diff-#{i}"} | 47 | + .diff-file.js-toggle-container{id: "diff-#{i}"} |
| 48 | .diff-header{id: "file-path-#{hexdigest(diff.new_path || diff.old_path)}"} | 48 | .diff-header{id: "file-path-#{hexdigest(diff.new_path || diff.old_path)}"} |
| 49 | - if diff.deleted_file | 49 | - if diff.deleted_file |
| 50 | %span= diff.old_path | 50 | %span= diff.old_path |
| @@ -60,6 +60,11 @@ | @@ -60,6 +60,11 @@ | ||
| 60 | %span.file-mode= "#{diff.a_mode} → #{diff.b_mode}" | 60 | %span.file-mode= "#{diff.a_mode} → #{diff.b_mode}" |
| 61 | 61 | ||
| 62 | .diff-btn-group | 62 | .diff-btn-group |
| 63 | + = link_to "#", class: "js-toggle-button btn btn-small" do | ||
| 64 | + %i.icon-chevron-down | ||
| 65 | + Diff comments | ||
| 66 | + | ||
| 67 | + | ||
| 63 | - if @merge_request && @merge_request.source_project | 68 | - if @merge_request && @merge_request.source_project |
| 64 | = link_to project_edit_tree_path(@merge_request.source_project, tree_join(@merge_request.source_branch, diff.new_path), from_merge_request_id: @merge_request.id), { class: 'btn btn-small' } do | 69 | = link_to project_edit_tree_path(@merge_request.source_project, tree_join(@merge_request.source_branch, diff.new_path), from_merge_request_id: @merge_request.id), { class: 'btn btn-small' } do |
| 65 | Edit | 70 | Edit |
app/views/projects/notes/_diff_notes_with_reply.html.haml
| 1 | - note = notes.first # example note | 1 | - note = notes.first # example note |
| 2 | -# Check if line want not changed since comment was left | 2 | -# Check if line want not changed since comment was left |
| 3 | - if !defined?(line) || line == note.diff_line | 3 | - if !defined?(line) || line == note.diff_line |
| 4 | - %tr.notes_holder | 4 | + %tr.notes_holder.js-toggle-content |
| 5 | %td.notes_line{ colspan: 2 } | 5 | %td.notes_line{ colspan: 2 } |
| 6 | %span.btn.disabled | 6 | %span.btn.disabled |
| 7 | %i.icon-comment | 7 | %i.icon-comment |
app/views/projects/notes/_diff_notes_with_reply_parallel.html.haml
| 1 | - note1 = notes1.first # example note | 1 | - note1 = notes1.first # example note |
| 2 | - note2 = notes2.first # example note | 2 | - note2 = notes2.first # example note |
| 3 | -%tr.notes_holder | 3 | +%tr.notes_holder.js-toggle-content |
| 4 | -# Check if line want not changed since comment was left | 4 | -# Check if line want not changed since comment was left |
| 5 | /- if !defined?(line1) || line1 == note1.diff_line | 5 | /- if !defined?(line1) || line1 == note1.diff_line |
| 6 | - if note1 | 6 | - if note1 |
features/project/merge_requests.feature
| @@ -95,3 +95,47 @@ Feature: Project Merge Requests | @@ -95,3 +95,47 @@ Feature: Project Merge Requests | ||
| 95 | Given I visit merge request page "Bug NS-04" | 95 | Given I visit merge request page "Bug NS-04" |
| 96 | And I leave a comment with a header containing "Comment with a header" | 96 | And I leave a comment with a header containing "Comment with a header" |
| 97 | Then The comment with the header should not have an ID | 97 | Then The comment with the header should not have an ID |
| 98 | + | ||
| 99 | + # Toggling inline comments | ||
| 100 | + | ||
| 101 | + @javascript | ||
| 102 | + Scenario: I hide comments on a merge request diff with comments in a single file | ||
| 103 | + Given project "Shop" have "Bug NS-05" open merge request with diffs inside | ||
| 104 | + And I visit merge request page "Bug NS-05" | ||
| 105 | + And I switch to the diff tab | ||
| 106 | + And I leave a comment like "Line is wrong" on line 39 of the second file | ||
| 107 | + And I click link "Hide inline discussion" of the second file | ||
| 108 | + Then I should not see a comment like "Line is wrong" in the second file | ||
| 109 | + | ||
| 110 | + @javascript | ||
| 111 | + Scenario: I show comments on a merge request diff with comments in a single file | ||
| 112 | + Given project "Shop" have "Bug NS-05" open merge request with diffs inside | ||
| 113 | + And I visit merge request page "Bug NS-05" | ||
| 114 | + And I switch to the diff tab | ||
| 115 | + And I leave a comment like "Line is wrong" on line 39 of the second file | ||
| 116 | + And I click link "Hide inline discussion" of the second file | ||
| 117 | + And I click link "Show inline discussion" of the second file | ||
| 118 | + Then I should see a comment like "Line is wrong" in the second file | ||
| 119 | + | ||
| 120 | + @javascript | ||
| 121 | + Scenario: I hide comments on a merge request diff with comments in multiple files | ||
| 122 | + Given project "Shop" have "Bug NS-05" open merge request with diffs inside | ||
| 123 | + And I visit merge request page "Bug NS-05" | ||
| 124 | + And I switch to the diff tab | ||
| 125 | + And I leave a comment like "Line is correct" on line 12 of the first file | ||
| 126 | + And I leave a comment like "Line is wrong" on line 39 of the second file | ||
| 127 | + And I click link "Hide inline discussion" of the second file | ||
| 128 | + Then I should not see a comment like "Line is wrong" in the second file | ||
| 129 | + And I should still see a comment like "Line is correct" in the first file | ||
| 130 | + | ||
| 131 | + @javascript | ||
| 132 | + Scenario: I show comments on a merge request diff with comments in multiple files | ||
| 133 | + Given project "Shop" have "Bug NS-05" open merge request with diffs inside | ||
| 134 | + And I visit merge request page "Bug NS-05" | ||
| 135 | + And I switch to the diff tab | ||
| 136 | + And I leave a comment like "Line is correct" on line 12 of the first file | ||
| 137 | + And I leave a comment like "Line is wrong" on line 39 of the second file | ||
| 138 | + And I click link "Hide inline discussion" of the second file | ||
| 139 | + And I click link "Show inline discussion" of the second file | ||
| 140 | + Then I should see a comment like "Line is wrong" in the second file | ||
| 141 | + And I should still see a comment like "Line is correct" in the first file |
features/steps/project/merge_requests.rb
| @@ -182,6 +182,62 @@ class ProjectMergeRequests < Spinach::FeatureSteps | @@ -182,6 +182,62 @@ class ProjectMergeRequests < Spinach::FeatureSteps | ||
| 182 | end | 182 | end |
| 183 | end | 183 | end |
| 184 | 184 | ||
| 185 | + step 'I click link "Hide inline discussion" of the second file' do | ||
| 186 | + within '.files [id^=diff]:nth-child(2)' do | ||
| 187 | + click_link "Diff comments" | ||
| 188 | + end | ||
| 189 | + end | ||
| 190 | + | ||
| 191 | + step 'I click link "Show inline discussion" of the second file' do | ||
| 192 | + within '.files [id^=diff]:nth-child(2)' do | ||
| 193 | + click_link "Diff comments" | ||
| 194 | + end | ||
| 195 | + end | ||
| 196 | + | ||
| 197 | + step 'I should not see a comment like "Line is wrong" in the second file' do | ||
| 198 | + within '.files [id^=diff]:nth-child(2)' do | ||
| 199 | + page.should_not have_visible_content "Line is wrong" | ||
| 200 | + end | ||
| 201 | + end | ||
| 202 | + | ||
| 203 | + step 'I should see a comment like "Line is wrong" in the second file' do | ||
| 204 | + within '.files [id^=diff]:nth-child(2) .note-text' do | ||
| 205 | + page.should have_visible_content "Line is wrong" | ||
| 206 | + end | ||
| 207 | + end | ||
| 208 | + | ||
| 209 | + step 'I leave a comment like "Line is correct" on line 12 of the first file' do | ||
| 210 | + init_diff_note_first_file | ||
| 211 | + | ||
| 212 | + within(".js-discussion-note-form") do | ||
| 213 | + fill_in "note_note", with: "Line is correct" | ||
| 214 | + click_button "Add Comment" | ||
| 215 | + end | ||
| 216 | + | ||
| 217 | + within ".files [id^=diff]:nth-child(1) .note-text" do | ||
| 218 | + page.should have_content "Line is correct" | ||
| 219 | + end | ||
| 220 | + end | ||
| 221 | + | ||
| 222 | + step 'I leave a comment like "Line is wrong" on line 39 of the second file' do | ||
| 223 | + init_diff_note_second_file | ||
| 224 | + | ||
| 225 | + within(".js-discussion-note-form") do | ||
| 226 | + fill_in "note_note", with: "Line is wrong" | ||
| 227 | + click_button "Add Comment" | ||
| 228 | + end | ||
| 229 | + | ||
| 230 | + within ".files [id^=diff]:nth-child(2) .note-text" do | ||
| 231 | + page.should have_content "Line is wrong" | ||
| 232 | + end | ||
| 233 | + end | ||
| 234 | + | ||
| 235 | + step 'I should still see a comment like "Line is correct" in the first file' do | ||
| 236 | + within '.files [id^=diff]:nth-child(1) .note-text' do | ||
| 237 | + page.should have_visible_content "Line is correct" | ||
| 238 | + end | ||
| 239 | + end | ||
| 240 | + | ||
| 185 | def project | 241 | def project |
| 186 | @project ||= Project.find_by!(name: "Shop") | 242 | @project ||= Project.find_by!(name: "Shop") |
| 187 | end | 243 | end |
| @@ -204,4 +260,16 @@ class ProjectMergeRequests < Spinach::FeatureSteps | @@ -204,4 +260,16 @@ class ProjectMergeRequests < Spinach::FeatureSteps | ||
| 204 | page.should have_content message | 260 | page.should have_content message |
| 205 | end | 261 | end |
| 206 | end | 262 | end |
| 263 | + | ||
| 264 | + def init_diff_note_first_file | ||
| 265 | + find('a[data-line-code="a5cc2925ca8258af241be7e5b0381edf30266302_12_12"]').click | ||
| 266 | + end | ||
| 267 | + | ||
| 268 | + def init_diff_note_second_file | ||
| 269 | + find('a[data-line-code="8ec9a00bfd09b3190ac6b22251dbb1aa95a0579d_28_39"]').click | ||
| 270 | + end | ||
| 271 | + | ||
| 272 | + def have_visible_content (text) | ||
| 273 | + have_css("*", text: text, visible: true) | ||
| 274 | + end | ||
| 207 | end | 275 | end |