Added new unique index

- On member_id, member_namespace_id, new_access_level to ensure
  only one record per member per namespace for the accesslevel exists
  in pending state

app/models/members/members/member_approval.rb

@@ -13,5 +13,15 @@ class MemberApproval < ApplicationRecord
 
     validates :new_access_level, presence: true
     validates :old_access_level, presence: true
+    validate :validate_unique_pending_approval, on: [:create, :update]
+
+    private
+
+    def validate_unique_pending_approval
+      if pending? && self.class.where(member_id: member_id, member_namespace_id: member_namespace_id,
+        new_access_level: new_access_level, status: 0).exists?
+        errors.add(:base, 'A pending approval for the same member, namespace, and access level already exists.')
+      end
+    end
   end
 end

db/migrate/20240207193743_add_conditional_unique_index_to_member_approvals.rb

+# frozen_string_literal: true
+
+class AddConditionalUniqueIndexToMemberApprovals < Gitlab::Database::Migration[2.2]
+  milestone '16.10'
+
+  disable_ddl_transaction!
+
+  INDEX_NAME = 'unique_member_approvals_on_pending_status'
+
+  def up
+    add_concurrent_index :member_approvals, [:member_id, :member_namespace_id, :new_access_level],
+      unique: true, where: "status = 0", name: INDEX_NAME
+  end
+
+  def down
+    remove_concurrent_index_by_name :member_approvals, INDEX_NAME
+  end
+end

db/schema_migrations/20240207193743

+f8ae8294d45a5d796a86d629bef9b25142895b497db778c694d8573338a274d2
\ No newline at end of file

db/structure.sql

@@ -36549,6 +36549,8 @@ CREATE UNIQUE INDEX unique_instance_google_cloud_logging_configurations ON audit
 CREATE UNIQUE INDEX unique_instance_google_cloud_logging_configurations_name ON audit_events_instance_google_cloud_logging_configurations USING btree (name);
+CREATE UNIQUE INDEX unique_member_approvals_on_pending_status ON member_approvals USING btree (member_id, member_namespace_id, new_access_level) WHERE (status = 0);
+
 CREATE UNIQUE INDEX unique_merge_request_diff_llm_summaries_on_mr_diff_id ON merge_request_diff_llm_summaries USING btree (merge_request_diff_id);
 CREATE UNIQUE INDEX unique_merge_request_metrics_by_merge_request_id ON merge_request_metrics USING btree (merge_request_id);

ee/spec/models/factories_spec.rb

@@ -170,6 +170,7 @@
     wiki_page_meta
     workspace
     workspace_variable
+    member_approval
   ].to_set.freeze
 
   # Some factories and their corresponding models are based on

spec/factories/member_approvals.rb

+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :member_approval, class: 'Members::MemberApproval' do
+    requested_by { association(:user) }
+    reviewed_by { association(:user) }
+    old_access_level { ::Gitlab::Access::GUEST }
+    new_access_level { ::Gitlab::Access::DEVELOPER }
+    status { ::Members::MemberApproval.statuses[:pending] }
+    member { association(:project_member) } # default
+    member_namespace_id { member.member_namespace_id }
+
+    # Traits for specific group members
+    trait :for_group_member do
+      member { association(:group_member) }
+    end
+
+    trait :for_project_member do
+      member { association(:project_member) }
+    end
+
+    trait(:guest)     { old_access_level { GroupMember::GUEST } }
+    trait(:reporter)  { old_access_level { GroupMember::REPORTER } }
+    trait(:developer) { old_access_level { GroupMember::DEVELOPER } }
+    trait(:maintainer) { old_access_level { GroupMember::MAINTAINER } }
+    trait(:owner) { old_access_level { GroupMember::OWNER } }
+
+    trait(:to_guest)     { new_access_level { GroupMember::GUEST } }
+    trait(:to_reporter)  { new_access_level { GroupMember::REPORTER } }
+    trait(:to_developer) { new_access_level { GroupMember::DEVELOPER } }
+    trait(:to_maintainer) { new_access_level { GroupMember::MAINTAINER } }
+    trait(:to_owner) { new_access_level { GroupMember::OWNER } }
+  end
+end

spec/models/members/members/member_approval_spec.rb

@@ -13,5 +13,46 @@
   describe 'validations' do
     it { is_expected.to validate_presence_of(:new_access_level) }
     it { is_expected.to validate_presence_of(:old_access_level) }
+
+    context 'when uniqness is enforced' do
+      let!(:member) { create(:project_member) }
+      let!(:member_approval) { create(:member_approval, member: member) }
+
+      context 'with same member, namespace, and access level and pending status' do
+        let(:message) { 'A pending approval for the same member, namespace, and access level already exists.' }
+
+        it 'disallows on create' do
+          duplicate_approval = build(:member_approval, member: member)
+
+          expect(duplicate_approval).not_to be_valid
+          expect(duplicate_approval.errors[:base]).to include(message)
+        end
+
+        it 'disallows on update' do
+          duplicate_approval = create(:member_approval, member: member, status: :approved)
+          expect(duplicate_approval).to be_valid
+
+          duplicate_approval.status = ::Members::MemberApproval.statuses[:pending]
+          expect(duplicate_approval).not_to be_valid
+          expect(duplicate_approval.errors[:base]).to include(message)
+        end
+      end
+
+      it 'allows duplicate member approvals with different statuses' do
+        member_approval.update!(status: ::Members::MemberApproval.statuses[:approved])
+
+        pending_approval = build(:member_approval, member: member)
+
+        expect(pending_approval).to be_valid
+      end
+
+      it 'allows duplicate member approvals with different access levels' do
+        different_approval = build(:member_approval,
+          member: member,
+          new_access_level: ::Gitlab::Access::MAINTAINER)
+
+        expect(different_approval).to be_valid
+      end
+    end
   end
 end