Commit 8c8bc07d authored by Z.J. van de Weg's avatar Z.J. van de Weg

Refactor and test Slash commands

This is the structure Kamil proposed, which leaves us with a bunch of
smaller classes. This commits deletes outdated files and tests
everything from the SlashCommandService and down (child classes and
subcommands)
parent 106b1e39
......@@ -17,13 +17,17 @@ def collection
private
def find_by_iid(iid)
def present(resource)
Mattermost::Presenter.present(resource)
end
def find_by_iid
resource = collection.find_by(iid: iid)
readable?(resource) ? resource : nil
end
def search
def search_results
collection.search(query).limit(QUERY_LIMIT).select do |resource|
readable?(resource)
end
......
module Mattermost
module Commands
class IssueShowService < Mattermost::Commands::BaseService
class IssueCreateService < IssueService
def execute
return Mattermost::Messages::Issues.not_available unless available?
title, description = parse_command
issue = find_by_iid(iid)
present Issues::CreateService.new(project, current_user, title: title, description: description).execute
end
private
def parse_command
match = params[:text].match(/\Aissue create (?<title>.*)\n*/)
title = match[:title]
description = match.post_match
present issue
[title, description]
end
end
end
......
module Mattermost
module Commands
class IssueShowService < IssueService
class IssueSearchService < IssueService
def execute
return Mattermost::Messages::Issues.not_available unless available?
present search_results
end
end
......
module Mattermost
module Commands
class IssueService < Mattermost::Commands::BaseService
def available?
def self.available?(project)
project.issues_enabled? && project.default_issues_tracker?
end
......@@ -12,10 +12,6 @@ def collection
def readable?(issue)
can?(current_user, :read_issue, issue)
end
def present
Mattermost::Presenter.issue
end
end
end
end
......@@ -2,10 +2,7 @@ module Mattermost
module Commands
class IssueShowService < IssueService
def execute
return Mattermost::Messages.not_available unless available?
issue = find_by_iid(iid)
present issue
present find_by_iid
end
end
end
......
module Mattermost
module Commands
class MergeRequestService < Mattermost::Commands::BaseService
class MergeRequestSearchService < MergeRequestService
def execute
return Mattermost::Messages::MergeRequests.not_available unless available?
Mattermost::Messages::IssuePresenter.present search_results
present search_results
end
end
end
......
module Mattermost
module Commands
class MergeRequestService < Mattermost::Commands::BaseService
def available?
project.issues_enabled? && project.default_issues_tracker?
def self.available?(project)
project.merge_requests_enabled?
end
def collection
......@@ -12,10 +12,6 @@ def collection
def readable?(_)
can?(current_user, :read_merge_request, project)
end
def present
Mattermost::Presenter.merge_request
end
end
end
end
module Mattermost
module Commands
class MergeRequestShowService < Mattermost::Commands::BaseService
class MergeRequestShowService < MergeRequestService
def execute
return Mattermost::Messages.not_available unless available?
merge_request = find_by_iid(iid)
present merge_request
present find_by_iid
end
end
end
......
module Mattermost
class SlashCommandService < BaseService
def self.registry
@registry ||= Hash.new({})
end
def self.command(command, sub_command, klass, help_message)
registry[command][sub_command] = { klass: klass, help_message: help_message }
end
......@@ -12,17 +16,24 @@ def self.command(command, sub_command, klass, help_message)
command 'mergerequest', 'search', Mattermost::Commands::MergeRequestSearchService, 'mergerequest search <query>'
def execute
service = registry[command][subcommand]
command, subcommand = parse_command
#TODO think how to do this to support ruby 2.1
service = registry.dig(command, subcommand, :klass)
return help_messages(registry) unless service.try(:available?)
return help_messages(registry) unless service.try(:available?, project)
service.new(project, current_user, params).execute
end
private
def self.registry
@registry ||= Hash.new({})
def parse_command
params[:text].split.first(2)
end
def registry
self.class.registry
end
end
end
......@@ -32,53 +32,59 @@ def not_found
text: "404 not found! GitLab couldn't find what your were looking for! :boom:",
}
end
end
attr_reader :result
def initialize(result)
@result = result
end
def present
if result.respond_to?(:count)
if result.count > 1
return respond_collection(result)
elsif result.count == 0
return not_found
else
result = result.first
def present(resource)
return not_found unless resource
if resource.respond_to?(:count)
if resource.count > 1
return multiple_resources(resource)
elsif resource.count == 0
return not_found
else
resource = resource.first
end
end
single_resource(resource)
end
single_resource
end
private
private
def single_resource(resource)
message = title(resource)
message << "\n\n#{resource.description}" if resource.description
def single_resource
message = title(resource)
message << "\n\n#{resource.description}" if resource.description
{
response_type: :in_channel,
text: message
}
end
{
response_type: :in_channel,
text: message
}
end
def multiple_resources(resources)
message = "Multiple results were found:\n"
message << resources.map { |resource| " #{title(resource)}" }.join("\n")
def multiple_resources(resources)
message = "Multiple results were found:\n"
message << resource.map { |resource| " #{title(resource)}" }.join("\n")
{
response_type: :ephemeral,
text: message
}
end
{
response_type: :ephemeral,
text: message
}
end
def title(resource)
"### [#{resource.to_reference} #{resource.title}](#{url(resource)})"
end
def url(resource)
helper = Rails.application.routes.url_helpers
def title(resource)
url = url_for([resource.project.namespace.becomes(Namespace), resource.project, resource])
"### [#{resource.to_reference} #{resource.title}](#{url})"
case resource
when Issue
helper.namespace_project_issue_url(resource.project.namespace.becomes(Namespace), resource.project, resource)
when MergeRequest
helper.namespace_project_merge_request_url(resource.project.namespace.becomes(Namespace), resource.project, resource)
end
end
end
end
end
require 'spec_helper'
describe Mattermost::Commands::IssueCreateService, service: true do
describe '#execute' do
let(:project) { create(:empty_project) }
let(:user) { create(:user) }
let(:params) { { text: "issue create bird is the word" } }
before { project.team << [user, :master] }
subject { described_class.new(project, user, params).execute }
context 'without description' do
it 'creates the issue' do
expect do
subject # this trigger the execution
end.to change { project.issues.count }.by(1)
expect(subject[:response_type]).to be :in_channel
expect(subject[:text]).to match 'bird is the word'
end
end
context 'with description' do
let(:description) { "Surfin bird" }
let(:params) { { text: "issue create The bird is the word\n#{description}" } }
before { subject }
it 'creates the issue with description' do
expect(Issue.last.description).to eq description
end
end
end
end
require 'spec_helper'
describe Mattermost::Commands::IssueSearchService, service: true do
describe '#execute' do
let!(:issue) { create(:issue, title: 'The bird is the word') }
let(:project) { issue.project }
let(:user) { issue.author }
let(:params) { { text: "issue search bird is the" } }
before { project.team << [user, :master] }
subject { described_class.new(project, user, params).execute }
context 'without results' do
let(:params) { { text: "issue search no results for this one" } }
it "returns nil" do
expect(subject[:response_type]).to be :ephemeral
expect(subject[:text]).to start_with '404 not found!'
end
end
context 'with 1 result' do
it 'returns the issue' do
expect(subject[:response_type]).to be :in_channel
expect(subject[:text]).to match issue.title
end
end
context 'with 2 or more results' do
let!(:issue2) { create(:issue, project: project, title: 'bird is the word!') }
it 'returns multiple resources' do
expect(subject[:response_type]).to be :ephemeral
expect(subject[:text]).to start_with 'Multiple results were found'
end
end
end
end
require 'spec_helper'
describe Mattermost::Commands::IssueService do
let(:project) { create(:project) }
let(:issue ) { create(:issue, :confidential, title: 'Bird is the word', project: project) }
let(:user) { issue.author }
subject { described_class.new(project, user, params).execute }
before do
project.team << [user, :developer]
end
describe '#execute' do
context 'show as subcommand' do
context 'issue can be found' do
let(:params) { { text: "issue show #{issue.iid}" } }
it 'returns the merge request' do
expect(subject).to eq issue
end
context 'the user has no access' do
let(:non_member) { create(:user) }
subject { described_class.new(project, non_member, params).execute }
it 'returns nil' do
expect(subject).to eq nil
end
end
end
context 'issue can not be found' do
let(:params) { { text: 'issue show 12345' } }
it 'returns nil' do
expect(subject).to eq nil
end
end
end
context 'search as a subcommand' do
context 'with results' do
let(:params) { { text: "issue search is the word" } }
it 'returns the issue' do
expect(subject).to eq [issue]
end
end
context 'without results' do
let(:params) { { text: 'issue search mepmep' } }
it 'returns an empty collection' do
expect(subject).to eq []
end
end
end
context 'create as subcommand' do
let(:title) { 'my new issue' }
let(:params) { { text: "issue create #{title}" } }
it 'return the new issue' do
expect(subject).to be_a Issue
end
it 'creates a new issue' do
expect { subject }.to change { Issue.count }.by(1)
end
end
end
describe 'help_message' do
context 'issues are disabled' do
it 'returns nil' do
allow(described_class).to receive(:available?).and_return false
expect(described_class.help_message(project)).to eq nil
end
end
end
end
require 'spec_helper'
describe Mattermost::Commands::IssueShowService, service: true do
describe '#execute' do
let(:issue) { create(:issue) }
let(:project) { issue.project }
let(:user) { issue.author }
let(:params) { { text: "issue show #{issue.iid}" } }
before { project.team << [user, :master] }
subject { described_class.new(project, user, params).execute }
context 'the issue exists' do
it 'returns the issue' do
expect(subject[:response_type]).to be :in_channel
expect(subject[:text]).to match issue.title
end
end
context 'the issue does not exist' do
let(:params) { { text: "issue show 12345" } }
it "returns nil" do
expect(subject[:response_type]).to be :ephemeral
expect(subject[:text]).to start_with '404 not found!'
end
end
end
end
require 'spec_helper'
describe Mattermost::Commands::MergeRequestSearchService, service: true do
describe '#execute' do
let!(:merge_request) { create(:merge_request, title: 'The bird is the word') }
let(:project) { merge_request.source_project }
let(:user) { merge_request.author }
let(:params) { { text: "mergerequest search #{merge_request.title}" } }
before { project.team << [user, :master] }
subject { described_class.new(project, user, params).execute }
context 'the merge request exists' do
it 'returns the merge request' do
expect(subject[:response_type]).to be :in_channel
expect(subject[:text]).to match merge_request.title
end
end
context 'no results can be found' do
let(:params) { { text: "mergerequest search 12345" } }
it "returns a 404 message" do
expect(subject[:response_type]).to be :ephemeral
expect(subject[:text]).to start_with '404 not found!'
end
end
end
end
require 'spec_helper'
describe Mattermost::Commands::MergeRequestService do
let(:project) { create(:project, :private) }
let(:merge_request) { create(:merge_request, title: 'Bird is the word', source_project: project) }
let(:user) { merge_request.author }
subject { described_class.new(project, user, params).execute }
before do
project.team << [user, :developer]
end
context 'show as subcommand' do
context 'merge request can be found' do
let(:params) { { text: "mergerequest show #{merge_request.iid}" } }
it 'returns the merge request' do
expect(subject).to eq merge_request
end
context 'the user has no access' do
let(:non_member) { create(:user) }
subject { described_class.new(project, non_member, params).execute }
it 'returns nil' do
expect(subject).to eq nil
end
end
end
context 'merge request can not be found' do
let(:params) { { text: 'mergerequest show 12345' } }
it 'returns nil' do
expect(subject).to eq nil
end
end
end
context 'search as a subcommand' do
context 'with results' do
let(:params) { { text: "mergerequest search is the word" } }
it 'returns the merge_request' do
expect(subject).to eq [merge_request]
end
end
context 'without results' do
let(:params) { { text: 'mergerequest search mepmep' } }
it 'returns an empty collection' do
expect(subject).to eq []
end
end
end
describe 'help_message' do
context 'issues are disabled' do
it 'returns nil' do
allow(described_class).to receive(:available?).and_return false
expect(described_class.help_message(project)).to eq nil
end
end
end
end
require 'spec_helper'
describe Mattermost::Commands::MergeRequestShowService, service: true do
describe '#execute' do
let!(:merge_request) { create(:merge_request) }
let(:project) { merge_request.source_project }
let(:user) { merge_request.author }
let(:params) { { text: "mergerequest show #{merge_request.iid}" } }
before { project.team << [user, :master] }
subject { described_class.new(project, user, params).execute }
context 'the merge request exists' do
it 'returns the merge request' do
expect(subject[:response_type]).to be :in_channel
expect(subject[:text]).to match merge_request.title
end
end
context 'the merge request does not exist' do
let(:params) { { text: "mergerequest show 12345" } }
it "returns nil" do
expect(subject[:response_type]).to be :ephemeral
expect(subject[:text]).to start_with '404 not found!'
end
end
end
end
......@@ -7,23 +7,12 @@
subject { described_class.new(project, user, params).execute }
describe '#execute' do
context 'no command service is triggered' do
let(:params) { { text: 'unknown_command' } }
xdescribe '#execute' do
context 'when issue show is triggered' do
it 'calls IssueShowService' do
expect_any_instance_of(Mattermost::Commands::IssueShowService).to receive(:new).with(project, user, params)
it 'shows the help messages' do
expect