Коммит aa237904 создал по автору Lukas 'Eipi' Eipert's avatar Lukas 'Eipi' Eipert Зафиксировано автором Peter Leitzen
Просмотр файлов

Remove group_administration_nav_item feature flag 

And it's related code. Apparently we introduced moving some settings
menu items, namely SAML SSO, Usage Quotas and Billing, into a [new
"Administration"][0] menu for group owners. The change was not
well-liked and was put [behind a feature flag][1].

The corresponding feature flag `group_administration_nav_item` is not
enabled per default _or otherwise_. It isn't documented, it is really
old, so let us just get rid of it.

[0]: https://gitlab.com/gitlab-org/gitlab/-/issues/209020
[1]: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/29084

Changelog: removed
EE: true
владелец 40324130
Скрыть пробелы
Построчно Рядом
.rubocop_todo/rspec/missing_feature_category.yml
Просмотр файла @ aa237904
......@@ -1135,7 +1135,6 @@ RSpec/MissingFeatureCategory:
- 'ee/spec/lib/omni_auth/strategies/kerberos_spec.rb'
- 'ee/spec/lib/peek/views/elasticsearch_spec.rb'
- 'ee/spec/lib/quality/seeders/vulnerabilities_spec.rb'
- 'ee/spec/lib/sidebars/groups/menus/administration_menu_spec.rb'
- 'ee/spec/lib/sidebars/groups/menus/analytics_menu_spec.rb'
- 'ee/spec/lib/sidebars/groups/menus/security_compliance_menu_spec.rb'
- 'ee/spec/lib/slack/api_spec.rb'
......
.rubocop_todo/style/if_unless_modifier.yml
Просмотр файла @ aa237904
......@@ -642,7 +642,6 @@ Style/IfUnlessModifier:
- 'ee/lib/gitlab/usage/metrics/instrumentations/count_users_creating_ci_builds_metric.rb'
- 'ee/lib/gitlab/usage/metrics/instrumentations/license_metric.rb'
- 'ee/lib/omni_auth/strategies/group_saml.rb'
- 'ee/lib/sidebars/groups/menus/administration_menu.rb'
- 'ee/lib/sidebars/groups/menus/analytics_menu.rb'
- 'ee/lib/sidebars/groups/menus/security_compliance_menu.rb'
- 'ee/lib/tasks/geo.rake'
......
ee/config/feature_flags/development/group_administration_nav_item.yml удалено 100644 → 0
Просмотр файла @ 40324130
---
name: group_administration_nav_item
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/29084
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/209020
milestone: '12.10'
type: development
group: group::workspace
default_enabled: false
ee/lib/ee/sidebars/groups/menus/settings_menu.rb
Просмотр файла @ aa237904
......@@ -6,7 +6,6 @@ module Groups
module Menus
module SettingsMenu
extend ::Gitlab::Utils::Override
include ::Gitlab::Utils::StrongMemoize
override :configure_menu_items
def configure_menu_items
......@@ -69,8 +68,7 @@ def saml_sso_menu_item
end
def saml_sso_enabled?
can?(context.current_user, :admin_group_saml, context.group) &&
administration_nav_item_disabled?
can?(context.current_user, :admin_group_saml, context.group)
end
def saml_group_links_menu_item
......@@ -119,11 +117,6 @@ def webhooks_enabled?
context.show_promotions
end
override :usage_quotas_menu_enabled?
def usage_quotas_menu_enabled?
super && administration_nav_item_disabled?
end
def billing_menu_item
unless billing_enabled?
return ::Sidebars::NilMenuItem.new(item_id: :billing)
......@@ -138,14 +131,7 @@ def billing_menu_item
end
def billing_enabled?
::Gitlab::CurrentSettings.should_check_namespace_plan? &&
administration_nav_item_disabled?
end
def administration_nav_item_disabled?
strong_memoize(:administration_nav_item_disabled) do
::Feature.disabled?(:group_administration_nav_item, context.group)
end
::Gitlab::CurrentSettings.should_check_namespace_plan?
end
def reporting_menu_item
......
ee/lib/ee/sidebars/groups/panel.rb
Просмотр файла @ aa237904
......@@ -21,7 +21,6 @@ def configure_menus
)
insert_menu_after(::Sidebars::Groups::Menus::PackagesRegistriesMenu, ::Sidebars::Groups::Menus::AnalyticsMenu.new(context))
insert_menu_after(::Sidebars::Groups::Menus::AnalyticsMenu, ::Sidebars::Groups::Menus::WikiMenu.new(context))
insert_menu_after(::Sidebars::Groups::Menus::SettingsMenu, ::Sidebars::Groups::Menus::AdministrationMenu.new(context))
end
end
end
......
ee/lib/sidebars/groups/menus/administration_menu.rb удалено 100644 → 0
Просмотр файла @ 40324130
# frozen_string_literal: true
module Sidebars
module Groups
module Menus
class AdministrationMenu < ::Sidebars::Menu
include Gitlab::Utils::StrongMemoize
override :configure_menu_items
def configure_menu_items
return false unless administration_menu_enabled?
add_item(saml_sso_menu_item)
add_item(usage_quotas_menu_item)
add_item(billing_menu_item)
true
end
override :title
def title
_('Administration')
end
override :sprite_icon
def sprite_icon
'admin'
end
private
def administration_menu_enabled?
::Feature.enabled?(:group_administration_nav_item, context.group) &&
context.group.root? &&
can?(context.current_user, :admin_group, context.group)
end
def saml_sso_menu_item
unless can?(context.current_user, :admin_group_saml, context.group)
return ::Sidebars::NilMenuItem.new(item_id: :saml_sso)
end
::Sidebars::MenuItem.new(
title: _('SAML SSO'),
link: group_saml_providers_path(context.group),
active_routes: { path: 'saml_providers#show' },
item_id: :saml_sso
)
end
def usage_quotas_menu_item
unless context.group.usage_quotas_enabled?
return ::Sidebars::NilMenuItem.new(item_id: :usage_quotas)
end
::Sidebars::MenuItem.new(
title: s_('UsageQuota|Usage Quotas'),
link: group_usage_quotas_path(context.group),
active_routes: { path: 'usage_quotas#index' },
item_id: :usage_quotas
)
end
def billing_menu_item
unless ::Gitlab::CurrentSettings.should_check_namespace_plan?
return ::Sidebars::NilMenuItem.new(item_id: :billing)
end
::Sidebars::MenuItem.new(
title: _('Billing'),
link: group_billings_path(context.group),
active_routes: { path: 'billings#index' },
item_id: :billing
)
end
end
end
end
end
ee/spec/features/groups/navbar_spec.rb
Просмотр файла @ aa237904
......@@ -221,7 +221,6 @@
before do
insert_after_nav_item(_('Security and Compliance'), new_nav_item: ci_cd_nav_item)
insert_after_nav_item(_('Analytics'), new_nav_item: settings_nav_item)
insert_after_nav_item(_('Settings'), new_nav_item: administration_nav_item)
visit group_path(group)
end
......@@ -235,15 +234,10 @@
insert_after_nav_item(_('Security and Compliance'), new_nav_item: ci_cd_nav_item)
insert_after_nav_item(_('Analytics'), new_nav_item: settings_nav_item)
insert_after_nav_item(
_('Settings'),
new_nav_item: {
nav_item: _('Administration'),
nav_sub_items: [
_('SAML SSO'),
s_('UsageQuota|Usage Quotas')
]
}
insert_after_sub_nav_item(
s_('UsageQuota|Usage Quotas'),
within: _('Settings'),
new_sub_nav_item_name: _('SAML SSO')
)
visit group_path(group)
......@@ -274,7 +268,6 @@
insert_after_nav_item(_('Security and Compliance'), new_nav_item: ci_cd_nav_item)
insert_after_nav_item(_('Analytics'), new_nav_item: settings_nav_item)
insert_after_nav_item(_('Settings'), new_nav_item: administration_nav_item)
visit group_path(group)
end
......
ee/spec/lib/ee/sidebars/groups/menus/settings_menu_spec.rb
Просмотр файла @ aa237904
......@@ -53,11 +53,9 @@
describe 'SAML SSO menu' do
let(:item_id) { :saml_sso }
let(:group_admin_nav_enabled) { false }
let(:saml_enabled) { true }
before do
stub_feature_flags(group_administration_nav_item: group_admin_nav_enabled)
stub_licensed_features(group_saml: saml_enabled)
allow(::Gitlab::Auth::GroupSaml::Config).to receive(:enabled?).and_return(saml_enabled)
end
......@@ -69,18 +67,10 @@
end
context 'when SAML is enabled' do
context 'when :group_administration_nav_item feature is disabled' do
it { is_expected.to be_present }
context 'when user cannot admin group SAML' do
let(:user) { nil }
it { is_expected.not_to be_present }
end
end
it { is_expected.to be_present }
context 'when :group_administration_nav_item feature is enabled' do
let(:group_admin_nav_enabled) { true }
context 'when user cannot admin group SAML' do
let(:user) { nil }
it { is_expected.not_to be_present }
end
......@@ -166,12 +156,10 @@
describe 'Usage quotas menu' do
let(:item_id) { :usage_quotas }
let(:group_admin_nav_enabled) { false }
let(:usage_quotas_enabled) { true }
before do
stub_feature_flags(
group_administration_nav_item: group_admin_nav_enabled,
usage_quotas_for_all_editions: false
)
stub_licensed_features(usage_quotas: usage_quotas_enabled)
......@@ -179,12 +167,6 @@
it { is_expected.to be_present }
context 'when feature flag :group_administration_nav_item is enabled' do
let(:group_admin_nav_enabled) { true }
it { is_expected.not_to be_present }
end
context 'when usage_quotas licensed feature is not enabled' do
let(:usage_quotas_enabled) { false }
......@@ -194,22 +176,14 @@
describe 'Billing menu' do
let(:item_id) { :billing }
let(:group_admin_nav_enabled) { false }
let(:check_billing) { true }
before do
stub_feature_flags(group_administration_nav_item: group_admin_nav_enabled)
allow(::Gitlab::CurrentSettings).to receive(:should_check_namespace_plan?).and_return(check_billing)
end
it { is_expected.to be_present }
context 'when feature flag :group_administration_nav_item is enabled' do
let(:group_admin_nav_enabled) { true }
it { is_expected.not_to be_present }
end
context 'when group billing does not apply' do
let(:check_billing) { false }
......@@ -242,11 +216,9 @@
describe 'Billing menu item' do
let(:item_id) { :billing }
let(:group_admin_nav_enabled) { false }
let(:check_billing) { true }
before do
stub_feature_flags(group_administration_nav_item: group_admin_nav_enabled)
stub_feature_flags(auditor_billing_page_access: group)
allow(::Gitlab::CurrentSettings).to receive(:should_check_namespace_plan?).and_return(check_billing)
end
......
ee/spec/lib/sidebars/groups/menus/administration_menu_spec.rb удалено 100644 → 0
Просмотр файла @ 40324130
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Sidebars::Groups::Menus::AdministrationMenu do
let_it_be(:owner) { create(:user) }
let_it_be_with_refind(:parent_group) do
create(:group, :private).tap do |g|
g.add_owner(owner)
end
end
let_it_be_with_refind(:child_group) do
create(:group, :private, parent: parent_group).tap do |g|
g.add_owner(owner)
end
end
let(:group) { parent_group }
let(:user) { owner }
let(:context) { Sidebars::Groups::Context.new(current_user: user, container: group) }
let(:menu) { described_class.new(context) }
describe '#render?' do
subject { menu.render? }
context 'when feature flag :group_administration_nav_item is enabled' do
specify { is_expected.to be true }
context 'when group is a subgroup' do
let(:group) { child_group }
specify { is_expected.to be false }
end
context 'when user cannot admin group' do
let(:user) { nil }
specify { is_expected.to be false }
end
end
context 'when feature flag :group_administration_nav_item is disabled' do
specify do
stub_feature_flags(group_administration_nav_item: false)
is_expected.to be false
end
end
end
describe 'Menu items' do
subject { menu.renderable_items.find { |e| e.item_id == item_id } }
describe 'SAML SSO menu' do
let(:item_id) { :saml_sso }
let(:saml_enabled) { true }
before do
stub_licensed_features(group_saml: saml_enabled)
allow(::Gitlab::Auth::GroupSaml::Config).to receive(:enabled?).and_return(saml_enabled)
end
context 'when SAML is disabled' do
let(:saml_enabled) { false }
specify { is_expected.to be_nil }
end
context 'when SAML is enabled' do
specify { is_expected.to be_present }
context 'when user does not have access' do
let(:user) { nil }
specify { is_expected.to be_nil }
end
end
end
describe 'Usage quotas menu' do
let(:item_id) { :usage_quotas }
let(:usage_quotas_enabled) { true }
before do
stub_licensed_features(usage_quotas: usage_quotas_enabled)
end
specify { is_expected.to be_present }
context 'when user does not have access' do
let(:user) { nil }
specify { is_expected.to be_nil }
end
end
describe 'Billing menu' do
let(:item_id) { :billing }
let(:check_billing) { true }
before do
allow(::Gitlab::CurrentSettings).to receive(:should_check_namespace_plan?).and_return(check_billing)
end
specify { is_expected.to be_present }
context 'when user does not have access' do
let(:user) { nil }
specify { is_expected.to be_nil }
end
end
end
end
ee/spec/views/layouts/nav/sidebar/_group.html.haml_spec.rb
Просмотр файла @ aa237904
......@@ -524,7 +524,6 @@
saml_group_sync: true,
ldap_group_sync: true
)
stub_feature_flags(group_administration_nav_item: false)
allow(view).to receive(:current_user).and_return(user)
end
......
locale/gitlab.pot
Просмотр файла @ aa237904
......@@ -3627,9 +3627,6 @@ msgstr ""
msgid "AdminUsers|user cap"
msgstr ""
 
