Merge branch 'slash-commands-changes-for-chatops' into 'master'
Expose ChatName objects to slash commands See merge request gitlab-org/gitlab-ce!17295
This commit is contained in:
commit
60503057bc
|
@ -1,4 +1,6 @@
|
|||
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
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -1,6 +1,6 @@
|
|||
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
|
||||
|
||||
Timecop.travel(2.days.from_now) { described_class.new(service, params).execute }
|
||||
it 'only updates an existing timestamp once within a certain time frame' do
|
||||
service = described_class.new(service, params)
|
||||
|
||||
expect(chat_name.reload.last_used_at).to be > initial_last_used
|
||||
expect(chat_name.last_used_at).to be_nil
|
||||
|
||||
service.execute
|
||||
|
||||
time = chat_name.reload.last_used_at
|
||||
|
||||
service.execute
|
||||
|
||||
expect(chat_name.reload.last_used_at).to eq(time)
|
||||
end
|
||||
end
|
||||
|
||||
|
|
Loading…
Reference in New Issue