Fix positioning for new designs

- Don't set `relative_position` on backend, it's not needed since
  we're already sorting NULLs last, and also additionally by ID.

- Insert them at the end on the frontend too.

app/assets/javascripts/design_management/utils/cache_update.js

 /* eslint-disable @gitlab/require-i18n-strings */
 
+import { groupBy } from 'lodash';
 import createFlash from '~/flash';
 import { extractCurrentDiscussion, extractDesign } from './design_management_utils';
 import {
@@ -159,13 +160,11 @@ const updateImageDiffNoteInStore = (store, updateImageDiffNote, query, variables
 const addNewDesignToStore = (store, designManagementUpload, query) => {
   const data = store.readQuery(query);
 
-  const newDesigns = data.project.issue.designCollection.designs.nodes.reduce((acc, design) => {
-    if (!acc.find(d => d.filename === design.filename)) {
-      acc.push(design);
-    }
-
-    return acc;
-  }, designManagementUpload.designs);
+  const currentDesigns = data.project.issue.designCollection.designs.nodes;
+  const existingDesigns = groupBy(currentDesigns, 'filename');
+  const newDesigns = currentDesigns.concat(
+    designManagementUpload.designs.filter(d => !existingDesigns[d.filename]),
+  );
 
   let newVersionNode;
   const findNewVersions = designManagementUpload.designs.find(design => design.versions);

app/models/design_management/design_collection.rb

@@ -16,9 +16,7 @@ module DesignManagement
 
     def find_or_create_design!(filename:)
       designs.find { |design| design.filename == filename } ||
-        designs.safe_find_or_create_by!(project: project, filename: filename) do |design|
-          design.move_to_end
-        end
+        designs.safe_find_or_create_by!(project: project, filename: filename)
     end
 
     def versions

spec/features/projects/issues/design_management/user_uploads_designs_spec.rb

@@ -5,9 +5,9 @@ require 'spec_helper'
 RSpec.describe 'User uploads new design', :js do
   include DesignManagementTestHelpers
 
-  let_it_be(:project) { create(:project_empty_repo, :public) }
-  let_it_be(:user) { project.owner }
-  let_it_be(:issue) { create(:issue, project: project) }
+  let(:project) { create(:project_empty_repo, :public) }
+  let(:user) { project.owner }
+  let(:issue) { create(:issue, project: project) }
 
   before do
     sign_in(user)
@@ -28,7 +28,7 @@ RSpec.describe 'User uploads new design', :js do
       let(:feature_enabled) { true }
 
       it 'uploads designs' do
-        attach_file(:design_file, logo_fixture, make_visible: true)
+        upload_design(logo_fixture, count: 1)
 
         expect(page).to have_selector('.js-design-list-item', count: 1)
 
@@ -36,9 +36,12 @@ RSpec.describe 'User uploads new design', :js do
           expect(page).to have_content('dk.png')
         end
 
-        attach_file(:design_file, gif_fixture, make_visible: true)
+        upload_design(gif_fixture, count: 2)
 
+        # Known bug in the legacy implementation: new designs are inserted
+        # in the beginning on the frontend.
         expect(page).to have_selector('.js-design-list-item', count: 2)
+        expect(page.all('.js-design-list-item').map(&:text)).to eq(['banana_sample.gif', 'dk.png'])
       end
     end
 
@@ -61,8 +64,8 @@ RSpec.describe 'User uploads new design', :js do
     context "when the feature is available" do
       let(:feature_enabled) { true }
 
-      it 'uploads designs', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/225616' do
-        attach_file(:design_file, logo_fixture, make_visible: true)
+      it 'uploads designs' do
+        upload_design(logo_fixture, count: 1)
 
         expect(page).to have_selector('.js-design-list-item', count: 1)
 
@@ -70,9 +73,10 @@ RSpec.describe 'User uploads new design', :js do
           expect(page).to have_content('dk.png')
         end
 
-        attach_file(:design_file, gif_fixture, make_visible: true)
+        upload_design(gif_fixture, count: 2)
 
         expect(page).to have_selector('.js-design-list-item', count: 2)
+        expect(page.all('.js-design-list-item').map(&:text)).to eq(['dk.png', 'banana_sample.gif'])
       end
     end
 
@@ -92,4 +96,12 @@ RSpec.describe 'User uploads new design', :js do
   def gif_fixture
     Rails.root.join('spec', 'fixtures', 'banana_sample.gif')
   end
+
+  def upload_design(fixture, count:)
+    attach_file(:design_file, fixture, match: :first, make_visible: true)
+
+    wait_for('designs uploaded') do
+      issue.reload.designs.count == count
+    end
+  end
 end

spec/models/design_management/design_collection_spec.rb

@@ -37,9 +37,11 @@ RSpec.describe DesignManagement::DesignCollection do
 
     it 'inserts the design after any existing designs' do
       design1 = collection.find_or_create_design!(filename: 'design1.jpg')
+      design1.update!(relative_position: 100)
+
       design2 = collection.find_or_create_design!(filename: 'design2.jpg')
 
-      expect(design1.relative_position).to be < design2.relative_position
+      expect(collection.designs.ordered(issue.project)).to eq([design1, design2])
     end
   end