Merge branch '346249-when-dependency-proxy-is-disabled-for-the-group-hide-the-page' into 'master'

Hide dependency proxy page when disabled for the group

See merge request gitlab-org/gitlab!76079

app/assets/javascripts/packages_and_registries/dependency_proxy/app.vue

@@ -4,7 +4,6 @@ import {
   GlEmptyState,
   GlFormGroup,
   GlFormInputGroup,
-  GlLink,
   GlSkeletonLoader,
   GlSprintf,
 } from '@gitlab/ui';
@@ -16,10 +15,7 @@ import {
   DEPENDENCY_PROXY_SETTINGS_DESCRIPTION,
   DEPENDENCY_PROXY_DOCS_PATH,
 } from '~/packages_and_registries/settings/group/constants';
-import {
-  GRAPHQL_PAGE_SIZE,
-  ENABLE_DEPENDENCY_PROXY_DOCS_PATH,
-} from '~/packages_and_registries/dependency_proxy/constants';
+import { GRAPHQL_PAGE_SIZE } from '~/packages_and_registries/dependency_proxy/constants';
 
 import getDependencyProxyDetailsQuery from '~/packages_and_registries/dependency_proxy/graphql/queries/get_dependency_proxy_details.query.graphql';
 
@@ -29,7 +25,6 @@ export default {
     GlEmptyState,
     GlFormGroup,
     GlFormInputGroup,
-    GlLink,
     GlSkeletonLoader,
     GlSprintf,
     ClipboardButton,
@@ -41,9 +36,6 @@ export default {
     proxyNotAvailableText: s__(
       'DependencyProxy|Dependency Proxy feature is limited to public groups for now.',
     ),
-    proxyDisabledText: s__(
-      'DependencyProxy|The Dependency Proxy is disabled. %{docLinkStart}Learn how to enable it%{docLinkEnd}.',
-    ),
     proxyImagePrefix: s__('DependencyProxy|Dependency Proxy image prefix'),
     copyImagePrefixText: s__('DependencyProxy|Copy prefix'),
     blobCountAndSize: s__('DependencyProxy|Contains %{count} blobs of images (%{size})'),
@@ -52,7 +44,6 @@ export default {
   },
   links: {
     DEPENDENCY_PROXY_DOCS_PATH,
-    ENABLE_DEPENDENCY_PROXY_DOCS_PATH,
   },
   data() {
     return {
@@ -79,9 +70,7 @@ export default {
         },
       ];
     },
-    dependencyProxyEnabled() {
-      return this.group?.dependencyProxySetting?.enabled;
-    },
+
     queryVariables() {
       return { fullPath: this.groupPath, first: GRAPHQL_PAGE_SIZE };
     },
@@ -131,7 +120,7 @@ export default {
 
     <gl-skeleton-loader v-else-if="$apollo.queries.group.loading" />
 
-    <div v-else-if="dependencyProxyEnabled" data-testid="main-area">
+    <div v-else data-testid="main-area">
       <gl-form-group :label="$options.i18n.proxyImagePrefix">
         <gl-form-input-group
           readonly
@@ -170,12 +159,5 @@ export default {
         :title="$options.i18n.noManifestTitle"
       />
     </div>
-    <gl-alert v-else :dismissible="false" data-testid="proxy-disabled">
-      <gl-sprintf :message="$options.i18n.proxyDisabledText">
-        <template #docLink="{ content }">
-          <gl-link :href="$options.links.ENABLE_DEPENDENCY_PROXY_DOCS_PATH">{{ content }}</gl-link>
-        </template>
-      </gl-sprintf>
-    </gl-alert>
   </div>
 </template>

app/assets/javascripts/packages_and_registries/dependency_proxy/constants.js

-import { helpPagePath } from '~/helpers/help_page_helper';
-
 export const GRAPHQL_PAGE_SIZE = 20;
-export const ENABLE_DEPENDENCY_PROXY_DOCS_PATH = helpPagePath(
-  'user/packages/dependency_proxy/index',
-  { anchor: 'enable-or-disable-the-dependency-proxy-for-a-group' },
-);

app/controllers/concerns/dependency_proxy/group_access.rb

@@ -5,13 +5,13 @@ module DependencyProxy
     extend ActiveSupport::Concern
 
     included do
-      before_action :verify_dependency_proxy_enabled!
+      before_action :verify_dependency_proxy_available!
       before_action :authorize_read_dependency_proxy!
     end
 
     private
 
-    def verify_dependency_proxy_enabled!
+    def verify_dependency_proxy_available!
       render_404 unless group&.dependency_proxy_feature_available?
     end
 

app/controllers/groups/dependency_proxies_controller.rb

@@ -5,30 +5,19 @@ module Groups
     include ::DependencyProxy::GroupAccess
 
     before_action :authorize_admin_dependency_proxy!, only: :update
-    before_action :dependency_proxy
+    before_action :verify_dependency_proxy_enabled!
 
     feature_category :package_registry
 
-    def show
-      @blobs_count = group.dependency_proxy_blobs.count
-      @blobs_total_size = group.dependency_proxy_blobs.total_size
-    end
-
-    def update
-      dependency_proxy.update(dependency_proxy_params)
-
-      redirect_to group_dependency_proxy_path(group)
-    end
-
     private
 
     def dependency_proxy
       @dependency_proxy ||=
-        group.dependency_proxy_setting || group.create_dependency_proxy_setting
+        group.dependency_proxy_setting || group.create_dependency_proxy_setting!
     end
 
-    def dependency_proxy_params
-      params.require(:dependency_proxy_group_setting).permit(:enabled)
+    def verify_dependency_proxy_enabled!
+      render_404 unless dependency_proxy.enabled?
     end
   end
 end

lib/sidebars/groups/menus/packages_registries_menu.rb

@@ -26,9 +26,7 @@ module Sidebars
         private
 
         def packages_registry_menu_item
-          unless context.group.packages_feature_enabled?
-            return ::Sidebars::NilMenuItem.new(item_id: :packages_registry)
-          end
+          return nil_menu_item(:packages_registry) unless context.group.packages_feature_enabled?
 
           ::Sidebars::MenuItem.new(
             title: _('Package Registry'),
@@ -40,7 +38,7 @@ module Sidebars
 
         def container_registry_menu_item
           if !::Gitlab.config.registry.enabled || !can?(context.current_user, :read_container_image, context.group)
-            return ::Sidebars::NilMenuItem.new(item_id: :container_registry)
+            return nil_menu_item(:container_registry)
           end
 
           ::Sidebars::MenuItem.new(
@@ -52,9 +50,11 @@ module Sidebars
         end
 
         def dependency_proxy_menu_item
-          unless can?(context.current_user, :read_dependency_proxy, context.group)
-            return ::Sidebars::NilMenuItem.new(item_id: :dependency_proxy)
-          end
+          setting_does_not_exist_or_is_enabled = !context.group.dependency_proxy_setting ||
+                                                  context.group.dependency_proxy_setting.enabled
+
+          return nil_menu_item(:dependency_proxy) unless can?(context.current_user, :read_dependency_proxy, context.group)
+          return nil_menu_item(:dependency_proxy) unless setting_does_not_exist_or_is_enabled
 
           ::Sidebars::MenuItem.new(
             title: _('Dependency Proxy'),
@@ -63,6 +63,10 @@ module Sidebars
             item_id: :dependency_proxy
           )
         end
+
+        def nil_menu_item(item_id)
+          ::Sidebars::NilMenuItem.new(item_id: item_id)
+        end
       end
     end
   end

locale/gitlab.pot

@@ -11567,9 +11567,6 @@ msgstr ""
 msgid "DependencyProxy|Storage settings"
 msgstr ""
 
-msgid "DependencyProxy|The Dependency Proxy is disabled. %{docLinkStart}Learn how to enable it%{docLinkEnd}."
-msgstr ""
-
 msgid "DependencyProxy|There are no images in the cache"
 msgstr ""
 

spec/controllers/groups/dependency_proxies_controller_spec.rb

@@ -3,8 +3,9 @@
 require 'spec_helper'
 
 RSpec.describe Groups::DependencyProxiesController do
-  let(:group) { create(:group) }
-  let(:user)  { create(:user) }
+  let_it_be(:group) { create(:group) }
+  let_it_be_with_reload(:dependency_proxy_group_setting) { create(:dependency_proxy_group_setting, group: group) }
+  let_it_be(:user) { create(:user) }
 
   before do
     group.add_owner(user)
@@ -12,62 +13,37 @@ RSpec.describe Groups::DependencyProxiesController do
   end
 
   describe 'GET #show' do
-    context 'feature enabled' do
+    subject { get :show, params: { group_id: group.to_param } }
+
     before do
-        enable_dependency_proxy
+      stub_config(dependency_proxy: { enabled: config_enabled })
     end
 
+    context 'with global config enabled' do
+      let(:config_enabled) { true }
+
+      context 'with the setting enabled' do
         it 'returns 200 and renders the view' do
-        get :show, params: { group_id: group.to_param }
+          subject
 
           expect(response).to have_gitlab_http_status(:ok)
           expect(response).to render_template('groups/dependency_proxies/show')
         end
       end
 
-    it 'returns 404 when feature is disabled' do
-      disable_dependency_proxy
-
-      get :show, params: { group_id: group.to_param }
-
-      expect(response).to have_gitlab_http_status(:not_found)
-    end
-  end
-
-  describe 'PUT #update' do
-    context 'feature enabled' do
+      context 'with the setting disabled' do
         before do
-        enable_dependency_proxy
+          dependency_proxy_group_setting.update!(enabled: false)
         end
 
-      it 'redirects back to show page' do
-        put :update, params: update_params
-
-        expect(response).to have_gitlab_http_status(:found)
+        it_behaves_like 'returning response status', :not_found
       end
     end
 
-    it 'returns 404 when feature is disabled' do
-      put :update, params: update_params
+    context 'with global config disabled' do
+      let(:config_enabled) { false }
 
-      expect(response).to have_gitlab_http_status(:not_found)
+      it_behaves_like 'returning response status', :not_found
     end
-
-    def update_params
-      {
-        group_id: group.to_param,
-        dependency_proxy_group_setting: { enabled: true }
-      }
-    end
-  end
-
-  def enable_dependency_proxy
-    stub_config(dependency_proxy: { enabled: true })
-
-    group.create_dependency_proxy_setting!(enabled: true)
-  end
-
-  def disable_dependency_proxy
-    group.create_dependency_proxy_setting!(enabled: false)
   end
 end

spec/frontend/packages_and_registries/dependency_proxy/app_spec.js

@@ -3,7 +3,6 @@ import {
   GlFormGroup,
   GlSkeletonLoader,
   GlSprintf,
-  GlLink,
   GlEmptyState,
 } from '@gitlab/ui';
 import { createLocalVue } from '@vue/test-utils';
@@ -12,10 +11,7 @@ import createMockApollo from 'helpers/mock_apollo_helper';
 import { shallowMountExtended } from 'helpers/vue_test_utils_helper';
 import { stripTypenames } from 'helpers/graphql_helpers';
 import waitForPromises from 'helpers/wait_for_promises';
-import {
-  GRAPHQL_PAGE_SIZE,
-  ENABLE_DEPENDENCY_PROXY_DOCS_PATH,
-} from '~/packages_and_registries/dependency_proxy/constants';
+import { GRAPHQL_PAGE_SIZE } from '~/packages_and_registries/dependency_proxy/constants';
 
 import DependencyProxyApp from '~/packages_and_registries/dependency_proxy/app.vue';
 import ClipboardButton from '~/vue_shared/components/clipboard_button.vue';
@@ -58,8 +54,6 @@ describe('DependencyProxyApp', () => {
   }
 
   const findProxyNotAvailableAlert = () => wrapper.findByTestId('proxy-not-available');
-  const findProxyDisabledAlert = () => wrapper.findByTestId('proxy-disabled');
-  const findDisabledAlertLink = () => findProxyDisabledAlert().findComponent(GlLink);
   const findClipBoardButton = () => wrapper.findComponent(ClipboardButton);
   const findFormGroup = () => wrapper.findComponent(GlFormGroup);
   const findFormInputGroup = () => wrapper.findComponent(GlFormInputGroup);
@@ -224,36 +218,6 @@ describe('DependencyProxyApp', () => {
           });
         });
       });
-
-      describe('when the dependency proxy is disabled', () => {
-        beforeEach(() => {
-          resolver = jest
-            .fn()
-            .mockResolvedValue(proxyDetailsQuery({ extendSettings: { enabled: false } }));
-          createComponent();
-          return waitForPromises();
-        });
-
-        it('does not show the main area', () => {
-          expect(findMainArea().exists()).toBe(false);
-        });
-
-        it('does not show the loader', () => {
-          expect(findSkeletonLoader().exists()).toBe(false);
-        });
-
-        it('shows a proxy disabled alert', () => {
-          expect(findProxyDisabledAlert().text()).toMatchInterpolatedText(
-            DependencyProxyApp.i18n.proxyDisabledText,
-          );
-        });
-
-        it('disabled alert has a link to the docs', () => {
-          expect(findDisabledAlertLink().attributes()).toMatchObject({
-            href: ENABLE_DEPENDENCY_PROXY_DOCS_PATH,
-          });
-        });
-      });
     });
   });
 });

spec/lib/sidebars/groups/menus/packages_registries_menu_spec.rb

@@ -4,7 +4,7 @@ require 'spec_helper'
 
 RSpec.describe Sidebars::Groups::Menus::PackagesRegistriesMenu do
   let_it_be(:owner) { create(:user) }
-  let_it_be(:group) do
+  let_it_be_with_reload(:group) do
     build(:group, :private).tap do |g|
       g.add_owner(owner)
     end
@@ -70,6 +70,18 @@ RSpec.describe Sidebars::Groups::Menus::PackagesRegistriesMenu do
   describe 'Menu items' do
     subject { find_menu(menu, item_id) }
 
+    shared_examples 'the menu entry is available' do
+      it 'the menu item is added to list of menu items' do
+        is_expected.not_to be_nil
+      end
+    end
+
+    shared_examples 'the menu entry is not available' do
+      it 'the menu item is not added to list of menu items' do
+        is_expected.to be_nil
+      end
+    end
+
     describe 'Packages Registry' do
       let(:item_id) { :packages_registry }
 
@@ -81,17 +93,13 @@ RSpec.describe Sidebars::Groups::Menus::PackagesRegistriesMenu do
         context 'when config package setting is disabled' do
           let(:packages_enabled) { false }
 
-          it 'the menu item is not added to list of menu items' do
-            is_expected.to be_nil
-          end
+          it_behaves_like 'the menu entry is not available'
         end
 
         context 'when config package setting is enabled' do
           let(:packages_enabled) { true }
 
-          it 'the menu item is added to list of menu items' do
-            is_expected.not_to be_nil
-          end
+          it_behaves_like 'the menu entry is available'
         end
       end
     end
@@ -107,24 +115,18 @@ RSpec.describe Sidebars::Groups::Menus::PackagesRegistriesMenu do
         context 'when config registry setting is disabled' do
           let(:container_enabled) { false }
 
-          it 'the menu item is not added to list of menu items' do
-            is_expected.to be_nil
-          end
+          it_behaves_like 'the menu entry is not available'
         end
 
         context 'when config registry setting is enabled' do
           let(:container_enabled) { true }
 
-          it 'the menu item is added to list of menu items' do
-            is_expected.not_to be_nil
-          end
+          it_behaves_like 'the menu entry is available'
 
           context 'when user cannot read container images' do
             let(:user) { nil }
 
-            it 'the menu item is not added to list of menu items' do
-              is_expected.to be_nil
-            end
+            it_behaves_like 'the menu entry is not available'
           end
         end
       end
@@ -141,17 +143,28 @@ RSpec.describe Sidebars::Groups::Menus::PackagesRegistriesMenu do
         context 'when config dependency_proxy is enabled' do
           let(:dependency_enabled) { true }
 
-          it 'the menu item is added to list of menu items' do
-            is_expected.not_to be_nil
+          it_behaves_like 'the menu entry is available'
+
+          context 'when the group settings exist' do
+            let_it_be(:dependency_proxy_group_setting) { create(:dependency_proxy_group_setting, group: group) }
+
+            it_behaves_like 'the menu entry is available'
+
+            context 'when the proxy is disabled at the group level' do
+              before do
+                dependency_proxy_group_setting.enabled = false
+                dependency_proxy_group_setting.save!
+              end
+
+              it_behaves_like 'the menu entry is not available'
+            end
           end
         end
 
         context 'when config dependency_proxy is not enabled' do
           let(:dependency_enabled) { false }
 
-          it 'the menu item is not added to list of menu items' do
-            is_expected.to be_nil
-          end
+          it_behaves_like 'the menu entry is not available'
         end
       end
 
@@ -159,9 +172,7 @@ RSpec.describe Sidebars::Groups::Menus::PackagesRegistriesMenu do
         let(:user) { nil }
         let(:dependency_enabled) { true }
 
-        it 'the menu item is not added to list of menu items' do
-          is_expected.to be_nil
-        end
+        it_behaves_like 'the menu entry is not available'
       end
     end
   end