From 145cc17e49765cbf1b4f4f92df3350a9b3d2f224 Mon Sep 17 00:00:00 2001 From: Brandon Date: Mon, 26 Jan 2026 15:01:04 -0500 Subject: [PATCH] clean up flagged logic --- .../common/people-table-data-source.ts | 25 +- .../members/deprecated_members.component.ts | 4 +- .../members/members.component.ts | 10 +- .../member-actions.service.spec.ts | 471 ++++++++---------- .../member-actions/member-actions.service.ts | 25 +- .../manage/deprecated_members.component.ts | 4 +- .../providers/manage/members.component.ts | 10 +- libs/common/src/enums/feature-flag.enum.ts | 2 - 8 files changed, 223 insertions(+), 328 deletions(-) diff --git a/apps/web/src/app/admin-console/common/people-table-data-source.ts b/apps/web/src/app/admin-console/common/people-table-data-source.ts index d39a4f29653..a3ffbaeb7b5 100644 --- a/apps/web/src/app/admin-console/common/people-table-data-source.ts +++ b/apps/web/src/app/admin-console/common/people-table-data-source.ts @@ -1,6 +1,6 @@ // FIXME: Update this file to be type safe and remove this and next line // @ts-strict-ignore -import { computed, Signal } from "@angular/core"; +import { Signal } from "@angular/core"; import { toSignal } from "@angular/core/rxjs-interop"; import { Observable, Subject, map } from "rxjs"; @@ -9,8 +9,6 @@ import { ProviderUserStatusType, } from "@bitwarden/common/admin-console/enums"; import { ProviderUserUserDetailsResponse } from "@bitwarden/common/admin-console/models/response/provider/provider-user.response"; -import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; -import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; import { TableDataSource } from "@bitwarden/components"; @@ -27,8 +25,7 @@ export type ProviderUser = ProviderUserUserDetailsResponse; export const MaxCheckedCount = 500; /** - * Maximum for bulk reinvite operations when the IncreaseBulkReinviteLimitForCloud - * feature flag is enabled on cloud environments. + * Maximum for bulk reinvite limit in cloud environments. */ export const CloudBulkReinviteLimit = 8000; @@ -78,18 +75,15 @@ export abstract class PeopleTableDataSource extends Tab confirmedUserCount: number; revokedUserCount: number; - /** True when increased bulk limit feature is enabled (feature flag + cloud environment) */ + /** True when increased bulk limit feature is enabled (cloud environment) */ readonly isIncreasedBulkLimitEnabled: Signal; - constructor(configService: ConfigService, environmentService: EnvironmentService) { + constructor(environmentService: EnvironmentService) { super(); - const featureFlagEnabled = toSignal( - configService.getFeatureFlag$(FeatureFlag.IncreaseBulkReinviteLimitForCloud), + this.isIncreasedBulkLimitEnabled = toSignal( + environmentService.environment$.pipe(map((env) => env.isCloud())), ); - const isCloud = toSignal(environmentService.environment$.pipe(map((env) => env.isCloud()))); - - this.isIncreasedBulkLimitEnabled = computed(() => featureFlagEnabled() && isCloud()); } override set data(data: T[]) { @@ -224,12 +218,9 @@ export abstract class PeopleTableDataSource extends Tab } /** - * Gets checked users with optional limiting based on the IncreaseBulkReinviteLimitForCloud feature flag. + * Returns checked users in visible order, optionally limited to the specified count. * - * When the feature flag is enabled: Returns checked users in visible order, limited to the specified count. - * When the feature flag is disabled: Returns all checked users without applying any limit. - * - * @param limit The maximum number of users to return (only applied when feature flag is enabled) + * @param limit The maximum number of users to return * @returns The checked users array */ getCheckedUsersWithLimit(limit: number): T[] { diff --git a/apps/web/src/app/admin-console/organizations/members/deprecated_members.component.ts b/apps/web/src/app/admin-console/organizations/members/deprecated_members.component.ts index 99fd81aa48d..93960820fbb 100644 --- a/apps/web/src/app/admin-console/organizations/members/deprecated_members.component.ts +++ b/apps/web/src/app/admin-console/organizations/members/deprecated_members.component.ts @@ -33,7 +33,6 @@ import { AccountService } from "@bitwarden/common/auth/abstractions/account.serv import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { OrganizationMetadataServiceAbstraction } from "@bitwarden/common/billing/abstractions/organization-metadata.service.abstraction"; import { OrganizationBillingMetadataResponse } from "@bitwarden/common/billing/models/response/organization-billing-metadata.response"; -import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; @@ -124,7 +123,6 @@ export class MembersComponent extends BaseMembersComponent private policyApiService: PolicyApiServiceAbstraction, private organizationMetadataService: OrganizationMetadataServiceAbstraction, private memberExportService: MemberExportService, - private configService: ConfigService, private environmentService: EnvironmentService, ) { super( @@ -139,7 +137,7 @@ export class MembersComponent extends BaseMembersComponent toastService, ); - this.dataSource = new MembersTableDataSource(this.configService, this.environmentService); + this.dataSource = new MembersTableDataSource(this.environmentService); const organization$ = this.route.params.pipe( concatMap((params) => diff --git a/apps/web/src/app/admin-console/organizations/members/members.component.ts b/apps/web/src/app/admin-console/organizations/members/members.component.ts index 9d367657d55..141d28a0b0e 100644 --- a/apps/web/src/app/admin-console/organizations/members/members.component.ts +++ b/apps/web/src/app/admin-console/organizations/members/members.component.ts @@ -33,7 +33,6 @@ import { AccountService } from "@bitwarden/common/auth/abstractions/account.serv import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { OrganizationMetadataServiceAbstraction } from "@bitwarden/common/billing/abstractions/organization-metadata.service.abstraction"; import { OrganizationBillingMetadataResponse } from "@bitwarden/common/billing/models/response/organization-billing-metadata.response"; -import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; @@ -100,7 +99,6 @@ export class vNextMembersComponent { private policyService = inject(PolicyService); private policyApiService = inject(PolicyApiServiceAbstraction); private organizationMetadataService = inject(OrganizationMetadataServiceAbstraction); - private configService = inject(ConfigService); private environmentService = inject(EnvironmentService); private memberExportService = inject(MemberExportService); @@ -114,7 +112,7 @@ export class vNextMembersComponent { protected statusToggle = new BehaviorSubject(undefined); protected readonly dataSource: Signal = signal( - new MembersTableDataSource(this.configService, this.environmentService), + new MembersTableDataSource(this.environmentService), ); protected readonly organization: Signal; protected readonly firstLoaded: WritableSignal = signal(false); @@ -389,7 +387,7 @@ export class vNextMembersComponent { // Capture the original count BEFORE enforcing the limit const originalInvitedCount = allInvitedUsers.length; - // When feature flag is enabled, limit invited users and uncheck the excess + // In cloud environments, limit invited users and uncheck the excess let filteredUsers: OrganizationUserView[]; if (this.dataSource().isIncreasedBulkLimitEnabled()) { filteredUsers = this.dataSource().limitAndUncheckExcess( @@ -418,7 +416,7 @@ export class vNextMembersComponent { this.validationService.showError(result.failed); } - // When feature flag is enabled, show toast instead of dialog + // In cloud environments, show toast instead of dialog if (this.dataSource().isIncreasedBulkLimitEnabled()) { const selectedCount = originalInvitedCount; const invitedCount = filteredUsers.length; @@ -441,7 +439,7 @@ export class vNextMembersComponent { }); } } else { - // Feature flag disabled - show legacy dialog + // In self-hosted environments, show legacy dialog await this.memberDialogManager.openBulkStatusDialog( users, filteredUsers, diff --git a/apps/web/src/app/admin-console/organizations/members/services/member-actions/member-actions.service.spec.ts b/apps/web/src/app/admin-console/organizations/members/services/member-actions/member-actions.service.spec.ts index 1df285d7ba2..423977e73c4 100644 --- a/apps/web/src/app/admin-console/organizations/members/services/member-actions/member-actions.service.spec.ts +++ b/apps/web/src/app/admin-console/organizations/members/services/member-actions/member-actions.service.spec.ts @@ -17,7 +17,6 @@ import { import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; import { OrganizationMetadataServiceAbstraction } from "@bitwarden/common/billing/abstractions/organization-metadata.service.abstraction"; import { ListResponse } from "@bitwarden/common/models/response/list.response"; -import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { OrganizationId, UserId } from "@bitwarden/common/types/guid"; import { DialogService } from "@bitwarden/components"; @@ -32,7 +31,6 @@ describe("MemberActionsService", () => { let service: MemberActionsService; let organizationUserApiService: MockProxy; let organizationUserService: MockProxy; - let configService: MockProxy; let organizationMetadataService: MockProxy; const organizationId = newGuid() as OrganizationId; @@ -44,7 +42,6 @@ describe("MemberActionsService", () => { beforeEach(() => { organizationUserApiService = mock(); organizationUserService = mock(); - configService = mock(); organizationMetadataService = mock(); mockOrganization = { @@ -68,7 +65,6 @@ describe("MemberActionsService", () => { MemberActionsService, { provide: OrganizationUserApiService, useValue: organizationUserApiService }, { provide: OrganizationUserService, useValue: organizationUserService }, - { provide: ConfigService, useValue: configService }, { provide: OrganizationMetadataServiceAbstraction, useValue: organizationMetadataService, @@ -279,308 +275,247 @@ describe("MemberActionsService", () => { }); describe("bulkReinvite", () => { - const userIds = [newGuid() as UserId, newGuid() as UserId, newGuid() as UserId]; + it("should process users in a single batch when count equals REQUESTS_PER_BATCH", async () => { + const userIdsBatch = Array.from({ length: REQUESTS_PER_BATCH }, () => newGuid() as UserId); + const mockResponse = new ListResponse( + { + data: userIdsBatch.map((id) => ({ + id, + error: null, + })), + continuationToken: null, + }, + OrganizationUserBulkResponse, + ); - describe("when feature flag is false", () => { - beforeEach(() => { - configService.getFeatureFlag$.mockReturnValue(of(false)); - }); + organizationUserApiService.postManyOrganizationUserReinvite.mockResolvedValue(mockResponse); - it("should successfully reinvite multiple users", async () => { - const mockResponse = new ListResponse( - { - data: userIds.map((id) => ({ - id, - error: null, - })), - continuationToken: null, - }, - OrganizationUserBulkResponse, - ); + const result = await service.bulkReinvite(mockOrganization, userIdsBatch); - organizationUserApiService.postManyOrganizationUserReinvite.mockResolvedValue(mockResponse); - - const result = await service.bulkReinvite(mockOrganization, userIds); - - expect(result.failed).toEqual([]); - expect(result.successful).toBeDefined(); - expect(result.successful).toEqual(mockResponse); - expect(organizationUserApiService.postManyOrganizationUserReinvite).toHaveBeenCalledWith( - organizationId, - userIds, - ); - }); - - it("should handle bulk reinvite errors", async () => { - const errorMessage = "Bulk reinvite failed"; - organizationUserApiService.postManyOrganizationUserReinvite.mockRejectedValue( - new Error(errorMessage), - ); - - const result = await service.bulkReinvite(mockOrganization, userIds); - - expect(result.successful).toBeUndefined(); - expect(result.failed).toHaveLength(3); - expect(result.failed[0]).toEqual({ id: userIds[0], error: errorMessage }); - }); + expect(result.successful).toBeDefined(); + expect(result.successful?.response).toHaveLength(REQUESTS_PER_BATCH); + expect(result.failed).toHaveLength(0); + expect(organizationUserApiService.postManyOrganizationUserReinvite).toHaveBeenCalledTimes(1); + expect(organizationUserApiService.postManyOrganizationUserReinvite).toHaveBeenCalledWith( + organizationId, + userIdsBatch, + ); }); - describe("when feature flag is true (batching behavior)", () => { - beforeEach(() => { - configService.getFeatureFlag$.mockReturnValue(of(true)); - }); - it("should process users in a single batch when count equals REQUESTS_PER_BATCH", async () => { - const userIdsBatch = Array.from({ length: REQUESTS_PER_BATCH }, () => newGuid() as UserId); - const mockResponse = new ListResponse( - { - data: userIdsBatch.map((id) => ({ - id, - error: null, - })), - continuationToken: null, - }, - OrganizationUserBulkResponse, - ); + it("should process users in multiple batches when count exceeds REQUESTS_PER_BATCH", async () => { + const totalUsers = REQUESTS_PER_BATCH + 100; + const userIdsBatch = Array.from({ length: totalUsers }, () => newGuid() as UserId); - organizationUserApiService.postManyOrganizationUserReinvite.mockResolvedValue(mockResponse); + const mockResponse1 = new ListResponse( + { + data: userIdsBatch.slice(0, REQUESTS_PER_BATCH).map((id) => ({ + id, + error: null, + })), + continuationToken: null, + }, + OrganizationUserBulkResponse, + ); - const result = await service.bulkReinvite(mockOrganization, userIdsBatch); + const mockResponse2 = new ListResponse( + { + data: userIdsBatch.slice(REQUESTS_PER_BATCH).map((id) => ({ + id, + error: null, + })), + continuationToken: null, + }, + OrganizationUserBulkResponse, + ); - expect(result.successful).toBeDefined(); - expect(result.successful?.response).toHaveLength(REQUESTS_PER_BATCH); - expect(result.failed).toHaveLength(0); - expect(organizationUserApiService.postManyOrganizationUserReinvite).toHaveBeenCalledTimes( - 1, - ); - expect(organizationUserApiService.postManyOrganizationUserReinvite).toHaveBeenCalledWith( - organizationId, - userIdsBatch, - ); - }); + organizationUserApiService.postManyOrganizationUserReinvite + .mockResolvedValueOnce(mockResponse1) + .mockResolvedValueOnce(mockResponse2); - it("should process users in multiple batches when count exceeds REQUESTS_PER_BATCH", async () => { - const totalUsers = REQUESTS_PER_BATCH + 100; - const userIdsBatch = Array.from({ length: totalUsers }, () => newGuid() as UserId); + const result = await service.bulkReinvite(mockOrganization, userIdsBatch); - const mockResponse1 = new ListResponse( - { - data: userIdsBatch.slice(0, REQUESTS_PER_BATCH).map((id) => ({ - id, - error: null, - })), - continuationToken: null, - }, - OrganizationUserBulkResponse, - ); + expect(result.successful).toBeDefined(); + expect(result.successful?.response).toHaveLength(totalUsers); + expect(result.failed).toHaveLength(0); + expect(organizationUserApiService.postManyOrganizationUserReinvite).toHaveBeenCalledTimes(2); + expect(organizationUserApiService.postManyOrganizationUserReinvite).toHaveBeenNthCalledWith( + 1, + organizationId, + userIdsBatch.slice(0, REQUESTS_PER_BATCH), + ); + expect(organizationUserApiService.postManyOrganizationUserReinvite).toHaveBeenNthCalledWith( + 2, + organizationId, + userIdsBatch.slice(REQUESTS_PER_BATCH), + ); + }); - const mockResponse2 = new ListResponse( - { - data: userIdsBatch.slice(REQUESTS_PER_BATCH).map((id) => ({ - id, - error: null, - })), - continuationToken: null, - }, - OrganizationUserBulkResponse, - ); + it("should aggregate results across multiple successful batches", async () => { + const totalUsers = REQUESTS_PER_BATCH + 50; + const userIdsBatch = Array.from({ length: totalUsers }, () => newGuid() as UserId); - organizationUserApiService.postManyOrganizationUserReinvite - .mockResolvedValueOnce(mockResponse1) - .mockResolvedValueOnce(mockResponse2); + const mockResponse1 = new ListResponse( + { + data: userIdsBatch.slice(0, REQUESTS_PER_BATCH).map((id) => ({ + id, + error: null, + })), + continuationToken: null, + }, + OrganizationUserBulkResponse, + ); - const result = await service.bulkReinvite(mockOrganization, userIdsBatch); + const mockResponse2 = new ListResponse( + { + data: userIdsBatch.slice(REQUESTS_PER_BATCH).map((id) => ({ + id, + error: null, + })), + continuationToken: null, + }, + OrganizationUserBulkResponse, + ); - expect(result.successful).toBeDefined(); - expect(result.successful?.response).toHaveLength(totalUsers); - expect(result.failed).toHaveLength(0); - expect(organizationUserApiService.postManyOrganizationUserReinvite).toHaveBeenCalledTimes( - 2, - ); - expect(organizationUserApiService.postManyOrganizationUserReinvite).toHaveBeenNthCalledWith( - 1, - organizationId, - userIdsBatch.slice(0, REQUESTS_PER_BATCH), - ); - expect(organizationUserApiService.postManyOrganizationUserReinvite).toHaveBeenNthCalledWith( - 2, - organizationId, - userIdsBatch.slice(REQUESTS_PER_BATCH), - ); - }); + organizationUserApiService.postManyOrganizationUserReinvite + .mockResolvedValueOnce(mockResponse1) + .mockResolvedValueOnce(mockResponse2); - it("should aggregate results across multiple successful batches", async () => { - const totalUsers = REQUESTS_PER_BATCH + 50; - const userIdsBatch = Array.from({ length: totalUsers }, () => newGuid() as UserId); + const result = await service.bulkReinvite(mockOrganization, userIdsBatch); - const mockResponse1 = new ListResponse( - { - data: userIdsBatch.slice(0, REQUESTS_PER_BATCH).map((id) => ({ - id, - error: null, - })), - continuationToken: null, - }, - OrganizationUserBulkResponse, - ); + expect(result.successful).toBeDefined(); + expect(result.successful?.response).toHaveLength(totalUsers); + expect(result.successful?.response.slice(0, REQUESTS_PER_BATCH)).toEqual(mockResponse1.data); + expect(result.successful?.response.slice(REQUESTS_PER_BATCH)).toEqual(mockResponse2.data); + expect(result.failed).toHaveLength(0); + }); - const mockResponse2 = new ListResponse( - { - data: userIdsBatch.slice(REQUESTS_PER_BATCH).map((id) => ({ - id, - error: null, - })), - continuationToken: null, - }, - OrganizationUserBulkResponse, - ); + it("should handle mixed individual errors across multiple batches", async () => { + const totalUsers = REQUESTS_PER_BATCH + 4; + const userIdsBatch = Array.from({ length: totalUsers }, () => newGuid() as UserId); - organizationUserApiService.postManyOrganizationUserReinvite - .mockResolvedValueOnce(mockResponse1) - .mockResolvedValueOnce(mockResponse2); + const mockResponse1 = new ListResponse( + { + data: userIdsBatch.slice(0, REQUESTS_PER_BATCH).map((id, index) => ({ + id, + error: index % 10 === 0 ? "Rate limit exceeded" : null, + })), + continuationToken: null, + }, + OrganizationUserBulkResponse, + ); - const result = await service.bulkReinvite(mockOrganization, userIdsBatch); + const mockResponse2 = new ListResponse( + { + data: [ + { id: userIdsBatch[REQUESTS_PER_BATCH], error: null }, + { id: userIdsBatch[REQUESTS_PER_BATCH + 1], error: "Invalid email" }, + { id: userIdsBatch[REQUESTS_PER_BATCH + 2], error: null }, + { id: userIdsBatch[REQUESTS_PER_BATCH + 3], error: "User suspended" }, + ], + continuationToken: null, + }, + OrganizationUserBulkResponse, + ); - expect(result.successful).toBeDefined(); - expect(result.successful?.response).toHaveLength(totalUsers); - expect(result.successful?.response.slice(0, REQUESTS_PER_BATCH)).toEqual( - mockResponse1.data, - ); - expect(result.successful?.response.slice(REQUESTS_PER_BATCH)).toEqual(mockResponse2.data); - expect(result.failed).toHaveLength(0); - }); + organizationUserApiService.postManyOrganizationUserReinvite + .mockResolvedValueOnce(mockResponse1) + .mockResolvedValueOnce(mockResponse2); - it("should handle mixed individual errors across multiple batches", async () => { - const totalUsers = REQUESTS_PER_BATCH + 4; - const userIdsBatch = Array.from({ length: totalUsers }, () => newGuid() as UserId); + const result = await service.bulkReinvite(mockOrganization, userIdsBatch); - const mockResponse1 = new ListResponse( - { - data: userIdsBatch.slice(0, REQUESTS_PER_BATCH).map((id, index) => ({ - id, - error: index % 10 === 0 ? "Rate limit exceeded" : null, - })), - continuationToken: null, - }, - OrganizationUserBulkResponse, - ); + // Count expected failures: every 10th index (0, 10, 20, ..., 490) in first batch + 2 explicit in second batch + // Indices 0 to REQUESTS_PER_BATCH-1 where index % 10 === 0: that's floor((BATCH_SIZE-1)/10) + 1 values + const expectedFailuresInBatch1 = Math.floor((REQUESTS_PER_BATCH - 1) / 10) + 1; + const expectedFailuresInBatch2 = 2; + const expectedTotalFailures = expectedFailuresInBatch1 + expectedFailuresInBatch2; + const expectedSuccesses = totalUsers - expectedTotalFailures; - const mockResponse2 = new ListResponse( - { - data: [ - { id: userIdsBatch[REQUESTS_PER_BATCH], error: null }, - { id: userIdsBatch[REQUESTS_PER_BATCH + 1], error: "Invalid email" }, - { id: userIdsBatch[REQUESTS_PER_BATCH + 2], error: null }, - { id: userIdsBatch[REQUESTS_PER_BATCH + 3], error: "User suspended" }, - ], - continuationToken: null, - }, - OrganizationUserBulkResponse, - ); + expect(result.successful).toBeDefined(); + expect(result.successful?.response).toHaveLength(expectedSuccesses); + expect(result.failed).toHaveLength(expectedTotalFailures); + expect(result.failed.some((f) => f.error === "Rate limit exceeded")).toBe(true); + expect(result.failed.some((f) => f.error === "Invalid email")).toBe(true); + expect(result.failed.some((f) => f.error === "User suspended")).toBe(true); + }); - organizationUserApiService.postManyOrganizationUserReinvite - .mockResolvedValueOnce(mockResponse1) - .mockResolvedValueOnce(mockResponse2); + it("should aggregate all failures when all batches fail", async () => { + const totalUsers = REQUESTS_PER_BATCH + 100; + const userIdsBatch = Array.from({ length: totalUsers }, () => newGuid() as UserId); + const errorMessage = "All batches failed"; - const result = await service.bulkReinvite(mockOrganization, userIdsBatch); + organizationUserApiService.postManyOrganizationUserReinvite.mockRejectedValue( + new Error(errorMessage), + ); - // Count expected failures: every 10th index (0, 10, 20, ..., 490) in first batch + 2 explicit in second batch - // Indices 0 to REQUESTS_PER_BATCH-1 where index % 10 === 0: that's floor((BATCH_SIZE-1)/10) + 1 values - const expectedFailuresInBatch1 = Math.floor((REQUESTS_PER_BATCH - 1) / 10) + 1; - const expectedFailuresInBatch2 = 2; - const expectedTotalFailures = expectedFailuresInBatch1 + expectedFailuresInBatch2; - const expectedSuccesses = totalUsers - expectedTotalFailures; + const result = await service.bulkReinvite(mockOrganization, userIdsBatch); - expect(result.successful).toBeDefined(); - expect(result.successful?.response).toHaveLength(expectedSuccesses); - expect(result.failed).toHaveLength(expectedTotalFailures); - expect(result.failed.some((f) => f.error === "Rate limit exceeded")).toBe(true); - expect(result.failed.some((f) => f.error === "Invalid email")).toBe(true); - expect(result.failed.some((f) => f.error === "User suspended")).toBe(true); - }); + expect(result.successful).toBeUndefined(); + expect(result.failed).toHaveLength(totalUsers); + expect(result.failed.every((f) => f.error === errorMessage)).toBe(true); + expect(organizationUserApiService.postManyOrganizationUserReinvite).toHaveBeenCalledTimes(2); + }); - it("should aggregate all failures when all batches fail", async () => { - const totalUsers = REQUESTS_PER_BATCH + 100; - const userIdsBatch = Array.from({ length: totalUsers }, () => newGuid() as UserId); - const errorMessage = "All batches failed"; + it("should handle empty data in batch response", async () => { + const totalUsers = REQUESTS_PER_BATCH + 50; + const userIdsBatch = Array.from({ length: totalUsers }, () => newGuid() as UserId); - organizationUserApiService.postManyOrganizationUserReinvite.mockRejectedValue( - new Error(errorMessage), - ); + const mockResponse1 = new ListResponse( + { + data: userIdsBatch.slice(0, REQUESTS_PER_BATCH).map((id) => ({ + id, + error: null, + })), + continuationToken: null, + }, + OrganizationUserBulkResponse, + ); - const result = await service.bulkReinvite(mockOrganization, userIdsBatch); + const mockResponse2 = new ListResponse( + { + data: [], + continuationToken: null, + }, + OrganizationUserBulkResponse, + ); - expect(result.successful).toBeUndefined(); - expect(result.failed).toHaveLength(totalUsers); - expect(result.failed.every((f) => f.error === errorMessage)).toBe(true); - expect(organizationUserApiService.postManyOrganizationUserReinvite).toHaveBeenCalledTimes( - 2, - ); - }); + organizationUserApiService.postManyOrganizationUserReinvite + .mockResolvedValueOnce(mockResponse1) + .mockResolvedValueOnce(mockResponse2); - it("should handle empty data in batch response", async () => { - const totalUsers = REQUESTS_PER_BATCH + 50; - const userIdsBatch = Array.from({ length: totalUsers }, () => newGuid() as UserId); + const result = await service.bulkReinvite(mockOrganization, userIdsBatch); - const mockResponse1 = new ListResponse( - { - data: userIdsBatch.slice(0, REQUESTS_PER_BATCH).map((id) => ({ - id, - error: null, - })), - continuationToken: null, - }, - OrganizationUserBulkResponse, - ); + expect(result.successful).toBeDefined(); + expect(result.successful?.response).toHaveLength(REQUESTS_PER_BATCH); + expect(result.failed).toHaveLength(0); + }); - const mockResponse2 = new ListResponse( - { - data: [], - continuationToken: null, - }, - OrganizationUserBulkResponse, - ); + it("should process batches sequentially in order", async () => { + const totalUsers = REQUESTS_PER_BATCH * 2; + const userIdsBatch = Array.from({ length: totalUsers }, () => newGuid() as UserId); + const callOrder: number[] = []; - organizationUserApiService.postManyOrganizationUserReinvite - .mockResolvedValueOnce(mockResponse1) - .mockResolvedValueOnce(mockResponse2); + organizationUserApiService.postManyOrganizationUserReinvite.mockImplementation( + async (orgId, ids) => { + const batchIndex = ids.includes(userIdsBatch[0]) ? 1 : 2; + callOrder.push(batchIndex); - const result = await service.bulkReinvite(mockOrganization, userIdsBatch); + return new ListResponse( + { + data: ids.map((id) => ({ + id, + error: null, + })), + continuationToken: null, + }, + OrganizationUserBulkResponse, + ); + }, + ); - expect(result.successful).toBeDefined(); - expect(result.successful?.response).toHaveLength(REQUESTS_PER_BATCH); - expect(result.failed).toHaveLength(0); - }); + await service.bulkReinvite(mockOrganization, userIdsBatch); - it("should process batches sequentially in order", async () => { - const totalUsers = REQUESTS_PER_BATCH * 2; - const userIdsBatch = Array.from({ length: totalUsers }, () => newGuid() as UserId); - const callOrder: number[] = []; - - organizationUserApiService.postManyOrganizationUserReinvite.mockImplementation( - async (orgId, ids) => { - const batchIndex = ids.includes(userIdsBatch[0]) ? 1 : 2; - callOrder.push(batchIndex); - - return new ListResponse( - { - data: ids.map((id) => ({ - id, - error: null, - })), - continuationToken: null, - }, - OrganizationUserBulkResponse, - ); - }, - ); - - await service.bulkReinvite(mockOrganization, userIdsBatch); - - expect(callOrder).toEqual([1, 2]); - expect(organizationUserApiService.postManyOrganizationUserReinvite).toHaveBeenCalledTimes( - 2, - ); - }); + expect(callOrder).toEqual([1, 2]); + expect(organizationUserApiService.postManyOrganizationUserReinvite).toHaveBeenCalledTimes(2); }); }); diff --git a/apps/web/src/app/admin-console/organizations/members/services/member-actions/member-actions.service.ts b/apps/web/src/app/admin-console/organizations/members/services/member-actions/member-actions.service.ts index 5833238209c..e8c4a21d675 100644 --- a/apps/web/src/app/admin-console/organizations/members/services/member-actions/member-actions.service.ts +++ b/apps/web/src/app/admin-console/organizations/members/services/member-actions/member-actions.service.ts @@ -16,9 +16,7 @@ import { import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; import { assertNonNullish } from "@bitwarden/common/auth/utils"; import { OrganizationMetadataServiceAbstraction } from "@bitwarden/common/billing/abstractions/organization-metadata.service.abstraction"; -import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { ListResponse } from "@bitwarden/common/models/response/list.response"; -import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { Utils } from "@bitwarden/common/platform/misc/utils"; import { DialogService } from "@bitwarden/components"; @@ -45,7 +43,6 @@ export interface BulkActionResult { export class MemberActionsService { private organizationUserApiService = inject(OrganizationUserApiService); private organizationUserService = inject(OrganizationUserService); - private configService = inject(ConfigService); private organizationMetadataService = inject(OrganizationMetadataServiceAbstraction); private apiService = inject(ApiService); private dialogService = inject(DialogService); @@ -175,18 +172,9 @@ export class MemberActionsService { async bulkReinvite(organization: Organization, userIds: UserId[]): Promise { this.startProcessing(); try { - const increaseBulkReinviteLimitForCloud = await firstValueFrom( - this.configService.getFeatureFlag$(FeatureFlag.IncreaseBulkReinviteLimitForCloud), + return this.processBatchedOperation(userIds, REQUESTS_PER_BATCH, (batch) => + this.organizationUserApiService.postManyOrganizationUserReinvite(organization.id, batch), ); - if (increaseBulkReinviteLimitForCloud) { - return await this.vNextBulkReinvite(organization, userIds); - } else { - const result = await this.organizationUserApiService.postManyOrganizationUserReinvite( - organization.id, - userIds, - ); - return { successful: result, failed: [] }; - } } catch (error) { return { failed: userIds.map((id) => ({ id, error: (error as Error).message ?? String(error) })), @@ -196,15 +184,6 @@ export class MemberActionsService { } } - async vNextBulkReinvite( - organization: Organization, - userIds: UserId[], - ): Promise { - return this.processBatchedOperation(userIds, REQUESTS_PER_BATCH, (batch) => - this.organizationUserApiService.postManyOrganizationUserReinvite(organization.id, batch), - ); - } - allowResetPassword( orgUser: OrganizationUserView, organization: Organization, diff --git a/bitwarden_license/bit-web/src/app/admin-console/providers/manage/deprecated_members.component.ts b/bitwarden_license/bit-web/src/app/admin-console/providers/manage/deprecated_members.component.ts index 004b0a8f7c9..3c6e530b686 100644 --- a/bitwarden_license/bit-web/src/app/admin-console/providers/manage/deprecated_members.component.ts +++ b/bitwarden_license/bit-web/src/app/admin-console/providers/manage/deprecated_members.component.ts @@ -19,7 +19,6 @@ import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { assertNonNullish } from "@bitwarden/common/auth/utils"; import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; import { ListResponse } from "@bitwarden/common/models/response/list.response"; -import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; @@ -85,7 +84,6 @@ export class MembersComponent extends BaseMembersComponent { private providerService: ProviderService, private router: Router, private accountService: AccountService, - private configService: ConfigService, private environmentService: EnvironmentService, ) { super( @@ -100,7 +98,7 @@ export class MembersComponent extends BaseMembersComponent { toastService, ); - this.dataSource = new MembersTableDataSource(this.configService, this.environmentService); + this.dataSource = new MembersTableDataSource(this.environmentService); combineLatest([ this.activatedRoute.parent.params, diff --git a/bitwarden_license/bit-web/src/app/admin-console/providers/manage/members.component.ts b/bitwarden_license/bit-web/src/app/admin-console/providers/manage/members.component.ts index d02b44af1be..a2330be4c6f 100644 --- a/bitwarden_license/bit-web/src/app/admin-console/providers/manage/members.component.ts +++ b/bitwarden_license/bit-web/src/app/admin-console/providers/manage/members.component.ts @@ -21,7 +21,6 @@ import { Provider } from "@bitwarden/common/admin-console/models/domain/provider import { ProviderUserBulkRequest } from "@bitwarden/common/admin-console/models/request/provider/provider-user-bulk.request"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { getUserId } from "@bitwarden/common/auth/services/account.service"; -import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { ValidationService } from "@bitwarden/common/platform/abstractions/validation.service"; @@ -72,7 +71,6 @@ export class vNextMembersComponent { private activatedRoute = inject(ActivatedRoute); private providerService = inject(ProviderService); private accountService = inject(AccountService); - private configService = inject(ConfigService); private environmentService = inject(EnvironmentService); private providerActionsService = inject(ProviderActionsService); private memberActionsService = inject(MemberActionsService); @@ -94,7 +92,7 @@ export class vNextMembersComponent { protected statusToggle = new BehaviorSubject(undefined); protected readonly dataSource: WritableSignal = signal( - new ProvidersTableDataSource(this.configService, this.environmentService), + new ProvidersTableDataSource(this.environmentService), ); protected readonly firstLoaded: WritableSignal = signal(false); @@ -177,7 +175,7 @@ export class vNextMembersComponent { // Capture the original count BEFORE enforcing the limit const originalInvitedCount = allInvitedUsers.length; - // When feature flag is enabled, limit invited users and uncheck the excess + // In cloud environments, limit invited users and uncheck the excess let checkedInvitedUsers: ProviderUser[]; if (this.dataSource().isIncreasedBulkLimitEnabled()) { checkedInvitedUsers = this.dataSource().limitAndUncheckExcess( @@ -198,7 +196,7 @@ export class vNextMembersComponent { } try { - // When feature flag is enabled, show toast instead of dialog + // In cloud environments, show toast instead of dialog if (this.dataSource().isIncreasedBulkLimitEnabled()) { await this.apiService.postManyProviderUserReinvite( providerId, @@ -226,7 +224,7 @@ export class vNextMembersComponent { }); } } else { - // Feature flag disabled - show legacy dialog + // In self-hosted environments, show legacy dialog const request = this.apiService.postManyProviderUserReinvite( providerId, new ProviderUserBulkRequest(checkedInvitedUsers.map((user) => user.id)), diff --git a/libs/common/src/enums/feature-flag.enum.ts b/libs/common/src/enums/feature-flag.enum.ts index f761aea1b08..86fd0927f28 100644 --- a/libs/common/src/enums/feature-flag.enum.ts +++ b/libs/common/src/enums/feature-flag.enum.ts @@ -13,7 +13,6 @@ export enum FeatureFlag { /* Admin Console Team */ AutoConfirm = "pm-19934-auto-confirm-organization-users", BlockClaimedDomainAccountCreation = "pm-28297-block-uninvited-claimed-domain-registration", - IncreaseBulkReinviteLimitForCloud = "pm-28251-increase-bulk-reinvite-limit-for-cloud", MembersComponentRefactor = "pm-29503-refactor-members-inheritance", /* Auth */ @@ -104,7 +103,6 @@ export const DefaultFeatureFlagValue = { /* Admin Console Team */ [FeatureFlag.AutoConfirm]: FALSE, [FeatureFlag.BlockClaimedDomainAccountCreation]: FALSE, - [FeatureFlag.IncreaseBulkReinviteLimitForCloud]: FALSE, [FeatureFlag.MembersComponentRefactor]: FALSE, /* Autofill */