From 19717954d6cbba78dee6c5e0fcbce728c33929b8 Mon Sep 17 00:00:00 2001
From: Schalli <kschaller@ctl.de>
Date: Thu, 6 Aug 2026 16:57:34 +0200
Subject: [PATCH] fix(16): WR-02 discriminate P2002 target in group rename
 branch

syncBoundGroupsForTenant()'s rename write updates name and ldapDn in one
call, so a P2002 there can come from either @@unique([tenantId, name])
or @@unique([tenantId, ldapDn]). The catch previously reported every
P2002 as a name collision unconditionally; it now inspects
err.meta.target the same way importGroupsByDn() already does for its
own create() call, so a non-name unique violation is no longer
mislabelled and sent the admin down the wrong troubleshooting path.
---
 apps/api/src/ldap/ldap.service.spec.ts | 42 +++++++++++++++++++++++++-
 apps/api/src/ldap/ldap.service.ts      | 15 ++++++++-
 2 files changed, 55 insertions(+), 2 deletions(-)

diff --git a/apps/api/src/ldap/ldap.service.spec.ts b/apps/api/src/ldap/ldap.service.spec.ts
index 98aad17..f87628e 100644
--- a/apps/api/src/ldap/ldap.service.spec.ts
+++ b/apps/api/src/ldap/ldap.service.spec.ts
@@ -1260,7 +1260,7 @@ describe('LdapService.syncBoundGroupsForTenant — Rekonziliation gegen das Verz
     expect(groups[0].ldapDn).toBe('cn=Vertrieb,dc=example,dc=com');
   });
 
-  it('a rename colliding with an existing local name (P2002) is reported and the group is left unchanged, run continues', async () => {
+  it('a rename colliding with an existing local name (P2002 on the name unique index) is reported and the group is left unchanged, run continues', async () => {
     groups = [
       {
         id: 'g1',
@@ -1283,6 +1283,10 @@ describe('LdapService.syncBoundGroupsForTenant — Rekonziliation gegen das Verz
       if (args.where.id === 'g1') {
         const err: any = new Error('Unique constraint');
         err.code = 'P2002';
+        // WR-02 (16-REVIEW.md): a realistic @@unique([tenantId, name])
+        // violation carries this target — the fix must inspect it rather
+        // than assuming every P2002 on this update() is a name collision.
+        err.meta = { target: ['tenantId', 'name'] };
         return Promise.reject(err);
       }
       const g = groups.find((x) => x.id === args.where.id);
@@ -1322,6 +1326,42 @@ describe('LdapService.syncBoundGroupsForTenant — Rekonziliation gegen das Verz
     expect(prisma.group.update).toHaveBeenCalledTimes(2);
   });
 
+  it('a rename P2002 whose meta.target does NOT include name (e.g. the ldapDn unique index) is reported as a binding collision, not mislabelled as a name collision (WR-02, 16-REVIEW.md)', async () => {
+    groups = [
+      {
+        id: 'g1',
+        tenantId: 't1',
+        name: 'Sales',
+        ldapDn: 'cn=Sales,dc=example,dc=com',
+        ldapObjectGuid: guidHex,
+        isDefault: false,
+      },
+    ];
+    prisma.group.update = vi.fn((args: any) => {
+      const err: any = new Error('Unique constraint');
+      err.code = 'P2002';
+      // A P2002 on the OTHER unique index this update() can hit —
+      // @@unique([tenantId, ldapDn]) — must not be reported as "kollidiert
+      // mit einer bestehenden Gruppe" (that message is specifically for a
+      // name collision).
+      err.meta = { target: ['tenantId', 'ldapDn'] };
+      return Promise.reject(err);
+    });
+    mockSearch.mockResolvedValue({
+      searchEntries: [{ dn: 'cn=Vertrieb,dc=example,dc=com', cn: 'Vertrieb' }],
+    });
+
+    const result = makeResult();
+    await run(result);
+
+    expect(result.errors).toEqual([
+      'Gruppe Sales: Aktualisierung kollidiert mit einer bestehenden Bindung (["tenantId","ldapDn"])',
+    ]);
+    expect(result.errors[0]).not.toContain('Umbenennung nach');
+    expect(result.groupsRenamed).toBe(0);
+    expect(groups[0].name).toBe('Sales');
+  });
+
   it('no hit, not the default group, deletes it and increments groupsDeleted without moving the marker', async () => {
     groups = [
       {
diff --git a/apps/api/src/ldap/ldap.service.ts b/apps/api/src/ldap/ldap.service.ts
index 3e08a0e..df3d72d 100644
--- a/apps/api/src/ldap/ldap.service.ts
+++ b/apps/api/src/ldap/ldap.service.ts
@@ -1317,8 +1317,21 @@ export class LdapService {
               result.groupsRenamed++;
             } catch (updateError: any) {
               if (updateError?.code === 'P2002') {
+                // WR-02 (16-REVIEW.md): this update() writes BOTH name and
+                // ldapDn in one call — @@unique([tenantId, name]) AND
+                // @@unique([tenantId, ldapDn]) are both potential triggers
+                // of a P2002 here, so the collision is only actually a name
+                // collision when updateError.meta.target says so. Mirrors
+                // the discrimination importGroupsByDn() already does above
+                // for its own create() call.
+                const target = updateError?.meta?.target;
+                const targetsName = Array.isArray(target)
+                  ? target.includes('name')
+                  : String(target ?? '').includes('name');
                 result.errors.push(
-                  `Gruppe ${group.name}: Umbenennung nach '${name}' kollidiert mit einer bestehenden Gruppe`,
+                  targetsName
+                    ? `Gruppe ${group.name}: Umbenennung nach '${name}' kollidiert mit einer bestehenden Gruppe`
+                    : `Gruppe ${group.name}: Aktualisierung kollidiert mit einer bestehenden Bindung (${JSON.stringify(target)})`,
                 );
               } else {
                 throw updateError;