Expose ChatName objects to slash commandsslash-commands-changes-for-chatops

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.

12 files changed, 81 insertions, 24 deletions

diff --git a/app/models/chat_name.rb b/app/models/chat_name.rb
index f321db75eeb..fbd0f123341 100644
--- a/app/models/chat_name.rb
+++ b/app/models/chat_name.rb
@@ -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
diff --git a/app/models/project_services/slash_commands_service.rb b/app/models/project_services/slash_commands_service.rb
index eb4da68bb7e..37ea45109ae 100644
--- a/app/models/project_services/slash_commands_service.rb
+++ b/app/models/project_services/slash_commands_service.rb
@@ -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
diff --git a/app/services/chat_names/find_user_service.rb b/app/services/chat_names/find_user_service.rb
index 4f5c5567b42..d458b814183 100644
--- a/app/services/chat_names/find_user_service.rb
+++ b/app/services/chat_names/find_user_service.rb
@@ -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
diff --git a/lib/gitlab/slash_commands/base_command.rb b/lib/gitlab/slash_commands/base_command.rb
index cc3c9a50555..466554e398c 100644
--- a/lib/gitlab/slash_commands/base_command.rb
+++ b/lib/gitlab/slash_commands/base_command.rb
@@ -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
diff --git a/lib/gitlab/slash_commands/command.rb b/lib/gitlab/slash_commands/command.rb
index a78408b0519..85aaa6b0eba 100644
--- a/lib/gitlab/slash_commands/command.rb
+++ b/lib/gitlab/slash_commands/command.rb
@@ -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
 
diff --git a/spec/lib/gitlab/slash_commands/command_spec.rb b/spec/lib/gitlab/slash_commands/command_spec.rb
index 0173a45d480..e3447d974aa 100644
--- a/spec/lib/gitlab/slash_commands/command_spec.rb
+++ b/spec/lib/gitlab/slash_commands/command_spec.rb
@@ -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' } }
diff --git a/spec/lib/gitlab/slash_commands/deploy_spec.rb b/spec/lib/gitlab/slash_commands/deploy_spec.rb
index 74b5ef4bb26..0d57334aa4c 100644
--- a/spec/lib/gitlab/slash_commands/deploy_spec.rb
+++ b/spec/lib/gitlab/slash_commands/deploy_spec.rb
@@ -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
diff --git a/spec/lib/gitlab/slash_commands/issue_new_spec.rb b/spec/lib/gitlab/slash_commands/issue_new_spec.rb
index 3b077c58c50..8e7df946529 100644
--- a/spec/lib/gitlab/slash_commands/issue_new_spec.rb
+++ b/spec/lib/gitlab/slash_commands/issue_new_spec.rb
@@ -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
diff --git a/spec/lib/gitlab/slash_commands/issue_search_spec.rb b/spec/lib/gitlab/slash_commands/issue_search_spec.rb
index 35d01efc1bd..189e9592f1b 100644
--- a/spec/lib/gitlab/slash_commands/issue_search_spec.rb
+++ b/spec/lib/gitlab/slash_commands/issue_search_spec.rb
@@ -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
diff --git a/spec/lib/gitlab/slash_commands/issue_show_spec.rb b/spec/lib/gitlab/slash_commands/issue_show_spec.rb
index e5834d5a2ee..b1db1638237 100644
--- a/spec/lib/gitlab/slash_commands/issue_show_spec.rb
+++ b/spec/lib/gitlab/slash_commands/issue_show_spec.rb
@@ -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
diff --git a/spec/models/chat_name_spec.rb b/spec/models/chat_name_spec.rb
index e89e534d914..504bc710b25 100644
--- a/spec/models/chat_name_spec.rb
+++ b/spec/models/chat_name_spec.rb
@@ -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
diff --git a/spec/services/chat_names/find_user_service_spec.rb b/spec/services/chat_names/find_user_service_spec.rb
index 79aaac3aeb6..5734b10109a 100644
--- a/spec/services/chat_names/find_user_service_spec.rb
+++ b/spec/services/chat_names/find_user_service_spec.rb
@@ -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
+
+        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