apply codestyle and implementation changes to the respective feature codepersonal_access_token_api_and_impersonation_token

10 files changed, 107 insertions, 168 deletions

diff --git a/app/assets/stylesheets/pages/settings.scss b/app/assets/stylesheets/pages/settings.scss
index 4a8e4344851..3889deee21a 100644
--- a/app/assets/stylesheets/pages/settings.scss
+++ b/app/assets/stylesheets/pages/settings.scss
@@ -25,7 +25,7 @@
   padding-top: 0;
 }
 
-.impersonation-token-token-container {
+.token-token-container {
   #impersonation-token-token {
     width: 80%;
     display: inline;
diff --git a/app/controllers/admin/impersonation_tokens_controller.rb b/app/controllers/admin/impersonation_tokens_controller.rb
index 448f2c881a1..d26004539b5 100644
--- a/app/controllers/admin/impersonation_tokens_controller.rb
+++ b/app/controllers/admin/impersonation_tokens_controller.rb
@@ -1,12 +1,12 @@
 class Admin::ImpersonationTokensController < Admin::ApplicationController
-  before_action :user, :finder
+  before_action :user
 
   def index
     set_index_vars
   end
 
   def create
-    @impersonation_token = finder.execute.build(impersonation_token_params)
+    @impersonation_token = finder.build(impersonation_token_params)
 
     if @impersonation_token.save
       flash[:impersonation_token] = @impersonation_token.token
@@ -18,7 +18,7 @@ class Admin::ImpersonationTokensController < Admin::ApplicationController
   end
 
   def revoke
-    @impersonation_token = finder.execute(id: params[:id])
+    @impersonation_token = finder.find(params[:id])
 
     if @impersonation_token.revoke!
       flash[:notice] = "Revoked impersonation token #{@impersonation_token.name}!"
@@ -35,8 +35,8 @@ class Admin::ImpersonationTokensController < Admin::ApplicationController
     @user ||= User.find_by!(username: params[:user_id])
   end
 
-  def finder
-    @finder ||= PersonalAccessTokensFinder.new(user: user, impersonation: true)
+  def finder(options = {})
+    PersonalAccessTokensFinder.new({ user: user, impersonation: true }.merge(options))
   end
 
   def impersonation_token_params
@@ -44,12 +44,10 @@ class Admin::ImpersonationTokensController < Admin::ApplicationController
   end
 
   def set_index_vars
-    finder.params[:state] = 'active'
-    @impersonation_token ||= finder.execute.build
     @scopes = Gitlab::Auth::SCOPES
-    finder.params[:order] = :expires_at
-    @active_impersonation_tokens = finder.execute
-    finder.params[:state] = 'inactive'
-    @inactive_impersonation_tokens = finder.execute
+
+    @impersonation_token ||= finder.build
+    @inactive_impersonation_tokens = finder(state: 'inactive').execute
+    @active_impersonation_tokens = finder(state: 'active').execute.order(:expires_at)
   end
 end
diff --git a/app/controllers/profiles/personal_access_tokens_controller.rb b/app/controllers/profiles/personal_access_tokens_controller.rb
index 2188350f2fd..d1f2374e9eb 100644
--- a/app/controllers/profiles/personal_access_tokens_controller.rb
+++ b/app/controllers/profiles/personal_access_tokens_controller.rb
@@ -1,12 +1,10 @@
 class Profiles::PersonalAccessTokensController < Profiles::ApplicationController
-  before_action :finder
-
   def index
     set_index_vars
   end
 
   def create
-    @personal_access_token = finder.execute.build(personal_access_token_params)
+    @personal_access_token = finder.build(personal_access_token_params)
 
     if @personal_access_token.save
       flash[:personal_access_token] = @personal_access_token.token
@@ -18,7 +16,7 @@ class Profiles::PersonalAccessTokensController < Profiles::ApplicationController
   end
 
   def revoke
-    @personal_access_token = finder.execute(id: params[:id])
+    @personal_access_token = finder.find(params[:id])
 
     if @personal_access_token.revoke!
       flash[:notice] = "Revoked personal access token #{@personal_access_token.name}!"
@@ -31,8 +29,8 @@ class Profiles::PersonalAccessTokensController < Profiles::ApplicationController
 
   private
 
-  def finder
-    @finder ||= PersonalAccessTokensFinder.new(user: current_user, impersonation: false)
+  def finder(options = {})
+    PersonalAccessTokensFinder.new({ user: current_user, impersonation: false }.merge(options))
   end
 
   def personal_access_token_params
@@ -40,12 +38,10 @@ class Profiles::PersonalAccessTokensController < Profiles::ApplicationController
   end
 
   def set_index_vars
-    finder.params[:state] = 'active'
-    @personal_access_token ||= finder.execute.build
     @scopes = Gitlab::Auth::SCOPES
-    finder.params[:order] = :expires_at
-    @active_personal_access_tokens = finder.execute
-    finder.params[:state] = 'inactive'
-    @inactive_personal_access_tokens = finder.execute
+
+    @personal_access_token = finder.build
+    @inactive_personal_access_tokens = finder(state: 'inactive').execute
+    @active_personal_access_tokens = finder(state: 'active').execute.order(:expires_at)
   end
 end
diff --git a/app/finders/personal_access_tokens_finder.rb b/app/finders/personal_access_tokens_finder.rb
index 7b9a2f6c0bb..760166b453f 100644
--- a/app/finders/personal_access_tokens_finder.rb
+++ b/app/finders/personal_access_tokens_finder.rb
@@ -1,36 +1,34 @@
 class PersonalAccessTokensFinder
   attr_accessor :params
 
+  delegate :build, :find, :find_by, to: :execute
+
   def initialize(params = {})
     @params = params
   end
 
-  def execute(token: nil, id: nil)
-    tokens = by_impersonation
-
-    return tokens.find_by_token(token) if token
-    return tokens.find_by_id(id) if id
-
-    tokens = by_state(tokens)
-    tokens.order(@params[:order]) if @params[:order]
-
-    tokens
+  def execute
+    tokens = PersonalAccessToken.all
+    tokens = by_user(tokens)
+    tokens = by_impersonation(tokens)
+    by_state(tokens)
   end
 
   private
 
-  def personal_access_tokens
-    @params[:user] ? @params[:user].personal_access_tokens : PersonalAccessToken.all
+  def by_user(tokens)
+    return tokens unless @params[:user]
+    tokens.where(user: @params[:user])
   end
 
-  def by_impersonation
+  def by_impersonation(tokens)
     case @params[:impersonation]
     when true
-      personal_access_tokens.with_impersonation
+      tokens.with_impersonation
     when false
-      personal_access_tokens.without_impersonation
+      tokens.without_impersonation
     else
-      personal_access_tokens
+      tokens
     end
   end
 
diff --git a/app/models/user.rb b/app/models/user.rb
index 6fb5ac4a4ef..187627247d2 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -322,8 +322,7 @@ class User < ActiveRecord::Base
     end
 
     def find_by_personal_access_token(token_string)
-      personal_access_token = PersonalAccessToken.active.find_by_token(token_string) if token_string
-      personal_access_token&.user
+      PersonalAccessTokensFinder.new(state: 'active').find_by(token: token_string)&.user
     end
 
     # Returns a user for the given SSH key.
diff --git a/app/views/admin/impersonation_tokens/index.html.haml b/app/views/admin/impersonation_tokens/index.html.haml
index 9116384e322..1378dde52ab 100644
--- a/app/views/admin/impersonation_tokens/index.html.haml
+++ b/app/views/admin/impersonation_tokens/index.html.haml
@@ -3,63 +3,6 @@
 
 .row.prepend-top-default
   .col-lg-12
-    %h5.prepend-top-0
-      Add a Impersonation Token
-    %p.profile-settings-content
-      Pick a name for the application, and we'll give the respective user a unique token.
-    = render "shared/personal_access_tokens_form", path: admin_user_impersonation_tokens_path, impersonation: true, personal_access_token: @impersonation_token, scopes: @scopes
+    = render "shared/personal_access_tokens_form", path: admin_user_impersonation_tokens_path, impersonation: true, token: @impersonation_token, scopes: @scopes
 
-    %hr
-
-    %h5 Active Impersonation Tokens (#{@active_impersonation_tokens.length})
-    %p.profile-settings-content
-      To see all the user's personal access tokens you must impersonate first
-    - if @active_impersonation_tokens.present?
-      .table-responsive
-        %table.table.active-impersonation-tokens
-          %thead
-            %tr
-              %th Name
-              %th Created
-              %th Expires
-              %th Scopes
-              %th Token
-              %th
-          %tbody
-            - @active_impersonation_tokens.each do |impersonation_token|
-              %tr
-                %td= impersonation_token.name
-                %td= impersonation_token.created_at.to_date.to_s(:medium)
-                %td
-                  - if impersonation_token.expires?
-                    %span{ class: ('text-warning' if impersonation_token.expires_soon?) }
-                      In #{distance_of_time_in_words_to_now(impersonation_token.expires_at)}
-                  - else
-                    %span.impersonation_tokens-never-expires-label Never
-                %td= impersonation_token.scopes.present? ? impersonation_token.scopes.join(", ") : "<no scopes selected>"
-                %td.impersonation-token-token-container
-                  = text_field_tag 'impersonation-token-token', impersonation_token.token, readonly: true, class: "form-control"
-                  = clipboard_button(clipboard_text: impersonation_token.token)
-                %td= link_to "Revoke", revoke_admin_user_impersonation_token_path(id: impersonation_token.id, user_id: impersonation_token.user.username), method: :put, class: "btn btn-danger pull-right", data: { confirm: "Are you sure you want to revoke this impersonation token? This action cannot be undone." }
-    - else
-      .settings-message.text-center
-        This user has no active impersonation tokens.
-
-    %hr
-
-    %h5 Inactive Impersonation Tokens (#{@inactive_impersonation_tokens.length})
-    - if @inactive_impersonation_tokens.present?
-      .table-responsive
-        %table.table.inactive-impersonation-tokens
-          %thead
-            %tr
-              %th Name
-              %th Created
-          %tbody
-            - @inactive_impersonation_tokens.each do |token|
-              %tr
-                %td= token.name
-                %td= token.created_at.to_date.to_s(:medium)
-    - else
-      .settings-message.text-center
-        This user has no inactive impersonation tokens.
+    = render "shared/personal_access_tokens_table", impersonation: true, active_tokens: @active_impersonation_tokens, inactive_tokens: @inactive_impersonation_tokens
diff --git a/app/views/admin/users/_head.html.haml b/app/views/admin/users/_head.html.haml
index 1ded8fa6086..d20be373564 100644
--- a/app/views/admin/users/_head.html.haml
+++ b/app/views/admin/users/_head.html.haml
@@ -22,5 +22,5 @@
   = nav_link(controller: :identities) do
     = link_to "Identities", admin_user_identities_path(@user)
   = nav_link(controller: :impersonation_tokens) do
-    = link_to "Access Tokens", admin_user_impersonation_tokens_path(@user)
+    = link_to "Impersonation Tokens", admin_user_impersonation_tokens_path(@user)
 .append-bottom-default
diff --git a/app/views/profiles/personal_access_tokens/index.html.haml b/app/views/profiles/personal_access_tokens/index.html.haml
index 7b5121ed550..0645ecad496 100644
--- a/app/views/profiles/personal_access_tokens/index.html.haml
+++ b/app/views/profiles/personal_access_tokens/index.html.haml
@@ -24,66 +24,9 @@
 
       %hr
 
-    %h5.prepend-top-0
-      Add a Personal Access Token
-    %p.profile-settings-content
-      Pick a name for the application, and we'll give you a unique token.
-
-    = render "shared/personal_access_tokens_form", path: profile_personal_access_tokens_path, personal_access_token: @personal_access_token, scopes: @scopes
-
-    %hr
-
-    %h5 Active Personal Access Tokens (#{@active_personal_access_tokens.length})
-
-    - if @active_personal_access_tokens.present?
-      .table-responsive
-        %table.table.active-personal-access-tokens
-          %thead
-            %tr
-              %th Name
-              %th Created
-              %th Expires
-              %th Scopes
-              %th
-          %tbody
-            - @active_personal_access_tokens.each do |token|
-              %tr
-                %td= token.name
-                %td= token.created_at.to_date.to_s(:medium)
-                %td
-                  - if token.expires?
-                    %span{ class: ('text-warning' if token.expires_soon?) }
-                      In #{distance_of_time_in_words_to_now(token.expires_at)}
-                  - else
-                    %span.personal-access-tokens-never-expires-label Never
-                %td= token.scopes.present? ? token.scopes.join(", ") : "<no scopes selected>"
-                %td= link_to "Revoke", revoke_profile_personal_access_token_path(token), method: :put, class: "btn btn-danger pull-right", data: { confirm: "Are you sure you want to revoke this token? This action cannot be undone." }
-
-    - else
-      .settings-message.text-center
-        You don't have any active tokens yet.
-
-    %hr
-
-    %h5 Inactive Personal Access Tokens (#{@inactive_personal_access_tokens.length})
-
-    - if @inactive_personal_access_tokens.present?
-      .table-responsive
-        %table.table.inactive-personal-access-tokens
-          %thead
-            %tr
-              %th Name
-              %th Created
-          %tbody
-            - @inactive_personal_access_tokens.each do |token|
-              %tr
-                %td= token.name
-                %td= token.created_at.to_date.to_s(:medium)
-
-    - else
-      .settings-message.text-center
-        There are no inactive tokens.
+    = render "shared/personal_access_tokens_form", path: profile_personal_access_tokens_path, impersonation: false, token: @personal_access_token, scopes: @scopes
 
+    = render "shared/personal_access_tokens_table", impersonation: false, active_tokens: @active_personal_access_tokens, inactive_tokens: @inactive_personal_access_tokens
 
 :javascript
   $("#created-personal-access-token").click(function() {
diff --git a/app/views/shared/_personal_access_tokens_form.html.haml b/app/views/shared/_personal_access_tokens_form.html.haml
index 074eeb7d038..af4cc90f4a7 100644
--- a/app/views/shared/_personal_access_tokens_form.html.haml
+++ b/app/views/shared/_personal_access_tokens_form.html.haml
@@ -1,10 +1,13 @@
-- impersonation = impersonation || false
-- personal_access_token = local_assigns.fetch(:personal_access_token)
-- scopes = local_assigns.fetch(:scopes)
+- type = impersonation ? "Impersonation" : "Personal Access"
 
-= form_for personal_access_token, url: path, method: :post, html: { class: 'js-requires-input' } do |f|
+%h5.prepend-top-0
+  Add a #{type} Token
+%p.profile-settings-content
+  Pick a name for the application, and we'll give you a unique #{type} Token.
 
-  = form_errors(personal_access_token)
+= form_for token, url: path, method: :post, html: { class: 'js-requires-input' } do |f|
+
+  = form_errors(token)
 
   .form-group
     = f.label :name, class: 'label-light'
@@ -16,10 +19,9 @@
 
   .form-group
     = f.label :scopes, class: 'label-light'
-    = render 'shared/tokens/scopes_form', prefix: 'personal_access_token', token: personal_access_token, scopes: scopes
+    = render 'shared/tokens/scopes_form', prefix: 'personal_access_token', token: token, scopes: scopes
 
   .prepend-top-default
-    - type = impersonation ? "Impersonation" : "Personal Access"
     = f.submit "Create #{type} Token", class: "btn btn-create"
 
 :javascript
diff --git a/app/views/shared/_personal_access_tokens_table.html.haml b/app/views/shared/_personal_access_tokens_table.html.haml
new file mode 100644
index 00000000000..67a49815478
--- /dev/null
+++ b/app/views/shared/_personal_access_tokens_table.html.haml
@@ -0,0 +1,60 @@
+- type = impersonation ? "Impersonation" : "Personal Access"
+%hr
+
+%h5 Active #{type} Tokens (#{active_tokens.length})
+- if impersonation
+  %p.profile-settings-content
+    To see all the user's personal access tokens you must impersonate them first.
+
+- if active_tokens.present?
+  .table-responsive
+    %table.table.active-tokens
+      %thead
+        %tr
+          %th Name
+          %th Created
+          %th Expires
+          %th Scopes
+          - if impersonation
+            %th Token
+          %th
+      %tbody
+        - active_tokens.each do |token|
+          %tr
+            %td= token.name
+            %td= token.created_at.to_date.to_s(:medium)
+            %td
+              - if token.expires?
+                %span{ class: ('text-warning' if token.expires_soon?) }
+                  In #{distance_of_time_in_words_to_now(token.expires_at)}
+              - else
+                %span.token-never-expires-label Never
+            %td= token.scopes.present? ? token.scopes.join(", ") : "<no scopes selected>"
+            - if impersonation
+              %td.token-token-container
+                = text_field_tag 'impersonation-token-token', token.token, readonly: true, class: "form-control"
+                = clipboard_button(clipboard_text: token.token)
+            - path = impersonation ? revoke_admin_user_impersonation_token_path(token.user, token) : revoke_profile_personal_access_token_path(token)
+            %td= link_to "Revoke", path, method: :put, class: "btn btn-danger pull-right", data: { confirm: "Are you sure you want to revoke this #{type} Token? This action cannot be undone." }
+- else
+  .settings-message.text-center
+    This user has no active #{type} Tokens.
+
+%hr
+
+%h5 Inactive #{type} Tokens (#{inactive_tokens.length})
+- if inactive_tokens.present?
+  .table-responsive
+    %table.table.inactive-tokens
+      %thead
+        %tr
+          %th Name
+          %th Created
+      %tbody
+        - inactive_tokens.each do |token|
+          %tr
+            %td= token.name
+            %td= token.created_at.to_date.to_s(:medium)
+- else
+  .settings-message.text-center
+    This user has no inactive #{type} Tokens.