Merge branch '7048_set_security_dashboard_as_default_view_for_groups_be' into 'master'

Set Security Dashboard as default view for groups

See merge request gitlab-org/gitlab-ee!7889

app/assets/javascripts/pages/groups/details/index.js

+import initGroupDetails from '../shared/group_details';
+
+document.addEventListener('DOMContentLoaded', () => {
+  initGroupDetails('details');
+});

app/assets/javascripts/pages/groups/shared/group_details.js

+/* eslint-disable no-new */
+
+import { getPagePath } from '~/lib/utils/common_utils';
+import { ACTIVE_TAB_SHARED, ACTIVE_TAB_ARCHIVED } from '~/groups/constants';
+import NewGroupChild from '~/groups/new_group_child';
+import notificationsDropdown from '~/notifications_dropdown';
+import NotificationsForm from '~/notifications_form';
+import ProjectsList from '~/projects_list';
+import ShortcutsNavigation from '~/behaviors/shortcuts/shortcuts_navigation';
+import GroupTabs from './group_tabs';
+
+export default function initGroupDetails(actionName = 'show') {
+  const newGroupChildWrapper = document.querySelector('.js-new-project-subgroup');
+  const loadableActions = [ACTIVE_TAB_SHARED, ACTIVE_TAB_ARCHIVED];
+  const paths = window.location.pathname.split('/');
+  const subpath = paths[paths.length - 1];
+  let action = loadableActions.includes(subpath) ? subpath : getPagePath(1);
+  if (actionName && action === actionName) {
+    action = 'show'; // 'show' resets GroupTabs to default action through base class
+  }
+
+  new GroupTabs({ parentEl: '.groups-listing', action });
+  new ShortcutsNavigation();
+  new NotificationsForm();
+  notificationsDropdown();
+  new ProjectsList();
+
+  if (newGroupChildWrapper) {
+    new NewGroupChild(newGroupChildWrapper);
+  }
+}

app/assets/javascripts/pages/groups/show/index.js

-/* eslint-disable no-new */
-
-import { getPagePath } from '~/lib/utils/common_utils';
-import { ACTIVE_TAB_SHARED, ACTIVE_TAB_ARCHIVED } from '~/groups/constants';
-import NewGroupChild from '~/groups/new_group_child';
-import notificationsDropdown from '~/notifications_dropdown';
-import NotificationsForm from '~/notifications_form';
-import ProjectsList from '~/projects_list';
-import ShortcutsNavigation from '~/behaviors/shortcuts/shortcuts_navigation';
-import GroupTabs from './group_tabs';
+import initGroupDetails from '../shared/group_details';
 
 document.addEventListener('DOMContentLoaded', () => {
-  const newGroupChildWrapper = document.querySelector('.js-new-project-subgroup');
-  const loadableActions = [ACTIVE_TAB_SHARED, ACTIVE_TAB_ARCHIVED];
-  const paths = window.location.pathname.split('/');
-  const subpath = paths[paths.length - 1];
-  const action = loadableActions.includes(subpath) ? subpath : getPagePath(1);
-
-  new GroupTabs({ parentEl: '.groups-listing', action });
-  new ShortcutsNavigation();
-  new NotificationsForm();
-  notificationsDropdown();
-  new ProjectsList();
-
-  if (newGroupChildWrapper) {
-    new NewGroupChild(newGroupChildWrapper);
-  }
+  initGroupDetails();
 });

app/assets/javascripts/pages/users/user_tabs.js

