Commit 36335ba9 authored by Yorick Peterse's avatar Yorick Peterse

Expose ChatName objects to slash commands

Instead of only exposing a User to slash commands we now also expose the
ChatName object that the User object is retrieved from. This is
necessary for GitLab Chatops as we need for example the user ID of the
chat user.
parent 3ffd40ae
class ChatName < ActiveRecord::Base class ChatName < ActiveRecord::Base
LAST_USED_AT_INTERVAL = 1.hour
belongs_to :service belongs_to :service
belongs_to :user belongs_to :user
...@@ -9,4 +11,23 @@ class ChatName < ActiveRecord::Base ...@@ -9,4 +11,23 @@ class ChatName < ActiveRecord::Base
validates :user_id, uniqueness: { scope: [:service_id] } validates :user_id, uniqueness: { scope: [:service_id] }
validates :chat_id, uniqueness: { scope: [:service_id, :team_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 end
...@@ -30,10 +30,10 @@ class SlashCommandsService < Service ...@@ -30,10 +30,10 @@ class SlashCommandsService < Service
def trigger(params) def trigger(params)
return unless valid_token?(params[:token]) return unless valid_token?(params[:token])
user = find_chat_user(params) chat_user = find_chat_user(params)
if user if chat_user&.user
Gitlab::SlashCommands::Command.new(project, user, params).execute Gitlab::SlashCommands::Command.new(project, chat_user, params).execute
else else
url = authorize_chat_name_url(params) url = authorize_chat_name_url(params)
Gitlab::SlashCommands::Presenters::Access.new(url).authorize Gitlab::SlashCommands::Presenters::Access.new(url).authorize
......
...@@ -9,8 +9,8 @@ module ChatNames ...@@ -9,8 +9,8 @@ module ChatNames
chat_name = find_chat_name chat_name = find_chat_name
return unless chat_name return unless chat_name
chat_name.touch(:last_used_at) chat_name.update_last_used_at
chat_name.user chat_name
end end
private private
......
...@@ -31,10 +31,11 @@ module Gitlab ...@@ -31,10 +31,11 @@ module Gitlab
raise NotImplementedError raise NotImplementedError
end end
attr_accessor :project, :current_user, :params attr_accessor :project, :current_user, :params, :chat_name
def initialize(project, user, params = {}) def initialize(project, chat_name, params = {})
@project, @current_user, @params = project, user, params.dup @project, @current_user, @params = project, chat_name.user, params.dup
@chat_name = chat_name
end end
private private
......
...@@ -13,12 +13,13 @@ module Gitlab ...@@ -13,12 +13,13 @@ module Gitlab
if command if command
if command.allowed?(project, current_user) if command.allowed?(project, current_user)
command.new(project, current_user, params).execute(match) command.new(project, chat_name, params).execute(match)
else else
Gitlab::SlashCommands::Presenters::Access.new.access_denied Gitlab::SlashCommands::Presenters::Access.new.access_denied
end end
else 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
end end
......
...@@ -3,10 +3,11 @@ require 'spec_helper' ...@@ -3,10 +3,11 @@ require 'spec_helper'
describe Gitlab::SlashCommands::Command do describe Gitlab::SlashCommands::Command do
let(:project) { create(:project) } let(:project) { create(:project) }
let(:user) { create(:user) } let(:user) { create(:user) }
let(:chat_name) { double(:chat_name, user: user) }
describe '#execute' do describe '#execute' do
subject do subject do
described_class.new(project, user, params).execute described_class.new(project, chat_name, params).execute
end end
context 'when no command is available' do context 'when no command is available' do
...@@ -88,7 +89,7 @@ describe Gitlab::SlashCommands::Command do ...@@ -88,7 +89,7 @@ describe Gitlab::SlashCommands::Command do
end end
describe '#match_command' do 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 context 'IssueShow is triggered' do
let(:params) { { text: 'issue show 123' } } let(:params) { { text: 'issue show 123' } }
......
...@@ -4,6 +4,7 @@ describe Gitlab::SlashCommands::Deploy do ...@@ -4,6 +4,7 @@ describe Gitlab::SlashCommands::Deploy do
describe '#execute' do describe '#execute' do
let(:project) { create(:project) } let(:project) { create(:project) }
let(:user) { create(:user) } let(:user) { create(:user) }
let(:chat_name) { double(:chat_name, user: user) }
let(:regex_match) { described_class.match('deploy staging to production') } let(:regex_match) { described_class.match('deploy staging to production') }
before do before do
...@@ -16,7 +17,7 @@ describe Gitlab::SlashCommands::Deploy do ...@@ -16,7 +17,7 @@ describe Gitlab::SlashCommands::Deploy do
end end
subject do subject do
described_class.new(project, user).execute(regex_match) described_class.new(project, chat_name).execute(regex_match)
end end
context 'if no environment is defined' do context 'if no environment is defined' do
......
...@@ -4,6 +4,7 @@ describe Gitlab::SlashCommands::IssueNew do ...@@ -4,6 +4,7 @@ describe Gitlab::SlashCommands::IssueNew do
describe '#execute' do describe '#execute' do
let(:project) { create(:project) } let(:project) { create(:project) }
let(:user) { create(:user) } let(:user) { create(:user) }
let(:chat_name) { double(:chat_name, user: user) }
let(:regex_match) { described_class.match("issue create bird is the word") } let(:regex_match) { described_class.match("issue create bird is the word") }
before do before do
...@@ -11,7 +12,7 @@ describe Gitlab::SlashCommands::IssueNew do ...@@ -11,7 +12,7 @@ describe Gitlab::SlashCommands::IssueNew do
end end
subject do subject do
described_class.new(project, user).execute(regex_match) described_class.new(project, chat_name).execute(regex_match)
end end
context 'without description' do context 'without description' do
......
...@@ -6,10 +6,11 @@ describe Gitlab::SlashCommands::IssueSearch do ...@@ -6,10 +6,11 @@ describe Gitlab::SlashCommands::IssueSearch do
let!(:confidential) { create(:issue, :confidential, project: project, title: 'mepmep find') } let!(:confidential) { create(:issue, :confidential, project: project, title: 'mepmep find') }
let(:project) { create(:project) } let(:project) { create(:project) }
let(:user) { create(:user) } let(:user) { create(:user) }
let(:chat_name) { double(:chat_name, user: user) }
let(:regex_match) { described_class.match("issue search find") } let(:regex_match) { described_class.match("issue search find") }
subject do subject do
described_class.new(project, user).execute(regex_match) described_class.new(project, chat_name).execute(regex_match)
end end
context 'when the user has no access' do context 'when the user has no access' do
......
...@@ -5,6 +5,7 @@ describe Gitlab::SlashCommands::IssueShow do ...@@ -5,6 +5,7 @@ describe Gitlab::SlashCommands::IssueShow do
let(:issue) { create(:issue, project: project) } let(:issue) { create(:issue, project: project) }
let(:project) { create(:project) } let(:project) { create(:project) }
let(:user) { issue.author } let(:user) { issue.author }
let(:chat_name) { double(:chat_name, user: user) }
let(:regex_match) { described_class.match("issue show #{issue.iid}") } let(:regex_match) { described_class.match("issue show #{issue.iid}") }
before do before do
...@@ -12,7 +13,7 @@ describe Gitlab::SlashCommands::IssueShow do ...@@ -12,7 +13,7 @@ describe Gitlab::SlashCommands::IssueShow do
end end
subject do subject do
described_class.new(project, user).execute(regex_match) described_class.new(project, chat_name).execute(regex_match)
end end
context 'the issue exists' do context 'the issue exists' do
......
...@@ -14,4 +14,24 @@ describe ChatName 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(:user_id).scoped_to(:service_id) }
it { is_expected.to validate_uniqueness_of(:chat_id).scoped_to(:service_id, :team_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 end
require 'spec_helper' require 'spec_helper'
describe ChatNames::FindUserService do describe ChatNames::FindUserService, :clean_gitlab_redis_shared_state do
describe '#execute' do describe '#execute' do
let(:service) { create(:service) } let(:service) { create(:service) }
...@@ -13,21 +13,30 @@ describe ChatNames::FindUserService do ...@@ -13,21 +13,30 @@ describe ChatNames::FindUserService do
context 'when existing user is requested' do context 'when existing user is requested' do
let(:params) { { team_id: chat_name.team_id, user_id: chat_name.chat_id } } let(:params) { { team_id: chat_name.team_id, user_id: chat_name.chat_id } }
it 'returns the existing user' do it 'returns the existing chat_name' do
is_expected.to eq(user) is_expected.to eq(chat_name)
end 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 expect(chat_name.last_used_at).to be_nil
subject subject
initial_last_used = chat_name.reload.last_used_at expect(chat_name.reload.last_used_at).to be_present
expect(initial_last_used).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
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