Commit 3dd44f2b authored by Sean McGivern's avatar Sean McGivern

Merge branch '45663-tag-quick-action-on-commit-comments' into 'master'

Resolve "`/tag` quick action on Commit comments"

Closes #45663

See merge request gitlab-org/gitlab-ce!20694
parents d7c52101 7b87c75c
......@@ -32,13 +32,8 @@ class Projects::AutocompleteSourcesController < Projects::ApplicationController
end
def target
case params[:type]&.downcase
when 'issue'
IssuesFinder.new(current_user, project_id: @project.id).find_by(iid: params[:type_id])
when 'mergerequest'
MergeRequestsFinder.new(current_user, project_id: @project.id).find_by(iid: params[:type_id])
when 'commit'
@project.commit(params[:type_id])
end
QuickActions::TargetService
.new(project, current_user)
.execute(params[:type], params[:type_id])
end
end
......@@ -448,6 +448,10 @@ class Commit
true
end
def to_ability_name
model_name.singular
end
def touch
# no-op but needs to be defined since #persisted? is defined
end
......
......@@ -15,7 +15,7 @@ class SystemNoteMetadata < ActiveRecord::Base
commit description merge confidential visible label assignee cross_reference
title time_tracking branch milestone discussion task moved
opened closed merged duplicate locked unlocked
outdated
outdated tag
].freeze
validates :note, presence: true
......
# frozen_string_literal: true
class CommitPolicy < BasePolicy
delegate { @subject.project }
end
# frozen_string_literal: true
module Commits
class TagService < BaseService
def execute(commit)
unless params[:tag_name]
return error('Missing parameter tag_name')
end
tag_name = params[:tag_name]
message = params[:tag_message]
release_description = nil
result = Tags::CreateService
.new(commit.project, current_user)
.execute(tag_name, commit.sha, message, release_description)
if result[:status] == :success
tag = result[:tag]
SystemNoteService.tag_commit(commit, commit.project, current_user, tag.name)
end
result
end
end
end
......@@ -4,7 +4,8 @@ module Notes
class QuickActionsService < BaseService
UPDATE_SERVICES = {
'Issue' => Issues::UpdateService,
'MergeRequest' => MergeRequests::UpdateService
'MergeRequest' => MergeRequests::UpdateService,
'Commit' => Commits::TagService
}.freeze
def self.noteable_update_service(note)
......
......@@ -16,7 +16,7 @@ class PreviewMarkdownService < BaseService
private
def explain_quick_actions(text)
return text, [] unless %w(Issue MergeRequest).include?(commands_target_type)
return text, [] unless %w(Issue MergeRequest Commit).include?(commands_target_type)
quick_actions_service = QuickActions::InterpretService.new(project, current_user)
quick_actions_service.explain(text, find_commands_target)
......@@ -29,13 +29,9 @@ class PreviewMarkdownService < BaseService
end
def find_commands_target
if commands_target_id.present?
finder = commands_target_type == 'Issue' ? IssuesFinder : MergeRequestsFinder
finder.new(current_user, project_id: project.id).find(commands_target_id)
else
collection = commands_target_type == 'Issue' ? project.issues : project.merge_requests
collection.build
end
QuickActions::TargetService
.new(project, current_user)
.execute(commands_target_type, commands_target_id)
end
def commands_target_type
......
......@@ -47,15 +47,7 @@ module Projects
end
def commands(noteable, type)
noteable ||=
case type
when 'Issue'
@project.issues.build
when 'MergeRequest'
@project.merge_requests.build
end
return [] unless noteable&.is_a?(Issuable)
return [] unless noteable
QuickActions::InterpretService.new(project, current_user).available_commands(noteable)
end
......
......@@ -60,6 +60,7 @@ module QuickActions
"Closes this #{issuable.to_ability_name.humanize(capitalize: false)}."
end
condition do
issuable.is_a?(Issuable) &&
issuable.persisted? &&
issuable.open? &&
current_user.can?(:"update_#{issuable.to_ability_name}", issuable)
......@@ -75,6 +76,7 @@ module QuickActions
"Reopens this #{issuable.to_ability_name.humanize(capitalize: false)}."
end
condition do
issuable.is_a?(Issuable) &&
issuable.persisted? &&
issuable.closed? &&
current_user.can?(:"update_#{issuable.to_ability_name}", issuable)
......@@ -149,6 +151,7 @@ module QuickActions
issuable.allows_multiple_assignees? ? '@user1 @user2' : ''
end
condition do
issuable.is_a?(Issuable) &&
issuable.persisted? &&
issuable.assignees.any? &&
current_user.can?(:"admin_#{issuable.to_ability_name}", project)
......@@ -188,6 +191,7 @@ module QuickActions
"Removes #{issuable.milestone.to_reference(format: :name)} milestone."
end
condition do
issuable.is_a?(Issuable) &&
issuable.persisted? &&
issuable.milestone_id? &&
current_user.can?(:"admin_#{issuable.to_ability_name}", project)
......@@ -231,6 +235,7 @@ module QuickActions
end
params '~label1 ~"label 2"'
condition do
issuable.is_a?(Issuable) &&
issuable.persisted? &&
issuable.labels.any? &&
current_user.can?(:"admin_#{issuable.to_ability_name}", project)
......@@ -257,6 +262,7 @@ module QuickActions
end
params '~label1 ~"label 2"'
condition do
issuable.is_a?(Issuable) &&
issuable.persisted? &&
issuable.labels.any? &&
current_user.can?(:"admin_#{issuable.to_ability_name}", project)
......@@ -295,6 +301,7 @@ module QuickActions
desc 'Add a todo'
explanation 'Adds a todo.'
condition do
issuable.is_a?(Issuable) &&
issuable.persisted? &&
!TodoService.new.todo_exist?(issuable, current_user)
end
......@@ -317,6 +324,7 @@ module QuickActions
"Subscribes to this #{issuable.to_ability_name.humanize(capitalize: false)}."
end
condition do
issuable.is_a?(Issuable) &&
issuable.persisted? &&
!issuable.subscribed?(current_user, project)
end
......@@ -329,6 +337,7 @@ module QuickActions
"Unsubscribes from this #{issuable.to_ability_name.humanize(capitalize: false)}."
end
condition do
issuable.is_a?(Issuable) &&
issuable.persisted? &&
issuable.subscribed?(current_user, project)
end
......@@ -385,6 +394,7 @@ module QuickActions
end
params ':emoji:'
condition do
issuable.is_a?(Issuable) &&
issuable.persisted?
end
parse_params do |emoji_param|
......@@ -574,6 +584,23 @@ module QuickActions
@updates[:confidential] = true
end
desc 'Tag this commit.'
explanation do |tag_name, message|
with_message = %{ with "#{message}"} if message.present?
"Tags this commit to #{tag_name}#{with_message}."
end
params 'v1.2.3 <message>'
parse_params do |tag_name_and_message|
tag_name_and_message.split(' ', 2)
end
condition do
issuable.is_a?(Commit) && current_user.can?(:push_code, project)
end
command :tag do |tag_name, message|
@updates[:tag_name] = tag_name
@updates[:tag_message] = message
end
def extract_users(params)
return [] if params.nil?
......
# frozen_string_literal: true
module QuickActions
class TargetService < BaseService
def execute(type, type_id)
case type&.downcase
when 'issue'
issue(type_id)
when 'mergerequest'
merge_request(type_id)
when 'commit'
commit(type_id)
end
end
private
def issue(type_id)
IssuesFinder.new(current_user, project_id: project.id).find_by(iid: type_id) || project.issues.build
end
def merge_request(type_id)
MergeRequestsFinder.new(current_user, project_id: project.id).find_by(iid: type_id) || project.merge_requests.build
end
def commit(type_id)
project.commit(type_id)
end
end
end
......@@ -32,6 +32,21 @@ module SystemNoteService
create_note(NoteSummary.new(noteable, project, author, body, action: 'commit', commit_count: total_count))
end
# Called when a commit was tagged
#
# noteable - Noteable object
# project - Project owning noteable
# author - User performing the tag
# tag_name - The created tag name
#
# Returns the created Note object
def tag_commit(noteable, project, author, tag_name)
link = url_helpers.project_tag_url(project, id: tag_name)
body = "tagged commit #{noteable.sha} to [`#{tag_name}`](#{link})"
create_note(NoteSummary.new(noteable, project, author, body, action: 'tag'))
end
# Called when the assignee of a Noteable is changed or removed
#
# noteable - Noteable object
......
......@@ -7,7 +7,7 @@ module Tags
return error('Tag name invalid') unless valid_tag
repository = project.repository
message&.strip!
message = message&.strip
new_tag = nil
......
---
title: "`/tag` quick action on Commit comments"
merge_request: 20694
author: Peter Leitzen
type: added
# GitLab quick actions
Quick actions are textual shortcuts for common actions on issues or merge
requests that are usually done by clicking buttons or dropdowns in GitLab's UI.
Quick actions are textual shortcuts for common actions on issues, merge requests
or commits that are usually done by clicking buttons or dropdowns in GitLab's UI.
You can enter these commands while creating a new issue or merge request, and
in comments. Each command should be on a separate line in order to be properly
detected and executed. The commands are removed from the issue, merge request or
......@@ -39,6 +39,7 @@ do.
| `/board_move ~column` | Move issue to column on the board |
| `/duplicate #issue` | Closes this issue and marks it as a duplicate of another issue |
| `/move path/to/project` | Moves issue to another project |
| `/tag v1.2.3 <message>` | Tags a commit with a given tag name and optional message |
| `/tableflip` | Append the comment with `(╯°□°)╯︵ ┻━┻` |
| `/shrug` | Append the comment with `¯\_(ツ)_/¯` |
| <code>/copy_metadata #issue &#124; !merge_request</code> | Copy labels and milestone from other issue or merge request |
......
# frozen_string_literal: true
require 'rails_helper'
describe 'Commit > User uses quick actions', :js do
include Spec::Support::Helpers::Features::NotesHelpers
include RepoHelpers
let(:project) { create(:project, :public, :repository) }
let(:user) { project.creator }
let(:commit) { project.commit }
before do
project.add_maintainer(user)
sign_in(user)
visit project_commit_path(project, commit.id)
end
describe 'Tagging a commit' do
let(:tag_name) { 'v1.2.3' }
let(:tag_message) { 'Stable release' }
let(:truncated_commit_sha) { Commit.truncate_sha(commit.sha) }
it 'tags this commit' do
add_note("/tag #{tag_name} #{tag_message}")
expect(page).to have_content 'Commands applied'
expect(page).to have_content "tagged commit #{truncated_commit_sha}"
expect(page).to have_content tag_name
visit project_tag_path(project, tag_name)
expect(page).to have_content tag_name
expect(page).to have_content tag_message
expect(page).to have_content truncated_commit_sha
end
describe 'preview', :js do
it 'removes quick action from note and explains it' do
preview_note("/tag #{tag_name} #{tag_message}")
expect(page).not_to have_content '/tag'
expect(page).to have_content %{Tags this commit to #{tag_name} with "#{tag_message}"}
expect(page).to have_content tag_name
end
end
end
end
# frozen_string_literal: true
require 'spec_helper'
describe Commits::TagService do
let(:project) { create(:project, :repository) }
let(:user) { create(:user) }
let(:commit) { project.commit }
before do
project.add_maintainer(user)
end
describe '#execute' do
let(:service) { described_class.new(project, user, opts) }
shared_examples 'tag failure' do
it 'returns a hash with the :error status' do
result = service.execute(commit)
expect(result[:status]).to eq(:error)
expect(result[:message]).to eq(error_message)
end
it 'does not add a system note' do
service.execute(commit)
description_notes = find_notes('tag')
expect(description_notes).to be_empty
end
end
def find_notes(action)
commit
.notes
.joins(:system_note_metadata)
.where(system_note_metadata: { action: action })
end
context 'valid params' do
let(:opts) do
{
tag_name: 'v1.2.3',
tag_message: 'Release'
}
end
def find_notes(action)
commit
.notes
.joins(:system_note_metadata)
.where(system_note_metadata: { action: action })
end
context 'when tagging succeeds' do
it 'returns a hash with the :success status and created tag' do
result = service.execute(commit)
expect(result[:status]).to eq(:success)
tag = result[:tag]
expect(tag.name).to eq(opts[:tag_name])
expect(tag.message).to eq(opts[:tag_message])
end
it 'adds a system note' do
service.execute(commit)
description_notes = find_notes('tag')
expect(description_notes.length).to eq(1)
end
end
context 'when tagging fails' do
let(:tag_error) { 'GitLab: You are not allowed to push code to this project.' }
before do
tag_stub = instance_double(Tags::CreateService)
allow(Tags::CreateService).to receive(:new).and_return(tag_stub)
allow(tag_stub).to receive(:execute).and_return({
status: :error, message: tag_error
})
end
it_behaves_like 'tag failure' do
let(:error_message) { tag_error }
end
end
end
context 'invalid params' do
let(:opts) do
{}
end
it_behaves_like 'tag failure' do
let(:error_message) { 'Missing parameter tag_name' }
end
end
end
end
......@@ -11,40 +11,6 @@ describe Notes::QuickActionsService do
end
end
shared_examples 'note on noteable that does not support quick actions' do
include_context 'note on noteable'
before do
note.note = note_text
end
describe 'note with only command' do
describe '/close, /label, /assign & /milestone' do
let(:note_text) { %(/close\n/assign @#{assignee.username}") }
it 'saves the note and does not alter the note text' do
content, command_params = service.extract_commands(note)
expect(content).to eq note_text
expect(command_params).to be_empty
end
end
end
describe 'note with command & text' do
describe '/close, /label, /assign & /milestone' do
let(:note_text) { %(HELLO\n/close\n/assign @#{assignee.username}\nWORLD) }
it 'saves the note and does not alter the note text' do
content, command_params = service.extract_commands(note)
expect(content).to eq note_text
expect(command_params).to be_empty
end
end
end
end
shared_examples 'note on noteable that supports quick actions' do
include_context 'note on noteable'
......@@ -147,16 +113,16 @@ describe Notes::QuickActionsService do
expect(described_class.noteable_update_service(note)).to eq(Issues::UpdateService)
end
it 'returns Issues::UpdateService for a note on a merge request' do
it 'returns MergeRequests::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
it 'returns Commits::TagService for a note on a commit' do
note = create(:note_on_commit, project: project)
expect(described_class.noteable_update_service(note)).to be_nil
expect(described_class.noteable_update_service(note)).to eq(Commits::TagService)
end
end
......@@ -175,7 +141,7 @@ describe Notes::QuickActionsService do
let(:note) { create(:note_on_commit, project: project) }
it 'returns false' do
expect(described_class.supported?(note)).to be_falsy
expect(described_class.supported?(note)).to be_truthy
end
end
end
......@@ -203,10 +169,6 @@ describe Notes::QuickActionsService do
it_behaves_like 'note on noteable that supports quick actions' do
let(:note) { build(:note_on_merge_request, project: project) }
end
it_behaves_like 'note on noteable that does not support quick actions' do
let(:note) { build(:note_on_commit, project: project) }
end
end
context 'CE restriction for issue assignees' do
......
......@@ -65,6 +65,31 @@ describe PreviewMarkdownService do
end
end
context 'commit description' do
let(:project) { create(:project, :repository) }
let(:commit) { project.commit }
let(:params) do
{
text: "My work\n/tag v1.2.3 Stable release",
quick_actions_target_type: 'Commit',
quick_actions_target_id: commit.id
}
end
let(:service) { described_class.new(project, user, params) }
it 'removes quick actions from text' do
result = service.execute
expect(result[:text]).to eq 'My work'
end
it 'explains quick actions effect' do
result = service.execute
expect(result[:commands]).to eq 'Tags this commit to v1.2.3 with "Stable release".'
end
end
it 'sets correct markdown engine' do
service = described_class.new(project, user, { markdown_version: CacheMarkdownField::CACHE_REDCARPET_VERSION })
result = service.execute
......
......@@ -6,6 +6,7 @@ describe QuickActions::InterpretService do
let(:developer2) { create(:user) }
let(:issue) { create(:issue, project: project) }
let(:milestone) { create(:milestone, project: project, title: '9.10') }
let(:commit) { create(:commit, project: project) }
let(:inprogress) { create(:label, project: project, title: 'In Progress') }
let(:bug) { create(:label, project: project, title: 'Bug') }
let(:note) { build(:note, commit_id: merge_request.diff_head_sha) }
......@@ -347,6 +348,14 @@ describe QuickActions::InterpretService do
end
end
shared_examples 'tag command' do
it 'tags a commit' do
_, updates = service.execute(content, issuable)
expect(updates).to eq(tag_name: tag_name, tag_message: tag_message)
end
end
it_behaves_like 'reopen command' do
let(:content) { '/reopen' }
let(:issuable) { issue }
......@@ -628,16 +637,6 @@ describe QuickActions::InterpretService do
let(:issuable) { merge_request }
end
it_behaves_like 'todo command' do
let(:content) { '/todo' }
let(:issuable) { issue }
end
it_behaves_like 'todo command' do
let(:content) { '/todo' }
let(:issuable) { merge_request }
end
it_behaves_like 'done command' do
let(:content) { '/done' }
let(:issuable) { issue }
......@@ -787,6 +786,28 @@ describe QuickActions::InterpretService do
let(:issuable) { issue }
end
context '/todo' do
let(:content) { '/todo' }
context 'if issuable is an Issue' do
it_behaves_like 'todo command' do
let(:issuable) { issue }
end
end
context 'if issuable is a MergeRequest' do
it_behaves_like 'todo command' do
let(:issuable) { merge_request }
end
end
context 'if issuable is a Commit' do
it_behaves_like 'empty command' do
let(:issuable) { commit }
end
end
end
context '/copy_metadata command' do
let(:todo_label) { create(:label, project: project, title: 'To Do') }
let(:inreview_label) { create(:label, project: project, title: 'In Review') }
......@@ -971,6 +992,12 @@ describe QuickActions::InterpretService do
let(:issuable) { issue }
end
end
context 'if issuable is a Commit' do
let(:content) { '/award :100:' }
let(:issuable) { commit }
it_behaves_like 'empty command'
end
end
context '/shrug command' do
......@@ -1102,6 +1129,32 @@ describe QuickActions::InterpretService do
it_behaves_like 'empty command'
end
end
context '/tag command' do
let(:issuable) { commit }
context 'ignores command with no argument' do
it_behaves_like 'empty command' do
let(:content) { '/tag' }
end
end
context 'tags a commit with a tag name' do
it_behaves_like 'tag command' do
let(:tag_name) { 'v1.2.3' }
let(:tag_message) { nil }
let(:content) { "/tag #{tag_name}" }
end
end
context 'tags a commit with a tag name and message' do
it_behaves_like 'tag command' do
let(:tag_name) { 'v1.2.3' }
let(:tag_message) { 'Stable release' }
let(:content) { "/tag #{tag_name} #{tag_message}" }
end
end
end
end
describe '#explain' do
......@@ -1319,5 +1372,39 @@ describe QuickActions::InterpretService do
expect(explanations).to eq(["Moves this issue to test/project."])
end
end
describe 'tag a commit' do
describe 'with a tag name' do
context 'without a message' do
let(:content) { '/tag v1.2.3' }
it 'includes the tag name only' do
_, explanations = service.explain(content, commit)
expect(explanations).to eq(["Tags this commit to v1.2.3."])
end
end
context 'with an empty message' do
let(:content) { '/tag v1.2.3 ' }
it 'includes the tag name only' do
_, explanations = service.explain(content, commit)
expect(explanations).to eq(["Tags this commit to v1.2.3."])
end
end
end
describe 'with a tag name and message' do
let(:content) { '/tag v1.2.3 Stable release' }
it 'includes the tag name and message' do
_, explanations = service.explain(content, commit)
expect(explanations).to eq(["Tags this commit to v1.2.3 with \"Stable release\"."])
end
end
end
end
end
# frozen_string_literal: true
require 'spec_helper'
describe QuickActions::TargetService do
let(:project) { create(:project) }
let(:user) { create(:user) }
let(:service) { described_class.new(project, user) }
before do
project.add_maintainer(user)
end
describe '#execute' do
shared_examples 'no target' do |type_id:|
it 'returns nil' do
target = service.execute(type, type_id)
expect(target).to be_nil
end
end
shared_examples 'find target' do
it 'returns the target' do
found_target = service.execute(type, target_id)
expect(found_target).to eq(target)
end
end
shared_examples 'build target' do |type_id:|
it 'builds a new target' do
target = service.execute(type, type_id)
expect(target.project).to eq(project)
expect(target).to be_new_record
end
end
context 'for issue' do
let(:target) { create(:issue, project: project) }
let(:target_id) { target.iid }
let(:type) { 'Issue' }
it_behaves_like 'find target'
it_behaves_like 'build target', type_id: nil
it_behaves_like 'build target', type_id: -1
end
context 'for merge request' do
let(:target) { create(:merge_request, source_project: project) }
let(:target_id) { target.iid }
let(:type) { 'MergeRequest' }
it_behaves_like 'find target'
it_behaves_like 'build target', type_id: nil
it_behaves_like 'build target', type_id: -1
end
context 'for commit' do
let(:project) { create(:project, :repository) }
let(:target) { project.commit.parent }
let(:target_id) { target.sha }
let(:type) { 'Commit' }
it_behaves_like 'find target'
it_behaves_like 'no target', type_id: 'invalid_sha'
context 'with nil target_id' do
let(:target) { project.commit }
let(:target_id) { nil }
it_behaves_like 'find target'
end
end
context 'for unknown type' do
let(:type) { 'unknown' }
it_behaves_like 'no target', type_id: :unused
end
end
end
......@@ -108,6 +108,25 @@ describe SystemNoteService do
end
end
describe '.tag_commit' do
let(:noteable) do
project.commit
end
let(:tag_name) { 'v1.2.3' }
subject { described_class.tag_commit(noteable, project, author, tag_name) }
it_behaves_like 'a system note' do
let(:action) { 'tag' }
end
it 'sets the note text' do
link = "http://localhost/#{project.full_path}/tags/#{tag_name}"
expect(subject.note).to eq "tagged commit #{noteable.sha} to [`#{tag_name}`](#{link})"
end
end
describe '.change_assignee' do
subject { described_class.change_assignee(noteable, project, author, assignee) }
......
......@@ -20,6 +20,13 @@ module Spec
end
end
end
def preview_note(text)
page.within('.js-main-target-form') do
fill_in('note[note]', with: text)
click_on('Preview')
end
end
end
end
end
......
......@@ -16,7 +16,7 @@ describe 'shared/notes/_form' do
render
end
%w[issue merge_request].each do |noteable|
%w[issue merge_request commit].each do |noteable|
context "with a note on #{noteable}" do
let(:note) { build(:"note_on_#{noteable}", project: project) }
......@@ -25,12 +25,4 @@ describe 'shared/notes/_form' do
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 quick actions' do
expect(rendered).to have_content('Markdown is supported')
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment