Service ownership phase 3: Ticket/Budget/Campaign triangle (#472)#224
Service ownership phase 3: Ticket/Budget/Campaign triangle (#472)#224peterdrier merged 2 commits intomainfrom
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9f7cb911b4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| await _dbContext.SaveChangesAsync(ct); | ||
|
|
||
| // Enqueue campaign emails via the Email service (owner of email_outbox_messages). | ||
| foreach (var user in eligibleUsers) | ||
| { |
There was a problem hiding this comment.
Persisting all CampaignGrant rows (already marked LatestEmailStatus = Queued) before the email loop makes partial failures unrecoverable: if SendCampaignCodeAsync throws mid-loop, some recipients end up with a saved grant but no outbox message. Those users are then excluded by the next SendWaveAsync run via alreadyGrantedSet, and their grants are not in Failed state, so RetryAllFailedAsync also skips them.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed — fixed in 845e904.
Changed SendWaveAsync to per-iteration save-then-enqueue: each CampaignGrant is persisted individually, then the email is enqueued, and if SendCampaignCodeAsync throws, a local catch flips that single grant to Failed in a follow-up save before the loop continues. A single bad enqueue now only affects one grant (which flows back through RetryAllFailedAsync normally), instead of orphaning the tail of the batch with no outbox row and a stuck Queued status.
Pre-nobodies-collective#472 this was implicitly safe because the grant write and the outbox write happened in the same DbContext.SaveChangesAsync — splitting ownership surfaced the bug. The per-iteration pattern is the minimum change that restores the atomicity guarantee without re-introducing a cross-service transaction.
| grant.LatestEmailStatus = EmailOutboxStatus.Queued; | ||
| grant.LatestEmailAt = now; | ||
|
|
||
| _metrics.RecordEmailQueued("campaign_code"); | ||
| } | ||
|
|
||
| await _dbContext.SaveChangesAsync(ct); |
There was a problem hiding this comment.
RetryAllFailedAsync flips all failed grants to Queued and commits that state before sending any retry emails. If one SendCampaignCodeAsync call fails, remaining grants have already left the Failed set without a corresponding outbox row, so subsequent retry runs (which filter on LatestEmailStatus == Failed) will never pick them up.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed — fixed in 845e904, same approach as SendWaveAsync.
RetryAllFailedAsync now flips each grant from Failed to Queued and enqueues it individually. If SendCampaignCodeAsync throws on a given grant, a local catch flips it straight back to Failed in a follow-up save, so it remains visible to the next retry pass instead of silently leaving the Failed set without a corresponding outbox row. A single bad retry can no longer orphan the tail of the batch.
…collective#472) Enforce table ownership across the Ticket, Budget, and Campaign sections: cross-service writes that previously bypassed owning services now go through service interfaces. 3a. TicketSyncService no longer writes CampaignGrants directly. New ICampaignService.MarkGrantsRedeemedAsync accepts a list of DiscountCodeRedemption records; TicketSyncService aggregates ticket orders with discount codes and delegates the RedeemedAt updates to CampaignService, which owns the grant table. 3b. TicketingBudgetService no longer writes BudgetLineItems or TicketingProjections directly. The weekly line-item upsert, projection-from-actuals update, and projected-line materialization logic moved into BudgetService as SyncTicketingActualsAsync, RefreshTicketingProjectionsAsync, GetTicketingProjectionEntriesAsync, and GetActualTicketsSold. TicketingBudgetService is now a thin orchestrator: it reads TicketOrders (a ticket-section table it co-owns), aggregates them per ISO week into TicketingWeeklyActuals, and delegates the budget mutations to BudgetService. 3c. CampaignService no longer creates EmailOutboxMessages rows directly. New IEmailService.SendCampaignCodeAsync takes a CampaignCodeEmailRequest describing the campaign's raw template (subject + markdown body + {{Code}}/{{Name}} placeholders). OutboxEmailService renders and wraps the email; SmtpEmailService sends inline. CampaignService now persists grants first, then delegates email enqueuing per grant. 3d. ProfileService no longer queries ticket tables. New methods on ITicketQueryService (HasCurrentEventTicketAsync, GetUserTicketExportDataAsync) replace ProfileService's direct reads of TicketOrders / TicketAttendees / TicketSyncStates used by the event-hold date computation and GDPR data export. Ticket caches (UserTicketCount, UserIdsWithTickets, TicketEventSummary, TicketDashboardStats) are verified to be accessed only by ticket services and the central MemoryCacheExtensions helpers. Tests: 947 Application + 108 Domain tests pass. Existing campaign/ticket- sync tests were updated to verify delegation via NSubstitute rather than asserting on the outbox/grant tables directly. Integration tests require Docker/Testcontainers which is unavailable in this environment. Closes nobodies-collective#472 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
peterdrier
force-pushed
the
sprint/2026-04-13/batch-4
branch
from
9f7cb91 to
cfda8f9
Compare
… review) Codex flagged two P1 atomicity bugs introduced by the service-boundary split in nobodies-collective#472: both SendWaveAsync and RetryAllFailedAsync saved the CampaignGrant rows as Queued in one transaction and then looped over SendCampaignCodeAsync to enqueue outbox messages. A throw mid-loop left the tail of grants in a lost state: - Persisted as Queued - No outbox row, so the email never goes out - Not Failed, so RetryAllFailedAsync skips them - Already granted, so SendWaveAsync's alreadyGrantedSet skips them on the next wave = silent data loss. Before nobodies-collective#472 these writes happened in a single DbContext save to the same service boundary, so the atomicity question didn't arise. Move to per-iteration save-then-enqueue with a local catch: each grant is saved, the email is enqueued, and if the enqueue throws the grant is flipped to Failed in a follow-up save. The loop continues so a single bad email can't orphan the rest of the batch, and the Failed grants flow back through RetryAllFailedAsync normally. Same treatment for RetryAllFailedAsync: per-grant flip-and-enqueue so a retry failure leaves only that one grant in Failed state.
…obodies-collective#472) (#224)" This reverts commit 4682a2d.
1 participant
Summary
Phase 3 of the service data ownership plan. Closes cross-section write violations between Tickets, Budget, and Campaigns.
TicketSyncServiceno longer writesCampaignGrantsdirectly. NewICampaignService.MarkGrantsRedeemedAsync(IReadOnlyCollection<DiscountCodeRedemption>, ct)accepts aggregated discount-code redemptions from ticket sync and applies them inside CampaignService.TicketingBudgetServiceno longer writesBudgetLineItemsorTicketingProjections. The weekly line-item upsert, projection-from-actuals update, and projected-line materialization moved intoBudgetService(SyncTicketingActualsAsync,RefreshTicketingProjectionsAsync,GetTicketingProjectionEntriesAsync,GetActualTicketsSold).TicketingBudgetServiceis now a thin orchestrator: reads ticket orders (a table it co-owns), aggregates per ISO week intoTicketingWeeklyActualsDTOs, and delegates budget mutations toBudgetService.CampaignServiceno longer createsEmailOutboxMessagesrows directly. NewIEmailService.SendCampaignCodeAsync(CampaignCodeEmailRequest, ct)onOutboxEmailServicehandles markdown rendering, unsubscribe headers, and outbox insert.CampaignServicepersists grants first, then delegates per-grant email enqueue.SmtpEmailServiceandStubEmailServicealso implement the new method for dev/inline paths.ProfileServiceno longer queries ticket tables. NewITicketQueryService.HasCurrentEventTicketAsync(userId, ct)replaces the hold-date computation's direct ticket reads;GetUserTicketExportDataAsync(userId, ct)replaces the GDPR export's ticket reads.Ticket caches (
UserTicketCount,UserIdsWithTickets,TicketEventSummary,TicketDashboardStats) are only accessed from ticket services (TicketQueryService,TicketSyncService,TicketTailorService) and the centralMemoryCacheExtensionshelpers.Acceptance Criteria (from nobodies-collective#472)
TicketSyncServicedoes not writeCampaignGrantsdirectly — grep and reforge confirm.TicketingBudgetServicedoes not writeBudgetLineItemsorTicketingProjectionsdirectly —reforge ownership-violationsreturns 0 on these tables.CampaignServicedoes not writeEmailOutboxMessagesdirectly — only the owningOutboxEmailServicetouches the table.ProfileServicedoes not query ticket tables —ProfileService.csno longer references_dbContext.TicketOrders / TicketAttendees / TicketSyncStates.src/.Test Plan
dotnet build Humans.slnx -v q— 0 errors, 0 warnings.dotnet test Humans.slnx— 947 Application tests pass, 108 Domain tests pass. Integration tests require Docker (Testcontainers/PostgreSQL) which is unavailable in this worker environment; pre-existing, unrelated to these changes.dotnet format Humans.slnx --verify-no-changes— clean.reforge ownership-violationsrerun against the four target table groups — 0 in-scope violations remain.RedeemedAtset), refresh ticketing projections, open the profile data export endpoint, verify the guest dashboard shows the hold date.Files Touched
src/Humans.Application/DTOs/TicketingBudgetDtos.cs— newTicketingWeeklyActualsrecordsrc/Humans.Application/Interfaces/IBudgetService.cs— +4 ticketing sync methodssrc/Humans.Application/Interfaces/ICampaignService.cs— +MarkGrantsRedeemedAsync,DiscountCodeRedemptionsrc/Humans.Application/Interfaces/IEmailService.cs— +SendCampaignCodeAsync,CampaignCodeEmailRequestsrc/Humans.Application/Interfaces/ITicketQueryService.cs— +HasCurrentEventTicketAsync,GetUserTicketExportDataAsync, export DTOssrc/Humans.Infrastructure/Services/BudgetService.cs— owns all ticketing-sync mutation logicsrc/Humans.Infrastructure/Services/CampaignService.cs— delegates email enqueue + grant-redeemsrc/Humans.Infrastructure/Services/OutboxEmailService.cs— +campaign-code enqueuesrc/Humans.Infrastructure/Services/SmtpEmailService.cs— +campaign-code inline sendsrc/Humans.Infrastructure/Services/StubEmailService.cs— +campaign-code stubsrc/Humans.Infrastructure/Services/ProfileService.cs— hold-date + export useITicketQueryServicesrc/Humans.Infrastructure/Services/TicketQueryService.cs— +hold-status + export readssrc/Humans.Infrastructure/Services/TicketSyncService.cs— delegates code matching to campaign servicesrc/Humans.Infrastructure/Services/TicketingBudgetService.cs— thin orchestratortests/Humans.Application.Tests/Services/CampaignServiceTests.cs— assertions viaIEmailServicemocktests/Humans.Application.Tests/Services/ProfileServiceTests.cs— newITicketQueryServicesubstitutetests/Humans.Application.Tests/Services/TicketSyncServiceTests.cs— verifies campaign-service delegationExecuted by
/execute-sprintas batch 4 of sprint 2026-04-13.🤖 Generated with Claude Code