Add most of specs for chat names

app/services/chat_names/request_service.rb → app/services/chat_names/authorize_user_service.rb

 module ChatNames
-  class RequestService
+  class AuthorizeUserService
     include Gitlab::Routing.url_helpers
 
     def initialize(service, params)
@@ -8,13 +8,19 @@ module ChatNames
     end
 
     def execute
-      token = chat_name_token.store!(chat_name_params)
+      return unless chat_name_params.values.all?(&:present?)
+
+      token = request_token
 
       new_profile_chat_name_url(token: token) if token
     end
 
     private
 
+    def request_token
+      chat_name_token.store!(chat_name_params)
+    end
+
     def chat_name_token
       Gitlab::ChatNameToken.new
     end

app/services/chat_names/find_user_service.rb

 module ChatNames
   class FindUserService
-    def initialize(chat_names, params)
-      @chat_names = chat_names
+    def initialize(service, params)
+      @service = service
       @params = params
     end
 
     def execute
-      @chat_names.find_by(
+      chat_name = find_chat_name
+      return unless chat_name
+
+      chat_name.update(used_at: Time.now)
+      chat_name.user
+    end
+
+    private
+
+    def find_chat_name
+      ChatName.find_by(
+        service: @service,
         team_id: @params[:team_id],
         chat_id: @params[:user_id]
       )

app/views/profiles/chat_names/index.html.haml

-- page_title "Chat"
+- page_title 'Chat'
 = render 'profiles/head'
 
 .row.prepend-top-default
@@ -20,7 +20,7 @@
               %th Service
               %th Team domain
               %th Nickname
-              %th Created
+              %th Last used
               %th
           %tbody
             - @chat_names.each do |chat_name|
@@ -41,8 +41,14 @@
                       = chat_name.service.title
                 %td= chat_name.team_domain
                 %td= chat_name.chat_name
-                %td= chat_name.created_at
-                %td= link_to "Remove", profile_chat_name_path(chat_name), method: :delete, class: "btn btn-danger pull-right", data: { confirm: "Are you sure you want to revoke this nickname?" }
+                %td=
+                  - if chat_name.used_at
+                    time_ago_with_tooltip(chat_name.used_at)
+                  - else
+                    Never
+
+                %td
+                  = link_to 'Remove', profile_chat_name_path(chat_name), method: :delete, class: 'btn btn-danger pull-right', data: { confirm: 'Are you sure you want to revoke this nickname?' }
 
     - else
       .settings-message.text-center

db/migrate/20161113184239_create_user_chat_names_table.rb

@@ -5,13 +5,14 @@ class CreateUserChatNamesTable < ActiveRecord::Migration
 
   def change
     create_table :chat_names do |t|
-      t.integer "user_id", null: false
-      t.integer "service_id", null: false
-      t.string "team_id"
-      t.string "team_domain"
-      t.string "chat_id"
-      t.string "chat_name"
-      t.timestamps
+      t.integer :user_id, null: false
+      t.integer :service_id, null: false
+      t.string :team_id, null: false
+      t.string :team_domain
+      t.string :chat_id, null: false
+      t.string :chat_name
+      t.datetime :used_at
+      t.timestamps null: false
     end
 
     add_index :chat_names, [:user_id, :service_id], unique: true

db/schema.rb

@@ -152,15 +152,16 @@ ActiveRecord::Schema.define(version: 20161113184239) do
   create_table "chat_names", force: :cascade do |t|
     t.integer "user_id", null: false
     t.integer "service_id", null: false
-    t.string "team_id"
+    t.string "team_id", null: false
     t.string "team_domain"
-    t.string "chat_id"
+    t.string "chat_id", null: false
     t.string "chat_name"
-    t.datetime "created_at"
-    t.datetime "updated_at"
+    t.datetime "used_at"
+    t.datetime "created_at", null: false
+    t.datetime "updated_at", null: false
   end
 
-  add_index "chat_names", ["service_id", "team_id", "user_id"], name: "index_chat_names_on_service_id_and_team_id_and_user_id", unique: true, using: :btree
+  add_index "chat_names", ["service_id", "team_id", "chat_id"], name: "index_chat_names_on_service_id_and_team_id_and_chat_id", unique: true, using: :btree
   add_index "chat_names", ["user_id", "service_id"], name: "index_chat_names_on_user_id_and_service_id", unique: true, using: :btree
 
   create_table "ci_application_settings", force: :cascade do |t|

lib/gitlab/chat_name_token.rb

@@ -5,7 +5,7 @@ module Gitlab
     attr_reader :token
 
     TOKEN_LENGTH = 50
-    EXPIRY_TIME = 1800
+    EXPIRY_TIME = 10.minutes # 10 minutes
 
     def initialize(token = new_token)
       @token = token

spec/factories/chat_names.rb

+FactoryGirl.define do
+  factory :chat_name, class: ChatName do
+    user factory: :user
+    service factory: :service
+
+    team_id 'T0001'
+    team_domain 'Awesome Team'
+
+    sequence :chat_id do |n|
+      "U#{n}"
+    end
+    sequence :chat_name do |n|
+      "user#{n}"
+    end
+  end
+end

spec/lib/gitlab/chat_name_token_spec.rb

+require 'spec_helper'
+
+describe Gitlab::ChatNameToken, lib: true do
+  context 'when using unknown token' do
+    let(:token) { }
+
+    subject { described_class.new(token).get }
+
+    it 'returns empty data' do
+      is_expected.to be_nil
+    end
+  end
+
+  context 'when storing data' do
+    let(:data) {
+      { key: 'value' }
+    }
+
+    subject { described_class.new(@token) }
+
+    before do
+      @token = described_class.new.store!(data)
+    end
+
+    it 'returns stored data' do
+      expect(subject.get).to eq(data)
+    end
+
+    context 'and after deleting them' do
+      before do
+        subject.delete
+      end
+
+      it 'data are removed' do
+        expect(subject.get).to be_nil
+      end
+    end
+  end
+end

spec/models/chat_name_spec.rb

+require 'spec_helper'
+
+describe ChatName, models: true do
+  subject { create(:chat_name) }
+
+  it { is_expected.to belong_to(:service) }
+  it { is_expected.to belong_to(:user) }
+
+  it { is_expected.to validate_presence_of(:user) }
+  it { is_expected.to validate_presence_of(:service) }
+  it { is_expected.to validate_presence_of(:team_id) }
+  it { is_expected.to validate_presence_of(:chat_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) }
+end

spec/models/user_spec.rb

@@ -33,6 +33,7 @@ describe User, models: true do
     it { is_expected.to have_many(:award_emoji).dependent(:destroy) }
     it { is_expected.to have_many(:builds).dependent(:nullify) }
     it { is_expected.to have_many(:pipelines).dependent(:nullify) }
+    it { is_expected.to have_many(:chat_names).dependent(:destroy) }
 
     describe '#group_members' do
       it 'does not include group memberships for which user is a requester' do

spec/services/chat_names/authorize_user_service_spec.rb

+require 'spec_helper'
+
+describe ChatNames::AuthorizeUserService, services: true do
+  describe '#execute' do
+    let(:service) { create(:service) }
+
+    subject { described_class.new(service, params).execute }
+
+    context 'when all parameters are valid' do
+      let(:params) { { team_id: 'T0001', team_domain: 'myteam', user_id: 'U0001', user_name: 'user' } }
+
+      it 'requests a new token' do
+        is_expected.to include('http')
+        is_expected.to include('://')
+        is_expected.to include('token=')
+      end
+    end
+
+    context 'when there are missing parameters' do
+      let(:params) { { } }
+
+      it 'does not request a new token' do
+        is_expected.to be_nil
+      end
+    end
+  end
+end

spec/services/chat_names/find_user_service_spec.rb

+require 'spec_helper'
+
+describe ChatNames::FindUserService, services: true do
+  describe '#execute' do
+    let(:service) { create(:service) }
+
+    subject { described_class.new(service, params).execute }
+
+    context 'find user mapping' do
+      let(:user) { create(:user) }
+      let!(:chat_name) { create(:chat_name, user: user, service: service) }
+
+      context 'when existing user is requested' do
+        let(:params) { { team_id: chat_name.team_id, user_id: chat_name.chat_id } }
+
+        it 'returns existing user' do
+          is_expected.to eq(user)
+        end
+
+        it 'updates when last time chat name was used' do
+          subject
+
+          expect(chat_name.reload.used_at).to be_like_time(Time.now)
+        end
+      end
+
+      context 'when different user is requested' do
+        let(:params) { { team_id: chat_name.team_id, user_id: 'non-existing-user' } }
+
+        it 'returns existing user' do
+          is_expected.to be_nil
+        end
+      end
+    end
+  end
+end