Merge branch '6216-extract-ee-specific-files-lines-for-app-views-admin-groups' into 'master'

Resolve "Extract EE specific files/lines for app/views/admin/groups"

Closes #6216

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

app/models/group.rb

 require 'carrierwave/orm/activerecord'
 
-# Contains methods common to both GitLab CE and EE.
-# All EE methods should be in `EE::Group` only.
 class Group < Namespace
-  include EE::Group
+  prepend EE::Group
+
   include Gitlab::ConfigHelper
   include AfterCommitQueue
   include AccessRequestable
@@ -34,14 +33,7 @@ class Group < Namespace
   has_many :variables, class_name: 'Ci::GroupVariable'
   has_many :custom_attributes, class_name: 'GroupCustomAttribute'
 
-  has_many :ldap_group_links, foreign_key: 'group_id', dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
-  has_many :hooks, dependent: :destroy, class_name: 'GroupHook' # rubocop:disable Cop/ActiveRecordDependent
-
   has_many :boards
-
-  # We cannot simply set `has_many :audit_events, as: :entity, dependent: :destroy`
-  # here since Group inherits from Namespace, the entity_type would be set to `Namespace`.
-  has_many :audit_events, -> { where(entity_type: Group) }, foreign_key: 'entity_id'
   has_many :badges, class_name: 'GroupBadge'
 
   accepts_nested_attributes_for :variables, allow_destroy: true
@@ -53,9 +45,6 @@ class Group < Namespace
 
   validates :two_factor_grace_period, presence: true, numericality: { greater_than_or_equal_to: 0 }
 
-  validates :repository_size_limit,
-            numericality: { only_integer: true, greater_than_or_equal_to: 0, allow_nil: true }
-
   add_authentication_token_field :runners_token
 
   after_create :post_create_hook
@@ -63,10 +52,6 @@ class Group < Namespace
   after_save :update_two_factor_requirement
   after_update :path_changed_hook, if: :path_changed?
 
-  scope :where_group_links_with_provider, ->(provider) do
-    joins(:ldap_group_links).where(ldap_group_links: { provider: provider })
-  end
-
   class << self
     def supports_nested_groups?
       Gitlab::Database.postgresql?
@@ -213,21 +198,8 @@ class Group < Namespace
     owners.include?(user) && owners.size == 1
   end
 
-  def human_ldap_access
-    Gitlab::Access.options_with_owner.key ldap_access
-  end
-
-  # NOTE: Backwards compatibility with old ldap situation
-  def ldap_cn
-    ldap_group_links.first.try(:cn)
-  end
-
-  def ldap_access
-    ldap_group_links.first.try(:group_access)
-  end
-
   def ldap_synced?
-    Gitlab.config.ldap.enabled && ldap_group_links.any?(&:active?)
+    false
   end
 
   def post_create_hook
@@ -242,20 +214,10 @@ class Group < Namespace
     system_hook_service.execute_hooks_for(self, :destroy)
   end
 
-  def actual_size_limit
-    return Gitlab::CurrentSettings.repository_size_limit if repository_size_limit.nil?
-
-    repository_size_limit
-  end
-
   def system_hook_service
     SystemHooksService.new
   end
 
-  def first_non_empty_project
-    projects.detect { |project| !project.empty_repo? }
-  end
-
   def refresh_members_authorized_projects(blocking: true)
     UserProjectAccessChangedService.new(user_ids_for_project_authorizations)
       .execute(blocking: blocking)

app/views/admin/groups/_form.html.haml

@@ -2,10 +2,8 @@
   = form_errors(@group)
   = render 'shared/group_form', f: f
 
-  = render 'shared/repository_size_limit_setting', form: f, type: :group
-
-  - if Gitlab::CurrentSettings.should_check_namespace_plan?
-    = render 'admin/namespace_plan', f: f
+  = render_if_exists 'shared/repository_size_limit_setting', form: f, type: :group
+  = render_if_exists 'admin/namespace_plan', f: f
 
   .form-group.row.group-description-holder
     = f.label :avatar, "Group avatar", class: 'col-form-label col-sm-2'