@@ -91,6 +91,7 @@ export default class UserTabs {
     this.actions = Object.keys(this.loaded);
     this.bindEvents();
 
+    // TODO: refactor to make this configurable via constructor params with a default value of 'show'
     if (this.action === 'show') {
       this.action = this.defaultAction;
     }

app/controllers/groups_controller.rb

@@ -58,11 +58,24 @@ class GroupsController < Groups::ApplicationController
 
   def show
     respond_to do |format|
-      format.html
+      format.html do
+        render_show_html
+      end
 
       format.atom do
-        load_events
-        render layout: 'xml.atom'
+        render_details_view_atom
+      end
+    end
+  end
+
+  def details
+    respond_to do |format|
+      format.html do
+        render_details_html
+      end
+
+      format.atom do
+        render_details_view_atom
       end
     end
   end
@@ -119,6 +132,19 @@ class GroupsController < Groups::ApplicationController
 
   protected
 
+  def render_show_html
+    render 'groups/show'
+  end
+
+  def render_details_html
+    render 'groups/show'
+  end
+
+  def render_details_view_atom
+    load_events
+    render layout: 'xml.atom', template: 'groups/show'
+  end
+
   # rubocop: disable CodeReuse/ActiveRecord
   def authorize_create_group!
     allowed = if params[:parent_id].present?
@@ -178,8 +204,8 @@ class GroupsController < Groups::ApplicationController
                   .includes(:namespace)
 
     @events = EventCollection
-      .new(@projects, offset: params[:offset].to_i, filter: event_filter)
-      .to_a
+                .new(@projects, offset: params[:offset].to_i, filter: event_filter)
+                .to_a
 
     Events::RenderService
       .new(current_user)

app/helpers/groups_helper.rb

@@ -4,6 +4,7 @@ module GroupsHelper
   def group_overview_nav_link_paths
     %w[
       groups#show
+      groups#details
       groups#activity
       groups#subgroups
       analytics#show

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

@@ -20,13 +20,14 @@
               = _('Overview')
 
           %ul.sidebar-sub-level-items
-            = nav_link(path: ['groups#show', 'groups#activity', 'groups#subgroups'], html_options: { class: "fly-out-top-item" } ) do
+            = nav_link(path: ['groups#show', 'groups#details', 'groups#activity', 'groups#subgroups'], html_options: { class: "fly-out-top-item" } ) do
               = link_to group_path(@group) do
                 %strong.fly-out-top-item-name
                   = _('Overview')
             %li.divider.fly-out-top-item
-            = nav_link(path: ['groups#show', 'groups#subgroups'], html_options: { class: 'home' }) do
-              = link_to group_path(@group), title: _('Group details') do
+
+            = nav_link(path: ['groups#show', 'groups#details', 'groups#subgroups'], html_options: { class: 'home' }) do
+              = link_to details_group_path(@group), title: _('Group details') do
                 %span
                   = _('Details')
 
@@ -40,9 +41,9 @@
 
             - if group_sidebar_link?(:contribution_analytics)
               = nav_link(path: 'analytics#show') do
-                = link_to group_analytics_path(@group), title: 'Contribution Analytics', data: {placement: 'right'} do
+                = link_to group_analytics_path(@group), title: _('Contribution Analytics'), data: { placement: 'right' } do
                   %span
-                    Contribution Analytics
+                    = _('Contribution Analytics')
 
       = render_if_exists "layouts/nav/ee/epic_link", group: @group
 

config/routes/group.rb

@@ -14,6 +14,7 @@ constraints(::Constraints::GroupUrlConstrainer.new) do
       get :issues, as: :issues_group
       get :merge_requests, as: :merge_requests_group
       get :projects, as: :projects_group
+      get :details, as: :details_group # needed for EE but left here to provide helpers for CE partials
       get :activity, as: :activity_group
       put :transfer, as: :transfer_group
       # TODO: Remove as part of refactor in https://gitlab.com/gitlab-org/gitlab-ce/issues/49693

ee/app/assets/javascripts/pages/groups/show/index.js

+import initGroupDetails from '~/pages/groups/shared/group_details';
+import initSecurityDashboard from 'ee/security_dashboard/index';
+
+document.addEventListener('DOMContentLoaded', () => {
+  if (document.querySelector('#js-group-security-dashboard')) {
+    initSecurityDashboard();
+  } else {
+    initGroupDetails();
+  }
+});

ee/app/controllers/ee/groups_controller.rb

@@ -3,6 +3,16 @@
 module EE
   module GroupsController
     extend ActiveSupport::Concern
+    extend ::Gitlab::Utils::Override
+
+    override :render_show_html
+    def render_show_html
+      if redirect_show_path
+        redirect_to redirect_show_path, status: :temporary_redirect
+      else
+        super
+      end
+    end
 
     def group_params_attributes
       super + group_params_ee
@@ -25,5 +35,24 @@ module EE
     def current_group
       @group
     end
+
+    def redirect_show_path
+      strong_memoize(:redirect_show_path) do
+        case group_view
+        when 'security_dashboard'
+          helpers.group_security_dashboard_path(group) if ::Feature.enabled?(:group_overview_security_dashboard)
+        else
+          nil
+        end
+      end
+    end
+
+    def group_view
+      current_user&.group_view || default_group_view
+    end
+
+    def default_group_view
+      EE::User::DEFAULT_GROUP_VIEW
+    end
   end
 end

ee/app/controllers/groups/security/application_controller.rb

 # frozen_string_literal: true
 class Groups::Security::ApplicationController < Groups::ApplicationController
-  before_action :ensure_security_dashboard_feature_enabled
+  before_action :ensure_security_dashboard_feature_enabled!
   before_action :authorize_read_group_security_dashboard!
 
-  private
+  protected
 
-  def ensure_security_dashboard_feature_enabled
-    render_404 unless @group.feature_available?(:security_dashboard)
+  def ensure_security_dashboard_feature_enabled!
+    render_404 unless group.feature_available?(:security_dashboard)
   end
 
   def authorize_read_group_security_dashboard!
-    render_403 unless can?(current_user, :read_group_security_dashboard, group)
+    render_403 unless helpers.can_read_group_security_dashboard?(group)
   end
 end

ee/app/helpers/ee/preferences_helper.rb

@@ -3,6 +3,7 @@
 module EE
   module PreferencesHelper
     extend ::Gitlab::Utils::Override
+    include ::Groups::Security::DashboardHelper
 
     override :excluded_dashboard_choices
     def excluded_dashboard_choices
@@ -13,9 +14,10 @@ module EE
 
     def group_view_choices
       strong_memoize(:group_view_choices) do
-        [[_('Details (default)'), :details]].tap do |choices|
-          choices << [_('Security dashboard'), :security_dashboard] if group_view_security_dashboard_enabled?
-        end
+        choices = []
+        choices << [_('Details (default)'), :details]
+        choices << [_('Security dashboard'), :security_dashboard] if group_view_security_dashboard_enabled?
+        choices
       end
     end
 

ee/app/helpers/groups/security/dashboard_helper.rb

+# frozen_string_literal: true
+
+module Groups
+  module Security
+    module DashboardHelper
+      def can_read_group_security_dashboard?(group)
+        can?(current_user, :read_group_security_dashboard, group)
+      end
+    end
+  end
+end

ee/app/views/groups/sidebar/_security_dashboard.html.haml

-- if can?(current_user, :read_group_security_dashboard, @group)
+- if @group.feature_available?(:security_dashboard)
   = nav_link(path: 'groups/security/dashboard#show') do
     = link_to group_security_dashboard_path(@group), title: _('Security Dashboard') do
       %span= _('Security Dashboard')

ee/changelogs/unreleased/7048_set_security_dashboard_as_default_view_for_groups_be.yml

+---
+title: Enabled setting the Security Dashboard as a default view for groups
+merge_request: 7889
+author:
+type: added

ee/spec/controllers/groups/security/dashboard_controller_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Groups::Security::DashboardController do
+  let(:user) { create(:user) }
+  let(:group) { create(:group) }
+
+  before do
+    sign_in(user)
+  end
+
+  describe 'GET show' do
+    subject { get :show, params: { group_id: group.to_param } }
+
+    context 'when security dashboard feature is enabled' do
+      before do
+        stub_licensed_features(security_dashboard: true)
+      end
+
+      context 'and user is allowed to access group security dashboard' do
+        before do
+          group.add_developer(user)
+        end
+
+        it { is_expected.to have_gitlab_http_status(200) }
+      end
+
+      context 'when user is not allowed to access group security dashboard' do
+        it { is_expected.to have_gitlab_http_status(403) }
+      end
+    end
+
+    context 'when security dashboard feature is disabled' do
+      it { is_expected.to have_gitlab_http_status(404) }
+    end
+  end
+end

ee/spec/helpers/ee/groups_helper_spec.rb

 require 'spec_helper'
 
 describe GroupsHelper do
-  describe '#group_sidebar_links' do
-    let(:user) { create(:user) }
-    let(:group) { create(:group, :private) }
+  before do
+    allow(helper).to receive(:current_user) { user }
+  end
 
+  let(:user) { create(:user, group_view: :security_dashboard) }
+  let(:group) { create(:group, :private) }
+
+  describe '#group_sidebar_links' do
     before do
-      allow(helper).to receive(:current_user) { user }
       group.add_owner(user)
       helper.instance_variable_set(:@group, group)
       allow(helper).to receive(:can?) { |*args| Ability.allowed?(*args) }

ee/spec/support/shared_examples/controllers/security_dashboard_shared_examples.rb

+# frozen_string_literal: true
+
+shared_examples 'ensures security dashboard permissions' do
+  context 'when security dashboard feature is enabled' do
+    before do
+      stub_licensed_features(security_dashboard: true)
+    end
+
+    context 'and user is allowed to access group security dashboard' do
+      before do
+        group.add_developer(user)
+      end
+
+      it { is_expected.to have_gitlab_http_status(200) }
+    end
+
+    context 'when user is not allowed to access group security dashboard' do
+      it { is_expected.to have_gitlab_http_status(403) }
+    end
+  end
+
+  context 'when security dashboard feature is disabled' do
+    it { is_expected.to have_gitlab_http_status(404) }
+  end
+end

ee/spec/views/layouts/nav/sidebar/_group.html.haml_spec.rb

@@ -2,9 +2,11 @@ require 'spec_helper'
 
 describe 'layouts/nav/sidebar/_group' do
   before do
-    assign(:group, create(:group))
+    assign(:group, group)
   end
 
+  let(:group) { create(:group) }
+
   describe 'contribution analytics tab' do
     it 'is not visible when there is no valid license and we dont show promotions' do
       stub_licensed_features(contribution_analytics: false)
@@ -66,24 +68,31 @@ describe 'layouts/nav/sidebar/_group' do
   end
 
   describe 'security dashboard tab' do
-    it 'is visible when user has enough permission' do
-      allow(view).to receive(:can?)
-        .with(anything, :read_group_security_dashboard, anything)
-        .and_return(true)
+    before do
+      stub_licensed_features(security_dashboard: true)
+      enable_namespace_license_check!
 
-      render
+      create(:gitlab_subscription, hosted_plan: group.plan, namespace: group)
+    end
+
+    context 'when security dashboard feature is enabled' do
+      let(:group) { create(:group, plan: :gold_plan) }
+
+      it 'is visible' do
+        render
 
-      expect(rendered).to have_text 'Security Dashboard'
+        expect(rendered).to have_link 'Security Dashboard'
+      end
     end
 
-    it 'is not visible when user does not have enough permission' do
-      allow(view).to receive(:can?)
-        .with(anything, :read_group_security_dashboard, anything)
-        .and_return(false)
+    context 'when security dashboard feature is disabled' do
+      let(:group) { create(:group, plan: :bronze_plan) }
 
-      render
+      it 'is not visible' do
+        render
 
-      expect(rendered).not_to have_text 'Security Dashboard'
+        expect(rendered).not_to have_link 'Security Dashboard'
+      end
     end
   end
 end

ee/spec/views/profiles/preferences/show.html.haml_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe 'profiles/preferences/show' do
+  before do
+    assign(:user, user)
+    allow(controller).to receive(:current_user).and_return(user)
+    view.extend ::Groups::Security::DashboardHelper
+  end
+
+  let(:user) { build(:user) }
+
+  context 'security dashboard feature is available' do
+    before do
+      stub_licensed_features(security_dashboard: true)
+    end
+
+    it 'renders the group view choice preference' do
+      render
+
+      expect(rendered).to have_select('Group overview content')
+    end
+  end
+
+  context 'security dashboard feature is unavailable' do
+    it 'does not render the group view choice preference' do
+      render
+
+      expect(rendered).not_to have_select('Group overview content')
+    end
+  end
+end

lib/gitlab/view/presenter/delegated.rb

@@ -3,6 +3,7 @@
 module Gitlab
   module View
     module Presenter
+      # TODO: find a way to delegate calls to `class` methods to the subject's class; see gitlab-ce/#57299
       class Delegated < SimpleDelegator
         include Gitlab::View::Presenter::Base
 

locale/gitlab.pot

@@ -2858,6 +2858,9 @@ msgstr ""
 msgid "Contribution"
 msgstr ""
 
+msgid "Contribution Analytics"
+msgstr ""
+
 msgid "Contribution Charts"
 msgstr ""
 

spec/controllers/groups_controller_spec.rb

@@ -32,21 +32,46 @@ describe GroupsController do
     end
   end
 
+  shared_examples 'details view' do
+    it { is_expected.to render_template('groups/show') }
+
+    context 'as atom' do
+      let!(:event) { create(:event, project: project) }
+      let(:format) { :atom }
+
+      it { is_expected.to render_template('groups/show') }
+
+      it 'assigns events for all the projects in the group' do
+        subject
+        expect(assigns(:events)).to contain_exactly(event)
+      end
+    end
+  end
+
   describe 'GET #show' do
     before do
       sign_in(user)
       project
     end
 
-    context 'as atom' do
-      it 'assigns events for all the projects in the group' do
-        create(:event, project: project)
+    let(:format) { :html }
 
-        get :show, params: { id: group.to_param }, format: :atom
+    subject { get :show, params: { id: group.to_param }, format: format }
 
-        expect(assigns(:events)).not_to be_empty
-      end
+    it_behaves_like 'details view'
+  end
+
+  describe 'GET #details' do
+    before do
+      sign_in(user)
+      project
     end
+
+    let(:format) { :html }
+
+    subject { get :details, params: { id: group.to_param }, format: format }
+
+    it_behaves_like 'details view'
   end
 
   describe 'GET edit' do

spec/routing/group_routing_spec.rb

@@ -17,6 +17,10 @@ describe "Groups", "routing" do
     expect(get("/#{group_path}")).to route_to('groups#show', id: group_path)
   end
 
+  it "to #details" do
+    expect(get("/groups/#{group_path}/-/details")).to route_to('groups#details', id: group_path)
+  end
+
   it "to #activity" do
     expect(get("/groups/#{group_path}/-/activity")).to route_to('groups#activity', id: group_path)
   end