diff --git a/CHANGELOG b/CHANGELOG index d7cdfe873cc..97e1c3b15c4 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..2edbd39a9e7 100644 --- a/app/services/notes/slash_commands_service.rb +++ b/app/services/notes/slash_commands_service.rb @@ -5,9 +5,18 @@ 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 && + 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 +30,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/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) } 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