Merge branch 'dz-admin-user-cohorts-own-controller' into 'master'

Move admin cohorts to separate controller

See merge request gitlab-org/gitlab!64015

app/controllers/admin/cohorts_controller.rb

 # frozen_string_literal: true
 
 class Admin::CohortsController < Admin::ApplicationController
+  include Analytics::UniqueVisitsHelper
+
   feature_category :devops_reports
 
-  # Backwards compatibility. Remove it and routing in 14.0
-  # @see https://gitlab.com/gitlab-org/gitlab/-/issues/299303
   def index
-    redirect_to cohorts_admin_users_path
+    @cohorts = load_cohorts
+    track_cohorts_visit
+  end
+
+  private
+
+  def load_cohorts
+    cohorts_results = Rails.cache.fetch('cohorts', expires_in: 1.day) do
+      CohortsService.new.execute
+    end
+
+    CohortsSerializer.new.represent(cohorts_results)
+  end
+
+  def track_cohorts_visit
+    if request.format.html? && request.headers['DNT'] != '1'
+      track_visit('i_analytics_cohorts')
+    end
   end
 end

app/controllers/admin/users_controller.rb

@@ -2,9 +2,8 @@
 
 class Admin::UsersController < Admin::ApplicationController
   include RoutableActions
-  include Analytics::UniqueVisitsHelper
 
-  before_action :user, except: [:index, :cohorts, :new, :create]
+  before_action :user, except: [:index, :new, :create]
   before_action :check_impersonation_availability, only: :impersonate
   before_action :ensure_destroy_prerequisites_met, only: [:destroy]
   before_action :check_ban_user_feature_flag, only: [:ban]
@@ -14,7 +13,7 @@ class Admin::UsersController < Admin::ApplicationController
   PAGINATION_WITH_COUNT_LIMIT = 1000
 
   def index
-    return redirect_to cohorts_admin_users_path if params[:tab] == 'cohorts'
+    return redirect_to admin_cohorts_path if params[:tab] == 'cohorts'
 
     @users = User.filter_items(params[:filter]).order_name_asc
     @users = @users.search_with_secondary_emails(params[:search_query]) if params[:search_query].present?
@@ -24,11 +23,6 @@ class Admin::UsersController < Admin::ApplicationController
     @users = @users.without_count if paginate_without_count?
   end
 
-  def cohorts
-    @cohorts = load_cohorts
-    track_cohorts_visit
-  end
-
   def show
   end
 
@@ -376,20 +370,6 @@ class Admin::UsersController < Admin::ApplicationController
   def log_impersonation_event
     Gitlab::AppLogger.info(_("User %{current_user_username} has started impersonating %{username}") % { current_user_username: current_user.username, username: user.username })
   end
-
-  def load_cohorts
-    cohorts_results = Rails.cache.fetch('cohorts', expires_in: 1.day) do
-      CohortsService.new.execute
-    end
-
-    CohortsSerializer.new.represent(cohorts_results)
-  end
-
-  def track_cohorts_visit
-    if request.format.html? && request.headers['DNT'] != '1'
-      track_visit('i_analytics_cohorts')
-    end
-  end
 end
 
 Admin::UsersController.prepend_mod_with('Admin::UsersController')

app/views/admin/users/cohorts.html.haml → app/views/admin/cohorts/index.html.haml

 - page_title _("Users")
 
-= render 'tabs'
+= render 'admin/users/tabs'
 
 .tab-content
   .tab-pane.active

app/views/admin/users/_tabs.html.haml

@@ -3,5 +3,5 @@
     %a.nav-link{ href: admin_users_path, class: active_when(current_page?(admin_users_path)), role: 'tab' }
       = s_('AdminUsers|Users')
   %li.nav-item{ role: 'presentation' }
-    %a.nav-link{ href: cohorts_admin_users_path, class: active_when(current_page?(cohorts_admin_users_path)), role: 'tab' }
+    %a.nav-link{ href: admin_cohorts_path, class: active_when(current_page?(admin_cohorts_path)), role: 'tab' }
       = s_('AdminUsers|Cohorts')