@@ -20,12 +18,7 @@
 
   = render 'groups/group_admin_settings', f: f
 
-  - if @group.shared_runner_minutes_supported?
-    = render 'namespaces/shared_runners_minutes_setting', f: f
-    - unless @group.new_record?
-      .form-group.row
-        .offset-sm-2.col-sm-10
-          = render 'namespaces/shared_runners_minutes_setting_reset', link_reset_minutes: admin_group_reset_runners_minutes_path(@group)
+  = render_if_exists 'namespaces/shared_runners_minutes_settings', group: @group, form: f
 
   - if @group.new_record?
     .form-group.row
@@ -41,7 +34,4 @@
       = f.submit 'Save changes', class: "btn btn-save"
       = link_to  'Cancel', admin_group_path(@group), class: "btn btn-cancel"
 
-- if @group.persisted? && Gitlab::Auth::LDAP::Config.group_sync_enabled?
-  %h3.page-title LDAP synchronizations
-  = render 'ldap_group_links/form', group: @group
-  = render 'ldap_group_links/ldap_group_links', group: @group
+= render_if_exists 'ldap_group_links/ldap_syncrhonizations', group: @group

app/views/admin/groups/_group.html.haml

+- group = local_assigns.fetch(:group)
 - css_class = 'no-description' if group.description.blank?
 
 %li.group-row{ class: css_class }
@@ -8,7 +9,7 @@
     %span.badge.badge-pill
       = storage_counter(group.storage_size)
 
-    = render 'admin/namespace_plan_badge', namespace: group
+    = render_if_exists 'admin/namespace_plan_badge', namespace: group
 
     %span
       = icon('bookmark')

app/views/admin/groups/show.html.haml

@@ -40,7 +40,7 @@
           %strong
             = @group.created_at.to_s(:medium)
 
-        = render 'admin/namespace_plan_info', namespace: @group
+        = render_if_exists 'admin/namespace_plan_info', namespace: @group
 
         %li
           %span.light Storage:
@@ -60,17 +60,9 @@
             = group_lfs_status(@group)
             = link_to icon('question-circle'), help_page_path('workflow/lfs/manage_large_binaries_with_git_lfs')
 
-        = render partial: "namespaces/shared_runner_status", locals: { namespace: @group }
+        = render_if_exists 'namespaces/shared_runner_status', namespace: @group
 
-    - if Gitlab::Auth::LDAP::Config.group_sync_enabled? && @group.ldap_synced?
-      .card
-        .card-header Active synchronizations
-        %ul.content-list
-          - @group.ldap_group_links.select(&:active?).each do |ldap_group_link|
-            %li
-              %strong= ldap_group_link.cn ? "Group: #{ldap_group_link.cn}" : "Filter: #{truncate(ldap_group_link.filter, length: 40)}"
-              as
-              %strong= ldap_group_link.human_access
+    = render_if_exists 'ldap_group_links/ldap_group_links_show', group: @group
 
     .card
       .card-header

ee/app/models/ee/group.rb

@@ -7,11 +7,25 @@ module EE
     extend ActiveSupport::Concern
     extend ::Gitlab::Utils::Override
 
-    included do
+    prepended do
       has_many :epics
 
       has_one :saml_provider
 
+      has_many :ldap_group_links, foreign_key: 'group_id', dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
+      has_many :hooks, dependent: :destroy, class_name: 'GroupHook' # rubocop:disable Cop/ActiveRecordDependent
+
+      # We cannot simply set `has_many :audit_events, as: :entity, dependent: :destroy`
+      # here since Group inherits from Namespace, the entity_type would be set to `Namespace`.
+      has_many :audit_events, -> { where(entity_type: ::Group) }, foreign_key: 'entity_id'
+
+      validates :repository_size_limit,
+                numericality: { only_integer: true, greater_than_or_equal_to: 0, allow_nil: true }
+
+      scope :where_group_links_with_provider, ->(provider) do
+        joins(:ldap_group_links).where(ldap_group_links: { provider: provider })
+      end
+
       state_machine :ldap_sync_status, namespace: :ldap_sync, initial: :ready do
         state :ready
         state :started
