diff --git a/packages/backend/CLAUDE.md b/packages/backend/CLAUDE.md index a4337cebe5..305ee2c201 100644 --- a/packages/backend/CLAUDE.md +++ b/packages/backend/CLAUDE.md @@ -83,10 +83,10 @@ Segment inclusion/exclusion for **feature flags** is precomputed and stored flat - **`FeatureFlagPrecomputedSegment` entity** (`src/api/models/FeatureFlagPrecomputedSegment.ts`) — one row per feature flag, columns: `featureFlagId` (PK), `inclusionIds: text[]`, `exclusionIds: text[]`. FK to `feature_flag` with `onDelete: CASCADE`. - **`FeatureFlagPrecomputedSegmentService`** (`src/api/services/FeatureFlagPrecomputedSegmentService.ts`) — owns all computation and cache logic: - - `recomputeForFlag(flagId)` — flattens all enabled inclusion/exclusion segments (recursive sub-segments) into flat ID arrays and upserts the row. This is the only method that `await`s — callers on write paths never call it directly. + - `recomputeForFlag(flagId)` — flattens all enabled inclusion/exclusion segments (recursive sub-segments) into flat ID arrays and upserts the row. Imports and batch segment deletion await this method directly. - `scheduleRecomputeForFlags(flagIds[])` — fire-and-forget recompute for a known set of flags (swallows/logs errors). The flag-side counterpart to `scheduleRecomputeForSegment`. - `scheduleRecomputeForSegment(segmentId)` — fire-and-forget; finds all flags referencing a segment (and its parents) and recomputes each. - - `withRecompute(logger, resolveAffectedFlagIds, work)` — **the wrapper all top-level write methods use.** Resolves affected flag IDs *before* `work`, runs `work` (which must own/commit its own transaction), then fires a fire-and-forget recompute *after* commit. Keeps mutation + recompute in one call so a refactor can't drop the recompute. `work` is never blocked on the recompute. + - `withRecompute(logger, resolveAffectedFlagIds, work)` — **the wrapper for background recomputation on write paths.** Resolves affected flag IDs *before* `work`, runs `work` (which must own/commit its own transaction), then fires a fire-and-forget recompute *after* commit. Keeps mutation + recompute in one call so a refactor can't drop the recompute. `work` is never blocked on the recompute. - `getAffectedFlagIds(segmentId)` — public helper that returns flag IDs affected by a given segment (used as the `resolveAffectedFlagIds` for segment deletes). - `getPrecomputedSets(flagIds[])` — cache-wrapped batch fetch, returns a `Map`. - `backfillMissingFlags(logger)` — called at startup; computes rows only for flags that have none yet (no-op once all flags are populated). @@ -105,14 +105,14 @@ Segment inclusion/exclusion for **feature flags** is precomputed and stored flat | Private list added to a shared segment | `SegmentService.addList` → `scheduleRecomputeForSegment` | | Private list removed from a shared segment | `SegmentService.deleteList` → `scheduleRecomputeForSegment` | | Segment members/structure updated | `SegmentService.addSegmentDataWithPipeline` → `scheduleRecomputeForSegment` | -| Segment deleted entirely | `SegmentService.deleteSegment` → `withRecompute` (collects affected flag IDs **before** the delete, recomputes **after** commit) | +| Segment deleted entirely | `SegmentService.deleteSegment` → `withRecompute` for single deletion; batch deletion collects affected flag IDs after the target lock and **before** deletion, then awaits recomputation **after** commit | | Server startup | `app.ts` → `backfillMissingFlags` — backfills any flag with no row | -All recomputes triggered from write paths are **fire-and-forget** — no request handler (flag-side or segment-side) ever blocks on a recompute. The `import*` paths are the one exception: they `await recomputeForFlag` so "import complete" means the rows are ready. +Ordinary write paths use **fire-and-forget** recomputation. The `import*` paths await `recomputeForFlag` so "import complete" means the rows are ready. Batch segment deletion also awaits post-commit recomputation for both flags and experiments before processing the next segment. `SegmentService.deleteSegment` selects this awaited branch when supplied with the batch transaction executor; ordinary callers retain background recomputation. ### Key invariant -The `feature_flag_precomputed_segment` row must always be recomputed **after** the structural change commits, so the flat arrays reflect the new state. For deletions specifically, affected flag IDs must be collected **before** the delete because the join table records are gone afterward. Both halves of this invariant are enforced by `withRecompute` (resolve-before → work → recompute-after), so top-level write methods get the ordering for free rather than hand-rolling it. +The `feature_flag_precomputed_segment` row must always be recomputed **after** the structural change commits, so the flat arrays reflect the new state. For deletions specifically, affected flag IDs must be collected **before** the delete because the join table records are gone afterward. `withRecompute` enforces this ordering for background recomputation. The batch branch collects owners inside the supplied transaction, after the target lock is held, and waits for all post-commit updates to settle before propagating any failure. ## Precomputed Segment Lists (Experiments) @@ -143,10 +143,10 @@ Experiment join tables (`ExperimentSegmentInclusion` / `ExperimentSegmentExclusi | Experiment lists imported | `ExperimentService.importExperimentLists` → `await recomputeForExperiment` after the import transaction commits | | Experiment context changed (deletes all its lists) | `ExperimentService.updateExperimentInDB` → `scheduleRecomputeForExperiments` after commit (recomputes to empty; `deleteAllListsFromExperiment` deletes the private segments directly, so the precomputed row would otherwise keep stale IDs) | | Shared segment members/structure changed | `SegmentService.addList` / `deleteList` / `addSegmentDataWithPipeline` → `scheduleRecomputeForSegment` for **both** the flag and experiment services | -| Segment deleted entirely | `SegmentService.deleteSegment` → collects affected experiment IDs **before** the delete, recomputes **after** commit (flags use `withRecompute` in the same method) | +| Segment deleted entirely | `SegmentService.deleteSegment` → collects affected experiment IDs **before** deletion and recomputes **after** commit; single deletion schedules background work, while batch deletion collects owners after the target lock and awaits recomputation | | Server startup | `app.ts` → `backfillExperimentPrecomputedSegments` (guarded by `.catch` — a missing table never crashes startup) | -Same invariant as feature flags: recompute **after** the change commits; for deletes, collect affected experiment IDs **before**. All write-path recomputes are fire-and-forget except the `create` / import paths, which `await` so "done" means the row is ready. +Same invariant as feature flags: recompute **after** the change commits; for deletes, collect affected experiment IDs **before**. Write-path recomputes are fire-and-forget except the `create` / import paths and batch segment deletion, which await recomputation. A batch finishes each segment's post-commit updates before deleting the next segment. ### INCLUDE_ALL semantics (resolved) diff --git a/packages/backend/src/api/controllers/ExperimentController.ts b/packages/backend/src/api/controllers/ExperimentController.ts index b3ce91cedb..3c7278545a 100644 --- a/packages/backend/src/api/controllers/ExperimentController.ts +++ b/packages/backend/src/api/controllers/ExperimentController.ts @@ -1,3 +1,7 @@ +import { BatchDeleteResult } from 'upgrade_types'; +import { Inject } from 'typedi'; +import { BatchDeleteService } from '../services/BatchDeleteService'; +import { BatchEntityIdsValidator } from './validators/BatchEntityIdsValidator'; import { Body, Get, @@ -664,9 +668,43 @@ export class ExperimentController { public importExportService: ImportExportService, public cacheService: CacheService, public thompsonSamplingCrudService: ThompsonSamplingExperimentCrudService, - public adaptiveExperimentConfigDispatcher: AdaptiveExperimentConfigDispatcherService + public adaptiveExperimentConfigDispatcher: AdaptiveExperimentConfigDispatcherService, + @Inject(() => BatchDeleteService) private batchDeleteService: BatchDeleteService ) {} + /** + * @swagger + * /experiments/batch-delete: + * post: + * summary: Delete the selected experiments + * description: Deletes selected experiments sequentially regardless of state, skipping missing items. Execution failures stop the remaining items. Returns one result per ID. + * tags: + * - Experiments + * parameters: + * - in: body + * name: selection + * required: true + * schema: + * $ref: '#/definitions/BatchEntityIdsRequest' + * responses: + * '200': + * description: Per-ID deletion outcomes, including failures and items not attempted. + * schema: + * $ref: '#/definitions/BatchDeleteResult' + * '400': + * description: Expected a nonempty array of unique UUIDs. + * '401': + * description: AuthorizationRequiredError + */ + @Post('/batch-delete') + public batchDelete( + @Body({ validate: true }) { ids }: BatchEntityIdsValidator, + @CurrentUser() currentUser: UserDTO, + @Req() request: AppRequest + ): Promise { + return this.batchDeleteService.delete('experiments', ids, currentUser, request.logger); + } + /** * @swagger * /experiments/names: diff --git a/packages/backend/src/api/controllers/FeatureFlagController.ts b/packages/backend/src/api/controllers/FeatureFlagController.ts index bbac5ebbe2..2cbbc0cd2f 100644 --- a/packages/backend/src/api/controllers/FeatureFlagController.ts +++ b/packages/backend/src/api/controllers/FeatureFlagController.ts @@ -1,3 +1,7 @@ +import { BatchDeleteResult } from 'upgrade_types'; +import { Inject } from 'typedi'; +import { BatchDeleteService } from '../services/BatchDeleteService'; +import { BatchEntityIdsValidator } from './validators/BatchEntityIdsValidator'; import { JsonController, Authorized, @@ -154,7 +158,44 @@ interface FeatureFlagsPaginationInfo extends PaginationResponse { @Authorized() @JsonController('/flags') export class FeatureFlagsController { - constructor(public featureFlagService: FeatureFlagService, public experimentUserService: ExperimentUserService) {} + constructor( + public featureFlagService: FeatureFlagService, + public experimentUserService: ExperimentUserService, + @Inject(() => BatchDeleteService) private batchDeleteService: BatchDeleteService + ) {} + + /** + * @swagger + * /flags/batch-delete: + * post: + * summary: Delete the selected flags + * description: Deletes selected feature flags sequentially regardless of status, skipping missing items. Execution failures stop the remaining items. Returns one result per ID. + * tags: + * - Feature Flags + * parameters: + * - in: body + * name: selection + * required: true + * schema: + * $ref: '#/definitions/BatchEntityIdsRequest' + * responses: + * '200': + * description: Per-ID deletion outcomes, including failures and items not attempted. + * schema: + * $ref: '#/definitions/BatchDeleteResult' + * '400': + * description: Expected a nonempty array of unique UUIDs. + * '401': + * description: AuthorizationRequiredError + */ + @Post('/batch-delete') + public batchDelete( + @Body({ validate: true }) { ids }: BatchEntityIdsValidator, + @CurrentUser() currentUser: UserDTO, + @Req() request: AppRequest + ): Promise { + return this.batchDeleteService.delete('flags', ids, currentUser, request.logger); + } /** * @swagger diff --git a/packages/backend/src/api/controllers/SegmentController.ts b/packages/backend/src/api/controllers/SegmentController.ts index 4255507ffa..c4949ff15f 100644 --- a/packages/backend/src/api/controllers/SegmentController.ts +++ b/packages/backend/src/api/controllers/SegmentController.ts @@ -1,5 +1,11 @@ +import { UserDTO } from '../DTO/UserDTO'; +import { BatchDeleteResult } from 'upgrade_types'; +import { Inject } from 'typedi'; +import { BatchDeleteService } from '../services/BatchDeleteService'; +import { BatchEntityIdsValidator } from './validators/BatchEntityIdsValidator'; import { JsonController, + CurrentUser, Get, Delete, Authorized, @@ -49,6 +55,38 @@ interface SegmentPaginationInfo extends PaginationResponse { /** * @swagger * definitions: + * BatchDeleteItemResult: + * type: object + * required: [id, outcome] + * properties: + * id: + * type: string + * format: uuid + * outcome: + * type: string + * enum: [deleted, not_found, failed, unknown, not_attempted] + * reasonCode: + * type: string + * enum: [not_found, delete_failed, lock_timeout, outcome_unknown, post_delete_failed] + * BatchDeleteResult: + * type: object + * required: [results] + * properties: + * results: + * type: array + * items: + * $ref: '#/definitions/BatchDeleteItemResult' + * BatchEntityIdsRequest: + * type: object + * required: [ids] + * properties: + * ids: + * type: array + * minItems: 1 + * uniqueItems: true + * items: + * type: string + * format: uuid * Segment: * required: * - name @@ -233,7 +271,43 @@ interface SegmentPaginationInfo extends PaginationResponse { @Authorized() @JsonController('/segments') export class SegmentController { - constructor(public segmentService: SegmentService) {} + constructor( + public segmentService: SegmentService, + @Inject(() => BatchDeleteService) private batchDeleteService: BatchDeleteService + ) {} + + /** + * @swagger + * /segments/batch-delete: + * post: + * summary: Delete the selected segments + * description: Deletes selected segments sequentially using the existing deletion workflow, skipping missing items. Execution failures stop the remaining items. Returns one result per ID. + * tags: + * - Segment + * parameters: + * - in: body + * name: selection + * required: true + * schema: + * $ref: '#/definitions/BatchEntityIdsRequest' + * responses: + * '200': + * description: Per-ID deletion outcomes, including failures and items not attempted. + * schema: + * $ref: '#/definitions/BatchDeleteResult' + * '400': + * description: Expected a nonempty array of unique UUIDs. + * '401': + * description: AuthorizationRequiredError + */ + @Post('/batch-delete') + public batchDelete( + @Body({ validate: true }) { ids }: BatchEntityIdsValidator, + @CurrentUser() currentUser: UserDTO, + @Req() request: AppRequest + ): Promise { + return this.batchDeleteService.delete('segments', ids, currentUser, request.logger); + } /** * @swagger diff --git a/packages/backend/src/api/controllers/validators/BatchEntityIdsValidator.ts b/packages/backend/src/api/controllers/validators/BatchEntityIdsValidator.ts new file mode 100644 index 0000000000..e949d2f9cf --- /dev/null +++ b/packages/backend/src/api/controllers/validators/BatchEntityIdsValidator.ts @@ -0,0 +1,26 @@ +import { ArrayNotEmpty, IsArray, IsUUID, registerDecorator } from 'class-validator'; +import { BatchEntityIdsRequest } from 'upgrade_types'; + +const HasUniqueIds = () => (object: object, propertyName: string) => { + registerDecorator({ + name: 'arrayUnique', + target: object.constructor, + propertyName, + options: { message: "All $property's elements must be unique" }, + validator: { + validate(value: unknown) { + if (!Array.isArray(value)) return false; + const normalizedIds = value.map((id: unknown) => (typeof id === 'string' ? id.toLowerCase() : id)); + return new Set(normalizedIds).size === value.length; + }, + }, + }); +}; + +export class BatchEntityIdsValidator implements BatchEntityIdsRequest { + @IsArray() + @ArrayNotEmpty() + @HasUniqueIds() + @IsUUID('all', { each: true }) + public ids: string[]; +} diff --git a/packages/backend/src/api/repositories/DeletionRepository.ts b/packages/backend/src/api/repositories/DeletionRepository.ts new file mode 100644 index 0000000000..6f6ca8d5a0 --- /dev/null +++ b/packages/backend/src/api/repositories/DeletionRepository.ts @@ -0,0 +1,25 @@ +import { EntityManager, Repository } from 'typeorm'; +import { BatchDeleteEntity } from 'upgrade_types'; +import { EntityRepository } from '../../typeorm-typedi-extensions'; +import { Experiment } from '../models/Experiment'; +import { FeatureFlag } from '../models/FeatureFlag'; +import { Segment } from '../models/Segment'; +import repositoryError from './utils/repositoryError'; + +/** All reads and locks use the caller's deletion transaction, never the repository's default manager. */ +@EntityRepository() +export class DeletionRepository extends Repository { + public async findForDeletion( + entity: BatchDeleteEntity, + id: string, + manager: EntityManager + ): Promise<{ id: string } | null> { + const target = { experiments: Experiment, flags: FeatureFlag, segments: Segment }[entity]; + return manager + .getRepository<{ id: string }>(target) + .findOne({ where: { id }, select: { id: true }, lock: { mode: 'pessimistic_write' } }) + .catch((errorMsg: any) => { + throw repositoryError('DeletionRepository', 'findForDeletion', { entity, id }, errorMsg); + }); + } +} diff --git a/packages/backend/src/api/repositories/ExperimentRepository.ts b/packages/backend/src/api/repositories/ExperimentRepository.ts index f1cfd1dcc9..4f32b1c3fb 100644 --- a/packages/backend/src/api/repositories/ExperimentRepository.ts +++ b/packages/backend/src/api/repositories/ExperimentRepository.ts @@ -460,16 +460,18 @@ export class ExperimentRepository extends Repository { } } - private buildConditionLevelPayloadQuery() { - return this.createQueryBuilder('experiment') + private buildConditionLevelPayloadQuery(repository: Repository = this) { + return repository + .createQueryBuilder('experiment') .leftJoinAndSelect('experiment.conditions', 'conditions') .leftJoinAndSelect('conditions.levelCombinationElements', 'levelCombinationElements') .leftJoinAndSelect('levelCombinationElements.level', 'level') .leftJoinAndSelect('conditions.conditionPayloads', 'conditionPayload'); } - private buildFactorDecisionPointPayloadQuery() { - return this.createQueryBuilder('experiment') + private buildFactorDecisionPointPayloadQuery(repository: Repository = this) { + return repository + .createQueryBuilder('experiment') .leftJoinAndSelect('experiment.partitions', 'partitions') .leftJoinAndSelect('experiment.stratificationFactor', 'stratificationFactor') .leftJoinAndSelect('partitions.conditionPayloads', 'conditionPayloads') @@ -478,8 +480,9 @@ export class ExperimentRepository extends Repository { .leftJoinAndSelect('factors.levels', 'levels'); } - private buildInclusionSegmentQuery() { - return this.createQueryBuilder('experiment') + private buildInclusionSegmentQuery(repository: Repository = this) { + return repository + .createQueryBuilder('experiment') .select('experiment.id') .leftJoinAndSelect('experiment.experimentSegmentInclusion', 'experimentSegmentInclusion') .leftJoinAndSelect('experimentSegmentInclusion.segment', 'segmentInclusion') @@ -488,8 +491,9 @@ export class ExperimentRepository extends Repository { .leftJoinAndSelect('segmentInclusion.subSegments', 'subSegment'); } - private buildExclusionSegmentQuery() { - return this.createQueryBuilder('experiment') + private buildExclusionSegmentQuery(repository: Repository = this) { + return repository + .createQueryBuilder('experiment') .select('experiment.id') .leftJoinAndSelect('experiment.experimentSegmentExclusion', 'experimentSegmentExclusion') .leftJoinAndSelect('experimentSegmentExclusion.segment', 'segmentExclusion') @@ -589,18 +593,20 @@ export class ExperimentRepository extends Repository { })); } - public async findOneExperiment(id: string): Promise { - const conditionLevelPayloadQuery = this.buildConditionLevelPayloadQuery() + public async findOneExperiment(id: string, entityManager?: EntityManager): Promise { + const repository = entityManager ? entityManager.getRepository(Experiment) : this; + const conditionLevelPayloadQuery = this.buildConditionLevelPayloadQuery(repository) .addOrderBy('conditions.order', 'ASC') .where({ id }); - const factorDecisionPointPayloadQuery = this.buildFactorDecisionPointPayloadQuery() + const factorDecisionPointPayloadQuery = this.buildFactorDecisionPointPayloadQuery(repository) .addOrderBy('partitions.order', 'ASC') .addOrderBy('factors.order', 'ASC') .addOrderBy('levels.order', 'ASC') .where({ id }); - const metricQuery = this.createQueryBuilder('experiment') + const metricQuery = repository + .createQueryBuilder('experiment') .leftJoinAndSelect('experiment.queries', 'queries') .leftJoinAndSelect('queries.metric', 'metric') .leftJoinAndSelect('experiment.stateTimeLogs', 'stateTimeLogs') @@ -608,8 +614,8 @@ export class ExperimentRepository extends Repository { .addOrderBy('queries.createdAt', 'ASC') .where({ id }); - const inclusionSegmentQuery = this.buildInclusionSegmentQuery().where({ id }); - const exclusionSegmentQuery = this.buildExclusionSegmentQuery().where({ id }); + const inclusionSegmentQuery = this.buildInclusionSegmentQuery(repository).where({ id }); + const exclusionSegmentQuery = this.buildExclusionSegmentQuery(repository).where({ id }); const [conditionLevelPayloadData, factorDecisionPointPayloadData, metricData, inclusionData, exclusionData] = await Promise.all([ diff --git a/packages/backend/src/api/repositories/FeatureFlagRepository.ts b/packages/backend/src/api/repositories/FeatureFlagRepository.ts index c6a1ea1f29..0defde4d7e 100644 --- a/packages/backend/src/api/repositories/FeatureFlagRepository.ts +++ b/packages/backend/src/api/repositories/FeatureFlagRepository.ts @@ -4,9 +4,69 @@ import { FeatureFlag } from '../models/FeatureFlag'; import repositoryError from './utils/repositoryError'; import { FEATURE_FLAG_STATUS, FILTER_MODE } from 'upgrade_types'; import { FeatureFlagValidation } from '../controllers/validators/FeatureFlagValidator'; +import { IndividualForSegment } from '../models/IndividualForSegment'; +import { GroupForSegment } from '../models/GroupForSegment'; @EntityRepository(FeatureFlag) export class FeatureFlagRepository extends Repository { + public async findOneForDetails(id: string, entityManager?: EntityManager): Promise { + const repository = entityManager ? entityManager.getRepository(FeatureFlag) : this; + const manager = entityManager || this.manager; + const featureFlag = await repository + .createQueryBuilder('feature_flag') + .leftJoinAndSelect('feature_flag.featureFlagSegmentInclusion', 'featureFlagSegmentInclusion') + .leftJoinAndSelect('featureFlagSegmentInclusion.segment', 'segmentInclusion') + .leftJoinAndSelect('segmentInclusion.subSegments', 'subSegment') + .leftJoinAndSelect('feature_flag.featureFlagSegmentExclusion', 'featureFlagSegmentExclusion') + .leftJoinAndSelect('featureFlagSegmentExclusion.segment', 'segmentExclusion') + .leftJoinAndSelect('segmentExclusion.subSegments', 'subSegmentExclusion') + .where({ id }) + .getOne(); + + if (!featureFlag) { + return undefined; + } + + // loadRelationCountAndMap was removed in TypeORM 1.0; fetch member counts with two batch queries. + const segments = [ + ...(featureFlag.featureFlagSegmentInclusion ?? []).map((r) => r.segment), + ...(featureFlag.featureFlagSegmentExclusion ?? []).map((r) => r.segment), + ].filter(Boolean); + + if (segments.length > 0) { + const segmentIds = segments.map((s) => s.id); + + const [individualCounts, groupCounts] = await Promise.all([ + manager + .createQueryBuilder() + .select('ifs.segmentId', 'segmentId') + .addSelect('COUNT(*)', 'count') + .from(IndividualForSegment, 'ifs') + .where('ifs.segmentId IN (:...segmentIds)', { segmentIds }) + .groupBy('ifs.segmentId') + .getRawMany<{ segmentId: string; count: string }>(), + manager + .createQueryBuilder() + .select('gfs.segmentId', 'segmentId') + .addSelect('COUNT(*)', 'count') + .from(GroupForSegment, 'gfs') + .where('gfs.segmentId IN (:...segmentIds)', { segmentIds }) + .groupBy('gfs.segmentId') + .getRawMany<{ segmentId: string; count: string }>(), + ]); + + const individualCountMap = new Map(individualCounts.map((r) => [r.segmentId, Number.parseInt(r.count, 10)])); + const groupCountMap = new Map(groupCounts.map((r) => [r.segmentId, Number.parseInt(r.count, 10)])); + + segments.forEach((segment) => { + segment.individualForSegmentCount = individualCountMap.get(segment.id) ?? 0; + segment.groupForSegmentCount = groupCountMap.get(segment.id) ?? 0; + }); + } + + return featureFlag; + } + public async insertFeatureFlag(flagDoc: FeatureFlag, entityManager: EntityManager): Promise { const result = await entityManager .createQueryBuilder() diff --git a/packages/backend/src/api/repositories/SegmentRepository.ts b/packages/backend/src/api/repositories/SegmentRepository.ts index 2759e17efa..8883f1a6ef 100644 --- a/packages/backend/src/api/repositories/SegmentRepository.ts +++ b/packages/backend/src/api/repositories/SegmentRepository.ts @@ -139,8 +139,8 @@ export class SegmentRepository extends Repository { return result.raw; } - public async findParentSegmentIds(segmentId: string): Promise { - const rows = await this.manager.query( + public async findParentSegmentIds(segmentId: string, entityManager?: EntityManager): Promise { + const rows = await (entityManager || this.manager).query( `SELECT "parentSegmentId" FROM "segment_for_segment" WHERE "childSegmentId" = $1`, [segmentId] ); diff --git a/packages/backend/src/api/services/BatchDeleteService.ts b/packages/backend/src/api/services/BatchDeleteService.ts new file mode 100644 index 0000000000..c029e410e0 --- /dev/null +++ b/packages/backend/src/api/services/BatchDeleteService.ts @@ -0,0 +1,140 @@ +import { Inject, Service } from 'typedi'; +import { DataSource, EntityManager } from 'typeorm'; +import { BatchDeleteEntity, BatchDeleteItemResult, BatchDeleteResult, DeletionReasonCode } from 'upgrade_types'; +import { UpgradeLogger } from '../../lib/logger/UpgradeLogger'; +import { InjectDataSource, InjectRepository } from '../../typeorm-typedi-extensions'; +import { DeletionTransaction } from '../../types/DeletionTransaction'; +import { UserDTO } from '../DTO/UserDTO'; +import { ExperimentService } from './ExperimentService'; +import { FeatureFlagService } from './FeatureFlagService'; +import { SegmentService } from './SegmentService'; +import { DeletionRepository } from '../repositories/DeletionRepository'; + +class BatchDeleteSkippedError extends Error { + constructor(public readonly result: BatchDeleteItemResult) { + super(result.reasonCode); + } +} + +@Service() +export class BatchDeleteService { + constructor( + @InjectDataSource() private dataSource: DataSource, + @Inject(() => ExperimentService) private experiments: ExperimentService, + @Inject(() => FeatureFlagService) private flags: FeatureFlagService, + @Inject(() => SegmentService) private segments: SegmentService, + @InjectRepository() private deletionRepository: DeletionRepository + ) {} + + public async delete( + entity: BatchDeleteEntity, + ids: string[], + user: UserDTO, + logger: UpgradeLogger + ): Promise { + const results: BatchDeleteItemResult[] = []; + for (const id of ids) { + const result = await this.deleteOne(entity, id, user, logger); + results.push(result); + // An already absent target does not prevent independent items from being deleted. + if (result.outcome === 'not_found') continue; + // A committed item with a post-delete failure must not be retried, but still stops this batch. + if (result.outcome !== 'deleted' || result.reasonCode) { + for (let index = results.length; index < ids.length; index++) { + results.push({ + id: ids[index], + outcome: 'not_attempted', + }); + } + break; + } + } + return { results }; + } + + private async deleteOne( + entity: BatchDeleteEntity, + id: string, + user: UserDTO, + logger: UpgradeLogger + ): Promise { + let committed = false; + let rolledBack = false; + let commitAttempted = false; + let mutationStarted = false; + let transactionError: unknown; + const executeTransaction: DeletionTransaction = async (work: (manager: EntityManager) => Promise) => { + const runner = this.dataSource.createQueryRunner(); + let response: T; + try { + await runner.connect(); + await runner.startTransaction('READ COMMITTED'); + const target = await this.deletionRepository.findForDeletion(entity, id, runner.manager); + if (!target) { + throw new BatchDeleteSkippedError({ id, outcome: 'not_found', reasonCode: DeletionReasonCode.NOT_FOUND }); + } + mutationStarted = true; + response = await work(runner.manager); + // Legacy experiment/flag repositories return arrays despite their declared return types. + const deleted = Array.isArray(response) ? response[0] : response; + if (!deleted || (deleted as { id?: string }).id?.toLowerCase() !== id.toLowerCase()) { + throw new Error('Deletion did not return the requested target'); + } + commitAttempted = true; + await runner.commitTransaction(); + committed = true; + } catch (error) { + transactionError = error; + if (runner.isTransactionActive) { + try { + await runner.rollbackTransaction(); + rolledBack = true; + } catch (rollbackError) { + logger.error({ message: 'Batch deletion rollback could not be confirmed', id, rollbackError }); + } + } + } finally { + try { + await runner.release(); + } catch (releaseError) { + logger.error({ message: 'Batch deletion connection release failed', id, releaseError }); + // Preserve an execution failure, but do not let a normal skip hide a release failure. + if (!transactionError || transactionError instanceof BatchDeleteSkippedError) transactionError = releaseError; + } + } + // Finish post-commit work after a confirmed commit or an ambiguous segment commit. + // Keep transactionError so the batch still stops and reports the original outcome. + if (transactionError && !committed && !(entity === 'segments' && commitAttempted)) throw transactionError; + return response; + }; + try { + if (entity === 'experiments') { + await this.experiments.delete(id, user, { logger, executeTransaction }); + } else if (entity === 'flags') { + await this.flags.delete(id, user, logger, executeTransaction); + } else { + await this.segments.deleteSegment(id, logger, executeTransaction); + } + if (transactionError) throw transactionError; + return committed + ? { id, outcome: 'deleted' } + : { id, outcome: 'unknown', reasonCode: DeletionReasonCode.OUTCOME_UNKNOWN }; + } catch (error) { + if (error instanceof BatchDeleteSkippedError && rolledBack) return error.result; + logger.error({ message: 'Batch deletion item failed', entity, id, committed, error }); + if (committed) return { id, outcome: 'deleted', reasonCode: DeletionReasonCode.POST_DELETE_FAILED }; + // A failed COMMIT can mean the server committed but its acknowledgment was lost. + if (commitAttempted || (mutationStarted && !rolledBack)) { + return { id, outcome: 'unknown', reasonCode: DeletionReasonCode.OUTCOME_UNKNOWN }; + } + return { + id, + outcome: 'failed', + reasonCode: + (error as { code?: string })?.code === '55P03' + ? DeletionReasonCode.LOCK_TIMEOUT + : DeletionReasonCode.DELETE_FAILED, + }; + } + } +} diff --git a/packages/backend/src/api/services/ExperimentPrecomputedSegmentService.ts b/packages/backend/src/api/services/ExperimentPrecomputedSegmentService.ts index 03b7ffb873..80c9111be3 100644 --- a/packages/backend/src/api/services/ExperimentPrecomputedSegmentService.ts +++ b/packages/backend/src/api/services/ExperimentPrecomputedSegmentService.ts @@ -6,6 +6,8 @@ import { ExperimentSegmentExclusionRepository } from '../repositories/Experiment import { ExperimentRepository } from '../repositories/ExperimentRepository'; import { SegmentRepository } from '../repositories/SegmentRepository'; import { ExperimentPrecomputedSegment } from '../models/ExperimentPrecomputedSegment'; +import { ExperimentSegmentInclusion } from '../models/ExperimentSegmentInclusion'; +import { ExperimentSegmentExclusion } from '../models/ExperimentSegmentExclusion'; import { CacheService } from './CacheService'; import { CACHE_PREFIX } from 'upgrade_types'; import { UpgradeLogger } from '../../lib/logger/UpgradeLogger'; @@ -53,13 +55,19 @@ export class ExperimentPrecomputedSegmentService extends PrecomputedSegmentServi }; } - protected async findOwnerIdsBySegmentId(segmentId: string): Promise { + protected async findOwnerIdsBySegmentId(segmentId: string, entityManager?: EntityManager): Promise { + const inclusionRepository = entityManager + ? entityManager.getRepository(ExperimentSegmentInclusion) + : this.experimentSegmentInclusionRepository; + const exclusionRepository = entityManager + ? entityManager.getRepository(ExperimentSegmentExclusion) + : this.experimentSegmentExclusionRepository; const [inclusionRecords, exclusionRecords] = await Promise.all([ - this.experimentSegmentInclusionRepository.find({ + inclusionRepository.find({ where: { segment: { id: segmentId } }, relations: { experiment: true }, }), - this.experimentSegmentExclusionRepository.find({ + exclusionRepository.find({ where: { segment: { id: segmentId } }, relations: { experiment: true }, }), @@ -98,8 +106,8 @@ export class ExperimentPrecomputedSegmentService extends PrecomputedSegmentServi this.scheduleRecomputeForOwners(experimentIds, logger); } - public getAffectedExperimentIds(segmentId: string): Promise { - return this.getAffectedOwnerIds(segmentId); + public getAffectedExperimentIds(segmentId: string, entityManager?: EntityManager): Promise { + return this.getAffectedOwnerIds(segmentId, entityManager); } public recomputeAllExperiments(logger: UpgradeLogger): Promise { diff --git a/packages/backend/src/api/services/ExperimentService.ts b/packages/backend/src/api/services/ExperimentService.ts index e17b7c0d56..c93f73906d 100644 --- a/packages/backend/src/api/services/ExperimentService.ts +++ b/packages/backend/src/api/services/ExperimentService.ts @@ -106,6 +106,7 @@ import { MetricService } from './MetricService'; import { ExperimentAuditLog } from '../models/ExperimentAuditLog'; import { SegmentRepository } from '../repositories/SegmentRepository'; import { NotFoundException } from '@nestjs/common/exceptions'; +import { DeletionTransaction } from '../../types/DeletionTransaction'; const errorRemovePart = 'An instance of ExperimentDTO has failed the validation:\n - '; const stratificationErrorMessage = @@ -425,15 +426,20 @@ export class ExperimentService { public async delete( experimentId: string, currentUser: UserDTO, - options?: { logger?: UpgradeLogger; existingEntityManager?: EntityManager } + options?: { + logger?: UpgradeLogger; + existingEntityManager?: EntityManager; + executeTransaction?: DeletionTransaction; + } ): Promise { - const { logger, existingEntityManager } = options; + const { logger, existingEntityManager, executeTransaction } = options; if (logger) { logger.info({ message: `Delete experiment => ${experimentId}` }); } const entityManager = existingEntityManager || this.dataSource.manager; - return await entityManager.transaction(async (transactionalEntityManager) => { - const experiment = await this.experimentRepository.findOneExperiment(experimentId); + const transaction: DeletionTransaction = executeTransaction || ((work) => entityManager.transaction(work)); + return await transaction(async (transactionalEntityManager) => { + const experiment = await this.experimentRepository.findOneExperiment(experimentId, transactionalEntityManager); if (experiment) { await this.clearExperimentCacheDetail(experiment.context[0]); @@ -447,7 +453,12 @@ export class ExperimentService { }; // Add log for experiment deleted - this.experimentAuditLogRepository.saveRawJson(LOG_TYPE.EXPERIMENT_DELETED, deleteAuditLogData, currentUser); + await this.experimentAuditLogRepository.saveRawJson( + LOG_TYPE.EXPERIMENT_DELETED, + deleteAuditLogData, + currentUser, + transactionalEntityManager + ); await Promise.all( experiment.experimentSegmentInclusion.map(async (segmentInclusion) => { diff --git a/packages/backend/src/api/services/FeatureFlagPrecomputedSegmentService.ts b/packages/backend/src/api/services/FeatureFlagPrecomputedSegmentService.ts index 0519419db7..dcbc1ba2d9 100644 --- a/packages/backend/src/api/services/FeatureFlagPrecomputedSegmentService.ts +++ b/packages/backend/src/api/services/FeatureFlagPrecomputedSegmentService.ts @@ -6,6 +6,8 @@ import { FeatureFlagSegmentExclusionRepository } from '../repositories/FeatureFl import { FeatureFlagRepository } from '../repositories/FeatureFlagRepository'; import { SegmentRepository } from '../repositories/SegmentRepository'; import { FeatureFlagPrecomputedSegment } from '../models/FeatureFlagPrecomputedSegment'; +import { FeatureFlagSegmentInclusion } from '../models/FeatureFlagSegmentInclusion'; +import { FeatureFlagSegmentExclusion } from '../models/FeatureFlagSegmentExclusion'; import { CacheService } from './CacheService'; import { CACHE_PREFIX } from 'upgrade_types'; import { UpgradeLogger } from '../../lib/logger/UpgradeLogger'; @@ -56,13 +58,19 @@ export class FeatureFlagPrecomputedSegmentService extends PrecomputedSegmentServ }; } - protected async findOwnerIdsBySegmentId(segmentId: string): Promise { + protected async findOwnerIdsBySegmentId(segmentId: string, entityManager?: EntityManager): Promise { + const inclusionRepository = entityManager + ? entityManager.getRepository(FeatureFlagSegmentInclusion) + : this.featureFlagSegmentInclusionRepository; + const exclusionRepository = entityManager + ? entityManager.getRepository(FeatureFlagSegmentExclusion) + : this.featureFlagSegmentExclusionRepository; const [inclusionRecords, exclusionRecords] = await Promise.all([ - this.featureFlagSegmentInclusionRepository.find({ + inclusionRepository.find({ where: { segment: { id: segmentId } }, relations: { featureFlag: true }, }), - this.featureFlagSegmentExclusionRepository.find({ + exclusionRepository.find({ where: { segment: { id: segmentId } }, relations: { featureFlag: true }, }), @@ -101,8 +109,8 @@ export class FeatureFlagPrecomputedSegmentService extends PrecomputedSegmentServ this.scheduleRecomputeForOwners(flagIds, logger); } - public getAffectedFlagIds(segmentId: string): Promise { - return this.getAffectedOwnerIds(segmentId); + public getAffectedFlagIds(segmentId: string, entityManager?: EntityManager): Promise { + return this.getAffectedOwnerIds(segmentId, entityManager); } public recomputeAllFlags(logger: UpgradeLogger): Promise { diff --git a/packages/backend/src/api/services/FeatureFlagService.ts b/packages/backend/src/api/services/FeatureFlagService.ts index e4742b89ff..f2ba94bf59 100644 --- a/packages/backend/src/api/services/FeatureFlagService.ts +++ b/packages/backend/src/api/services/FeatureFlagService.ts @@ -1,8 +1,6 @@ import { Service } from 'typedi'; import { FeatureFlag } from '../models/FeatureFlag'; import { Segment } from '../models/Segment'; -import { IndividualForSegment } from '../models/IndividualForSegment'; -import { GroupForSegment } from '../models/GroupForSegment'; import { FeatureFlagSegmentInclusion } from '../models/FeatureFlagSegmentInclusion'; import { FeatureFlagSegmentExclusion } from '../models/FeatureFlagSegmentExclusion'; import { FeatureFlagPrecomputedSegment } from '../models/FeatureFlagPrecomputedSegment'; @@ -60,6 +58,7 @@ import { NotFoundException } from '@nestjs/common/exceptions'; import { CacheService } from './CacheService'; import { FeatureFlagPrecomputedSegmentService, precomputedGroupKey } from './FeatureFlagPrecomputedSegmentService'; import { EntitySegmentResolutionInput } from '../../types'; +import { DeletionTransaction } from '../../types/DeletionTransaction'; import { SegmentFile, SegmentInputValidator } from '../controllers/validators/SegmentInputValidator'; import dayjs from 'dayjs'; import { getDateRangeNames } from '../repositories/utils/dateQuery'; @@ -184,63 +183,15 @@ export class FeatureFlagService { // Counts-only variant of findOne for the details page: maps member counts instead of loading // the member lists. Callers that need the actual members (e.g. exports) must use findOne. - public async findOneForDetails(id: string, logger?: UpgradeLogger): Promise { + public async findOneForDetails( + id: string, + logger?: UpgradeLogger, + entityManager?: EntityManager + ): Promise { if (logger) { logger.info({ message: `Find feature flag (details view) by id => ${id}` }); } - const featureFlag = await this.featureFlagRepository - .createQueryBuilder('feature_flag') - .leftJoinAndSelect('feature_flag.featureFlagSegmentInclusion', 'featureFlagSegmentInclusion') - .leftJoinAndSelect('featureFlagSegmentInclusion.segment', 'segmentInclusion') - .leftJoinAndSelect('segmentInclusion.subSegments', 'subSegment') - .leftJoinAndSelect('feature_flag.featureFlagSegmentExclusion', 'featureFlagSegmentExclusion') - .leftJoinAndSelect('featureFlagSegmentExclusion.segment', 'segmentExclusion') - .leftJoinAndSelect('segmentExclusion.subSegments', 'subSegmentExclusion') - .where({ id }) - .getOne(); - - if (!featureFlag) { - return undefined; - } - - // loadRelationCountAndMap was removed in TypeORM 1.0; fetch member counts with two batch queries. - const segments = [ - ...(featureFlag.featureFlagSegmentInclusion ?? []).map((r) => r.segment), - ...(featureFlag.featureFlagSegmentExclusion ?? []).map((r) => r.segment), - ].filter(Boolean); - - if (segments.length > 0) { - const segmentIds = segments.map((s) => s.id); - - const [individualCounts, groupCounts] = await Promise.all([ - this.dataSource - .createQueryBuilder() - .select('ifs.segmentId', 'segmentId') - .addSelect('COUNT(*)', 'count') - .from(IndividualForSegment, 'ifs') - .where('ifs.segmentId IN (:...segmentIds)', { segmentIds }) - .groupBy('ifs.segmentId') - .getRawMany<{ segmentId: string; count: string }>(), - this.dataSource - .createQueryBuilder() - .select('gfs.segmentId', 'segmentId') - .addSelect('COUNT(*)', 'count') - .from(GroupForSegment, 'gfs') - .where('gfs.segmentId IN (:...segmentIds)', { segmentIds }) - .groupBy('gfs.segmentId') - .getRawMany<{ segmentId: string; count: string }>(), - ]); - - const individualCountMap = new Map(individualCounts.map((r) => [r.segmentId, Number.parseInt(r.count, 10)])); - const groupCountMap = new Map(groupCounts.map((r) => [r.segmentId, Number.parseInt(r.count, 10)])); - - segments.forEach((segment) => { - segment.individualForSegmentCount = individualCountMap.get(segment.id) ?? 0; - segment.groupForSegmentCount = groupCountMap.get(segment.id) ?? 0; - }); - } - - return featureFlag; + return this.featureFlagRepository.findOneForDetails(id, entityManager); } public async create( @@ -342,11 +293,13 @@ export class FeatureFlagService { public async delete( featureFlagId: string, currentUser: UserDTO, - logger: UpgradeLogger + logger: UpgradeLogger, + executeTransaction?: DeletionTransaction ): Promise { logger.info({ message: `Delete Feature Flag => ${featureFlagId}` }); - return await this.dataSource.transaction(async (transactionalEntityManager) => { - const featureFlag = await this.findOneForDetails(featureFlagId, logger); + const transaction: DeletionTransaction = executeTransaction || ((work) => this.dataSource.transaction(work)); + return await transaction(async (transactionalEntityManager) => { + const featureFlag = await this.findOneForDetails(featureFlagId, logger, transactionalEntityManager); if (featureFlag) { await this.clearCachedFlagsForContext(featureFlag.context[0]); @@ -381,7 +334,8 @@ export class FeatureFlagService { await this.experimentAuditLogRepository.saveRawJson( LOG_TYPE.FEATURE_FLAG_DELETED, createAuditLogData, - currentUser + currentUser, + transactionalEntityManager ); return deletedFlag; } diff --git a/packages/backend/src/api/services/PrecomputedSegmentServiceBase.ts b/packages/backend/src/api/services/PrecomputedSegmentServiceBase.ts index d268b660a4..5d3dac473a 100644 --- a/packages/backend/src/api/services/PrecomputedSegmentServiceBase.ts +++ b/packages/backend/src/api/services/PrecomputedSegmentServiceBase.ts @@ -3,6 +3,7 @@ import { SegmentRepository } from '../repositories/SegmentRepository'; import { CACHE_PREFIX } from 'upgrade_types'; import { UpgradeLogger } from '../../lib/logger/UpgradeLogger'; import { flattenSegmentMembers } from './precomputedSegmentHelpers'; +import { EntityManager } from 'typeorm'; export interface PrecomputedSegmentRow { inclusionIds: string[]; @@ -43,7 +44,7 @@ export abstract class PrecomputedSegmentServiceBase; /** owner IDs that DIRECTLY reference a given segment via inclusion or exclusion */ - protected abstract findOwnerIdsBySegmentId(segmentId: string): Promise; + protected abstract findOwnerIdsBySegmentId(segmentId: string, entityManager?: EntityManager): Promise; /** persist the flat arrays for one owner (subclass repo upsert) */ protected abstract upsertOwner(ownerId: string, inclusionIds: string[], exclusionIds: string[]): Promise; @@ -170,20 +171,24 @@ export abstract class PrecomputedSegmentServiceBase { - return [...(await this.collectAffectedOwnerIds(segmentId, new Set()))]; + public async getAffectedOwnerIds(segmentId: string, entityManager?: EntityManager): Promise { + return [...(await this.collectAffectedOwnerIds(segmentId, new Set(), entityManager))]; } - protected async collectAffectedOwnerIds(segmentId: string, visited: Set): Promise> { + protected async collectAffectedOwnerIds( + segmentId: string, + visited: Set, + entityManager?: EntityManager + ): Promise> { if (visited.has(segmentId)) return new Set(); visited.add(segmentId); - const ownerIds = new Set(await this.findOwnerIdsBySegmentId(segmentId)); + const ownerIds = new Set(await this.findOwnerIdsBySegmentId(segmentId, entityManager)); - const parentIds = await this.segmentRepository.findParentSegmentIds(segmentId); + const parentIds = await this.segmentRepository.findParentSegmentIds(segmentId, entityManager); await Promise.all( parentIds.map(async (parentId) => { - const parentOwnerIds = await this.collectAffectedOwnerIds(parentId, visited); + const parentOwnerIds = await this.collectAffectedOwnerIds(parentId, visited, entityManager); parentOwnerIds.forEach((id) => ownerIds.add(id)); }) ); diff --git a/packages/backend/src/api/services/SegmentService.ts b/packages/backend/src/api/services/SegmentService.ts index a235f97a07..715e15f24d 100644 --- a/packages/backend/src/api/services/SegmentService.ts +++ b/packages/backend/src/api/services/SegmentService.ts @@ -18,6 +18,7 @@ import { normalizeStandardListType, } from 'upgrade_types'; import { EntityManager, DataSource, Not, In } from 'typeorm'; +import { DeletionTransaction } from '../../types/DeletionTransaction'; import Papa from 'papaparse'; import { env } from '../../env'; @@ -535,21 +536,51 @@ export class SegmentService { return this.addSegmentDataWithPipeline(segment, logger, transactionalEntityManager, skipScheduleRecompute); } - public async deleteSegment(id: string, logger: UpgradeLogger): Promise { + public async deleteSegment( + id: string, + logger: UpgradeLogger, + executeTransaction?: DeletionTransaction + ): Promise { logger.info({ message: `Delete segment by id. segmentId: ${id}` }); - // Both flags and experiments can reference this segment, so both precomputed tables must be - // refreshed. The affected experiment IDs must be collected BEFORE the delete (the join rows are - // gone after), so resolve them here; the fire-and-forget recompute is fired after the delete - // transaction below commits. Flags use withRecompute, which enforces the same - // resolve-before -> delete -> recompute-after ordering internally. - const affectedExperimentIds = await this.experimentPrecomputedSegmentService.getAffectedExperimentIds(id); + const transaction: DeletionTransaction = executeTransaction || ((work) => this.dataSource.transaction(work)); + + if (executeTransaction) { + let affectedFlagIds: string[]; + let affectedExperimentIds: string[]; + const deletedSegment = await transaction(async (transactionalEntityManager) => { + // The batch executor holds the target lock before invoking this callback. + // Collect owners before deletion removes the joins, including edits committed while waiting for the lock. + affectedFlagIds = await this.featureFlagPrecomputedSegmentService.getAffectedFlagIds( + id, + transactionalEntityManager + ); + affectedExperimentIds = await this.experimentPrecomputedSegmentService.getAffectedExperimentIds( + id, + transactionalEntityManager + ); + return this.deleteSegmentAndPrivateSubsegments(id, logger, transactionalEntityManager); + }); + // A batch must finish this item's post-commit writes before deleting another segment for the same owner. + const updates = await Promise.allSettled([ + ...affectedFlagIds.map((flagId) => this.featureFlagPrecomputedSegmentService.recomputeForFlag(flagId, logger)), + ...affectedExperimentIds.map((experimentId) => + this.experimentPrecomputedSegmentService.recomputeForExperiment(experimentId, logger) + ), + this.cacheService.resetPrefixCache(CACHE_PREFIX.SEGMENT_KEY_PREFIX), + this.cacheService.resetPrefixCache(CACHE_PREFIX.GLOBAL_EXCLUDE_SEGMENT_KEY_PREFIX), + ]); + const failure = updates.find((update): update is PromiseRejectedResult => update.status === 'rejected'); + if (failure) throw failure.reason; + return deletedSegment; + } + const affectedExperimentIds = await this.experimentPrecomputedSegmentService.getAffectedExperimentIds(id); const deletedSegment = await this.featureFlagPrecomputedSegmentService.withRecompute( logger, () => this.featureFlagPrecomputedSegmentService.getAffectedFlagIds(id), () => - this.dataSource.transaction((transactionalEntityManager) => + transaction((transactionalEntityManager) => this.deleteSegmentAndPrivateSubsegments(id, logger, transactionalEntityManager) ) ); diff --git a/packages/backend/src/types/DeletionTransaction.ts b/packages/backend/src/types/DeletionTransaction.ts new file mode 100644 index 0000000000..a5f191cbff --- /dev/null +++ b/packages/backend/src/types/DeletionTransaction.ts @@ -0,0 +1,4 @@ +import { EntityManager } from 'typeorm'; + +/** Optional internal transaction boundary; ordinary deletion callers retain their existing behavior. */ +export type DeletionTransaction = (work: (manager: EntityManager) => Promise) => Promise; diff --git a/packages/backend/test/integration/BatchDelete/index.ts b/packages/backend/test/integration/BatchDelete/index.ts new file mode 100644 index 0000000000..24b2d1680d --- /dev/null +++ b/packages/backend/test/integration/BatchDelete/index.ts @@ -0,0 +1,674 @@ +import { randomUUID } from 'crypto'; +import { Application } from 'express'; +import request from 'supertest'; +import Container from 'typedi'; +import { createExpressServer } from 'routing-controllers'; +import { DataSource, In } from 'typeorm'; +import { + ASSIGNMENT_ALGORITHM, + ASSIGNMENT_UNIT, + BatchDeleteEntity, + CACHE_PREFIX, + DeletionReasonCode, + EXPERIMENT_STATE, + EXPERIMENT_STATE_INTERNAL_NAME_OVERRIDES, + FEATURE_FLAG_STATUS, + LOG_TYPE, + POST_EXPERIMENT_RULE, + SEGMENT_TYPE, + STANDARD_LIST_TYPE, + SYSTEM_USER_EMAIL, + UserRole, +} from 'upgrade_types'; +import { ExperimentController } from '../../../src/api/controllers/ExperimentController'; +import { FeatureFlagsController } from '../../../src/api/controllers/FeatureFlagController'; +import { SegmentController } from '../../../src/api/controllers/SegmentController'; +import { ErrorHandlerMiddleware } from '../../../src/api/middlewares/ErrorHandlerMiddleware'; +import { LogMiddleware } from '../../../src/api/middlewares/LogMiddleware'; +import { ConditionPosteriorState } from '../../../src/api/models/ConditionPosteriorState'; +import { Experiment } from '../../../src/api/models/Experiment'; +import { ExperimentAuditLog } from '../../../src/api/models/ExperimentAuditLog'; +import { ExperimentCondition } from '../../../src/api/models/ExperimentCondition'; +import { ExperimentPrecomputedSegment } from '../../../src/api/models/ExperimentPrecomputedSegment'; +import { ExperimentSegmentInclusion } from '../../../src/api/models/ExperimentSegmentInclusion'; +import { FeatureFlag } from '../../../src/api/models/FeatureFlag'; +import { FeatureFlagPrecomputedSegment } from '../../../src/api/models/FeatureFlagPrecomputedSegment'; +import { FeatureFlagSegmentInclusion } from '../../../src/api/models/FeatureFlagSegmentInclusion'; +import { IndividualForSegment } from '../../../src/api/models/IndividualForSegment'; +import { Segment } from '../../../src/api/models/Segment'; +import { ThompsonSamplingExperimentConfig } from '../../../src/api/models/ThompsonSamplingExperimentConfig'; +import { ThompsonSamplingReward } from '../../../src/api/models/ThompsonSamplingReward'; +import { User } from '../../../src/api/models/User'; +import { DeletionRepository } from '../../../src/api/repositories/DeletionRepository'; +import { CacheService } from '../../../src/api/services/CacheService'; +import { ExperimentPrecomputedSegmentService } from '../../../src/api/services/ExperimentPrecomputedSegmentService'; +import { FeatureFlagPrecomputedSegmentService } from '../../../src/api/services/FeatureFlagPrecomputedSegmentService'; +import { SegmentService } from '../../../src/api/services/SegmentService'; +import { BatchDeleteService } from '../../../src/api/services/BatchDeleteService'; +import { currentUserChecker } from '../../../src/auth/currentUserChecker'; +import { env } from '../../../src/env'; +import { UpgradeLogger } from '../../../src/lib/logger/UpgradeLogger'; +import { iocLoader } from '../../../src/loaders/iocLoader'; + +const entities: BatchDeleteEntity[] = ['experiments', 'flags', 'segments']; +const model = { experiments: Experiment, flags: FeatureFlag, segments: Segment }; +const route = (entity: BatchDeleteEntity) => `/api/${entity}/batch-delete`; + +export function registerBatchDeleteTests(connections: () => [DataSource, DataSource]) { + describe('Batch deletion', () => { + let db: DataSource; + let writer: DataSource; + let app: Application; + let originalAuth: boolean; + beforeAll(() => { + iocLoader(); + const { authorizationChecker } = jest.requireActual('../../../src/auth/authorizationChecker'); + app = createExpressServer({ + routePrefix: '/api', + controllers: [ExperimentController, FeatureFlagsController, SegmentController], + middlewares: [LogMiddleware, ErrorHandlerMiddleware], + classTransformer: true, + validation: { whitelist: true, validationError: { target: false, value: false } }, + defaultErrorHandler: false, + authorizationChecker: authorizationChecker(), + currentUserChecker, + }); + }); + beforeEach(() => { + [db, writer] = connections(); + originalAuth = env.google.authTokenRequired; + env.google.authTokenRequired = false; + }); + afterEach(() => { + env.google.authTokenRequired = originalAuth; + jest.restoreAllMocks(); + }); + + const create = async (entity: BatchDeleteEntity, count = 1) => { + const rows = Array.from({ length: count }, (_, i) => ({ id: randomUUID(), name: `Batch ${i}`, context: 'home' })); + if (entity === 'experiments') { + await db.getRepository(Experiment).insert( + rows.map((row) => ({ + ...row, + context: [row.context], + description: '', + assignmentUnit: ASSIGNMENT_UNIT.INDIVIDUAL, + postExperimentRule: POST_EXPERIMENT_RULE.CONTINUE, + })) + ); + } else if (entity === 'flags') { + await db.getRepository(FeatureFlag).insert( + rows.map((row) => ({ + ...row, + context: [row.context], + key: row.id, + description: '', + })) + ); + } else await db.getRepository(Segment).insert(rows.map((row) => ({ ...row, type: SEGMENT_TYPE.PUBLIC }))); + return rows; + }; + const nativeExperimentData = async (experimentId: string) => { + const conditionId = randomUUID(); + await db.getRepository(Experiment).update(experimentId, { + assignmentAlgorithm: ASSIGNMENT_ALGORITHM.THOMPSON_SAMPLING, + }); + await db.getRepository(ExperimentCondition).insert({ + id: conditionId, + experiment: { id: experimentId }, + conditionCode: 'native-condition', + assignmentWeight: 100, + }); + await db.getRepository(ThompsonSamplingExperimentConfig).insert({ experimentId }); + await db.getRepository(ConditionPosteriorState).insert({ conditionId, successCount: 1, totalCount: 1 }); + await db.getRepository(ThompsonSamplingReward).insert({ conditionId, userId: 'batch-member', success: true }); + return conditionId; + }; + const ownedList = async (entity: BatchDeleteEntity, ownerId: string, childId?: string, connection = db) => { + const list = { + id: randomUUID(), + name: 'Owned list', + context: 'home', + type: SEGMENT_TYPE.PRIVATE, + listType: childId ? STANDARD_LIST_TYPE.SEGMENT : STANDARD_LIST_TYPE.INDIVIDUAL, + }; + await connection.getRepository(Segment).insert(list); + await connection.getRepository(IndividualForSegment).insert({ segmentId: list.id, userId: 'batch-member' }); + if (childId) await connection.createQueryBuilder().relation(Segment, 'subSegments').of(list.id).add(childId); + if (entity === 'experiments') { + await connection + .getRepository(ExperimentSegmentInclusion) + .insert({ experimentId: ownerId, segmentId: list.id }); + } else if (entity === 'flags') { + await connection.getRepository(FeatureFlagSegmentInclusion).insert({ + featureFlagId: ownerId, + segmentId: list.id, + listType: list.listType, + enabled: false, + }); + } else await connection.createQueryBuilder().relation(Segment, 'subSegments').of(ownerId).add(list.id); + return list; + }; + + test.each(entities)('%s validates IDs and follows single-delete authentication', async (entity) => { + const spy = jest.spyOn(Container.get(BatchDeleteService), 'delete'); + const id = randomUUID(); + for (const body of [ + {}, + { ids: null }, + { ids: id }, + { ids: [] }, + { ids: [null] }, + { ids: [123] }, + { ids: ['bad-id'] }, + { ids: [id, id] }, + { ids: [id, id.toUpperCase()] }, + ]) { + await request(app).post(route(entity)).send(body).expect(400); + } + env.google.authTokenRequired = true; + await request(app) + .post(route(entity)) + .send({ ids: [id] }) + .expect(401); + expect(spy).not.toHaveBeenCalled(); + + env.google.authTokenRequired = false; + await db.getRepository(User).delete({ email: SYSTEM_USER_EMAIL }); + const [single, batch] = await create(entity, 2); + await request(app).delete(`/api/${entity}/${single.id}`).expect(200); + const { body } = await request(app) + .post(route(entity)) + .send({ ids: [batch.id] }) + .expect(200); + expect(body.results).toEqual([{ id: batch.id, outcome: 'deleted' }]); + expect(await db.getRepository(model[entity]).countBy({ id: In([single.id, batch.id]) })).toBe(0); + }); + + test.each(Object.values(UserRole))('matches single-delete behavior for the %s role', async (role) => { + await db.getRepository(User).update(SYSTEM_USER_EMAIL, { role }); + for (const entity of entities) { + const [single, batch] = await create(entity, 2); + await request(app).delete(`/api/${entity}/${single.id}`).expect(200); + const { body } = await request(app) + .post(route(entity)) + .send({ ids: [batch.id] }) + .expect(200); + expect(body.results).toEqual([{ id: batch.id, outcome: 'deleted' }]); + expect(await db.getRepository(model[entity]).countBy({ id: In([single.id, batch.id]) })).toBe(0); + } + }); + + test.each(entities)('%s deletes owned lists and members while preserving public children', async (entity) => { + const rows = await create(entity, 2); + const [child] = await create('segments'); + const lists = await Promise.all(rows.map((row) => ownedList(entity, row.id, child.id))); + const ids = rows.map((row) => row.id.toUpperCase()).reverse(); + const { body } = await request(app).post(route(entity)).send({ ids }).expect(200); + expect(body).toEqual({ results: ids.map((id) => ({ id, outcome: 'deleted' })) }); + expect(await db.getRepository(model[entity]).countBy({ id: In(ids) })).toBe(0); + expect(await db.getRepository(Segment).countBy({ id: In(lists.map((list) => list.id)) })).toBe(0); + expect( + await db.getRepository(IndividualForSegment).countBy({ segmentId: In(lists.map((list) => list.id)) }) + ).toBe(0); + expect(await db.getRepository(Segment).countBy({ id: child.id })).toBe(1); + }); + + test.each(entities)('%s deletes with one pooled connection', async (entity) => { + const [row] = await create(entity); + const list = await ownedList(entity, row.id); + let flagId: string; + let experimentId: string; + if (entity === 'segments') { + const [flag] = await create('flags'); + const [experiment] = await create('experiments'); + flagId = flag.id; + experimentId = experiment.id; + const flagList = await ownedList('flags', flagId, row.id); + await db.getRepository(FeatureFlagSegmentInclusion).update({ segmentId: flagList.id }, { enabled: true }); + await ownedList('experiments', experimentId, row.id); + await db.getRepository(IndividualForSegment).insert({ segmentId: row.id, userId: 'deleted-member' }); + const logger = new UpgradeLogger(); + await Container.get(FeatureFlagPrecomputedSegmentService).recomputeForFlag(flagId, logger); + await Container.get(ExperimentPrecomputedSegmentService).recomputeForExperiment(experimentId, logger); + expect( + await db.getRepository(FeatureFlagPrecomputedSegment).findOneBy({ featureFlagId: flagId }) + ).toMatchObject({ + inclusionIds: expect.arrayContaining(['deleted-member']), + }); + expect(await db.getRepository(ExperimentPrecomputedSegment).findOneBy({ experimentId })).toMatchObject({ + inclusionIds: expect.arrayContaining(['deleted-member']), + }); + } + const originalExtra = db.options.extra; + await db.destroy(); + db.setOptions({ extra: { ...originalExtra, max: 1, connectionTimeoutMillis: 500 } }); + try { + await db.initialize(); + if (entity === 'flags') { + const { body: details } = await request(app).get(`/api/flags/${row.id}`).expect(200); + expect(details.featureFlagSegmentInclusion[0].segment).toMatchObject({ + id: list.id, + individualForSegmentCount: 1, + groupForSegmentCount: 0, + }); + } + const { body } = await request(app) + .post(route(entity)) + .send({ ids: [row.id] }) + .expect(200); + expect(body.results).toEqual([{ id: row.id, outcome: 'deleted' }]); + expect(await db.getRepository(model[entity]).countBy({ id: row.id })).toBe(0); + expect(await db.getRepository(Segment).countBy({ id: list.id })).toBe(0); + if (entity === 'segments') { + expect( + await db.getRepository(FeatureFlagPrecomputedSegment).findOneBy({ featureFlagId: flagId }) + ).toMatchObject({ + inclusionIds: ['batch-member'], + exclusionIds: [], + }); + expect(await db.getRepository(ExperimentPrecomputedSegment).findOneBy({ experimentId })).toMatchObject({ + inclusionIds: ['batch-member'], + exclusionIds: [], + }); + } + } finally { + if (db.isInitialized) await db.destroy(); + db.setOptions({ extra: originalExtra }); + await db.initialize(); + } + }); + + test.each(entities)('%s skips a missing ID and deletes the remaining selection', async (entity) => { + const [row] = await create(entity); + const missing = randomUUID(); + const { body } = await request(app) + .post(route(entity)) + .send({ ids: [row.id, missing] }) + .expect(200); + expect(body).toEqual({ + results: [ + { id: row.id, outcome: 'deleted' }, + { id: missing, outcome: 'not_found', reasonCode: DeletionReasonCode.NOT_FOUND }, + ], + }); + expect(await db.getRepository(model[entity]).countBy({ id: row.id })).toBe(0); + }); + + test.each(['cleanup', 'commit'])('rolls back flag deletion when %s fails', async (failureStage) => { + const rows = await create('flags', 3); + const list = await ownedList('flags', rows[1].id); + const atCommit = failureStage === 'commit'; + try { + await db.query(`CREATE OR REPLACE FUNCTION batch_test_reject_delete() RETURNS trigger AS $$ + BEGIN IF OLD.id = TG_ARGV[0]::uuid THEN RAISE EXCEPTION 'batch deletion failure'; END IF; + RETURN OLD; END; $$ LANGUAGE plpgsql`); + await db.query('DROP TRIGGER IF EXISTS batch_test_reject_delete ON segment'); + // Deferring this test-only trigger forces a commit failure after the audit has been written. + const trigger = atCommit + ? 'CREATE CONSTRAINT TRIGGER batch_test_reject_delete AFTER DELETE ON segment DEFERRABLE INITIALLY DEFERRED' + : 'CREATE TRIGGER batch_test_reject_delete BEFORE DELETE ON segment'; + // The interpolated UUID is generated only by this fixture. + await db.query(`${trigger} FOR EACH ROW EXECUTE FUNCTION batch_test_reject_delete('${list.id}')`); + const { body } = await request(app) + .post(route('flags')) + .send({ ids: rows.map((row) => row.id) }) + .expect(200); + expect(body).toEqual({ + results: [ + { id: rows[0].id, outcome: 'deleted' }, + { + id: rows[1].id, + outcome: atCommit ? 'unknown' : 'failed', + reasonCode: atCommit ? DeletionReasonCode.OUTCOME_UNKNOWN : DeletionReasonCode.DELETE_FAILED, + }, + { id: rows[2].id, outcome: 'not_attempted' }, + ], + }); + expect(await db.getRepository(FeatureFlag).countBy({ id: rows[0].id })).toBe(0); + expect(await db.getRepository(FeatureFlag).countBy({ id: In(rows.slice(1).map((row) => row.id)) })).toBe(2); + expect(await db.getRepository(Segment).countBy({ id: list.id })).toBe(1); + expect(await db.getRepository(IndividualForSegment).countBy({ segmentId: list.id })).toBe(1); + expect(await db.getRepository(FeatureFlagSegmentInclusion).countBy({ segmentId: list.id })).toBe(1); + expect(await db.getRepository(ExperimentAuditLog).countBy({ type: LOG_TYPE.FEATURE_FLAG_DELETED })).toBe(1); + } finally { + await db.query('DROP TRIGGER IF EXISTS batch_test_reject_delete ON segment'); + await db.query('DROP FUNCTION IF EXISTS batch_test_reject_delete()'); + } + }); + + test('rolls back an experiment after an audit insert failure and stops the remaining items', async () => { + const rows = await create('experiments', 3); + const list = await ownedList('experiments', rows[1].id); + const conditionId = await nativeExperimentData(rows[1].id); + try { + await db.query(`CREATE OR REPLACE FUNCTION batch_test_reject_audit() RETURNS trigger AS $$ + BEGIN IF NEW.data->>'experimentId' = TG_ARGV[0] THEN RAISE EXCEPTION 'batch audit failure'; END IF; + RETURN NEW; END; $$ LANGUAGE plpgsql`); + // The interpolated UUID is generated only by this fixture. + await db.query(`CREATE TRIGGER batch_test_reject_audit BEFORE INSERT ON experiment_audit_log + FOR EACH ROW EXECUTE FUNCTION batch_test_reject_audit('${rows[1].id}')`); + const { body } = await request(app) + .post(route('experiments')) + .send({ ids: rows.map((row) => row.id) }) + .expect(200); + expect(body.results).toEqual([ + { id: rows[0].id, outcome: 'deleted' }, + { id: rows[1].id, outcome: 'failed', reasonCode: DeletionReasonCode.DELETE_FAILED }, + { id: rows[2].id, outcome: 'not_attempted' }, + ]); + expect(await db.getRepository(Experiment).countBy({ id: rows[0].id })).toBe(0); + expect(await db.getRepository(Experiment).countBy({ id: In(rows.slice(1).map((row) => row.id)) })).toBe(2); + expect(await db.getRepository(Segment).countBy({ id: list.id })).toBe(1); + expect(await db.getRepository(ExperimentSegmentInclusion).countBy({ segmentId: list.id })).toBe(1); + expect(await db.getRepository(ExperimentAuditLog).countBy({ type: LOG_TYPE.EXPERIMENT_DELETED })).toBe(1); + expect(await db.getRepository(ExperimentCondition).countBy({ id: conditionId })).toBe(1); + expect(await db.getRepository(ThompsonSamplingExperimentConfig).countBy({ experimentId: rows[1].id })).toBe(1); + expect(await db.getRepository(ConditionPosteriorState).findOneBy({ conditionId })).toMatchObject({ + successCount: 1, + totalCount: 1, + }); + expect(await db.getRepository(ThompsonSamplingReward).findOneBy({ conditionId })).toMatchObject({ + userId: 'batch-member', + success: true, + }); + } finally { + await db.query('DROP TRIGGER IF EXISTS batch_test_reject_audit ON experiment_audit_log'); + await db.query('DROP FUNCTION IF EXISTS batch_test_reject_audit()'); + } + }); + + test('reports a database-configured lock timeout without deleting', async () => { + const [row] = await create('flags'); + const blocker = writer.createQueryRunner(); + await blocker.connect(); + await blocker.startTransaction(); + try { + await blocker.query('SELECT id FROM feature_flag WHERE id = $1 FOR UPDATE', [row.id]); + const original = db.createQueryRunner.bind(db); + jest.spyOn(db, 'createQueryRunner').mockImplementation((...args) => { + const runner = original(...args); + const start = runner.startTransaction.bind(runner); + jest.spyOn(runner, 'startTransaction').mockImplementation(async (...startArgs) => { + await start(...startArgs); + // Model a database-configured limit; the deletion service does not set its own timeout. + await runner.query("SET LOCAL lock_timeout = '100ms'"); + }); + return runner; + }); + const { body } = await request(app) + .post(route('flags')) + .send({ ids: [row.id] }) + .expect(200); + expect(body.results).toEqual([{ id: row.id, outcome: 'failed', reasonCode: DeletionReasonCode.LOCK_TIMEOUT }]); + expect(await db.getRepository(FeatureFlag).countBy({ id: row.id })).toBe(1); + } finally { + await blocker.rollbackTransaction(); + await blocker.release(); + } + }); + + test.each([false, true])('updates shared owners after segment deletion (recompute fails: %s)', async (fails) => { + const rows = await create('segments', 2); + const [flag] = await create('flags'); + const [experiment] = await create('experiments'); + const members = rows.map((row) => `member-${row.id}`); + await db + .getRepository(IndividualForSegment) + .insert(rows.map((row, index) => ({ segmentId: row.id, userId: members[index] }))); + await db.getRepository(FeatureFlagSegmentInclusion).insert( + rows.map((row) => ({ + featureFlagId: flag.id, + segmentId: row.id, + enabled: true, + listType: STANDARD_LIST_TYPE.SEGMENT, + })) + ); + await db + .getRepository(ExperimentSegmentInclusion) + .insert(rows.map((row) => ({ experimentId: experiment.id, segmentId: row.id }))); + const flags = Container.get(FeatureFlagPrecomputedSegmentService); + const experiments = Container.get(ExperimentPrecomputedSegmentService); + const logger = new UpgradeLogger(); + await flags.recomputeForFlag(flag.id, logger); + await experiments.recomputeForExperiment(experiment.id, logger); + if (fails) jest.spyOn(flags, 'recomputeForFlag').mockRejectedValueOnce(new Error('Recompute failed')); + + const { body } = await request(app) + .post(route('segments')) + .send({ ids: rows.map((row) => row.id) }) + .expect(200); + if (fails) { + expect(body.results).toEqual([ + { id: rows[0].id, outcome: 'deleted', reasonCode: DeletionReasonCode.POST_DELETE_FAILED }, + { id: rows[1].id, outcome: 'not_attempted' }, + ]); + expect(await db.getRepository(Segment).countBy({ id: rows[0].id })).toBe(0); + expect(await db.getRepository(Segment).countBy({ id: rows[1].id })).toBe(1); + } else { + expect(body.results).toEqual(rows.map((row) => ({ id: row.id, outcome: 'deleted' }))); + expect( + await db.getRepository(FeatureFlagPrecomputedSegment).findOneBy({ featureFlagId: flag.id }) + ).toMatchObject({ + inclusionIds: [], + exclusionIds: [], + }); + expect( + await db.getRepository(ExperimentPrecomputedSegment).findOneBy({ experimentId: experiment.id }) + ).toMatchObject({ + inclusionIds: [], + exclusionIds: [], + }); + } + }); + + test('recomputes owners attached before the segment deletion lock is acquired', async () => { + const [segment] = await create('segments'); + const [flag] = await create('flags'); + const [experiment] = await create('experiments'); + await db.getRepository(IndividualForSegment).insert({ segmentId: segment.id, userId: 'deleted-member' }); + const flags = Container.get(FeatureFlagPrecomputedSegmentService); + const experiments = Container.get(ExperimentPrecomputedSegmentService); + const logger = new UpgradeLogger(); + const findForDeletion = DeletionRepository.prototype.findForDeletion; + jest + .spyOn(DeletionRepository.prototype, 'findForDeletion') + .mockImplementationOnce(async function (this: DeletionRepository, ...args) { + // Commit a list edit on another connection just before the deletion acquires its target lock. + const flagList = await ownedList('flags', flag.id, segment.id, writer); + await writer.getRepository(FeatureFlagSegmentInclusion).update({ segmentId: flagList.id }, { enabled: true }); + await ownedList('experiments', experiment.id, segment.id, writer); + await flags.recomputeForFlag(flag.id, logger); + await experiments.recomputeForExperiment(experiment.id, logger); + return findForDeletion.apply(this, args); + }); + + const { body } = await request(app) + .post(route('segments')) + .send({ ids: [segment.id] }) + .expect(200); + expect(body.results).toEqual([{ id: segment.id, outcome: 'deleted' }]); + expect(await db.getRepository(Segment).countBy({ id: segment.id })).toBe(0); + expect(await db.getRepository(FeatureFlagPrecomputedSegment).findOneBy({ featureFlagId: flag.id })).toMatchObject( + { inclusionIds: ['batch-member'], exclusionIds: [] } + ); + expect( + await db.getRepository(ExperimentPrecomputedSegment).findOneBy({ experimentId: experiment.id }) + ).toMatchObject({ inclusionIds: ['batch-member'], exclusionIds: [] }); + }); + + test('keeps a committed segment deletion when post-commit cache invalidation fails', async () => { + const rows = await create('segments', 2); + const cache = Container.get(CacheService); + const reset = cache.resetPrefixCache.bind(cache); + let observedCount: number; + jest.spyOn(cache, 'resetPrefixCache').mockImplementation(async (prefix) => { + if (prefix === CACHE_PREFIX.SEGMENT_KEY_PREFIX) { + observedCount = await writer.getRepository(Segment).countBy({ id: rows[0].id }); + throw new Error('Cache unavailable after commit'); + } + return reset(prefix); + }); + const { body } = await request(app) + .post(route('segments')) + .send({ ids: rows.map((row) => row.id) }) + .expect(200); + expect(body.results).toEqual([ + { id: rows[0].id, outcome: 'deleted', reasonCode: DeletionReasonCode.POST_DELETE_FAILED }, + { id: rows[1].id, outcome: 'not_attempted' }, + ]); + expect(observedCount).toBe(0); + expect(await db.getRepository(Segment).countBy({ id: rows[1].id })).toBe(1); + }); + + test.each(['single', 'batch'])( + '%s deletion cascades through native Thompson Sampling config, posteriors and rewards', + async (mode) => { + const [row] = await create('experiments'); + const conditionId = await nativeExperimentData(row.id); + if (mode === 'single') { + const { body } = await request(app).delete(`/api/experiments/${row.id}`).expect(200); + expect(body).toEqual([expect.objectContaining({ id: row.id })]); + } else { + const { body } = await request(app) + .post(route('experiments')) + .send({ ids: [row.id] }) + .expect(200); + expect(body.results).toEqual([{ id: row.id, outcome: 'deleted' }]); + } + expect(await db.getRepository(Experiment).countBy({ id: row.id })).toBe(0); + expect(await db.getRepository(ExperimentCondition).countBy({ id: conditionId })).toBe(0); + expect(await db.getRepository(ThompsonSamplingExperimentConfig).countBy({ experimentId: row.id })).toBe(0); + expect(await db.getRepository(ConditionPosteriorState).countBy({ conditionId })).toBe(0); + expect(await db.getRepository(ThompsonSamplingReward).countBy({ conditionId })).toBe(0); + expect(await db.getRepository(ExperimentAuditLog).countBy({ type: LOG_TYPE.EXPERIMENT_DELETED })).toBe(1); + } + ); + + test.each(entities)( + '%s single-delete route keeps its existing roles, cleanup and success response', + async (entity) => { + await db.getRepository(User).update(SYSTEM_USER_EMAIL, { role: UserRole.READER }); + const [row] = await create(entity); + const [child] = await create('segments'); + const list = await ownedList(entity, row.id, child.id); + const { body } = await request(app).delete(`/api/${entity}/${row.id}`).expect(200); + expect(body).toEqual( + entity === 'segments' ? expect.objectContaining({ id: row.id }) : [expect.objectContaining({ id: row.id })] + ); + expect(await db.getRepository(model[entity]).countBy({ id: row.id })).toBe(0); + expect(await db.getRepository(Segment).countBy({ id: list.id })).toBe(0); + expect(await db.getRepository(IndividualForSegment).countBy({ segmentId: list.id })).toBe(0); + expect(await db.getRepository(Segment).countBy({ id: child.id })).toBe(1); + } + ); + + test.each(['single', 'batch'])('experiment %s deletion allows every experiment state', async (mode) => { + const states = [ + ...new Set( + Object.values(EXPERIMENT_STATE).map((state) => EXPERIMENT_STATE_INTERNAL_NAME_OVERRIDES[state] || state) + ), + ]; + const rows = await create('experiments', states.length); + for (const [index, row] of rows.entries()) { + await db.getRepository(Experiment).update(row.id, { state: states[index] }); + } + if (mode === 'single') { + for (const row of rows) await request(app).delete(`/api/experiments/${row.id}`).expect(200); + } else { + const { body } = await request(app) + .post(route('experiments')) + .send({ ids: rows.map(({ id }) => id) }) + .expect(200); + expect(body.results).toEqual(rows.map(({ id }) => ({ id, outcome: 'deleted' }))); + } + expect(await db.getRepository(Experiment).countBy({ id: In(rows.map(({ id }) => id)) })).toBe(0); + expect(await db.getRepository(ExperimentAuditLog).countBy({ type: LOG_TYPE.EXPERIMENT_DELETED })).toBe( + rows.length + ); + }); + + test.each(['single', 'batch'])('%s deletion allows Enabled flags and Used segments', async (mode) => { + for (const entity of ['flags', 'segments'] as const) { + const [row] = await create(entity); + const list = await ownedList(entity, row.id); + let ownerId: string; + if (entity === 'flags') { + await db.getRepository(FeatureFlag).update(row.id, { status: FEATURE_FLAG_STATUS.ENABLED }); + } else { + const [owner] = await create('flags'); + ownerId = owner.id; + await ownedList('flags', owner.id, row.id); + } + if (mode === 'single') { + await request(app).delete(`/api/${entity}/${row.id}`).expect(200); + } else { + const { body } = await request(app) + .post(route(entity)) + .send({ ids: [row.id] }) + .expect(200); + expect(body.results).toEqual([{ id: row.id, outcome: 'deleted' }]); + } + expect(await db.getRepository(model[entity]).countBy({ id: row.id })).toBe(0); + expect(await db.getRepository(Segment).countBy({ id: list.id })).toBe(0); + expect(await db.getRepository(IndividualForSegment).countBy({ segmentId: list.id })).toBe(0); + if (ownerId) expect(await db.getRepository(FeatureFlag).countBy({ id: ownerId })).toBe(1); + } + }); + + test.each(entities)('%s single-delete preserves its original missing-target response', async (entity) => { + const { body } = await request(app) + .delete(`/api/${entity}/${randomUUID()}`) + .expect(entity === 'segments' ? 500 : 404); + if (entity === 'experiments') expect(body.message).toBe('Experiment not found.'); + }); + + test.each(['single', 'batch'])('%s deletion allows private and global-exclude segments', async (mode) => { + for (const type of [SEGMENT_TYPE.PRIVATE, SEGMENT_TYPE.GLOBAL_EXCLUDE]) { + const [row] = await create('segments'); + await db.getRepository(Segment).update(row.id, { type }); + if (mode === 'single') { + await request(app).delete(`/api/segments/${row.id}`).expect(200); + } else { + const { body } = await request(app) + .post(route('segments')) + .send({ ids: [row.id] }) + .expect(200); + expect(body.results).toEqual([{ id: row.id, outcome: 'deleted' }]); + } + expect(await db.getRepository(Segment).countBy({ id: row.id })).toBe(0); + } + }); + + test.each(entities)( + '%s private-list endpoint still deletes lists and preserves their public children', + async (entity) => { + const [owner] = await create(entity); + const [child] = await create('segments'); + const list = await ownedList(entity, owner.id, child.id); + const path = entity === 'segments' ? 'list' : 'inclusionList'; + await request(app) + .delete(`/api/${entity}/${path}/${list.id}`) + .send(entity === 'segments' ? { parentSegmentId: owner.id } : {}) + .expect(200); + expect(await db.getRepository(model[entity]).countBy({ id: owner.id })).toBe(1); + expect(await db.getRepository(Segment).countBy({ id: child.id })).toBe(1); + expect(await db.getRepository(Segment).countBy({ id: list.id })).toBe(0); + expect(await db.getRepository(IndividualForSegment).countBy({ segmentId: list.id })).toBe(0); + } + ); + + test.each(entities.flatMap((entity) => [20, 100, 500].map((count) => ({ entity, count }))))( + '$entity deletes $count selections without global eligibility reads', + async ({ entity, count }) => { + const rows = await create(entity, count); + const ids = rows.map((row) => row.id).reverse(); + const status = jest.spyOn(Container.get(SegmentService), 'getSegmentStatus'); + const { body } = await request(app).post(route(entity)).send({ ids }).expect(200); + expect(body).toEqual({ results: ids.map((id) => ({ id, outcome: 'deleted' })) }); + expect(status).not.toHaveBeenCalled(); + expect(await db.getRepository(model[entity]).countBy({ id: In(ids) })).toBe(0); + } + ); + }); +} diff --git a/packages/backend/test/integration/index.test.ts b/packages/backend/test/integration/index.test.ts index 2ce690e0b7..267f4f244b 100644 --- a/packages/backend/test/integration/index.test.ts +++ b/packages/backend/test/integration/index.test.ts @@ -124,6 +124,7 @@ import { ExperimentValidation } from './Experiment/validation'; import { FeatureFlagInclusionExclusion } from './FeatureFlags'; import { ListValueFiltering } from './ListValueFiltering'; import { FeatureFlagDeleteCleanup, SegmentDeleteCleanup } from './DeleteCleanup'; +import { registerBatchDeleteTests } from './BatchDelete'; describe('Integration Tests', () => { jest.setTimeout(100000000); @@ -133,6 +134,7 @@ describe('Integration Tests', () => { let defaultConnection: DataSource; let exportConnection: DataSource; + registerBatchDeleteTests(() => [defaultConnection, exportConnection]); beforeAll(async () => { configureLogger(); [defaultConnection, exportConnection] = await createDatabaseConnection(); diff --git a/packages/backend/test/unit/controllers/ExperimentController.test.ts b/packages/backend/test/unit/controllers/ExperimentController.test.ts index 6ca6e745a3..234e3359b5 100644 --- a/packages/backend/test/unit/controllers/ExperimentController.test.ts +++ b/packages/backend/test/unit/controllers/ExperimentController.test.ts @@ -1,3 +1,4 @@ +import { BatchDeleteService } from '../../../src/api/services/BatchDeleteService'; import app from '../../utils/expressApp'; import request from 'supertest'; import { configureLogger } from '../../utils/logger'; @@ -29,6 +30,7 @@ describe('Experiment Controller Testing', () => { configureLogger(); routingUseContainer(Container); classValidatorUseContainer(Container); + Container.set(BatchDeleteService, {} as BatchDeleteService); // set mock container Container.set(ExperimentService, new ExperimentServiceMock()); diff --git a/packages/backend/test/unit/controllers/ExperimentControllerAdaptiveConfig.test.ts b/packages/backend/test/unit/controllers/ExperimentControllerAdaptiveConfig.test.ts index b13f1acc8e..72d3cd13ab 100644 --- a/packages/backend/test/unit/controllers/ExperimentControllerAdaptiveConfig.test.ts +++ b/packages/backend/test/unit/controllers/ExperimentControllerAdaptiveConfig.test.ts @@ -29,7 +29,8 @@ describe('ExperimentController adaptive config wiring', () => { {} as any, {} as any, {} as any, - adaptiveExperimentConfigDispatcher + adaptiveExperimentConfigDispatcher, + {} as any ); }); diff --git a/packages/backend/test/unit/controllers/FeatureFlagController.test.ts b/packages/backend/test/unit/controllers/FeatureFlagController.test.ts index 5708f78414..3cfc4084b8 100644 --- a/packages/backend/test/unit/controllers/FeatureFlagController.test.ts +++ b/packages/backend/test/unit/controllers/FeatureFlagController.test.ts @@ -1,3 +1,4 @@ +import { BatchDeleteService } from '../../../src/api/services/BatchDeleteService'; import app from '../../utils/expressApp'; import request from 'supertest'; import { configureLogger } from '../../utils/logger'; @@ -18,6 +19,7 @@ describe('Feature Flag Controller Testing', () => { configureLogger(); routingUseContainer(Container); classValidatorUseContainer(Container); + Container.set(BatchDeleteService, {} as BatchDeleteService); Container.set(FeatureFlagService, new FeatureFlagServiceMock()); Container.set(ExperimentUserService, new ExperimentUserServiceMock()); diff --git a/packages/backend/test/unit/controllers/SegmentController.test.ts b/packages/backend/test/unit/controllers/SegmentController.test.ts index fa3b6126e8..9c7436ad71 100644 --- a/packages/backend/test/unit/controllers/SegmentController.test.ts +++ b/packages/backend/test/unit/controllers/SegmentController.test.ts @@ -1,3 +1,4 @@ +import { BatchDeleteService } from '../../../src/api/services/BatchDeleteService'; import app from '../../utils/expressApp'; import request from 'supertest'; import { configureLogger } from '../../utils/logger'; @@ -14,6 +15,7 @@ describe('Segment Controller Testing', () => { configureLogger(); routingUseContainer(Container); classValidatorUseContainer(Container); + Container.set(BatchDeleteService, {} as BatchDeleteService); // set mock container Container.set(SegmentService, new SegmentServiceMock()); diff --git a/packages/backend/test/unit/services/BatchDeleteService.test.ts b/packages/backend/test/unit/services/BatchDeleteService.test.ts new file mode 100644 index 0000000000..b09ad4348d --- /dev/null +++ b/packages/backend/test/unit/services/BatchDeleteService.test.ts @@ -0,0 +1,340 @@ +import { randomUUID } from 'crypto'; +import { DataSource, EntityManager, QueryRunner } from 'typeorm'; +import { BatchDeleteEntity, CACHE_PREFIX, DeletionReasonCode, UserRole } from 'upgrade_types'; +import { BatchDeleteService } from '../../../src/api/services/BatchDeleteService'; +import { ExperimentService } from '../../../src/api/services/ExperimentService'; +import { FeatureFlagService } from '../../../src/api/services/FeatureFlagService'; +import { SegmentService } from '../../../src/api/services/SegmentService'; +import { FeatureFlagPrecomputedSegmentService } from '../../../src/api/services/FeatureFlagPrecomputedSegmentService'; +import { ExperimentPrecomputedSegmentService } from '../../../src/api/services/ExperimentPrecomputedSegmentService'; +import { UpgradeLogger } from '../../../src/lib/logger/UpgradeLogger'; +import { DeletionRepository } from '../../../src/api/repositories/DeletionRepository'; + +describe('BatchDeleteService transaction outcomes', () => { + const entities: BatchDeleteEntity[] = ['experiments', 'flags', 'segments']; + const user = { email: 'batch@example.com', firstName: 'Batch', lastName: 'User', role: UserRole.ADMIN }; + const logger = { error: jest.fn(), info: jest.fn() } as unknown as UpgradeLogger; + let ids: string[]; + let service: BatchDeleteService; + let runners: QueryRunner[]; + let mutations: string[]; + let experiments: { delete: jest.Mock }; + let flags: { delete: jest.Mock }; + let segments: { deleteSegment: jest.Mock }; + let createQueryRunner: jest.Mock; + let work: jest.Mock; + let configureRunner: (runner: QueryRunner, index: number) => void; + + beforeEach(() => { + (logger.error as jest.Mock).mockClear(); + ids = [randomUUID(), randomUUID(), randomUUID()]; + mutations = []; + runners = []; + configureRunner = () => undefined; + createQueryRunner = jest.fn(() => { + let active = false; + const runner = { + get isTransactionActive() { + return active; + }, + manager: { + getRepository: jest.fn(() => ({ + findOne: jest.fn(async ({ where }) => ({ id: where.id })), + })), + }, + connect: jest.fn().mockResolvedValue(undefined), + startTransaction: jest.fn(async () => { + active = true; + }), + commitTransaction: jest.fn(async () => { + active = false; + }), + rollbackTransaction: jest.fn(async () => { + active = false; + }), + release: jest.fn().mockResolvedValue(undefined), + } as unknown as QueryRunner; + configureRunner(runner, runners.length); + runners.push(runner); + return runner; + }); + work = jest.fn(async (id) => { + mutations.push(id); + return [{ id }]; + }); + experiments = { delete: jest.fn((id, _user, options) => options.executeTransaction(() => work(id))) }; + flags = { delete: jest.fn((id, _user, _logger, transaction) => transaction(() => work(id))) }; + segments = { + deleteSegment: jest.fn((id, _logger, transaction) => + transaction(async () => { + const [row] = await work(id); + return row; + }) + ), + }; + service = new BatchDeleteService( + { createQueryRunner } as unknown as DataSource, + experiments as unknown as ExperimentService, + flags as unknown as FeatureFlagService, + segments as unknown as SegmentService, + new DeletionRepository(DeletionRepository, {} as EntityManager) + ); + }); + afterEach(() => { + jest.restoreAllMocks(); + }); + + test.each(entities)('%s commits sequentially and normalizes existing service responses', async (entity) => { + const result = await service.delete(entity, ids, user, logger); + expect(result).toEqual({ results: ids.map((id) => ({ id, outcome: 'deleted' })) }); + expect(mutations).toEqual(ids); + for (const runner of runners) { + expect(runner.commitTransaction).toHaveBeenCalledTimes(1); + expect(runner.rollbackTransaction).not.toHaveBeenCalled(); + expect(runner.release).toHaveBeenCalledTimes(1); + } + }); + + const postCommitScenarios = ['success', 'release failure', 'commit response lost', 'commit rejected']; + test.each(postCommitScenarios)('awaits segment post-commit work after %s', async (scenario) => { + const selectedIds = ids.slice(0, 2); + const storedIds = new Set(selectedIds); + const commitFails = scenario === 'commit response lost' || scenario === 'commit rejected'; + const stopsBatch = scenario !== 'success'; + configureRunner = (runner, index) => { + const commit = (runner.commitTransaction as jest.Mock).getMockImplementation(); + (runner.commitTransaction as jest.Mock).mockImplementation(async () => { + if (scenario === 'commit rejected') throw new Error('commit rejected'); + storedIds.delete(selectedIds[index]); + if (scenario === 'commit response lost') throw new Error('commit response lost'); + await commit(); + }); + if (scenario === 'release failure') { + (runner.release as jest.Mock).mockRejectedValue(new Error('release failed')); + } + }; + const saved = { flags: [...selectedIds], experiments: [...selectedIds] }; + let releaseFirstWrites: () => void; + const firstWrites = new Promise((resolve) => (releaseFirstWrites = resolve)); + const recompute = (entity: keyof typeof saved) => async () => { + const members = [...storedIds]; + if (members.length) await firstWrites; + saved[entity] = members; + }; + // Use the real deletion/scheduling methods, holding the first post-delete writes to expose overlap. + const segmentService = Object.assign(Object.create(SegmentService.prototype), { + featureFlagPrecomputedSegmentService: Object.assign( + Object.create(FeatureFlagPrecomputedSegmentService.prototype), + { + getAffectedFlagIds: async () => ['flag'], + recomputeOwner: recompute('flags'), + } + ), + experimentPrecomputedSegmentService: Object.assign(Object.create(ExperimentPrecomputedSegmentService.prototype), { + getAffectedExperimentIds: async () => ['experiment'], + recomputeOwner: recompute('experiments'), + }), + cacheService: { resetPrefixCache: jest.fn().mockResolvedValue(undefined) }, + deleteSegmentAndPrivateSubsegments: async (id) => (await work(id))[0], + }); + segments.deleteSegment.mockImplementation(segmentService.deleteSegment.bind(segmentService)); + let completed = false; + const deletion = service.delete('segments', selectedIds, user, logger).then((result) => { + completed = true; + return result; + }); + try { + await new Promise((resolve) => setImmediate(resolve)); + expect(mutations).toEqual([selectedIds[0]]); + expect(completed).toBe(false); + } finally { + releaseFirstWrites(); + await deletion; + } + const remainingIds = scenario === 'commit rejected' ? selectedIds : stopsBatch ? selectedIds.slice(1) : []; + expect(saved).toEqual({ flags: remainingIds, experiments: remainingIds }); + expect(segmentService.cacheService.resetPrefixCache).toHaveBeenCalledWith(CACHE_PREFIX.SEGMENT_KEY_PREFIX); + expect(segmentService.cacheService.resetPrefixCache).toHaveBeenCalledWith( + CACHE_PREFIX.GLOBAL_EXCLUDE_SEGMENT_KEY_PREFIX + ); + expect((await deletion).results).toEqual( + stopsBatch + ? [ + { + id: selectedIds[0], + outcome: commitFails ? 'unknown' : 'deleted', + reasonCode: commitFails ? DeletionReasonCode.OUTCOME_UNKNOWN : DeletionReasonCode.POST_DELETE_FAILED, + }, + { id: selectedIds[1], outcome: 'not_attempted' }, + ] + : selectedIds.map((id) => ({ id, outcome: 'deleted' })) + ); + expect(createQueryRunner).toHaveBeenCalledTimes(stopsBatch ? 1 : 2); + if (commitFails) expect(runners[0].rollbackTransaction).toHaveBeenCalledTimes(1); + }); + + test('stops without mutation when the target lookup fails', async () => { + configureRunner = (runner) => { + (runner.manager.getRepository as jest.Mock).mockReturnValue({ + findOne: jest.fn().mockRejectedValue(new Error('read failed')), + }); + }; + const result = await service.delete('flags', ids, user, logger); + expect(result.results.map((item) => item.outcome)).toEqual(['failed', 'not_attempted', 'not_attempted']); + expect(result.results[0].reasonCode).toBe(DeletionReasonCode.DELETE_FAILED); + expect(mutations).toEqual([]); + expect(runners[0].rollbackTransaction).toHaveBeenCalledTimes(1); + }); + + test('reports no attempted mutation when every target is missing', async () => { + configureRunner = (runner) => { + (runner.manager.getRepository as jest.Mock).mockReturnValue({ findOne: jest.fn().mockResolvedValue(null) }); + }; + expect(await service.delete('flags', ids, user, logger)).toEqual({ + results: ids.map((id) => ({ id, outcome: 'not_found', reasonCode: DeletionReasonCode.NOT_FOUND })), + }); + expect(mutations).toEqual([]); + expect(runners.every((runner) => (runner.rollbackTransaction as jest.Mock).mock.calls.length === 1)).toBe(true); + expect(logger.error).not.toHaveBeenCalled(); + }); + + test.each(['rollbackTransaction', 'release'] as const)( + 'stops when %s fails even if the target was missing before mutation', + async (method) => { + configureRunner = (runner) => { + (runner.manager.getRepository as jest.Mock).mockReturnValue({ + findOne: jest.fn().mockResolvedValue(null), + }); + (runner[method] as jest.Mock).mockRejectedValue(new Error('connection lost')); + }; + const result = await service.delete('flags', ids, user, logger); + expect(result.results.map((item) => item.outcome)).toEqual(['failed', 'not_attempted', 'not_attempted']); + expect(mutations).toEqual([]); + expect(createQueryRunner).toHaveBeenCalledTimes(1); + expect(logger.error).toHaveBeenCalledWith(expect.objectContaining({ message: 'Batch deletion item failed' })); + } + ); + + test.each([0, 1])('stops after item %i fails and preserves earlier commits', async (index) => { + work.mockImplementation(async (id) => { + if (id === ids[index]) throw new Error('cleanup failed'); + mutations.push(id); + return [{ id }]; + }); + const result = await service.delete('flags', ids, user, logger); + expect(result.results.map((item) => item.outcome)).toEqual( + index === 0 ? ['failed', 'not_attempted', 'not_attempted'] : ['deleted', 'failed', 'not_attempted'] + ); + expect(result.results.slice(index + 1)).toEqual( + ids.slice(index + 1).map((id) => ({ id, outcome: 'not_attempted' })) + ); + expect(runners[index].rollbackTransaction).toHaveBeenCalledTimes(1); + expect(runners[index].commitTransaction).not.toHaveBeenCalled(); + expect(mutations).toEqual(ids.slice(0, index)); + }); + + test.each(entities)('%s skips a missing locked target and continues deletion', async (entity) => { + configureRunner = (runner, index) => { + if (index !== 0) return; + (runner.manager.getRepository as jest.Mock).mockReturnValue({ + findOne: jest.fn().mockResolvedValue(null), + }); + }; + const result = await service.delete(entity, ids, user, logger); + expect(result.results[0].outcome).toBe('not_found'); + expect(result.results.slice(1).map((item) => item.outcome)).toEqual(['deleted', 'deleted']); + expect(mutations).toEqual(ids.slice(1)); + }); + + test('reports a lock timeout only after a confirmed rollback', async () => { + work.mockRejectedValue(Object.assign(new Error('lock wait'), { code: '55P03' })); + const result = await service.delete('flags', ids, user, logger); + expect(result.results[0]).toEqual({ id: ids[0], outcome: 'failed', reasonCode: DeletionReasonCode.LOCK_TIMEOUT }); + expect(runners[0].rollbackTransaction).toHaveBeenCalled(); + }); + + test('does not claim rollback when COMMIT acknowledgment is lost, even if ROLLBACK succeeds', async () => { + configureRunner = (runner) => { + (runner.commitTransaction as jest.Mock).mockRejectedValue(new Error('connection lost')); + }; + const result = await service.delete('experiments', ids, user, logger); + expect(result.results[0]).toEqual({ + id: ids[0], + outcome: 'unknown', + reasonCode: DeletionReasonCode.OUTCOME_UNKNOWN, + }); + expect(result.results[1].outcome).toBe('not_attempted'); + expect(runners[0].rollbackTransaction).toHaveBeenCalled(); + }); + + test('reports unknown when mutation failed and rollback cannot be confirmed', async () => { + work.mockRejectedValue(new Error('write lost')); + configureRunner = (runner) => { + (runner.rollbackTransaction as jest.Mock).mockRejectedValue(new Error('rollback lost')); + }; + const result = await service.delete('flags', ids, user, logger); + expect(result.results[0].outcome).toBe('unknown'); + }); + + test('does not promote an empty legacy deletion response to a committed success', async () => { + work.mockResolvedValue([]); + const result = await service.delete('experiments', ids, user, logger); + expect(result.results[0].outcome).toBe('failed'); + expect(runners[0].commitTransaction).not.toHaveBeenCalled(); + expect(runners[0].rollbackTransaction).toHaveBeenCalled(); + }); + + test('keeps a known committed deletion when later cache cleanup fails and stops remaining items', async () => { + const original = segments.deleteSegment.getMockImplementation(); + segments.deleteSegment.mockImplementation(async (...args) => { + await original(...args); + throw new Error('cache failed'); + }); + const result = await service.delete('segments', ids, user, logger); + expect(result.results[0]).toEqual({ + id: ids[0], + outcome: 'deleted', + reasonCode: DeletionReasonCode.POST_DELETE_FAILED, + }); + expect(result.results[1].outcome).toBe('not_attempted'); + expect(runners[0].rollbackTransaction).not.toHaveBeenCalled(); + }); + + test('keeps a committed deletion when connection release fails', async () => { + configureRunner = (runner) => { + (runner.release as jest.Mock).mockRejectedValue(new Error('release failed')); + }; + expect((await service.delete('flags', ids, user, logger)).results[0]).toMatchObject({ + outcome: 'deleted', + reasonCode: DeletionReasonCode.POST_DELETE_FAILED, + }); + }); + + test('returns every result in a large batch after failure', async () => { + // Fits within the 5MB JSON limit, but exceeds the argument limit of a spread-based push. + ids = Array.from({ length: 130_000 }, () => randomUUID()); + work.mockImplementation(async (id) => { + if (id === ids[1]) throw new Error('cleanup failed'); + return [{ id }]; + }); + + const result = await service.delete('flags', ids, user, logger); + expect(result.results).toHaveLength(ids.length); + expect(result.results[0]).toEqual({ id: ids[0], outcome: 'deleted' }); + expect(result.results[1]).toEqual({ + id: ids[1], + outcome: 'failed', + reasonCode: DeletionReasonCode.DELETE_FAILED, + }); + expect( + result.results + .slice(2) + .every( + (item, index) => + item.id === ids[2 + index] && item.outcome === 'not_attempted' && item.reasonCode === undefined + ) + ).toBe(true); + expect(createQueryRunner).toHaveBeenCalledTimes(2); + expect(runners[0].commitTransaction).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/backend/test/unit/services/ExperimentPrecomputedSegmentService.test.ts b/packages/backend/test/unit/services/ExperimentPrecomputedSegmentService.test.ts index 223006af53..0c7779dd9b 100644 --- a/packages/backend/test/unit/services/ExperimentPrecomputedSegmentService.test.ts +++ b/packages/backend/test/unit/services/ExperimentPrecomputedSegmentService.test.ts @@ -143,7 +143,7 @@ describe('ExperimentPrecomputedSegmentService', () => { const affected = await service.getAffectedExperimentIds('segChild'); expect(affected).toEqual(['expP']); - expect(segmentRepository.findParentSegmentIds).toHaveBeenCalledWith('segChild'); + expect(segmentRepository.findParentSegmentIds).toHaveBeenCalledWith('segChild', undefined); }); it('does not infinitely recurse on a segment cycle', async () => { diff --git a/packages/backend/test/unit/services/FeatureFlagPrecomputedSegmentService.test.ts b/packages/backend/test/unit/services/FeatureFlagPrecomputedSegmentService.test.ts index 9287083130..3aa494bab9 100644 --- a/packages/backend/test/unit/services/FeatureFlagPrecomputedSegmentService.test.ts +++ b/packages/backend/test/unit/services/FeatureFlagPrecomputedSegmentService.test.ts @@ -139,7 +139,7 @@ describe('FeatureFlagPrecomputedSegmentService', () => { const affected = await service.getAffectedFlagIds('segChild'); expect(affected).toEqual(['flagP']); - expect(segmentRepository.findParentSegmentIds).toHaveBeenCalledWith('segChild'); + expect(segmentRepository.findParentSegmentIds).toHaveBeenCalledWith('segChild', undefined); }); it('does not infinitely recurse on a segment cycle', async () => { diff --git a/packages/backend/test/unit/services/FeatureFlagService.test.ts b/packages/backend/test/unit/services/FeatureFlagService.test.ts index e5d9f87928..e1e601de5b 100644 --- a/packages/backend/test/unit/services/FeatureFlagService.test.ts +++ b/packages/backend/test/unit/services/FeatureFlagService.test.ts @@ -234,6 +234,7 @@ describe('Feature Flag Service Testing', () => { getFlagsForKeys: jest.fn().mockResolvedValue(mockFlagArr), getFlagsFromContext: jest.fn().mockResolvedValue(mockFlagArr), findOne: jest.fn().mockResolvedValue(mockFlag1), + findOneForDetails: jest.fn().mockResolvedValue(mockFlag1), findWithNames: jest.fn().mockResolvedValue(mockFlagArr), findOneById: jest.fn().mockResolvedValue(mockFlag1), count: jest.fn().mockResolvedValue(mockFlagArr.length), diff --git a/packages/frontend/jest.config.js b/packages/frontend/jest.config.js index 2901cd3024..fa9f2900d6 100644 --- a/packages/frontend/jest.config.js +++ b/packages/frontend/jest.config.js @@ -36,8 +36,8 @@ module.exports = { '/dist/', '/e2e/', '/src/environments/', - // Dashboard specs are excluded except the routing spec, which guards subtle redirect behavior - '/src/app/features/dashboard/(?!dashboard-routing\\.spec)', + // Keep the routing and batch root UI integration specs discoverable. + '/src/app/features/dashboard/(?!dashboard-routing\\.spec|batch-delete-ui\\.spec)', '/src/app/shared/', ], diff --git a/packages/frontend/projects/upgrade/src/app/core/api-endpoints.constants.ts b/packages/frontend/projects/upgrade/src/app/core/api-endpoints.constants.ts index ce9a4efd48..b60eb2b412 100644 --- a/packages/frontend/projects/upgrade/src/app/core/api-endpoints.constants.ts +++ b/packages/frontend/projects/upgrade/src/app/core/api-endpoints.constants.ts @@ -5,6 +5,9 @@ import { APIEndpoints } from '../../environments/environment-types'; * These are relative paths that will be prepended with the environment's apiBaseUrl by the HTTP interceptor. */ export const API_ENDPOINTS: APIEndpoints = { + experimentsBatchDelete: '/experiments/batch-delete', + flagsBatchDelete: '/flags/batch-delete', + segmentsBatchDelete: '/segments/batch-delete', getAllExperiments: '/experiments/paginated', createNewExperiments: '/experiments', validateExperiment: '/experiments/validation', diff --git a/packages/frontend/projects/upgrade/src/app/core/batch-actions/batch-actions.actions.ts b/packages/frontend/projects/upgrade/src/app/core/batch-actions/batch-actions.actions.ts new file mode 100644 index 0000000000..867dee3fb6 --- /dev/null +++ b/packages/frontend/projects/upgrade/src/app/core/batch-actions/batch-actions.actions.ts @@ -0,0 +1,28 @@ +import { createAction, props } from '@ngrx/store'; +import { BatchDeleteResult } from 'upgrade_types'; +import { BatchDeleteSnapshot, RootSelectionItem } from './batch-actions.models'; + +export function createBatchDeleteActions(source: string) { + const prefix = `[${source} Batch]`; + return { + toggleRow: createAction(`${prefix} Toggle Row`, props<{ item: RootSelectionItem }>()), + toggleHeader: createAction(`${prefix} Toggle Header`, props<{ items: RootSelectionItem[] }>()), + rootPageLeft: createAction(`${prefix} Root Page Left`), + confirmedRemoved: createAction(`${prefix} Confirmed Removed`, props<{ ids: string[] }>()), + prepareConfirmation: createAction(`${prefix} Prepare Confirmation`, props<{ operationId: string }>()), + dismissConfirmation: createAction(`${prefix} Dismiss Confirmation`), + batchDeleteRequested: createAction(`${prefix} Delete Requested`, props<{ snapshot: BatchDeleteSnapshot }>()), + batchDeleteCompleted: createAction( + `${prefix} Delete Completed`, + props<{ operationId: string; result: BatchDeleteResult }>() + ), + batchDeleteRequestFailed: createAction( + `${prefix} Delete Request Failed`, + props<{ operationId: string; status: number }>() + ), + listRequested: createAction(`${prefix} List Requested`, props<{ requestId: string }>()), + listFailed: createAction(`${prefix} List Failed`, props<{ requestId: string }>()), + }; +} + +export type RootBatchDeleteActions = ReturnType; diff --git a/packages/frontend/projects/upgrade/src/app/core/batch-actions/batch-actions.effects.ts b/packages/frontend/projects/upgrade/src/app/core/batch-actions/batch-actions.effects.ts new file mode 100644 index 0000000000..58d3f00d57 --- /dev/null +++ b/packages/frontend/projects/upgrade/src/app/core/batch-actions/batch-actions.effects.ts @@ -0,0 +1,140 @@ +import { Action } from '@ngrx/store'; +import { TranslateService } from '@ngx-translate/core'; +import { Observable, concat, defer, of } from 'rxjs'; +import { + catchError, + distinctUntilChanged, + exhaustMap, + filter, + map, + skipWhile, + switchMap, + takeUntil, + throwIfEmpty, + withLatestFrom, +} from 'rxjs/operators'; +import { BatchDeleteEntity, BatchDeleteResult, DeletionReasonCode } from 'upgrade_types'; +import { NotificationService } from '../notifications/notification.service'; +import { RootBatchDeleteActions } from './batch-actions.actions'; +import { RootBatchDeleteState, newBatchRequestId } from './batch-actions.models'; +import { + batchDeleteResultCounts, + batchDeleteResultMessage, + validateBatchDeleteResponse, +} from './batch-actions.helpers'; + +interface BatchDeleteDataSource { + batchDelete(ids: string[]): Observable; +} + +export function batchDeleteEffect( + events: Observable, + state$: Observable, + actions: RootBatchDeleteActions, + data: BatchDeleteDataSource +) { + return events.pipe( + filter((action) => action.type === actions.batchDeleteRequested.type), + withLatestFrom(state$), + filter( + ([action, state]) => + state.operation?.status === 'submitting' && + state.operation.snapshot.operationId === + (action as ReturnType).snapshot.operationId + ), + exhaustMap(([, state]) => { + const { snapshot } = state.operation; + const ids = snapshot.items.map((item) => item.id); + return defer(() => data.batchDelete(ids)).pipe( + throwIfEmpty(), + map((result) => + actions.batchDeleteCompleted({ + operationId: snapshot.operationId, + result: validateBatchDeleteResponse(result, ids), + }) + ), + catchError((error) => + of( + error?.status !== undefined + ? actions.batchDeleteRequestFailed({ operationId: snapshot.operationId, status: error.status }) + : actions.batchDeleteCompleted({ + operationId: snapshot.operationId, + // An invalid/empty successful response has no HTTP error for the interceptor to report. + result: { + results: ids.map((id) => ({ + id, + outcome: 'unknown', + reasonCode: DeletionReasonCode.OUTCOME_UNKNOWN, + })), + }, + }) + ) + ), + // Logout/user replacement discards session state. Navigation alone never cancels this request. + takeUntil(state$.pipe(filter((current) => current.userEmail !== state.userEmail || !current.operation))) + ); + }) + ); +} + +export function batchDeleteFinishedEffect( + events: Observable, + state$: Observable, + actions: RootBatchDeleteActions, + notification: { entity: BatchDeleteEntity; translate: TranslateService; service: NotificationService }, + finish: (counts: ReturnType) => Action[] +) { + return events.pipe( + filter((action) => + [actions.batchDeleteCompleted.type, actions.batchDeleteRequestFailed.type].some((type) => type === action.type) + ), + withLatestFrom(state$), + filter( + ([action, state]) => + state.operation?.status === 'complete' && + state.operation.snapshot.operationId === (action as ReturnType).operationId + ), + distinctUntilChanged( + (previous, current) => previous[1].operation.snapshot.operationId === current[1].operation.snapshot.operationId + ), + switchMap(([, state]) => { + // HTTP failures already use the same error notification as single deletion. Do not refresh or notify twice. + if (state.operation.transportStatus !== undefined) return []; + const counts = batchDeleteResultCounts(state); + const message = batchDeleteResultMessage(notification.entity, counts, (key, params) => + notification.translate.instant(key, params) + ); + if (!counts.hasErrors) notification.service.showSuccess(message); + else if (counts.deleted || counts.absent) notification.service.showWarning(message); + else notification.service.showError(message); + return finish(counts); + }) + ); +} + +/** Read current state at subscription time, after the list-start action reaches the reducer. */ +export function trackedListRequest( + state$: Observable, + actions: RootBatchDeleteActions, + dispatch: (action: Action) => void, + request: () => Observable, + success: (data: T, requestId: string) => Action[], + failure: (error: unknown) => Action[] +) { + return defer(() => { + const requestId = newBatchRequestId(); + dispatch(actions.listRequested({ requestId })); + return request().pipe( + switchMap((data) => success(data, requestId)), + catchError((error) => concat(of(actions.listFailed({ requestId })), of(...failure(error)))), + // NgRx queues a nested dispatch until the current action finishes. Observe our start before + // treating a different token (including null on deletion/logout) as cancellation. + takeUntil( + state$.pipe( + skipWhile((state) => state.listRequestId !== requestId), + filter((state) => state.listRequestId !== requestId) + ) + ) + ); + }); +} diff --git a/packages/frontend/projects/upgrade/src/app/core/batch-actions/batch-actions.facade.ts b/packages/frontend/projects/upgrade/src/app/core/batch-actions/batch-actions.facade.ts new file mode 100644 index 0000000000..6928332198 --- /dev/null +++ b/packages/frontend/projects/upgrade/src/app/core/batch-actions/batch-actions.facade.ts @@ -0,0 +1,42 @@ +import { Store, select } from '@ngrx/store'; +import { map, take } from 'rxjs/operators'; +import { BatchDeleteEntity } from 'upgrade_types'; +import { RootBatchDeleteActions } from './batch-actions.actions'; +import { RootBatchDeleteState, newBatchRequestId } from './batch-actions.models'; +import { selectionItem, batchDeleteSelectionView } from './batch-actions.helpers'; + +/** Entity facades expose this same interface to root tables and the shared confirmation dialog. */ +export function createBatchDeleteFacade( + store: Store, + entity: BatchDeleteEntity, + actions: RootBatchDeleteActions, + selectState: (state: any) => RootBatchDeleteState, + selectRows: (state: any) => { id?: string; name?: string }[] +) { + const state$ = store.pipe(select(selectState)); + return { + state$, + selection$: state$.pipe( + map((state) => ({ + ...batchDeleteSelectionView(state, entity), + loadedIds: new Set(state.loadedIds), + })) + ), + toggleRow: (row: Parameters[0]) => + store.dispatch(actions.toggleRow({ item: selectionItem(row) })), + toggleHeader: () => + store + .pipe(select(selectRows), take(1)) + .subscribe((rows) => store.dispatch(actions.toggleHeader({ items: rows.map(selectionItem) }))), + leaveRootPage: () => store.dispatch(actions.rootPageLeft()), + prepareConfirmation: () => store.dispatch(actions.prepareConfirmation({ operationId: newBatchRequestId() })), + dismissConfirmation: () => store.dispatch(actions.dismissConfirmation()), + submit: (operationId: string) => + state$.pipe(take(1)).subscribe((state) => { + if (state.confirmation?.operationId === operationId) + store.dispatch(actions.batchDeleteRequested({ snapshot: state.confirmation })); + }), + }; +} + +export type BatchDeleteFacade = ReturnType; diff --git a/packages/frontend/projects/upgrade/src/app/core/batch-actions/batch-actions.helpers.spec.ts b/packages/frontend/projects/upgrade/src/app/core/batch-actions/batch-actions.helpers.spec.ts new file mode 100644 index 0000000000..bd8d161382 --- /dev/null +++ b/packages/frontend/projects/upgrade/src/app/core/batch-actions/batch-actions.helpers.spec.ts @@ -0,0 +1,117 @@ +import { + BatchDeleteEntity, + BatchDeleteResult, + EXPERIMENT_STATE, + FEATURE_FLAG_STATUS, + SEGMENT_STATUS, +} from 'upgrade_types'; +import { createBatchDeleteActions } from './batch-actions.actions'; +import { localDeletionReason, batchDeleteSelectionView, validateBatchDeleteResponse } from './batch-actions.helpers'; +import { + DeletionEligibilityReasonCode, + RootBatchDeleteState, + RootSelectionItem, + initialRootBatchDeleteState, +} from './batch-actions.models'; +import { reduceRootBatchDelete } from './batch-actions.reducer'; + +describe('Root selection rules', () => { + const actions = createBatchDeleteActions('Test'); + const item = (id: string): RootSelectionItem => ({ + id, + name: id, + stateOrStatus: SEGMENT_STATUS.UNUSED, + }); + const state = (selected: string[], loaded: string[]): RootBatchDeleteState => ({ + ...initialRootBatchDeleteState, + selectedById: Object.fromEntries(selected.map((id) => [id, item(id)])), + loadedIds: loaded, + }); + + it.each([ + [[], [], false, false], + [['a'], ['a', 'b'], false, true], + [['a', 'b'], ['a', 'b'], true, false], + [['a', 'b', 'hidden'], ['a', 'b'], true, false], + [['a', 'hidden'], [], false, true], + ])( + 'derives the header from selected=%j and loaded=%j', + (selected: string[], loaded: string[], checked: boolean, indeterminate: boolean) => { + expect(batchDeleteSelectionView(state(selected, loaded), 'segments')).toMatchObject({ + checked, + indeterminate, + }); + } + ); + + it('excludes IDs outside the loaded root rows from header selection', () => { + const current = state([], ['loaded']); + const next = reduceRootBatchDelete( + current, + actions.toggleHeader({ + items: [item('loaded'), item('detail-only')], + }), + actions, + 'segments' + ); + expect(Object.keys(next.selectedById)).toEqual(['loaded']); + expect(current.selectedById).toEqual({}); + }); + + it('can clear hidden selections during replacement loading but cannot select an obsolete page', () => { + const current = { ...state(['hidden'], []), listLoading: true }; + const cleared = reduceRootBatchDelete(current, actions.toggleHeader({ items: [item('old')] }), actions, 'segments'); + expect(cleared.selectedById).toEqual({}); + expect( + reduceRootBatchDelete(cleared, actions.toggleHeader({ items: [item('old')] }), actions, 'segments').selectedById + ).toEqual({}); + }); + + it.each([ + [EXPERIMENT_STATE.DRAFT, true], + [EXPERIMENT_STATE.INACTIVE, true], + [EXPERIMENT_STATE.COMPLETED, true], + [EXPERIMENT_STATE.CANCELLED, true], + [EXPERIMENT_STATE.ARCHIVED, true], + [EXPERIMENT_STATE.PREVIEW, false], + [EXPERIMENT_STATE.SCHEDULED, false], + [EXPERIMENT_STATE.RUNNING, false], + [EXPERIMENT_STATE.ENROLLING, false], + [EXPERIMENT_STATE.PAUSED, false], + [EXPERIMENT_STATE.ENROLLMENT_COMPLETE, false], + ])('applies the experiment deletion policy to %s', (status: EXPERIMENT_STATE, allowed: boolean) => { + expect(localDeletionReason('experiments', { id: 'a', stateOrStatus: status })).toBe( + allowed ? undefined : DeletionEligibilityReasonCode.EXPERIMENT_ACTIVE + ); + }); + + it.each<[BatchDeleteEntity, RootSelectionItem, DeletionEligibilityReasonCode]>([ + [ + 'flags', + { id: 'a', stateOrStatus: FEATURE_FLAG_STATUS.ENABLED }, + DeletionEligibilityReasonCode.FEATURE_FLAG_ENABLED, + ], + ['segments', { ...item('a'), stateOrStatus: SEGMENT_STATUS.USED }, DeletionEligibilityReasonCode.SEGMENT_USED], + ])('applies entity-specific status rules for %s', (entity, selected, reason) => { + expect(localDeletionReason(entity, selected)).toBe(reason); + }); + + it('accepts reordered results but rejects incomplete, duplicate, or unexpected result IDs', () => { + const complete: BatchDeleteResult = { + results: [ + { id: 'b', outcome: 'not_found' }, + { id: 'a', outcome: 'deleted' }, + ], + }; + expect(validateBatchDeleteResponse(complete, ['a', 'b'])).toBe(complete); + expect(() => validateBatchDeleteResponse({ results: [{ id: 'a', outcome: 'deleted' }] }, ['a', 'b'])).toThrow(); + for (const returnedIds of [ + ['a', 'a'], + ['a', 'unexpected'], + ]) { + expect(() => + validateBatchDeleteResponse({ results: returnedIds.map((id) => ({ id, outcome: 'deleted' })) }, ['a', 'b']) + ).toThrow(); + } + }); +}); diff --git a/packages/frontend/projects/upgrade/src/app/core/batch-actions/batch-actions.helpers.ts b/packages/frontend/projects/upgrade/src/app/core/batch-actions/batch-actions.helpers.ts new file mode 100644 index 0000000000..441b50371d --- /dev/null +++ b/packages/frontend/projects/upgrade/src/app/core/batch-actions/batch-actions.helpers.ts @@ -0,0 +1,125 @@ +import { + BatchDeleteEntity, + BatchDeleteResult, + DeletionReasonCode, + EXPERIMENT_STATE, + EXPERIMENT_STATE_DISPLAY_NAME_OVERRIDES, + FEATURE_FLAG_STATUS, + SEGMENT_STATUS, +} from 'upgrade_types'; +import { + DeletionEligibilityReasonCode, + RootBatchDeleteState, + RootSelectionItem, + isBatchDeleteBusy, +} from './batch-actions.models'; + +export function selectionItem(row: { + id?: string; + name?: string; + state?: EXPERIMENT_STATE; + status?: FEATURE_FLAG_STATUS | SEGMENT_STATUS; +}): RootSelectionItem { + return { id: row.id, name: row.name, stateOrStatus: row.state || row.status }; +} + +export function getExperimentDeletionReason(state?: EXPERIMENT_STATE): DeletionEligibilityReasonCode | undefined { + const displayState = EXPERIMENT_STATE_DISPLAY_NAME_OVERRIDES[state] || state; + return [ + EXPERIMENT_STATE.DRAFT, + EXPERIMENT_STATE.INACTIVE, + EXPERIMENT_STATE.COMPLETED, + EXPERIMENT_STATE.ARCHIVED, + ].includes(displayState) + ? undefined + : DeletionEligibilityReasonCode.EXPERIMENT_ACTIVE; +} + +export function localDeletionReason( + entity: BatchDeleteEntity, + item: RootSelectionItem +): DeletionEligibilityReasonCode | undefined { + if (entity === 'experiments') return getExperimentDeletionReason(item.stateOrStatus as EXPERIMENT_STATE); + if (entity === 'flags') + return item.stateOrStatus === FEATURE_FLAG_STATUS.ENABLED + ? DeletionEligibilityReasonCode.FEATURE_FLAG_ENABLED + : undefined; + return item.stateOrStatus === SEGMENT_STATUS.USED ? DeletionEligibilityReasonCode.SEGMENT_USED : undefined; +} + +export function batchDeleteSelectionView(state: RootBatchDeleteState, entity: BatchDeleteEntity) { + const items = Object.values(state.selectedById); + const loaded = state.loadedIds; + const checked = loaded.length > 0 && loaded.every((id) => !!state.selectedById[id]); + const reasons = items.map((item) => localDeletionReason(entity, item)).filter(Boolean); + return { + items, + selectedCount: items.length, + checked, + indeterminate: items.length > 0 && !checked, + canToggleHeader: !isBatchDeleteBusy(state) && (items.length > 0 || loaded.length > 0), + canRequestConfirmation: items.length > 0 && !reasons.length && !isBatchDeleteBusy(state), + reasonCode: reasons[0], + busy: isBatchDeleteBusy(state), + }; +} + +export const confirmedRemovedIds = (result?: BatchDeleteResult) => + result?.results.filter((item) => item.outcome === 'deleted' || item.outcome === 'not_found').map((item) => item.id) || + []; + +export function validateBatchDeleteResponse(result: BatchDeleteResult, ids: string[]): BatchDeleteResult { + const outcomes = ['deleted', 'not_found', 'failed', 'unknown', 'not_attempted']; + if (!result || !Array.isArray(result.results) || result.results.length !== ids.length) { + throw new Error('Incomplete batch deletion response'); + } + const resultIds = new Set(result.results.map((item) => item.id)); + if ( + resultIds.size !== ids.length || + ids.some((id) => !resultIds.has(id)) || + result.results.some((item) => !outcomes.includes(item.outcome)) + ) { + throw new Error('Incomplete batch deletion response'); + } + return result; +} + +export function batchDeleteResultCounts(state: RootBatchDeleteState) { + const results = state.operation?.result?.results || []; + return { + deleted: results.filter((item) => item.outcome === 'deleted').length, + absent: results.filter((item) => item.outcome === 'not_found').length, + uncertain: results.some((item) => item.outcome === 'unknown'), + failed: results.filter((item) => item.outcome === 'failed').length, + notAttempted: results.filter((item) => item.outcome === 'not_attempted').length, + hasErrors: results.some((item) => item.outcome !== 'not_found' && (item.outcome !== 'deleted' || item.reasonCode)), + postDeleteFailed: results.some((item) => item.reasonCode === DeletionReasonCode.POST_DELETE_FAILED), + }; +} + +/** One existing snackbar, with only the counts that apply to this result. */ +export function batchDeleteResultMessage( + entity: BatchDeleteEntity, + counts: ReturnType, + translate: (key: string, params?: Record) => string +): string { + const parts: string[] = []; + if (counts.deleted) + parts.push( + translate(`batch-delete.success.${entity}.${counts.deleted === 1 ? 'one' : 'other'}`, { deleted: counts.deleted }) + ); + if (counts.absent) + parts.push( + translate(`batch-delete.result.absent.${counts.absent === 1 ? 'one' : 'other'}`, { count: counts.absent }) + ); + if (counts.failed) parts.push(translate('batch-delete.result.failed')); + if (counts.notAttempted) + parts.push( + translate(`batch-delete.result.notAttempted.${counts.notAttempted === 1 ? 'one' : 'other'}`, { + count: counts.notAttempted, + }) + ); + if (counts.postDeleteFailed) parts.push(translate('batch-delete.result.post-delete-failed')); + if (counts.uncertain) parts.push(translate('batch-delete.result.uncertain')); + return parts.join(' '); +} diff --git a/packages/frontend/projects/upgrade/src/app/core/batch-actions/batch-actions.http.spec.ts b/packages/frontend/projects/upgrade/src/app/core/batch-actions/batch-actions.http.spec.ts new file mode 100644 index 0000000000..1bd64e3cf6 --- /dev/null +++ b/packages/frontend/projects/upgrade/src/app/core/batch-actions/batch-actions.http.spec.ts @@ -0,0 +1,57 @@ +import { HttpClientTestingModule, HttpTestingController } from '@angular/common/http/testing'; +import { HttpRequest } from '@angular/common/http'; +import { TestBed } from '@angular/core/testing'; +import { ActivationEnd } from '@angular/router'; +import { Subject } from 'rxjs'; +import { ExperimentDataService } from '../experiments/experiments.data.service'; +import { FeatureFlagsDataService } from '../feature-flags/feature-flags.data.service'; +import { SegmentsDataService } from '../segments/segments.data.service'; +import { HttpCancelInterceptor, SKIP_NAVIGATION_CANCEL } from '../http-interceptors/http-cancel.interceptor'; +import { batchHttpContext } from './batch-actions.http'; + +describe('Batch HTTP contracts', () => { + let http: HttpTestingController; + beforeEach(() => { + TestBed.configureTestingModule({ + imports: [HttpClientTestingModule], + providers: [ExperimentDataService, FeatureFlagsDataService, SegmentsDataService], + }); + http = TestBed.inject(HttpTestingController); + }); + afterEach(() => { + http.verify(); + TestBed.resetTestingModule(); + }); + + it.each([ + ['experiments', ExperimentDataService], + ['flags', FeatureFlagsDataService], + ['segments', SegmentsDataService], + ] as const)('%s sends the complete selection in one deletion request', (entity, token) => { + const service = TestBed.inject(token as typeof ExperimentDataService); + const ids = ['00000000-0000-4000-8000-000000000001', '00000000-0000-4000-8000-000000000002']; + service.batchDelete(ids).subscribe(); + const deletion = http.expectOne(`/${entity}/batch-delete`); + expect(deletion.request.method).toBe('POST'); + expect(deletion.request.body).toEqual({ ids }); + expect(deletion.request.context.get(SKIP_NAVIGATION_CANCEL)).toBe(true); + deletion.flush({ results: ids.map((id) => ({ id, outcome: 'deleted' })) }); + }); + + it('continues observing a batch response through navigation', () => { + const navigation = new Subject(); + const response = new Subject(); + const interceptor = new HttpCancelInterceptor({ events: navigation } as any); + const received = jest.fn(); + const subscription = interceptor + .intercept(new HttpRequest('POST', '/flags/batch-delete', {}, { context: batchHttpContext() }), { + handle: () => response, + }) + .subscribe(received); + navigation.next(new ActivationEnd({} as any)); + response.next({ status: 200 }); + expect(received).toHaveBeenCalledWith({ status: 200 }); + subscription.unsubscribe(); + navigation.complete(); + }); +}); diff --git a/packages/frontend/projects/upgrade/src/app/core/batch-actions/batch-actions.http.ts b/packages/frontend/projects/upgrade/src/app/core/batch-actions/batch-actions.http.ts new file mode 100644 index 0000000000..fee629e530 --- /dev/null +++ b/packages/frontend/projects/upgrade/src/app/core/batch-actions/batch-actions.http.ts @@ -0,0 +1,4 @@ +import { HttpContext } from '@angular/common/http'; +import { SKIP_NAVIGATION_CANCEL } from '../http-interceptors/http-cancel.interceptor'; + +export const batchHttpContext = () => new HttpContext().set(SKIP_NAVIGATION_CANCEL, true); diff --git a/packages/frontend/projects/upgrade/src/app/core/batch-actions/batch-actions.integration.spec.ts b/packages/frontend/projects/upgrade/src/app/core/batch-actions/batch-actions.integration.spec.ts new file mode 100644 index 0000000000..d3a393c718 --- /dev/null +++ b/packages/frontend/projects/upgrade/src/app/core/batch-actions/batch-actions.integration.spec.ts @@ -0,0 +1,812 @@ +import { TestBed } from '@angular/core/testing'; +import { TranslateModule, TranslateService } from '@ngx-translate/core'; +import { Actions, getEffectsMetadata } from '@ngrx/effects'; +import { Action, ScannedActionsSubject, Store, StoreModule } from '@ngrx/store'; +import { Subject, Subscription, of, throwError } from 'rxjs'; +import { + EXPERIMENT_STATE, + FEATURE_FLAG_STATUS, + FLAG_SEARCH_KEY, + SEGMENT_SEARCH_KEY, + SEGMENT_STATUS, + SORT_AS_DIRECTION, + UserRole, +} from 'upgrade_types'; +import * as experimentActions from '../experiments/store/experiments.actions'; +import * as flagActions from '../feature-flags/store/feature-flags.actions'; +import * as segmentActions from '../segments/store/segments.actions'; +import * as analysisActions from '../analysis/store/analysis.actions'; +import { experimentsReducer } from '../experiments/store/experiments.reducer'; +import { featureFlagsReducer } from '../feature-flags/store/feature-flags.reducer'; +import { segmentsReducer } from '../segments/store/segments.reducer'; +import { selectExperimentDetailsPageError, selectSelectedExperiment } from '../experiments/store/experiments.selectors'; +import { selectSegmentDetailsPageError, selectSelectedSegment } from '../segments/store/segments.selectors'; +import { + selectFeatureFlagDetailsPageError, + selectSelectedFeatureFlag, +} from '../feature-flags/store/feature-flags.selectors'; +import { PAGE_ERROR_TYPE } from '@shared-component-lib/common-page-error/common-page-error.model'; +import { ExperimentEffects } from '../experiments/store/experiments.effects'; +import { FeatureFlagsEffects } from '../feature-flags/store/feature-flags.effects'; +import { FeatureFlagsService } from '../feature-flags/feature-flags.service'; +import { FeatureFlagRootSectionCardComponent } from '../../features/dashboard/feature-flags/pages/feature-flag-root-page/feature-flag-root-page-content/feature-flag-root-section-card/feature-flag-root-section-card.component'; +import { SegmentsEffects } from '../segments/store/segments.effects'; +import { SegmentsService } from '../segments/segments.service'; +import { SegmentRootSectionCardTableComponent } from '../../features/dashboard/segments/pages/segment-root-page/segment-root-page-content/segment-root-section-card/segment-root-section-card-table/segment-root-section-card-table.component'; +import { actionLogoutStart, actionSetUserInfo } from '../auth/store/auth.actions'; +import { batchDeleteResultCounts, selectionItem, batchDeleteSelectionView } from './batch-actions.helpers'; +import { RootBatchDeleteState, newBatchRequestId } from './batch-actions.models'; + +const fixtures = [ + { + entity: 'experiments', + rootPath: '/home', + key: 'experiments', + loadingKey: 'isLoadingExperiment', + actions: experimentActions, + fetchEffect: 'getPaginatedExperiment$', + fetchMethod: 'getAllExperiment', + fetch: experimentActions.actionGetExperiments, + }, + { + entity: 'flags', + rootPath: '/featureflags', + key: 'featureFlags', + loadingKey: 'isLoadingFeatureFlags', + actions: flagActions, + fetchEffect: 'fetchFeatureFlags$', + fetchMethod: 'fetchFeatureFlagsPaginated', + fetch: flagActions.actionFetchFeatureFlags, + }, + { + entity: 'segments', + rootPath: '/segments', + key: 'segments', + loadingKey: 'isLoadingSegments', + actions: segmentActions, + fetchEffect: 'fetchSegmentsPaginated$', + fetchMethod: 'fetchSegmentsPaginated', + fetch: segmentActions.actionFetchSegments, + }, +] as const; + +describe.each(fixtures)('$entity batch store/effects integration', (config) => { + let store: Store; + let state: any; + let subscriptions: Subscription; + let events: Action[]; + let data: any; + let response: Subject; + let effects: any; + let router: any; + let notifications: any; + const actions = config.actions.batchActions; + const rows = ['a', 'b', 'c'].map((name, index) => ({ + id: `11111111-2222-4333-8444-${String(index + 1).padStart(12, '0')}`, + name, + state: config.entity === 'experiments' ? EXPERIMENT_STATE.INACTIVE : undefined, + status: config.entity === 'segments' ? SEGMENT_STATUS.UNUSED : FEATURE_FLAG_STATUS.DISABLED, + })); + const batch = (): RootBatchDeleteState => state[config.key].rootBatch; + const currentRows = () => state[config.key][config.entity === 'flags' ? 'featureFlags' : config.entity]; + const page = (items = rows) => + config.entity === 'segments' + ? { + total: items.length, + nodes: { + segmentsData: items, + experimentSegmentInclusionData: [], + experimentSegmentExclusionData: [], + featureFlagSegmentInclusionData: [], + featureFlagSegmentExclusionData: [], + allParentSegments: [], + }, + } + : { total: items.length, nodes: items }; + const selectRows = (count = rows.length) => + rows.slice(0, count).forEach((row) => store.dispatch(actions.toggleRow({ item: selectionItem(row) }))); + const prepare = () => { + store.dispatch(actions.prepareConfirmation({ operationId: newBatchRequestId() })); + expect(batch().confirmation).not.toBeNull(); + return batch().confirmation; + }; + + beforeEach(() => { + TestBed.configureTestingModule({ + imports: [ + TranslateModule.forRoot(), + StoreModule.forRoot({ + experiments: experimentsReducer, + featureFlags: featureFlagsReducer, + segments: segmentsReducer, + }), + ], + }); + store = TestBed.inject(Store); + subscriptions = new Subscription(); + subscriptions.add(store.subscribe((value) => (state = value))); + const events$ = new Actions(TestBed.inject(ScannedActionsSubject)); + events = []; + subscriptions.add(events$.subscribe((action) => events.push(action))); + response = new Subject(); + data = { + batchDelete: jest.fn(() => response), + [config.fetchMethod]: jest.fn(() => of(page(rows.filter((row) => !batch().removedIds.includes(row.id))))), + }; + router = { url: config.rootPath, navigate: jest.fn() }; + notifications = { showSuccess: jest.fn(), showWarning: jest.fn(), showError: jest.fn(), showInfo: jest.fn() }; + const translate = TestBed.inject(TranslateService); + translate.setTranslation('en', jest.requireActual('../../../assets/i18n/en.json')); + translate.use('en'); + effects = + config.entity === 'experiments' + ? new ExperimentEffects( + events$, + store as any, + data, + router, + translate as any, + notifications, + {} as any, + {} as any, + {} as any + ) + : config.entity === 'flags' + ? new FeatureFlagsEffects( + store as any, + events$, + data, + router, + notifications, + translate as any, + {} as any, + {} as any + ) + : new SegmentsEffects( + store as any, + events$, + data, + {} as any, + router, + notifications, + translate as any, + {} as any + ); + // Include every batch effect to exercise submission and completion together. + const batchEffects = Object.keys(getEffectsMetadata(effects)).filter((field) => /batch/i.test(field)); + for (const field of [...batchEffects, config.fetchEffect]) { + subscriptions.add(effects[field].subscribe((action) => store.dispatch(action))); + } + store.dispatch(actionSetUserInfo({ user: { email: 'review@example.com', role: UserRole.ADMIN } })); + store.dispatch(config.fetch({ fromStarting: true })); + }); + afterEach(() => { + subscriptions.unsubscribe(); + TestBed.resetTestingModule(); + }); + + if (config.entity === 'flags') { + it('clears the search through the root handler without cancelling the unfiltered request', () => { + subscriptions.add(effects.fetchFeatureFlagsOnSearchString$.subscribe()); + subscriptions.add(effects.fetchFlagsOnSearchKeyChange$.subscribe()); + const featureFlagService = new FeatureFlagsService(store as any, { setItem: jest.fn() } as any); + const search = (searchString: string) => + FeatureFlagRootSectionCardComponent.prototype.onSearch.call({ featureFlagService } as any, { + searchKey: FLAG_SEARCH_KEY.NAME, + searchString, + }); + data[config.fetchMethod].mockReturnValue(of(page([rows[0]]))); + search('a'); + const unfiltered = new Subject(); + data[config.fetchMethod].mockReturnValue(unfiltered); + search(''); + expect(state.featureFlags.searchValue).toBe(''); + expect(data[config.fetchMethod].mock.calls.at(-1)[0].searchParams).toBeUndefined(); + unfiltered.next(page()); + unfiltered.complete(); + expect(currentRows().map(({ id }) => id)).toEqual(rows.map(({ id }) => id)); + expect(batch().loadedIds).toHaveLength(3); + expect(batch().listLoading).toBe(false); + expect(state.featureFlags.isLoadingFeatureFlags).toBe(false); + }); + } + + if (config.entity === 'segments') { + it('fetches filtered rows when a tag is clicked and keeps the returned rows selectable', () => { + const segmentsService = new SegmentsService(store as any, data, { setItem: jest.fn() } as any); + const table = new SegmentRootSectionCardTableComponent(segmentsService); + data[config.fetchMethod].mockReturnValueOnce(of(page([rows[0]]))); + + table.filterSegmentByChips('tag', SEGMENT_SEARCH_KEY.TAG); + + expect(data[config.fetchMethod]).toHaveBeenCalledTimes(2); + expect(data[config.fetchMethod]).toHaveBeenLastCalledWith( + expect.objectContaining({ skip: 0, searchParams: { key: SEGMENT_SEARCH_KEY.TAG, string: 'tag' } }), + false + ); + expect(currentRows().map(({ id }) => id)).toEqual([rows[0].id]); + expect(batch().loadedIds).toEqual([rows[0].id]); + expect(state.segments.isLoadingSegments).toBe(false); + store.dispatch(actions.toggleRow({ item: selectionItem(rows[0]) })); + expect(Object.keys(batch().selectedById)).toEqual([rows[0].id]); + }); + } + + it('selects loaded rows, retains hidden selections across replacement reads, and clears all from the header', () => { + expect(batch().loadedIds).toHaveLength(3); // Also exercises NgRx's queued list-start dispatch. + selectRows(2); + data[config.fetchMethod].mockReturnValueOnce(of(page([rows[0]]))); + store.dispatch(config.fetch({ fromStarting: true })); + expect(batchDeleteSelectionView(batch(), config.entity)).toMatchObject({ + selectedCount: 2, + checked: true, + indeterminate: false, + }); + store.dispatch(actions.toggleHeader({ items: [selectionItem(rows[0])] })); + expect(Object.keys(batch().selectedById)).toEqual([]); + store.dispatch(config.actions.actionSetSearchString({ searchString: 'replacement query' })); + expect(batchDeleteSelectionView(batch(), config.entity).canToggleHeader).toBe(false); + store.dispatch(actions.toggleHeader({ items: rows.map(selectionItem) })); + expect(Object.keys(batch().selectedById)).toEqual([]); + }); + + it.each(['search', 'sort'])('restores selection controls after a failed replacement %s', (change) => { + selectRows(1); + const beforeRows = currentRows(); + const pendingList = new Subject(); + data[config.fetchMethod].mockReturnValueOnce(pendingList); + store.dispatch( + change === 'search' + ? config.actions.actionSetSearchString({ searchString: 'b' }) + : config.actions.actionSetSortingType({ sortingType: SORT_AS_DIRECTION.DESCENDING }) + ); + store.dispatch(config.fetch({ fromStarting: true })); + expect(batch().loadedIds).toEqual([]); + store.dispatch(actions.listFailed({ requestId: 'obsolete-request' })); + expect(batch().loadedIds).toEqual([]); + expect(batch().listLoading).toBe(true); + + pendingList.error({ status: 0 }); + expect(currentRows()).toEqual(beforeRows); + expect(batch().listLoading).toBe(false); + expect(state[config.key][config.loadingKey]).toBe(false); + expect(batch().loadedIds).toEqual(rows.map(({ id }) => id)); + expect(Object.keys(batch().selectedById)).toEqual([rows[0].id]); + + store.dispatch(actions.toggleRow({ item: selectionItem(rows[1]) })); + expect(Object.keys(batch().selectedById)).toEqual([rows[0].id, rows[1].id]); + store.dispatch(actions.toggleRow({ item: selectionItem(rows[0]) })); + expect(Object.keys(batch().selectedById)).toEqual([rows[1].id]); + store.dispatch(actions.toggleHeader({ items: rows.map(selectionItem) })); + expect(batchDeleteSelectionView(batch(), config.entity).canToggleHeader).toBe(true); + store.dispatch(actions.toggleHeader({ items: rows.map(selectionItem) })); + expect(Object.keys(batch().selectedById)).toEqual(rows.map(({ id }) => id)); + expect(data[config.fetchMethod]).toHaveBeenCalledTimes(2); + expect(notifications.showWarning).not.toHaveBeenCalled(); + }); + + it('selects loaded rows during incremental loading and leaves newly appended rows unselected', () => { + data[config.fetchMethod].mockReturnValueOnce(of({ ...page(rows.slice(0, 2)), total: 3 })); + store.dispatch(config.fetch({ fromStarting: true })); + const nextPage = new Subject(); + data[config.fetchMethod].mockReturnValueOnce(nextPage); + store.dispatch(config.fetch({ fromStarting: false })); + expect(data[config.fetchMethod]).toHaveBeenLastCalledWith(expect.objectContaining({ skip: 2 }), false); + expect(batch().listLoading).toBe(true); + const fetchCount = data[config.fetchMethod].mock.calls.length; + store.dispatch(config.fetch({ fromStarting: false })); + expect(data[config.fetchMethod]).toHaveBeenCalledTimes(fetchCount); + expect(nextPage.observed).toBe(true); + expect(batchDeleteSelectionView(batch(), config.entity).canToggleHeader).toBe(true); + store.dispatch(actions.toggleHeader({ items: currentRows().map(selectionItem) })); + expect(Object.keys(batch().selectedById)).toEqual(rows.slice(0, 2).map(({ id }) => id)); + nextPage.next({ ...page(rows.slice(1)), total: 3 }); + nextPage.complete(); + expect(batch().loadedIds).toHaveLength(3); + expect(currentRows()).toHaveLength(3); + expect(batchDeleteSelectionView(batch(), config.entity)).toMatchObject({ + selectedCount: 2, + checked: false, + indeterminate: true, + }); + const pendingReplacement = new Subject(); + data[config.fetchMethod].mockReturnValueOnce(pendingReplacement); + store.dispatch(config.fetch({ fromStarting: true })); + expect(batch().listLoading).toBe(true); + data[config.fetchMethod].mockReturnValueOnce(of(page([rows[0], rows[0]]))); + store.dispatch(config.fetch({ fromStarting: true })); + expect(pendingReplacement.observed).toBe(false); + expect(currentRows()).toHaveLength(1); + expect(batch().listLoading).toBe(false); + }); + + it('prepares immediately from retained metadata and invalidates the snapshot after deselection', () => { + selectRows(); + const snapshot = prepare(); + expect(snapshot.items).toHaveLength(3); + store.dispatch(actions.prepareConfirmation({ operationId: newBatchRequestId() })); + expect(batch().confirmation).toBe(snapshot); + store.dispatch(actions.toggleRow({ item: selectionItem(rows[0]) })); + expect(batch().confirmation).toBeNull(); + expect(batch().selectedById[rows[0].id]).toBeUndefined(); + + store.dispatch(actions.batchDeleteRequested({ snapshot })); + expect(data.batchDelete).not.toHaveBeenCalled(); + }); + + it('keeps selection-time availability when refreshed rows change state, including hidden selections', () => { + selectRows(); + const blocked = { + ...rows[2], + state: config.entity === 'experiments' ? EXPERIMENT_STATE.RUNNING : undefined, + status: config.entity === 'segments' ? SEGMENT_STATUS.USED : FEATURE_FLAG_STATUS.ENABLED, + }; + data[config.fetchMethod].mockReturnValueOnce(of(page([rows[0], rows[1], blocked]))); + store.dispatch(config.fetch({ fromStarting: true })); + data[config.fetchMethod].mockReturnValueOnce(of(page([rows[0]]))); + store.dispatch(config.fetch({ fromStarting: true })); + const snapshot = prepare(); + expect(Object.keys(batch().selectedById)).toHaveLength(3); + expect(batch().selectedById[blocked.id]).toEqual(selectionItem(rows[2])); + expect(batchDeleteSelectionView(batch(), config.entity).canRequestConfirmation).toBe(true); + + store.dispatch(actions.batchDeleteRequested({ snapshot })); + expect(data.batchDelete).toHaveBeenCalledWith(rows.map(({ id }) => id)); + }); + + it('removes an absent item and retains failed and unattempted selections with a warning', () => { + selectRows(); + const snapshot = prepare(); + expect(snapshot.items).toHaveLength(3); + + store.dispatch(actions.batchDeleteRequested({ snapshot })); + response.next({ + results: [ + { id: rows[0].id, outcome: 'not_found', reasonCode: 'not_found' }, + { id: rows[1].id, outcome: 'failed', reasonCode: 'delete_failed' }, + { id: rows[2].id, outcome: 'not_attempted' }, + ], + }); + expect(Object.keys(batch().selectedById)).toEqual([rows[1].id, rows[2].id]); + expect(currentRows().map(({ id }) => id)).toEqual([rows[1].id, rows[2].id]); + expect(batchDeleteResultCounts(batch())).toMatchObject({ deleted: 0, absent: 1, failed: 1, notAttempted: 1 }); + expect(notifications.showWarning).toHaveBeenCalledWith( + '1 item was already absent. 1 item could not be deleted. 1 item was not attempted.' + ); + expect(notifications.showWarning).toHaveBeenCalledTimes(1); + expect(notifications.showSuccess).not.toHaveBeenCalled(); + expect(notifications.showError).not.toHaveBeenCalled(); + expect(data[config.fetchMethod]).toHaveBeenLastCalledWith(expect.objectContaining({ skip: 0 }), true); + expect(batch().confirmation).toBeNull(); + }); + + it.each([0, 1])('reports success when %i of two items are deleted and the rest were already absent', (deleted) => { + selectRows(2); + const snapshot = prepare(); + store.dispatch(actions.batchDeleteRequested({ snapshot })); + response.next({ + results: rows + .slice(0, 2) + .map(({ id }, index) => + index < deleted ? { id, outcome: 'deleted' } : { id, outcome: 'not_found', reasonCode: 'not_found' } + ), + }); + const noun = { experiments: 'experiment', flags: 'feature flag', segments: 'segment' }[config.entity]; + expect(notifications.showSuccess).toHaveBeenCalledWith( + deleted ? `1 ${noun} deleted. 1 item was already absent.` : '2 items were already absent.' + ); + expect(notifications.showSuccess).toHaveBeenCalledTimes(1); + expect(notifications.showWarning).not.toHaveBeenCalled(); + expect(notifications.showError).not.toHaveBeenCalled(); + expect(Object.keys(batch().selectedById)).toEqual([]); + expect(currentRows().map(({ id }) => id)).toEqual([rows[2].id]); + }); + + it('reports an error and retains selections when the first deletion fails', () => { + selectRows(); + const snapshot = prepare(); + store.dispatch(actions.batchDeleteRequested({ snapshot })); + response.next({ + results: rows.map(({ id }, index) => ({ id, outcome: index === 0 ? 'failed' : 'not_attempted' })), + }); + expect(notifications.showError).toHaveBeenCalledWith('1 item could not be deleted. 2 items were not attempted.'); + expect(notifications.showError).toHaveBeenCalledTimes(1); + expect(notifications.showSuccess).not.toHaveBeenCalled(); + expect(notifications.showWarning).not.toHaveBeenCalled(); + expect(Object.keys(batch().selectedById)).toEqual(rows.map(({ id }) => id)); + expect(currentRows()).toEqual(rows); + expect(batchDeleteSelectionView(batch(), config.entity).busy).toBe(false); + }); + + it('freezes the request, rejects duplicate submits, and preserves failures while refreshing the current query', () => { + selectRows(); + const snapshot = prepare(); + store.dispatch(actions.batchDeleteRequested({ snapshot })); + store.dispatch(actions.batchDeleteRequested({ snapshot: { ...snapshot, operationId: 'duplicate' } })); + store.dispatch(actions.toggleHeader({ items: rows.map(selectionItem) })); + store.dispatch(config.actions.actionSetSearchString({ searchString: 'latest query' })); + router.url = `${config.rootPath}?view=all#table`; + expect(data.batchDelete).toHaveBeenCalledTimes(1); + expect(data.batchDelete).toHaveBeenCalledWith(rows.map((row) => row.id)); + response.next({ + results: rows.map((row, index) => ({ + id: row.id, + outcome: index === 0 ? 'deleted' : index === 1 ? 'failed' : 'not_attempted', + })), + }); + expect(batch().selectedById[rows[0].id]).toBeUndefined(); + expect(Object.keys(batch().selectedById)).toEqual([rows[1].id, rows[2].id]); + const noun = { experiments: 'experiment', flags: 'feature flag', segments: 'segment' }[config.entity]; + expect(notifications.showWarning).toHaveBeenCalledWith( + `1 ${noun} deleted. 1 item could not be deleted. 1 item was not attempted.` + ); + expect(data[config.fetchMethod]).toHaveBeenLastCalledWith( + expect.objectContaining({ skip: 0, searchParams: expect.objectContaining({ string: 'latest query' }) }), + true + ); + expect(notifications.showWarning).toHaveBeenCalledTimes(1); + expect(notifications.showSuccess).not.toHaveBeenCalled(); + expect(notifications.showError).not.toHaveBeenCalled(); + expect(router.navigate).not.toHaveBeenCalled(); + }); + + it.each(['search', 'sort'])( + 'applies a user-requested %s while deletion is pending even if deletion fails', + (change) => { + selectRows(); + const snapshot = prepare(); + store.dispatch(actions.batchDeleteRequested({ snapshot })); + const pendingList = new Subject(); + data[config.fetchMethod].mockReturnValueOnce(pendingList); + if (change === 'search') { + store.dispatch(config.actions.actionSetSearchString({ searchString: 'b' })); + } else { + store.dispatch(config.actions.actionSetSortingType({ sortingType: SORT_AS_DIRECTION.DESCENDING })); + } + store.dispatch(config.fetch({ fromStarting: true })); + expect(data[config.fetchMethod]).toHaveBeenCalledTimes(2); + expect(data[config.fetchMethod]).toHaveBeenLastCalledWith( + expect.objectContaining( + change === 'search' + ? { searchParams: expect.objectContaining({ string: 'b' }) } + : { sortParams: expect.objectContaining({ sortAs: SORT_AS_DIRECTION.DESCENDING }) } + ), + false + ); + response.error({ status: 0 }); + expect(state[config.key][config.loadingKey]).toBe(true); + const resultRows = change === 'search' ? [rows[1]] : [...rows].reverse(); + pendingList.next(page(resultRows)); + pendingList.complete(); + expect(currentRows().map(({ id }) => id)).toEqual(resultRows.map(({ id }) => id)); + expect(batch().loadedIds).toEqual(resultRows.map(({ id }) => id)); + expect(batch().listLoading).toBe(false); + expect(state[config.key][config.loadingKey]).toBe(false); + expect(Object.keys(batch().selectedById)).toHaveLength(3); + expect(data[config.fetchMethod]).toHaveBeenCalledTimes(2); + + expect(notifications.showWarning).not.toHaveBeenCalled(); + expect(notifications.showSuccess).not.toHaveBeenCalled(); + } + ); + + it('keeps the displayed confirmation snapshot when updated rows arrive', () => { + selectRows(2); + const snapshot = prepare(); + data[config.fetchMethod].mockReturnValueOnce( + of( + page([ + { + ...rows[0], + name: 'renamed', + state: config.entity === 'experiments' ? EXPERIMENT_STATE.RUNNING : undefined, + status: config.entity === 'segments' ? SEGMENT_STATUS.USED : FEATURE_FLAG_STATUS.ENABLED, + }, + ]) + ) + ); + store.dispatch(config.fetch({ fromStarting: true })); + expect(batch().confirmation).toBe(snapshot); + expect(snapshot.items.map(({ name }) => name)).toEqual(['a', 'b']); + expect(batch().selectedById[rows[0].id]).toEqual(selectionItem(rows[0])); + + store.dispatch(actions.batchDeleteRequested({ snapshot })); + expect(data.batchDelete).toHaveBeenCalledWith([rows[0].id, rows[1].id]); + }); + + it.each([1, 3])('reports %i deletions once and ignores stale reads started before or during deletion', (count) => { + const options = rows.map(({ id, name }) => ({ id, name, context: 'home' })); + store.dispatch(experimentActions.actionFetchAllExperimentNamesSuccess({ allExperimentNames: options })); + store.dispatch(segmentActions.actionFetchListSegmentOptionsSuccess({ listSegmentOptions: options })); + selectRows(count); + const snapshot = prepare(); + const pending = new Subject(); + data[config.fetchMethod].mockReturnValueOnce(pending); + store.dispatch(config.fetch({ fromStarting: true })); + const oldRequestId = batch().listRequestId; + store.dispatch(actions.batchDeleteRequested({ snapshot })); + const duringDeletion = new Subject(); + data[config.fetchMethod].mockReturnValueOnce(duringDeletion); + store.dispatch(config.fetch({ fromStarting: true })); + response.next({ results: rows.slice(0, count).map(({ id }) => ({ id, outcome: 'deleted' })) }); + pending.next(page()); + duringDeletion.next(page()); + const oldSuccess = events.find( + (action: any) => action.batchListRequestId && action.type.includes('Success') + ) as any; + const completedState = state[config.key]; + store.dispatch({ ...oldSuccess, batchListRequestId: oldRequestId }); + expect(state[config.key]).toBe(completedState); + store.dispatch({ type: '[Test] Unrelated action after deletion' }); + expect(state[config.key]).toBe(completedState); + expect(currentRows().some((row) => row.id === rows[0].id)).toBe(false); + expect(state.experiments.allExperimentNames).toEqual( + config.entity === 'experiments' ? options.slice(count) : options + ); + expect(state.segments.listSegmentOptions).toEqual(config.entity === 'segments' ? options.slice(count) : options); + const eventTypes = events.map((action) => action.type); + expect(eventTypes).not.toContain(experimentActions.actionFetchAllExperimentNames.type); + expect(eventTypes).not.toContain(segmentActions.actionFetchListSegmentOptions.type); + expect(notifications.showSuccess).toHaveBeenCalledTimes(1); + const noun = { experiments: 'experiment', flags: 'feature flag', segments: 'segment' }[config.entity]; + expect(notifications.showSuccess).toHaveBeenCalledWith(`${count} ${noun}${count === 1 ? '' : 's'} deleted.`); + }); + + it.each([0, 500])( + 'restores cancelled replacement rows after HTTP %i without another request or snackbar', + (status) => { + selectRows(); + const pendingList = new Subject(); + data[config.fetchMethod].mockReturnValueOnce(pendingList); + store.dispatch(config.actions.actionSetSearchString({ searchString: 'b' })); + store.dispatch(config.fetch({ fromStarting: true })); + expect(state[config.key][config.loadingKey]).toBe(true); + expect(batch().loadedIds).toEqual([]); + const fetchCount = data[config.fetchMethod].mock.calls.length; + const beforeRows = currentRows(); + const snapshot = prepare(); + store.dispatch(actions.batchDeleteRequested({ snapshot })); + expect(pendingList.observed).toBe(false); + response.error({ status }); + pendingList.next(page([])); + pendingList.complete(); + expect(batch().operation.status).toBe('complete'); + expect(batch().listLoading).toBe(false); + expect(state[config.key][config.loadingKey]).toBe(false); + expect(currentRows()).toEqual(beforeRows); + expect(batch().loadedIds).toEqual(beforeRows.map(({ id }) => id)); + + expect(data[config.fetchMethod]).toHaveBeenCalledTimes(fetchCount); + expect(notifications.showWarning).not.toHaveBeenCalled(); + expect(notifications.showSuccess).not.toHaveBeenCalled(); + expect(notifications.showError).not.toHaveBeenCalled(); + expect(Object.keys(batch().selectedById)).toHaveLength(3); + expect(data.batchDelete).toHaveBeenCalledTimes(1); + store.dispatch(actions.toggleRow({ item: selectionItem(rows[0]) })); + expect(batch().selectedById[rows[0].id]).toBeUndefined(); + store.dispatch(actions.toggleRow({ item: selectionItem(rows[0]) })); + expect(batch().selectedById[rows[0].id]).toBeDefined(); + store.dispatch(actions.toggleHeader({ items: rows.map(selectionItem) })); + expect(Object.keys(batch().selectedById)).toHaveLength(0); + store.dispatch(actions.toggleHeader({ items: rows.map(selectionItem) })); + expect(Object.keys(batch().selectedById)).toHaveLength(3); + } + ); + + it('shows the existing not-found state for a deleted detail, including a late detail response', () => { + selectRows(1); + const snapshot = prepare(); + store.dispatch(actions.batchDeleteRequested({ snapshot })); + const viewed = rows[0]; + router.url = `${config.rootPath}/detail/${viewed.id}`; + store.dispatch(actions.rootPageLeft()); + const detailLoaded = + config.entity === 'experiments' + ? experimentActions.actionGetExperimentByIdSuccess({ experiment: viewed as any }) + : config.entity === 'flags' + ? flagActions.actionFetchFeatureFlagByIdSuccess({ flag: viewed as any }) + : segmentActions.actionGetSegmentByIdSuccess({ + segment: viewed as any, + experimentSegmentInclusion: [], + experimentSegmentExclusion: [], + featureFlagSegmentInclusion: [], + featureFlagSegmentExclusion: [], + allParentSegments: [], + }); + const errorSelector = + config.entity === 'experiments' + ? selectExperimentDetailsPageError + : config.entity === 'flags' + ? selectFeatureFlagDetailsPageError + : selectSegmentDetailsPageError; + const detailsError = (id = viewed.id) => + errorSelector.projector( + { state: { params: { experimentId: id, flagId: id, segmentId: id } } } as any, + state[config.key] + ); + store.dispatch(detailLoaded); + expect(detailsError()).toBeNull(); + response.next({ results: [{ id: viewed.id, outcome: 'deleted' }] }); + const notFound = { entityId: viewed.id, errorType: PAGE_ERROR_TYPE.NOT_FOUND }; + expect(detailsError()).toEqual(notFound); + store.dispatch(detailLoaded); + expect(detailsError()).toEqual(notFound); + expect(detailsError(rows[1].id)).toBeNull(); + expect(router.navigate).not.toHaveBeenCalled(); + }); + + it.each(['deletion', 'refresh'])('preserves open details when leaving during %s', (pending) => { + selectRows(1); + const snapshot = prepare(); + store.dispatch(actions.batchDeleteRequested({ snapshot })); + const pendingRefresh = new Subject(); + data[config.fetchMethod].mockReturnValue(pendingRefresh); + const result = { results: [{ id: rows[0].id, outcome: 'deleted' }] }; + if (pending === 'refresh') { + response.next(result); + expect(pendingRefresh.observed).toBe(true); + } + + const viewed = { ...rows[2], description: 'Loaded detail' }; + router.url = `${config.rootPath}/detail/${viewed.id}`; + store.dispatch(actions.rootPageLeft()); + if (config.entity === 'experiments') { + store.dispatch(experimentActions.actionGetExperimentByIdSuccess({ experiment: viewed as any })); + } else if (config.entity === 'flags') { + store.dispatch(flagActions.actionFetchFeatureFlagByIdSuccess({ flag: viewed as any })); + } else { + store.dispatch( + segmentActions.actionGetSegmentByIdSuccess({ + segment: viewed as any, + experimentSegmentInclusion: [], + experimentSegmentExclusion: [], + featureFlagSegmentInclusion: [], + featureFlagSegmentExclusion: [], + allParentSegments: [], + }) + ); + } + const selectedDetail = () => + config.entity === 'experiments' + ? selectSelectedExperiment.projector( + { state: { params: { experimentId: viewed.id } } } as any, + state.experiments + ) + : config.entity === 'flags' + ? selectSelectedFeatureFlag.projector({ state: { params: { flagId: viewed.id } } } as any, state.featureFlags) + : selectSelectedSegment.projector( + { state: { params: { segmentId: viewed.id } } } as any, + state.segments.segments + ); + expect(selectedDetail()?.id).toBe(viewed.id); + if (pending === 'deletion') response.next(result); + pendingRefresh.next(page([rows[1]])); + pendingRefresh.complete(); + expect(selectedDetail()?.id).toBe(viewed.id); + expect(batch().operation.status).toBe('complete'); + expect(batch().removedIds).toContain(rows[0].id); + expect(batch().listLoading).toBe(false); + expect(state[config.key][config.loadingKey]).toBe(false); + expect(Object.keys(batch().selectedById)).toEqual([]); + expect(data[config.fetchMethod]).toHaveBeenCalledTimes(pending === 'refresh' ? 2 : 1); + expect(notifications.showSuccess).toHaveBeenCalledTimes(1); + expect(router.navigate).not.toHaveBeenCalled(); + if (config.entity === 'experiments') { + expect(events).toEqual( + expect.arrayContaining([experimentActions.actionFetchAllDecisionPoints(), analysisActions.actionFetchMetrics()]) + ); + } + + router.url = `${config.rootPath}?view=all#table`; + data[config.fetchMethod].mockReturnValue(of(page([rows[1], viewed]))); + store.dispatch(config.fetch({ fromStarting: true })); + expect(currentRows().map(({ id }) => id)).toEqual([rows[1].id, viewed.id]); + expect(batch().loadedIds).toEqual([rows[1].id, viewed.id]); + }); + + it('keeps confirmed deletion when the post-commit cleanup or list refresh fails', () => { + selectRows(1); + const snapshot = prepare(); + data[config.fetchMethod].mockReturnValue(throwError(() => new Error('refresh unavailable'))); + store.dispatch(actions.batchDeleteRequested({ snapshot })); + response.next({ + results: [{ id: rows[0].id, outcome: 'deleted', reasonCode: 'post_delete_failed' }], + }); + const noun = { experiments: 'experiment', flags: 'feature flag', segments: 'segment' }[config.entity]; + expect(notifications.showWarning).toHaveBeenCalledWith(`1 ${noun} deleted. An error occurred after deletion.`); + expect(batch().selectedById[rows[0].id]).toBeUndefined(); + expect(batch().operation.result.results[0].outcome).toBe('deleted'); + expect(batch().listLoading).toBe(false); + expect(batch().loadedIds).toEqual([rows[1].id, rows[2].id]); + store.dispatch(actions.toggleRow({ item: selectionItem(rows[1]) })); + expect(batch().selectedById[rows[1].id]).toBeDefined(); + expect(notifications.showWarning).toHaveBeenCalledTimes(1); + expect(notifications.showSuccess).not.toHaveBeenCalled(); + expect(notifications.showError).not.toHaveBeenCalled(); + + // Scrolling retries from offset zero after the replacement request failed. + data[config.fetchMethod].mockReturnValue(of(page([rows[1]]))); + store.dispatch(config.fetch({ fromStarting: false })); + expect(data[config.fetchMethod]).toHaveBeenLastCalledWith(expect.objectContaining({ skip: 0 }), false); + // A zero offset replaces the retained rows instead of appending the first page to them. + expect(currentRows().map(({ id }) => id)).toEqual([rows[1].id]); + expect(batch().loadedIds).toEqual(currentRows().map(({ id }) => id)); + store.dispatch(actions.toggleRow({ item: selectionItem(rows[1]) })); + store.dispatch(actions.toggleHeader({ items: currentRows().map(selectionItem) })); + expect(batchDeleteSelectionView(batch(), config.entity)).toMatchObject({ checked: true, indeterminate: false }); + }); + + it('retains an unknown item after refreshing the list and reports the confirmed results once', () => { + selectRows(); + const snapshot = prepare(); + const fetchCount = data[config.fetchMethod].mock.calls.length; + data[config.fetchMethod].mockReturnValueOnce(of(page([rows[2]]))); + store.dispatch(actions.batchDeleteRequested({ snapshot })); + response.next({ + results: [ + { id: rows[0].id, outcome: 'deleted' }, + { id: rows[1].id, outcome: 'unknown' }, + { id: rows[2].id, outcome: 'not_attempted' }, + ], + }); + expect(batch().operation.status).toBe('complete'); + expect(Object.keys(batch().selectedById)).toEqual([rows[1].id, rows[2].id]); + expect(batchDeleteResultCounts(batch())).toMatchObject({ deleted: 1, absent: 0, uncertain: true, notAttempted: 1 }); + expect(batchDeleteSelectionView(batch(), config.entity).busy).toBe(false); + const noun = { experiments: 'experiment', flags: 'feature flag', segments: 'segment' }[config.entity]; + expect(notifications.showWarning).toHaveBeenCalledWith( + `1 ${noun} deleted. 1 item was not attempted. Deletion could not be confirmed for some items.` + ); + expect(notifications.showWarning).toHaveBeenCalledTimes(1); + expect(notifications.showSuccess).not.toHaveBeenCalled(); + expect(notifications.showError).not.toHaveBeenCalled(); + expect(data.batchDelete).toHaveBeenCalledTimes(1); + expect(data[config.fetchMethod]).toHaveBeenCalledTimes(fetchCount + 1); + }); + + it('retains uncertain selections and reports an error once about a malformed deletion response', () => { + selectRows(); + const snapshot = prepare(); + store.dispatch(actions.batchDeleteRequested({ snapshot })); + response.next({ results: [] }); + expect(batch().operation.result.results.every((result) => result.outcome === 'unknown')).toBe(true); + expect(Object.keys(batch().selectedById)).toHaveLength(3); + expect(batch().operation.status).toBe('complete'); + expect(batchDeleteSelectionView(batch(), config.entity).busy).toBe(false); + expect(data.batchDelete).toHaveBeenCalledTimes(1); + expect(notifications.showSuccess).not.toHaveBeenCalled(); + expect(notifications.showWarning).not.toHaveBeenCalled(); + expect(notifications.showError).toHaveBeenCalledWith('Deletion could not be confirmed for some items.'); + expect(notifications.showError).toHaveBeenCalledTimes(1); + }); + + it('requires a new confirmation and sends only retained IDs on an explicit retry', () => { + selectRows(); + const first = prepare(); + store.dispatch(actions.batchDeleteRequested({ snapshot: first })); + response.next({ + results: rows.map(({ id }, index) => ({ id, outcome: index === 0 ? 'deleted' : 'not_attempted' })), + }); + response.complete(); + store.dispatch(actions.batchDeleteRequested({ snapshot: first })); + expect(data.batchDelete).toHaveBeenCalledTimes(1); + const second = prepare(); + expect(second.operationId).not.toBe(first.operationId); + data.batchDelete.mockReturnValue(new Subject()); + store.dispatch(actions.batchDeleteRequested({ snapshot: second })); + expect(data.batchDelete).toHaveBeenLastCalledWith([rows[1].id, rows[2].id]); + }); + + it('clears state on logout and ignores a late deletion response in the next session', () => { + selectRows(); + const snapshot = prepare(); + store.dispatch(actions.batchDeleteRequested({ snapshot })); + store.dispatch(actionLogoutStart()); + store.dispatch(actionSetUserInfo({ user: { email: 'next@example.com', role: UserRole.ADMIN } })); + response.next({ results: rows.map(({ id }) => ({ id, outcome: 'deleted' })) }); + expect(batch().operation).toBeNull(); + expect(Object.keys(batch().selectedById)).toHaveLength(0); + expect(batch().removedIds).toHaveLength(0); + expect(notifications.showSuccess).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/frontend/projects/upgrade/src/app/core/batch-actions/batch-actions.models.ts b/packages/frontend/projects/upgrade/src/app/core/batch-actions/batch-actions.models.ts new file mode 100644 index 0000000000..6ea8c3a6cb --- /dev/null +++ b/packages/frontend/projects/upgrade/src/app/core/batch-actions/batch-actions.models.ts @@ -0,0 +1,49 @@ +import { BatchDeleteResult, EXPERIMENT_STATE, FEATURE_FLAG_STATUS, SEGMENT_STATUS } from 'upgrade_types'; + +export enum DeletionEligibilityReasonCode { + EXPERIMENT_ACTIVE = 'experiment_active', + FEATURE_FLAG_ENABLED = 'feature_flag_enabled', + SEGMENT_USED = 'segment_used', +} + +export interface RootSelectionItem { + id: string; + name?: string; + stateOrStatus?: EXPERIMENT_STATE | FEATURE_FLAG_STATUS | SEGMENT_STATUS; +} + +export interface BatchDeleteSnapshot { + operationId: string; + items: RootSelectionItem[]; +} + +export interface RootBatchDeleteState { + selectedById: Record; + userEmail: string | null; + loadedIds: string[]; + listRequestId: string | null; + listLoading: boolean; + removedIds: string[]; + confirmation: BatchDeleteSnapshot | null; + operation: { + snapshot: BatchDeleteSnapshot; + status: 'submitting' | 'complete'; + result?: BatchDeleteResult; + transportStatus?: number; + } | null; +} + +export const initialRootBatchDeleteState: RootBatchDeleteState = { + selectedById: {}, + userEmail: null, + loadedIds: [], + listRequestId: null, + listLoading: false, + removedIds: [], + confirmation: null, + operation: null, +}; + +let requestSequence = 0; +export const newBatchRequestId = () => `batch-${++requestSequence}`; +export const isBatchDeleteBusy = (state: RootBatchDeleteState) => state.operation?.status === 'submitting'; diff --git a/packages/frontend/projects/upgrade/src/app/core/batch-actions/batch-actions.reducer.ts b/packages/frontend/projects/upgrade/src/app/core/batch-actions/batch-actions.reducer.ts new file mode 100644 index 0000000000..c4e8eedd60 --- /dev/null +++ b/packages/frontend/projects/upgrade/src/app/core/batch-actions/batch-actions.reducer.ts @@ -0,0 +1,157 @@ +import { Action } from '@ngrx/store'; +import { BatchDeleteEntity } from 'upgrade_types'; +import * as auth from '../auth/store/auth.actions'; +import { RootBatchDeleteActions } from './batch-actions.actions'; +import { + RootBatchDeleteState, + RootSelectionItem, + initialRootBatchDeleteState, + isBatchDeleteBusy, +} from './batch-actions.models'; +import { confirmedRemovedIds, batchDeleteSelectionView } from './batch-actions.helpers'; + +function matches( + action: Action, + creator: C +): action is ReturnType { + return action.type === creator.type; +} + +function invalidateSelection(state: RootBatchDeleteState): RootBatchDeleteState { + return { ...state, confirmation: null }; +} + +function removeConfirmed(state: RootBatchDeleteState, ids: string[]): RootBatchDeleteState { + if (!ids.length) return state; + const removed = new Set(ids); + return { + ...invalidateSelection(state), + removedIds: [...new Set([...state.removedIds, ...ids])], + selectedById: Object.fromEntries(Object.entries(state.selectedById).filter(([id]) => !removed.has(id))), + loadedIds: state.loadedIds.filter((id) => !removed.has(id)), + }; +} + +export function receiveListRows( + state: RootBatchDeleteState, + items: RootSelectionItem[], + fromStarting: boolean +): RootBatchDeleteState { + const removed = new Set(state.removedIds); + const rows = items.filter((item) => !removed.has(item.id)); + return { + // Keep selection-time metadata when searches or later pages update the displayed rows. + ...state, + loadedIds: [...new Set([...(fromStarting ? [] : state.loadedIds), ...rows.map((item) => item.id)])], + listLoading: false, + }; +} + +export function reduceRootBatchDelete( + state: RootBatchDeleteState, + action: Action, + actions: RootBatchDeleteActions, + entity: BatchDeleteEntity +): RootBatchDeleteState { + if (matches(action, auth.actionLogoutStart) || matches(action, auth.actionLogoutSuccess)) + return initialRootBatchDeleteState; + if ( + matches(action, auth.actionSetUserInfo) || + matches(action, auth.actionSetUserInfoSuccess) || + matches(action, auth.actionLoginSuccess) + ) { + const email = action.user?.email || null; + return state.userEmail === email ? state : { ...initialRootBatchDeleteState, userEmail: email }; + } + if (matches(action, actions.listRequested)) + return { + ...state, + listRequestId: action.requestId, + // Keep IDs for displayed rows until replacement rows arrive. Query changes clear them separately. + loadedIds: state.loadedIds, + listLoading: true, + }; + if (matches(action, actions.listFailed)) + return action.requestId === state.listRequestId ? { ...state, listLoading: false } : state; + if (matches(action, actions.confirmedRemoved)) return removeConfirmed(state, action.ids); + // Cancel root reads before they can replace detail data; submitted deletion remains observable. + if (matches(action, actions.rootPageLeft)) + return { ...invalidateSelection(state), selectedById: {}, listRequestId: null, listLoading: false }; + if (matches(action, actions.toggleHeader) || matches(action, actions.toggleRow)) { + if (isBatchDeleteBusy(state)) return state; + let selectedById = { ...state.selectedById }; + if (matches(action, actions.toggleHeader) && Object.keys(selectedById).length) { + selectedById = {}; + } else { + const items = matches(action, actions.toggleRow) + ? [action.item] + : matches(action, actions.toggleHeader) + ? action.items + : []; + const loadedIds = new Set(state.loadedIds); + for (const item of items) { + if (selectedById[item.id] && matches(action, actions.toggleRow)) delete selectedById[item.id]; + else if (loadedIds.has(item.id)) { + selectedById[item.id] = { ...item }; + } + } + } + return { ...invalidateSelection(state), selectedById }; + } + if (matches(action, actions.prepareConfirmation)) { + const selection = batchDeleteSelectionView(state, entity); + if (state.confirmation || !selection.canRequestConfirmation) return state; + // Use retained row metadata, including hidden selections. UI availability is based on this cached data. + return { + ...state, + confirmation: { + operationId: action.operationId, + items: selection.items.map((item) => ({ ...item })), + }, + }; + } + if (matches(action, actions.dismissConfirmation)) + return isBatchDeleteBusy(state) ? state : { ...state, confirmation: null }; + if (matches(action, actions.batchDeleteRequested)) { + const snapshot = state.confirmation; + if (isBatchDeleteBusy(state) || !snapshot || snapshot.operationId !== action.snapshot.operationId) return state; + return { + ...state, + listRequestId: null, + listLoading: false, + confirmation: null, + operation: { snapshot, status: 'submitting' }, + }; + } + if (matches(action, actions.batchDeleteCompleted) || matches(action, actions.batchDeleteRequestFailed)) { + if ( + !state.operation || + action.operationId !== state.operation.snapshot.operationId || + state.operation.status === 'complete' + ) + return state; + if (matches(action, actions.batchDeleteCompleted)) { + const next = removeConfirmed(state, confirmedRemovedIds(action.result)); + return { + ...invalidateSelection(next), + operation: { + ...state.operation, + result: action.result, + status: 'complete', + }, + }; + } + if (matches(action, actions.batchDeleteRequestFailed)) { + return { + ...invalidateSelection(state), + operation: { + ...state.operation, + transportStatus: action.status, + // The shared HTTP interceptor reports request errors. Release controls without follow-up requests. + status: 'complete', + }, + }; + } + } + return state; +} diff --git a/packages/frontend/projects/upgrade/src/app/core/batch-actions/batch-actions.store.ts b/packages/frontend/projects/upgrade/src/app/core/batch-actions/batch-actions.store.ts new file mode 100644 index 0000000000..b6a990f687 --- /dev/null +++ b/packages/frontend/projects/upgrade/src/app/core/batch-actions/batch-actions.store.ts @@ -0,0 +1,120 @@ +import { Action, ActionReducer } from '@ngrx/store'; +import { BatchDeleteEntity } from 'upgrade_types'; +import { RootBatchDeleteActions } from './batch-actions.actions'; +import { RootBatchDeleteState, isBatchDeleteBusy } from './batch-actions.models'; +import { receiveListRows, reduceRootBatchDelete } from './batch-actions.reducer'; +import { selectionItem } from './batch-actions.helpers'; + +interface RootBatchDeleteConfig { + entity: BatchDeleteEntity; + actions: RootBatchDeleteActions; + rowsKey: string; + loadingKey: string; + skipKey: string; + totalKey: string; + queryTypes: string[]; + listSuccessType: string; + responseRowsKey: string; + deletedId: (action: Action) => string | undefined; +} + +/** + * Each entity's existing feature store owns rootBatch; this wrapper composes its deletion lifecycle + * with that feature's reducer. Shared effects run in the existing entity effect classes, not a separate store. + */ +export function withRootBatchDelete( + reducer: ActionReducer, + initialState: S, + config: RootBatchDeleteConfig +): ActionReducer { + return (input, action: Action & { batchListRequestId?: string; fromStarting?: boolean }) => { + const state = input || initialState; + if (action.batchListRequestId && action.batchListRequestId !== state.rootBatch.listRequestId) return state; + let rootBatch = reduceBatchDeleteAction(state.rootBatch, action, config); + let result = reducer(state, action); + if (action.type === config.listSuccessType) { + rootBatch = receiveListRows(rootBatch, action[config.responseRowsKey].map(selectionItem), !!action.fromStarting); + } + // A cancelled tracked read cannot dispatch its usual success/failure action to clear loading. + if (state.rootBatch.listLoading && state.rootBatch.listRequestId && !rootBatch.listRequestId) + result = { ...result, [config.loadingKey]: false }; + if (action.type === config.listSuccessType) result = deduplicateRows(result, config.rowsKey); + result = removeConfirmedData(result, rootBatch.removedIds, config.rowsKey); + if ( + (action.type === config.actions.listFailed.type || action.type === config.actions.batchDeleteRequested.type) && + rootBatch !== state.rootBatch + ) { + // Failed reads and reads cancelled by submission leave the displayed rows available for selection. + rootBatch = { ...rootBatch, loadedIds: result[config.rowsKey].map((row) => row.id) }; + } + result = resetPaginationAfterDeletion(result, state.rootBatch, rootBatch, config); + return rootBatch === state.rootBatch && result === state ? state : { ...result, rootBatch }; + }; +} + +function reduceBatchDeleteAction( + state: RootBatchDeleteState, + action: Action, + config: RootBatchDeleteConfig +): RootBatchDeleteState { + let result = reduceRootBatchDelete(state, action, config.actions, config.entity); + const deletedId = config.deletedId(action); + if (deletedId) + result = reduceRootBatchDelete( + result, + config.actions.confirmedRemoved({ ids: [deletedId] }), + config.actions, + config.entity + ); + if (config.queryTypes.includes(action.type)) + result = { ...result, loadedIds: [], listRequestId: null, listLoading: false }; + return result; +} + +function deduplicateRows(state: S, rowsKey: string): S { + const rows = state[rowsKey]; + const distinct = new Map(rows.map((row) => [row.id, row])); + return distinct.size === rows.length ? state : { ...state, [rowsKey]: [...distinct.values()] }; +} + +/** Keep IDs confirmed deleted or absent out of state, including data from late detail/stat responses. */ +function removeConfirmedData(state: S, removedIds: string[], rowsKey: string): S { + if (!removedIds.length) return state; + let result = state; + const removed = new Set(removedIds); + const prune = (key: string) => { + const rows = result[key]; + if (Array.isArray(rows) && rows.some((row) => removed.has(row.id))) + result = { ...result, [key]: rows.filter((row) => !removed.has(row.id)) }; + }; + prune(rowsKey); + prune('allExperimentNames'); + prune('listSegmentOptions'); + if (removed.has(result['selectedFlag']?.id)) result = { ...result, selectedFlag: null }; + for (const key of ['stats', 'rewardsSummaries']) { + if (result[key] && Object.keys(result[key]).some((id) => removed.has(id))) { + result = { + ...result, + [key]: Object.fromEntries(Object.entries(result[key]).filter(([id]) => !removed.has(id))), + }; + } + } + return result; +} + +function resetPaginationAfterDeletion( + state: S, + previous: RootBatchDeleteState, + current: RootBatchDeleteState, + config: RootBatchDeleteConfig +): S { + if ( + current.removedIds.length > previous.removedIds.length || + (isBatchDeleteBusy(previous) && + current.operation?.status === 'complete' && + current.operation.transportStatus === undefined) + ) { + return { ...state, [config.skipKey]: 0, [config.totalKey]: null }; + } + return state; +} diff --git a/packages/frontend/projects/upgrade/src/app/core/experiments/experiments.data.service.ts b/packages/frontend/projects/upgrade/src/app/core/experiments/experiments.data.service.ts index e471ec11e9..a43c9c7485 100644 --- a/packages/frontend/projects/upgrade/src/app/core/experiments/experiments.data.service.ts +++ b/packages/frontend/projects/upgrade/src/app/core/experiments/experiments.data.service.ts @@ -1,3 +1,5 @@ +import { BatchDeleteResult } from 'upgrade_types'; +import { batchHttpContext } from '../batch-actions/batch-actions.http'; import { Injectable } from '@angular/core'; import { Experiment, @@ -19,11 +21,19 @@ import { IImportFile, LIST_FILTER_MODE, ExperimentRewardsSummary } from 'upgrade @Injectable() export class ExperimentDataService { + batchDelete(ids: string[]): Observable { + return this.http.post( + API_ENDPOINTS.experimentsBatchDelete, + { ids }, + { context: batchHttpContext() } + ); + } + constructor(private http: HttpClient) {} - getAllExperiment(params: ExperimentPaginationParams) { + getAllExperiment(params: ExperimentPaginationParams, batchRefresh = false) { const url = API_ENDPOINTS.getAllExperiments; - return this.http.post(url, params); + return batchRefresh ? this.http.post(url, params, { context: batchHttpContext() }) : this.http.post(url, params); } getAllExperimentsStats(experimentIds: string[]) { diff --git a/packages/frontend/projects/upgrade/src/app/core/experiments/experiments.service.ts b/packages/frontend/projects/upgrade/src/app/core/experiments/experiments.service.ts index 0f77bd74a9..d0bf6562fc 100644 --- a/packages/frontend/projects/upgrade/src/app/core/experiments/experiments.service.ts +++ b/packages/frontend/projects/upgrade/src/app/core/experiments/experiments.service.ts @@ -1,3 +1,6 @@ +import { createBatchDeleteFacade } from '../batch-actions/batch-actions.facade'; +import { batchActions } from './store/experiments.actions'; +import { selectRootBatch } from './store/experiments.selectors'; import { Injectable } from '@angular/core'; import { Observable, combineLatest } from 'rxjs'; import { @@ -68,6 +71,14 @@ import { selectCurrentUserEmail } from '../auth/store/auth.selectors'; @Injectable() export class ExperimentService { + readonly batch = createBatchDeleteFacade( + this.store$, + 'experiments', + batchActions, + selectRootBatch, + selectAllExperiment + ); + constructor(private readonly store$: Store, private readonly localStorageService: LocalStorageService) {} experiments$: Observable = this.store$.pipe(select(selectAllExperiment)); diff --git a/packages/frontend/projects/upgrade/src/app/core/experiments/store/experiments.actions.ts b/packages/frontend/projects/upgrade/src/app/core/experiments/store/experiments.actions.ts index ff95af4096..65fd9607c9 100644 --- a/packages/frontend/projects/upgrade/src/app/core/experiments/store/experiments.actions.ts +++ b/packages/frontend/projects/upgrade/src/app/core/experiments/store/experiments.actions.ts @@ -1,3 +1,4 @@ +import { createBatchDeleteActions } from '../../batch-actions/batch-actions.actions'; import { createAction, props } from '@ngrx/store'; import { PAGE_ERROR_TYPE } from '@shared-component-lib/common-page-error/common-page-error.model'; import { @@ -21,11 +22,14 @@ import { import { ExperimentSegmentListRequest } from '../../segments/store/segments.model'; import { ExperimentRewardsSummary } from 'upgrade_types'; -export const actionGetExperiments = createAction('[Experiment] Get Experiments', props<{ fromStarting?: boolean }>()); +export const actionGetExperiments = createAction( + '[Experiment] Get Experiments', + props<{ fromStarting?: boolean; batchRefresh?: boolean }>() +); export const actionGetExperimentsSuccess = createAction( '[Experiment] Get Experiments Success', - props<{ experiments: Experiment[]; totalExperiments: number; fromStarting?: boolean }>() + props<{ batchListRequestId?: string; experiments: Experiment[]; totalExperiments: number; fromStarting?: boolean }>() ); export const actionGetExperimentsFailure = createAction('[Experiment] Get Experiment Failure', props<{ error: any }>()); @@ -154,11 +158,6 @@ export const actionUpdateExperimentConditionsFailure = createAction( '[Experiment] Update Experiment Conditions Failure' ); -export const actionSetSkipExperiment = createAction( - '[Experiment] Set Skip Experiment Value', - props<{ skipExperiment: number }>() -); - export const actionSetSearchKey = createAction( '[Experiment] Set Search key value', props<{ searchKey: EXPERIMENT_SEARCH_KEY }>() @@ -428,3 +427,5 @@ export const actionFetchRewardsDataForExperimentFailure = createAction( '[Experiment] Fetch Rewards Data For Experiment Failure', props<{ error: any }>() ); + +export const batchActions = createBatchDeleteActions('experiments'); diff --git a/packages/frontend/projects/upgrade/src/app/core/experiments/store/experiments.effects.spec.ts b/packages/frontend/projects/upgrade/src/app/core/experiments/store/experiments.effects.spec.ts index 0f82d15cbf..738c719fc3 100644 --- a/packages/frontend/projects/upgrade/src/app/core/experiments/store/experiments.effects.spec.ts +++ b/packages/frontend/projects/upgrade/src/app/core/experiments/store/experiments.effects.spec.ts @@ -1,3 +1,5 @@ +import { initialState as initialExperimentState } from './experiments.reducer'; +import { batchActions } from './experiments.actions'; import { fakeAsync, tick } from '@angular/core/testing'; import { ActionsSubject } from '@ngrx/store'; import { BehaviorSubject, of, throwError, timer } from 'rxjs'; @@ -26,7 +28,6 @@ import { actionFetchExperimentDetailStatSuccess, actionFetchExperimentDetailStat, actionGetExperimentsSuccess, - actionSetSkipExperiment, actionFetchExperimentGraphInfo, actionUpsertExperiment, actionUpsertExperimentFailure, @@ -61,14 +62,7 @@ import { actionAddExperimentExclusionListSuccess, } from './experiments.actions'; import { ExperimentEffects } from './experiments.effects'; -import { - DATE_RANGE, - EXPERIMENT_SEARCH_KEY, - SORT_AS_DIRECTION, - EXPERIMENT_SORT_KEY, - EXPERIMENT_STATE, - UpsertExperimentType, -} from './experiments.model'; +import { DATE_RANGE, EXPERIMENT_SEARCH_KEY, EXPERIMENT_STATE, UpsertExperimentType } from './experiments.model'; import * as Selectors from './experiments.selectors'; import { environment } from '../../../../environments/environment'; import { actionExecuteQuery, actionFetchMetrics } from '../../analysis/store/analysis.actions'; @@ -130,122 +124,60 @@ describe('ExperimentEffects', () => { }); describe('#getPaginatedExperiment$', () => { - it('should catch and dispatch actionGetExperimentsFailure on error and exercise skip { - experimentDataService.getAllExperiment = jest.fn().mockReturnValue(throwError('testError')); - Selectors.selectSkipExperiment.setResult(0); - Selectors.selectTotalExperiment.setResult(1); - Selectors.selectSearchKey.setResult(EXPERIMENT_SEARCH_KEY.ALL); - Selectors.selectSortKey.setResult(EXPERIMENT_SORT_KEY.UPDATED_AT); - Selectors.selectSortAs.setResult(SORT_AS_DIRECTION.ASCENDING); - Selectors.selectSearchString.setResult('test'); - - service.getPaginatedExperiment$.subscribe((result: any) => { - tick(0); - - const failureAction = actionGetExperimentsFailure(result); - - expect(result).toEqual(failureAction); - }); - - actions$.next(actionGetExperiments({})); - })); - - it('should catch and dispatch actionGetExperimentsFailure on error and exercise total=null filter', fakeAsync(() => { - experimentDataService.getAllExperiment = jest.fn().mockReturnValue(throwError('testError')); - Selectors.selectSkipExperiment.setResult(0); - Selectors.selectTotalExperiment.setResult(null); - Selectors.selectSearchKey.setResult(EXPERIMENT_SEARCH_KEY.ALL); - Selectors.selectSortKey.setResult(EXPERIMENT_SORT_KEY.UPDATED_AT); - Selectors.selectSortAs.setResult(SORT_AS_DIRECTION.ASCENDING); - Selectors.selectSearchString.setResult('test'); - - service.getPaginatedExperiment$.subscribe((result: any) => { - tick(0); - - const failureAction = actionGetExperimentsFailure(result); - - expect(result).toEqual(failureAction); - }); - - actions$.next(actionGetExperiments({})); - })); - - it('should dispatch actionGetExperimentsSuccess and actionFetchExperimentStats when fromStaring is undefined', fakeAsync(() => { - const experiments = [ - { - id: 'test1', - } as any, - ]; - - const experimentIds = ['test1']; - const totalExperiments = 1; - - experimentDataService.getAllExperiment = jest.fn().mockReturnValue(of({ nodes: experiments, total: 1 })); - Selectors.selectSkipExperiment.setResult(0); - Selectors.selectTotalExperiment.setResult(1); - Selectors.selectSearchKey.setResult(EXPERIMENT_SEARCH_KEY.ALL); - Selectors.selectSortKey.setResult(EXPERIMENT_SORT_KEY.UPDATED_AT); - Selectors.selectSortAs.setResult(SORT_AS_DIRECTION.ASCENDING); - Selectors.selectSearchString.setResult('test'); - - service.getPaginatedExperiment$.pipe(take(2), pairwise()).subscribe((result: any) => { - tick(0); - - const successAction = actionGetExperimentsSuccess({ experiments, totalExperiments }); - const fetchAction = actionFetchExperimentStats({ experimentIds }); - - expect(result).toEqual([successAction, fetchAction]); - }); + beforeEach(() => { + store$.next({ experiments: { ...initialExperimentState, totalExperiments: 1, searchString: 'test' } }); + }); + it.each([1, null])('reports a tracked list failure when total is %s', (totalExperiments) => { + store$.next({ experiments: { ...initialExperimentState, totalExperiments } }); + const error = new Error('testError'); + experimentDataService.getAllExperiment = jest.fn().mockReturnValue(throwError(() => error)); + const results = []; + const subscription = service.getPaginatedExperiment$.subscribe((result) => results.push(result)); actions$.next(actionGetExperiments({})); - tick(0); - })); - - it('should dispatch actionSetSkipExperiment, actionGetExperimentsSuccess and actionFetchExperimentStats when fromStaring is true', fakeAsync(() => { - const experiments = [ - { - id: 'test1', - } as any, - ]; - - const experimentIds = ['test1']; - const totalExperiments = 1; - - experimentDataService.getAllExperiment = jest.fn().mockReturnValue(of({ nodes: experiments, total: 1 })); - Selectors.selectSkipExperiment.setResult(2); - Selectors.selectTotalExperiment.setResult(1); - Selectors.selectSearchKey.setResult(EXPERIMENT_SEARCH_KEY.ALL); - Selectors.selectSortKey.setResult(EXPERIMENT_SORT_KEY.UPDATED_AT); - Selectors.selectSortAs.setResult(SORT_AS_DIRECTION.ASCENDING); - Selectors.selectSearchString.setResult('test'); - - service.getPaginatedExperiment$ - .pipe( - take(3), - scan((acc, val) => { - acc.unshift(val); - acc.splice(3); - return acc; - }, []), - last() - ) - .subscribe((result: any) => { - tick(0); - - const skipAction = actionSetSkipExperiment({ skipExperiment: 0 }); - const successAction = actionGetExperimentsSuccess({ - experiments, - fromStarting: true, - totalExperiments, - }); - const fetchAction = actionFetchExperimentStats({ experimentIds }); + expect(results).toEqual([ + batchActions.listFailed({ requestId: expect.any(String) }), + actionGetExperimentsFailure({ error }), + ]); + subscription.unsubscribe(); + }); - expect(result.reverse()).toEqual([skipAction, successAction, fetchAction]); + it.each([false, true])( + 'correlates root rows with the request and fetches stats (replacement=%s)', + (fromStarting) => { + // A nonzero offset is required for pagination, since a zero offset is itself a replacement. + store$.next({ + experiments: { + ...initialExperimentState, + skipExperiment: 2, + totalExperiments: fromStarting ? 1 : 3, + searchString: 'test', + }, }); - - actions$.next(actionGetExperiments({ fromStarting: true })); - tick(0); - })); + const experiments = [{ id: 'test1' } as any]; + experimentDataService.getAllExperiment = jest.fn().mockReturnValue(of({ nodes: experiments, total: 1 })); + const results = []; + const subscription = service.getPaginatedExperiment$.subscribe((result) => results.push(result)); + actions$.next(actionGetExperiments({ fromStarting })); + expect(results).toEqual([ + actionGetExperimentsSuccess({ + experiments, + totalExperiments: 1, + fromStarting, + batchListRequestId: expect.any(String), + }), + actionFetchExperimentStats({ experimentIds: ['test1'] }), + ]); + expect(experimentDataService.getAllExperiment).toHaveBeenCalledWith( + expect.objectContaining({ + skip: fromStarting ? 0 : 2, + searchParams: { key: initialExperimentState.searchKey, string: 'test' }, + }), + false + ); + subscription.unsubscribe(); + } + ); }); describe('#fetchExperimentStatsForHome$', () => { diff --git a/packages/frontend/projects/upgrade/src/app/core/experiments/store/experiments.effects.ts b/packages/frontend/projects/upgrade/src/app/core/experiments/store/experiments.effects.ts index 02fb6e26e8..46820aed13 100644 --- a/packages/frontend/projects/upgrade/src/app/core/experiments/store/experiments.effects.ts +++ b/packages/frontend/projects/upgrade/src/app/core/experiments/store/experiments.effects.ts @@ -1,15 +1,20 @@ +import { + batchDeleteEffect, + batchDeleteFinishedEffect, + trackedListRequest, +} from '../../batch-actions/batch-actions.effects'; +import { selectRootBatch, selectExperimentState } from './experiments.selectors'; import { Inject, Injectable } from '@angular/core'; import { Actions, createEffect, ofType } from '@ngrx/effects'; import * as experimentAction from './experiments.actions'; import * as analysisActions from '../../analysis/store/analysis.actions'; import { ExperimentDataService } from '../experiments.data.service'; -import { map, filter, switchMap, catchError, tap, withLatestFrom, first, mergeMap, takeUntil } from 'rxjs/operators'; +import { map, filter, switchMap, catchError, tap, withLatestFrom, mergeMap, takeUntil } from 'rxjs/operators'; import { UpsertExperimentType, IExperimentEnrollmentStats, Experiment, NUMBER_OF_EXPERIMENTS, - ExperimentPaginationParams, IExperimentEnrollmentDetailStats, IContextMetaData, } from './experiments.model'; @@ -18,11 +23,6 @@ import { Store, select } from '@ngrx/store'; import { AppState, NotificationService } from '../../core.module'; import { selectExperimentStats, - selectSkipExperiment, - selectSearchKey, - selectSortAs, - selectSortKey, - selectTotalExperiment, selectSearchString, selectExperimentGraphInfo, selectContextMetaData, @@ -39,6 +39,35 @@ import { LIST_FILTER_MODE } from 'upgrade_types'; import { LIST_OPTION_TYPE } from '../../segments/store/segments.model'; @Injectable() export class ExperimentEffects { + batchDelete$ = createEffect(() => + batchDeleteEffect( + this.actions$, + this.store$.pipe(select(selectRootBatch)), + experimentAction.batchActions, + this.experimentDataService + ) + ); + finishBatchDelete$ = createEffect(() => + batchDeleteFinishedEffect( + this.actions$, + this.store$.pipe(select(selectRootBatch)), + experimentAction.batchActions, + { entity: 'experiments', translate: this.translate, service: this.notificationService }, + (counts) => { + const pathname = (this.router.url || '').split('?')[0].split('#')[0]; + return [ + // Detail selectors share the rows array; replace it only while the root table is displayed. + ...(pathname === '/home' + ? [experimentAction.actionGetExperiments({ fromStarting: true, batchRefresh: true })] + : []), + ...(counts.deleted || counts.absent + ? [experimentAction.actionFetchAllDecisionPoints(), analysisActions.actionFetchMetrics()] + : []), + ]; + } + ) + ); + constructor( private actions$: Actions, private store$: Store, @@ -54,59 +83,38 @@ export class ExperimentEffects { getPaginatedExperiment$ = createEffect(() => this.actions$.pipe( ofType(experimentAction.actionGetExperiments), - map((action) => action.fromStarting), - withLatestFrom( - this.store$.pipe(select(selectSkipExperiment)), - this.store$.pipe(select(selectTotalExperiment)), - this.store$.pipe(select(selectSearchKey)), - this.store$.pipe(select(selectSortKey)), - this.store$.pipe(select(selectSortAs)) + withLatestFrom(this.store$.pipe(select(selectExperimentState))), + filter( + ([action, state]) => + (!state.rootBatch.listLoading || action.fromStarting) && + (action.fromStarting || state.totalExperiments === null || state.skipExperiment < state.totalExperiments) ), - filter(([fromStarting, skip, total]) => skip < total || total === null || fromStarting), - tap(() => { - this.store$.dispatch(experimentAction.actionSetIsLoadingExperiment({ isLoadingExperiment: true })); - }), - switchMap(([fromStarting, skip, _, searchKey, sortKey, sortAs]) => { - let searchString = null; - // As withLatestFrom does not support more than 5 arguments - // TODO: Find alternative - this.getSearchString$().subscribe((searchInput) => { - searchString = searchInput; - }); - let params: ExperimentPaginationParams = { - skip: fromStarting ? 0 : skip, + switchMap(([action, state]) => { + const fromStarting = !!action.fromStarting || state.skipExperiment === 0; + const params = { + skip: fromStarting ? 0 : state.skipExperiment, take: NUMBER_OF_EXPERIMENTS, + ...(state.sortKey ? { sortParams: { key: state.sortKey, sortAs: state.sortAs } } : {}), + searchParams: { key: state.searchKey, string: state.searchString || '' }, }; - if (sortKey) { - params = { - ...params, - sortParams: { - key: sortKey, - sortAs, - }, - }; - } - // Always send searchParams for experiments, even when searchString is blank - params = { - ...params, - searchParams: { - key: searchKey, - string: searchString || '', + return trackedListRequest( + this.store$.pipe(select(selectRootBatch)), + experimentAction.batchActions, + (event) => this.store$.dispatch(event), + () => { + this.store$.dispatch(experimentAction.actionSetIsLoadingExperiment({ isLoadingExperiment: true })); + return this.experimentDataService.getAllExperiment(params, !!action.batchRefresh); }, - }; - return this.experimentDataService.getAllExperiment(params).pipe( - switchMap((data: any) => { - const experiments = data.nodes; - const experimentIds = experiments.map((experiment) => experiment.id); - const actions = fromStarting ? [experimentAction.actionSetSkipExperiment({ skipExperiment: 0 })] : []; - - return [ - ...actions, - experimentAction.actionGetExperimentsSuccess({ experiments, totalExperiments: data.total, fromStarting }), - experimentAction.actionFetchExperimentStats({ experimentIds }), - ]; - }), - catchError((error) => [experimentAction.actionGetExperimentsFailure(error)]) + (data: any, requestId) => [ + experimentAction.actionGetExperimentsSuccess({ + experiments: data.nodes, + totalExperiments: data.total, + fromStarting, + batchListRequestId: requestId, + }), + experimentAction.actionFetchExperimentStats({ experimentIds: data.nodes.map((row) => row.id) }), + ], + (error) => [experimentAction.actionGetExperimentsFailure({ error })] ); }) ) @@ -801,5 +809,4 @@ export class ExperimentEffects { element.click(); document.body.removeChild(element); } - private getSearchString$ = () => this.store$.pipe(select(selectSearchString)).pipe(first()); } diff --git a/packages/frontend/projects/upgrade/src/app/core/experiments/store/experiments.model.ts b/packages/frontend/projects/upgrade/src/app/core/experiments/store/experiments.model.ts index edd521c118..926256b5e1 100644 --- a/packages/frontend/projects/upgrade/src/app/core/experiments/store/experiments.model.ts +++ b/packages/frontend/projects/upgrade/src/app/core/experiments/store/experiments.model.ts @@ -1,3 +1,4 @@ +import { RootBatchDeleteState } from '../../batch-actions/batch-actions.models'; import { AppState } from '../../core.module'; import { DetailsPageError } from '@shared-component-lib/common-page-error/common-page-error.model'; import { @@ -612,6 +613,7 @@ export const THOMPSON_SAMPLING_WEIGHT_TOOLTIP_KEY = 'experiments.details.conditi export const EXPERIMENT_ROOT_DISPLAYED_COLUMNS = Object.values(EXPERIMENT_ROOT_COLUMN_NAMES); export interface ExperimentState { + rootBatch: RootBatchDeleteState; // List page data - plain array preserves backend sort order experiments: ExperimentVM[]; isLoadingExperiment: boolean; diff --git a/packages/frontend/projects/upgrade/src/app/core/experiments/store/experiments.reducer.spec.ts b/packages/frontend/projects/upgrade/src/app/core/experiments/store/experiments.reducer.spec.ts index 14a81f2c21..ff45bbee48 100644 --- a/packages/frontend/projects/upgrade/src/app/core/experiments/store/experiments.reducer.spec.ts +++ b/packages/frontend/projects/upgrade/src/app/core/experiments/store/experiments.reducer.spec.ts @@ -26,7 +26,6 @@ import { actionSetSearchKey, actionSetSearchParams, actionSetSearchString, - actionSetSkipExperiment, actionSetSortingType, actionSetSortKey, actionUpdateExperimentState, @@ -96,6 +95,7 @@ describe('ExperimentsReducer', () => { expect(newState).not.toBe(previousState); expect(newState).toEqual({ ...previousState, + rootBatch: { ...previousState.rootBatch, loadedIds: ['1'] }, experiments: [ { id: '1', @@ -501,6 +501,8 @@ describe('ExperimentsReducer', () => { expect(newState).not.toBe(previousState); expect(newState).toEqual({ ...previousState, + rootBatch: { ...previousState.rootBatch, removedIds: ['1'] }, + stats: {}, experiments: [], isLoadingExperimentDelete: false, }); @@ -654,20 +656,6 @@ describe('ExperimentsReducer', () => { expect(newState.sortAs).toEqual(SORT_AS_DIRECTION.ASCENDING); }); - it('action "actionSetSkipExperiment" should set experiment skip value', () => { - const previousState = { ...initialState }; - previousState.skipExperiment = 2; - - const testAction: Action = actionSetSkipExperiment({ - skipExperiment: 3, - }); - - const newState = experimentsReducer(previousState, testAction); - - expect(newState).not.toBe(previousState); - expect(newState.skipExperiment).toEqual(3); - }); - it('action "actionFetchAllExperimentNamesSuccess" should set all experimet names', () => { const previousState = { ...initialState }; previousState.allExperimentNames = []; diff --git a/packages/frontend/projects/upgrade/src/app/core/experiments/store/experiments.reducer.ts b/packages/frontend/projects/upgrade/src/app/core/experiments/store/experiments.reducer.ts index af7e6182d9..69107856b9 100644 --- a/packages/frontend/projects/upgrade/src/app/core/experiments/store/experiments.reducer.ts +++ b/packages/frontend/projects/upgrade/src/app/core/experiments/store/experiments.reducer.ts @@ -1,8 +1,11 @@ +import { initialRootBatchDeleteState } from '../../batch-actions/batch-actions.models'; +import { withRootBatchDelete } from '../../batch-actions/batch-actions.store'; import { ExperimentState, EXPERIMENT_SEARCH_KEY, SORT_AS_DIRECTION, EXPERIMENT_SORT_KEY } from './experiments.model'; import { createReducer, on, Action } from '@ngrx/store'; import * as experimentsAction from './experiments.actions'; export const initialState: ExperimentState = { + rootBatch: initialRootBatchDeleteState, // List page state experiments: [], isLoadingExperiment: false, @@ -232,7 +235,6 @@ const reducer = createReducer( })), on(experimentsAction.actionSetSortKey, (state, { sortKey }) => ({ ...state, sortKey })), on(experimentsAction.actionSetSortingType, (state, { sortingType }) => ({ ...state, sortAs: sortingType })), - on(experimentsAction.actionSetSkipExperiment, (state, { skipExperiment }) => ({ ...state, skipExperiment })), on(experimentsAction.actionFetchAllExperimentNamesSuccess, (state, { allExperimentNames }) => ({ ...state, allExperimentNames, @@ -528,6 +530,28 @@ const reducer = createReducer( })) ); +const batchReducer = withRootBatchDelete(reducer, initialState, { + entity: 'experiments', + actions: experimentsAction.batchActions, + rowsKey: 'experiments', + loadingKey: 'isLoadingExperiment', + skipKey: 'skipExperiment', + totalKey: 'totalExperiments', + queryTypes: [ + experimentsAction.actionSetSearchKey.type, + experimentsAction.actionSetSearchString.type, + experimentsAction.actionSetSearchParams.type, + experimentsAction.actionSetSortKey.type, + experimentsAction.actionSetSortingType.type, + ], + deletedId: (action) => + action.type === experimentsAction.actionDeleteExperimentSuccess.type + ? (action as ReturnType).experimentId + : undefined, + listSuccessType: experimentsAction.actionGetExperimentsSuccess.type, + responseRowsKey: 'experiments', +}); + export function experimentsReducer(state: ExperimentState | undefined, action: Action) { - return reducer(state, action); + return batchReducer(state, action); } diff --git a/packages/frontend/projects/upgrade/src/app/core/experiments/store/experiments.selector.spec.ts b/packages/frontend/projects/upgrade/src/app/core/experiments/store/experiments.selector.spec.ts index 970b89251c..4391f77595 100644 --- a/packages/frontend/projects/upgrade/src/app/core/experiments/store/experiments.selector.spec.ts +++ b/packages/frontend/projects/upgrade/src/app/core/experiments/store/experiments.selector.spec.ts @@ -9,6 +9,7 @@ import { ASSIGNMENT_UNIT, CONSISTENCY_RULE, CONDITION_ORDER, + EXPERIMENT_DETAILS_PAGE_ACTIONS, } from './experiments.model'; import { initialState } from './experiments.reducer'; import { @@ -35,6 +36,7 @@ import { selectRewardsDataForSelectedExperiment, selectIsLoadingRewardsSummary, selectExperimentDetailsPageError, + selectExperimentMenuItems, } from './experiments.selectors'; import { PAGE_ERROR_TYPE } from '@shared-component-lib/common-page-error/common-page-error.model'; @@ -952,6 +954,43 @@ describe('Experiments Selectors', () => { }); }); + describe('#selectExperimentMenuItems', () => { + it.each([ + [EXPERIMENT_STATE.DRAFT, false, true, true], + [EXPERIMENT_STATE.INACTIVE, false, true, false], + [EXPERIMENT_STATE.COMPLETED, false, false, false], + [EXPERIMENT_STATE.CANCELLED, false, true, true], + [EXPERIMENT_STATE.ARCHIVED, false, true, false], + [EXPERIMENT_STATE.PREVIEW, true, true, true], + [EXPERIMENT_STATE.SCHEDULED, true, true, true], + [EXPERIMENT_STATE.RUNNING, true, true, false], + [EXPERIMENT_STATE.ENROLLING, true, true, true], + [EXPERIMENT_STATE.PAUSED, true, true, false], + [EXPERIMENT_STATE.ENROLLMENT_COMPLETE, true, true, true], + [undefined, true, true, true], + ])( + 'applies Delete rules in %s while preserving other menu actions', + (state: EXPERIMENT_STATE, deleteDisabled: boolean, archiveDisabled: boolean, otherDisabled: boolean) => { + const items = selectExperimentMenuItems.projector({ ...mockState.experiments[0], state }); + expect(items.find((item) => item.action === EXPERIMENT_DETAILS_PAGE_ACTIONS.DELETE).disabled).toBe( + deleteDisabled + ); + expect(items.find((item) => item.action === EXPERIMENT_DETAILS_PAGE_ACTIONS.ARCHIVE).disabled).toBe( + archiveDisabled + ); + for (const action of [ + EXPERIMENT_DETAILS_PAGE_ACTIONS.EDIT, + EXPERIMENT_DETAILS_PAGE_ACTIONS.DUPLICATE, + EXPERIMENT_DETAILS_PAGE_ACTIONS.EXPORT_DESIGN, + EXPERIMENT_DETAILS_PAGE_ACTIONS.EMAIL_DATA, + EXPERIMENT_DETAILS_PAGE_ACTIONS.EXPORT_STATE_CHANGE_LOGS, + ]) { + expect(items.find((item) => item.action === action).disabled).toBe(otherDisabled); + } + } + ); + }); + describe('#selectExperimentDetailsPageError', () => { const detailsPageError = { entityId: 'abc123', errorType: PAGE_ERROR_TYPE.NOT_FOUND }; const routerStateFor = (experimentId: string) => ({ state: { params: { experimentId } } } as any); diff --git a/packages/frontend/projects/upgrade/src/app/core/experiments/store/experiments.selectors.ts b/packages/frontend/projects/upgrade/src/app/core/experiments/store/experiments.selectors.ts index 7ef30cfe10..b466e521fe 100644 --- a/packages/frontend/projects/upgrade/src/app/core/experiments/store/experiments.selectors.ts +++ b/packages/frontend/projects/upgrade/src/app/core/experiments/store/experiments.selectors.ts @@ -28,7 +28,8 @@ import { import { determineWeightingMethod, isWeightSumValid } from '../condition-helper.service'; import { formatThompsonSamplingConfigDetails } from '../thompson-sampling-helper.service'; import { KeyValueFormat } from '@shared-component-lib/common-section-card-overview-details/common-section-card-overview-details.component'; -import { DetailsPageError } from '@shared-component-lib/common-page-error/common-page-error.model'; +import { DetailsPageError, PAGE_ERROR_TYPE } from '@shared-component-lib/common-page-error/common-page-error.model'; +import { getExperimentDeletionReason } from '../../batch-actions/batch-actions.helpers'; export const selectExperimentState = createFeatureSelector('experiments'); @@ -90,6 +91,9 @@ export const selectExperimentDetailsPageError = createSelector( const experimentId = routerState?.state?.params?.experimentId; const detailsPageError = experimentState?.detailsPageError; + if (experimentState?.rootBatch?.removedIds.includes(experimentId)) + return { entityId: experimentId, errorType: PAGE_ERROR_TYPE.NOT_FOUND }; + // Only surface the error if it belongs to the experiment currently in the route return detailsPageError && detailsPageError.entityId === experimentId ? detailsPageError : null; } @@ -541,6 +545,10 @@ const isMenuItemDisabled = (action: EXPERIMENT_DETAILS_PAGE_ACTIONS, state?: EXP return true; // No state = disabled } + if (action === EXPERIMENT_DETAILS_PAGE_ACTIONS.DELETE) { + return !!getExperimentDeletionReason(state); + } + // Archive only enabled when COMPLETED if (action === EXPERIMENT_DETAILS_PAGE_ACTIONS.ARCHIVE) { return state !== EXPERIMENT_STATE.COMPLETED; @@ -596,3 +604,5 @@ export const selectExperimentMenuItems = createSelector( ]; } ); + +export const selectRootBatch = createSelector(selectExperimentState, (state) => state.rootBatch); diff --git a/packages/frontend/projects/upgrade/src/app/core/feature-flags/feature-flags.data.service.ts b/packages/frontend/projects/upgrade/src/app/core/feature-flags/feature-flags.data.service.ts index 22211f86fb..267ccbd942 100644 --- a/packages/frontend/projects/upgrade/src/app/core/feature-flags/feature-flags.data.service.ts +++ b/packages/frontend/projects/upgrade/src/app/core/feature-flags/feature-flags.data.service.ts @@ -1,3 +1,5 @@ +import { BatchDeleteResult } from 'upgrade_types'; +import { batchHttpContext } from '../batch-actions/batch-actions.http'; import { Injectable } from '@angular/core'; import { HttpClient, HttpContext, HttpParams } from '@angular/common/http'; import { HANDLES_404_CONTEXTUALLY } from '../http-interceptors/http-context-tokens'; @@ -25,12 +27,21 @@ import { IImportFile, LIST_FILTER_MODE } from 'upgrade_types'; @Injectable() export class FeatureFlagsDataService { + batchDelete(ids: string[]): Observable { + return this.http.post(API_ENDPOINTS.flagsBatchDelete, { ids }, { context: batchHttpContext() }); + } + mockFeatureFlags: FeatureFlag[] = []; constructor(private http: HttpClient) {} - fetchFeatureFlagsPaginated(params: FeatureFlagsPaginationParams): Observable { + fetchFeatureFlagsPaginated( + params: FeatureFlagsPaginationParams, + batchRefresh = false + ): Observable { const url = API_ENDPOINTS.getPaginatedFlags; - return this.http.post(url, params); + return batchRefresh + ? this.http.post(url, params, { context: batchHttpContext() }) + : this.http.post(url, params); } fetchFeatureFlagById(id: string) { diff --git a/packages/frontend/projects/upgrade/src/app/core/feature-flags/feature-flags.service.ts b/packages/frontend/projects/upgrade/src/app/core/feature-flags/feature-flags.service.ts index d8141a7987..02cef205d3 100644 --- a/packages/frontend/projects/upgrade/src/app/core/feature-flags/feature-flags.service.ts +++ b/packages/frontend/projects/upgrade/src/app/core/feature-flags/feature-flags.service.ts @@ -1,3 +1,6 @@ +import { createBatchDeleteFacade } from '../batch-actions/batch-actions.facade'; +import { batchActions } from './store/feature-flags.actions'; +import { selectRootBatch } from './store/feature-flags.selectors'; import { Injectable } from '@angular/core'; import { Store, select } from '@ngrx/store'; import { AppState } from '../core.state'; @@ -51,6 +54,8 @@ import { LocalStorageService } from '../local-storage/local-storage.service'; @Injectable() export class FeatureFlagsService { + readonly batch = createBatchDeleteFacade(this.store$, 'flags', batchActions, selectRootBatch, selectAllFeatureFlags); + constructor(private store$: Store, private localStorageService: LocalStorageService) {} currentUserEmailAddress$ = this.store$.pipe(select(selectCurrentUserEmail)); diff --git a/packages/frontend/projects/upgrade/src/app/core/feature-flags/store/feature-flags.actions.ts b/packages/frontend/projects/upgrade/src/app/core/feature-flags/store/feature-flags.actions.ts index f4ae3da4b2..af0bec6c57 100644 --- a/packages/frontend/projects/upgrade/src/app/core/feature-flags/store/feature-flags.actions.ts +++ b/packages/frontend/projects/upgrade/src/app/core/feature-flags/store/feature-flags.actions.ts @@ -1,3 +1,4 @@ +import { createBatchDeleteActions } from '../../batch-actions/batch-actions.actions'; import { createAction, props } from '@ngrx/store'; import { PAGE_ERROR_TYPE } from '@shared-component-lib/common-page-error/common-page-error.model'; import { @@ -15,12 +16,12 @@ import { AddPrivateSegmentListRequest, EditPrivateSegmentListRequest } from '../ export const actionFetchFeatureFlags = createAction( '[Feature Flags] Fetch Feature Flags Paginated', - props<{ fromStarting?: boolean }>() + props<{ fromStarting?: boolean; batchRefresh?: boolean }>() ); export const actionFetchFeatureFlagsSuccess = createAction( '[Feature Flags] Fetch Feature Flags Paginated Success', - props<{ flags: FeatureFlag[]; totalFlags: number }>() + props<{ batchListRequestId?: string; flags: FeatureFlag[]; totalFlags: number; fromStarting?: boolean }>() ); export const actionFetchFeatureFlagsFailure = createAction('[Feature Flags] Fetch Feature Flags Paginated Failure'); @@ -131,8 +132,6 @@ export const actionSetIsLoadingFeatureFlags = createAction( props<{ isLoadingFeatureFlags: boolean }>() ); -export const actionSetSkipFlags = createAction('[Feature Flags] Set Skip Flags', props<{ skipFlags: number }>()); - export const actionSetSearchKey = createAction( '[Feature Flags] Set Search key value', props<{ searchKey: FLAG_SEARCH_KEY }>() @@ -337,3 +336,5 @@ export const actionSetFeatureFlagTotalExposures = createAction( '[Feature Flags] Set Feature Flag Total Exposures', props<{ totalExposures: number | null }>() ); + +export const batchActions = createBatchDeleteActions('flags'); diff --git a/packages/frontend/projects/upgrade/src/app/core/feature-flags/store/feature-flags.effects.ts b/packages/frontend/projects/upgrade/src/app/core/feature-flags/store/feature-flags.effects.ts index 018d8a3bcc..e5cec5dda7 100644 --- a/packages/frontend/projects/upgrade/src/app/core/feature-flags/store/feature-flags.effects.ts +++ b/packages/frontend/projects/upgrade/src/app/core/feature-flags/store/feature-flags.effects.ts @@ -1,15 +1,21 @@ +import { + batchDeleteEffect, + batchDeleteFinishedEffect, + trackedListRequest, +} from '../../batch-actions/batch-actions.effects'; +import { selectRootBatch, selectFeatureFlagsState } from './feature-flags.selectors'; import { FeatureFlagsDataService } from '../feature-flags.data.service'; import { Actions, createEffect, ofType } from '@ngrx/effects'; import { Injectable } from '@angular/core'; import * as FeatureFlagsActions from './feature-flags.actions'; -import { catchError, switchMap, mergeMap, map, filter, withLatestFrom, tap, first } from 'rxjs/operators'; -import { FeatureFlag, FeatureFlagsPaginationParams, NUMBER_OF_FLAGS } from './feature-flags.model'; +import { catchError, switchMap, mergeMap, map, filter, withLatestFrom, tap } from 'rxjs/operators'; +import { FeatureFlag, NUMBER_OF_FLAGS } from './feature-flags.model'; import { DATE_RANGE } from '../../experiments/store/experiments.model'; import { Router } from '@angular/router'; import { Store, select } from '@ngrx/store'; import { AppState, NotificationService } from '../../core.module'; import { TranslateService } from '@ngx-translate/core'; -import { selectSearchString, selectFeatureFlagPaginationParams } from './feature-flags.selectors'; +import { selectSearchString } from './feature-flags.selectors'; import { selectCurrentUser } from '../../auth/store/auth.selectors'; import { CommonExportHelpersService } from '../../../shared/services/common-export-helpers.service'; import { of } from 'rxjs'; @@ -20,6 +26,29 @@ import { isCanonicalEntityId, PAGE_ERROR_TYPE } from '@shared-component-lib/comm @Injectable() export class FeatureFlagsEffects { + batchDelete$ = createEffect(() => + batchDeleteEffect( + this.actions$, + this.store$.pipe(select(selectRootBatch)), + FeatureFlagsActions.batchActions, + this.featureFlagsDataService + ) + ); + finishBatchDelete$ = createEffect(() => + batchDeleteFinishedEffect( + this.actions$, + this.store$.pipe(select(selectRootBatch)), + FeatureFlagsActions.batchActions, + { entity: 'flags', translate: this.translate, service: this.notificationService }, + () => { + const pathname = (this.router.url || '').split('?')[0].split('#')[0]; + return pathname === '/featureflags' + ? [FeatureFlagsActions.actionFetchFeatureFlags({ fromStarting: true, batchRefresh: true })] + : []; + } + ) + ); + constructor( private store$: Store, private actions$: Actions, @@ -34,51 +63,37 @@ export class FeatureFlagsEffects { fetchFeatureFlags$ = createEffect(() => this.actions$.pipe( ofType(FeatureFlagsActions.actionFetchFeatureFlags), - map((action) => action.fromStarting), - withLatestFrom(this.store$.pipe(select(selectFeatureFlagPaginationParams))), - filter(([fromStarting, pagination]) => { - return ( - !pagination.isAllFlagsFetched || - pagination.skip < pagination.total || - pagination.total === null || - fromStarting - ); - }), - tap(() => { - this.store$.dispatch(FeatureFlagsActions.actionSetIsLoadingFeatureFlags({ isLoadingFeatureFlags: true })); - }), - switchMap(([fromStarting, pagination]) => { - let params: FeatureFlagsPaginationParams = { - skip: fromStarting ? 0 : pagination.skip, + withLatestFrom(this.store$.pipe(select(selectFeatureFlagsState))), + filter( + ([action, state]) => + (!state.rootBatch.listLoading || action.fromStarting) && + (action.fromStarting || state.totalFlags === null || state.skipFlags < state.totalFlags) + ), + switchMap(([action, state]) => { + const fromStarting = !!action.fromStarting || state.skipFlags === 0; + const params = { + skip: fromStarting ? 0 : state.skipFlags, take: NUMBER_OF_FLAGS, + ...(state.sortKey ? { sortParams: { key: state.sortKey, sortAs: state.sortAs } } : {}), + ...(state.searchValue ? { searchParams: { key: state.searchKey, string: state.searchValue } } : {}), }; - if (pagination.sortKey) { - params = { - ...params, - sortParams: { - key: pagination.sortKey, - sortAs: pagination.sortAs, - }, - }; - } - if (pagination.searchString) { - params = { - ...params, - searchParams: { - key: pagination.searchKey, - string: pagination.searchString, - }, - }; - } - return this.featureFlagsDataService.fetchFeatureFlagsPaginated(params).pipe( - switchMap((data: any) => { - const actions = fromStarting ? [FeatureFlagsActions.actionSetSkipFlags({ skipFlags: 0 })] : []; - return [ - ...actions, - FeatureFlagsActions.actionFetchFeatureFlagsSuccess({ flags: data.nodes, totalFlags: data.total }), - ]; - }), - catchError(() => [FeatureFlagsActions.actionFetchFeatureFlagsFailure()]) + return trackedListRequest( + this.store$.pipe(select(selectRootBatch)), + FeatureFlagsActions.batchActions, + (event) => this.store$.dispatch(event), + () => { + this.store$.dispatch(FeatureFlagsActions.actionSetIsLoadingFeatureFlags({ isLoadingFeatureFlags: true })); + return this.featureFlagsDataService.fetchFeatureFlagsPaginated(params, !!action.batchRefresh); + }, + (data: any, requestId) => [ + FeatureFlagsActions.actionFetchFeatureFlagsSuccess({ + flags: data.nodes, + totalFlags: data.total, + fromStarting, + batchListRequestId: requestId, + }), + ], + () => [FeatureFlagsActions.actionFetchFeatureFlagsFailure()] ); }) ) @@ -547,6 +562,4 @@ export class FeatureFlagsEffects { ) ) ); - - private getSearchString$ = () => this.store$.pipe(select(selectSearchString)).pipe(first()); } diff --git a/packages/frontend/projects/upgrade/src/app/core/feature-flags/store/feature-flags.model.ts b/packages/frontend/projects/upgrade/src/app/core/feature-flags/store/feature-flags.model.ts index 2be1b480d3..6f118720a1 100644 --- a/packages/frontend/projects/upgrade/src/app/core/feature-flags/store/feature-flags.model.ts +++ b/packages/frontend/projects/upgrade/src/app/core/feature-flags/store/feature-flags.model.ts @@ -1,3 +1,4 @@ +import { RootBatchDeleteState } from '../../batch-actions/batch-actions.models'; import { AppState } from '../../core.state'; import { DetailsPageError } from '@shared-component-lib/common-page-error/common-page-error.model'; import { FEATURE_FLAG_STATUS, FILTER_MODE, FLAG_SEARCH_KEY, FLAG_SORT_KEY, SORT_AS_DIRECTION } from 'upgrade_types'; @@ -190,6 +191,7 @@ export interface IExposureStatByDate { } export interface FeatureFlagState { + rootBatch: RootBatchDeleteState; // List page data - plain array preserves backend sort order featureFlags: FeatureFlag[]; isLoadingFeatureFlags: boolean; diff --git a/packages/frontend/projects/upgrade/src/app/core/feature-flags/store/feature-flags.reducer.ts b/packages/frontend/projects/upgrade/src/app/core/feature-flags/store/feature-flags.reducer.ts index e6f6e1884f..911d6beeb0 100644 --- a/packages/frontend/projects/upgrade/src/app/core/feature-flags/store/feature-flags.reducer.ts +++ b/packages/frontend/projects/upgrade/src/app/core/feature-flags/store/feature-flags.reducer.ts @@ -1,9 +1,12 @@ +import { initialRootBatchDeleteState } from '../../batch-actions/batch-actions.models'; +import { withRootBatchDelete } from '../../batch-actions/batch-actions.store'; import { createReducer, Action, on } from '@ngrx/store'; import { FeatureFlagState } from './feature-flags.model'; import * as FeatureFlagsActions from './feature-flags.actions'; import { FLAG_SEARCH_KEY, FLAG_SORT_KEY, SORT_AS_DIRECTION } from 'upgrade_types'; export const initialState: FeatureFlagState = { + rootBatch: initialRootBatchDeleteState, // List page state featureFlags: [], isLoadingFeatureFlags: false, @@ -52,10 +55,10 @@ const mergeSelectedFlagWithPartialResponse = ( const reducer = createReducer( initialState, - on(FeatureFlagsActions.actionFetchFeatureFlagsSuccess, (state, { flags, totalFlags }) => { + on(FeatureFlagsActions.actionFetchFeatureFlagsSuccess, (state, { flags, totalFlags, fromStarting }) => { // Replace entire array with backend data - preserves exact sort order const featureFlags = - state.skipFlags === 0 + fromStarting || state.skipFlags === 0 ? flags // First fetch - use backend data directly : [...state.featureFlags, ...flags]; // Pagination - append to existing @@ -63,7 +66,7 @@ const reducer = createReducer( ...state, featureFlags, totalFlags, - skipFlags: state.skipFlags + flags.length, + skipFlags: fromStarting ? flags.length : state.skipFlags + flags.length, isLoadingFeatureFlags: false, hasInitialFeatureFlagsDataLoaded: true, }; @@ -153,7 +156,6 @@ const reducer = createReducer( ...state, isLoadingImportFeatureFlag, })), - on(FeatureFlagsActions.actionSetSkipFlags, (state, { skipFlags }) => ({ ...state, skipFlags })), on(FeatureFlagsActions.actionSetSearchKey, (state, { searchKey }) => ({ ...state, searchKey })), on(FeatureFlagsActions.actionSetSearchString, (state, { searchString }) => ({ ...state, searchValue: searchString })), on(FeatureFlagsActions.actionSetSortKey, (state, { sortKey }) => ({ ...state, sortKey })), @@ -394,6 +396,28 @@ const reducer = createReducer( })) ); +const batchReducer = withRootBatchDelete(reducer, initialState, { + entity: 'flags', + actions: FeatureFlagsActions.batchActions, + rowsKey: 'featureFlags', + loadingKey: 'isLoadingFeatureFlags', + skipKey: 'skipFlags', + totalKey: 'totalFlags', + queryTypes: [ + FeatureFlagsActions.actionSetSearchKey.type, + FeatureFlagsActions.actionSetSearchString.type, + FeatureFlagsActions.actionSetSortKey.type, + FeatureFlagsActions.actionSetSortingType.type, + ], + deletedId: (action) => { + if (action.type !== FeatureFlagsActions.actionDeleteFeatureFlagSuccess.type) return undefined; + const response = (action as ReturnType).flag; + return (Array.isArray(response) ? response[0] : response)?.id; + }, + listSuccessType: FeatureFlagsActions.actionFetchFeatureFlagsSuccess.type, + responseRowsKey: 'flags', +}); + export function featureFlagsReducer(state: FeatureFlagState | undefined, action: Action) { - return reducer(state, action); + return batchReducer(state, action); } diff --git a/packages/frontend/projects/upgrade/src/app/core/feature-flags/store/feature-flags.selectors.ts b/packages/frontend/projects/upgrade/src/app/core/feature-flags/store/feature-flags.selectors.ts index f296430bd1..ec293fedda 100644 --- a/packages/frontend/projects/upgrade/src/app/core/feature-flags/store/feature-flags.selectors.ts +++ b/packages/frontend/projects/upgrade/src/app/core/feature-flags/store/feature-flags.selectors.ts @@ -1,5 +1,5 @@ import { createSelector, createFeatureSelector } from '@ngrx/store'; -import { DetailsPageError } from '@shared-component-lib/common-page-error/common-page-error.model'; +import { DetailsPageError, PAGE_ERROR_TYPE } from '@shared-component-lib/common-page-error/common-page-error.model'; import { FeatureFlag, FeatureFlagState, ParticipantListTableRow } from './feature-flags.model'; import { selectRouterState } from '../../core.state'; import { selectContextMetaData } from '../../experiments/store/experiments.selectors'; @@ -78,6 +78,9 @@ export const selectFeatureFlagDetailsPageError = createSelector( const flagId = routerState?.state?.params?.flagId; const detailsPageError = featureFlagState?.detailsPageError; + if (featureFlagState?.rootBatch?.removedIds.includes(flagId)) + return { entityId: flagId, errorType: PAGE_ERROR_TYPE.NOT_FOUND }; + // Only surface the error if it belongs to the feature flag currently in the route return detailsPageError && detailsPageError.entityId === flagId ? detailsPageError : null; } @@ -187,25 +190,6 @@ export const selectFeatureFlagExclusions = createSelector( } ); -export const selectFeatureFlagPaginationParams = createSelector( - selectSkipFlags, - selectTotalFlags, - selectSearchKey, - selectSortKey, - selectSortAs, - selectIsAllFlagsFetched, - selectSearchString, - (skip, total, searchKey, sortKey, sortAs, isAllFlagsFetched, searchString) => ({ - skip, - total, - searchKey, - sortKey, - sortAs, - isAllFlagsFetched, - searchString, - }) -); - // Helper function returns array of translation keys (extensible for future warning types) const getWarningKeysForFlag = (flag: FeatureFlag): string[] => { const warnings: string[] = []; @@ -247,3 +231,5 @@ export const selectWarningKeysForAllFlags = createSelector(selectFeatureFlagsSta }); return warningKeys; }); + +export const selectRootBatch = createSelector(selectFeatureFlagsState, (state) => state.rootBatch); diff --git a/packages/frontend/projects/upgrade/src/app/core/local-storage/local-storage.service.spec.ts b/packages/frontend/projects/upgrade/src/app/core/local-storage/local-storage.service.spec.ts index 1f6de6ce49..645afef84a 100755 --- a/packages/frontend/projects/upgrade/src/app/core/local-storage/local-storage.service.spec.ts +++ b/packages/frontend/projects/upgrade/src/app/core/local-storage/local-storage.service.spec.ts @@ -1,3 +1,4 @@ +import { initialRootBatchDeleteState } from '../batch-actions/batch-actions.models'; import { ExperimentState, EXPERIMENT_SEARCH_KEY, @@ -21,6 +22,7 @@ describe('LocalStorageService', () => { describe('#loadInitialState', () => { const expectedStateWithFetchedValues: ExperimentState = { + rootBatch: initialRootBatchDeleteState, experiments: [], isLoadingExperiment: false, isLoadingExperimentDetailStats: false, @@ -50,6 +52,7 @@ describe('LocalStorageService', () => { detailsPageError: null, }; const expectedStateWithDefaults: ExperimentState = { + rootBatch: initialRootBatchDeleteState, experiments: [], isLoadingExperiment: false, isLoadingExperimentDetailStats: false, diff --git a/packages/frontend/projects/upgrade/src/app/core/local-storage/local-storage.service.ts b/packages/frontend/projects/upgrade/src/app/core/local-storage/local-storage.service.ts index 69a5e3dcf4..e6df1f2bfa 100755 --- a/packages/frontend/projects/upgrade/src/app/core/local-storage/local-storage.service.ts +++ b/packages/frontend/projects/upgrade/src/app/core/local-storage/local-storage.service.ts @@ -1,3 +1,4 @@ +import { initialRootBatchDeleteState } from '../batch-actions/batch-actions.models'; import { Injectable } from '@angular/core'; import { ExperimentLocalStorageKeys, @@ -33,6 +34,7 @@ export class LocalStorageService { // 1. Populate experiment state const experimentState: ExperimentState = { + rootBatch: initialRootBatchDeleteState, experiments: [], isLoadingExperiment: false, isLoadingExperimentDetailStats: false, @@ -63,6 +65,7 @@ export class LocalStorageService { }; const featureFlagState: FeatureFlagState = { + rootBatch: initialRootBatchDeleteState, featureFlags: [], selectedFlag: null, isLoadingUpsertFeatureFlag: false, @@ -87,6 +90,7 @@ export class LocalStorageService { }; const segmentState: SegmentState = { + rootBatch: initialRootBatchDeleteState, segments: [], isLoadingSegments: false, hasInitialSegmentsDataLoaded: false, diff --git a/packages/frontend/projects/upgrade/src/app/core/segments/segments.data.service.ts b/packages/frontend/projects/upgrade/src/app/core/segments/segments.data.service.ts index a5ab4ccce7..9c3c269f33 100644 --- a/packages/frontend/projects/upgrade/src/app/core/segments/segments.data.service.ts +++ b/packages/frontend/projects/upgrade/src/app/core/segments/segments.data.service.ts @@ -1,3 +1,5 @@ +import { BatchDeleteResult } from 'upgrade_types'; +import { batchHttpContext } from '../batch-actions/batch-actions.http'; import { Injectable } from '@angular/core'; import { AddPrivateSegmentListRequest, @@ -17,6 +19,14 @@ import { API_ENDPOINTS } from '../api-endpoints.constants'; @Injectable() export class SegmentsDataService { + batchDelete(ids: string[]): Observable { + return this.http.post( + API_ENDPOINTS.segmentsBatchDelete, + { ids }, + { context: batchHttpContext() } + ); + } + constructor(private http: HttpClient) {} fetchAllSegments() { @@ -24,9 +34,11 @@ export class SegmentsDataService { return this.http.get(url); } - fetchSegmentsPaginated(params: SegmentsPaginationParams): Observable { + fetchSegmentsPaginated(params: SegmentsPaginationParams, batchRefresh = false): Observable { const url = API_ENDPOINTS.getPaginatedSegments; - return this.http.post(url, params); + return batchRefresh + ? this.http.post(url, params, { context: batchHttpContext() }) + : this.http.post(url, params); } fetchGlobalSegments() { diff --git a/packages/frontend/projects/upgrade/src/app/core/segments/segments.service.ts b/packages/frontend/projects/upgrade/src/app/core/segments/segments.service.ts index dd72007cd7..2d3ee2a157 100644 --- a/packages/frontend/projects/upgrade/src/app/core/segments/segments.service.ts +++ b/packages/frontend/projects/upgrade/src/app/core/segments/segments.service.ts @@ -1,3 +1,6 @@ +import { createBatchDeleteFacade } from '../batch-actions/batch-actions.facade'; +import { batchActions } from './store/segments.actions'; +import { selectRootBatch } from './store/segments.selectors'; import { Injectable } from '@angular/core'; import { Store, select } from '@ngrx/store'; import { AppState } from '../core.state'; @@ -52,6 +55,8 @@ import { actionFetchContextMetaData } from '../experiments/store/experiments.act @Injectable({ providedIn: 'root' }) export class SegmentsService { + readonly batch = createBatchDeleteFacade(this.store$, 'segments', batchActions, selectRootBatch, selectAllSegments); + constructor( private readonly store$: Store, private readonly segmentsDataService: SegmentsDataService, diff --git a/packages/frontend/projects/upgrade/src/app/core/segments/store/segments.actions.ts b/packages/frontend/projects/upgrade/src/app/core/segments/store/segments.actions.ts index 35c65648f4..ad7d6ce55c 100644 --- a/packages/frontend/projects/upgrade/src/app/core/segments/store/segments.actions.ts +++ b/packages/frontend/projects/upgrade/src/app/core/segments/store/segments.actions.ts @@ -1,3 +1,4 @@ +import { createBatchDeleteActions } from '../../batch-actions/batch-actions.actions'; import { createAction, props } from '@ngrx/store'; import { PAGE_ERROR_TYPE } from '@shared-component-lib/common-page-error/common-page-error.model'; import { @@ -19,11 +20,15 @@ import { } from '../../../../../../../../types/src/Experiment/enums'; import { FeatureFlagSegmentListDetails } from '../../feature-flags/store/feature-flags.model'; -export const actionFetchSegments = createAction('[Segments] Segments', props<{ fromStarting?: boolean }>()); +export const actionFetchSegments = createAction( + '[Segments] Segments', + props<{ fromStarting?: boolean; batchRefresh?: boolean }>() +); export const actionFetchSegmentsSuccess = createAction( '[Segments] Fetch Segments Success', props<{ + batchListRequestId?: string; segments: Segment[]; totalSegments: number; experimentSegmentInclusion: experimentSegmentInclusionExclusionData[]; @@ -205,3 +210,5 @@ export const actionDeleteSegmentListFailure = createAction( '[Segments] Delete Segment List Failure', props<{ error: any }>() ); + +export const batchActions = createBatchDeleteActions('segments'); diff --git a/packages/frontend/projects/upgrade/src/app/core/segments/store/segments.effects.ts b/packages/frontend/projects/upgrade/src/app/core/segments/store/segments.effects.ts index 858d820707..aa3735a748 100644 --- a/packages/frontend/projects/upgrade/src/app/core/segments/store/segments.effects.ts +++ b/packages/frontend/projects/upgrade/src/app/core/segments/store/segments.effects.ts @@ -1,25 +1,20 @@ +import { + batchDeleteEffect, + batchDeleteFinishedEffect, + trackedListRequest, +} from '../../batch-actions/batch-actions.effects'; +import { selectRootBatch, selectSegmentsState } from './segments.selectors'; import { Injectable } from '@angular/core'; import { Router } from '@angular/router'; import { Actions, createEffect, ofType } from '@ngrx/effects'; import { select, Store } from '@ngrx/store'; -import { catchError, concatMap, filter, first, map, switchMap, tap, withLatestFrom } from 'rxjs/operators'; +import { catchError, concatMap, filter, map, switchMap, tap, withLatestFrom } from 'rxjs/operators'; import { AppState, NotificationService } from '../../core.module'; import { TranslateService } from '@ngx-translate/core'; import { SegmentsDataService } from '../segments.data.service'; import * as SegmentsActions from './segments.actions'; -import { - LIST_OPTION_TYPE, - NUMBER_OF_SEGMENTS, - Segment, - SegmentsPaginationParams, - UpsertSegmentType, -} from './segments.model'; -import { - selectAllSegments, - selectGlobalSegments, - selectSearchString, - selectSegmentPaginationParams, -} from './segments.selectors'; +import { LIST_OPTION_TYPE, NUMBER_OF_SEGMENTS, Segment, UpsertSegmentType } from './segments.model'; +import { selectGlobalSegments } from './segments.selectors'; import JSZip from 'jszip'; import { of } from 'rxjs'; import { isCanonicalEntityId, PAGE_ERROR_TYPE } from '@shared-component-lib/common-page-error/common-page-error.model'; @@ -29,6 +24,32 @@ import { CommonModalEventsService } from '../../../shared/services/common-modal- @Injectable() export class SegmentsEffects { + batchDelete$ = createEffect(() => + batchDeleteEffect( + this.actions$, + this.store$.pipe(select(selectRootBatch)), + SegmentsActions.batchActions, + this.segmentsDataService + ) + ); + finishBatchDelete$ = createEffect(() => + batchDeleteFinishedEffect( + this.actions$, + this.store$.pipe(select(selectRootBatch)), + SegmentsActions.batchActions, + { entity: 'segments', translate: this.translate, service: this.notificationService }, + () => { + const pathname = (this.router.url || '').split('?')[0].split('#')[0]; + return [ + // Detail selectors share the rows array; replace it only while the root table is displayed. + ...(pathname === '/segments' + ? [SegmentsActions.actionFetchSegments({ fromStarting: true, batchRefresh: true })] + : []), + ]; + } + ) + ); + constructor( private store$: Store, private actions$: Actions, @@ -43,55 +64,42 @@ export class SegmentsEffects { fetchSegmentsPaginated$ = createEffect(() => this.actions$.pipe( ofType(SegmentsActions.actionFetchSegments), - map((action) => action.fromStarting), - withLatestFrom(this.store$.pipe(select(selectSegmentPaginationParams))), - filter(([fromStarting, pagination]) => { - return ( - !pagination.areAllFetched || pagination.skip < pagination.total || pagination.total === null || fromStarting - ); - }), - tap(() => { - this.store$.dispatch(SegmentsActions.actionSetIsLoadingSegments({ isLoadingSegments: true })); - }), - switchMap(([fromStarting, pagination]) => { - let params: SegmentsPaginationParams = { - skip: fromStarting ? 0 : pagination.skip, + withLatestFrom(this.store$.pipe(select(selectSegmentsState))), + filter( + ([action, state]) => + (!state.rootBatch.listLoading || action.fromStarting) && + (action.fromStarting || state.totalSegments === null || state.skipSegments < state.totalSegments) + ), + switchMap(([action, state]) => { + const fromStarting = !!action.fromStarting || state.skipSegments === 0; + const params = { + skip: fromStarting ? 0 : state.skipSegments, take: NUMBER_OF_SEGMENTS, + ...(state.sortKey ? { sortParams: { key: state.sortKey, sortAs: state.sortAs } } : {}), + ...(state.searchString ? { searchParams: { key: state.searchKey, string: state.searchString } } : {}), }; - if (pagination.sortKey) { - params = { - ...params, - sortParams: { - key: pagination.sortKey, - sortAs: pagination.sortAs, - }, - }; - } - if (pagination.searchString) { - params = { - ...params, - searchParams: { - key: pagination.searchKey, - string: pagination.searchString, - }, - }; - } - return this.segmentsDataService.fetchSegmentsPaginated(params).pipe( - switchMap((data: any) => { - return [ - SegmentsActions.actionFetchSegmentsSuccess({ - segments: data.nodes.segmentsData, - totalSegments: data.total, - experimentSegmentInclusion: data.nodes.experimentSegmentInclusionData, - experimentSegmentExclusion: data.nodes.experimentSegmentExclusionData, - featureFlagSegmentInclusion: data.nodes.featureFlagSegmentInclusionData, - featureFlagSegmentExclusion: data.nodes.featureFlagSegmentExclusionData, - allParentSegments: data.nodes.allParentSegments, - fromStarting, - }), - ]; - }), - catchError(() => [SegmentsActions.actionFetchSegmentsFailure()]) + return trackedListRequest( + this.store$.pipe(select(selectRootBatch)), + SegmentsActions.batchActions, + (event) => this.store$.dispatch(event), + () => { + this.store$.dispatch(SegmentsActions.actionSetIsLoadingSegments({ isLoadingSegments: true })); + return this.segmentsDataService.fetchSegmentsPaginated(params, !!action.batchRefresh); + }, + (data: any, requestId) => [ + SegmentsActions.actionFetchSegmentsSuccess({ + segments: data.nodes.segmentsData, + totalSegments: data.total, + experimentSegmentInclusion: data.nodes.experimentSegmentInclusionData, + experimentSegmentExclusion: data.nodes.experimentSegmentExclusionData, + featureFlagSegmentInclusion: data.nodes.featureFlagSegmentInclusionData, + featureFlagSegmentExclusion: data.nodes.featureFlagSegmentExclusionData, + allParentSegments: data.nodes.allParentSegments, + fromStarting, + batchListRequestId: requestId, + }), + ], + () => [SegmentsActions.actionFetchSegmentsFailure()] ); }) ) @@ -293,8 +301,6 @@ export class SegmentsEffects { ) ); - private getSearchString$ = () => this.store$.pipe(select(selectSearchString)).pipe(first()); - // TODO: this should be replaced with the common download() method in common-export-helpers service in new experience private download(filename, text, isZip: boolean) { const element = document.createElement('a'); diff --git a/packages/frontend/projects/upgrade/src/app/core/segments/store/segments.model.ts b/packages/frontend/projects/upgrade/src/app/core/segments/store/segments.model.ts index d9914e090e..e484bfc3d9 100644 --- a/packages/frontend/projects/upgrade/src/app/core/segments/store/segments.model.ts +++ b/packages/frontend/projects/upgrade/src/app/core/segments/store/segments.model.ts @@ -1,3 +1,4 @@ +import { RootBatchDeleteState } from '../../batch-actions/batch-actions.models'; import { AppState } from '../../core.state'; import { DetailsPageError } from '@shared-component-lib/common-page-error/common-page-error.model'; import { SEGMENT_TYPE, SEGMENT_STATUS, SEGMENT_SEARCH_KEY, SORT_AS_DIRECTION, SEGMENT_SORT_KEY } from 'upgrade_types'; @@ -232,6 +233,7 @@ export enum SEGMENT_LIST_ACTIONS { } export interface SegmentState { + rootBatch: RootBatchDeleteState; // List page data - plain array preserves backend sort order segments: Segment[]; isLoadingSegments: boolean; diff --git a/packages/frontend/projects/upgrade/src/app/core/segments/store/segments.reducer.ts b/packages/frontend/projects/upgrade/src/app/core/segments/store/segments.reducer.ts index 38b26a64e7..f2585d631c 100644 --- a/packages/frontend/projects/upgrade/src/app/core/segments/store/segments.reducer.ts +++ b/packages/frontend/projects/upgrade/src/app/core/segments/store/segments.reducer.ts @@ -1,3 +1,5 @@ +import { initialRootBatchDeleteState } from '../../batch-actions/batch-actions.models'; +import { withRootBatchDelete } from '../../batch-actions/batch-actions.store'; import { createReducer, Action, on } from '@ngrx/store'; import { SegmentState, GlobalSegmentState } from './segments.model'; import * as SegmentsActions from './segments.actions'; @@ -8,6 +10,7 @@ import { } from '../../../../../../../../types/src/Experiment/enums'; export const initialState: SegmentState = { + rootBatch: initialRootBatchDeleteState, // List page data - plain array preserves backend sort order segments: [], isLoadingSegments: false, @@ -187,8 +190,30 @@ const reducer = createReducer( })) ); +const batchReducer = withRootBatchDelete(reducer, initialState, { + entity: 'segments', + actions: SegmentsActions.batchActions, + rowsKey: 'segments', + loadingKey: 'isLoadingSegments', + skipKey: 'skipSegments', + totalKey: 'totalSegments', + queryTypes: [ + SegmentsActions.actionSetSearchKey.type, + SegmentsActions.actionSetSearchString.type, + SegmentsActions.actionSetSortKey.type, + SegmentsActions.actionSetSortingType.type, + ], + deletedId: (action) => { + if (action.type !== SegmentsActions.actionDeleteSegmentSuccess.type) return undefined; + const response = (action as ReturnType).segment; + return (Array.isArray(response) ? response[0] : response)?.id; + }, + listSuccessType: SegmentsActions.actionFetchSegmentsSuccess.type, + responseRowsKey: 'segments', +}); + export function segmentsReducer(state: SegmentState | undefined, action: Action) { - return reducer(state, action); + return batchReducer(state, action); } export const initalGlobalState: GlobalSegmentState = { diff --git a/packages/frontend/projects/upgrade/src/app/core/segments/store/segments.selectors.ts b/packages/frontend/projects/upgrade/src/app/core/segments/store/segments.selectors.ts index bb3ef98920..765f4c1945 100644 --- a/packages/frontend/projects/upgrade/src/app/core/segments/store/segments.selectors.ts +++ b/packages/frontend/projects/upgrade/src/app/core/segments/store/segments.selectors.ts @@ -1,5 +1,5 @@ import { createSelector, createFeatureSelector } from '@ngrx/store'; -import { DetailsPageError } from '@shared-component-lib/common-page-error/common-page-error.model'; +import { DetailsPageError, PAGE_ERROR_TYPE } from '@shared-component-lib/common-page-error/common-page-error.model'; import { SegmentState, ParticipantListTableRow, @@ -108,6 +108,9 @@ export const selectSegmentDetailsPageError = createSelector( const segmentId = routerState?.state?.params?.segmentId; const detailsPageError = segmentState?.detailsPageError; + if (segmentState?.rootBatch?.removedIds.includes(segmentId)) + return { entityId: segmentId, errorType: PAGE_ERROR_TYPE.NOT_FOUND }; + // Only surface the error if it belongs to the segment currently in the route return detailsPageError && detailsPageError.entityId === segmentId ? detailsPageError : null; } @@ -119,16 +122,6 @@ export const selectSegmentOverviewDetails = createSelector(selectSelectedSegment ['Tags']: segment?.tags, })); -export const selectSkipSegments = createSelector(selectSegmentsState, (state) => state.skipSegments); - -export const selectTotalSegments = createSelector(selectSegmentsState, (state) => state.totalSegments); - -export const selectAreAllSegmentsFetched = createSelector( - selectSkipSegments, - selectTotalSegments, - (skipSegments, totalSegments) => skipSegments === totalSegments -); - export const selectSearchKey = createSelector(selectSegmentsState, (state) => state.searchKey); export const selectSearchString = createSelector(selectSegmentsState, (state) => state.searchString); @@ -219,25 +212,6 @@ export const selectSegmentUsageData = createSelector( } ); -export const selectSegmentPaginationParams = createSelector( - selectSkipSegments, - selectTotalSegments, - selectSearchKey, - selectSortKey, - selectSortAs, - selectAreAllSegmentsFetched, - selectSearchString, - (skip, total, searchKey, sortKey, sortAs, areAllFetched, searchString) => ({ - skip, - total, - searchKey, - sortKey, - sortAs, - areAllFetched, - searchString, - }) -); - export const selectListSegmentOptionsByContext = (context: string) => { return createSelector(selectSegmentsState, (segmentState: SegmentState) => { if (!segmentState?.listSegmentOptions) { @@ -328,3 +302,5 @@ function processParentSegments(segmentData: Segment[], segmentId: string, result } }); } + +export const selectRootBatch = createSelector(selectSegmentsState, (state) => state.rootBatch); diff --git a/packages/frontend/projects/upgrade/src/app/features/dashboard/batch-delete-ui.spec.ts b/packages/frontend/projects/upgrade/src/app/features/dashboard/batch-delete-ui.spec.ts new file mode 100644 index 0000000000..2f8940d692 --- /dev/null +++ b/packages/frontend/projects/upgrade/src/app/features/dashboard/batch-delete-ui.spec.ts @@ -0,0 +1,581 @@ +import { ComponentFixture, TestBed, fakeAsync, tick } from '@angular/core/testing'; +import { By } from '@angular/platform-browser'; +import { NoopAnimationsModule } from '@angular/platform-browser/animations'; +import { provideRouter, Router } from '@angular/router'; +import { OverlayContainer } from '@angular/cdk/overlay'; +import { MatDialog } from '@angular/material/dialog'; +import { MatTooltip } from '@angular/material/tooltip'; +import { Store, StoreModule } from '@ngrx/store'; +import { TranslateModule, TranslateService } from '@ngx-translate/core'; +import { BehaviorSubject, Subject, Subscription, of } from 'rxjs'; +import { EXPERIMENT_STATE, FEATURE_FLAG_STATUS, SEGMENT_STATUS, UserRole } from 'upgrade_types'; +import { ExperimentRootSectionCardComponent } from './experiments/pages/experiment-root-page/experiment-root-page-content/experiment-root-section-card/experiment-root-section-card.component'; +import { FeatureFlagRootSectionCardComponent } from './feature-flags/pages/feature-flag-root-page/feature-flag-root-page-content/feature-flag-root-section-card/feature-flag-root-section-card.component'; +import { SegmentRootSectionCardComponent } from './segments/pages/segment-root-page/segment-root-page-content/segment-root-section-card/segment-root-section-card.component'; +import { ExperimentService } from '../../core/experiments/experiments.service'; +import { FeatureFlagsService } from '../../core/feature-flags/feature-flags.service'; +import { SegmentsService } from '../../core/segments/segments.service'; +import { AuthService } from '../../core/auth/auth.service'; +import { StratificationFactorsService } from '../../core/stratification-factors/stratification-factors.service'; +import { DialogService } from '../../shared/services/common-dialog.service'; +import { createBatchDeleteFacade } from '../../core/batch-actions/batch-actions.facade'; +import { RootBatchDeleteDirective } from '../../shared/directives/root-batch-delete.directive'; +import { actionSetUserInfo } from '../../core/auth/store/auth.actions'; +import { experimentsReducer } from '../../core/experiments/store/experiments.reducer'; +import { featureFlagsReducer } from '../../core/feature-flags/store/feature-flags.reducer'; +import { segmentsReducer } from '../../core/segments/store/segments.reducer'; +import * as experiments from '../../core/experiments/store/experiments.actions'; +import * as flags from '../../core/feature-flags/store/feature-flags.actions'; +import * as segments from '../../core/segments/store/segments.actions'; + +const translations = jest.requireActual('../../../assets/i18n/en.json'); +const cases = [ + { + entity: 'experiments', + key: 'experiments', + component: ExperimentRootSectionCardComponent, + token: ExperimentService, + actions: experiments, + }, + { + entity: 'flags', + key: 'featureFlags', + component: FeatureFlagRootSectionCardComponent, + token: FeatureFlagsService, + actions: flags, + }, + { + entity: 'segments', + key: 'segments', + component: SegmentRootSectionCardComponent, + token: SegmentsService, + actions: segments, + }, +] as const; + +describe.each(cases)('$entity root batch UI', (config) => { + let fixture: ComponentFixture; + let store: Store; + let state: any; + let subscription: Subscription; + let service: any; + let dialogs: any; + let closed: Subject; + let overlay: OverlayContainer; + let permissions$: BehaviorSubject; + let listLoading$: BehaviorSubject; + const actions = config.actions.batchActions; + const rows = ['Alpha', 'Beta'].map((name, index) => ({ + id: `11111111-2222-4333-8444-${String(index + 1).padStart(12, '0')}`, + name, + description: 'Description', + context: ['test'], + tags: [], + state: config.entity === 'experiments' ? EXPERIMENT_STATE.INACTIVE : undefined, + status: config.entity === 'segments' ? SEGMENT_STATUS.UNUSED : FEATURE_FLAG_STATUS.DISABLED, + })); + const batch = () => state[config.key].rootBatch; + const checkboxes = (): HTMLInputElement[] => [...fixture.nativeElement.querySelectorAll('input[type=checkbox]')]; + function load(items = rows) { + const action = + config.entity === 'experiments' + ? experiments.actionGetExperimentsSuccess({ + experiments: items as any, + totalExperiments: items.length, + fromStarting: true, + }) + : config.entity === 'flags' + ? flags.actionFetchFeatureFlagsSuccess({ flags: items as any, totalFlags: items.length, fromStarting: true }) + : segments.actionFetchSegmentsSuccess({ + segments: items as any, + totalSegments: items.length, + fromStarting: true, + experimentSegmentInclusion: [], + experimentSegmentExclusion: [], + featureFlagSegmentInclusion: [], + featureFlagSegmentExclusion: [], + allParentSegments: [], + }); + store.dispatch(action); + } + function selectFirst() { + checkboxes()[1].click(); + fixture.detectChanges(); + } + function openMenu() { + fixture.nativeElement.querySelector('.section-card-menu-trigger').click(); + fixture.detectChanges(); + tick(); + } + beforeEach(async () => { + permissions$ = new BehaviorSubject({ + experiments: { create: true, delete: true }, + featureFlags: { create: true, delete: true }, + segments: { create: true, delete: true }, + }); + global.IntersectionObserver = jest.fn(() => ({ observe: jest.fn(), disconnect: jest.fn() })) as any; + await TestBed.configureTestingModule({ + imports: [ + config.component, + NoopAnimationsModule, + TranslateModule.forRoot(), + StoreModule.forRoot({ + experiments: experimentsReducer, + featureFlags: featureFlagsReducer, + segments: segmentsReducer, + }), + ], + providers: [ + provideRouter([]), + { provide: config.token, useFactory: () => service }, + { + provide: AuthService, + useValue: { + userPermissions$: permissions$, + }, + }, + { provide: StratificationFactorsService, useValue: { fetchStratificationFactors: jest.fn() } }, + { provide: DialogService, useFactory: () => dialogs }, + ], + }).compileComponents(); + store = TestBed.inject(Store); + subscription = store.subscribe((value) => (state = value)); + store.dispatch(actionSetUserInfo({ user: { email: 'test@example.com', role: UserRole.ADMIN } })); + load(); + const batchFacade = createBatchDeleteFacade( + store, + config.entity, + actions, + (s) => s[config.key].rootBatch, + (s) => s[config.key][config.key] + ); + const rows$ = store.select((s) => s[config.key][config.key]); + listLoading$ = new BehaviorSubject(false); + service = { + batch: batchFacade, + experiments$: rows$, + featureFlags$: rows$, + selectAllSegments$: rows$, + isLoadingExperiment$: listLoading$, + isLoadingFeatureFlags$: listLoading$, + isLoadingSegments$: listLoading$, + haveInitialExperimentsLoaded: () => of(true), + isInitialFeatureFlagsLoading$: of(true), + isInitialSegmentsLoading: () => of(true), + selectSearchString$: of(''), + searchString$: of(''), + selectSearchKey$: of('name'), + searchKey$: of('name'), + searchParams$: of({}), + selectRootTableState$: of({}), + selectExperimentSortKey$: of('name'), + selectExperimentSortAs$: of('ASC'), + sortKey$: of('name'), + sortAs$: of('ASC'), + selectSegmentSortKey$: of('name'), + selectSegmentSortAs$: of('ASC'), + warningKeysForAllExperiments$: of({}), + warningKeysForAllFlags$: of({}), + loadExperiments: jest.fn(), + fetchFeatureFlags: jest.fn(), + fetchSegmentsPaginated: jest.fn(), + fetchAllExperimentNames: jest.fn(), + setSearchParams: jest.fn(), + setSearchString: jest.fn(), + setSearchKey: jest.fn(), + setSortingType: jest.fn(), + setSortKey: jest.fn(), + }; + closed = new Subject(); + dialogs = { openBatchDeleteModal: jest.fn(() => ({ afterClosed: () => closed, close: jest.fn() })) }; + overlay = TestBed.inject(OverlayContainer); + const translate = TestBed.inject(TranslateService); + translate.setTranslation('en', translations); + translate.use('en'); + fixture = TestBed.createComponent(config.component as any); + fixture.detectChanges(); + fixture.detectChanges(); + }); + afterEach(() => { + fixture.destroy(); + subscription.unsubscribe(); + closed.complete(); + TestBed.resetTestingModule(); + }); + + it('keeps Name sort semantics on the header and checkbox interactions separate from sorting', () => { + const nameHeader: HTMLElement = fixture.nativeElement.querySelector('th.name-column'); + const sortButton = nameHeader.querySelector('[role="button"]'); + expect(nameHeader.getAttribute('aria-sort')).toBe('ascending'); + expect(nameHeader.querySelector('[aria-sort]')).toBeNull(); + expect(nameHeader.querySelector('input[type=checkbox]')).toBeNull(); + expect(document.getElementById(sortButton.getAttribute('aria-describedby')).textContent).toBe('Sort by Name'); + + const table = nameHeader.closest('table'); + const click = jest.fn(); + const keydown = jest.fn(); + table.addEventListener('click', click); + table.addEventListener('keydown', keydown); + const input = checkboxes()[1]; + expect(input.getAttribute('aria-label')).toContain('Alpha'); + input.dispatchEvent(new KeyboardEvent('keydown', { key: ' ', bubbles: true })); + selectFirst(); + expect(Object.keys(batch().selectedById)).toEqual([rows[0].id]); + expect(service.setSortKey).not.toHaveBeenCalled(); + expect(fixture.nativeElement.textContent).toContain('1 Selected'); + expect(fixture.nativeElement.querySelector('a').getAttribute('href')).toContain(rows[0].id); + const headerCheckbox = checkboxes()[0]; + expect(headerCheckbox.closest('[role="button"]')).toBeNull(); + headerCheckbox.dispatchEvent(new KeyboardEvent('keydown', { key: ' ', keyCode: 32, bubbles: true })); + headerCheckbox.click(); + fixture.detectChanges(); + expect(service.setSortKey).not.toHaveBeenCalled(); + expect(click).not.toHaveBeenCalled(); + expect(keydown).not.toHaveBeenCalled(); + table.removeEventListener('click', click); + table.removeEventListener('keydown', keydown); + + nameHeader.click(); + fixture.detectChanges(); + expect(service.setSortKey).toHaveBeenCalledWith('name'); + expect(service.setSortKey).toHaveBeenCalledTimes(1); + expect(nameHeader.getAttribute('aria-sort')).toBe('descending'); + expect(nameHeader.querySelector('[aria-sort]')).toBeNull(); + + nameHeader.closest('table').parentElement.scroll = jest.fn(); + sortButton.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter', keyCode: 13, bubbles: true })); + fixture.detectChanges(); + expect(service.setSortKey).toHaveBeenLastCalledWith(null); + expect(nameHeader.getAttribute('aria-sort')).toBe('none'); + + sortButton.dispatchEvent(new KeyboardEvent('keydown', { key: ' ', keyCode: 32, bubbles: true })); + fixture.detectChanges(); + expect(service.setSortKey).toHaveBeenLastCalledWith('name'); + expect(nameHeader.getAttribute('aria-sort')).toBe('ascending'); + }); + + it('clears a mixed header visually and keeps subsequent select-all toggles synchronized', fakeAsync(() => { + selectFirst(); + const header = checkboxes()[0]; + const tooltip = fixture.debugElement.query(By.css('th.batch-select-column mat-checkbox')).injector.get(MatTooltip); + expect(header.indeterminate).toBe(true); + expect(header.getAttribute('aria-label')).toBe('Clear all selections'); + expect(tooltip.message).toBe('Clear all selections'); + expect(tooltip.position).toBe('above'); + + header.click(); + fixture.detectChanges(); + tick(); + fixture.detectChanges(); + expect(Object.keys(batch().selectedById)).toHaveLength(0); + expect(header.checked).toBe(false); + expect(header.indeterminate).toBe(false); + expect(header.getAttribute('aria-label')).toBe('Select all loaded items'); + expect(tooltip.message).toBe('Select all loaded items'); + + header.click(); + fixture.detectChanges(); + expect(Object.keys(batch().selectedById)).toHaveLength(rows.length); + expect(header.checked).toBe(true); + expect(header.indeterminate).toBe(false); + + header.click(); + fixture.detectChanges(); + expect(Object.keys(batch().selectedById)).toHaveLength(0); + expect(checkboxes().every((input) => !input.checked && !input.indeterminate)).toBe(true); + })); + + it('retains a mixed header with no matching rows and clears hidden selections to restore Import', fakeAsync(() => { + selectFirst(); + load([]); + fixture.detectChanges(); + expect(checkboxes()).toHaveLength(1); + expect(checkboxes()[0].indeterminate).toBe(true); + expect(checkboxes()[0].getAttribute('aria-label')).toBe('Clear all selections'); + expect(fixture.nativeElement.textContent).toContain('1 Selected'); + expect(fixture.nativeElement.querySelector('td[colspan]').colSpan).toBe( + fixture.nativeElement.querySelectorAll('th').length + ); + checkboxes()[0].click(); + fixture.detectChanges(); + expect(Object.keys(batch().selectedById)).toHaveLength(0); + expect(checkboxes()[0].checked).toBe(false); + expect(checkboxes()[0].indeterminate).toBe(false); + expect(fixture.nativeElement.textContent).not.toContain('Selected'); + openMenu(); + expect(overlay.getContainerElement().textContent).toContain('Import'); + expect(overlay.getContainerElement().textContent).not.toContain('Delete'); + })); + + it('opens one immutable confirmation immediately from the selection including hidden items', fakeAsync(() => { + checkboxes()[0].click(); + fixture.detectChanges(); + load([rows[0]]); + fixture.detectChanges(); + openMenu(); + const menuItem = overlay.getContainerElement().querySelector('button[mat-menu-item]') as HTMLButtonElement; + expect(menuItem.textContent).toContain('Delete'); + menuItem.click(); + fixture.detectChanges(); + tick(); + expect(dialogs.openBatchDeleteModal).toHaveBeenCalledTimes(1); + expect(fixture.nativeElement.querySelector('.selection-status')).toBeNull(); + const [entity, snapshot] = dialogs.openBatchDeleteModal.mock.calls[0]; + expect(entity).toBe(config.entity); + expect(snapshot.items.map((item) => item.id)).toEqual(rows.map((row) => row.id)); + closed.next(undefined); + fixture.detectChanges(); + expect(Object.keys(batch().selectedById)).toHaveLength(2); + expect(batch().confirmation).toBeNull(); + })); + + it('hides batch controls for a Reader while retaining the name and sort content', () => { + permissions$.next({ + experiments: { create: false, delete: false }, + featureFlags: { create: false, delete: false }, + segments: { create: false, delete: false }, + }); + store.dispatch(actionSetUserInfo({ user: { email: 'test@example.com', role: UserRole.READER } })); + fixture.detectChanges(); + expect(fixture.nativeElement.querySelectorAll('.batch-checkbox')).toHaveLength(0); + expect(fixture.nativeElement.querySelector('.section-card-menu-trigger')).toBeNull(); + expect(fixture.nativeElement.querySelector('.batch-select-column')).toBeNull(); + const nameCells: HTMLElement[] = [...fixture.nativeElement.querySelectorAll('.name-column')]; + expect(nameCells).toHaveLength(rows.length + 1); + expect(nameCells[0].querySelector('[role="button"]').textContent.trim()).toBe('Name'); + expect(nameCells[0].getAttribute('aria-sort')).toBe('ascending'); + rows.forEach((row, index) => expect(nameCells[index + 1].querySelector('a').textContent).toContain(row.name)); + }); + + it('preserves selection across collapse and removes tag expansion only for confirmed removals', () => { + selectFirst(); + fixture.componentInstance.onTagsExpanded(rows[0].id, true); + fixture.componentInstance.onTagsExpanded(rows[1].id, true); + fixture.componentInstance.onSectionCardExpandChange(false); + fixture.detectChanges(); + fixture.componentInstance.onSectionCardExpandChange(true); + fixture.detectChanges(); + expect(checkboxes()[1].checked).toBe(true); + store.dispatch(actions.confirmedRemoved({ ids: [rows[0].id] })); + fixture.detectChanges(); + expect(fixture.componentInstance.expandedTagsMap.has(rows[0].id)).toBe(false); + expect(fixture.componentInstance.expandedTagsMap.has(rows[1].id)).toBe(true); + expect(fixture.debugElement.query(By.directive(RootBatchDeleteDirective))).toBeTruthy(); + }); + + it('clears selection on leaving the root page and starts empty when returning', () => { + selectFirst(); + fixture.destroy(); + expect(Object.keys(batch().selectedById)).toHaveLength(0); + expect(batch().confirmation).toBeNull(); + fixture = TestBed.createComponent(config.component as any); + fixture.detectChanges(); + expect(checkboxes().every((input) => !input.checked && !input.indeterminate)).toBe(true); + }); + + it('clears navigation selection without discarding an in-flight deletion or its result', () => { + selectFirst(); + service.batch.prepareConfirmation(); + const snapshot = batch().confirmation; + service.batch.submit(snapshot.operationId); + fixture.destroy(); + expect(Object.keys(batch().selectedById)).toHaveLength(0); + expect(batch().operation.snapshot).toEqual(snapshot); + expect(batch().operation.status).toBe('submitting'); + store.dispatch( + actions.batchDeleteCompleted({ + operationId: snapshot.operationId, + result: { results: [{ id: rows[0].id, outcome: 'deleted' }] }, + }) + ); + expect(batch().operation.status).toBe('complete'); + expect(batch().removedIds).toContain(rows[0].id); + }); + + it('starts deletion only after the common dialog closes with confirmation', fakeAsync(() => { + selectFirst(); + service.batch.prepareConfirmation(); + const snapshot = batch().confirmation; + expect(batch().operation?.status).not.toBe('submitting'); + closed.next(true); + fixture.detectChanges(); + expect(batch().operation.snapshot).toEqual(snapshot); + expect(batch().operation.status).toBe('submitting'); + expect(batch().confirmation).toBeNull(); + service.batch.prepareConfirmation(); + expect(dialogs.openBatchDeleteModal).toHaveBeenCalledTimes(1); + })); + + it.each(['cancel', 'close', 'confirm'] as const)( + 'uses the common dialog and does not refocus the menu trigger after %s', + fakeAsync((action) => { + const realDialogs = new DialogService(TestBed.inject(MatDialog), TestBed.inject(TranslateService)); + dialogs.openBatchDeleteModal.mockImplementation((entity, snapshot) => + realDialogs.openBatchDeleteModal(entity, snapshot) + ); + selectFirst(); + openMenu(); + const item = overlay.getContainerElement().querySelector('button[mat-menu-item]') as HTMLButtonElement; + expect(item.textContent.trim()).toBe(translations[`batch-delete.dialog.${config.entity}.title`]); + item.focus(); + item.click(); + fixture.detectChanges(); + tick(); + const ref = dialogs.openBatchDeleteModal.mock.results[0].value; + const container = overlay.getContainerElement(); + const input = container.querySelector('input') as HTMLInputElement; + input.focus(); + const trigger = fixture.nativeElement.querySelector('.section-card-menu-trigger') as HTMLButtonElement; + const focus = jest.spyOn(trigger, 'focus'); + if (action === 'confirm') { + input.value = 'delete'; + input.dispatchEvent(new Event('input')); + ref.componentRef.changeDetectorRef.detectChanges(); + tick(); + (container.querySelector('.footer-container button:not(.cancel-btn)') as HTMLButtonElement).click(); + } else { + (container.querySelector(`.${action}-btn`) as HTMLButtonElement).click(); + } + tick(); + fixture.detectChanges(); + expect(TestBed.inject(MatDialog).openDialogs).toHaveLength(0); + expect(focus).not.toHaveBeenCalled(); + expect(document.activeElement).not.toBe(trigger); + expect(batch().confirmation).toBeNull(); + expect(batch().operation?.status === 'submitting').toBe(action === 'confirm'); + focus.mockRestore(); + }) + ); + + it('separates User Manager delete permission from create permission', () => { + permissions$.next({ + experiments: { create: false, delete: false }, + featureFlags: { create: false, delete: false }, + segments: { create: false, delete: true }, + }); + store.dispatch(actionSetUserInfo({ user: { email: 'test@example.com', role: UserRole.USER_MANAGER } })); + fixture.detectChanges(); + expect(fixture.nativeElement.querySelector('.section-card-menu-trigger')).toBeNull(); + expect(fixture.nativeElement.querySelectorAll('.batch-checkbox')).toHaveLength( + config.entity === 'segments' ? rows.length + 1 : 0 + ); + if (config.entity === 'segments') { + selectFirst(); + const trigger = fixture.nativeElement.querySelector('.section-card-menu-trigger') as HTMLButtonElement; + expect(trigger).not.toBeNull(); + expect(trigger.disabled).toBe(false); + } + }); + + it.each(['list', 'deletion'])('keeps the existing progress bar until both requests finish (%s first)', (first) => { + const progressBar = () => fixture.nativeElement.querySelector('mat-progress-bar'); + const nameLinks = (): HTMLAnchorElement[] => [...fixture.nativeElement.querySelectorAll('td.name-column a')]; + const navigate = jest.spyOn(TestBed.inject(Router), 'navigateByUrl').mockResolvedValue(true); + selectFirst(); + expect(progressBar()).toBeNull(); + expect(nameLinks().every((link) => link.hasAttribute('href'))).toBe(true); + store.dispatch(actions.prepareConfirmation({ operationId: 'pending-delete' })); + store.dispatch(actions.batchDeleteRequested({ snapshot: batch().confirmation })); + fixture.detectChanges(); + expect(progressBar()).not.toBeNull(); + const trigger = fixture.debugElement.query(By.css('.section-card-menu-trigger')); + expect(trigger.nativeElement.disabled).toBe(true); + expect(trigger.parent.injector.get(MatTooltip).message).toBe(''); + for (const link of nameLinks()) { + expect(link.hasAttribute('href')).toBe(false); + expect(link.getAttribute('aria-disabled')).toBe('true'); + link.click(); + } + expect(navigate).not.toHaveBeenCalled(); + + listLoading$.next(true); + const finishDeletion = () => + store.dispatch( + actions.batchDeleteCompleted({ + operationId: 'pending-delete', + result: { results: [{ id: rows[0].id, outcome: 'deleted' }] }, + }) + ); + if (first === 'list') listLoading$.next(false); + else finishDeletion(); + fixture.detectChanges(); + expect(progressBar()).not.toBeNull(); + expect(nameLinks().every((link) => link.hasAttribute('href'))).toBe(first === 'deletion'); + + if (first === 'list') finishDeletion(); + else listLoading$.next(false); + fixture.detectChanges(); + expect(progressBar()).toBeNull(); + expect(nameLinks().every((link) => link.hasAttribute('href') && !link.hasAttribute('aria-disabled'))).toBe(true); + nameLinks()[0].click(); + expect(navigate).toHaveBeenCalledTimes(1); + navigate.mockRestore(); + }); + + it('keeps checkboxes usable without a banner or reload button after request and refresh failures', () => { + selectFirst(); + store.dispatch(actions.prepareConfirmation({ operationId: 'offline-delete' })); + store.dispatch(actions.batchDeleteRequested({ snapshot: batch().confirmation })); + fixture.detectChanges(); + expect(checkboxes()[1].disabled).toBe(true); + expect(fixture.nativeElement.querySelector('mat-progress-bar')).not.toBeNull(); + store.dispatch(actions.batchDeleteRequestFailed({ operationId: 'offline-delete', status: 0 })); + fixture.detectChanges(); + expect(checkboxes().every((input) => !input.disabled)).toBe(true); + expect(fixture.nativeElement.querySelector('td.name-column a').hasAttribute('href')).toBe(true); + expect(fixture.nativeElement.querySelector('mat-progress-bar')).toBeNull(); + checkboxes()[1].click(); + fixture.detectChanges(); + expect(Object.keys(batch().selectedById)).toHaveLength(0); + checkboxes()[1].click(); + fixture.detectChanges(); + expect(Object.keys(batch().selectedById)).toHaveLength(1); + + store.dispatch(actions.listRequested({ requestId: 'failed-refresh' })); + store.dispatch(actions.listFailed({ requestId: 'failed-refresh' })); + fixture.detectChanges(); + expect(fixture.nativeElement.querySelector('app-common-batch-selection-status')).toBeNull(); + expect(fixture.nativeElement.querySelector('.selection-status')).toBeNull(); + expect(fixture.nativeElement.textContent).not.toContain('Reload list'); + checkboxes()[2].click(); + fixture.detectChanges(); + expect(Object.keys(batch().selectedById)).toHaveLength(2); + }); + + it.each( + config.entity === 'experiments' + ? [EXPERIMENT_STATE.PREVIEW, EXPERIMENT_STATE.SCHEDULED, EXPERIMENT_STATE.RUNNING, EXPERIMENT_STATE.PAUSED] + : [config.entity === 'flags' ? FEATURE_FLAG_STATUS.ENABLED : SEGMENT_STATUS.USED] + )('blocks mixed selections with a hidden %s item using only the existing menu tooltip', (status) => { + load([ + { + ...rows[0], + state: config.entity === 'experiments' ? (status as EXPERIMENT_STATE) : undefined, + status: config.entity === 'segments' ? SEGMENT_STATUS.USED : FEATURE_FLAG_STATUS.ENABLED, + }, + rows[1], + ]); + fixture.detectChanges(); + checkboxes()[0].click(); + fixture.detectChanges(); + const trigger = fixture.nativeElement.querySelector('.section-card-menu-trigger') as HTMLButtonElement; + expect(trigger.disabled).toBe(true); + load([rows[1]]); + fixture.detectChanges(); + fixture.componentInstance.batchUi.requestDelete(); + fixture.detectChanges(); + expect(trigger.disabled).toBe(true); + expect(trigger.parentElement.getAttribute('aria-label')).toContain('Deselect'); + expect(fixture.nativeElement.querySelector('.selection-status')).toBeNull(); + expect(fixture.nativeElement.textContent).not.toContain('Refresh selection status'); + expect(Object.keys(batch().selectedById)).toEqual(rows.map(({ id }) => id)); + expect(batch().confirmation).toBeNull(); + expect(dialogs.openBatchDeleteModal).not.toHaveBeenCalled(); + + checkboxes()[0].click(); + fixture.detectChanges(); + selectFirst(); + fixture.componentInstance.batchUi.requestDelete(); + expect(dialogs.openBatchDeleteModal).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/frontend/projects/upgrade/src/app/features/dashboard/experiments/pages/experiment-root-page/experiment-root-page-content/experiment-root-section-card/experiment-root-section-card-table/experiment-root-section-card-table.component.html b/packages/frontend/projects/upgrade/src/app/features/dashboard/experiments/pages/experiment-root-page/experiment-root-page-content/experiment-root-section-card/experiment-root-section-card-table/experiment-root-section-card-table.component.html index fcb68092da..0d8d673272 100644 --- a/packages/frontend/projects/upgrade/src/app/features/dashboard/experiments/pages/experiment-root-page/experiment-root-page-content/experiment-root-section-card/experiment-root-section-card-table/experiment-root-section-card-table.component.html +++ b/packages/frontend/projects/upgrade/src/app/features/dashboard/experiments/pages/experiment-root-page/experiment-root-page-content/experiment-root-section-card/experiment-root-section-card-table/experiment-root-section-card-table.component.html @@ -1,5 +1,6 @@ +@let selection = (batch.selection$ | async); @let batchState = (batch.state$ | async);
- @if (isLoading$ | async) { + @if ((isLoading$ | async) || selection?.busy) { } + + + + + + -
+ + + + + - {{ EXPERIMENT_TRANSLATION_KEYS.NAME | translate }} + + {{ + selection?.selectedCount + ? ('batch-delete.selection.count' | translate : { count: selection.selectedCount }) + : (EXPERIMENT_TRANSLATION_KEYS.NAME | translate) + }} ; @Input() isSearchActive$: Observable; @Input() expandedTagsMap: Map; + @Input() canSelect = false; @Output() tagsExpanded = new EventEmitter<{ experimentId: string; expanded: boolean }>(); experimentSortKey$ = this.experimentService.selectExperimentSortKey$; experimentSortAs$ = this.experimentService.selectExperimentSortAs$; @@ -58,6 +64,7 @@ export class ExperimentRootSectionCardTableComponent implements AfterViewInit, O @ViewChild('bottomTrigger') bottomTrigger: ElementRef; private observer: IntersectionObserver; + readonly batch = this.experimentService.batch; constructor(private readonly experimentService: ExperimentService) {} @@ -106,7 +113,7 @@ export class ExperimentRootSectionCardTableComponent implements AfterViewInit, O } get displayedColumns(): string[] { - return EXPERIMENT_ROOT_DISPLAYED_COLUMNS; + return this.canSelect ? ['select', ...EXPERIMENT_ROOT_DISPLAYED_COLUMNS] : EXPERIMENT_ROOT_DISPLAYED_COLUMNS; } get EXPERIMENT_TRANSLATION_KEYS() { diff --git a/packages/frontend/projects/upgrade/src/app/features/dashboard/experiments/pages/experiment-root-page/experiment-root-page-content/experiment-root-section-card/experiment-root-section-card.component.html b/packages/frontend/projects/upgrade/src/app/features/dashboard/experiments/pages/experiment-root-page/experiment-root-page-content/experiment-root-section-card/experiment-root-section-card.component.html index 860baeb00b..29d197176a 100644 --- a/packages/frontend/projects/upgrade/src/app/features/dashboard/experiments/pages/experiment-root-page/experiment-root-page-content/experiment-root-section-card/experiment-root-section-card.component.html +++ b/packages/frontend/projects/upgrade/src/app/features/dashboard/experiments/pages/experiment-root-page/experiment-root-page-content/experiment-root-section-card/experiment-root-section-card.component.html @@ -1,4 +1,11 @@ - + + @let batchView = (batchUi.view$ | async); diff --git a/packages/frontend/projects/upgrade/src/app/features/dashboard/experiments/pages/experiment-root-page/experiment-root-page-content/experiment-root-section-card/experiment-root-section-card.component.ts b/packages/frontend/projects/upgrade/src/app/features/dashboard/experiments/pages/experiment-root-page/experiment-root-page-content/experiment-root-section-card/experiment-root-section-card.component.ts index c182f35dbc..60ca46a8d7 100644 --- a/packages/frontend/projects/upgrade/src/app/features/dashboard/experiments/pages/experiment-root-page/experiment-root-page-content/experiment-root-section-card/experiment-root-section-card.component.ts +++ b/packages/frontend/projects/upgrade/src/app/features/dashboard/experiments/pages/experiment-root-page/experiment-root-page-content/experiment-root-section-card/experiment-root-section-card.component.ts @@ -1,4 +1,5 @@ -import { ChangeDetectionStrategy, Component } from '@angular/core'; +import { ChangeDetectionStrategy, Component, ViewChild } from '@angular/core'; +import { RootBatchDeleteDirective } from '../../../../../../../shared/directives/root-batch-delete.directive'; import { CommonSectionCardComponent, CommonSectionCardSearchHeaderComponent, @@ -23,6 +24,7 @@ import { StratificationFactorsService } from '../../../../../../../core/stratifi @Component({ selector: 'app-experiment-root-section-card', imports: [ + RootBatchDeleteDirective, CommonSectionCardComponent, CommonSectionCardSearchHeaderComponent, CommonSectionCardActionButtonsComponent, @@ -37,6 +39,9 @@ import { StratificationFactorsService } from '../../../../../../../core/stratifi }) export class ExperimentRootSectionCardComponent { permissions$: Observable; + readonly batch = this.experimentService.batch; + @ViewChild(RootBatchDeleteDirective) batchUi: RootBatchDeleteDirective; + experiments$ = this.experimentService.experiments$; isLoadingExperiments$ = this.experimentService.isLoadingExperiment$; isInitialLoading$ = this.experimentService.haveInitialExperimentsLoaded(); @@ -101,6 +106,10 @@ export class ExperimentRootSectionCardComponent { } onMenuButtonItemClick(action: string) { + if (action === 'batch-delete') { + this.batchUi.requestDelete(); + return; + } if (action === EXPERIMENT_BUTTON_ACTION.IMPORT) { this.dialogService.openImportExperimentModal(); } diff --git a/packages/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/pages/feature-flag-root-page/feature-flag-root-page-content/feature-flag-root-section-card/feature-flag-root-section-card-table/feature-flag-root-section-card-table.component.html b/packages/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/pages/feature-flag-root-page/feature-flag-root-page-content/feature-flag-root-section-card/feature-flag-root-section-card-table/feature-flag-root-section-card-table.component.html index 2901f6f0d6..62f208f99b 100644 --- a/packages/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/pages/feature-flag-root-page/feature-flag-root-page-content/feature-flag-root-section-card/feature-flag-root-section-card-table/feature-flag-root-section-card-table.component.html +++ b/packages/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/pages/feature-flag-root-page/feature-flag-root-page-content/feature-flag-root-section-card/feature-flag-root-section-card-table/feature-flag-root-section-card-table.component.html @@ -1,5 +1,6 @@ +@let selection = (batch.selection$ | async); @let batchState = (batch.state$ | async);
- @if (isLoading$ | async) { + @if ((isLoading$ | async) || selection?.busy) { } @let featureFlags = (featureFlags$ | async); + + + + + + -
+ + + + + - {{ FLAG_TRANSLATION_KEYS.NAME | translate }} + + {{ + selection?.selectedCount + ? ('batch-delete.selection.count' | translate : { count: selection.selectedCount }) + : (FLAG_TRANSLATION_KEYS.NAME | translate) + }} ; @Input() isSearchActive$: Observable; @Input() expandedTagsMap: Map; + @Input() canSelect = false; @Output() tagsExpanded = new EventEmitter<{ flagId: string; expanded: boolean }>(); flagSortKey$ = this.featureFlagsService.sortKey$; flagSortAs$ = this.featureFlagsService.sortAs$; @@ -56,6 +62,7 @@ export class FeatureFlagRootSectionCardTableComponent implements AfterViewInit, @ViewChild('bottomTrigger') bottomTrigger: ElementRef; private observer: IntersectionObserver; + readonly batch = this.featureFlagsService.batch; constructor(private featureFlagsService: FeatureFlagsService) {} @@ -109,7 +116,7 @@ export class FeatureFlagRootSectionCardTableComponent implements AfterViewInit, } get displayedColumns(): string[] { - return FLAG_ROOT_DISPLAYED_COLUMNS; + return this.canSelect ? ['select', ...FLAG_ROOT_DISPLAYED_COLUMNS] : FLAG_ROOT_DISPLAYED_COLUMNS; } get FLAG_TRANSLATION_KEYS() { diff --git a/packages/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/pages/feature-flag-root-page/feature-flag-root-page-content/feature-flag-root-section-card/feature-flag-root-section-card.component.html b/packages/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/pages/feature-flag-root-page/feature-flag-root-page-content/feature-flag-root-section-card/feature-flag-root-section-card.component.html index 3e339033c4..c633571e02 100644 --- a/packages/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/pages/feature-flag-root-page/feature-flag-root-page-content/feature-flag-root-section-card/feature-flag-root-section-card.component.html +++ b/packages/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/pages/feature-flag-root-page/feature-flag-root-page-content/feature-flag-root-section-card/feature-flag-root-section-card.component.html @@ -1,4 +1,11 @@ - + + @let batchView = (batchUi.view$ | async); ; - featureFlags$: Observable; + readonly batch = this.featureFlagService.batch; + @ViewChild(RootBatchDeleteDirective) batchUi: RootBatchDeleteDirective; + + featureFlags$: Observable = this.featureFlagService.featureFlags$; isLoadingFeatureFlags$ = this.featureFlagService.isLoadingFeatureFlags$; isInitialLoading$ = this.featureFlagService.isInitialFeatureFlagsLoading$; isAllFlagsFetched$ = this.featureFlagService.isAllFlagsFetched$; @@ -86,13 +91,9 @@ export class FeatureFlagRootSectionCardComponent { this.featureFlagService.fetchFeatureFlags(true); } - ngAfterViewInit() { - this.featureFlags$ = this.featureFlagService.featureFlags$; - } - onSearch(params: CommonSearchWidgetSearchParams) { - this.featureFlagService.setSearchString(params.searchString?.trim() || ''); this.featureFlagService.setSearchKey(params.searchKey as FLAG_SEARCH_KEY); + this.featureFlagService.setSearchString(params.searchString?.trim() || ''); } onAddFeatureFlagButtonClick() { @@ -100,6 +101,10 @@ export class FeatureFlagRootSectionCardComponent { } onMenuButtonItemClick(action: string) { + if (action === 'batch-delete') { + this.batchUi.requestDelete(); + return; + } if (action === FEATURE_FLAG_BUTTON_ACTION.IMPORT) { this.dialogService.openImportFeatureFlagModal(); } diff --git a/packages/frontend/projects/upgrade/src/app/features/dashboard/segments/pages/segment-root-page/segment-root-page-content/segment-root-section-card/segment-root-section-card-table/segment-root-section-card-table.component.html b/packages/frontend/projects/upgrade/src/app/features/dashboard/segments/pages/segment-root-page/segment-root-page-content/segment-root-section-card/segment-root-section-card-table/segment-root-section-card-table.component.html index e0e3e68e29..f6d185aa9f 100644 --- a/packages/frontend/projects/upgrade/src/app/features/dashboard/segments/pages/segment-root-page/segment-root-page-content/segment-root-section-card/segment-root-section-card-table/segment-root-section-card-table.component.html +++ b/packages/frontend/projects/upgrade/src/app/features/dashboard/segments/pages/segment-root-page/segment-root-page-content/segment-root-section-card/segment-root-section-card-table/segment-root-section-card-table.component.html @@ -1,5 +1,6 @@ +@let selection = (batch.selection$ | async); @let batchState = (batch.state$ | async);
- @if (isLoading$ | async) { + @if ((isLoading$ | async) || selection?.busy) { } @let segments = (segments$ | async); + + + + + + -
+ + + + + - {{ SEGMENT_TRANSLATION_KEYS.NAME | translate }} + + {{ + selection?.selectedCount + ? ('batch-delete.selection.count' | translate : { count: selection.selectedCount }) + : (SEGMENT_TRANSLATION_KEYS.NAME | translate) + }} ; @Input() isSearchActive$: Observable; @Input() expandedTagsMap: Map; + @Input() canSelect = false; @Output() tagsExpanded = new EventEmitter<{ segmentId: string; expanded: boolean }>(); segmentSortKey$ = this.segmentsService.selectSegmentSortKey$; segmentSortAs$ = this.segmentsService.selectSegmentSortAs$; @@ -55,6 +61,7 @@ export class SegmentRootSectionCardTableComponent implements AfterViewInit, OnDe @ViewChild('bottomTrigger') bottomTrigger: ElementRef; private observer: IntersectionObserver; + readonly batch = this.segmentsService.batch; constructor(private segmentsService: SegmentsService) {} @@ -89,6 +96,7 @@ export class SegmentRootSectionCardTableComponent implements AfterViewInit, OnDe filterSegmentByChips(tagValue: string, type: SEGMENT_SEARCH_KEY) { this.setSearchKey(type); this.setSearchString(tagValue); + this.segmentsService.fetchSegmentsPaginated(true); } setSearchKey(searchKey: SEGMENT_SEARCH_KEY) { @@ -100,7 +108,7 @@ export class SegmentRootSectionCardTableComponent implements AfterViewInit, OnDe } get displayedColumns(): string[] { - return SEGMENT_ROOT_DISPLAYED_COLUMNS; + return this.canSelect ? ['select', ...SEGMENT_ROOT_DISPLAYED_COLUMNS] : SEGMENT_ROOT_DISPLAYED_COLUMNS; } get SEGMENT_TRANSLATION_KEYS() { diff --git a/packages/frontend/projects/upgrade/src/app/features/dashboard/segments/pages/segment-root-page/segment-root-page-content/segment-root-section-card/segment-root-section-card.component.html b/packages/frontend/projects/upgrade/src/app/features/dashboard/segments/pages/segment-root-page/segment-root-page-content/segment-root-section-card/segment-root-section-card.component.html index 8f94b291d0..c6501d58eb 100644 --- a/packages/frontend/projects/upgrade/src/app/features/dashboard/segments/pages/segment-root-page/segment-root-page-content/segment-root-section-card/segment-root-section-card.component.html +++ b/packages/frontend/projects/upgrade/src/app/features/dashboard/segments/pages/segment-root-page/segment-root-page-content/segment-root-section-card/segment-root-section-card.component.html @@ -1,4 +1,11 @@ - + + @let batchView = (batchUi.view$ | async); ; + readonly batch = this.segmentsService.batch; + @ViewChild(RootBatchDeleteDirective) batchUi: RootBatchDeleteDirective; + segments$ = this.segmentsService.selectAllSegments$; isLoadingSegments$ = this.segmentsService.isLoadingSegments$; isInitialLoading$ = this.segmentsService.isInitialSegmentsLoading(); @@ -90,6 +95,10 @@ export class SegmentRootSectionCardComponent { } onMenuButtonItemClick(action: string) { + if (action === 'batch-delete') { + this.batchUi.requestDelete(); + return; + } if (action === SEGMENTS_BUTTON_ACTION.IMPORT) { this.dialogService.openImportSegmentModal(); } else if (action === SEGMENTS_BUTTON_ACTION.EXPORT_ALL) { diff --git a/packages/frontend/projects/upgrade/src/app/shared-standalone-component-lib/components/common-modal/common-modal.component.html b/packages/frontend/projects/upgrade/src/app/shared-standalone-component-lib/components/common-modal/common-modal.component.html index d95976d75b..384eee0582 100644 --- a/packages/frontend/projects/upgrade/src/app/shared-standalone-component-lib/components/common-modal/common-modal.component.html +++ b/packages/frontend/projects/upgrade/src/app/shared-standalone-component-lib/components/common-modal/common-modal.component.html @@ -2,7 +2,12 @@

{{ title | translate }}

-
diff --git a/packages/frontend/projects/upgrade/src/app/shared-standalone-component-lib/components/common-section-card-action-buttons/common-section-card-action-buttons.component.html b/packages/frontend/projects/upgrade/src/app/shared-standalone-component-lib/components/common-section-card-action-buttons/common-section-card-action-buttons.component.html index 6c7db85e24..714de676fd 100644 --- a/packages/frontend/projects/upgrade/src/app/shared-standalone-component-lib/components/common-section-card-action-buttons/common-section-card-action-buttons.component.html +++ b/packages/frontend/projects/upgrade/src/app/shared-standalone-component-lib/components/common-section-card-action-buttons/common-section-card-action-buttons.component.html @@ -54,8 +54,22 @@
@if (showMenuButton) { -
-
diff --git a/packages/frontend/projects/upgrade/src/app/shared-standalone-component-lib/components/common-selection-checkbox/common-selection-checkbox.component.html b/packages/frontend/projects/upgrade/src/app/shared-standalone-component-lib/components/common-selection-checkbox/common-selection-checkbox.component.html new file mode 100644 index 0000000000..a794cd4fb6 --- /dev/null +++ b/packages/frontend/projects/upgrade/src/app/shared-standalone-component-lib/components/common-selection-checkbox/common-selection-checkbox.component.html @@ -0,0 +1,13 @@ + diff --git a/packages/frontend/projects/upgrade/src/app/shared-standalone-component-lib/components/common-selection-checkbox/common-selection-checkbox.component.scss b/packages/frontend/projects/upgrade/src/app/shared-standalone-component-lib/components/common-selection-checkbox/common-selection-checkbox.component.scss new file mode 100644 index 0000000000..c340541b9a --- /dev/null +++ b/packages/frontend/projects/upgrade/src/app/shared-standalone-component-lib/components/common-selection-checkbox/common-selection-checkbox.component.scss @@ -0,0 +1,31 @@ +@use '@angular/material' as mat; + +:host { + display: inline-block; +} + +// Match the theme's checkbox specificity with the component's scoped selector. +:host .mat-mdc-checkbox { + width: 40px; + + @include mat.checkbox-overrides( + ( + unselected-icon-color: var(--grey-7), + unselected-hover-icon-color: var(--grey-7), + unselected-focus-icon-color: var(--grey-7), + selected-hover-state-layer-opacity: 0, + unselected-hover-state-layer-opacity: 0, + selected-focus-state-layer-opacity: 0, + unselected-focus-state-layer-opacity: 0, + selected-pressed-state-layer-opacity: 0, + unselected-pressed-state-layer-opacity: 0, + ) + ); + + // Keep keyboard focus visible without leaving a circle after a pointer click. + &:has(input:focus-visible) { + outline: 2px solid var(--blue); + outline-offset: -4px; + border-radius: 4px; + } +} diff --git a/packages/frontend/projects/upgrade/src/app/shared-standalone-component-lib/components/common-selection-checkbox/common-selection-checkbox.component.ts b/packages/frontend/projects/upgrade/src/app/shared-standalone-component-lib/components/common-selection-checkbox/common-selection-checkbox.component.ts new file mode 100644 index 0000000000..40f2ca6b4e --- /dev/null +++ b/packages/frontend/projects/upgrade/src/app/shared-standalone-component-lib/components/common-selection-checkbox/common-selection-checkbox.component.ts @@ -0,0 +1,20 @@ +import { ChangeDetectionStrategy, Component, EventEmitter, Input, Output } from '@angular/core'; +import { MatCheckboxModule } from '@angular/material/checkbox'; +import { MatTooltipModule } from '@angular/material/tooltip'; + +/** Presentational selection control; the parent owns row/header selection and permissions. */ +@Component({ + selector: 'app-common-selection-checkbox', + imports: [MatCheckboxModule, MatTooltipModule], + templateUrl: './common-selection-checkbox.component.html', + styleUrl: './common-selection-checkbox.component.scss', + changeDetection: ChangeDetectionStrategy.OnPush, +}) +export class CommonSelectionCheckboxComponent { + @Input() checked = false; + @Input() indeterminate = false; + @Input() disabled = false; + @Input() ariaLabel = ''; + @Input() tooltip = ''; + @Output() toggle = new EventEmitter(); +} diff --git a/packages/frontend/projects/upgrade/src/app/shared-standalone-component-lib/components/common-simple-text-validated-confirmation-modal/batch-confirmation.spec.ts b/packages/frontend/projects/upgrade/src/app/shared-standalone-component-lib/components/common-simple-text-validated-confirmation-modal/batch-confirmation.spec.ts new file mode 100644 index 0000000000..6f9a4cfc4a --- /dev/null +++ b/packages/frontend/projects/upgrade/src/app/shared-standalone-component-lib/components/common-simple-text-validated-confirmation-modal/batch-confirmation.spec.ts @@ -0,0 +1,101 @@ +import { TestBed, fakeAsync, tick } from '@angular/core/testing'; +import { NoopAnimationsModule } from '@angular/platform-browser/animations'; +import { MatDialog, MatDialogModule, MatDialogRef } from '@angular/material/dialog'; +import { OverlayContainer } from '@angular/cdk/overlay'; +import { TranslateModule, TranslateService } from '@ngx-translate/core'; +import { BatchDeleteEntity } from 'upgrade_types'; +import { DialogService } from '../../../shared/services/common-dialog.service'; +import { CommonSimpleTextValidatedConfirmationModalComponent } from './common-simple-text-validated-confirmation-modal.component'; + +const translations = jest.requireActual('../../../../assets/i18n/en.json'); + +describe('Batch deletion using the existing text confirmation dialog', () => { + let ref: MatDialogRef; + let container: HTMLElement; + const primary = () => container.querySelector('.footer-container button:not(.cancel-btn)') as HTMLButtonElement; + function detect() { + ref.componentRef.changeDetectorRef.detectChanges(); + tick(); + } + function open(entity: BatchDeleteEntity = 'experiments', count = 1) { + const snapshot = { + operationId: 'operation', + items: Array.from({ length: count }, (_, index) => ({ id: String(index), name: `Item ${index}` })), + }; + ref = TestBed.inject(DialogService).openBatchDeleteModal(entity, snapshot); + detect(); + } + function input(value: string) { + const field = container.querySelector('input'); + field.value = value; + field.dispatchEvent(new Event('input')); + detect(); + } + beforeEach(async () => { + await TestBed.configureTestingModule({ + imports: [ + CommonSimpleTextValidatedConfirmationModalComponent, + MatDialogModule, + NoopAnimationsModule, + TranslateModule.forRoot(), + ], + }).compileComponents(); + const translate = TestBed.inject(TranslateService); + translate.setTranslation('en', translations); + translate.use('en'); + container = TestBed.inject(OverlayContainer).getContainerElement(); + }); + afterEach(() => { + TestBed.inject(MatDialog).closeAll(); + TestBed.resetTestingModule(); + }); + + it.each([ + ['experiments', 1, 'Delete Experiments', '1 experiment'], + ['experiments', 3, 'Delete Experiments', '3 experiments'], + ['flags', 1, 'Delete Feature Flags', '1 feature flag'], + ['flags', 3, 'Delete Feature Flags', '3 feature flags'], + ['segments', 1, 'Delete Segments', '1 segment'], + ['segments', 3, 'Delete Segments', '3 segments'], + ] as const)( + 'uses common confirmation for %s with %i items', + fakeAsync((entity, count, title, phrase) => { + open(entity, count); + expect(ref.componentInstance).toBeInstanceOf(CommonSimpleTextValidatedConfirmationModalComponent); + expect(container.querySelector('h4').textContent).toBe(title); + expect(container.textContent).toContain(`Are you sure you want to delete ${phrase}?`); + expect(container.querySelectorAll('.validation-modal-content > p')).toHaveLength(1); + expect((container.querySelector('.cdk-overlay-pane') as HTMLElement).style.width).toBe('480px'); + expect(container.querySelector('input').autocomplete).toBe('off'); + expect(primary().classList.contains('mat-warn')).toBe(true); + expect(primary().disabled).toBe(true); + }) + ); + + it('closes with confirmation before deletion, without any progress or result screen', fakeAsync(() => { + open(); + const closed = jest.fn(); + ref.afterClosed().subscribe(closed); + input('wrong'); + expect(primary().disabled).toBe(true); + input(' DeLeTe '); + expect(primary().disabled).toBe(false); + primary().click(); + detect(); + expect(closed).toHaveBeenCalledTimes(1); + expect(closed).toHaveBeenCalledWith(true); + expect(TestBed.inject(MatDialog).openDialogs).toHaveLength(0); + expect(container.textContent).not.toContain('Deleting'); + expect(container.querySelector('mat-progress-bar')).toBeNull(); + })); + + it('ignores Escape and backdrop clicks', fakeAsync(() => { + open(); + container + .querySelector('mat-dialog-container') + .dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape', keyCode: 27, bubbles: true })); + (container.querySelector('.cdk-overlay-backdrop') as HTMLElement).click(); + tick(); + expect(TestBed.inject(MatDialog).openDialogs).toHaveLength(1); + })); +}); diff --git a/packages/frontend/projects/upgrade/src/app/shared-standalone-component-lib/components/common-simple-text-validated-confirmation-modal/common-simple-text-validated-confirmation-modal.component.html b/packages/frontend/projects/upgrade/src/app/shared-standalone-component-lib/components/common-simple-text-validated-confirmation-modal/common-simple-text-validated-confirmation-modal.component.html index b84f44c062..6017fb16fc 100644 --- a/packages/frontend/projects/upgrade/src/app/shared-standalone-component-lib/components/common-simple-text-validated-confirmation-modal/common-simple-text-validated-confirmation-modal.component.html +++ b/packages/frontend/projects/upgrade/src/app/shared-standalone-component-lib/components/common-simple-text-validated-confirmation-modal/common-simple-text-validated-confirmation-modal.component.html @@ -21,6 +21,8 @@ ; + +/** Connect root-card controls to one confirmation dialog without owning the deletion request. */ +@Directive({ selector: '[appRootBatchDelete]', exportAs: 'rootBatchDelete' }) +export class RootBatchDeleteDirective implements OnInit, OnDestroy { + @Input() batchFacade: BatchDeleteFacade; + @Input() batchEntity: BatchDeleteEntity; + @Input() batchExpandedTags: Map; + view$: Observable; + private subscriptions = new Subscription(); + private dialogRef?: MatDialogRef; + constructor(private dialogs: DialogService) {} + + ngOnInit() { + this.view$ = this.batchFacade.state$.pipe( + map((state) => rootBatchDeleteView(state, this.batchEntity)), + shareReplay({ bufferSize: 1, refCount: true }) + ); + this.subscriptions.add( + this.batchFacade.state$.subscribe((state) => { + state.removedIds.forEach((id) => this.batchExpandedTags?.delete(id)); + if (!state.confirmation || this.dialogRef) return; + const ref = this.dialogs.openBatchDeleteModal(this.batchEntity, state.confirmation); + const operationId = state.confirmation.operationId; + this.dialogRef = ref; + this.subscriptions.add( + ref.afterClosed().subscribe((confirmed) => { + this.dialogRef = undefined; + if (confirmed) this.batchFacade.submit(operationId); + this.batchFacade.dismissConfirmation(); + }) + ); + }) + ); + } + + requestDelete() { + this.batchFacade.prepareConfirmation(); + } + + ngOnDestroy() { + this.subscriptions.unsubscribe(); + this.dialogRef?.close(); + this.batchFacade.leaveRootPage(); + } +} diff --git a/packages/frontend/projects/upgrade/src/app/shared/services/common-dialog.service.ts b/packages/frontend/projects/upgrade/src/app/shared/services/common-dialog.service.ts index 31cf18d105..3bb1ec2181 100644 --- a/packages/frontend/projects/upgrade/src/app/shared/services/common-dialog.service.ts +++ b/packages/frontend/projects/upgrade/src/app/shared/services/common-dialog.service.ts @@ -71,6 +71,9 @@ import { EditPayloadModalParams, } from '../../features/dashboard/experiments/modals/edit-payload-modal/edit-payload-modal.component'; import { Observable } from 'rxjs'; +import { TranslateService } from '@ngx-translate/core'; +import { BatchDeleteEntity } from 'upgrade_types'; +import { BatchDeleteSnapshot } from '../../core/batch-actions/batch-actions.models'; export interface ImportModalParams { importTypeAdapterToken: InjectionToken; @@ -107,7 +110,26 @@ export interface UpsertMetricModalParams { providedIn: 'root', }) export class DialogService { - constructor(private dialog: MatDialog) {} + constructor(private dialog: MatDialog, private translate: TranslateService) {} + + openBatchDeleteModal(entity: BatchDeleteEntity, snapshot: BatchDeleteSnapshot) { + const config: CommonModalConfig = { + title: `batch-delete.dialog.${entity}.title`, + primaryActionBtnLabel: 'Delete', + primaryActionBtnColor: 'warn', + cancelBtnLabel: 'Cancel', + params: { + message: this.translate.instant( + `batch-delete.dialog.${entity}.${snapshot.items.length === 1 ? 'one' : 'other'}`, + { count: snapshot.items.length } + ), + validationKeyword: 'delete', + validationPlaceholder: 'Type delete', + }, + }; + // Restoring focus here leaves the root menu trigger's focus circle visible after Cancel or Close. + return this.openTextValidatedConfirmationModal(config, ModalSize.SMALL, false); + } openAddExperimentModal() { const commonModalConfig: CommonModalConfig = { @@ -1320,13 +1342,15 @@ export class DialogService { openTextValidatedConfirmationModal( commonModalConfig: CommonModalConfig, - modalSize: ModalSize = ModalSize.MEDIUM + modalSize: ModalSize = ModalSize.MEDIUM, + restoreFocus = true ): MatDialogRef { const config: MatDialogConfig = { data: commonModalConfig, width: modalSize, autoFocus: 'input', disableClose: true, + restoreFocus, }; return this.dialog.open(CommonSimpleTextValidatedConfirmationModalComponent, config); diff --git a/packages/frontend/projects/upgrade/src/assets/i18n/en.json b/packages/frontend/projects/upgrade/src/assets/i18n/en.json index 50b54acdbc..13eb9a6bdb 100644 --- a/packages/frontend/projects/upgrade/src/assets/i18n/en.json +++ b/packages/frontend/projects/upgrade/src/assets/i18n/en.json @@ -1,4 +1,39 @@ { + "global.more-actions.text": "More actions", + "global.close-dialog.text": "Close dialog", + "batch-delete.selection.clear": "Clear all selections", + "batch-delete.selection.select-loaded": "Select all loaded items", + "batch-delete.selection.sort-name": "Sort by Name", + "batch-delete.selection.count": "{{count}} Selected", + "batch-delete.selection.experiments.row": "Select experiment {{name}}", + "batch-delete.selection.flags.row": "Select feature flag {{name}}", + "batch-delete.selection.segments.row": "Select segment {{name}}", + "batch-delete.dialog.experiments.title": "Delete Experiments", + "batch-delete.dialog.experiments.one": "Are you sure you want to delete 1 experiment?", + "batch-delete.dialog.experiments.other": "Are you sure you want to delete {{count}} experiments?", + "batch-delete.dialog.flags.title": "Delete Feature Flags", + "batch-delete.dialog.flags.one": "Are you sure you want to delete 1 feature flag?", + "batch-delete.dialog.flags.other": "Are you sure you want to delete {{count}} feature flags?", + "batch-delete.dialog.segments.title": "Delete Segments", + "batch-delete.dialog.segments.one": "Are you sure you want to delete 1 segment?", + "batch-delete.dialog.segments.other": "Are you sure you want to delete {{count}} segments?", + "batch-delete.reason.experiment_active": "Some selected experiments are active. Deselect them before deleting.", + "batch-delete.reason.feature_flag_enabled": "Some selected feature flags are enabled. Deselect them before deleting.", + "batch-delete.reason.segment_used": "Some selected segments are in use. Deselect them before deleting.", + "batch-delete.success.experiments.one": "1 experiment deleted.", + "batch-delete.success.experiments.other": "{{deleted}} experiments deleted.", + "batch-delete.success.flags.one": "1 feature flag deleted.", + "batch-delete.success.flags.other": "{{deleted}} feature flags deleted.", + "batch-delete.success.segments.one": "1 segment deleted.", + "batch-delete.success.segments.other": "{{deleted}} segments deleted.", + "batch-delete.result.absent.one": "1 item was already absent.", + "batch-delete.result.absent.other": "{{count}} items were already absent.", + "batch-delete.result.failed": "1 item could not be deleted.", + "batch-delete.result.notAttempted.one": "1 item was not attempted.", + "batch-delete.result.notAttempted.other": "{{count}} items were not attempted.", + "batch-delete.result.post-delete-failed": "An error occurred after deletion.", + "batch-delete.result.uncertain": "Deletion could not be confirmed for some items.", + "global.app.name": "UpGrade", "global.app.description": "Open Source A/B Testing Platform for Education Software", "global.experiment.title": "Experiments", diff --git a/packages/frontend/projects/upgrade/src/environments/environment-types.ts b/packages/frontend/projects/upgrade/src/environments/environment-types.ts index 75ab1ecf33..cde8e16b1b 100644 --- a/packages/frontend/projects/upgrade/src/environments/environment-types.ts +++ b/packages/frontend/projects/upgrade/src/environments/environment-types.ts @@ -3,6 +3,9 @@ import { InjectionToken } from '@angular/core'; export const ENV = new InjectionToken('env.token'); export interface APIEndpoints { + experimentsBatchDelete: string; + flagsBatchDelete: string; + segmentsBatchDelete: string; getAllExperiments: string; createNewExperiments: string; validateExperiment: string; diff --git a/packages/frontend/projects/upgrade/src/styles.scss b/packages/frontend/projects/upgrade/src/styles.scss index 940972c21b..0964b675f1 100755 --- a/packages/frontend/projects/upgrade/src/styles.scss +++ b/packages/frontend/projects/upgrade/src/styles.scss @@ -41,6 +41,8 @@ @include custom-components-theme($light-theme); } +@import './styles/shared-batch-selection.scss'; + .dense-1 { @include mat.all-component-densities(-1); } diff --git a/packages/frontend/projects/upgrade/src/styles/shared-batch-selection.scss b/packages/frontend/projects/upgrade/src/styles/shared-batch-selection.scss new file mode 100644 index 0000000000..da7a7953e3 --- /dev/null +++ b/packages/frontend/projects/upgrade/src/styles/shared-batch-selection.scss @@ -0,0 +1,4 @@ +// Keep the header aligned; hidden selections must still be available to clear. +.no-data:not(.has-batch-selection) .mat-mdc-header-row .batch-checkbox { + visibility: hidden; +} diff --git a/packages/frontend/projects/upgrade/src/styles/variables.scss b/packages/frontend/projects/upgrade/src/styles/variables.scss index 5950417625..75431a19f7 100644 --- a/packages/frontend/projects/upgrade/src/styles/variables.scss +++ b/packages/frontend/projects/upgrade/src/styles/variables.scss @@ -43,6 +43,7 @@ --grey-4: #959dac; --grey-5: #939bab; --grey-6: #6e6f72; + --grey-7: #adadad; --light-grey: #d3d3d3; --light-black: #222b45; --grey-message: #8f9bb3; diff --git a/packages/types/src/BatchActions/index.ts b/packages/types/src/BatchActions/index.ts new file mode 100644 index 0000000000..e7cdb725b6 --- /dev/null +++ b/packages/types/src/BatchActions/index.ts @@ -0,0 +1,25 @@ +export interface BatchEntityIdsRequest { + ids: string[]; +} + +export enum DeletionReasonCode { + NOT_FOUND = 'not_found', + DELETE_FAILED = 'delete_failed', + LOCK_TIMEOUT = 'lock_timeout', + OUTCOME_UNKNOWN = 'outcome_unknown', + POST_DELETE_FAILED = 'post_delete_failed', +} + +export type BatchDeleteEntity = 'experiments' | 'flags' | 'segments'; + +export type BatchDeleteItemOutcome = 'deleted' | 'not_found' | 'failed' | 'unknown' | 'not_attempted'; + +export interface BatchDeleteItemResult { + id: string; + outcome: BatchDeleteItemOutcome; + reasonCode?: DeletionReasonCode; +} + +export interface BatchDeleteResult { + results: BatchDeleteItemResult[]; +} diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index 9720688124..75d8ae6ca9 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -94,6 +94,14 @@ export { ExperimentQueryComparator, } from './Experiment/interfaces'; export { SYSTEM_USER_EMAIL, DEV_USER_EMAIL, FAKE_DEV_CREDENTIAL } from './User'; +export { + BatchEntityIdsRequest, + BatchDeleteEntity, + DeletionReasonCode, + BatchDeleteItemOutcome, + BatchDeleteItemResult, + BatchDeleteResult, +} from './BatchActions'; export { Prior, BinaryRewardAllowedValue,