Order designs by relative position

This is behind a feature flag, if disabled we fall back to the previous
order by ID.

app/finders/design_management/designs_finder.rb

@@ -22,7 +22,9 @@ module DesignManagement
       items = by_filename(items)
       items = by_id(items)
 
-      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)
     end
 
     private

app/models/design_management/design.rb

@@ -76,7 +76,19 @@ module DesignManagement
       join = designs.join(actions)
         .on(actions[:design_id].eq(designs[:id]))
 
-      joins(join.join_sources).where(actions[:event].not_eq(deletion)).order(:id)
+      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)
+        # 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
+        order(:id)
+      end
     end
 
     scope :with_filename, -> (filenames) { where(filename: filenames) }

spec/finders/design_management/designs_finder_spec.rb

@@ -8,9 +8,9 @@ RSpec.describe DesignManagement::DesignsFinder do
   let_it_be(:user) { create(:user) }
   let_it_be(:project) { create(:project, :private) }
   let_it_be(:issue) { create(:issue, project: project) }
-  let_it_be(:design1) { create(:design, :with_file, issue: issue, versions_count: 1) }
-  let_it_be(:design2) { create(:design, :with_file, issue: issue, versions_count: 1) }
-  let_it_be(:design3) { create(:design, :with_file, issue: issue, versions_count: 1) }
+  let_it_be(:design1) { create(:design, :with_file, issue: issue, versions_count: 1, relative_position: 3) }
+  let_it_be(:design2) { create(:design, :with_file, issue: issue, versions_count: 1, relative_position: 2) }
+  let_it_be(:design3) { create(:design, :with_file, issue: issue, versions_count: 1, relative_position: 1) }
   let(:params) { {} }
 
   subject(:designs) { described_class.new(issue, user, params).execute }
@@ -38,8 +38,28 @@ RSpec.describe DesignManagement::DesignsFinder do
           enable_design_management
         end
 
-        it 'returns the designs' do
-          is_expected.to contain_exactly(design1, design2, design3)
+        it 'returns the designs sorted by their relative position' 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

spec/models/design_management/design_spec.rb

@@ -152,6 +152,39 @@ RSpec.describe DesignManagement::Design do
       end
     end
 
+    describe '.ordered' do
+      before do
+        design1.update!(relative_position: 2)
+        design2.update!(relative_position: 1)
+        design3.update!(relative_position: nil)
+        deleted_design.update!(relative_position: nil)
+      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
+      end
+    end
+
     describe '.with_filename' do
       it 'returns correct design when passed a single filename' do
         expect(described_class.with_filename(design1.filename)).to eq([design1])