@@ -54,6 +68,24 @@ module EE
       end
     end
 
+    def human_ldap_access
+      ::Gitlab::Access.options_with_owner.key(ldap_access)
+    end
+
+    # NOTE: Backwards compatibility with old ldap situation
+    def ldap_cn
+      ldap_group_links.first.try(:cn)
+    end
+
+    def ldap_access
+      ldap_group_links.first.try(:group_access)
+    end
+
+    override :ldap_synced?
+    def ldap_synced?
+      (::Gitlab.config.ldap.enabled && ldap_group_links.any?(&:active?)) || super
+    end
+
     def mark_ldap_sync_as_failed(error_message)
       return false unless ldap_sync_started?
 
@@ -69,5 +101,15 @@ module EE
     def multiple_issue_boards_available?
       feature_available?(:multiple_group_issue_boards)
     end
+
+    def actual_size_limit
+      return ::Gitlab::CurrentSettings.repository_size_limit if repository_size_limit.nil?
+
+      repository_size_limit
+    end
+
+    def first_non_empty_project
+      projects.detect { |project| !project.empty_repo? }
+    end
   end
 end

ee/app/views/admin/_namespace_plan.html.haml

+- return unless Gitlab::CurrentSettings.should_check_namespace_plan?
+
 .form-group
   = f.label :plan, class: 'col-form-label'
   .col-sm-10

ee/app/views/admin/_namespace_plan_badge.html.haml

-- namespace = local_assigns[:namespace]
-- if Gitlab::CurrentSettings.should_check_namespace_plan? && namespace && namespace.plan.present?
-  %span.plan-badge.has-tooltip{ data: { plan: namespace.plan.name }, title: "#{namespace.plan.title} Plan" }
+- namespace = local_assigns.fetch(:namespace)
+- return unless Gitlab::CurrentSettings.should_check_namespace_plan? && namespace.plan.present
+
+%span.plan-badge.has-tooltip{ data: { plan: namespace.plan.name }, title: "#{namespace.plan.title} Plan" }
   = custom_icon('icon_premium')

ee/app/views/admin/_namespace_plan_info.html.haml

-- namespace = local_assigns[:namespace]
-- if Gitlab::CurrentSettings.should_check_namespace_plan? && namespace
-  %li
+- namespace = local_assigns.fetch(:namespace)
+- return unless Gitlab::CurrentSettings.should_check_namespace_plan? && namespace
+
+%li
   %span.light Plan:
   - if namespace.plan.present?
     %strong.plan-badge.inline{ data: { plan: namespace.plan.name } }

ee/app/views/admin/users/_limits.html.haml

@@ -2,7 +2,7 @@
   = namespace_form.hidden_field :id
   %fieldset
     %legend Limits
-    = render 'namespaces/shared_runners_minutes_setting', f: namespace_form
+    = render 'namespaces/shared_runners_minutes_setting', form: namespace_form
     .form-group.append-bottom-0
       .offset-sm-2.col-sm-10
         = render 'namespaces/shared_runners_minutes_setting_reset', link_reset_minutes: reset_runners_minutes_admin_user_path(@user), css_class: 'append-bottom-0'

ee/app/views/ldap_group_links/_ldap_group_links_show.html.haml

+- group = local_assigns.fetch(:group)
+- return unless Gitlab::Auth::LDAP::Config.group_sync_enabled? && group.ldap_synced?
+
+.card
+  .card-header Active synchronizations
+  %ul.content-list
+    - group.ldap_group_links.select(&:active?).each do |ldap_group_link|
+      %li
+        %strong= ldap_group_link.cn ? "Group: #{ldap_group_link.cn}" : "Filter: #{truncate(ldap_group_link.filter, length: 40)}"
+        as
+        %strong= ldap_group_link.human_access

ee/app/views/ldap_group_links/_ldap_group_links_synchronizations.html.haml