msgid "Administration"
msgstr ""
msgid "Administrators"
msgstr ""
 
qa/qa/ee/page/group/menu.rb
Просмотр файла @ aa237904
......@@ -32,7 +32,7 @@ def go_to_group_iterations
end
def go_to_saml_sso_group_settings
hover_group_administration do
hover_group_settings do
within_submenu do
click_element(:sidebar_menu_item_link, menu_item: 'SAML SSO')
end
......@@ -143,15 +143,6 @@ def hover_group_analytics
yield
end
end
def hover_group_administration
within_sidebar do
scroll_to_element(:sidebar_menu_link, menu_item: 'Administration')
find_element(:sidebar_menu_link, menu_item: 'Administration').hover
yield
end
end
end
end
end
......
qa/qa/flow/saml.rb
Просмотр файла @ aa237904
......@@ -19,8 +19,6 @@ def logout_from_idp(saml_idp_service)
end
def enable_saml_sso(group, saml_idp_service, enforce_sso: false, default_membership_role: 'Guest')
Runtime::Feature.enable(:group_administration_nav_item)
page.visit Runtime::Scenario.gitlab_address
Page::Main::Login.perform(&:sign_in_using_credentials) unless Page::Main::Menu.perform(&:signed_in?)
......
qa/qa/specs/features/ee/browser_ui/1_manage/group/group_saml_enforced_sso_git_access_spec.rb
Просмотр файла @ aa237904
# frozen_string_literal: true
module QA
RSpec.describe 'Manage', :group_saml, :orchestrated, requires_admin: 'creates a user via API',
feature_flag: { name: 'group_administration_nav_item', scope: :global } do
RSpec.describe 'Manage', :group_saml, :orchestrated, requires_admin: 'creates a user via API' do
describe 'Group SAML SSO - Enforced SSO', product_group: :system_access do
include Support::API
......@@ -48,7 +47,6 @@ module QA
after do
page.visit Runtime::Scenario.gitlab_address
Runtime::Feature.remove(:group_administration_nav_item)
group.remove_via_api!
......
qa/qa/specs/features/ee/browser_ui/1_manage/group/group_saml_enforced_sso_new_account_spec.rb
Просмотр файла @ aa237904
# frozen_string_literal: true
module QA
RSpec.describe 'Manage', :group_saml, :orchestrated, requires_admin: 'for various user admin functions', feature_flag: {
name: :group_administration_nav_item,
scope: :global
} do
RSpec.describe 'Manage', :group_saml, :orchestrated, requires_admin: 'for various user admin functions' do
describe 'Group SAML SSO - Enforced SSO', product_group: :system_access do
include Support::API
......@@ -119,8 +116,6 @@ module QA
after do
Flow::Saml.remove_saml_idp_service(saml_idp_service)
Runtime::Feature.remove(:group_administration_nav_item)
group.remove_via_api!
remove_user
......
qa/qa/specs/features/ee/browser_ui/1_manage/group/group_saml_non_enforced_sso_spec.rb
Просмотр файла @ aa237904
# frozen_string_literal: true
module QA
RSpec.describe 'Manage', :group_saml, :orchestrated, requires_admin: 'creates a user via API',
feature_flag: { name: 'group_administration_nav_item', scope: :global } do
RSpec.describe 'Manage', :group_saml, :orchestrated, requires_admin: 'creates a user via API' do
describe 'Group SAML SSO - Non enforced SSO', product_group: :system_access do
include Support::API
......@@ -13,8 +12,6 @@ module QA
sandbox_group.path = "saml_sso_group_#{SecureRandom.hex(8)}"
end
Runtime::Feature.enable(:group_administration_nav_item)
@saml_idp_service = Flow::Saml.run_saml_idp_service(@group.path)
end
......@@ -103,8 +100,6 @@ module QA
after(:all) do
@group.remove_via_api!
Runtime::Feature.remove(:group_administration_nav_item)
page.visit Runtime::Scenario.gitlab_address
Page::Main::Menu.perform(&:sign_out_if_signed_in)
......
qa/qa/specs/features/ee/browser_ui/5_package/dependency_proxy_sso_spec.rb
Просмотр файла @ aa237904
# frozen_string_literal: true
module QA
RSpec.describe 'Package', :orchestrated, :group_saml, requires_admin: 'for various user admin functions',
feature_flag: { name: 'group_administration_nav_item', scope: :global } do
RSpec.describe 'Package', :orchestrated, :group_saml, requires_admin: 'for various user admin functions' do
describe 'Dependency Proxy Group SSO', product_group: :container_registry do
include Support::API
......@@ -67,8 +66,6 @@ module QA
after do
Flow::Saml.remove_saml_idp_service(saml_idp_service)
Runtime::Feature.remove(:group_administration_nav_item)
remove_user
group.remove_via_api!
runner.remove_via_api!
......
spec/support/rspec_order_todo.yml
Просмотр файла @ aa237904
......@@ -1575,7 +1575,6 @@
- './ee/spec/lib/omni_auth/strategies/group_saml_spec.rb'
- './ee/spec/lib/omni_auth/strategies/kerberos_spec.rb'
- './ee/spec/lib/peek/views/elasticsearch_spec.rb'
- './ee/spec/lib/sidebars/groups/menus/administration_menu_spec.rb'
- './ee/spec/lib/sidebars/groups/menus/analytics_menu_spec.rb'
- './ee/spec/lib/sidebars/groups/menus/epics_menu_spec.rb'
- './ee/spec/lib/sidebars/groups/menus/security_compliance_menu_spec.rb'
......
spec/support/shared_contexts/navbar_structure_context.rb
Просмотр файла @ aa237904
......@@ -140,6 +140,7 @@
_('CI/CD'),
_('Applications'),
_('Packages and registries'),
s_('UsageQuota|Usage Quotas'),
_('Domain Verification')
]
}
......@@ -154,15 +155,6 @@
}
end
let(:administration_nav_item) do
{
nav_item: _('Administration'),
nav_sub_items: [
s_('UsageQuota|Usage Quotas')
]
}
end
let(:security_and_compliance_nav_item) do
{
nav_item: _('Security and Compliance'),
......
Поддерживает Markdown
0% или .
You are about to add 0 people to the discussion. Proceed with caution.
Сначала завершите редактирование этого сообщения!
Пожалуйста, зарегистрируйтесь или чтобы прокомментировать