From 3970640b48fe9647ad97cf795aa2bb44a81d21a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Thu, 15 Sep 2016 16:57:06 +0200 Subject: [PATCH 1/3] Fix note form hint showing slash commands supported for commits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- CHANGELOG | 1 + app/helpers/notes_helper.rb | 4 ++++ app/services/notes/slash_commands_service.rb | 20 +++++++++++--------- app/views/projects/notes/_form.html.haml | 6 ++++-- spec/features/projects/commits/note_spec.rb | 16 ++++++++++++++++ 5 files changed, 36 insertions(+), 11 deletions(-) create mode 100644 spec/features/projects/commits/note_spec.rb diff --git a/CHANGELOG b/CHANGELOG index bc19048e94b..78f7e8fe8f9 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -14,6 +14,7 @@ v 8.12.0 (unreleased) - Filter tags by name !6121 - Update gitlab shell secret file also when it is empty. !3774 (glensc) - Give project selection dropdowns responsive width, make non-wrapping. + - Fix note form hint showing slash commands supported for commits. - Make push events have equal vertical spacing. - API: Ensure invitees are not returned in Members API. - Add two-factor recovery endpoint to internal API !5510 diff --git a/app/helpers/notes_helper.rb b/app/helpers/notes_helper.rb index da230f76bae..b0331f36a2f 100644 --- a/app/helpers/notes_helper.rb +++ b/app/helpers/notes_helper.rb @@ -10,6 +10,10 @@ module NotesHelper Ability.can_edit_note?(current_user, note) end + def note_supports_slash_commands?(note) + Notes::SlashCommandsService.supported?(note, current_user) + end + def noteable_json(noteable) { id: noteable.id, diff --git a/app/services/notes/slash_commands_service.rb b/app/services/notes/slash_commands_service.rb index 4a9a8a64653..a14898920a1 100644 --- a/app/services/notes/slash_commands_service.rb +++ b/app/services/notes/slash_commands_service.rb @@ -5,9 +5,17 @@ module Notes 'MergeRequest' => MergeRequests::UpdateService } - def supported?(note) + def self.noteable_update_service(note) + UPDATE_SERVICES[note.noteable_type] + end + + def self.supported?(note, current_user) noteable_update_service(note) && - can?(current_user, :"update_#{note.noteable_type.underscore}", note.noteable) + current_user.can?(:"update_#{note.noteable_type.underscore}", note.noteable) + end + + def supported?(note) + self.class.supported?(note, current_user) end def extract_commands(note) @@ -21,13 +29,7 @@ module Notes return if command_params.empty? return unless supported?(note) - noteable_update_service(note).new(project, current_user, command_params).execute(note.noteable) - end - - private - - def noteable_update_service(note) - UPDATE_SERVICES[note.noteable_type] + self.class.noteable_update_service(note).new(project, current_user, command_params).execute(note.noteable) end end end diff --git a/app/views/projects/notes/_form.html.haml b/app/views/projects/notes/_form.html.haml index 402f5b52f5e..46b402545cd 100644 --- a/app/views/projects/notes/_form.html.haml +++ b/app/views/projects/notes/_form.html.haml @@ -1,3 +1,5 @@ +- supports_slash_commands = note_supports_slash_commands?(@note) + = form_for [@project.namespace.becomes(Namespace), @project, @note], remote: true, html: { :'data-type' => 'json', multipart: true, id: nil, class: "new-note js-new-note-form js-quick-submit common-note-form", "data-noteable-iid" => @note.noteable.try(:iid), }, authenticity_token: true do |f| = hidden_field_tag :view, diff_view = hidden_field_tag :line_type @@ -14,8 +16,8 @@ attr: :note, classes: 'note-textarea js-note-text', placeholder: "Write a comment or drag your files here...", - supports_slash_commands: true - = render 'projects/notes/hints', supports_slash_commands: true + supports_slash_commands: supports_slash_commands + = render 'projects/notes/hints', supports_slash_commands: supports_slash_commands .error-alert .note-form-actions.clearfix diff --git a/spec/features/projects/commits/note_spec.rb b/spec/features/projects/commits/note_spec.rb new file mode 100644 index 00000000000..bc42b63c371 --- /dev/null +++ b/spec/features/projects/commits/note_spec.rb @@ -0,0 +1,16 @@ +require 'spec_helper' + +describe 'Projects > Commits > Note' do + let(:project) { create(:project) } + let(:commit) { project.commit('7d3b0f7cff5f37573aea97cebfd5692ea1689924') } + + before do + login_as :user + project.team << [@user, :master] + visit namespace_project_commit_path(project.namespace, project, commit.id) + end + + it 'says that only markdown is supported, not slash commands' do + expect(page).to have_content('Styling with Markdown is supported') + end +end From 929ff01ac08db320402c31bb70b463007d1b379d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Tue, 20 Sep 2016 10:23:13 +0200 Subject: [PATCH 2/3] Ensure we have a user before checking for their permission in Notes::SlashCommandsService MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- app/services/notes/slash_commands_service.rb | 1 + .../notes/slash_commands_service_spec.rb | 69 +++++++++++++++++++ 2 files changed, 70 insertions(+) diff --git a/app/services/notes/slash_commands_service.rb b/app/services/notes/slash_commands_service.rb index a14898920a1..2edbd39a9e7 100644 --- a/app/services/notes/slash_commands_service.rb +++ b/app/services/notes/slash_commands_service.rb @@ -11,6 +11,7 @@ module Notes def self.supported?(note, current_user) noteable_update_service(note) && + current_user && current_user.can?(:"update_#{note.noteable_type.underscore}", note.noteable) end diff --git a/spec/services/notes/slash_commands_service_spec.rb b/spec/services/notes/slash_commands_service_spec.rb index 4f231aab161..d1099884a02 100644 --- a/spec/services/notes/slash_commands_service_spec.rb +++ b/spec/services/notes/slash_commands_service_spec.rb @@ -122,6 +122,75 @@ describe Notes::SlashCommandsService, services: true do end end + describe '.noteable_update_service' do + include_context 'note on noteable' + + it 'returns Issues::UpdateService for a note on an issue' do + note = create(:note_on_issue, project: project) + + expect(described_class.noteable_update_service(note)).to eq(Issues::UpdateService) + end + + it 'returns Issues::UpdateService for a note on a merge request' do + note = create(:note_on_merge_request, project: project) + + expect(described_class.noteable_update_service(note)).to eq(MergeRequests::UpdateService) + end + + it 'returns nil for a note on a commit' do + note = create(:note_on_commit, project: project) + + expect(described_class.noteable_update_service(note)).to be_nil + end + end + + describe '.supported?' do + include_context 'note on noteable' + + let(:note) { create(:note_on_issue, project: project) } + + context 'with no current_user' do + it 'returns false' do + expect(described_class.supported?(note, nil)).to be_falsy + end + end + + context 'when current_user cannot update the noteable' do + it 'returns false' do + user = create(:user) + + expect(described_class.supported?(note, user)).to be_falsy + end + end + + context 'when current_user can update the noteable' do + it 'returns true' do + expect(described_class.supported?(note, master)).to be_truthy + end + + context 'with a note on a commit' do + let(:note) { create(:note_on_commit, project: project) } + + it 'returns false' do + expect(described_class.supported?(note, nil)).to be_falsy + end + end + end + end + + describe '#supported?' do + include_context 'note on noteable' + + it 'delegates to the class method' do + service = described_class.new(project, master) + note = create(:note_on_issue, project: project) + + expect(described_class).to receive(:supported?).with(note, master) + + service.supported?(note) + end + end + describe '#execute' do let(:service) { described_class.new(project, master) } From f6e5cc3c37f83044ca06d82aacf8ccb2796df724 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Tue, 20 Sep 2016 10:45:00 +0200 Subject: [PATCH 3/3] Add a view spec for projects/notes/_form MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- spec/features/projects/commits/note_spec.rb | 16 --------- .../projects/notes/_form.html.haml_spec.rb | 36 +++++++++++++++++++ 2 files changed, 36 insertions(+), 16 deletions(-) delete mode 100644 spec/features/projects/commits/note_spec.rb create mode 100644 spec/views/projects/notes/_form.html.haml_spec.rb diff --git a/spec/features/projects/commits/note_spec.rb b/spec/features/projects/commits/note_spec.rb deleted file mode 100644 index bc42b63c371..00000000000 --- a/spec/features/projects/commits/note_spec.rb +++ /dev/null @@ -1,16 +0,0 @@ -require 'spec_helper' - -describe 'Projects > Commits > Note' do - let(:project) { create(:project) } - let(:commit) { project.commit('7d3b0f7cff5f37573aea97cebfd5692ea1689924') } - - before do - login_as :user - project.team << [@user, :master] - visit namespace_project_commit_path(project.namespace, project, commit.id) - end - - it 'says that only markdown is supported, not slash commands' do - expect(page).to have_content('Styling with Markdown is supported') - end -end diff --git a/spec/views/projects/notes/_form.html.haml_spec.rb b/spec/views/projects/notes/_form.html.haml_spec.rb new file mode 100644 index 00000000000..932d6756ad2 --- /dev/null +++ b/spec/views/projects/notes/_form.html.haml_spec.rb @@ -0,0 +1,36 @@ +require 'spec_helper' + +describe 'projects/notes/_form' do + include Devise::TestHelpers + + let(:user) { create(:user) } + let(:project) { create(:empty_project) } + + before do + project.team << [user, :master] + assign(:project, project) + assign(:note, note) + + allow(view).to receive(:current_user).and_return(user) + + render + end + + %w[issue merge_request].each do |noteable| + context "with a note on #{noteable}" do + let(:note) { build(:"note_on_#{noteable}", project: project) } + + it 'says that only markdown is supported, not slash commands' do + expect(rendered).to have_content('Styling with Markdown and slash commands are supported') + end + end + end + + context 'with a note on a commit' do + let(:note) { build(:note_on_commit, project: project) } + + it 'says that only markdown is supported, not slash commands' do + expect(rendered).to have_content('Styling with Markdown is supported') + end + end +end