Skip to content

G
gitlab-ce
Commit 6db15755 authored by James Fargher's avatar James Fargher
Browse files
Options

Merge branch 'remove-design-reordering-feature-flag' into 'master' 

Remove reorder_designs feature flag

See merge request gitlab-org/gitlab!40866
parents 998b2cd1 671a38b3
Hide whitespace changes
Inline Side-by-side
Showing with 8 additions and 113 deletions
app/finders/design_management/designs_finder.rb
View file @ 6db15755
......@@ -22,10 +22,7 @@ module DesignManagement
items = by_visible_at_version(items)
items = by_filename(items)
items = by_id(items)
# TODO: We don't need to pass the project anymore after the feature flag is removed
# https://gitlab.com/gitlab-org/gitlab/-/issues/34382
items.ordered(issue.project)
items.ordered
end
private
......
app/graphql/mutations/design_management/move.rb
View file @ 6db15755
......@@ -20,10 +20,6 @@ module Mutations
null: true,
description: "The current state of the collection"
def ready(*)
raise ::Gitlab::Graphql::Errors::ResourceNotAvailable unless ::Feature.enabled?(:reorder_designs, default_enabled: true)
end
def resolve(**args)
service = ::DesignManagement::MoveDesignsService.new(current_user, parameters(args))
......
app/models/design_management/design.rb
View file @ 6db15755
......@@ -79,16 +79,10 @@ module DesignManagement
joins(join.join_sources).where(actions[:event].not_eq(deletion))
end
scope :ordered, -> (project) do
# TODO: Always order by relative position after the feature flag is removed
# https://gitlab.com/gitlab-org/gitlab/-/issues/34382
if Feature.enabled?(:reorder_designs, project, default_enabled: true)
# We need to additionally sort by `id` to support keyset pagination.
# See https://gitlab.com/gitlab-org/gitlab/-/merge_requests/17788/diffs#note_230875678
order(:relative_position, :id)
else
in_creation_order
end
scope :ordered, -> do
# We need to additionally sort by `id` to support keyset pagination.
# See https://gitlab.com/gitlab-org/gitlab/-/merge_requests/17788/diffs#note_230875678
order(:relative_position, :id)
end
scope :in_creation_order, -> { reorder(:id) }
......
app/policies/project_policy.rb
View file @ 6db15755
......@@ -101,11 +101,6 @@ class ProjectPolicy < BasePolicy
!@subject.design_management_enabled?
end
with_scope :subject
condition(:moving_designs_disabled) do
!::Feature.enabled?(:reorder_designs, @subject, default_enabled: true)
end
with_scope :subject
condition(:service_desk_enabled) { @subject.service_desk_enabled? }
......@@ -557,10 +552,6 @@ class ProjectPolicy < BasePolicy
prevent :move_design
end
rule { moving_designs_disabled }.policy do
prevent :move_design
end
rule { read_package_registry_deploy_token }.policy do
enable :read_package
enable :read_project
......
app/services/design_management/move_designs_service.rb
View file @ 6db15755
......@@ -13,7 +13,6 @@ module DesignManagement
def execute
return error(:no_focus) unless current_design.present?
return error(:cannot_move) unless ::Feature.enabled?(:reorder_designs, project, default_enabled: true)
return error(:cannot_move) unless current_user.can?(:move_design, current_design)
return error(:no_neighbors) unless neighbors.present?
return error(:not_distinct) unless all_distinct?
......
config/feature_flags/development/reorder_designs.yml deleted 100644 → 0
View file @ 998b2cd1
---
name: reorder_designs
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/37835
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/232992
group: group::knowledge
type: development
default_enabled: true
spec/finders/design_management/designs_finder_spec.rb
View file @ 6db15755
......@@ -42,26 +42,6 @@ RSpec.describe DesignManagement::DesignsFinder do
is_expected.to eq([design3, design2, design1])
end
context 'when the :reorder_designs feature is enabled for the project' do
before do
stub_feature_flags(reorder_designs: project)
end
it 'returns the designs sorted by their relative position' do
is_expected.to eq([design3, design2, design1])
end
end
context 'when the :reorder_designs feature is disabled' do
before do
stub_feature_flags(reorder_designs: false)
end
it 'returns the designs sorted by ID' do
is_expected.to eq([design1, design2, design3])
end
end
context 'when argument is the ids of designs' do
let(:params) { { ids: [design1.id] } }
......
spec/models/design_management/design_collection_spec.rb
View file @ 6db15755
......@@ -41,7 +41,7 @@ RSpec.describe DesignManagement::DesignCollection do
design2 = collection.find_or_create_design!(filename: 'design2.jpg')
expect(collection.designs.ordered(issue.project)).to eq([design1, design2])
expect(collection.designs.ordered).to eq([design1, design2])
end
end
......
spec/models/design_management/design_spec.rb
View file @ 6db15755
......@@ -161,27 +161,7 @@ RSpec.describe DesignManagement::Design do
end
it 'sorts by relative position and ID in ascending order' do
expect(described_class.ordered(issue.project)).to eq([design2, design1, design3, deleted_design])
end
context 'when the :reorder_designs feature is enabled for the project' do
before do
stub_feature_flags(reorder_designs: issue.project)
end
it 'sorts by relative position and ID in ascending order' do
expect(described_class.ordered(issue.project)).to eq([design2, design1, design3, deleted_design])
end
end
context 'when the :reorder_designs feature is disabled' do
before do
stub_feature_flags(reorder_designs: false)
end
it 'sorts by ID in ascending order' do
expect(described_class.ordered(issue.project)).to eq([design1, design2, design3, deleted_design])
end
expect(described_class.ordered).to eq([design2, design1, design3, deleted_design])
end
end
......
spec/policies/design_management/design_policy_spec.rb
View file @ 6db15755
......@@ -131,17 +131,6 @@ RSpec.describe DesignManagement::DesignPolicy do
it_behaves_like "design abilities available for members"
context 'when reorder_designs is not enabled' do
before do
stub_feature_flags(reorder_designs: false)
end
let(:current_user) { developer }
it { is_expected.to be_allowed(*(developer_design_abilities - [:move_design])) }
it { is_expected.to be_disallowed(:move_design) }
end
context "for guests in private projects" do
let_it_be(:project) { create(:project, :private) }
let(:current_user) { guest }
......
spec/services/design_management/move_designs_service_spec.rb
View file @ 6db15755
......@@ -32,30 +32,6 @@ RSpec.describe DesignManagement::MoveDesignsService do
describe '#execute' do
subject { service.execute }
context 'the feature is unavailable' do
let(:current_design) { designs.first }
let(:previous_design) { designs.second }
let(:next_design) { designs.third }
before do
stub_feature_flags(reorder_designs: false)
end
it 'raises cannot_move' do
expect(subject).to be_error.and(have_attributes(message: :cannot_move))
end
context 'but it is available on the current project' do
before do
stub_feature_flags(reorder_designs: issue.project)
end
it 'is successful' do
expect(subject).to be_success
end
end
end
context 'the user cannot move designs' do
let(:current_design) { designs.first }
let(:current_user) { build_stubbed(:user) }
......@@ -124,7 +100,7 @@ RSpec.describe DesignManagement::MoveDesignsService do
expect(subject).to be_success
expect(issue.designs.ordered(issue.project)).to eq([
expect(issue.designs.ordered).to eq([
# Existing designs which already had a relative_position set.
# These should stay at the beginning, in the same order.
other_design1,
......
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
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7