From 298d05a5c3cc3c2f1daa4d77c45f9c90b53248df Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Mon, 19 Dec 2016 15:40:06 +0100 Subject: [PATCH] Improve after feedback --- features/project/service.feature | 6 ++-- features/steps/project/services.rb | 8 +++--- lib/api/services.rb | 9 +++++- lib/gitlab/chat_commands/help.rb | 28 ------------------- lib/gitlab/chat_commands/presenter.rb | 2 +- .../services/slack_slash_command_spec.rb | 18 ++++++------ .../mattermost_slash_commands_service_spec.rb | 4 +-- .../slack_slash_commands_service.rb | 6 ++-- .../chat_slash_commands_shared_examples.rb | 2 +- 9 files changed, 32 insertions(+), 51 deletions(-) delete mode 100644 lib/gitlab/chat_commands/help.rb diff --git a/features/project/service.feature b/features/project/service.feature index 3a7b8308524..892db48d785 100644 --- a/features/project/service.feature +++ b/features/project/service.feature @@ -39,9 +39,9 @@ Feature: Project Services Scenario: Activate Slack service When I visit project "Shop" services page - And I click Slack service link - And I fill Slack settings - Then I should see Slack service settings saved + And I click Slack Notifications service link + And I fill Slack Notifications settings + Then I should see Slack Notifications service settings saved Scenario: Activate Pushover service When I visit project "Shop" services page diff --git a/features/steps/project/services.rb b/features/steps/project/services.rb index bd6466f3686..06a1afedbd9 100644 --- a/features/steps/project/services.rb +++ b/features/steps/project/services.rb @@ -137,17 +137,17 @@ class Spinach::Features::ProjectServices < Spinach::FeatureSteps expect(find_field('Colorize messages').value).to eq '1' end - step 'I click Slack service link' do - click_link 'Slack' + step 'I click Slack Notifications service link' do + click_link 'Slack Notifications' end - step 'I fill Slack settings' do + step 'I fill Slack Notifications settings' do check 'Active' fill_in 'Webhook', with: 'https://hooks.slack.com/services/SVRWFV0VVAR97N/B02R25XN3/ZBqu7xMupaEEICInN685' click_button 'Save' end - step 'I should see Slack service settings saved' do + step 'I should see Slack Notifications service settings saved' do expect(find_field('Webhook').value).to eq 'https://hooks.slack.com/services/SVRWFV0VVAR97N/B02R25XN3/ZBqu7xMupaEEICInN685' end diff --git a/lib/api/services.rb b/lib/api/services.rb index 59232c84c24..aa97f6af0b2 100644 --- a/lib/api/services.rb +++ b/lib/api/services.rb @@ -378,7 +378,6 @@ module API desc: 'A custom certificate authority bundle to verify the Kubernetes cluster with (PEM format)' }, ], - 'mattermost-slash-commands' => [ { required: true, @@ -387,6 +386,14 @@ module API desc: 'The Mattermost token' } ], + 'slack-slash-commands' => [ + { + required: true, + name: :token, + type: String, + desc: 'The Slack token' + } + ], 'pipelines-email' => [ { required: true, diff --git a/lib/gitlab/chat_commands/help.rb b/lib/gitlab/chat_commands/help.rb deleted file mode 100644 index e76733f5445..00000000000 --- a/lib/gitlab/chat_commands/help.rb +++ /dev/null @@ -1,28 +0,0 @@ -module Gitlab - module ChatCommands - class Help < BaseCommand - # This class has to be used last, as it always matches. It has to match - # because other commands were not triggered and we want to show the help - # command - def self.match(_text) - true - end - - def self.help_message - 'help' - end - - def self.allowed?(_project, _user) - true - end - - def execute(commands) - Gitlab::ChatCommands::Presenters::Help.new(commands).present(trigger) - end - - def trigger - params[:command] - end - end - end -end diff --git a/lib/gitlab/chat_commands/presenter.rb b/lib/gitlab/chat_commands/presenter.rb index b4c4dc252ca..caceaa25391 100644 --- a/lib/gitlab/chat_commands/presenter.rb +++ b/lib/gitlab/chat_commands/presenter.rb @@ -1,7 +1,7 @@ module Gitlab module ChatCommands class Presenter - include Gitlab::Routing.url_helpers + include Gitlab::Routing def authorize_chat_name(url) message = if url diff --git a/spec/features/projects/services/slack_slash_command_spec.rb b/spec/features/projects/services/slack_slash_command_spec.rb index dee43d69895..70e203efcf5 100644 --- a/spec/features/projects/services/slack_slash_command_spec.rb +++ b/spec/features/projects/services/slack_slash_command_spec.rb @@ -1,18 +1,18 @@ require 'spec_helper' -feature 'Setup Slack slash commands', feature: true do +feature 'Slack slash commands', feature: true do include WaitForAjax - let(:user) { create(:user) } - let(:project) { create(:project) } - let(:service) { project.create_slack_slash_commands_service } + given(:user) { create(:user) } + given(:project) { create(:project) } + given(:service) { project.create_slack_slash_commands_service } - before do + background do project.team << [user, :master] login_as(user) end - describe 'user visits the slack slash command config page', js: true do + scenario 'user visits the slack slash command config page', js: true do it 'shows a help message' do visit edit_namespace_project_service_path(project.namespace, project, service) @@ -22,8 +22,8 @@ feature 'Setup Slack slash commands', feature: true do end end - describe 'saving a token' do - let(:token) { ('a'..'z').to_a.join } + scenario 'saving a token' do + given(:token) { ('a'..'z').to_a.join } it 'shows the token after saving' do visit edit_namespace_project_service_path(project.namespace, project, service) @@ -37,7 +37,7 @@ feature 'Setup Slack slash commands', feature: true do end end - describe 'the trigger url' do + scenario 'the trigger url' do it 'shows the correct url' do visit edit_namespace_project_service_path(project.namespace, project, service) diff --git a/spec/models/project_services/mattermost_slash_commands_service_spec.rb b/spec/models/project_services/mattermost_slash_commands_service_spec.rb index 5c34cb6b4cf..1ae1483e2a4 100644 --- a/spec/models/project_services/mattermost_slash_commands_service_spec.rb +++ b/spec/models/project_services/mattermost_slash_commands_service_spec.rb @@ -1,5 +1,5 @@ require 'spec_helper' -describe MattermostSlashCommandsService, models: true do - it_behaves_like "chat slash commands" +describe MattermostSlashCommandsService, :models do + it_behaves_like "chat slash commands service" end diff --git a/spec/models/project_services/slack_slash_commands_service.rb b/spec/models/project_services/slack_slash_commands_service.rb index c3fa80caebe..5775e439906 100644 --- a/spec/models/project_services/slack_slash_commands_service.rb +++ b/spec/models/project_services/slack_slash_commands_service.rb @@ -1,7 +1,7 @@ require 'spec_helper' -describe SlackSlashCommandsService, models: true do - it_behaves_like "chat slash commands" +describe SlackSlashCommandsService, :models do + it_behaves_like "chat slash commands service" describe '#trigger' do context 'when an auth url is generated' do @@ -15,11 +15,13 @@ describe SlackSlashCommandsService, models: true do token: 'token' } end + let(:service) do project.create_slack_slash_commands_service( properties: { token: 'token' } ) end + let(:authorize_url) do 'http://authorize.example.com/' end diff --git a/spec/support/chat_slash_commands_shared_examples.rb b/spec/support/chat_slash_commands_shared_examples.rb index 96130b45235..4dfa29849ee 100644 --- a/spec/support/chat_slash_commands_shared_examples.rb +++ b/spec/support/chat_slash_commands_shared_examples.rb @@ -1,4 +1,4 @@ -RSpec.shared_examples 'chat slash commands' do +RSpec.shared_examples 'chat slash commands service' do describe "Associations" do it { is_expected.to respond_to :token } it { is_expected.to have_many :chat_names }