Improve code design after code review

Author: ayufan

app/controllers/profiles/chat_names_controller.rb

@@ -13,7 +13,7 @@ def create
     new_chat_name = current_user.chat_names.new(chat_name_params)
 
     if new_chat_name.save
-      flash[:notice] = "Authorized chat nickname #{new_chat_name.chat_name}"
+      flash[:notice] = "Authorized #{new_chat_name.chat_name}"
     else
       flash[:alert] = "Could not authorize chat nickname. Try again!"
     end
@@ -34,7 +34,7 @@ def destroy
     @chat_name = chat_names.find(params[:id])
 
     if @chat_name.destroy
-      flash[:notice] = "Delete chat nickname: #{@chat_name.chat_name}!"
+      flash[:notice] = "Deleted chat nickname: #{@chat_name.chat_name}!"
     else
       flash[:alert] = "Could not delete chat nickname #{@chat_name.chat_name}."
     end

app/services/chat_names/find_user_service.rb

@@ -9,7 +9,7 @@ def execute
       chat_name = find_chat_name
       return unless chat_name
 
-      chat_name.update(used_at: Time.now)
+      chat_name.update(last_used_at: Time.now)
       chat_name.user
     end
 

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

@@ -6,7 +6,7 @@
     %h4.prepend-top-0
       = page_title
     %p
-      You can see your Chat integrations.
+      You can see your Chat accounts.
 
   .col-lg-9
     %h5 Active chat names (#{@chat_names.length})
@@ -39,11 +39,13 @@
                       = link_to service.title, edit_namespace_project_service_path(project.namespace, project, service)
                     - else
                       = chat_name.service.title
-                %td= chat_name.team_domain
-                %td= chat_name.chat_name
-                %td=
-                  - if chat_name.used_at
-                    time_ago_with_tooltip(chat_name.used_at)
+                %td
+                  = chat_name.team_domain
+                %td
+                  = chat_name.chat_name
+                %td
+                  - if chat_name.last_used_at
+                    time_ago_with_tooltip(chat_name.last_used_at)
                   - else
                     Never
 

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

@@ -1,11 +1,11 @@
 %h3.page-title Authorization required
 %main{:role => "main"}
   %p.h4
-    Authorize the chat user
+    Authorize
     %strong.text-info= @chat_name_params[:chat_name]
     to use your account?
 
-  %hr/
+  %hr
   .actions
     = form_tag profile_chat_names_path, method: :post do
       = hidden_field_tag :token, @chat_name_token.token

db/migrate/20161113184239_create_user_chat_names_table.rb

@@ -11,7 +11,7 @@ def change
       t.string :team_domain
       t.string :chat_id, null: false
       t.string :chat_name
-      t.datetime :used_at
+      t.datetime :last_used_at
       t.timestamps null: false
     end
 

db/schema.rb

@@ -159,7 +159,7 @@
     t.string "team_domain"
     t.string "chat_id", null: false
     t.string "chat_name"
-    t.datetime "used_at"
+    t.datetime "last_used_at"
     t.datetime "created_at", null: false
     t.datetime "updated_at", null: false
   end

lib/gitlab/chat_name_token.rb

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

spec/lib/gitlab/chat_name_token_spec.rb

@@ -12,9 +12,7 @@
   end
 
   context 'when storing data' do
-    let(:data) {
-      { key: 'value' }
-    }
+    let(:data) { { key: 'value' } }
 
     subject { described_class.new(@token) }
 

spec/services/chat_names/authorize_user_service_spec.rb

@@ -10,9 +10,7 @@
       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=')
+        is_expected.to be_url
       end
     end
 

spec/services/chat_names/find_user_service_spec.rb

@@ -20,7 +20,7 @@
         it 'updates when last time chat name was used' do
           subject
 
-          expect(chat_name.reload.used_at).to be_like_time(Time.now)
+          expect(chat_name.reload.last_used_at).to be_like_time(Time.now)
         end
       end
 

spec/support/matchers/be_url.rb

@@ -0,0 +1,5 @@
+RSpec::Matchers.define :be_url do |_|
+  match do |actual|
+    URI.parse(actual) rescue false
+  end
+end