+- group = local_assigns.fetch(:group)
+- return unless Gitlab::Auth::LDAP::Config.group_sync_enabled? && group.persisted?
+
+%h3.page-title LDAP synchronizations
+= render 'ldap_group_links/form', group: group
+= render 'ldap_group_links/ldap_group_links', group: group

ee/app/views/namespaces/_shared_runner_status.html.haml

 - namespace = local_assigns.fetch(:namespace)
+
 - if namespace.shared_runners_enabled?
   %li
     %span.light Pipeline minutes quota:

ee/app/views/namespaces/_shared_runners_minutes_setting.html.haml

-- if current_user.admin?
-  .form-group.row
-    = f.label :shared_runners_minutes_limit, class: 'col-form-label col-sm-2' do
+- return unless current_user.admin?
+
+- form = local_assigns.fetch(:form)
+
+.form-group.row
+  = form.label :shared_runners_minutes_limit, class: 'col-form-label col-sm-2' do
     Pipeline Minutes Quota
   .col-sm-10
-      = f.number_field :shared_runners_minutes_limit, class: 'form-control', min: 0
+    = form.number_field :shared_runners_minutes_limit, class: 'form-control', min: 0
     %span.form-text.text-muted#shared_runners_minutes_limit_help_block
       Set the maximum number of pipeline minutes that a group can use on shared Runners per month.
       Set 0 for unlimited.

ee/app/views/namespaces/_shared_runners_minutes_settings.html.haml

+- group = local_assigns.fetch(:group)
+- form = local_assigns.fetch(:form)
+
+- return unless group.shared_runner_minutes_supported?
+
+= render 'namespaces/shared_runners_minutes_setting', form: form
+- unless group.new_record?
+  .form-group.row
+    .offset-sm-2.col-sm-10
+      = render 'namespaces/shared_runners_minutes_setting_reset', link_reset_minutes: admin_group_reset_runners_minutes_path(group)

ee/spec/features/groups/group_settings_spec.rb

@@ -11,6 +11,31 @@ feature 'Edit group settings' do
     sign_in(user)
   end
 
+  describe 'navbar' do
+    context 'with LDAP enabled' do
+      before do
+        allow_any_instance_of(EE::Group).to receive(:ldap_synced?).and_return(true)
+        allow(Gitlab::Auth::LDAP::Config).to receive(:enabled?).and_return(true)
+      end
+
+      scenario 'is able to navigate to LDAP group section' do
+        visit edit_group_path(group)
+
+        expect(find('.nav-sidebar')).to have_content('LDAP Synchronization')
+      end
+
+      context 'with owners not being able to manage LDAP' do
+        scenario 'is not able to navigate to LDAP group section' do
+          stub_application_setting(allow_group_owners_to_manage_ldap: false)
+
+          visit edit_group_path(group)
+
+          expect(find('.nav-sidebar')).not_to have_content('LDAP Synchronization')
+        end
+      end
+    end
+  end
+
   context 'with webhook feature enabled' do
     it 'shows the menu item' do
       stub_licensed_features(group_webhooks: true)

spec/features/groups/group_settings_spec.rb

@@ -9,31 +9,6 @@ feature 'Edit group settings' do
     sign_in(user)
   end
 
-  describe 'navbar' do
-    context 'with LDAP enabled' do
-      before do
-        allow_any_instance_of(Group).to receive(:ldap_synced?).and_return(true)
-        allow(Gitlab::Auth::LDAP::Config).to receive(:enabled?).and_return(true)
-      end
-
-      scenario 'is able to navigate to LDAP group section' do
-        visit edit_group_path(group)
-
-        expect(find('.nav-sidebar')).to have_content('LDAP Synchronization')
-      end
-
-      context 'with owners not being able to manage LDAP' do
-        scenario 'is not able to navigate to LDAP group section' do
-          stub_application_setting(allow_group_owners_to_manage_ldap: false)
-
-          visit edit_group_path(group)
-
-          expect(find('.nav-sidebar')).not_to have_content('LDAP Synchronization')
-        end
-      end
-    end
-  end
-
   describe 'when the group path is changed' do
     let(:new_group_path) { 'bar' }
     let(:old_group_full_path) { "/#{group.path}" }