Merge branch 'add-more-actions-report-as-abuse-for-notes' into 'master'

Added more actions and report as abuse to all notes

See merge request !11352

app/assets/stylesheets/framework/dropdowns.scss

@@ -201,6 +201,11 @@
     width: 100%;
   }
 
+  &.dropdown-open-left {
+    right: 0;
+    left: auto;
+  }
+
   &.is-loading {
     .dropdown-content {
       display: none;

app/assets/stylesheets/pages/notes.scss

@@ -38,9 +38,12 @@ ul.notes {
   }
 
   .discussion {
-    overflow: hidden;
     display: block;
     position: relative;
+
+    .diff-content {
+      overflow: visible;
+    }
   }
 
   > li {
@@ -443,6 +446,53 @@ ul.notes {
   .note-action-button {
     margin-left: 8px;
   }
+
+  .more-actions-toggle {
+    margin-left: 2px;
+  }
+}
+
+.more-actions {
+  display: inline;
+
+  .tooltip {
+    white-space: nowrap;
+  }
+}
+
+.more-actions-toggle {
+  padding: 0;
+  outline: none;
+
+  &:hover .icon,
+  &:focus .icon {
+    color: $blue-600;
+  }
+
+  .icon {
+    padding: 0 6px;
+  }
+}
+
+.more-actions-dropdown {
+  width: 180px;
+  min-width: 180px;
+  margin-top: $gl-btn-padding;
+
+  li > a,
+  li > .btn {
+    color: $gl-text-color;
+    padding: $gl-btn-padding;
+    width: 100%;
+    text-align: left;
+
+    &:hover,
+    &:focus {
+      color: $gl-text-color;
+      background-color: $blue-25;
+      border-radius: $border-radius-default;
+    }
+  }
 }
 
 .discussion-actions {

app/helpers/notes_helper.rb

@@ -90,14 +90,18 @@ module NotesHelper
     end
   end
 
-  def note_url(note)
+  def note_url(note, project = @project)
     if note.noteable.is_a?(PersonalSnippet)
       snippet_note_path(note.noteable, note)
     else
-      namespace_project_note_path(@project.namespace, @project, note)
+      namespace_project_note_path(project.namespace, project, note)
     end
   end
 
+  def noteable_note_url(note)
+    Gitlab::UrlBuilder.build(note)
+  end
+
   def form_resources
     if @snippet.is_a?(PersonalSnippet)
       [@note]

app/views/projects/notes/_actions.html.haml

@@ -37,8 +37,4 @@
       %span{ class: 'link-highlight award-control-icon-positive' }= custom_icon('emoji_smiley')
       %span{ class: 'link-highlight award-control-icon-super-positive' }= custom_icon('emoji_smile')
 
-  - if note_editable
-    = link_to '#', title: 'Edit comment', class: 'note-action-button js-note-edit has-tooltip' do
-      = icon('pencil', class: 'link-highlight')
-    = link_to namespace_project_note_path(note.project.namespace, note.project, note), title: 'Remove comment', method: :delete, data: { confirm: 'Are you sure you want to remove this comment?' }, remote: true, class: 'note-action-button js-note-delete danger has-tooltip' do
-      = icon('trash-o', class: 'danger-highlight')
+    = render 'projects/notes/more_actions_dropdown', note: note, note_editable: note_editable

app/views/projects/notes/_more_actions_dropdown.html.haml

+.dropdown.more-actions
+  = button_tag title: 'More actions', class: 'note-action-button more-actions-toggle has-tooltip btn btn-transparent', data: { toggle: 'dropdown', container: 'body' } do
+    = icon('ellipsis-v', class: 'icon')
+  %ul.dropdown-menu.more-actions-dropdown.dropdown-open-left
+    %li
+      = button_tag 'Edit comment', class: 'js-note-edit btn btn-transparent'
+    %li.divider
+      %li
+        = link_to new_abuse_report_path(user_id: note.author.id, ref_url: noteable_note_url(note)) do
+          Report as abuse
+    - if note_editable
+      %li
+        = link_to note_url(note), method: :delete, data: { confirm: 'Are you sure you want to delete this comment?' }, remote: true, class: 'js-note-delete' do
+          %span.text-danger Delete comment

app/views/snippets/notes/_actions.html.haml

@@ -6,8 +6,5 @@
       %span{ class: 'link-highlight award-control-icon-neutral' }= custom_icon('emoji_slightly_smiling_face')
       %span{ class: 'link-highlight award-control-icon-positive' }= custom_icon('emoji_smiley')
       %span{ class: 'link-highlight award-control-icon-super-positive' }= custom_icon('emoji_smile')
-  - if note_editable
-    = link_to '#', title: 'Edit comment', class: 'note-action-button js-note-edit has-tooltip' do
-      = icon('pencil', class: 'link-highlight')
-    = link_to snippet_note_path(note.noteable, note), title: 'Remove comment', method: :delete, data: { confirm: 'Are you sure you want to remove this comment?' }, remote: true, class: 'note-action-button js-note-delete danger has-tooltip' do
-      = icon('trash-o', class: 'danger-highlight')
+
+  = render 'projects/notes/more_actions_dropdown', note: note, note_editable: note_editable

features/steps/project/merge_requests.rb

@@ -297,6 +297,9 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps
 
   step 'I change the comment "Line is wrong" to "Typo, please fix" on diff' do
     page.within('.diff-file:nth-of-type(5) .note') do
+      find('.more-actions').click
+      find('.more-actions .dropdown-menu li', match: :first)
+
       find('.js-note-edit').click
 
       page.within('.current-note-edit-form', visible: true) do
@@ -322,6 +325,9 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps
 
   step 'I delete the comment "Line is wrong" on diff' do
     page.within('.diff-file:nth-of-type(5) .note') do
+      find('.more-actions').click
+      find('.more-actions .dropdown-menu li', match: :first)
+
       find('.js-note-delete').click
     end
   end

features/steps/shared/note.rb

@@ -8,7 +8,12 @@ module SharedNote
 
   step 'I delete a comment' do
     page.within('.main-notes-list') do
-      find('.note').hover
+      note = find('.note')
+      note.hover
+
+      note.find('.more-actions').click
+      note.find('.more-actions .dropdown-menu li', match: :first)
+
       find(".js-note-delete").click
     end
   end
@@ -139,8 +144,13 @@ module SharedNote
 
   step 'I edit the last comment with a +1' do
     page.within(".main-notes-list") do
-      find(".note").hover
-      find('.js-note-edit').click
+      note = find('.note')
+      note.hover
+
+      note.find('.more-actions').click
+      note.find('.more-actions .dropdown-menu li', match: :first)
+
+      note.find('.js-note-edit').click
     end
 
     page.within(".current-note-edit-form") do

lib/gitlab/url_builder.rb

@@ -61,7 +61,12 @@ module Gitlab
 
       elsif object.for_snippet?
         snippet = Snippet.find(object.noteable_id)
-        project_snippet_url(snippet, anchor: dom_id(object))
+
+        if snippet.is_a?(PersonalSnippet)
+          snippet_url(snippet, anchor: dom_id(object))
+        else
+          project_snippet_url(snippet, anchor: dom_id(object))
+        end
       end
     end
 

spec/features/issues/note_polling_spec.rb

 require 'spec_helper'
 
 feature 'Issue notes polling', :feature, :js do
+  include NoteInteractionHelpers
+
   let(:project) { create(:empty_project, :public) }
   let(:issue) { create(:issue, project: project) }
 
@@ -48,7 +50,7 @@ feature 'Issue notes polling', :feature, :js do
       end
 
       it 'when editing but have not changed anything, and an update comes in, show the updated content in the textarea' do
-        find("#note_#{existing_note.id} .js-note-edit").click
+        click_edit_action(existing_note)
 
         expect(page).to have_field("note[note]", with: note_text)
 
@@ -58,19 +60,18 @@ feature 'Issue notes polling', :feature, :js do
       end
 
       it 'when editing but you changed some things, and an update comes in, show a warning' do
-        find("#note_#{existing_note.id} .js-note-edit").click
+        click_edit_action(existing_note)
 
         expect(page).to have_field("note[note]", with: note_text)
 
         find("#note_#{existing_note.id} .js-note-text").set('something random')
-
         update_note(existing_note, updated_text)
 
         expect(page).to have_selector(".alert")
       end
 
       it 'when editing but you changed some things, an update comes in, and you press cancel, show the updated content' do
-        find("#note_#{existing_note.id} .js-note-edit").click
+        click_edit_action(existing_note)
 
         expect(page).to have_field("note[note]", with: note_text)
 
@@ -128,4 +129,12 @@ feature 'Issue notes polling', :feature, :js do
     note.update(note: new_text)
     page.execute_script('notes.refresh();')
   end
+
+  def click_edit_action(note)
+    note_element = find("#note_#{note.id}")
+
+    open_more_actions_dropdown(note)
+
+    note_element.find('.js-note-edit').click
+  end
 end

spec/features/merge_requests/diff_notes_avatars_spec.rb

 require 'spec_helper'
 
 feature 'Diff note avatars', feature: true, js: true do
+  include NoteInteractionHelpers
+
   let(:user)          { create(:user) }
   let(:project)       { create(:project, :public) }
   let(:merge_request) { create(:merge_request_with_diffs, source_project: project, author: user, title: "Bug NS-04") }
@@ -110,6 +112,8 @@ feature 'Diff note avatars', feature: true, js: true do
       end
 
       it 'removes avatar when note is deleted' do
+        open_more_actions_dropdown(note)
+
         page.within find(".note-row-#{note.id}") do
           find('.js-note-delete').click
         end

spec/features/merge_requests/user_posts_notes_spec.rb

 require 'spec_helper'
 
 describe 'Merge requests > User posts notes', :js do
+  include NoteInteractionHelpers
+
   let(:project) { create(:project) }
   let(:merge_request) do
     create(:merge_request, source_project: project, target_project: project)
@@ -73,6 +75,8 @@ describe 'Merge requests > User posts notes', :js do
     describe 'editing the note' do
       before do
         find('.note').hover
+        open_more_actions_dropdown(note)
+
         find('.js-note-edit').click
       end
 
@@ -100,6 +104,8 @@ describe 'Merge requests > User posts notes', :js do
 
         wait_for_requests
         find('.note').hover
+        open_more_actions_dropdown(note)
+
         find('.js-note-edit').click
 
         page.within('.current-note-edit-form') do
@@ -126,6 +132,8 @@ describe 'Merge requests > User posts notes', :js do
     describe 'deleting an attachment' do
       before do
         find('.note').hover
+        open_more_actions_dropdown(note)
+
         find('.js-note-edit').click
       end
 

spec/features/reportable_note/commit_spec.rb

+require 'spec_helper'
+
+describe 'Reportable note on commit', :feature, :js do
+  include RepoHelpers
+
+  let(:user) { create(:user) }
+  let(:project) { create(:project) }
+
+  before do
+    project.add_master(user)
+    login_as user
+  end
+
+  context 'a normal note' do
+    let!(:note) { create(:note_on_commit, commit_id: sample_commit.id, project: project) }
+
+    before do
+      visit namespace_project_commit_path(project.namespace, project, sample_commit.id)
+    end
+
+    it_behaves_like 'reportable note'
+  end
+
+  context 'a diff note' do
+    let!(:note) { create(:diff_note_on_commit, commit_id: sample_commit.id, project: project) }
+
+    before do
+      visit namespace_project_commit_path(project.namespace, project, sample_commit.id)
+    end
+
+    it_behaves_like 'reportable note'
+  end
+end

spec/features/reportable_note/issue_spec.rb

+require 'spec_helper'
+
+describe 'Reportable note on issue', :feature, :js do
+  let(:user) { create(:user) }
+  let(:project) { create(:empty_project) }
+  let(:issue) { create(:issue, project: project) }
+  let!(:note) { create(:note_on_issue, noteable: issue, project: project) }
+
+  before do
+    project.add_master(user)
+    login_as user
+
+    visit namespace_project_issue_path(project.namespace, project, issue)
+  end
+
+  it_behaves_like 'reportable note'
+end

spec/features/reportable_note/merge_request_spec.rb

+require 'spec_helper'
+
+describe 'Reportable note on merge request', :feature, :js do
+  let(:user) { create(:user) }
+  let(:project) { create(:project) }
+  let(:merge_request) { create(:merge_request, source_project: project) }
+
+  before do
+    project.add_master(user)
+    login_as user
+
+    visit namespace_project_merge_request_path(project.namespace, project, merge_request)
+  end
+
+  context 'a normal note' do
+    let!(:note) { create(:note_on_merge_request, noteable: merge_request, project: project) }
+
+    it_behaves_like 'reportable note'
+  end
+
+  context 'a diff note' do
+    let!(:note) { create(:diff_note_on_merge_request, noteable: merge_request, project: project) }
+
+    it_behaves_like 'reportable note'
+  end
+end

spec/features/reportable_note/snippets_spec.rb

+require 'spec_helper'
+
+describe 'Reportable note on snippets', :feature, :js do
+  let(:user) { create(:user) }
+  let(:project) { create(:empty_project) }
+
+  before do
+    project.add_master(user)
+    login_as user
+  end
+
+  describe 'on project snippet' do
+    let(:snippet) { create(:project_snippet, :public, project: project, author: user) }
+    let!(:note) { create(:note_on_project_snippet, noteable: snippet, project: project) }
+
+    before do
+      visit namespace_project_snippet_path(project.namespace, project, snippet)
+    end
+
+    it_behaves_like 'reportable note'
+  end
+
+  describe 'on personal snippet' do
+    let(:snippet) { create(:personal_snippet, :public, author: user) }
+    let!(:note) { create(:note_on_personal_snippet, noteable: snippet, author: user) }
+
+    before do
+      visit snippet_path(snippet)
+    end
+
+    it_behaves_like 'reportable note'
+  end
+end

spec/features/snippets/notes_on_personal_snippets_spec.rb

 require 'spec_helper'
 
 describe 'Comments on personal snippets', :js, feature: true do
+  include NoteInteractionHelpers
+
   let!(:user)    { create(:user) }
   let!(:snippet) { create(:personal_snippet, :public) }
   let!(:snippet_notes) do
@@ -22,6 +24,8 @@ describe 'Comments on personal snippets', :js, feature: true do
     it 'contains notes for a snippet with correct action icons' do
       expect(page).to have_selector('#notes-list li', count: 2)
 
+      open_more_actions_dropdown(snippet_notes[0])
+
       # comment authored by current user
       page.within("#notes-list li#note_#{snippet_notes[0].id}") do
         expect(page).to have_content(snippet_notes[0].note)
@@ -29,6 +33,8 @@ describe 'Comments on personal snippets', :js, feature: true do
         expect(page).to have_selector('.note-emoji-button')
       end
 
+      open_more_actions_dropdown(snippet_notes[1])
+
       page.within("#notes-list li#note_#{snippet_notes[1].id}") do
         expect(page).to have_content(snippet_notes[1].note)
         expect(page).not_to have_selector('.js-note-delete')
@@ -68,6 +74,8 @@ describe 'Comments on personal snippets', :js, feature: true do
 
   context 'when editing a note' do
     it 'changes the text' do
+      open_more_actions_dropdown(snippet_notes[0])
+
       page.within("#notes-list li#note_#{snippet_notes[0].id}") do
         click_on 'Edit comment'
       end
@@ -89,8 +97,10 @@ describe 'Comments on personal snippets', :js, feature: true do
 
   context 'when deleting a note' do
     it 'removes the note from the snippet detail page' do
+      open_more_actions_dropdown(snippet_notes[0])
+
       page.within("#notes-list li#note_#{snippet_notes[0].id}") do
-        click_on 'Remove comment'
+        click_on 'Delete comment'
       end
 
       wait_for_requests

spec/helpers/notes_helper_spec.rb

@@ -256,4 +256,14 @@ describe NotesHelper do
       expect(helper.form_resources).to eq([@project.namespace, @project, @note])
     end
   end
+
+  describe '#noteable_note_url' do
+    let(:project) { create(:empty_project) }
+    let(:issue) { create(:issue, project: project) }
+    let(:note) { create(:note_on_issue, noteable: issue, project: project) }
+
+    it 'returns the noteable url with an anchor to the note' do
+      expect(noteable_note_url(note)).to match("/#{project.namespace.path}/#{project.path}/issues/#{issue.iid}##{dom_id(note)}")
+    end
+  end
 end

spec/lib/gitlab/url_builder_spec.rb

@@ -97,6 +97,17 @@ describe Gitlab::UrlBuilder, lib: true do
         end
       end
 
+      context 'on a PersonalSnippet' do
+        it 'returns a proper URL' do
+          personal_snippet = create(:personal_snippet)
+          note = build_stubbed(:note_on_personal_snippet, noteable: personal_snippet)
+
+          url = described_class.build(note)
+
+          expect(url).to eq "#{Settings.gitlab['url']}/snippets/#{note.noteable_id}#note_#{note.id}"
+        end
+      end
+
       context 'on another object' do
         it 'returns a proper URL' do
           project = build_stubbed(:empty_project)

spec/support/features/reportable_note_shared_examples.rb

+require 'spec_helper'
+
+shared_examples 'reportable note' do
+  include NotesHelper
+
+  let(:comment) { find("##{ActionView::RecordIdentifier.dom_id(note)}") }
+  let(:more_actions_selector) { '.more-actions.dropdown' }
+  let(:abuse_report_path) { new_abuse_report_path(user_id: note.author.id, ref_url: noteable_note_url(note)) }
+
+  it 'has a `More actions` dropdown' do
+    expect(comment).to have_selector(more_actions_selector)
+  end
+
+  it 'dropdown has Edit, Report and Delete links' do
+    dropdown = comment.find(more_actions_selector)
+
+    dropdown.click
+    dropdown.find('.dropdown-menu li', match: :first)
+
+    expect(dropdown).to have_button('Edit comment')
+    expect(dropdown).to have_link('Report as abuse', href: abuse_report_path)
+    expect(dropdown).to have_link('Delete comment', href: note_url(note, project))
+  end
+
+  it 'Report button links to a report page' do
+    dropdown = comment.find(more_actions_selector)
+
+    dropdown.click
+    dropdown.find('.dropdown-menu li', match: :first)
+
+    dropdown.click_link('Report as abuse')
+
+    expect(find('#user_name')['value']).to match(note.author.username)
+    expect(find('#abuse_report_message')['value']).to match(noteable_note_url(note))
+  end
+end

spec/support/helpers/note_interaction_helpers.rb

+module NoteInteractionHelpers
+  def open_more_actions_dropdown(note)
+    note_element = find("#note_#{note.id}")
+
+    note_element.find('.more-actions').click
+    note_element.find('.more-actions .dropdown-menu li', match: :first)
+  end
+end