[PM-33527] Implement Organization Delete Tasks with Events

[Banrion]

🎟️ Tracking

PM-33527

Supersedes #7783 and #7910, opened by a teammate who has since moved teams. This branch carries both change sets rebased onto current main, with all reviewer feedback from both applied for cloud and self-host environments.

📔 Objective

Deleting an organization removes its database rows but leaves its event logs behind in storage, so personal data survives the deletion indefinitely. Product identified this as one of three storage-retention gaps for a departing Tier 1 customer (attachments, Sends, event logs) that were previously cleaned up by hand.

The purge cannot run inline with the delete — an organization can have millions of events, and the work has to survive process restarts. So this adds a durable queue: Organization_DeleteById writes one OrganizationDeleteTask row per cleanup type in the same transaction as the delete, meaning the obligation can never be lost if the deletion commits. An Admin background job then claims tasks under a lease and purges in bounded, resumable batches.

Dispatch is by task type, so another team can add a cleanup (attachments, Sends) by adding an enum value and registering a handler — no job changes.

Runs on every deployment

IEventRepository resolves to Azure Table Storage on cloud and to the SQL-backed Dapper/EF implementations on self-host, so all three purge paths are live:

Deployment	Events live in	Purge path
Cloud	Azure Table Storage	TableStorage.EventRepository
Self-host, MSSQL	the Event database table	Event_DeleteManyByOrganizationId via Dapper
Self-host, MySQL/Postgres/SQLite	the Event table, via EF	bounded ExecuteDelete via EF

Product confirmed self-host should get the same cleanup. Nothing purges those rows today — there is no cascade from Organization to Event and no retention job — so self-host organizations orphan their events on delete just as cloud does.

Dual-ORM coverage

Layer	Both tracks	Notes
Schema	✅	4 MSSQL scripts + EF migrations for MySQL/Postgres/SQLite
Enqueue	✅	Dapper OPENJSON, EF AddRangeAsync
Claim / lease / progress	✅	Different strategies, same contract — see below

MSSQL claims in one statement with WITH (UPDLOCK, READPAST) + OUTPUT, which has no EF Core translation. EF uses optimistic concurrency instead: read the oldest claimable task, then ExecuteUpdate conditioned on it still being claimable, so a worker that loses the race updates zero rows and moves to the next candidate. Bounded at 3 attempts. All 8 queue tests plus the 3 purge tests run against SQL Server, Postgres, MySQL and SQLite.

Bounded per call

Every implementation deletes a bounded amount and returns the count; the job loops until it gets 0, refreshing the lease between calls. Run budget 4 minutes against a 10-minute lease, triggered every 5 minutes.

Path	Bound
MSSQL	@MaxRows = 50000 in TOP(1000) batches, commandTimeout 300s
EF Core	1000 rows per call
Table Storage	100 ops per transaction, ≤18,000 batches, 8 concurrent

A task is abandoned after MaxFailureCount (5) failures. That transition now logs at Error, because otherwise a cleanup that has permanently stopped retrying would be invisible — individual failures surface through the job's rethrow, but the stop does not.

Also included

• Fixes a transaction bug in the EF OrganizationRepository: SaveChangesAsync() ran after CommitAsync(), so the sponsorship nulling and organization removal were landing outside the transaction.
• Enqueues on all three organization-delete paths, not just the delete command: the Admin portal controller and the sole-owner organization deleted during account deletion (a right-to-erasure flow) previously orphaned events. The four signup-rollback call sites are deliberately exempt — they delete an organization created seconds earlier that has no events.
• Feature flagged behind pm-33527-organization-event-cleanup.

Notes for reviewers

• Organization_DeleteById takes a JSON parameter, not a table type, per the T-SQL guidance against new user-defined types. Its NOT NULL columns intentionally have no ISNULL fallbacks; the inline comment explains why the rolling-deployment hazard that guidance protects against cannot occur here, since the parameter, the JSON contract and the table all ship together.
• OrganizationDeleteTask rows are never deleted. Completed rows are kept deliberately, as evidence the GDPR cleanup ran. LastError stores an exception type and sanitized Azure status codes only — never ex.Message, which can embed row-key identifiers.
• IX_Event_OrganizationId is filtered on MSSQL and unfiltered on EF, matching the precedent set by IX_Event_OrganizationIdSendIdDate in [PM-37944] Differentiate Send events according to domain #7690: MySQL does not support filtered indexes.
• context.CancellationToken is inert in practice. InterruptJobsOnShutdown is never set and
  Quartz defaults it to false, so executing jobs are not signalled on shutdown. The cancellation handling here is defensive; an interrupted purge is recovered by lease expiry and reclaim.

Known gap

The Table Storage purge — cloud's actual path, and the one carrying the batching, concurrency throttle and error-drain logic — has no automated coverage; the harness that runs the SQL tests cannot reach it.

[github-actions]

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

Reviewed the full change set: the OrganizationDeleteTask durable queue (MSSQL procs + EF repository with optimistic-concurrency claim), the three purge paths (Table Storage, Dapper Event_DeleteManyByOrganizationId, EF bounded ExecuteDelete), the Quartz job, and the three organization-delete call sites now enqueuing cleanup. Verified dual-ORM parity across schema, enqueue, claim/lease, and progress; verified the Table Storage partition filter is complete (EventTableEntity.IndexEvent writes every index row for an org event under the same OrganizationId= partition, so no org event rows are left behind); and confirmed the four remaining DeleteAsync call sites are all signup rollbacks. Also re-checked the previously flagged items: the 500-batch (50,000-entity) bound on the Table Storage call now sits comfortably inside the 10-minute claim lease, and CombGuid.Generate() replaces the obsolete CoreHelpers wrapper.

