Commit 7bc440c8 authored by Kamil Trzciński's avatar Kamil Trzciński

Merge branch 'ee-slash-commands-changes-for-chatops' into 'master'

EE: Expose ChatName objects to slash commands

See merge request gitlab-org/gitlab-ee!4693
parents dbbe6452 7c5872c1
class ChatName < ActiveRecord::Base
LAST_USED_AT_INTERVAL = 1.hour
belongs_to :service
belongs_to :user
......@@ -9,4 +11,23 @@ class ChatName < ActiveRecord::Base
validates :user_id, uniqueness: { scope: [:service_id] }
validates :chat_id, uniqueness: { scope: [:service_id, :team_id] }
# Updates the "last_used_timestamp" but only if it wasn't already updated
# recently.
#
# The throttling this method uses is put in place to ensure that high chat
# traffic doesn't result in many UPDATE queries being performed.
def update_last_used_at
return unless update_last_used_at?
obtained = Gitlab::ExclusiveLease
.new("chat_name/last_used_at/#{id}", timeout: LAST_USED_AT_INTERVAL.to_i)
.try_obtain
touch(:last_used_at) if obtained
end
def update_last_used_at?
last_used_at.nil? || last_used_at > LAST_USED_AT_INTERVAL.ago
end
end
......@@ -30,10 +30,10 @@ class SlashCommandsService < Service
def trigger(params)
return unless valid_token?(params[:token])
user = find_chat_user(params)
chat_user = find_chat_user(params)
if user
Gitlab::SlashCommands::Command.new(project, user, params).execute
if chat_user&.user
Gitlab::SlashCommands::Command.new(project, chat_user, params).execute
else
url = authorize_chat_name_url(params)
Gitlab::SlashCommands::Presenters::Access.new(url).authorize
......
......@@ -9,8 +9,8 @@ module ChatNames
chat_name = find_chat_name
return unless chat_name
chat_name.touch(:last_used_at)
chat_name.user
chat_name.update_last_used_at
chat_name
end
private
......
......@@ -22,10 +22,10 @@ module SlashCommands
service = integration.service
project = service.project
user = ChatNames::FindUserService.new(service, params).execute
chat_user = ChatNames::FindUserService.new(service, params).execute
if user
Gitlab::SlashCommands::Command.new(project, user, params).execute
if chat_user&.user
Gitlab::SlashCommands::Command.new(project, chat_user, params).execute
else
url = ChatNames::AuthorizeUserService.new(service, params).execute
Gitlab::SlashCommands::Presenters::Access.new(url).authorize
......
......@@ -3,6 +3,7 @@ require 'spec_helper'
describe SlashCommands::GlobalSlackHandler do
let(:project) { create(:project) }
let(:user) { create(:user) }
let(:chat_name) { double(:chat_name, user: user) }
let(:verification_token) { '123' }
before do
......@@ -32,7 +33,7 @@ describe SlashCommands::GlobalSlackHandler do
context 'Valid token' do
it 'calls command handler if project alias is valid' do
expect_any_instance_of(Gitlab::SlashCommands::Command).to receive(:execute)
expect_any_instance_of(ChatNames::FindUserService).to receive(:execute).and_return(user)
expect_any_instance_of(ChatNames::FindUserService).to receive(:execute).and_return(chat_name)
enable_slack_application(project)
......
......@@ -31,10 +31,11 @@ module Gitlab
raise NotImplementedError
end
attr_accessor :project, :current_user, :params
attr_accessor :project, :current_user, :params, :chat_name
def initialize(project, user, params = {})
@project, @current_user, @params = project, user, params.dup
def initialize(project, chat_name, params = {})
@project, @current_user, @params = project, chat_name.user, params.dup
@chat_name = chat_name
end
private
......
......@@ -13,12 +13,13 @@ module Gitlab
if command
if command.allowed?(project, current_user)
command.new(project, current_user, params).execute(match)
command.new(project, chat_name, params).execute(match)
else
Gitlab::SlashCommands::Presenters::Access.new.access_denied
end
else
Gitlab::SlashCommands::Help.new(project, current_user, params).execute(available_commands, params[:text])
Gitlab::SlashCommands::Help.new(project, chat_name, params)
.execute(available_commands, params[:text])
end
end
......
......@@ -3,10 +3,11 @@ require 'spec_helper'
describe Gitlab::SlashCommands::Command do
let(:project) { create(:project) }
let(:user) { create(:user) }
let(:chat_name) { double(:chat_name, user: user) }
describe '#execute' do
subject do
described_class.new(project, user, params).execute
described_class.new(project, chat_name, params).execute
end
context 'when no command is available' do
......@@ -88,7 +89,7 @@ describe Gitlab::SlashCommands::Command do
end
describe '#match_command' do
subject { described_class.new(project, user, params).match_command.first }
subject { described_class.new(project, chat_name, params).match_command.first }
context 'IssueShow is triggered' do
let(:params) { { text: 'issue show 123' } }
......
......@@ -4,6 +4,7 @@ describe Gitlab::SlashCommands::Deploy do
describe '#execute' do
let(:project) { create(:project) }
let(:user) { create(:user) }
let(:chat_name) { double(:chat_name, user: user) }
let(:regex_match) { described_class.match('deploy staging to production') }
before do
......@@ -16,7 +17,7 @@ describe Gitlab::SlashCommands::Deploy do
end
subject do
described_class.new(project, user).execute(regex_match)
described_class.new(project, chat_name).execute(regex_match)
end
context 'if no environment is defined' do
......
......@@ -4,6 +4,7 @@ describe Gitlab::SlashCommands::IssueNew do
describe '#execute' do
let(:project) { create(:project) }
let(:user) { create(:user) }
let(:chat_name) { double(:chat_name, user: user) }
let(:regex_match) { described_class.match("issue create bird is the word") }
before do
......@@ -11,7 +12,7 @@ describe Gitlab::SlashCommands::IssueNew do
end
subject do
described_class.new(project, user).execute(regex_match)
described_class.new(project, chat_name).execute(regex_match)
end
context 'without description' do
......
......@@ -6,10 +6,11 @@ describe Gitlab::SlashCommands::IssueSearch do
let!(:confidential) { create(:issue, :confidential, project: project, title: 'mepmep find') }
let(:project) { create(:project) }
let(:user) { create(:user) }
let(:chat_name) { double(:chat_name, user: user) }
let(:regex_match) { described_class.match("issue search find") }
subject do
described_class.new(project, user).execute(regex_match)
described_class.new(project, chat_name).execute(regex_match)
end
context 'when the user has no access' do
......
......@@ -5,6 +5,7 @@ describe Gitlab::SlashCommands::IssueShow do
let(:issue) { create(:issue, project: project) }
let(:project) { create(:project) }
let(:user) { issue.author }
let(:chat_name) { double(:chat_name, user: user) }
let(:regex_match) { described_class.match("issue show #{issue.iid}") }
before do
......@@ -12,7 +13,7 @@ describe Gitlab::SlashCommands::IssueShow do
end
subject do
described_class.new(project, user).execute(regex_match)
described_class.new(project, chat_name).execute(regex_match)
end
context 'the issue exists' do
......
......@@ -14,4 +14,24 @@ describe ChatName do
it { is_expected.to validate_uniqueness_of(:user_id).scoped_to(:service_id) }
it { is_expected.to validate_uniqueness_of(:chat_id).scoped_to(:service_id, :team_id) }
describe '#update_last_used_at', :clean_gitlab_redis_shared_state do
it 'updates the last_used_at timestamp' do
expect(subject.last_used_at).to be_nil
subject.update_last_used_at
expect(subject.last_used_at).to be_present
end
it 'does not update last_used_at if it was recently updated' do
subject.update_last_used_at
time = subject.last_used_at
subject.update_last_used_at
expect(subject.last_used_at).to eq(time)
end
end
end
require 'spec_helper'
describe ChatNames::FindUserService do
describe ChatNames::FindUserService, :clean_gitlab_redis_shared_state do
describe '#execute' do
let(:service) { create(:service) }
......@@ -13,21 +13,30 @@ describe ChatNames::FindUserService do
context 'when existing user is requested' do
let(:params) { { team_id: chat_name.team_id, user_id: chat_name.chat_id } }
it 'returns the existing user' do
is_expected.to eq(user)
it 'returns the existing chat_name' do
is_expected.to eq(chat_name)
end
it 'updates when last time chat name was used' do
it 'updates the last used timestamp if one is not already set' do
expect(chat_name.last_used_at).to be_nil
subject
initial_last_used = chat_name.reload.last_used_at
expect(initial_last_used).to be_present
expect(chat_name.reload.last_used_at).to be_present
end
it 'only updates an existing timestamp once within a certain time frame' do
service = described_class.new(service, params)
expect(chat_name.last_used_at).to be_nil
service.execute
time = chat_name.reload.last_used_at
Timecop.travel(2.days.from_now) { described_class.new(service, params).execute }
service.execute
expect(chat_name.reload.last_used_at).to be > initial_last_used
expect(chat_name.reload.last_used_at).to eq(time)
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