Skip to content

feat: Add personal rate notifications - #2271

Open
niemyjski wants to merge 3 commits into
mainfrom
niemyjski/add-personal-rate-notifications-spec
Open

niemyjski wants to merge 3 commits into
mainfrom
niemyjski/add-personal-rate-notifications-spec

Conversation

@niemyjski

@niemyjski niemyjski commented May 31, 2026 •

Copy link
Copy Markdown
Member

Add personal email alerts when a project or stack reaches a configured event count within a time window. Account notification settings let users create, edit, disable, snooze, resume, and delete rules, helping them catch sustained bursts that occurrence notifications can miss.

Rules share minute counters, then scheduled evaluation applies thresholds and cooldowns. Delivery rechecks rule version, membership, email preferences, and the shared project notification budget. Distributed locking prevents stale cache fills from overwriting rule invalidation; the UI uses one combined capability gate. This update merges current main without changing the feature behavior.

Validation: Release build (0 warnings/errors), 70 focused backend tests, 815 frontend tests, frontend validation/build, PR-file C# formatting, and Helm lint/render passed. Hosted checks on 6a491ead1 passed: API, client, browser E2E, Docker build, and CLA. Local dogfooding passed on the scoped Aspire stack: 70 service-backed tests, rule-management and mobile browser flows, and a synthetic error delivered a rate email to a verified disposable user through Mailpit after the settled-minute evaluation. The broad formatting check reports pre-existing issues in 12 files unchanged from main.

API behavior and verification details
  • Add CRUD and snooze/resume endpoints under /api/v2/users/{userId}/projects/{projectId}/rate-notifications and has_rate_notifications on project responses.
  • Evaluation and the Svelte UI require premium access plus the rate-notifications organization feature. Stored rules survive downgrade or feature removal; user removal and project/organization deletion clean up owned rules.
  • The evaluator allows a full minute for in-flight counter writes to settle, adding one minute of alert latency. Emails use the typed Razor renderer.
  • Regression coverage includes cross-replica cache invalidation, minute-boundary writes, counter behavior, serialization, and email rendering. The local rule-management browser scenario covered create, edit, snooze, resume, disable, and delete. An unverified test user was correctly skipped by delivery.
  • The adjacent login-to-notification-settings test failed once while a newly created project was absent from the project list, then passed on one retry. This was outside the rate-rule flow and is retained as a test reliability caveat.
  • Latest main integrated: deffcf18550f38813823e0ed9be4e83d04ad3f66.
  • No breaking changes; existing routes and payloads remain compatible.

@niemyjski

Copy link
Copy Markdown
Member Author

@github-copilot review

Comment thread src/Exceptionless.Core/Pipeline/075_UpdateRateCountersAction.cs Fixed
Comment thread src/Exceptionless.Core/Pipeline/075_UpdateRateCountersAction.cs Fixed
Comment thread src/Exceptionless.Core/Pipeline/075_UpdateRateCountersAction.cs Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@niemyjski niemyjski changed the title feat(spec): Add Personal Rate Notifications feat: Add personal rate notifications Jul 10, 2026
@niemyjski

Copy link
Copy Markdown
Member Author

Follow-up audit complete

Audited live head 862ad4ffe0f7406e541c36d944d08d39a80c5d13 against origin/main dc940dd15c8d222d9764080622fdf4583d4546b8, every GitHub feedback surface, and the complete effective diff using the thermo-nuclear code-quality review.

Feedback classification

  • Three github-code-quality inline findings are resolved, outdated, already fixed, and superseded by the architecture pivot. The criticized rule-filtering loop no longer exists; RateNotificationCounterPlan.GetCounterKeys(...) now compiles signal/stack eligibility centrally and ingestion iterates only matching counter keys.
  • Copilot's submitted review contains no finding; the reviewer reported an internal error. It is non-actionable.
  • No human reviewer submitted a finding. The top-level Copilot review request is administrative, not feedback.
  • The current coverage comment is informational and green: 77% line / 66% branch.
  • Live result: 0 unresolved review threads and 0 unresolved actionable findings.

Thermo-nuclear finding and RCA

The evaluator producer and delivery consumer independently formatted the queued SubjectKey. That duplicated a protocol invariant across process boundaries; a future one-sided edit could enqueue valid notifications that delivery silently rejected.

