Commit d1e8a0f6 authored by Reuben Pereira's avatar Reuben Pereira

Update readers of projects.container_registry_enabled

Update readers of projects.container_registry_enabled to read
project_features.container_registry_access_level.

Writers will continue writing to projects.container_registry_enabled.
The before_update callback on the Project model and the before_create
on ProjectFeature model will copy the value to
project_features.container_registry_access_level.
parent d888a0fa
...@@ -24,7 +24,7 @@ class ContainerRepository < ApplicationRecord ...@@ -24,7 +24,7 @@ class ContainerRepository < ApplicationRecord
scope :for_group_and_its_subgroups, ->(group) do scope :for_group_and_its_subgroups, ->(group) do
project_scope = Project project_scope = Project
.for_group_and_its_subgroups(group) .for_group_and_its_subgroups(group)
.with_container_registry .with_feature_enabled(:container_registry)
.select(:id) .select(:id)
joins("INNER JOIN (#{project_scope.to_sql}) projects on projects.id=container_repositories.project_id") joins("INNER JOIN (#{project_scope.to_sql}) projects on projects.id=container_repositories.project_id")
......
...@@ -406,8 +406,9 @@ class Project < ApplicationRecord ...@@ -406,8 +406,9 @@ class Project < ApplicationRecord
:wiki_access_level, :snippets_access_level, :builds_access_level, :wiki_access_level, :snippets_access_level, :builds_access_level,
:repository_access_level, :pages_access_level, :metrics_dashboard_access_level, :analytics_access_level, :repository_access_level, :pages_access_level, :metrics_dashboard_access_level, :analytics_access_level,
:operations_enabled?, :operations_access_level, :security_and_compliance_access_level, :operations_enabled?, :operations_access_level, :security_and_compliance_access_level,
:container_registry_access_level, :container_registry_access_level, :container_registry_enabled?,
to: :project_feature, allow_nil: true to: :project_feature, allow_nil: true
alias_method :container_registry_enabled, :container_registry_enabled?
delegate :show_default_award_emojis, :show_default_award_emojis=, delegate :show_default_award_emojis, :show_default_award_emojis=,
:show_default_award_emojis?, :show_default_award_emojis?,
to: :project_setting, allow_nil: true to: :project_setting, allow_nil: true
...@@ -546,7 +547,6 @@ class Project < ApplicationRecord ...@@ -546,7 +547,6 @@ class Project < ApplicationRecord
scope :include_project_feature, -> { includes(:project_feature) } scope :include_project_feature, -> { includes(:project_feature) }
scope :with_service, ->(service) { joins(service).eager_load(service) } scope :with_service, ->(service) { joins(service).eager_load(service) }
scope :with_shared_runners, -> { where(shared_runners_enabled: true) } scope :with_shared_runners, -> { where(shared_runners_enabled: true) }
scope :with_container_registry, -> { where(container_registry_enabled: true) }
scope :inside_path, ->(path) do scope :inside_path, ->(path) do
# We need routes alias rs for JOIN so it does not conflict with # We need routes alias rs for JOIN so it does not conflict with
# includes(:route) which we use in ProjectsFinder. # includes(:route) which we use in ProjectsFinder.
......
...@@ -96,7 +96,7 @@ class ProjectFeature < ApplicationRecord ...@@ -96,7 +96,7 @@ class ProjectFeature < ApplicationRecord
def set_container_registry_access_level def set_container_registry_access_level
self.container_registry_access_level = self.container_registry_access_level =
if project&.container_registry_enabled if project&.read_attribute(:container_registry_enabled)
ENABLED ENABLED
else else
DISABLED DISABLED
......
...@@ -43,7 +43,6 @@ module API ...@@ -43,7 +43,6 @@ module API
expose :visibility expose :visibility
expose :owner, using: Entities::UserBasic, unless: ->(project, options) { project.group } expose :owner, using: Entities::UserBasic, unless: ->(project, options) { project.group }
expose :resolve_outdated_diff_discussions expose :resolve_outdated_diff_discussions
expose :container_registry_enabled
expose :container_expiration_policy, using: Entities::ContainerExpirationPolicy, expose :container_expiration_policy, using: Entities::ContainerExpirationPolicy,
if: -> (project, _) { project.container_expiration_policy } if: -> (project, _) { project.container_expiration_policy }
...@@ -54,6 +53,7 @@ module API ...@@ -54,6 +53,7 @@ module API
expose(:wiki_enabled) { |project, options| project.feature_available?(:wiki, options[:current_user]) } expose(:wiki_enabled) { |project, options| project.feature_available?(:wiki, options[:current_user]) }
expose(:jobs_enabled) { |project, options| project.feature_available?(:builds, options[:current_user]) } expose(:jobs_enabled) { |project, options| project.feature_available?(:builds, options[:current_user]) }
expose(:snippets_enabled) { |project, options| project.feature_available?(:snippets, options[:current_user]) } expose(:snippets_enabled) { |project, options| project.feature_available?(:snippets, options[:current_user]) }
expose(:container_registry_enabled) { |project, options| project.feature_available?(:container_registry, options[:current_user]) }
expose :service_desk_enabled expose :service_desk_enabled
expose :service_desk_address expose :service_desk_address
......
...@@ -3,7 +3,8 @@ ...@@ -3,7 +3,8 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe Sidebars::Projects::Menus::PackagesRegistriesMenu do RSpec.describe Sidebars::Projects::Menus::PackagesRegistriesMenu do
let(:project) { build(:project) } let_it_be(:project) { create(:project) }
let(:user) { project.owner } let(:user) { project.owner }
let(:context) { Sidebars::Projects::Context.new(current_user: user, container: project) } let(:context) { Sidebars::Projects::Context.new(current_user: user, container: project) }
......
...@@ -3,7 +3,8 @@ ...@@ -3,7 +3,8 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe Sidebars::Projects::Menus::SettingsMenu do RSpec.describe Sidebars::Projects::Menus::SettingsMenu do
let(:project) { build(:project) } let_it_be(:project) { create(:project) }
let(:user) { project.owner } let(:user) { project.owner }
let(:context) { Sidebars::Projects::Context.new(current_user: user, container: project) } let(:context) { Sidebars::Projects::Context.new(current_user: user, container: project) }
......
...@@ -653,6 +653,8 @@ RSpec.describe Project, factory_default: :keep do ...@@ -653,6 +653,8 @@ RSpec.describe Project, factory_default: :keep do
it { is_expected.to delegate_method(:root_ancestor).to(:namespace).with_arguments(allow_nil: true) } it { is_expected.to delegate_method(:root_ancestor).to(:namespace).with_arguments(allow_nil: true) }
it { is_expected.to delegate_method(:last_pipeline).to(:commit).with_arguments(allow_nil: true) } it { is_expected.to delegate_method(:last_pipeline).to(:commit).with_arguments(allow_nil: true) }
it { is_expected.to delegate_method(:allow_editing_commit_messages?).to(:project_setting) } it { is_expected.to delegate_method(:allow_editing_commit_messages?).to(:project_setting) }
it { is_expected.to delegate_method(:container_registry_enabled?).to(:project_feature) }
it { is_expected.to delegate_method(:container_registry_access_level).to(:project_feature) }
end end
describe 'reference methods' do describe 'reference methods' do
...@@ -2285,35 +2287,41 @@ RSpec.describe Project, factory_default: :keep do ...@@ -2285,35 +2287,41 @@ RSpec.describe Project, factory_default: :keep do
it 'updates project_feature', :aggregate_failures do it 'updates project_feature', :aggregate_failures do
# Simulate an existing project that has container_registry enabled # Simulate an existing project that has container_registry enabled
project.update_column(:container_registry_enabled, true) project.update_column(:container_registry_enabled, true)
project.project_feature.update_column(:container_registry_access_level, ProjectFeature::DISABLED) project.project_feature.update_column(:container_registry_access_level, ProjectFeature::ENABLED)
expect(project.container_registry_enabled).to eq(true)
expect(project.project_feature.container_registry_access_level).to eq(ProjectFeature::DISABLED)
project.update!(container_registry_enabled: false) project.update!(container_registry_enabled: false)
expect(project.container_registry_enabled).to eq(false) expect(project.read_attribute(:container_registry_enabled)).to eq(false)
expect(project.project_feature.container_registry_access_level).to eq(ProjectFeature::DISABLED) expect(project.project_feature.container_registry_access_level).to eq(ProjectFeature::DISABLED)
project.update!(container_registry_enabled: true) project.update!(container_registry_enabled: true)
expect(project.container_registry_enabled).to eq(true) expect(project.read_attribute(:container_registry_enabled)).to eq(true)
expect(project.project_feature.container_registry_access_level).to eq(ProjectFeature::ENABLED) expect(project.project_feature.container_registry_access_level).to eq(ProjectFeature::ENABLED)
end end
it 'rollsback both projects and project_features row in case of error', :aggregate_failures do it 'rollsback both projects and project_features row in case of error', :aggregate_failures do
project.update_column(:container_registry_enabled, true) project.update_column(:container_registry_enabled, true)
project.project_feature.update_column(:container_registry_access_level, ProjectFeature::DISABLED) project.project_feature.update_column(:container_registry_access_level, ProjectFeature::ENABLED)
expect(project.container_registry_enabled).to eq(true)
expect(project.project_feature.container_registry_access_level).to eq(ProjectFeature::DISABLED)
allow(project).to receive(:valid?).and_return(false) allow(project).to receive(:valid?).and_return(false)
expect { project.update!(container_registry_enabled: false) }.to raise_error(ActiveRecord::RecordInvalid) expect { project.update!(container_registry_enabled: false) }.to raise_error(ActiveRecord::RecordInvalid)
expect(project.reload.container_registry_enabled).to eq(true) expect(project.reload.read_attribute(:container_registry_enabled)).to eq(true)
expect(project.project_feature.reload.container_registry_access_level).to eq(ProjectFeature::DISABLED) expect(project.project_feature.reload.container_registry_access_level).to eq(ProjectFeature::ENABLED)
end
end
describe '#container_registry_enabled' do
let_it_be_with_reload(:project) { create(:project) }
it 'delegates to project_feature', :aggregate_failures do
project.update_column(:container_registry_enabled, true)
project.project_feature.update_column(:container_registry_access_level, ProjectFeature::DISABLED)
expect(project.container_registry_enabled).to eq(false)
expect(project.container_registry_enabled?).to eq(false)
end end
end end
......
...@@ -184,6 +184,17 @@ RSpec.describe API::Projects do ...@@ -184,6 +184,17 @@ RSpec.describe API::Projects do
end end
end end
it 'includes correct value of container_registry_enabled', :aggregate_failures do
project.project_feature.update!(container_registry_access_level: 0)
get api('/projects', user)
project_response = json_response.find { |p| p['id'] == project.id }
expect(response).to have_gitlab_http_status(:ok)
expect(json_response).to be_an Array
expect(project_response['container_registry_enabled']).to eq(false)
end
it 'includes project topics' do it 'includes project topics' do
get api('/projects', user) get api('/projects', user)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment