Commit 0e577c07 authored by Nikola Milojevic's avatar Nikola Milojevic

Merge branch 'afontaine/drop-old-feature-flag-scopes' into 'master'

Remove Legacy Feature Flag Scopes

See merge request gitlab-org/gitlab!69806
parents 71059e50 ff282fb5
...@@ -19,13 +19,10 @@ module Operations ...@@ -19,13 +19,10 @@ module Operations
default_value_for :active, true default_value_for :active, true
default_value_for :version, :new_version_flag default_value_for :version, :new_version_flag
# scopes exists only for the first version
has_many :scopes, class_name: 'Operations::FeatureFlagScope'
# strategies exists only for the second version # strategies exists only for the second version
has_many :strategies, class_name: 'Operations::FeatureFlags::Strategy' has_many :strategies, class_name: 'Operations::FeatureFlags::Strategy'
has_many :feature_flag_issues has_many :feature_flag_issues
has_many :issues, through: :feature_flag_issues has_many :issues, through: :feature_flag_issues
has_one :default_scope, -> { where(environment_scope: '*') }, class_name: 'Operations::FeatureFlagScope'
validates :project, presence: true validates :project, presence: true
validates :name, validates :name,
...@@ -37,10 +34,7 @@ module Operations ...@@ -37,10 +34,7 @@ module Operations
} }
validates :name, uniqueness: { scope: :project_id } validates :name, uniqueness: { scope: :project_id }
validates :description, allow_blank: true, length: 0..255 validates :description, allow_blank: true, length: 0..255
validate :first_default_scope, on: :create, if: :has_scopes?
validate :version_associations
accepts_nested_attributes_for :scopes, allow_destroy: true
accepts_nested_attributes_for :strategies, allow_destroy: true accepts_nested_attributes_for :strategies, allow_destroy: true
scope :ordered, -> { order(:name) } scope :ordered, -> { order(:name) }
...@@ -56,7 +50,7 @@ module Operations ...@@ -56,7 +50,7 @@ module Operations
class << self class << self
def preload_relations def preload_relations
preload(:scopes, strategies: :scopes) preload(strategies: :scopes)
end end
def for_unleash_client(project, environment) def for_unleash_client(project, environment)
...@@ -119,27 +113,5 @@ module Operations ...@@ -119,27 +113,5 @@ module Operations
active: active active: active
} }
end end
private
def version_associations
if new_version_flag? && scopes.any?
errors.add(:version_associations, 'version 2 feature flags may not have scopes')
end
end
def first_default_scope
unless scopes.first.environment_scope == '*'
errors.add(:default_scope, 'has to be the first element')
end
end
def build_default_scope
scopes.build(environment_scope: '*', active: self.active)
end
def has_scopes?
scopes.any?
end
end end
end end
# frozen_string_literal: true
# All of the legacy flags have been removed in 14.1, including all of the
# `operations_feature_flag_scopes` rows. Therefore, this model and the database
# table are unused and should be removed.
module Operations
class FeatureFlagScope < ApplicationRecord
prepend HasEnvironmentScope
include Gitlab::Utils::StrongMemoize
self.table_name = 'operations_feature_flag_scopes'
belongs_to :feature_flag
validates :environment_scope, uniqueness: {
scope: :feature_flag,
message: "(%{value}) has already been taken"
}
validates :environment_scope,
if: :default_scope?, on: :update,
inclusion: { in: %w(*), message: 'cannot be changed from default scope' }
validates :strategies, feature_flag_strategies: true
before_destroy :prevent_destroy_default_scope, if: :default_scope?
scope :ordered, -> { order(:id) }
scope :enabled, -> { where(active: true) }
scope :disabled, -> { where(active: false) }
def self.with_name_and_description
joins(:feature_flag)
.select(FeatureFlag.arel_table[:name], FeatureFlag.arel_table[:description])
end
def self.for_unleash_client(project, environment)
select_columns = [
'DISTINCT ON (operations_feature_flag_scopes.feature_flag_id) operations_feature_flag_scopes.id',
'(operations_feature_flags.active AND operations_feature_flag_scopes.active) AS active',
'operations_feature_flag_scopes.strategies',
'operations_feature_flag_scopes.environment_scope',
'operations_feature_flag_scopes.created_at',
'operations_feature_flag_scopes.updated_at'
]
select(select_columns)
.with_name_and_description
.where(feature_flag_id: project.operations_feature_flags.select(:id))
.order(:feature_flag_id)
.on_environment(environment)
.reverse_order
end
private
def default_scope?
environment_scope_was == '*'
end
def prevent_destroy_default_scope
raise ActiveRecord::ReadOnlyRecord, "default scope cannot be destroyed"
end
end
end
...@@ -24,8 +24,8 @@ class FeatureFlagEntity < Grape::Entity ...@@ -24,8 +24,8 @@ class FeatureFlagEntity < Grape::Entity
project_feature_flag_path(feature_flag.project, feature_flag) project_feature_flag_path(feature_flag.project, feature_flag)
end end
expose :scopes, with: FeatureFlagScopeEntity do |feature_flag| expose :scopes do |_ff|
feature_flag.scopes.sort_by(&:id) []
end end
expose :strategies, with: FeatureFlags::StrategyEntity do |feature_flag| expose :strategies, with: FeatureFlags::StrategyEntity do |feature_flag|
......
# frozen_string_literal: true
class FeatureFlagScopeEntity < Grape::Entity
include RequestAwareEntity
expose :id
expose :active
expose :environment_scope
expose :created_at
expose :updated_at
expose :strategies
end
...@@ -9,7 +9,9 @@ module API ...@@ -9,7 +9,9 @@ module API
expose :version expose :version
expose :created_at expose :created_at
expose :updated_at expose :updated_at
expose :scopes, using: FeatureFlag::LegacyScope expose :scopes do |_ff|
[]
end
expose :strategies, using: FeatureFlag::Strategy expose :strategies, using: FeatureFlag::Strategy
end end
end end
......
# frozen_string_literal: true
module API
module Entities
class FeatureFlag < Grape::Entity
class DetailedLegacyScope < LegacyScope
expose :name
end
end
end
end
# frozen_string_literal: true
module API
module Entities
class FeatureFlag < Grape::Entity
class LegacyScope < Grape::Entity
expose :id
expose :active
expose :environment_scope
expose :strategies
expose :created_at
expose :updated_at
end
end
end
end
...@@ -13,7 +13,7 @@ RSpec.describe Operations::FeatureFlag do ...@@ -13,7 +13,7 @@ RSpec.describe Operations::FeatureFlag do
describe 'associations' do describe 'associations' do
it { is_expected.to belong_to(:project) } it { is_expected.to belong_to(:project) }
it { is_expected.to have_many(:scopes) } it { is_expected.to have_many(:strategies) }
end end
describe '.reference_pattern' do describe '.reference_pattern' do
...@@ -52,17 +52,6 @@ RSpec.describe Operations::FeatureFlag do ...@@ -52,17 +52,6 @@ RSpec.describe Operations::FeatureFlag do
it { is_expected.to define_enum_for(:version).with_values(new_version_flag: 2) } it { is_expected.to define_enum_for(:version).with_values(new_version_flag: 2) }
context 'a version 2 feature flag' do context 'a version 2 feature flag' do
it 'is invalid if associated with Operations::FeatureFlagScope models' do
project = create(:project)
feature_flag = described_class.new({ name: 'test', project: project, version: 2,
scopes_attributes: [{ environment_scope: '*', active: false }] })
expect(feature_flag.valid?).to eq(false)
expect(feature_flag.errors.messages).to eq({
version_associations: ["version 2 feature flags may not have scopes"]
})
end
it 'is valid if associated with Operations::FeatureFlags::Strategy models' do it 'is valid if associated with Operations::FeatureFlags::Strategy models' do
project = create(:project) project = create(:project)
feature_flag = described_class.create!({ name: 'test', project: project, version: 2, feature_flag = described_class.create!({ name: 'test', project: project, version: 2,
...@@ -81,18 +70,6 @@ RSpec.describe Operations::FeatureFlag do ...@@ -81,18 +70,6 @@ RSpec.describe Operations::FeatureFlag do
end end
end end
describe 'the default scope' do
let_it_be(:project) { create(:project) }
context 'with a version 2 feature flag' do
it 'does not create a default scope' do
feature_flag = described_class.create!({ name: 'test', project: project, scopes_attributes: [], version: 2 })
expect(feature_flag.scopes).to eq([])
end
end
end
describe '.enabled' do describe '.enabled' do
subject { described_class.enabled } subject { described_class.enabled }
......
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