From 066e047d3e798d02f0619c012228ad46d27a130c Mon Sep 17 00:00:00 2001
From: Alexander Ungefug <82446024+AlexanderUngefug@users.noreply.github.com>
Date: Mon, 16 Dec 2024 06:48:34 +0100
Subject: [PATCH] fixing coverage

---
 .../ldap/domain/ldap-client.service.spec.ts   | 24 ++++++++++++++++
 src/core/ldap/domain/ldap-client.service.ts   | 28 +++++++++++--------
 2 files changed, 40 insertions(+), 12 deletions(-)

diff --git a/src/core/ldap/domain/ldap-client.service.spec.ts b/src/core/ldap/domain/ldap-client.service.spec.ts
index 44e4efaa8..2da975229 100644
--- a/src/core/ldap/domain/ldap-client.service.spec.ts
+++ b/src/core/ldap/domain/ldap-client.service.spec.ts
@@ -901,6 +901,30 @@ describe('LDAP Client Service', () => {
                     expect(clientMock.modify).not.toHaveBeenCalled();
                     expect(clientMock.modifyDN).not.toHaveBeenCalled();
                 });
+
+                it('should return error if updateMemberDnInGroups fails', async () => {
+                    const oldReferrer: string = faker.internet.userName();
+                    const newUid: string = faker.string.alphanumeric(6);
+
+                    jest.spyOn(ldapClientService, 'updateMemberDnInGroups').mockResolvedValueOnce({
+                        ok: false,
+                        error: new Error('Failed to update groups'),
+                    });
+
+                    const result: Result<PersonID> = await ldapClientService.modifyPersonAttributes(
+                        oldReferrer,
+                        undefined,
+                        undefined,
+                        newUid,
+                    );
+
+                    expect(result.ok).toBeFalsy();
+                    if (result.ok) throw Error();
+                    expect(result.error?.message).toBe('Failed to update groups');
+                    expect(loggerMock.error).toHaveBeenCalledWith(
+                        `LDAP: Failed to update groups for person: ${oldReferrer}`,
+                    );
+                });
             });
         });
     });
diff --git a/src/core/ldap/domain/ldap-client.service.ts b/src/core/ldap/domain/ldap-client.service.ts
index 82d968cbc..fb848f696 100644
--- a/src/core/ldap/domain/ldap-client.service.ts
+++ b/src/core/ldap/domain/ldap-client.service.ts
@@ -55,6 +55,8 @@ export class LdapClientService {
 
     private static readonly GROUPS: string = 'groups';
 
+    private static readonly USERS_OU: string = 'users';
+
     private mutex: Mutex;
 
     public constructor(
@@ -298,7 +300,10 @@ export class LdapClientService {
                     newUid,
                     client,
                 );
-                if (!groupUpdateResult.ok) return groupUpdateResult;
+                if (!groupUpdateResult.ok) {
+                    this.logger.error(`LDAP: Failed to update groups for person: ${oldReferrer}`);
+                    return groupUpdateResult;
+                }
             }
 
             return { ok: true, value: oldReferrer };
@@ -310,8 +315,8 @@ export class LdapClientService {
         newReferrer: string,
         client: Client,
     ): Promise<Result<string>> {
-        const oldReferrerUid: string = this.getLehrerUid(oldReferrer, 'users');
-        const newReferrerUid: string = this.getLehrerUid(newReferrer, 'users');
+        const oldReferrerUid: string = this.getLehrerUid(oldReferrer, LdapClientService.USERS_OU);
+        const newReferrerUid: string = this.getLehrerUid(newReferrer, LdapClientService.USERS_OU);
 
         const searchResult: SearchResult = await client.search(`${this.ldapInstanceConfig.BASE_DN}`, {
             scope: 'sub',
@@ -562,7 +567,7 @@ export class LdapClientService {
             const newLehrerGroup: { cn: string; objectclass: string[]; member: string[] } = {
                 cn: groupId,
                 objectclass: ['groupOfNames'],
-                member: [this.getLehrerUid(personUid, 'users')],
+                member: [this.getLehrerUid(personUid, LdapClientService.USERS_OU)],
             };
             try {
                 await client.add(lehrerDn, newLehrerGroup);
@@ -575,7 +580,7 @@ export class LdapClientService {
             }
         }
 
-        if (this.isPersonInSearchResult(searchResultGroupOfNames, personUid)) {
+        if (this.isPersonInSearchResult(searchResultGroupOfNames.searchEntries[0], personUid)) {
             this.logger.info(`LDAP: Person ${personUid} is already in group ${groupId}`);
             return { ok: true, value: false };
         }
@@ -586,7 +591,7 @@ export class LdapClientService {
                     operation: 'add',
                     modification: new Attribute({
                         type: 'member',
-                        values: [this.getLehrerUid(personUid, 'users')],
+                        values: [this.getLehrerUid(personUid, LdapClientService.USERS_OU)],
                     }),
                 }),
             ]);
@@ -618,7 +623,7 @@ export class LdapClientService {
             return { ok: false, error: new Error(errMsg) };
         }
 
-        if (!this.isPersonInSearchResult(searchResultOrgUnit, personUid)) {
+        if (!this.isPersonInSearchResult(searchResultOrgUnit.searchEntries[0], personUid)) {
             this.logger.info(`LDAP: Person ${personUid} is not in group ${groupId}`);
             return { ok: false, error: new Error(`Person ${personUid} is not in group ${groupId}`) };
         }
@@ -635,7 +640,7 @@ export class LdapClientService {
                     operation: 'delete',
                     modification: new Attribute({
                         type: 'member',
-                        values: [this.getLehrerUid(personUid, 'users')],
+                        values: [this.getLehrerUid(personUid, LdapClientService.USERS_OU)],
                     }),
                 }),
             ]);
@@ -648,10 +653,9 @@ export class LdapClientService {
         }
     }
 
-    private isPersonInSearchResult(searchResult: SearchResult, personUid: string): boolean | undefined {
-        if (!searchResult.searchEntries[0]) return;
-        const member: string | string[] | Buffer | Buffer[] | undefined = searchResult.searchEntries[0]['member'];
-        const lehrerUid: string = this.getLehrerUid(personUid, 'users');
+    private isPersonInSearchResult(searchEntry: Entry, personUid: string): boolean | undefined {
+        const member: string | string[] | Buffer | Buffer[] | undefined = searchEntry['member'];
+        const lehrerUid: string = this.getLehrerUid(personUid, LdapClientService.USERS_OU);
 
         if (typeof member === 'string') {
             return member === lehrerUid;