Preserve order when applying scoped label titles

Order matters when applying scoped labels in a way it does not when
applying regular labels. When applying scoped labels by id this order
is maintained but when applying by title, such as via the API, it is
not.

This change stores the intended order by title when provided as an
array or comma-separated string of label titles and applies it to the
label order when updating an issuable.

app/services/labels/available_labels_service.rb

@@ -30,11 +30,13 @@ module Labels
     end
 
     def filter_labels_ids_in_param(key)
-      return [] if params[key].to_a.empty?
+      ids = params[key].to_a
+      return [] if ids.empty?
 
       # rubocop:disable CodeReuse/ActiveRecord
-      available_labels.by_ids(params[key]).pluck(:id)
+      existing_ids = available_labels.by_ids(ids).pluck(:id)
       # rubocop:enable CodeReuse/ActiveRecord
+      ids.map(&:to_i) & existing_ids
     end
 
     private

db/fixtures/development/10_merge_requests.rb

@@ -19,7 +19,7 @@ Gitlab::Seeder.quiet do
       target_branch = branches.pop
 
       label_ids = project.labels.pluck(:id).sample(3)
-      label_ids += project.group.labels.sample(3) if project.group
+      label_ids += project.group.labels.sample(3).pluck(:id) if project.group
 
       params = {
         source_branch: source_branch,

ee/app/services/ee/issuable_base_service.rb

@@ -5,10 +5,10 @@ module EE
     extend ::Gitlab::Utils::Override
     include ::Gitlab::Utils::StrongMemoize
 
-    attr_reader :label_ids_ordered_by_selection
-
     private
 
+    attr_reader :label_ids_ordered_by_selection
+
     override :filter_params
     def filter_params(issuable)
       can_admin_issuable = can_admin_issuable?(issuable)
@@ -26,9 +26,9 @@ module EE
 
     override :filter_labels
     def filter_labels
-      @label_ids_ordered_by_selection = params[:add_label_ids].to_a + params[:label_ids].to_a # rubocop:disable Gitlab/ModuleWithInstanceVariables
-
       super
+
+      @label_ids_ordered_by_selection = params[:add_label_ids].to_a + params[:label_ids].to_a # rubocop:disable Gitlab/ModuleWithInstanceVariables
     end
 
     def update_task_event?

ee/changelogs/unreleased/fix-scoped-label-application-order.yml

+---
+title: Preserve order when applying scoped labels by title
+merge_request: 33420
+author:
+type: fixed

ee/spec/support/shared_examples/services/scoped_label_shared_examples.rb

@@ -16,26 +16,38 @@ end
 
 RSpec.shared_examples 'new issuable with scoped labels' do
   include_context 'exclusive labels creation' do
-    context 'scoped labels are avaialble' do
+    context 'when scoped labels are available' do
       before do
         stub_licensed_features(scoped_labels: true)
       end
 
-      it 'adds only last selected exclusive scoped label' do
-        label1 = create_label('label1')
-        label2 = create_label('key::label1')
-        label3 = create_label('key::label2')
-        label4 = create_label('key::label3')
+      let!(:label1) { create_label('label1') }
+      let!(:label2) { create_label('key::label1') }
+      let!(:label3) { create_label('key::label2') }
+      let!(:label4) { create_label('key::label3') }
 
-        issuable = described_class.new(
-          parent, user, title: 'test', label_ids: [label1.id, label3.id, label4.id, label2.id]
-        ).execute
+      context 'when using label_ids parameter' do
+        it 'adds only last selected exclusive scoped label' do
+          issuable = described_class.new(
+            parent, user, title: 'test', label_ids: [label1.id, label3.id, label4.id, label2.id]
+          ).execute
 
-        expect(issuable.labels).to match_array([label1, label2])
+          expect(issuable.labels).to match_array([label1, label2])
+        end
+      end
+
+      context 'when using labels parameter' do
+        it 'adds only last selected exclusive scoped label' do
+          issuable = described_class.new(
+            parent, user, title: 'test', labels: [label1.title, label3.title, label4.title, label2.title]
+          ).execute
+
+          expect(issuable.labels).to match_array([label1, label2])
+        end
       end
     end
 
-    context 'scoped labels are not available' do
+    context 'when scoped labels are not available' do
       before do
         stub_licensed_features(scoped_labels: false)
       end
@@ -62,40 +74,59 @@ RSpec.shared_examples 'existing issuable with scoped labels' do
     let(:label2) { create_label('key::label2') }
     let(:label3) { create_label('key::label3') }
 
-    context 'scoped labels are avaialble' do
+    context 'when scoped labels are available' do
       before do
         stub_licensed_features(scoped_labels: true, epics: true)
       end
 
-      it 'adds only last selected exclusive scoped label' do
-        create(:label_link, label: label1, target: issuable)
-        create(:label_link, label: label2, target: issuable)
+      context 'when using label_ids parameter' do
+        it 'adds only last selected exclusive scoped label' do
+          create(:label_link, label: label1, target: issuable)
+          create(:label_link, label: label2, target: issuable)
 
-        issuable.reload
+          issuable.reload
 
-        described_class.new(
-          parent, user, label_ids: [label1.id, label3.id]
-        ).execute(issuable)
+          described_class.new(
+            parent, user, label_ids: [label1.id, label3.id]
+          ).execute(issuable)
 
-        expect(issuable.reload.labels).to match_array([label3])
+          expect(issuable.reload.labels).to match_array([label3])
+        end
       end
 
-      it 'preserves multiple exclusive scoped labels when only removing labels' do
-        create(:label_link, label: label1, target: issuable)
-        create(:label_link, label: label2, target: issuable)
-        create(:label_link, label: label3, target: issuable)
+      context 'when using label_ids parameter' do
+        it 'adds only last selected exclusive scoped label' do
+          create(:label_link, label: label1, target: issuable)
+          create(:label_link, label: label2, target: issuable)
 
-        issuable.reload
+          issuable.reload
 
-        described_class.new(
-          parent, user, label_ids: [label2.id, label3.id]
-        ).execute(issuable)
+          described_class.new(
+            parent, user, labels: [label1.title, label3.title]
+          ).execute(issuable)
+
+          expect(issuable.reload.labels).to match_array([label3])
+        end
+      end
+
+      context 'when only removing labels' do
+        it 'preserves multiple exclusive scoped labels' do
+          create(:label_link, label: label1, target: issuable)
+          create(:label_link, label: label2, target: issuable)
+          create(:label_link, label: label3, target: issuable)
+
+          issuable.reload
+
+          described_class.new(
+            parent, user, label_ids: [label2.id, label3.id]
+          ).execute(issuable)
 
-        expect(issuable.reload.labels).to match_array([label2, label3])
+          expect(issuable.reload.labels).to match_array([label2, label3])
+        end
       end
     end
 
-    context 'scoped labels are not available' do
+    context 'when scoped labels are not available' do
       before do
         stub_licensed_features(scoped_labels: false, epics: true)
       end

spec/services/labels/available_labels_service_spec.rb

@@ -73,6 +73,12 @@ describe Labels::AvailableLabelsService do
 
         expect(result).to match_array([project_label.id, group_label.id])
       end
+
+      it 'returns labels in preserved order' do
+        result = described_class.new(user, project, ids: label_ids.reverse).filter_labels_ids_in_param(:ids)
+
+        expect(result).to eq([group_label.id, project_label.id])
+      end
     end
 
     context 'when parent is a group' do