Add project notification settings maintenance cleanup#2145
Merged
Conversation
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
Adds an admin maintenance action + work item/handler to clean up Project.NotificationSettings by removing orphaned user entries, and centralizes the cleanup logic in OrganizationService so it’s reused by org deletion and targeted user removal. Also expands integration test coverage across controller and handler paths.
Changes:
- Added
update-project-notification-settingsadmin maintenance action that enqueues a new work item. - Implemented centralized project notification settings cleanup in
OrganizationServiceand reused it fromOrganizationController.RemoveUserAsync. - Added integration tests covering orphan cleanup, org filtering, and paging behavior.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/Exceptionless.Tests/Jobs/WorkItemHandlers/UpdateProjectNotificationSettingsWorkItemHandlerTests.cs | Adds integration tests validating handler-driven cleanup (orphans, org filter, paging). |
| tests/Exceptionless.Tests/Controllers/OrganizationControllerTests.cs | Adds integration tests ensuring user removal cleans notification settings and preserves other entries. |
| tests/Exceptionless.Tests/Controllers/AdminControllerTests.cs | Adds integration tests ensuring the admin maintenance endpoint enqueues and results in cleanup. |
| src/Exceptionless.Web/Controllers/OrganizationController.cs | Replaces inline project notification settings cleanup with centralized service method. |
| src/Exceptionless.Web/Controllers/AdminController.cs | Adds the new maintenance action switch case to enqueue the work item. |
| src/Exceptionless.Core/Services/OrganizationService.cs | Adds paged cleanup implementation and integrates it into org user removal flow. |
| src/Exceptionless.Core/Models/WorkItems/UpdateProjectNotificationSettingsWorkItem.cs | Defines the new work item payload (optional org filter). |
| src/Exceptionless.Core/Jobs/WorkItemHandlers/UpdateProjectNotificationSettingsWorkItemHandler.cs | Implements the handler to run cleanup for one org or all orgs with progress reporting and locking. |
| src/Exceptionless.Core/Bootstrapper.cs | Registers the new work item handler mapping. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Adjusts the processing loop in the notification settings cleanup job to improve batch handling and timing. Also updates AI agent skill documentation to provide clearer usage instructions and normalize code formatting across the skill library.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 25 out of 25 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
ejsmith
added a commit
that referenced
this pull request
Mar 19, 2026
* Improves serialization and project config Addresses issues with serialization of various models, ensuring data integrity during round trips. Specifically: - Uses init accessors for SettingsDictionary to enable STJ to populate during deserialization. - Adds JToken handling in ObjectToInferredTypesConverter to correctly serialize JObject and JArray types. - Updates ExtendedEntityChanged properties to use setter injection instead of create method to support STJ deserialization via message bus. - Adds tests to validate serialization of core models. - Improves Project Configuration management. * Fixed linting errors * Optimize CloseInactiveSessionsJob: batch Redis calls (#2119) * Optimize the close inactive sessions job * Feedback * refactor: return tuples from GetHeartbeatsBatchAsync for cleaner foreach loop Address PR feedback: - Remove weird IReadOnlyList cast at call site - GetHeartbeatsBatchAsync now returns (PersistentEvent, HeartbeatResult?) tuples - Caller uses simple foreach instead of index-based for loop - Accept IReadOnlyCollection to avoid cast at call site * Filter messages that nobody is listening for * Fixes previous day/week/month range selection Updates the quick ranges in the date range picker to correctly select the previous day, week, and month. The previous implementation was selecting from the end of the previous period to the end of the current period. * Adds index to stack and event queries Improves stack and event query performance by adding the index to the query. This change ensures that date range queries on stacks and events leverage the index for faster retrieval. * Improves stack usage tracking and serialization Refactors stack usage tracking to use a record struct for cache keys, ensuring proper serialization and distinct keys for accurate accounting. #2124 #2113 Enables field inclusion in JsonSerializerOptions to correctly serialize tuples and records with their field names, instead of default Item1, Item2. This is crucial for the stack usage key to be properly serialized in Redis. Also fixes a date range filter on the stacks controller and adds a test case. Fixes #2124 Fixes #2113 * Optimizes daily summary job and email sending Improves the daily summary job by removing an unnecessary index call, streamlining stack retrieval. Enhances email sending robustness by preventing cancellation exceptions from being caught during health checks, ensuring more accurate error reporting. * Fixes stack stats with a work item job (#2129) * Fixes stack stats with a work item job Addresses an issue where stack statistics may become inaccurate due to a bug. Introduces a work item handler and job to recalculate stack stats based on event data within a specified time range. An admin endpoint has been added to queue the work item. Adds tests to ensure the stats are correctly repaired. * PR feedback * increased batch size * Improves stack event stats calculation Refactors the stack event stats calculation to use aggregations, resulting in significantly improved performance. Removes unnecessary data loading, and simplifies the stats computation logic. * Fixes stack stats job logic Ensures the stack stats job correctly processes and updates stack statistics by adjusting the UTC timestamp handling and fixing validation issues. Improves the accuracy of the fix stack stats job and updates related tests to reflect the changes and ensure they account for all edge cases. * Refactors stack stats fix job Refactors the stack stats fix job to improve performance and correctness. The job now processes stacks within a specific organization or all organizations with events in the time window. It also uses aggregations to calculate stack event stats more efficiently and avoid unnecessary stack updates. The previous implementation was inefficient and could lead to incorrect stack stats. * Renames `Organization` to `OrganizationId` Updates the `FixStackStatsWorkItem` model and related code to use `OrganizationId` instead of `Organization` for clarity and consistency with the rest of the codebase. This change ensures that the correct organization is targeted when fixing stack statistics. * Ensures stack change notifications are sent Refactors stack patching to ensure that notifications are always sent after a stack is updated. This addresses an issue where the stack usage job was not triggering notifications due to a conditional check. * Replaces DocumentNotFoundException and handles missing stacks. Replaces the custom DocumentNotFoundException with the one provided by Foundatio. Handles potential DocumentNotFoundException when patching a stack, preventing unexpected errors if a stack has been deleted. * Handles missing stacks during event counting Ensures that stack event counter updates succeed even if the stack document does not exist. This prevents issues in background jobs that process event counts asynchronously, where the stack may have been deleted between event processing and the update operation. * Removes unused Foundatio references Removes the `Foundatio.Repositories.Extensions` and other unused references. These references were no longer required after a refactoring. Cleaning up unused dependencies improves build times and reduces potential conflicts. * Updated agent skills * Updates dependencies Updates various NuGet packages, including .NET extensions to 10.0.4, Aspire to 13.1.2, and several testing and instrumentation libraries. * Updated next deps * Fixed skill names * fixed lint * Add project notification settings maintenance cleanup (#2145) * Add project notification settings maintenance cleanup * Fix organization cleanup review feedback * Revert cleanup notes from agents * Refine notification cleanup logic and update agent skills Adjusts the processing loop in the notification settings cleanup job to improve batch handling and timing. Also updates AI agent skill documentation to provide clearer usage instructions and normalize code formatting across the skill library. * Update agent skills documentation and consistency rules Fixes broken relative links across the skill library and updates backend testing snippets to use System.Text.Json-based serializers. Additionally, adds a strict consistency rule for frontend development requiring the use of shared formatter components. * Implement AI agent framework and skill evolution protocol Introduces specialized personas for engineering, triaging, and code review alongside a formal "observe-inspect-amend" cycle for self-improving agent skills. This establishes a structured, automated workflow for the development lifecycle, including security-first PR reviews and root-cause-driven triage. * Add context-aware default build task to VS Code Introduces a smart build task that detects the active file extension and automatically executes the appropriate build command (npm or dotnet). This streamlines the development workflow when switching between frontend and backend files. * Update dependencies and adopt new repository APIs Updates Foundatio and Microsoft library versions. This includes migrating from `AddPropertyRequiredForRemove` to `AddRequiredField` and optimizing cache invalidation logic by checking `PatchAsync` results. * Optimize models with nullable properties and migrate tests to System.Text.Json Removes default initializers for dictionary properties in core models to reduce unnecessary allocations and handles resulting nullability in the event pipeline. This also completes the migration of the test suite from Newtonsoft.Json to System.Text.Json, updating serialization logic and helpers. * Use Update for appsettings files in project configuration Switches from Include to Update for appsettings.yml files to avoid potential item duplication, as these files are typically already included by the .NET SDK's default globbing patterns. * Update stack action response codes to 200 OK Changes the response status code for marking stacks as fixed or snoozed from 202 Accepted to 200 OK to accurately reflect that the operations are completed. This also updates the corresponding XML documentation and OpenAPI specification. * Enhance engineer agent workflow for PR context and specify agent models Updates agent definitions to include preferred model assignments (Sonnet and Opus) and adds a new pre-planning step for the engineer agent to fetch PR comments and CI status via the GitHub CLI. This also restructures the review loop to be non-blocking, allowing the agent to address existing feedback while waiting for CI and automated reviews to complete. * refactor: Migrate from AutoMapper to Mapperly (#2093) * refactor: migrate from AutoMapper to Mapperly - Replace AutoMapper (runtime reflection) with Mapperly (compile-time source generation) - Add Riok.Mapperly 4.2.1 to Exceptionless.Web - Remove AutoMapper 14.0.0 from Exceptionless.Core Breaking changes: - Controllers now use abstract mapping methods instead of generic MapAsync<T> - Base controllers require derived classes to implement MapToModel, MapToViewModel, MapToViewModels Mapping structure: - Created dedicated mapper files per type in src/Exceptionless.Web/Mapping/ - OrganizationMapper: NewOrganization -> Organization, Organization -> ViewOrganization - ProjectMapper: NewProject -> Project, Project -> ViewProject - TokenMapper: NewToken -> Token, Token -> ViewToken - UserMapper: User -> ViewUser - WebHookMapper: NewWebHook -> WebHook - InvoiceMapper: Stripe.Invoice -> InvoiceGridModel - ApiMapper facade delegates to individual mappers Testing: - Added comprehensive unit tests for all mappers (29 tests) - Tests follow backend-testing skill patterns Benefits: - Compile-time type safety for mappings - Better performance (no runtime reflection) - Cleaner separation of concerns with per-type mappers refactor: improve Mapperly mapper safety and configuration - Add [MapperIgnoreTarget] for computed/populated-later properties on OrganizationMapper (IsOverRequestLimit, IsThrottled, ProjectCount, StackCount, EventCount) and ProjectMapper (HasPremiumFeatures, OrganizationName, StackCount, EventCount) - Upgrade UserMapper to RequiredMappingStrategy.Target since it only maps Model→ViewModel; new ViewUser properties will produce compile warnings unless explicitly mapped or ignored - Add [MapperIgnoreTarget] for ViewUser.IsInvite (manually constructed) - Let Mapperly generate collection methods (MapToViewTokens, MapToViewUsers) natively instead of manual .Select().ToList() - Add SuspensionCode enum→string mapping tests - Fix StackService ValueTuple serialization bug (restore StackUsageKey) - Update Mapperly 4.2.1 → 4.3.1 * PR feedback * fix: align serialization input with localhost domain and update stale AutoMapper comments The event-serialization-input.json still used test.example.com while the response baseline was updated to test.localhost, causing CI failure. Also updated stale AutoMapper references in test comments to reflect Mapperly migration. * Apply suggestion from @niemyjski * fix: deep-copy Roles and OrganizationIds in UserMapper Mapperly does shallow reference assignments for collection properties, which means mutating ViewUser.Roles (e.g. removing GlobalAdmin in UserController) would also mutate the source User model. Fixed by using a core mapper plus manual deep-copy via HashSet constructor. Added regression test. * refactor: clarify TokenMapper Type test name and comment NewToken has no Type property, so the test was really just asserting the C# enum default (Authentication=0). Rename the test and update the comment to explain: NewToken lacks Type, [MapperIgnoreTarget] makes the intent explicit, and the controller sets Type=Access in AddModelAsync. * Next: System Admin Page (#2151) * System Admin Page * fix: address PR review feedback - Fix typo: 'Highest completed versioned' → 'Highest completed version' - Fix Migrations sidebar icon: Play → DatabaseZap to match routes.svelte.ts - Fix nullable dereference warnings in AdminControllerTests: split Terms()?.Buckets into separate assertions - Fix generic catch clause in AdminController: catch Exception, log error before returning Problem * pr feedback * fix: replace all underscores in snapshot status Update the snapshot status cell to use `replaceAll` instead of `replace`, ensuring that status values with multiple underscores are correctly formatted with spaces in the UI. * PR Feedback * PR Feedback * PR feedback * PR Feedback * Update agent descriptions with usage guidelines and trigger phrases Enhance agent metadata in both the individual agent definitions and AGENTS.md to include specific "Use when" scenarios and keyword triggers. This helps improve agent selection and intent matching for tasks like feature implementation, PR reviews, and issue triage. * Bump Scalar.AspNetCore from 2.13.8 to 2.13.9 (#2158) * Update deps * Initial plan --------- Co-authored-by: Blake Niemyjski <bniemyjski@gmail.com> Co-authored-by: Eric J. Smith <eric@codesmithtools.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
update-project-notification-settingsadmin maintenance action and corresponding work item/handlerOrganizationServicefor org deletion and targeted user removalTesting
env DOTNET_CLI_HOME=/tmp/dotnet-home DOTNET_SKIP_FIRST_TIME_EXPERIENCE=1 dotnet test --project tests/Exceptionless.Tests/Exceptionless.Tests.csproj --no-restore -- --filter-class Exceptionless.Tests.Controllers.AdminControllerTests --filter-class Exceptionless.Tests.Controllers.OrganizationControllerTests --filter-class Exceptionless.Tests.Jobs.WorkItemHandlers.UpdateProjectNotificationSettingsWorkItemHandlerTests --max-threads 1env DOTNET_CLI_HOME=/tmp/dotnet-home DOTNET_SKIP_FIRST_TIME_EXPERIENCE=1 dotnet test --project tests/Exceptionless.Tests/Exceptionless.Tests.csproj --no-restore(still running locally at PR creation time; no failures emitted yet beyond existingNU1902warnings forMimeKit 4.15.0)