diff --git a/app/helpers/container_registry/container_registry_helper.rb b/app/helpers/container_registry/container_registry_helper.rb index ef78e26ad42d7382e3a90e284647f2be1aae67c6..25aa21cbd90a1ebcb6dc5d508eb811bb0da1dafe 100644 --- a/app/helpers/container_registry/container_registry_helper.rb +++ b/app/helpers/container_registry/container_registry_helper.rb @@ -29,7 +29,7 @@ def project_container_registry_template_data(project, connection_error, invalid_ is_admin: current_user&.admin.to_s, show_cleanup_policy_link: show_cleanup_policy_link(project).to_s, show_container_registry_settings: show_container_registry_settings(project).to_s, - cleanup_policies_settings_path: project_settings_packages_and_registries_path(project), + cleanup_policies_settings_path: cleanup_image_tags_project_settings_packages_and_registries_path(project), connection_error: (!!connection_error).to_s, invalid_path_error: (!!invalid_path_error).to_s, user_callouts_path: callouts_path, diff --git a/app/helpers/packages_helper.rb b/app/helpers/packages_helper.rb index be5a5ab21bead1e0faf70abf05a33d75658a0049..88bb04c904bd1758fb8d5b1a008ad974cdc417e8 100644 --- a/app/helpers/packages_helper.rb +++ b/app/helpers/packages_helper.rb @@ -48,11 +48,10 @@ def track_package_event(event_name, scope, **args) end def show_cleanup_policy_link(project) - Gitlab.com? && - Gitlab.config.registry.enabled && + show_container_registry_settings(project) && project.feature_available?(:container_registry, current_user) && - project.container_expiration_policy.nil? && - project.container_repositories.exists? + project.container_repositories.exists? && + !project.container_expiration_policy&.enabled end def show_container_registry_settings(project) diff --git a/spec/helpers/container_registry/container_registry_helper_spec.rb b/spec/helpers/container_registry/container_registry_helper_spec.rb index 5174c477ff0a62787af8a730152ec479dcd83499..b34235327ef6a2fdbf1675ac6c978c38de9ec3d0 100644 --- a/spec/helpers/container_registry/container_registry_helper_spec.rb +++ b/spec/helpers/container_registry/container_registry_helper_spec.rb @@ -48,7 +48,8 @@ is_admin: user.admin.to_s, show_cleanup_policy_link: helper.show_cleanup_policy_link(project).to_s, show_container_registry_settings: helper.show_container_registry_settings(project).to_s, - cleanup_policies_settings_path: helper.project_settings_packages_and_registries_path(project), + cleanup_policies_settings_path: + helper.cleanup_image_tags_project_settings_packages_and_registries_path(project), connection_error: (!!connection_error).to_s, invalid_path_error: (!!invalid_path_error).to_s, user_callouts_path: callouts_path, diff --git a/spec/helpers/packages_helper_spec.rb b/spec/helpers/packages_helper_spec.rb index 2d21961f9f11b09ff2a20b2f622ee2551108a385..f5beb65eb22091e1bfec085134f084635c06db50 100644 --- a/spec/helpers/packages_helper_spec.rb +++ b/spec/helpers/packages_helper_spec.rb @@ -115,53 +115,69 @@ subject { helper.show_cleanup_policy_link(project.reload) } - where(:com, :config_registry, :project_registry, :nil_policy, :container_repositories_exist, :expected_result) do - false | false | false | false | false | false - false | false | false | false | true | false - false | false | false | true | false | false - false | false | false | true | true | false - false | false | true | false | false | false - false | false | true | false | true | false - false | false | true | true | false | false - false | false | true | true | true | false - false | true | false | false | false | false - false | true | false | false | true | false - false | true | false | true | false | false - false | true | false | true | true | false - false | true | true | false | false | false - false | true | true | false | true | false - false | true | true | true | false | false - false | true | true | true | true | false - true | false | false | false | false | false - true | false | false | false | true | false - true | false | false | true | false | false - true | false | false | true | true | false - true | false | true | false | false | false - true | false | true | false | true | false - true | false | true | true | false | false - true | false | true | true | true | false - true | true | false | false | false | false - true | true | false | false | true | false - true | true | false | true | false | false - true | true | false | true | true | false - true | true | true | false | false | false - true | true | true | false | true | false - true | true | true | true | false | false - true | true | true | true | true | true + context 'when user has permission' do + where(:config_registry, :project_registry, :nil_policy, :container_repositories_exist, :expected_result) do + false | false | false | false | false + false | false | false | true | false + false | false | true | false | false + false | false | true | true | false + false | true | false | false | false + false | true | false | true | false + false | true | true | false | false + false | true | true | true | false + true | false | false | false | false + true | false | false | true | false + true | false | true | false | false + true | false | true | true | false + true | true | false | false | false + true | true | false | true | false + true | true | true | false | false + true | true | true | true | true + end + + with_them do + before do + project.add_owner(user) + allow(helper).to receive(:current_user).and_return(user) + stub_config(registry: { enabled: config_registry }) + allow(project).to receive(:feature_available?).with(:container_registry, user).and_return(project_registry) + + project.container_expiration_policy.update!(enabled: true) + + project.container_expiration_policy.destroy! if nil_policy + container_repository.update!(project_id: project.id) if container_repositories_exist + end + + it { is_expected.to eq(expected_result) } + end end - with_them do + context 'when user does not have permission' do before do + project.add_developer(user) allow(helper).to receive(:current_user).and_return(user) - allow(Gitlab).to receive(:com?).and_return(com) - stub_config(registry: { enabled: config_registry }) - allow(project).to receive(:feature_available?).with(:container_registry, user).and_return(project_registry) + stub_config(registry: { enabled: true }) + allow(project).to receive(:feature_available?).with(:container_registry, user).and_return(true) + + project.container_expiration_policy.update!(enabled: true) + container_repository.update!(project_id: project.id) + end + + it { is_expected.to eq(false) } + end + + context 'when container expiration policy is disabled' do + before do + project.add_owner(user) + allow(helper).to receive(:current_user).and_return(user) + stub_config(registry: { enabled: true }) + allow(project).to receive(:feature_available?).with(:container_registry, user).and_return(true) + container_repository.update!(project_id: project.id) - project.container_expiration_policy.destroy! if nil_policy - container_repository.update!(project_id: project.id) if container_repositories_exist + project.container_expiration_policy.update!(enabled: false) end - it { is_expected.to eq(expected_result) } + it { is_expected.to eq(true) } end end