7 files changed, 40 insertions, 11 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/ci/group_variable.rb b/app/models/ci/group_variable.rb
index afeae69ba39..1dd0e050ba9 100644
--- a/app/models/ci/group_variable.rb
+++ b/app/models/ci/group_variable.rb
@@ -6,7 +6,10 @@ module Ci
 
     belongs_to :group
 
-    validates :key, uniqueness: { scope: :group_id }
+    validates :key, uniqueness: {
+      scope: :group_id,
+      message: "(%{value}) has already been taken"
+    }
 
     scope :unprotected, -> { where(protected: false) }
   end
diff --git a/app/models/ci/variable.rb b/app/models/ci/variable.rb
index 67d3ec81b6f..7c71291de84 100644
--- a/app/models/ci/variable.rb
+++ b/app/models/ci/variable.rb
@@ -6,7 +6,10 @@ module Ci
 
     belongs_to :project
 
-    validates :key, uniqueness: { scope: [:project_id, :environment_scope] }
+    validates :key, uniqueness: {
+      scope: [:project_id, :environment_scope],
+      message: "(%{value}) has already been taken"
+    }
 
     scope :unprotected, -> { where(protected: false) }
   end
diff --git a/app/models/concerns/access_requestable.rb b/app/models/concerns/access_requestable.rb
index 62bc6b809f4..d502e7e54c6 100644
--- a/app/models/concerns/access_requestable.rb
+++ b/app/models/concerns/access_requestable.rb
@@ -8,6 +8,6 @@ module AccessRequestable
   extend ActiveSupport::Concern
 
   def request_access(user)
-    Members::RequestAccessService.new(self, user).execute
+    Members::RequestAccessService.new(user).execute(self)
   end
 end
diff --git a/app/models/member.rb b/app/models/member.rb
index 2d17795e62d..408e8b2d704 100644
--- a/app/models/member.rb
+++ b/app/models/member.rb
@@ -128,7 +128,7 @@ class Member < ActiveRecord::Base
       find_by(invite_token: invite_token)
     end
 
-    def add_user(source, user, access_level, existing_members: nil, current_user: nil, expires_at: nil)
+    def add_user(source, user, access_level, existing_members: nil, current_user: nil, expires_at: nil, ldap: false)
       # `user` can be either a User object, User ID or an email to be invited
       member = retrieve_member(source, user, existing_members)
       access_level = retrieve_access_level(access_level)
@@ -143,11 +143,13 @@ class Member < ActiveRecord::Base
 
       if member.request?
         ::Members::ApproveAccessRequestService.new(
-          source,
           current_user,
-          id: member.id,
           access_level: access_level
-        ).execute
+        ).execute(
+          member,
+          skip_authorization: ldap,
+          skip_log_audit_event: ldap
+        )
       else
         member.save
       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/models/user.rb b/app/models/user.rb
index 8610ca27b7f..982080763d2 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -431,7 +431,7 @@ class User < ActiveRecord::Base
   end
 
   def self.non_internal
-    where(Hash[internal_attributes.zip([[false, nil]] * internal_attributes.size)])
+    where(internal_attributes.map { |attr| "#{attr} IS NOT TRUE" }.join(" AND "))
   end
 
   #