From 5a07b760dff04660d9c7da84852c710b1fc2f786 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Fri, 12 Aug 2016 20:17:18 -0500 Subject: Refactor slash command definition --- app/controllers/projects_controller.rb | 20 +++++++++-- app/services/issuable_base_service.rb | 7 ++-- app/services/notes/create_service.rb | 6 ++-- app/services/notes/slash_commands_service.rb | 11 +++--- app/services/projects/autocomplete_service.rb | 46 ++++++++++-------------- app/services/projects/participants_service.rb | 41 +++++++-------------- app/services/slash_commands/interpret_service.rb | 24 ++++++++----- 7 files changed, 78 insertions(+), 77 deletions(-) (limited to 'app') diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 9c387fd3daa..93338dba51e 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -134,8 +134,22 @@ class ProjectsController < Projects::ApplicationController end def autocomplete_sources - autocomplete = ::Projects::AutocompleteService.new(@project, current_user, params) - participants = ::Projects::ParticipantsService.new(@project, current_user, params).execute + noteable = + case params[:type] + when 'Issue' + IssuesFinder.new(current_user, project_id: project.id, state: 'all'). + execute.find_by(iid: params[:type_id]) + when 'MergeRequest' + MergeRequestsFinder.new(current_user, project_id: project.id, state: 'all'). + execute.find_by(iid: params[:type_id]) + when 'Commit' + project.commit(params[:type_id]) + else + nil + end + + autocomplete = ::Projects::AutocompleteService.new(@project, current_user) + participants = ::Projects::ParticipantsService.new(@project, current_user).execute(noteable) @suggestions = { emojis: Gitlab::AwardEmoji.urls, @@ -144,7 +158,7 @@ class ProjectsController < Projects::ApplicationController mergerequests: autocomplete.merge_requests, labels: autocomplete.labels, members: participants, - commands: autocomplete.commands + commands: autocomplete.commands(noteable, params[:type]) } respond_to do |format| diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index 1a01b333366..aa08eef081c 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -94,10 +94,13 @@ class IssuableBaseService < BaseService end def merge_slash_commands_into_params!(issuable) - commands = SlashCommands::InterpretService.new(project, current_user). + description, command_params = + SlashCommands::InterpretService.new(project, current_user). execute(params[:description], issuable) - params.merge!(commands) + params[:description] = description + + params.merge!(command_params) end def create_issuable(issuable, attributes) diff --git a/app/services/notes/create_service.rb b/app/services/notes/create_service.rb index 1b2d63034b8..f7cf4a8edc0 100644 --- a/app/services/notes/create_service.rb +++ b/app/services/notes/create_service.rb @@ -15,7 +15,9 @@ module Notes # **before** we save the note because if the note consists of commands # only, there is no need be create a note! slash_commands_service = SlashCommandsService.new(project, current_user) - commands = slash_commands_service.extract_commands(note) + content, command_params = slash_commands_service.extract_commands(note) + + note.note = content if note.save # Finish the harder work in the background @@ -25,7 +27,7 @@ module Notes # We must add the error after we call #save because errors are reset # when #save is called - if slash_commands_service.execute(commands, note) && note.note.blank? + if slash_commands_service.execute(command_params, note) && note.note.blank? note.errors.add(:commands_only, 'Your commands are being executed.') end diff --git a/app/services/notes/slash_commands_service.rb b/app/services/notes/slash_commands_service.rb index ebced9577d8..f2c43775b72 100644 --- a/app/services/notes/slash_commands_service.rb +++ b/app/services/notes/slash_commands_service.rb @@ -1,6 +1,5 @@ module Notes class SlashCommandsService < BaseService - UPDATE_SERVICES = { 'Issue' => Issues::UpdateService, 'MergeRequest' => MergeRequests::UpdateService @@ -15,11 +14,11 @@ module Notes execute(note.note, note.noteable) end - def execute(commands, note) - if commands.any? - @noteable_update_service.new(project, current_user, commands). - execute(note.noteable) - end + def execute(command_params, note) + return if command_params.empty? + + @noteable_update_service.new(project, current_user, command_params). + execute(note.noteable) end end end diff --git a/app/services/projects/autocomplete_service.rb b/app/services/projects/autocomplete_service.rb index 477c999eff4..cb85ee6694d 100644 --- a/app/services/projects/autocomplete_service.rb +++ b/app/services/projects/autocomplete_service.rb @@ -1,7 +1,7 @@ module Projects class AutocompleteService < BaseService def issues - @project.issues.visible_to_user(current_user).opened.select([:iid, :title]) + IssuesFinder.new(current_user, project_id: project.id, state: 'opened').execute.select([:iid, :title]) end def milestones @@ -9,42 +9,34 @@ module Projects end def merge_requests - @project.merge_requests.opened.select([:iid, :title]) + MergeRequestsFinder.new(current_user, project_id: project.id, state: 'opened').execute.select([:iid, :title]) end def labels @project.labels.select([:title, :color]) end - def commands - # We don't return commands when editing an issue or merge request - # This should be improved by not enabling autocomplete at the JS-level - # following this suggestion: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5021#note_13837384 - return [] if !target || %w[edit update].include?(params[:action_name]) + def commands(noteable, type) + noteable ||= + case type + when 'Issue' + @project.issues.build + when 'MergeRequest' + @project.merge_requests.build + end - SlashCommands::InterpretService.command_definitions( + return [] unless noteable && noteable.is_a?(Issuable) + + opts = { project: project, - noteable: target, + noteable: noteable, current_user: current_user - ) - end + } + SlashCommands::InterpretService.command_definitions.map do |definition| + next unless definition.available?(opts) - private - - def target - @target ||= begin - noteable_id = params[:type_id] - case params[:type] - when 'Issue' - IssuesFinder.new(current_user, project_id: project.id, state: 'all'). - execute.find_or_initialize_by(iid: noteable_id) - when 'MergeRequest' - MergeRequestsFinder.new(current_user, project_id: project.id, state: 'all'). - execute.find_or_initialize_by(iid: noteable_id) - else - nil - end - end + definition.to_h(opts) + end.compact end end end diff --git a/app/services/projects/participants_service.rb b/app/services/projects/participants_service.rb index 1c8f2913e8b..d38328403c1 100644 --- a/app/services/projects/participants_service.rb +++ b/app/services/projects/participants_service.rb @@ -1,45 +1,28 @@ module Projects class ParticipantsService < BaseService - attr_reader :noteable_type, :noteable_id - - def execute - @noteable_type = params[:type] - @noteable_id = params[:type_id] + attr_reader :noteable + + def execute(noteable) + @noteable = noteable project_members = sorted(project.team.members) - participants = target_owner + participants_in_target + all_members + groups + project_members + participants = noteable_owner + participants_in_noteable + all_members + groups + project_members participants.uniq end - def target - @target ||= - case noteable_type - when 'Issue' - IssuesFinder.new(current_user, project_id: project.id, state: 'all'). - execute.find_by(iid: noteable_id) - when 'MergeRequest' - MergeRequestsFinder.new(current_user, project_id: project.id, state: 'all'). - execute.find_by(iid: noteable_id) - when 'Commit' - project.commit(noteable_id) - else - nil - end - end - - def target_owner - return [] unless target && target.author.present? + def noteable_owner + return [] unless noteable && noteable.author.present? [{ - name: target.author.name, - username: target.author.username + name: noteable.author.name, + username: noteable.author.username }] end - def participants_in_target - return [] unless target + def participants_in_noteable + return [] unless noteable - users = target.participants(current_user) + users = noteable.participants(current_user) sorted(users) end diff --git a/app/services/slash_commands/interpret_service.rb b/app/services/slash_commands/interpret_service.rb index 112bebe423a..a2b92d70f9f 100644 --- a/app/services/slash_commands/interpret_service.rb +++ b/app/services/slash_commands/interpret_service.rb @@ -10,20 +10,28 @@ module SlashCommands @noteable = noteable @updates = {} - commands = extractor(noteable: noteable).extract_commands!(content) - commands.each do |command, *args| - execute_command(command, *args) + opts = { + noteable: noteable, + current_user: current_user, + project: project + } + + content, commands = extractor.extract_commands(content, opts) + + commands.each do |name, *args| + definition = self.class.command_definitions_by_name[name.to_sym] + next unless definition + + definition.execute(self, opts, *args) end - @updates + [content, @updates] end private - def extractor(opts = {}) - opts.merge!(current_user: current_user, project: project) - - Gitlab::SlashCommands::Extractor.new(self.class.command_names(opts)) + def extractor + Gitlab::SlashCommands::Extractor.new(self.class.command_definitions) end desc do -- cgit v1.2.3