Commit 671a38b3 authored by Markus Koller's avatar Markus Koller

Remove reorder_designs feature flag

This feature was enabled by default for 13.3 and can now be removed.
parent cccc9bad
...@@ -22,10 +22,7 @@ module DesignManagement ...@@ -22,10 +22,7 @@ module DesignManagement
items = by_visible_at_version(items) items = by_visible_at_version(items)
items = by_filename(items) items = by_filename(items)
items = by_id(items) items = by_id(items)
items.ordered
# 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)
end end
private private
......
...@@ -20,10 +20,6 @@ module Mutations ...@@ -20,10 +20,6 @@ module Mutations
null: true, null: true,
description: "The current state of the collection" 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) def resolve(**args)
service = ::DesignManagement::MoveDesignsService.new(current_user, parameters(args)) service = ::DesignManagement::MoveDesignsService.new(current_user, parameters(args))
......
...@@ -79,16 +79,10 @@ module DesignManagement ...@@ -79,16 +79,10 @@ module DesignManagement
joins(join.join_sources).where(actions[:event].not_eq(deletion)) joins(join.join_sources).where(actions[:event].not_eq(deletion))
end end
scope :ordered, -> (project) do scope :ordered, -> 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. # We need to additionally sort by `id` to support keyset pagination.
# See https://gitlab.com/gitlab-org/gitlab/-/merge_requests/17788/diffs#note_230875678 # See https://gitlab.com/gitlab-org/gitlab/-/merge_requests/17788/diffs#note_230875678
order(:relative_position, :id) order(:relative_position, :id)
else
in_creation_order
end
end end
scope :in_creation_order, -> { reorder(:id) } scope :in_creation_order, -> { reorder(:id) }
......
...@@ -101,11 +101,6 @@ class ProjectPolicy < BasePolicy ...@@ -101,11 +101,6 @@ class ProjectPolicy < BasePolicy
!@subject.design_management_enabled? !@subject.design_management_enabled?
end end
with_scope :subject
condition(:moving_designs_disabled) do
!::Feature.enabled?(:reorder_designs, @subject, default_enabled: true)
end
with_scope :subject with_scope :subject
condition(:service_desk_enabled) { @subject.service_desk_enabled? } condition(:service_desk_enabled) { @subject.service_desk_enabled? }
...@@ -557,10 +552,6 @@ class ProjectPolicy < BasePolicy ...@@ -557,10 +552,6 @@ class ProjectPolicy < BasePolicy
prevent :move_design prevent :move_design
end end
rule { moving_designs_disabled }.policy do
prevent :move_design
end
rule { read_package_registry_deploy_token }.policy do rule { read_package_registry_deploy_token }.policy do
enable :read_package enable :read_package
enable :read_project enable :read_project
......
...@@ -13,7 +13,6 @@ module DesignManagement ...@@ -13,7 +13,6 @@ module DesignManagement
def execute def execute
return error(:no_focus) unless current_design.present? 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(:cannot_move) unless current_user.can?(:move_design, current_design)
return error(:no_neighbors) unless neighbors.present? return error(:no_neighbors) unless neighbors.present?
return error(:not_distinct) unless all_distinct? return error(:not_distinct) unless all_distinct?
......
---
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
...@@ -42,26 +42,6 @@ RSpec.describe DesignManagement::DesignsFinder do ...@@ -42,26 +42,6 @@ RSpec.describe DesignManagement::DesignsFinder do
is_expected.to eq([design3, design2, design1]) is_expected.to eq([design3, design2, design1])
end 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 context 'when argument is the ids of designs' do
let(:params) { { ids: [design1.id] } } let(:params) { { ids: [design1.id] } }
......
...@@ -41,7 +41,7 @@ RSpec.describe DesignManagement::DesignCollection do ...@@ -41,7 +41,7 @@ RSpec.describe DesignManagement::DesignCollection do
design2 = collection.find_or_create_design!(filename: 'design2.jpg') 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
end end
......
...@@ -161,27 +161,7 @@ RSpec.describe DesignManagement::Design do ...@@ -161,27 +161,7 @@ RSpec.describe DesignManagement::Design do
end end
it 'sorts by relative position and ID in ascending order' do it 'sorts by relative position and ID in ascending order' do
expect(described_class.ordered(issue.project)).to eq([design2, design1, design3, deleted_design]) expect(described_class.ordered).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
end end
end end
......
...@@ -131,17 +131,6 @@ RSpec.describe DesignManagement::DesignPolicy do ...@@ -131,17 +131,6 @@ RSpec.describe DesignManagement::DesignPolicy do
it_behaves_like "design abilities available for members" 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 context "for guests in private projects" do
let_it_be(:project) { create(:project, :private) } let_it_be(:project) { create(:project, :private) }
let(:current_user) { guest } let(:current_user) { guest }
......
...@@ -32,30 +32,6 @@ RSpec.describe DesignManagement::MoveDesignsService do ...@@ -32,30 +32,6 @@ RSpec.describe DesignManagement::MoveDesignsService do
describe '#execute' do describe '#execute' do
subject { service.execute } 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 context 'the user cannot move designs' do
let(:current_design) { designs.first } let(:current_design) { designs.first }
let(:current_user) { build_stubbed(:user) } let(:current_user) { build_stubbed(:user) }
...@@ -124,7 +100,7 @@ RSpec.describe DesignManagement::MoveDesignsService do ...@@ -124,7 +100,7 @@ RSpec.describe DesignManagement::MoveDesignsService do
expect(subject).to be_success 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. # Existing designs which already had a relative_position set.
# These should stay at the beginning, in the same order. # These should stay at the beginning, in the same order.
other_design1, 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