app/views/layouts/nav/sidebar/_admin.html.haml

@@ -10,14 +10,14 @@
         %span.sidebar-context-title
           = _('Admin Area')
     %ul.sidebar-top-level-items{ data: { qa_selector: 'admin_sidebar_overview_submenu_content' } }
-      = nav_link(controller: %w(dashboard admin admin/projects users groups jobs runners gitaly_servers), html_options: {class: 'home'}) do
+      = nav_link(controller: %w(dashboard admin admin/projects users groups jobs runners gitaly_servers cohorts), html_options: {class: 'home'}) do
         = link_to admin_root_path, class: 'has-sub-items' do
           .nav-icon-container
             = sprite_icon('overview')
           %span.nav-item-name
             = _('Overview')
         %ul.sidebar-sub-level-items
-          = nav_link(controller: %w(dashboard admin admin/projects users groups jobs runners gitaly_servers), html_options: { class: "fly-out-top-item" } ) do
+          = nav_link(controller: %w(dashboard admin admin/projects users groups jobs runners gitaly_servers cohorts), html_options: { class: "fly-out-top-item" } ) do
             = link_to admin_root_path do
               %strong.fly-out-top-item-name
                 = _('Overview')
@@ -30,7 +30,7 @@
             = link_to admin_projects_path, title: _('Projects') do
               %span
                 = _('Projects')
-          = nav_link(controller: :users) do
+          = nav_link(controller: %w(users cohorts)) do
             = link_to admin_users_path, title: _('Users'), data: { qa_selector: 'users_overview_link' } do
               %span
                 = _('Users')

config/routes/admin.rb

@@ -10,12 +10,6 @@ namespace :admin do
       end
     end
 
-    collection do
-      scope '/-/' do
-        get :cohorts
-      end
-    end
-
     member do
       get :projects
       get :keys

spec/controllers/admin/cohorts_controller_spec.rb

@@ -9,9 +9,9 @@ RSpec.describe Admin::CohortsController do
     sign_in(user)
   end
 
-  it 'redirects to Overview->Users' do
-    get :index
-
-    expect(response).to redirect_to(cohorts_admin_users_path)
+  describe 'GET #index' do
+    it_behaves_like 'tracking unique visits', :index do
+      let(:target_id) { 'i_analytics_cohorts' }
+    end
   end
 end

spec/controllers/admin/users_controller_spec.rb

@@ -54,12 +54,6 @@ RSpec.describe Admin::UsersController do
     end
   end
 
-  describe 'GET #cohorts' do
-    it_behaves_like 'tracking unique visits', :cohorts do
-      let(:target_id) { 'i_analytics_cohorts' }
-    end
-  end
-
   describe 'GET :id' do
     it 'finds a user case-insensitively' do
       user = create(:user, username: 'CaseSensitive')

spec/features/admin/admin_users_spec.rb

@@ -15,7 +15,7 @@ RSpec.describe "Admin::Users" do
     let(:active_tab_selector) { '.nav-link.active' }
 
     it 'links to the Users tab' do
-      visit cohorts_admin_users_path
+      visit admin_cohorts_path
 
       within tabs_selector do
         click_link 'Users'
@@ -35,14 +35,14 @@ RSpec.describe "Admin::Users" do
         expect(page).to have_selector active_tab_selector, text: 'Cohorts'
       end
 
-      expect(page).to have_current_path(cohorts_admin_users_path)
+      expect(page).to have_current_path(admin_cohorts_path)
       expect(page).to have_selector active_tab_selector, text: 'Cohorts'
     end
 
     it 'redirects legacy route' do
       visit admin_users_path(tab: 'cohorts')
 
-      expect(page).to have_current_path(cohorts_admin_users_path)
+      expect(page).to have_current_path(admin_cohorts_path)
     end
   end