change the logic to only affect other group members

hpi-schul-cloud · Dec 12, 2024 · d3eaf4a · d3eaf4a
1 parent 7f36a8f
commit d3eaf4a
Show file tree

Hide file tree

Showing 4 changed files with 73 additions and 51 deletions.
diff --git a/.../server/src/modules/provisioning/strategy/schulconnex/schulconnex-response-mapper.spec.ts b/.../server/src/modules/provisioning/strategy/schulconnex/schulconnex-response-mapper.spec.ts
@@ -47,6 +47,11 @@ describe(SchulconnexResponseMapper.name, () => {
 		mapper = module.get(SchulconnexResponseMapper);
 	});
 
+	beforeEach(() => {
+		config.FEATURE_OTHER_GROUPUSERS_PROVISIONING_ENABLED = false;
+		config.PROVISIONING_SCHULCONNEX_GROUP_USERS_LIMIT = undefined;
+	});
+
 	describe('mapToExternalSchoolDto', () => {
 		describe('when a schulconnex response is provided', () => {
 			const setup = () => {
@@ -316,6 +321,8 @@ describe(SchulconnexResponseMapper.name, () => {
 
 		describe('when other participants have unknown roles', () => {
 			const setup = () => {
+				config.FEATURE_OTHER_GROUPUSERS_PROVISIONING_ENABLED = true;
+
 				const schulconnexResponse: SchulconnexResponse = schulconnexResponseFactory.build();
 				schulconnexResponse.personenkontexte[0].gruppen![0]!.sonstige_gruppenzugehoerige = [
 					{
@@ -514,6 +521,56 @@ describe(SchulconnexResponseMapper.name, () => {
 				);
 			});
 		});
+
+		describe('when there are too many users in groups', () => {
+			const setup = () => {
+				config.FEATURE_OTHER_GROUPUSERS_PROVISIONING_ENABLED = true;
+				config.PROVISIONING_SCHULCONNEX_GROUP_USERS_LIMIT = 1;
+
+				const schulconnexResponse: SchulconnexResponse = schulconnexResponseFactory.build();
+
+				return {
+					schulconnexResponse,
+				};
+			};
+
+			it('should not map other group users', () => {
+				const { schulconnexResponse } = setup();
+
+				const result: ExternalGroupDto[] | undefined = mapper.mapToExternalGroupDtos(schulconnexResponse);
+
+				expect(result).toEqual([
+					expect.objectContaining<Partial<ExternalGroupDto>>({
+						otherUsers: undefined,
+					}),
+				]);
+			});
+		});
+
+		describe('when there are not too many users in groups', () => {
+			const setup = () => {
+				config.FEATURE_OTHER_GROUPUSERS_PROVISIONING_ENABLED = true;
+				config.PROVISIONING_SCHULCONNEX_GROUP_USERS_LIMIT = 10;
+
+				const schulconnexResponse: SchulconnexResponse = schulconnexResponseFactory.build();
+
+				return {
+					schulconnexResponse,
+				};
+			};
+
+			it('should not map other group users', () => {
+				const { schulconnexResponse } = setup();
+
+				const result: ExternalGroupDto[] | undefined = mapper.mapToExternalGroupDtos(schulconnexResponse);
+
+				expect(result).not.toEqual([
+					expect.objectContaining({
+						otherUsers: undefined,
+					}),
+				]);
+			});
+		});
 	});
 
 	describe('mapLernperiode', () => {

diff --git a/apps/server/src/modules/provisioning/strategy/schulconnex/schulconnex-response-mapper.ts b/apps/server/src/modules/provisioning/strategy/schulconnex/schulconnex-response-mapper.ts
@@ -120,14 +120,25 @@ export class SchulconnexResponseMapper {
 			return undefined;
 		}
 
+		const usersInGroupsCount: number = groups.reduce(
+			(count: number, group: SchulconnexGruppenResponse) => count + (group.sonstige_gruppenzugehoerige?.length ?? 0),
+			groups.length
+		);
+		const limit: number | undefined = this.configService.get('PROVISIONING_SCHULCONNEX_GROUP_USERS_LIMIT');
+		const shouldProvisionOtherUsers: boolean = !limit || usersInGroupsCount < limit;
+
 		const mapped: ExternalGroupDto[] = groups
-			.map((group) => this.mapExternalGroup(source, group))
-			.filter((group): group is ExternalGroupDto => group !== null);
+			.map((group: SchulconnexGruppenResponse) => this.mapExternalGroup(source, group, shouldProvisionOtherUsers))
+			.filter((group: ExternalGroupDto | null): group is ExternalGroupDto => group !== null);
 
 		return mapped;
 	}
 
-	private mapExternalGroup(source: SchulconnexResponse, group: SchulconnexGruppenResponse): ExternalGroupDto | null {
+	private mapExternalGroup(
+		source: SchulconnexResponse,
+		group: SchulconnexGruppenResponse,
+		shouldProvisionOtherUsers: boolean
+	): ExternalGroupDto | null {
 		const groupType: GroupTypes | undefined = GroupTypeMapping[group.gruppe.typ];
 
 		if (!groupType) {
@@ -144,7 +155,7 @@ export class SchulconnexResponseMapper {
 		}
 
 		let otherUsers: ExternalGroupUserDto[] | undefined;
-		if (this.configService.get('FEATURE_OTHER_GROUPUSERS_PROVISIONING_ENABLED')) {
+		if (this.configService.get('FEATURE_OTHER_GROUPUSERS_PROVISIONING_ENABLED') && shouldProvisionOtherUsers) {
 			otherUsers = group.sonstige_gruppenzugehoerige
 				? group.sonstige_gruppenzugehoerige
 						.map((relation): ExternalGroupUserDto | null => this.mapToExternalGroupUser(relation))

diff --git a/apps/server/src/modules/provisioning/strategy/schulconnex/schulconnex.strategy.spec.ts b/apps/server/src/modules/provisioning/strategy/schulconnex/schulconnex.strategy.spec.ts
@@ -116,7 +116,6 @@ describe(SchulconnexProvisioningStrategy.name, () => {
 		config.FEATURE_SANIS_GROUP_PROVISIONING_ENABLED = false;
 		config.FEATURE_SCHULCONNEX_COURSE_SYNC_ENABLED = false;
 		config.FEATURE_OTHER_GROUPUSERS_PROVISIONING_ENABLED = true;
-		config.PROVISIONING_SCHULCONNEX_GROUP_USERS_LIMIT = undefined;
 	});
 
 	afterAll(async () => {
@@ -337,42 +336,6 @@ describe(SchulconnexProvisioningStrategy.name, () => {
 			});
 		});
 
-		describe('when there are too many users in groups', () => {
-			const setup = () => {
-				config.PROVISIONING_SCHULCONNEX_GROUP_USERS_LIMIT = 1;
-
-				const externalUserId = 'externalUserId';
-				const externalGroups: ExternalGroupDto[] = externalGroupDtoFactory.buildList(2);
-				const oauthData: OauthDataDto = new OauthDataDto({
-					system: new ProvisioningSystemDto({
-						systemId: new ObjectId().toHexString(),
-						provisioningStrategy: SystemProvisioningStrategy.OIDC,
-					}),
-					externalSchool: externalSchoolDtoFactory.build(),
-					externalUser: externalUserDtoFactory.build({ externalId: externalUserId }),
-					externalGroups,
-				});
-
-				const user: UserDO = userDoFactory.withRoles([{ id: 'roleId', name: RoleName.USER }]).build({
-					externalId: externalUserId,
-				});
-
-				schulconnexUserProvisioningService.provisionExternalUser.mockResolvedValueOnce(user);
-
-				return {
-					oauthData,
-				};
-			};
-
-			it('should not run group provisioning', async () => {
-				const { oauthData } = setup();
-
-				await strategy.apply(oauthData);
-
-				expect(schulconnexGroupProvisioningService.provisionExternalGroup).not.toHaveBeenCalled();
-			});
-		});
-
 		describe('when group data is not provided', () => {
 			const setup = () => {
 				config.FEATURE_SANIS_GROUP_PROVISIONING_ENABLED = true;

diff --git a/apps/server/src/modules/provisioning/strategy/schulconnex/schulconnex.strategy.ts b/apps/server/src/modules/provisioning/strategy/schulconnex/schulconnex.strategy.ts
@@ -45,16 +45,7 @@ export abstract class SchulconnexProvisioningStrategy extends ProvisioningStrate
 		);
 
 		if (this.configService.get('FEATURE_SANIS_GROUP_PROVISIONING_ENABLED')) {
-			const usersInGroupsCount: number =
-				data.externalGroups?.reduce(
-					(count: number, group: ExternalGroupDto) => count + (group.otherUsers?.length ?? 0),
-					data.externalGroups?.length ?? 0
-				) ?? 0;
-
-			const limit: number | undefined = this.configService.get('PROVISIONING_SCHULCONNEX_GROUP_USERS_LIMIT');
-			if (!limit || usersInGroupsCount < limit) {
-				await this.provisionGroups(data, school);
-			}
+			await this.provisionGroups(data, school);
 		}
 
 		if (this.configService.get('FEATURE_SCHULCONNEX_MEDIA_LICENSE_ENABLED') && user.id) {