Code Review Details

No blocking findings.

Verified during this pass:

• SaveChangesAsync() now precedes CommitAsync() in the EF OrganizationRepository delete, so the sponsorship nulling, organization removal, and task enqueue all land inside organizationDeleteTransaction.
• LastError and the job's log statements carry only exception type names and sanitized Azure status codes — no ex.Message, no PII.
• The Table Storage purge drains in-flight batch submissions under SuppressThrowing before rethrowing, so no batch releases a disposed SemaphoreSlim, and totalDeleted is only incremented after a transaction succeeds.
• Completion is gated on an empty batch rather than an empty loop, so a cancelled or budget-exhausted run cannot mark a task complete with events still present.
• IEventRepository resolves in Admin on every deployment shape (Table Storage on cloud, Dapper on self-host MSSQL, EF elsewhere), so the EventsCleanupOrganizationDeleteTaskHandler registration cannot fail to construct.
• The Organization_DeleteById migration body matches the SSDT definition statement-for-statement; the OPENJSON + DATETIME2(7) contract matches existing precedent in OrganizationUser_CreateMany.

One minor, non-blocking note: on MySQL/Postgres/SQLite the new unfiltered IX_Event_OrganizationId is a leftmost prefix of the existing unfiltered IX_Event_OrganizationIdSendIdDate, so it adds write amplification on the highest-volume table without a read benefit those providers don't already have (src/Infrastructure.EntityFramework/Dirt/Configurations/EventEntityTypeConfiguration.cs:36) — it is genuinely needed on MSSQL, where the composite index is filtered to SendId IS NOT NULL.

[codecov]

Codecov Report

❌ Patch coverage is 80.54795% with 71 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.08%. Comparing base (21eeb74) to head (e768529).
⚠️ Report is 5 commits behind head on main.

Files with missing lines	Patch %	Lines
.../Dirt/Repositories/TableStorage/EventRepository.cs	0.00%	46 Missing ⚠️
src/Admin/Jobs/JobsHostedService.cs	0.00%	9 Missing ⚠️
...ions/EventsCleanupOrganizationDeleteTaskHandler.cs	0.00%	6 Missing ⚠️
src/Admin/Jobs/OrganizationDeleteTasksJob.cs	94.28%	1 Missing and 3 partials ⚠️
...dminConsole/Controllers/OrganizationsController.cs	0.00%	2 Missing ⚠️
src/Core/Services/Implementations/UserService.cs	0.00%	2 Missing ⚠️
...t/Repositories/OrganizationDeleteTaskRepository.cs	97.43%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8182      +/-   ##
==========================================
+ Coverage   63.41%   68.08%   +4.66%     
==========================================
  Files        2339     2345       +6     
  Lines      101534   101885     +351     
  Branches     9179     9198      +19     
==========================================
+ Hits        64385    69365    +4980     
+ Misses      34935    30188    -4747     
- Partials     2214     2332     +118     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[claude]

⚠️ IMPORTANT: A single Table Storage call can outlive the 10-minute claim lease, letting the task be reclaimed and purged concurrently.

Details and fix

OrganizationDeleteTasksJob only refreshes the lease between handler calls — UpdateProgressAsync (which writes RevisionDate) runs after DeleteBatchAsync returns. So the per-call bound has to stay comfortably under LeaseDurationMinutes (10).

The SQL paths respect that: MSSQL is capped at @MaxRows = 50000 with a 300s commandTimeout, EF at 1,000 rows. This path allows 18_000 * 100 = 1,800,000 entities in one call — 36× the MSSQL bound, with no time bound at all.

For the multi-million-event organization that motivated this PR, that call plausibly exceeds 10 minutes (~2,250 sequential rounds at maxConcurrentBatches = 8, plus ~1,800 query page round-trips interleaved). Once it does:

1. The 5-minute trigger fires a fresh OrganizationDeleteTasksJob (the job has no [DisallowConcurrentExecution], so this happens even in a single Admin instance).
2. ClaimNextPendingAsync sees RevisionDate older than the stale threshold and reclaims the same task.
3. Both runs enumerate and delete the same partition. Whichever batch races loses gets a 404 from SubmitTransactionAsync → TableTransactionFailedException → UpdateErrorAsync.
4. Five such collisions hit MaxFailureCount and the task is abandoned — the GDPR purge stops permanently (logged at Error, but incomplete).

Suggested fix — bound this call to something comparable to the SQL paths, e.g.:

const int maxBatchesPerCall = 500; // 50,000 entities, matching Event_DeleteManyByOrganizationId's @MaxRows

The job already loops until an empty batch is returned, so a smaller bound only means more calls — and each one refreshes the lease. Adding [DisallowConcurrentExecution] to the job would also close the single-instance case, though not the multi-instance one.

This is also the one path with no automated coverage, as the PR notes, so the bound is worth setting conservatively.

[Banrion]

Addressed in 4722df2.

[lastbestdev]

LGTM