Commit 862ad4ffe centralizes project/stack subject-key construction in the existing rate-notification counter plan and uses it from both evaluator and delivery. Focused regression coverage proves both subject scopes. No other actionable correctness, authorization, serialization, concurrency, non-atomic update, file-size, or unnecessary orchestration issue remained after the complete diff audit.

Product/UI dogfood

Dogfooded the exact PR runtime on desktop (1280x720) and mobile (390x844): empty state, create, edit threshold, snooze, resume, disable, and delete all succeeded. The UI states the cost-saving purpose, provides a useful 10 errors / 5 minutes / 30-minute cooldown starting point, and keeps actions usable at both viewports. The committed Playwright scenario independently passed.

Verification

  • dotnet test ... --filter-class Exceptionless.Tests.Pipeline.UpdateRateCountersActionTests — 11 passed
  • dotnet test ... --filter-class Exceptionless.Tests.Jobs.RateNotificationEvaluatorJobTests — 10 passed
  • dotnet test ... --filter-class Exceptionless.Tests.Jobs.RateNotificationsJobTests — 19 passed
  • dotnet test Exceptionless.slnx --no-restore --no-build — 2,608 total; 2,606 passed; 2 intentional skips; 0 failed
  • dotnet build Exceptionless.slnx --no-restore — succeeded; 0 warnings; 0 errors
  • focused dotnet format --verify-no-changes — passed
  • rate-notification Vitest files — 2 files / 12 tests passed
  • local Playwright rate-notification E2E — 1 passed
  • git diff --check — passed
  • GitHub required checks on 862ad4ffe — version, test-client, test-e2e, test-api/coverage, docker-build, and CLA all passed

No external blocker remains.

@niemyjski

Copy link
Copy Markdown
Member Author

Final follow-up on head 1a9cbe377e3bb7ff70f20eb7b12578f545e13905:

  • Re-audited all review threads, reviews, and conversation: the three inline findings remain resolved/outdated and superseded by RateNotificationCounterPlan; no new actionable feedback appeared.
  • Commit 50d919437 fixes the failing test-client check by restoring deterministic class/module sort order in e2e/fixtures/api-client.ts without behavior changes.
  • Merged current main normally in 1a9cbe377 to satisfy strict up-to-date branch protection; no rebase or force-push.
  • Post-push local validation: npm run lint, npm run check (0 errors/warnings), npm run build, and git diff --check all pass.
  • Final GitHub run 31624111842: version, test-client, test-api/coverage, test-e2e (Aspire + Playwright), docker-build, and CLA all passed; publish/deploy jobs skipped as expected for a PR.

Final state: open, current with main, mergeable, CLEAN, zero unresolved review threads.

Comment thread src/Exceptionless.Core/Services/RateNotificationRuleCache.cs Outdated
@niemyjski

niemyjski commented Aug 13, 2026 •

Copy link
Copy Markdown
Member Author

Thermo-nuclear follow-up on head 002497e:

Commit 002497e applies the surgical review fixes: removes the resurrected migration cron resume, aligns the canonical email source with singular/plural rendering, reuses the E2E polling helper, restores PUT stack-scope validation parity, centralizes supported-window policy across API/runtime, makes active-key discovery precede increments, memoizes duplicate bucket reads, and aligns generated nullable-enum schemas with the API contract.

Static evidence: git diff --check passed; OpenAPI and endpoint-manifest JSON parse; source/compiled template placeholders and generated TypeScript/Zod nullability agree. The contract generator could not run because this isolated worktree has no installed swagger-typescript-api. Per the explicit review constraint, no local tests, builds, Aspire, E2E, browser, runtime services, or dogfood were run, so these fixes are not locally test-verified. Two deeper non-surgical findings remain as inline review threads.

@niemyjski
niemyjski force-pushed the niemyjski/add-personal-rate-notifications-spec branch from 58e3c30 to ae336a0 Compare September 16, 2026 00:40
@github-actions

Copy link
Copy Markdown

Code Coverage

Package Line Rate Branch Rate Complexity Health
Exceptionless.Insulation 37% 35% 286 ❌
Exceptionless.Web 85% 70% 8281 ✔
Exceptionless.Core 77% 69% 10900 ✔
Exceptionless.AppHost 38% 41% 147 ❌
Summary 79% (27240 / 34301) 68% (12691 / 18543) 19614 ✔

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants