From 3094ff49bfd91a9f8c7327df060b517fbafa13b2 Mon Sep 17 00:00:00 2001 From: Blake Niemyjski Date: Tue, 10 Mar 2026 22:27:31 -0500 Subject: [PATCH 1/4] Add project notification settings maintenance cleanup --- src/Exceptionless.Core/Bootstrapper.cs | 1 + ...jectNotificationSettingsWorkItemHandler.cs | 103 ++++++ ...dateProjectNotificationSettingsWorkItem.cs | 6 + .../Services/OrganizationService.cs | 148 ++++++++- .../Controllers/AdminController.cs | 54 ++-- .../Controllers/OrganizationController.cs | 9 +- .../Controllers/AdminControllerTests.cs | 155 +++++++++ .../OrganizationControllerTests.cs | 124 +++++++ ...otificationSettingsWorkItemHandlerTests.cs | 305 ++++++++++++++++++ 9 files changed, 856 insertions(+), 49 deletions(-) create mode 100644 src/Exceptionless.Core/Jobs/WorkItemHandlers/UpdateProjectNotificationSettingsWorkItemHandler.cs create mode 100644 src/Exceptionless.Core/Models/WorkItems/UpdateProjectNotificationSettingsWorkItem.cs create mode 100644 tests/Exceptionless.Tests/Jobs/WorkItemHandlers/UpdateProjectNotificationSettingsWorkItemHandlerTests.cs diff --git a/src/Exceptionless.Core/Bootstrapper.cs b/src/Exceptionless.Core/Bootstrapper.cs index c2016d7f70..04290b3d4a 100644 --- a/src/Exceptionless.Core/Bootstrapper.cs +++ b/src/Exceptionless.Core/Bootstrapper.cs @@ -111,6 +111,7 @@ public static void RegisterServices(IServiceCollection services, AppOptions appO handlers.Register(s.GetRequiredService); handlers.Register(s.GetRequiredService); handlers.Register(s.GetRequiredService); + handlers.Register(s.GetRequiredService); handlers.Register(s.GetRequiredService); return handlers; }); diff --git a/src/Exceptionless.Core/Jobs/WorkItemHandlers/UpdateProjectNotificationSettingsWorkItemHandler.cs b/src/Exceptionless.Core/Jobs/WorkItemHandlers/UpdateProjectNotificationSettingsWorkItemHandler.cs new file mode 100644 index 0000000000..801a582e6e --- /dev/null +++ b/src/Exceptionless.Core/Jobs/WorkItemHandlers/UpdateProjectNotificationSettingsWorkItemHandler.cs @@ -0,0 +1,103 @@ +using Exceptionless.Core.Models.WorkItems; +using Exceptionless.Core.Repositories; +using Exceptionless.Core.Services; +using Foundatio.Jobs; +using Foundatio.Lock; +using Foundatio.Repositories; +using Microsoft.Extensions.Logging; + +namespace Exceptionless.Core.Jobs.WorkItemHandlers; + +public class UpdateProjectNotificationSettingsWorkItemHandler : WorkItemHandlerBase +{ + private const int BATCH_SIZE = 50; + + private readonly IOrganizationRepository _organizationRepository; + private readonly OrganizationService _organizationService; + private readonly ILockProvider _lockProvider; + private readonly TimeProvider _timeProvider; + + public UpdateProjectNotificationSettingsWorkItemHandler( + IOrganizationRepository organizationRepository, + OrganizationService organizationService, + ILockProvider lockProvider, + TimeProvider timeProvider, + ILoggerFactory loggerFactory) : base(loggerFactory) + { + _organizationRepository = organizationRepository; + _organizationService = organizationService; + _lockProvider = lockProvider; + _timeProvider = timeProvider; + } + + public override Task GetWorkItemLockAsync(object workItem, CancellationToken cancellationToken = new()) + { + return _lockProvider.AcquireAsync(nameof(UpdateProjectNotificationSettingsWorkItemHandler), TimeSpan.FromMinutes(15), cancellationToken); + } + + public override async Task HandleItemAsync(WorkItemContext context) + { + var workItem = context.GetData(); + Log.LogInformation("Received update project notification settings work item. Organization={Organization}", workItem.OrganizationId); + + long totalNotificationSettingsRemoved = 0; + long organizationsProcessed = 0; + + if (!String.IsNullOrEmpty(workItem.OrganizationId)) + { + await context.ReportProgressAsync(0, $"Starting project notification settings update for organization {workItem.OrganizationId}"); + + var organization = await _organizationRepository.GetByIdAsync(workItem.OrganizationId); + if (organization is null) + { + Log.LogWarning("Organization {Organization} not found", workItem.OrganizationId); + return; + } + + totalNotificationSettingsRemoved += await _organizationService.CleanupProjectNotificationSettingsAsync( + organization, + [], + context.CancellationToken, + context.RenewLockAsync); + organizationsProcessed++; + } + else + { + await context.ReportProgressAsync(0, "Starting project notification settings update for all organizations"); + + var results = await _organizationRepository.FindAsync( + q => q.Include(o => o.Id), + o => o.SearchAfterPaging().PageLimit(BATCH_SIZE)); + + long totalOrganizations = results.Total; + + while (results.Documents.Count > 0 && !context.CancellationToken.IsCancellationRequested) + { + foreach (var organization in results.Documents) + { + totalNotificationSettingsRemoved += await _organizationService.CleanupProjectNotificationSettingsAsync( + organization, + [], + context.CancellationToken, + context.RenewLockAsync); + organizationsProcessed++; + } + + int percentage = totalOrganizations > 0 + ? (int)Math.Min(99, organizationsProcessed * 100.0 / totalOrganizations) + : 99; + await context.ReportProgressAsync(percentage, $"Processed {organizationsProcessed}/{totalOrganizations} organizations, removed {totalNotificationSettingsRemoved} invalid notification settings"); + + await Task.Delay(TimeSpan.FromSeconds(2.5), _timeProvider); + + if (context.CancellationToken.IsCancellationRequested || !await results.NextPageAsync()) + break; + + if (results.Documents.Count > 0) + await context.RenewLockAsync(); + } + } + + Log.LogInformation("Project notification settings update complete. Organizations processed: {OrganizationsProcessed}, invalid notification settings removed: {RemovedNotificationSettings}", organizationsProcessed, totalNotificationSettingsRemoved); + } +} diff --git a/src/Exceptionless.Core/Models/WorkItems/UpdateProjectNotificationSettingsWorkItem.cs b/src/Exceptionless.Core/Models/WorkItems/UpdateProjectNotificationSettingsWorkItem.cs new file mode 100644 index 0000000000..b58b117694 --- /dev/null +++ b/src/Exceptionless.Core/Models/WorkItems/UpdateProjectNotificationSettingsWorkItem.cs @@ -0,0 +1,6 @@ +namespace Exceptionless.Core.Models.WorkItems; + +public record UpdateProjectNotificationSettingsWorkItem +{ + public string? OrganizationId { get; init; } +} diff --git a/src/Exceptionless.Core/Services/OrganizationService.cs b/src/Exceptionless.Core/Services/OrganizationService.cs index 206f5cccf2..e57e5a6bb1 100644 --- a/src/Exceptionless.Core/Services/OrganizationService.cs +++ b/src/Exceptionless.Core/Services/OrganizationService.cs @@ -1,6 +1,6 @@ +using Exceptionless.Core.Extensions; using Exceptionless.Core.Models; using Exceptionless.Core.Repositories; -using Foundatio.Caching; using Foundatio.Extensions.Hosting.Startup; using Foundatio.Repositories; using Foundatio.Repositories.Models; @@ -11,22 +11,23 @@ namespace Exceptionless.Core.Services; public class OrganizationService : IStartupAction { + private const int BATCH_SIZE = 50; private readonly IOrganizationRepository _organizationRepository; + private readonly IProjectRepository _projectRepository; private readonly ITokenRepository _tokenRepository; private readonly IUserRepository _userRepository; private readonly IWebHookRepository _webHookRepository; - private readonly ICacheClient _cache; private readonly AppOptions _appOptions; private readonly UsageService _usageService; private readonly ILogger _logger; - public OrganizationService(IOrganizationRepository organizationRepository, ITokenRepository tokenRepository, IUserRepository userRepository, IWebHookRepository webHookRepository, ICacheClient cache, AppOptions appOptions, UsageService usageService, ILoggerFactory loggerFactory) + public OrganizationService(IOrganizationRepository organizationRepository, IProjectRepository projectRepository, ITokenRepository tokenRepository, IUserRepository userRepository, IWebHookRepository webHookRepository, AppOptions appOptions, UsageService usageService, ILoggerFactory loggerFactory) { _organizationRepository = organizationRepository; + _projectRepository = projectRepository; _tokenRepository = tokenRepository; _userRepository = userRepository; _webHookRepository = webHookRepository; - _cache = cache; _appOptions = appOptions; _usageService = usageService; _logger = loggerFactory.CreateLogger(); @@ -72,36 +73,113 @@ public async Task CancelSubscriptionsAsync(Organization organization) public async Task RemoveUsersAsync(Organization organization, string? currentUserId) { - var users = await _userRepository.GetByOrganizationIdAsync(organization.Id, o => o.PageLimit(1000)); - foreach (var user in users.Documents) + long totalUsersAffected = 0; + var userResults = await _userRepository.GetByOrganizationIdAsync(organization.Id, o => o.SearchAfterPaging().PageLimit(BATCH_SIZE)); + + while (userResults.Documents.Count > 0) { - // delete the user if they are not associated to any other organizations and they are not the current user - if (user.OrganizationIds.All(oid => String.Equals(oid, organization.Id)) && !String.Equals(user.Id, currentUserId)) + var usersToDelete = new List(userResults.Documents.Count); + var usersToUpdate = new List(userResults.Documents.Count); + + foreach (var user in userResults.Documents) { - _logger.LogInformation("Removing user {User} as they do not belong to any other organizations", user.Id); - await _userRepository.RemoveAsync(user.Id); + // delete the user if they are not associated to any other organizations and they are not the current user + if (user.OrganizationIds.All(oid => String.Equals(oid, organization.Id)) && !String.Equals(user.Id, currentUserId)) + { + _logger.LogInformation("Removing user {User} as they do not belong to any other organizations", user.Id); + usersToDelete.Add(user); + } + else + { + _logger.LogInformation("Removing user {User} from organization: {OrganizationName} ({Organization})", user.Id, organization.Name, organization.Id); + user.OrganizationIds.Remove(organization.Id); + usersToUpdate.Add(user); + } } - else + + if (usersToDelete.Count > 0) + await _userRepository.RemoveAsync(usersToDelete); + + if (usersToUpdate.Count > 0) + await _userRepository.SaveAsync(usersToUpdate, o => o.Cache()); + + totalUsersAffected += usersToDelete.Count + usersToUpdate.Count; + + if (!await userResults.NextPageAsync()) + break; + } + + await CleanupProjectNotificationSettingsAsync(organization, []); + + return totalUsersAffected; + } + + public Task CleanupProjectNotificationSettingsAsync(Organization organization, IReadOnlyCollection userIdsToRemove, CancellationToken cancellationToken = default, Func? renewWorkItemLockAsync = null) + { + ArgumentNullException.ThrowIfNull(organization); + ArgumentNullException.ThrowIfNull(userIdsToRemove); + + return CleanupProjectNotificationSettingsAsync(organization.Id, userIdsToRemove, cancellationToken, renewWorkItemLockAsync); + } + + private async Task CleanupProjectNotificationSettingsAsync(string organizationId, IReadOnlyCollection userIdsToRemove, CancellationToken cancellationToken, Func? renewWorkItemLockAsync) + { + ArgumentException.ThrowIfNullOrEmpty(organizationId); + ArgumentNullException.ThrowIfNull(userIdsToRemove); + + using var _ = _logger.BeginScope(new ExceptionlessState().Organization(organizationId)); + + var userIdsToRemoveSet = userIdsToRemove.Count == 0 + ? new HashSet(StringComparer.Ordinal) + : new HashSet(userIdsToRemove, StringComparer.Ordinal); + + long removed = 0; + var projectResults = await _projectRepository.GetByOrganizationIdAsync(organizationId, o => o.SearchAfterPaging().PageLimit(BATCH_SIZE)); + while (projectResults.Documents.Count > 0 && !cancellationToken.IsCancellationRequested) + { + var candidateUserIds = projectResults.Documents + .SelectMany(project => project.NotificationSettings.Keys) + .Where(key => !IsNotificationIntegrationKey(key)) + .ToHashSet(StringComparer.Ordinal); + + var validUserIds = await GetValidNotificationUserIdsAsync(organizationId, candidateUserIds, cancellationToken); + var projectsToSave = new List(projectResults.Documents.Count); + + foreach (var project in projectResults.Documents) { - _logger.LogInformation("Removing user {User} from organization: {OrganizationName} ({OrganizationId})", user.Id, organization.Name, organization.Id); - user.OrganizationIds.Remove(organization.Id); + int removedFromProject = RemoveInvalidNotificationSettings(project, validUserIds, userIdsToRemoveSet); + if (removedFromProject <= 0) + continue; - await _userRepository.SaveAsync(user, o => o.Cache()); + removed += removedFromProject; + projectsToSave.Add(project); } + + if (projectsToSave.Count > 0) + await _projectRepository.SaveAsync(projectsToSave); + + if (renewWorkItemLockAsync is not null) + await renewWorkItemLockAsync(); + + if (!await projectResults.NextPageAsync()) + break; } - return users.Documents.Count; + if (removed > 0) + _logger.LogInformation("Removed {Count} invalid notification settings", removed); + + return removed; } public Task RemoveTokensAsync(Organization organization) { - _logger.LogInformation("Removing tokens for {OrganizationName} ({OrganizationId})", organization.Name, organization.Id); + _logger.LogInformation("Removing tokens for {OrganizationName} ({Organization})", organization.Name, organization.Id); return _tokenRepository.RemoveAllByOrganizationIdAsync(organization.Id); } public Task RemoveWebHooksAsync(Organization organization) { - _logger.LogInformation("Removing web hooks for {OrganizationName} ({OrganizationId})", organization.Name, organization.Id); + _logger.LogInformation("Removing web hooks for {OrganizationName} ({Organization})", organization.Name, organization.Id); return _webHookRepository.RemoveAllByOrganizationIdAsync(organization.Id); } @@ -118,4 +196,40 @@ public async Task SoftDeleteOrganizationAsync(Organization organization, string organization.IsDeleted = true; await _organizationRepository.SaveAsync(organization); } + + private async Task> GetValidNotificationUserIdsAsync(string organizationId, IReadOnlyCollection userIds, CancellationToken cancellationToken) + { + var validUserIds = new HashSet(StringComparer.Ordinal); + if (userIds.Count == 0) + return validUserIds; + + foreach (string[] batch in userIds.Chunk(BATCH_SIZE)) + { + cancellationToken.ThrowIfCancellationRequested(); + + var users = await _userRepository.GetByIdsAsync(batch); + foreach (var user in users) + { + if (user.OrganizationIds.Contains(organizationId)) + validUserIds.Add(user.Id); + } + } + + return validUserIds; + } + + private static int RemoveInvalidNotificationSettings(Project project, IReadOnlySet validUserIds, IReadOnlySet userIdsToRemove) + { + var keysToRemove = project.NotificationSettings.Keys + .Where(key => !IsNotificationIntegrationKey(key) && (userIdsToRemove.Contains(key) || !validUserIds.Contains(key))) + .ToList(); + + foreach (var key in keysToRemove) + project.NotificationSettings.Remove(key); + + return keysToRemove.Count; + } + + private static bool IsNotificationIntegrationKey(string key) => + String.Equals(key, Project.NotificationIntegrations.Slack, StringComparison.OrdinalIgnoreCase); } diff --git a/src/Exceptionless.Web/Controllers/AdminController.cs b/src/Exceptionless.Web/Controllers/AdminController.cs index dfdef0339a..5e54941854 100644 --- a/src/Exceptionless.Web/Controllers/AdminController.cs +++ b/src/Exceptionless.Web/Controllers/AdminController.cs @@ -163,45 +163,51 @@ public async Task RunJobAsync(string name, DateTime? utcStart = n switch (name.ToLowerInvariant()) { + case "fix-stack-stats": + var defaultUtcStart = new DateTime(2026, 2, 10, 0, 0, 0, DateTimeKind.Utc); + var effectiveUtcStart = utcStart ?? defaultUtcStart; + + if (utcEnd.HasValue && utcEnd.Value.IsBefore(effectiveUtcStart)) + { + ModelState.AddModelError(nameof(utcEnd), "utcEnd must be greater than or equal to utcStart."); + return ValidationProblem(ModelState); + } + + await _workItemQueue.EnqueueAsync(new FixStackStatsWorkItem + { + UtcStart = effectiveUtcStart, + UtcEnd = utcEnd, + OrganizationId = organizationId + }); + break; + case "increment-project-configuration-version": + await _workItemQueue.EnqueueAsync(new ProjectMaintenanceWorkItem { IncrementConfigurationVersion = true }); + break; case "indexes": if (!_appOptions.ElasticsearchOptions.DisableIndexConfiguration) await _configuration.ConfigureIndexesAsync(beginReindexingOutdated: false); break; - case "update-organization-plans": - await _workItemQueue.EnqueueAsync(new OrganizationMaintenanceWorkItem { UpgradePlans = true }); + case "normalize-user-email-address": + await _workItemQueue.EnqueueAsync(new UserMaintenanceWorkItem { Normalize = true }); break; case "remove-old-organization-usage": await _workItemQueue.EnqueueAsync(new OrganizationMaintenanceWorkItem { RemoveOldUsageStats = true }); break; - case "update-project-default-bot-lists": - await _workItemQueue.EnqueueAsync(new ProjectMaintenanceWorkItem { UpdateDefaultBotList = true, IncrementConfigurationVersion = true }); - break; - case "increment-project-configuration-version": - await _workItemQueue.EnqueueAsync(new ProjectMaintenanceWorkItem { IncrementConfigurationVersion = true }); - break; case "remove-old-project-usage": await _workItemQueue.EnqueueAsync(new ProjectMaintenanceWorkItem { RemoveOldUsageStats = true }); break; - case "normalize-user-email-address": - await _workItemQueue.EnqueueAsync(new UserMaintenanceWorkItem { Normalize = true }); - break; case "reset-verify-email-address-token-and-expiration": await _workItemQueue.EnqueueAsync(new UserMaintenanceWorkItem { ResetVerifyEmailAddressToken = true }); break; - case "fix-stack-stats": - var defaultUtcStart = new DateTime(2026, 2, 10, 0, 0, 0, DateTimeKind.Utc); - var effectiveUtcStart = utcStart ?? defaultUtcStart; - - if (utcEnd.HasValue && utcEnd.Value.IsBefore(effectiveUtcStart)) - { - ModelState.AddModelError(nameof(utcEnd), "utcEnd must be greater than or equal to utcStart."); - return ValidationProblem(ModelState); - } - - await _workItemQueue.EnqueueAsync(new FixStackStatsWorkItem + case "update-organization-plans": + await _workItemQueue.EnqueueAsync(new OrganizationMaintenanceWorkItem { UpgradePlans = true }); + break; + case "update-project-default-bot-lists": + await _workItemQueue.EnqueueAsync(new ProjectMaintenanceWorkItem { UpdateDefaultBotList = true, IncrementConfigurationVersion = true }); + break; + case "update-project-notification-settings": + await _workItemQueue.EnqueueAsync(new UpdateProjectNotificationSettingsWorkItem { - UtcStart = effectiveUtcStart, - UtcEnd = utcEnd, OrganizationId = organizationId }); break; diff --git a/src/Exceptionless.Web/Controllers/OrganizationController.cs b/src/Exceptionless.Web/Controllers/OrganizationController.cs index a8c8d48c7e..20d8b48e76 100644 --- a/src/Exceptionless.Web/Controllers/OrganizationController.cs +++ b/src/Exceptionless.Web/Controllers/OrganizationController.cs @@ -584,14 +584,7 @@ public async Task RemoveUserAsync(string id, string email) if ((await _userRepository.GetByOrganizationIdAsync(organization.Id)).Total == 1) return BadRequest("An organization must contain at least one user."); - var projects = (await _projectRepository.GetByOrganizationIdAsync(organization.Id)).Documents.Where(p => p.NotificationSettings.ContainsKey(user.Id)).ToList(); - if (projects.Count > 0) - { - foreach (var project in projects) - project.NotificationSettings.Remove(user.Id); - - await _projectRepository.SaveAsync(projects); - } + await _organizationService.CleanupProjectNotificationSettingsAsync(organization, [user.Id]); user.OrganizationIds.Remove(organization.Id); await _userRepository.SaveAsync(user, o => o.Cache()); diff --git a/tests/Exceptionless.Tests/Controllers/AdminControllerTests.cs b/tests/Exceptionless.Tests/Controllers/AdminControllerTests.cs index b03ae571ce..3c75abef39 100644 --- a/tests/Exceptionless.Tests/Controllers/AdminControllerTests.cs +++ b/tests/Exceptionless.Tests/Controllers/AdminControllerTests.cs @@ -6,6 +6,7 @@ using Foundatio.Jobs; using Foundatio.Queues; using Foundatio.Repositories; +using Foundatio.Repositories.Utility; using Xunit; namespace Exceptionless.Tests.Controllers; @@ -16,6 +17,8 @@ public class AdminControllerTests : IntegrationTestsBase private readonly IQueue _workItemQueue; private readonly IStackRepository _stackRepository; private readonly IEventRepository _eventRepository; + private readonly IProjectRepository _projectRepository; + private readonly IUserRepository _userRepository; private readonly StackData _stackData; private readonly EventData _eventData; @@ -25,6 +28,8 @@ public AdminControllerTests(ITestOutputHelper output, AppWebHostFactory factory) _workItemQueue = GetService>(); _stackRepository = GetService(); _eventRepository = GetService(); + _projectRepository = GetService(); + _userRepository = GetService(); _stackData = GetService(); _eventData = GetService(); } @@ -172,4 +177,154 @@ await _eventRepository.AddAsync( await RefreshDataAsync(); return stack; } + + [Fact] + public async Task RunJobAsync_WhenUpdateProjectNotificationSettingsWithOrphanedUser_ShouldRemoveOrphanedEntries() + { + // Arrange + var project = await _projectRepository.GetByIdAsync(TestConstants.ProjectId); + Assert.NotNull(project); + + string orphanedUserId = ObjectId.GenerateNewId().ToString(); + project.NotificationSettings[orphanedUserId] = new NotificationSettings { ReportNewErrors = true }; + await _projectRepository.SaveAsync(project, o => o.ImmediateConsistency()); + + int settingsCountBefore = project.NotificationSettings.Count; + + // Act + await SendRequestAsync(r => r + .AsGlobalAdminUser() + .AppendPaths("admin", "maintenance", "update-project-notification-settings") + .StatusCodeShouldBeOk()); + + await _workItemJob.RunUntilEmptyAsync(TestCancellationToken); + + // Assert + project = await _projectRepository.GetByIdAsync(TestConstants.ProjectId); + Assert.NotNull(project); + Assert.DoesNotContain(orphanedUserId, project.NotificationSettings.Keys); + Assert.Equal(settingsCountBefore - 1, project.NotificationSettings.Count); + } + + [Fact] + public async Task RunJobAsync_WhenUpdateProjectNotificationSettingsWithValidUser_ShouldPreserveSettings() + { + // Arrange + var project = await _projectRepository.GetByIdAsync(TestConstants.ProjectId); + Assert.NotNull(project); + + var globalAdmin = await _userRepository.GetByEmailAddressAsync(SampleDataService.TEST_USER_EMAIL); + Assert.NotNull(globalAdmin); + Assert.Contains(globalAdmin.Id, project.NotificationSettings.Keys); + + int settingsCountBefore = project.NotificationSettings.Count; + + // Act + await SendRequestAsync(r => r + .AsGlobalAdminUser() + .AppendPaths("admin", "maintenance", "update-project-notification-settings") + .StatusCodeShouldBeOk()); + + await _workItemJob.RunUntilEmptyAsync(TestCancellationToken); + + // Assert + project = await _projectRepository.GetByIdAsync(TestConstants.ProjectId); + Assert.NotNull(project); + Assert.Contains(globalAdmin.Id, project.NotificationSettings.Keys); + Assert.Equal(settingsCountBefore, project.NotificationSettings.Count); + } + + [Fact] + public async Task RunJobAsync_WhenUpdateProjectNotificationSettingsWithIntegration_ShouldPreserveIntegrationKey() + { + // Arrange + var project = await _projectRepository.GetByIdAsync(TestConstants.ProjectId); + Assert.NotNull(project); + + project.NotificationSettings[Project.NotificationIntegrations.Slack] = new NotificationSettings { ReportNewErrors = true }; + string orphanedUserId = ObjectId.GenerateNewId().ToString(); + project.NotificationSettings[orphanedUserId] = new NotificationSettings { ReportNewErrors = true }; + await _projectRepository.SaveAsync(project, o => o.ImmediateConsistency()); + + // Act + await SendRequestAsync(r => r + .AsGlobalAdminUser() + .AppendPaths("admin", "maintenance", "update-project-notification-settings") + .StatusCodeShouldBeOk()); + + await _workItemJob.RunUntilEmptyAsync(TestCancellationToken); + + // Assert + project = await _projectRepository.GetByIdAsync(TestConstants.ProjectId); + Assert.NotNull(project); + Assert.Contains(Project.NotificationIntegrations.Slack, project.NotificationSettings.Keys); + Assert.DoesNotContain(orphanedUserId, project.NotificationSettings.Keys); + } + + [Fact] + public async Task RunJobAsync_WhenUpdateProjectNotificationSettingsWithDeletedUser_ShouldRemoveOrphanedEntries() + { + // Arrange + var project = await _projectRepository.GetByIdAsync(TestConstants.ProjectId); + Assert.NotNull(project); + + string deletedUserId = ObjectId.GenerateNewId().ToString(); + project.NotificationSettings[deletedUserId] = new NotificationSettings { ReportNewErrors = true }; + await _projectRepository.SaveAsync(project, o => o.ImmediateConsistency()); + + Assert.Null(await _userRepository.GetByIdAsync(deletedUserId)); + + // Act + await SendRequestAsync(r => r + .AsGlobalAdminUser() + .AppendPaths("admin", "maintenance", "update-project-notification-settings") + .StatusCodeShouldBeOk()); + + await _workItemJob.RunUntilEmptyAsync(TestCancellationToken); + + // Assert + project = await _projectRepository.GetByIdAsync(TestConstants.ProjectId); + Assert.NotNull(project); + Assert.DoesNotContain(deletedUserId, project.NotificationSettings.Keys); + } + + [Fact] + public async Task RunJobAsync_WhenUpdateProjectNotificationSettingsWithOrgFilter_ShouldOnlyProcessTargetOrg() + { + // Arrange + var project = await _projectRepository.GetByIdAsync(TestConstants.ProjectId); + Assert.NotNull(project); + + string orphanedUserId = ObjectId.GenerateNewId().ToString(); + project.NotificationSettings[orphanedUserId] = new NotificationSettings { ReportNewErrors = true }; + await _projectRepository.SaveAsync(project, o => o.ImmediateConsistency()); + + // Act: run cleanup for a different org + await SendRequestAsync(r => r + .AsGlobalAdminUser() + .AppendPaths("admin", "maintenance", "update-project-notification-settings") + .QueryString("organizationId", TestConstants.OrganizationId2) + .StatusCodeShouldBeOk()); + + await _workItemJob.RunUntilEmptyAsync(TestCancellationToken); + + // Assert: orphaned user in the OTHER org should still be there + project = await _projectRepository.GetByIdAsync(TestConstants.ProjectId); + Assert.NotNull(project); + Assert.Contains(orphanedUserId, project.NotificationSettings.Keys); + + // Act: now run for the correct org + await SendRequestAsync(r => r + .AsGlobalAdminUser() + .AppendPaths("admin", "maintenance", "update-project-notification-settings") + .QueryString("organizationId", TestConstants.OrganizationId) + .StatusCodeShouldBeOk()); + + await _workItemJob.RunUntilEmptyAsync(TestCancellationToken); + + // Assert + project = await _projectRepository.GetByIdAsync(TestConstants.ProjectId); + Assert.NotNull(project); + Assert.DoesNotContain(orphanedUserId, project.NotificationSettings.Keys); + } } diff --git a/tests/Exceptionless.Tests/Controllers/OrganizationControllerTests.cs b/tests/Exceptionless.Tests/Controllers/OrganizationControllerTests.cs index f73422fcf7..d8d7379b68 100644 --- a/tests/Exceptionless.Tests/Controllers/OrganizationControllerTests.cs +++ b/tests/Exceptionless.Tests/Controllers/OrganizationControllerTests.cs @@ -1,7 +1,11 @@ +using Exceptionless.Core.Models; using Exceptionless.Core.Repositories; using Exceptionless.Core.Utility; using Exceptionless.Tests.Extensions; +using Exceptionless.Tests.Utility; using Exceptionless.Web.Models; +using Foundatio.Repositories; +using Foundatio.Repositories.Utility; using Xunit; namespace Exceptionless.Tests.Controllers; @@ -13,10 +17,14 @@ namespace Exceptionless.Tests.Controllers; public sealed class OrganizationControllerTests : IntegrationTestsBase { private readonly IOrganizationRepository _organizationRepository; + private readonly IProjectRepository _projectRepository; + private readonly IUserRepository _userRepository; public OrganizationControllerTests(ITestOutputHelper output, AppWebHostFactory factory) : base(output, factory) { _organizationRepository = GetService(); + _projectRepository = GetService(); + _userRepository = GetService(); } protected override async Task ResetDataAsync() @@ -241,4 +249,120 @@ await SendRequestAsync(r => r .StatusCodeShouldBeNotFound() ); } + + [Fact] + public async Task RemoveUserAsync_UserWithNotificationSettings_CleansUpNotificationSettings() + { + // Arrange + var organizationAdminUser = await _userRepository.GetByEmailAddressAsync(SampleDataService.TEST_ORG_USER_EMAIL); + Assert.NotNull(organizationAdminUser); + Assert.Contains(SampleDataService.TEST_ORG_ID, organizationAdminUser.OrganizationIds); + + var project = await _projectRepository.GetByIdAsync(SampleDataService.TEST_PROJECT_ID); + Assert.NotNull(project); + project.NotificationSettings[organizationAdminUser.Id] = new NotificationSettings + { + SendDailySummary = true, + ReportNewErrors = true, + ReportCriticalErrors = true + }; + await _projectRepository.SaveAsync(project, o => o.ImmediateConsistency()); + + project = await _projectRepository.GetByIdAsync(SampleDataService.TEST_PROJECT_ID); + Assert.NotNull(project); + Assert.True(project.NotificationSettings.ContainsKey(organizationAdminUser.Id)); + + // Act + await SendRequestAsync(r => r + .AsGlobalAdminUser() + .Delete() + .AppendPaths("organizations", SampleDataService.TEST_ORG_ID, "users", SampleDataService.TEST_ORG_USER_EMAIL) + .StatusCodeShouldBeOk() + ); + + await RefreshDataAsync(); + + // Assert + project = await _projectRepository.GetByIdAsync(SampleDataService.TEST_PROJECT_ID); + Assert.NotNull(project); + Assert.False(project.NotificationSettings.ContainsKey(organizationAdminUser.Id)); + + organizationAdminUser = await _userRepository.GetByIdAsync(organizationAdminUser.Id); + Assert.NotNull(organizationAdminUser); + Assert.DoesNotContain(SampleDataService.TEST_ORG_ID, organizationAdminUser.OrganizationIds); + } + + [Fact] + public async Task RemoveUserAsync_WithExistingOrphanedNotificationSettings_CleansTargetAndHistoricalOrphans() + { + // Arrange + var organizationAdminUser = await _userRepository.GetByEmailAddressAsync(SampleDataService.TEST_ORG_USER_EMAIL); + Assert.NotNull(organizationAdminUser); + + var project = await _projectRepository.GetByIdAsync(SampleDataService.TEST_PROJECT_ID); + Assert.NotNull(project); + string orphanedUserId = ObjectId.GenerateNewId().ToString(); + project.NotificationSettings[organizationAdminUser.Id] = new NotificationSettings + { + SendDailySummary = true, + ReportNewErrors = true, + ReportCriticalErrors = true + }; + project.NotificationSettings[orphanedUserId] = new NotificationSettings { ReportNewErrors = true }; + await _projectRepository.SaveAsync(project, o => o.ImmediateConsistency()); + + Assert.Null(await _userRepository.GetByIdAsync(orphanedUserId)); + + // Act + await SendRequestAsync(r => r + .AsGlobalAdminUser() + .Delete() + .AppendPaths("organizations", SampleDataService.TEST_ORG_ID, "users", SampleDataService.TEST_ORG_USER_EMAIL) + .StatusCodeShouldBeOk() + ); + + await RefreshDataAsync(); + + project = await _projectRepository.GetByIdAsync(SampleDataService.TEST_PROJECT_ID); + Assert.NotNull(project); + + // Assert + Assert.DoesNotContain(organizationAdminUser.Id, project.NotificationSettings.Keys); + Assert.DoesNotContain(orphanedUserId, project.NotificationSettings.Keys); + } + + [Fact] + public async Task RemoveUserAsync_UserWithNotificationSettings_PreservesOtherUsersAndIntegrations() + { + // Arrange + var organizationAdminUser = await _userRepository.GetByEmailAddressAsync(SampleDataService.TEST_ORG_USER_EMAIL); + Assert.NotNull(organizationAdminUser); + + var globalAdmin = await _userRepository.GetByEmailAddressAsync(SampleDataService.TEST_USER_EMAIL); + Assert.NotNull(globalAdmin); + + var project = await _projectRepository.GetByIdAsync(SampleDataService.TEST_PROJECT_ID); + Assert.NotNull(project); + project.NotificationSettings[organizationAdminUser.Id] = new NotificationSettings { ReportNewErrors = true }; + project.NotificationSettings[globalAdmin.Id] = new NotificationSettings { ReportCriticalErrors = true }; + project.NotificationSettings[Project.NotificationIntegrations.Slack] = new NotificationSettings { SendDailySummary = true }; + await _projectRepository.SaveAsync(project, o => o.ImmediateConsistency()); + + // Act + await SendRequestAsync(r => r + .AsGlobalAdminUser() + .Delete() + .AppendPaths("organizations", SampleDataService.TEST_ORG_ID, "users", SampleDataService.TEST_ORG_USER_EMAIL) + .StatusCodeShouldBeOk() + ); + + await RefreshDataAsync(); + + // Assert + project = await _projectRepository.GetByIdAsync(SampleDataService.TEST_PROJECT_ID); + Assert.NotNull(project); + Assert.DoesNotContain(organizationAdminUser.Id, project.NotificationSettings.Keys); + Assert.Contains(globalAdmin.Id, project.NotificationSettings.Keys); + Assert.Contains(Project.NotificationIntegrations.Slack, project.NotificationSettings.Keys); + } } diff --git a/tests/Exceptionless.Tests/Jobs/WorkItemHandlers/UpdateProjectNotificationSettingsWorkItemHandlerTests.cs b/tests/Exceptionless.Tests/Jobs/WorkItemHandlers/UpdateProjectNotificationSettingsWorkItemHandlerTests.cs new file mode 100644 index 0000000000..ee05bc4995 --- /dev/null +++ b/tests/Exceptionless.Tests/Jobs/WorkItemHandlers/UpdateProjectNotificationSettingsWorkItemHandlerTests.cs @@ -0,0 +1,305 @@ +using Exceptionless.Core.Extensions; +using Exceptionless.Core.Models; +using Exceptionless.Core.Models.WorkItems; +using Exceptionless.Core.Repositories; +using Exceptionless.Core.Utility; +using Exceptionless.Tests.Extensions; +using Exceptionless.Tests.Utility; +using Foundatio.Jobs; +using Foundatio.Queues; +using Foundatio.Repositories; +using Foundatio.Repositories.Utility; +using Xunit; + +namespace Exceptionless.Tests.Jobs.WorkItemHandlers; + +public class UpdateProjectNotificationSettingsWorkItemHandlerTests : IntegrationTestsBase +{ + private readonly WorkItemJob _workItemJob; + private readonly IQueue _workItemQueue; + private readonly IProjectRepository _projectRepository; + private readonly IUserRepository _userRepository; + private readonly UserData _userData; + private readonly ProjectData _projectData; + + public UpdateProjectNotificationSettingsWorkItemHandlerTests(ITestOutputHelper output, AppWebHostFactory factory) : base(output, factory) + { + _workItemJob = GetService(); + _workItemQueue = GetService>(); + _projectRepository = GetService(); + _userRepository = GetService(); + _userData = GetService(); + _projectData = GetService(); + } + + protected override async Task ResetDataAsync() + { + await base.ResetDataAsync(); + var service = GetService(); + await service.CreateDataAsync(); + } + + [Fact] + public async Task RunUntilEmptyAsync_WithOrphanedUserSettings_RemovesOrphanedEntries() + { + // Arrange + var project = await _projectRepository.GetByIdAsync(TestConstants.ProjectId); + Assert.NotNull(project); + + string orphanedUserId = ObjectId.GenerateNewId().ToString(); + project.NotificationSettings[orphanedUserId] = new NotificationSettings { ReportNewErrors = true }; + await _projectRepository.SaveAsync(project, o => o.ImmediateConsistency()); + + int settingsCountBefore = project.NotificationSettings.Count; + + // Act + await _workItemQueue.EnqueueAsync(new UpdateProjectNotificationSettingsWorkItem()); + await _workItemJob.RunUntilEmptyAsync(TestCancellationToken); + + // Assert + project = await _projectRepository.GetByIdAsync(TestConstants.ProjectId); + Assert.NotNull(project); + Assert.DoesNotContain(orphanedUserId, project.NotificationSettings.Keys); + Assert.Equal(settingsCountBefore - 1, project.NotificationSettings.Count); + } + + [Fact] + public async Task RunUntilEmptyAsync_WithValidOrgMemberSettings_PreservesSettings() + { + // Arrange + var globalAdmin = await _userRepository.GetByEmailAddressAsync(SampleDataService.TEST_USER_EMAIL); + Assert.NotNull(globalAdmin); + + var project = await _projectRepository.GetByIdAsync(TestConstants.ProjectId); + Assert.NotNull(project); + Assert.Contains(globalAdmin.Id, project.NotificationSettings.Keys); + + int settingsCountBefore = project.NotificationSettings.Count; + + // Act + await _workItemQueue.EnqueueAsync(new UpdateProjectNotificationSettingsWorkItem()); + await _workItemJob.RunUntilEmptyAsync(TestCancellationToken); + + // Assert + project = await _projectRepository.GetByIdAsync(TestConstants.ProjectId); + Assert.NotNull(project); + Assert.Contains(globalAdmin.Id, project.NotificationSettings.Keys); + Assert.Equal(settingsCountBefore, project.NotificationSettings.Count); + } + + [Fact] + public async Task RunUntilEmptyAsync_WithSlackIntegrationKey_PreservesIntegrationSettings() + { + // Arrange + var project = await _projectRepository.GetByIdAsync(TestConstants.ProjectId); + Assert.NotNull(project); + + project.NotificationSettings[Project.NotificationIntegrations.Slack] = new NotificationSettings { ReportNewErrors = true }; + string orphanedUserId = ObjectId.GenerateNewId().ToString(); + project.NotificationSettings[orphanedUserId] = new NotificationSettings { ReportNewErrors = true }; + await _projectRepository.SaveAsync(project, o => o.ImmediateConsistency()); + + // Act + await _workItemQueue.EnqueueAsync(new UpdateProjectNotificationSettingsWorkItem()); + await _workItemJob.RunUntilEmptyAsync(TestCancellationToken); + + // Assert + project = await _projectRepository.GetByIdAsync(TestConstants.ProjectId); + Assert.NotNull(project); + Assert.Contains(Project.NotificationIntegrations.Slack, project.NotificationSettings.Keys); + Assert.DoesNotContain(orphanedUserId, project.NotificationSettings.Keys); + } + + [Fact] + public async Task RunUntilEmptyAsync_WithDeletedUserAccount_RemovesOrphanedEntry() + { + // Arrange + var project = await _projectRepository.GetByIdAsync(TestConstants.ProjectId); + Assert.NotNull(project); + + string deletedUserId = ObjectId.GenerateNewId().ToString(); + project.NotificationSettings[deletedUserId] = new NotificationSettings { ReportNewErrors = true }; + await _projectRepository.SaveAsync(project, o => o.ImmediateConsistency()); + + Assert.Null(await _userRepository.GetByIdAsync(deletedUserId)); + + // Act + await _workItemQueue.EnqueueAsync(new UpdateProjectNotificationSettingsWorkItem()); + await _workItemJob.RunUntilEmptyAsync(TestCancellationToken); + + // Assert + project = await _projectRepository.GetByIdAsync(TestConstants.ProjectId); + Assert.NotNull(project); + Assert.DoesNotContain(deletedUserId, project.NotificationSettings.Keys); + } + + [Fact] + public async Task RunUntilEmptyAsync_WithOrganizationIdFilter_OnlyProcessesTargetOrg() + { + // Arrange + var project = await _projectRepository.GetByIdAsync(TestConstants.ProjectId); + Assert.NotNull(project); + + string orphanedUserId = ObjectId.GenerateNewId().ToString(); + project.NotificationSettings[orphanedUserId] = new NotificationSettings { ReportNewErrors = true }; + await _projectRepository.SaveAsync(project, o => o.ImmediateConsistency()); + + // Act: cleanup a different org — should NOT affect our project + await _workItemQueue.EnqueueAsync(new UpdateProjectNotificationSettingsWorkItem { OrganizationId = TestConstants.OrganizationId2 }); + await _workItemJob.RunUntilEmptyAsync(TestCancellationToken); + + // Assert: orphaned entry should still exist + project = await _projectRepository.GetByIdAsync(TestConstants.ProjectId); + Assert.NotNull(project); + Assert.Contains(orphanedUserId, project.NotificationSettings.Keys); + + // Act: now cleanup the correct org + await _workItemQueue.EnqueueAsync(new UpdateProjectNotificationSettingsWorkItem { OrganizationId = TestConstants.OrganizationId }); + await _workItemJob.RunUntilEmptyAsync(TestCancellationToken); + + // Assert + project = await _projectRepository.GetByIdAsync(TestConstants.ProjectId); + Assert.NotNull(project); + Assert.DoesNotContain(orphanedUserId, project.NotificationSettings.Keys); + } + + [Fact] + public async Task RunUntilEmptyAsync_WithMultipleProjectsContainingOrphans_CleansAllProjects() + { + // Arrange: create a second project in the same org + var project1 = await _projectRepository.GetByIdAsync(TestConstants.ProjectId); + Assert.NotNull(project1); + + var project2 = _projectData.GenerateProject(generateId: true, organizationId: TestConstants.OrganizationId); + project2 = await _projectRepository.AddAsync(project2, o => o.ImmediateConsistency()); + + string orphanedUserId = ObjectId.GenerateNewId().ToString(); + project1.NotificationSettings[orphanedUserId] = new NotificationSettings { ReportNewErrors = true }; + project2.NotificationSettings[orphanedUserId] = new NotificationSettings { ReportCriticalErrors = true }; + await _projectRepository.SaveAsync([project1, project2], o => o.ImmediateConsistency()); + + // Act + await _workItemQueue.EnqueueAsync(new UpdateProjectNotificationSettingsWorkItem { OrganizationId = TestConstants.OrganizationId }); + await _workItemJob.RunUntilEmptyAsync(TestCancellationToken); + + // Assert: both projects should be cleaned + project1 = await _projectRepository.GetByIdAsync(project1.Id); + project2 = await _projectRepository.GetByIdAsync(project2.Id); + Assert.NotNull(project1); + Assert.NotNull(project2); + Assert.DoesNotContain(orphanedUserId, project1.NotificationSettings.Keys); + Assert.DoesNotContain(orphanedUserId, project2.NotificationSettings.Keys); + } + + [Fact] + public async Task RunUntilEmptyAsync_WithMoreThanOnePageOfProjects_CleansEveryProjectPage() + { + // Arrange + var firstProject = await _projectRepository.GetByIdAsync(TestConstants.ProjectId); + Assert.NotNull(firstProject); + + string orphanedUserId = ObjectId.GenerateNewId().ToString(); + firstProject.NotificationSettings[orphanedUserId] = new NotificationSettings { ReportNewErrors = true }; + await _projectRepository.SaveAsync(firstProject, o => o.ImmediateConsistency()); + + var additionalProjects = Enumerable.Range(0, 55) + .Select(_ => + { + var project = _projectData.GenerateProject(generateId: true, organizationId: TestConstants.OrganizationId); + project.NotificationSettings[orphanedUserId] = new NotificationSettings { ReportCriticalErrors = true }; + return project; + }) + .ToList(); + + await _projectRepository.AddAsync(additionalProjects, o => o.ImmediateConsistency()); + + // Act + await _workItemQueue.EnqueueAsync(new UpdateProjectNotificationSettingsWorkItem { OrganizationId = TestConstants.OrganizationId }); + await _workItemJob.RunUntilEmptyAsync(TestCancellationToken); + + // Assert + var refreshedProjects = await _projectRepository.GetByIdsAsync([firstProject.Id, ..additionalProjects.Select(project => project.Id)]); + Assert.Equal(56, refreshedProjects.Count); + Assert.All(refreshedProjects, project => Assert.DoesNotContain(orphanedUserId, project.NotificationSettings.Keys)); + } + + [Fact] + public async Task RunUntilEmptyAsync_WithNoOrphans_MakesNoChanges() + { + // Arrange: project only has valid user settings (global admin from sample data) + var project = await _projectRepository.GetByIdAsync(TestConstants.ProjectId); + Assert.NotNull(project); + + var settingsBefore = new Dictionary(project.NotificationSettings); + + // Act + await _workItemQueue.EnqueueAsync(new UpdateProjectNotificationSettingsWorkItem()); + await _workItemJob.RunUntilEmptyAsync(TestCancellationToken); + + // Assert + project = await _projectRepository.GetByIdAsync(TestConstants.ProjectId); + Assert.NotNull(project); + Assert.Equal(settingsBefore.Count, project.NotificationSettings.Count); + foreach (var key in settingsBefore.Keys) + Assert.Contains(key, project.NotificationSettings.Keys); + } + + [Fact] + public async Task RunUntilEmptyAsync_WithMixedOrphanTypes_RemovesAllOrphansAndPreservesValid() + { + // Arrange + var globalAdmin = await _userRepository.GetByEmailAddressAsync(SampleDataService.TEST_USER_EMAIL); + Assert.NotNull(globalAdmin); + + var project = await _projectRepository.GetByIdAsync(TestConstants.ProjectId); + Assert.NotNull(project); + + string deletedUserId = ObjectId.GenerateNewId().ToString(); + string removedFromOrgUserId = ObjectId.GenerateNewId().ToString(); + + var removedUser = _userData.GenerateUser(generateId: true, organizationId: TestConstants.OrganizationId2); + removedUser.Id = removedFromOrgUserId; + removedUser.ResetVerifyEmailAddressTokenAndExpiration(TimeProvider); + await _userRepository.AddAsync(removedUser, o => o.ImmediateConsistency()); + + project.NotificationSettings[deletedUserId] = new NotificationSettings { ReportNewErrors = true }; + project.NotificationSettings[removedFromOrgUserId] = new NotificationSettings { ReportCriticalErrors = true }; + project.NotificationSettings[Project.NotificationIntegrations.Slack] = new NotificationSettings { ReportNewErrors = true }; + await _projectRepository.SaveAsync(project, o => o.ImmediateConsistency()); + + // Act + await _workItemQueue.EnqueueAsync(new UpdateProjectNotificationSettingsWorkItem { OrganizationId = TestConstants.OrganizationId }); + await _workItemJob.RunUntilEmptyAsync(TestCancellationToken); + + // Assert + project = await _projectRepository.GetByIdAsync(TestConstants.ProjectId); + Assert.NotNull(project); + Assert.DoesNotContain(deletedUserId, project.NotificationSettings.Keys); + Assert.DoesNotContain(removedFromOrgUserId, project.NotificationSettings.Keys); + Assert.Contains(globalAdmin.Id, project.NotificationSettings.Keys); + Assert.Contains(Project.NotificationIntegrations.Slack, project.NotificationSettings.Keys); + } + + [Fact] + public async Task RunUntilEmptyAsync_WithUserInDifferentOrganization_RemovesOrphanedEntry() + { + // Arrange: user exists but belongs to a different org, not this one + var otherOrgUser = _userData.GenerateUser(generateId: true, organizationId: TestConstants.OrganizationId2); + otherOrgUser.ResetVerifyEmailAddressTokenAndExpiration(TimeProvider); + otherOrgUser = await _userRepository.AddAsync(otherOrgUser, o => o.ImmediateConsistency()); + + var project = await _projectRepository.GetByIdAsync(TestConstants.ProjectId); + Assert.NotNull(project); + project.NotificationSettings[otherOrgUser.Id] = new NotificationSettings { ReportNewErrors = true }; + await _projectRepository.SaveAsync(project, o => o.ImmediateConsistency()); + + // Act + await _workItemQueue.EnqueueAsync(new UpdateProjectNotificationSettingsWorkItem { OrganizationId = TestConstants.OrganizationId }); + await _workItemJob.RunUntilEmptyAsync(TestCancellationToken); + + // Assert + project = await _projectRepository.GetByIdAsync(TestConstants.ProjectId); + Assert.NotNull(project); + Assert.DoesNotContain(otherOrgUser.Id, project.NotificationSettings.Keys); + } +} From d28eda7fa12662a47e1afcf1cabe1144e151d952 Mon Sep 17 00:00:00 2001 From: Blake Niemyjski Date: Wed, 11 Mar 2026 06:09:22 -0500 Subject: [PATCH 2/4] Fix organization cleanup review feedback --- AGENTS.md | 4 ++++ src/Exceptionless.Core/Services/OrganizationService.cs | 9 ++------- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index c40b6f5404..4c04e1ab6b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -16,6 +16,10 @@ Run `Exceptionless.AppHost` from your IDE. Aspire starts all services (Elasticse | Frontend test | `npm run test:unit` | | E2E test | `npm run test:e2e` | +Testing note: `IntegrationTestsBase.ResetDataAsync()` clears indexes, cache, file storage, and queues, but it does not seed sample org/project/user fixtures. Tests that depend on `SampleDataService` constants must override `ResetDataAsync()` and call `SampleDataService.CreateDataAsync()`. +Work item note: Foundatio `WorkItemJob` already emits the final `100%` progress report for progress-enabled work items, so handlers should only report intermediate progress and avoid a duplicate terminal `ReportProgressAsync(100, ...)`. +Organization cleanup note: `OrganizationService.RemoveUsersAsync()` is shared by both soft-delete and hard-delete organization flows. Project notification-settings cleanup belongs in `SoftDeleteOrganizationAsync()` or other project-preserving paths, not in the shared user-removal helper. + ## Project Structure ```text diff --git a/src/Exceptionless.Core/Services/OrganizationService.cs b/src/Exceptionless.Core/Services/OrganizationService.cs index e57e5a6bb1..59dcca29d9 100644 --- a/src/Exceptionless.Core/Services/OrganizationService.cs +++ b/src/Exceptionless.Core/Services/OrganizationService.cs @@ -109,8 +109,6 @@ public async Task RemoveUsersAsync(Organization organization, string? curr break; } - await CleanupProjectNotificationSettingsAsync(organization, []); - return totalUsersAffected; } @@ -192,6 +190,7 @@ public async Task SoftDeleteOrganizationAsync(Organization organization, string await RemoveWebHooksAsync(organization); await CancelSubscriptionsAsync(organization); await RemoveUsersAsync(organization, currentUserId); + await CleanupProjectNotificationSettingsAsync(organization, []); organization.IsDeleted = true; await _organizationRepository.SaveAsync(organization); @@ -208,11 +207,7 @@ private async Task> GetValidNotificationUserIdsAsync(string orga cancellationToken.ThrowIfCancellationRequested(); var users = await _userRepository.GetByIdsAsync(batch); - foreach (var user in users) - { - if (user.OrganizationIds.Contains(organizationId)) - validUserIds.Add(user.Id); - } + validUserIds.UnionWith(users.Where(user => user.OrganizationIds.Contains(organizationId)).Select(user => user.Id)); } return validUserIds; From b695414083706e93599dbc4419b8f2c1a4b35840 Mon Sep 17 00:00:00 2001 From: Blake Niemyjski Date: Wed, 11 Mar 2026 06:30:30 -0500 Subject: [PATCH 3/4] Revert cleanup notes from agents --- AGENTS.md | 4 ---- 1 file changed, 4 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 4c04e1ab6b..c40b6f5404 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -16,10 +16,6 @@ Run `Exceptionless.AppHost` from your IDE. Aspire starts all services (Elasticse | Frontend test | `npm run test:unit` | | E2E test | `npm run test:e2e` | -Testing note: `IntegrationTestsBase.ResetDataAsync()` clears indexes, cache, file storage, and queues, but it does not seed sample org/project/user fixtures. Tests that depend on `SampleDataService` constants must override `ResetDataAsync()` and call `SampleDataService.CreateDataAsync()`. -Work item note: Foundatio `WorkItemJob` already emits the final `100%` progress report for progress-enabled work items, so handlers should only report intermediate progress and avoid a duplicate terminal `ReportProgressAsync(100, ...)`. -Organization cleanup note: `OrganizationService.RemoveUsersAsync()` is shared by both soft-delete and hard-delete organization flows. Project notification-settings cleanup belongs in `SoftDeleteOrganizationAsync()` or other project-preserving paths, not in the shared user-removal helper. - ## Project Structure ```text From 4fddf6a50f3ebe94907076ed0f63d2e466c602e2 Mon Sep 17 00:00:00 2001 From: Blake Niemyjski Date: Wed, 11 Mar 2026 13:37:18 -0500 Subject: [PATCH 4/4] 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. --- .agents/skills/accessibility/SKILL.md | 19 +-- .agents/skills/backend-architecture/SKILL.md | 10 +- .agents/skills/backend-testing/SKILL.md | 10 +- .agents/skills/dotnet-cli/SKILL.md | 10 +- .agents/skills/dotnet-conventions/SKILL.md | 10 +- .agents/skills/e2e-testing/SKILL.md | 78 +++++----- .agents/skills/foundatio/SKILL.md | 10 +- .agents/skills/frontend-architecture/SKILL.md | 52 ++++--- .agents/skills/frontend-testing/SKILL.md | 137 +++++++++-------- .agents/skills/security-principles/SKILL.md | 10 +- .agents/skills/shadcn-svelte/SKILL.md | 24 +-- .agents/skills/storybook/SKILL.md | 9 +- .agents/skills/svelte-components/SKILL.md | 12 +- .agents/skills/tanstack-form/SKILL.md | 24 +-- .agents/skills/tanstack-query/SKILL.md | 143 +++++++++++------- .../skills/typescript-conventions/SKILL.md | 40 ++--- ...jectNotificationSettingsWorkItemHandler.cs | 4 +- .../Services/OrganizationService.cs | 2 +- 18 files changed, 334 insertions(+), 270 deletions(-) diff --git a/.agents/skills/accessibility/SKILL.md b/.agents/skills/accessibility/SKILL.md index 48d5541049..0db396127f 100644 --- a/.agents/skills/accessibility/SKILL.md +++ b/.agents/skills/accessibility/SKILL.md @@ -1,10 +1,11 @@ --- name: accessibility -description: | - WCAG 2.2 AA accessibility standards for the Exceptionless frontend. Semantic HTML, keyboard - navigation, ARIA patterns, focus management, and form accessibility. - Keywords: WCAG, accessibility, a11y, ARIA, semantic HTML, keyboard navigation, focus management, - screen reader, alt text, aria-label, aria-describedby, skip links, focus trap +description: > + Use this skill when building or reviewing frontend components for accessibility compliance. + Covers WCAG 2.2 AA standards including semantic HTML, keyboard navigation, ARIA patterns, + focus management, screen reader support, and form accessibility. Apply when creating new + UI components, fixing accessibility bugs, adding skip links or focus traps, or ensuring + inclusive markup — even if the user doesn't explicitly mention "a11y" or "WCAG." --- # Accessibility (WCAG 2.2 AA) @@ -110,7 +111,7 @@ description: | // When dialog opens, focus first interactive element $effect(() => { if (open) { - dialogRef?.querySelector('input, button')?.focus(); + dialogRef?.querySelector("input, button")?.focus(); } }); @@ -240,10 +241,10 @@ npm run test:e2e ```typescript // In Playwright tests -import AxeBuilder from '@axe-core/playwright'; +import AxeBuilder from "@axe-core/playwright"; -test('page is accessible', async ({ page }) => { - await page.goto('/dashboard'); +test("page is accessible", async ({ page }) => { + await page.goto("/dashboard"); const results = await new AxeBuilder({ page }).analyze(); expect(results.violations).toEqual([]); }); diff --git a/.agents/skills/backend-architecture/SKILL.md b/.agents/skills/backend-architecture/SKILL.md index 29e09c15df..c771180466 100644 --- a/.agents/skills/backend-architecture/SKILL.md +++ b/.agents/skills/backend-architecture/SKILL.md @@ -1,10 +1,10 @@ --- name: backend-architecture -description: | - Backend architecture for Exceptionless. Project layering, repositories, validation, - controllers, authorization, WebSockets, configuration, and Aspire orchestration. - Keywords: Core, Insulation, repositories, FluentValidation, MiniValidator, controllers, - AuthorizationRoles, ProblemDetails, Aspire, WebSockets, AppOptions +description: > + Use this skill when working on the ASP.NET Core backend — adding controllers, repositories, + validators, authorization, WebSocket endpoints, or Aspire orchestration. Apply when modifying + project layering (Core, Insulation, Web, Job), configuring services, returning ProblemDetails + errors, or understanding how the backend is structured. --- # Backend Architecture diff --git a/.agents/skills/backend-testing/SKILL.md b/.agents/skills/backend-testing/SKILL.md index 0afdd6582e..08acef96e8 100644 --- a/.agents/skills/backend-testing/SKILL.md +++ b/.agents/skills/backend-testing/SKILL.md @@ -1,10 +1,10 @@ --- name: backend-testing -description: | - Backend testing with xUnit, Foundatio.Xunit, integration tests with AppWebHostFactory, - FluentClient, ProxyTimeProvider for time manipulation, and test data builders. - Keywords: xUnit, Fact, Theory, integration tests, AppWebHostFactory, FluentClient, - ProxyTimeProvider, TimeProvider, Foundatio.Xunit, TestWithLoggingBase, test data builders +description: > + Use this skill when writing or modifying C# tests — unit tests, integration tests, or + test fixtures. Covers xUnit patterns, AppWebHostFactory for integration testing, FluentClient + for API assertions, ProxyTimeProvider for time manipulation, and test data builders. Apply + when adding new test cases, debugging test failures, or setting up test infrastructure. --- # Backend Testing diff --git a/.agents/skills/dotnet-cli/SKILL.md b/.agents/skills/dotnet-cli/SKILL.md index 289015780e..b20841d86c 100644 --- a/.agents/skills/dotnet-cli/SKILL.md +++ b/.agents/skills/dotnet-cli/SKILL.md @@ -1,10 +1,10 @@ --- name: dotnet-cli -description: | - .NET command-line tools for building, testing, and formatting. Common dotnet commands - and development workflow. - Keywords: dotnet build, dotnet restore, dotnet test, dotnet format, dotnet run, - NuGet, package restore, CLI commands, build system +description: > + Use this skill when running .NET CLI commands — building, testing, restoring packages, + formatting code, or running projects. Covers dotnet build, test, restore, format, run, + and NuGet package management. Apply when troubleshooting build errors, running the backend, + or executing any dotnet command-line operation. --- # .NET CLI diff --git a/.agents/skills/dotnet-conventions/SKILL.md b/.agents/skills/dotnet-conventions/SKILL.md index f04363bcff..540bbf10e6 100644 --- a/.agents/skills/dotnet-conventions/SKILL.md +++ b/.agents/skills/dotnet-conventions/SKILL.md @@ -1,10 +1,10 @@ --- name: dotnet-conventions -description: | - C# coding standards for the Exceptionless codebase. Naming conventions, async patterns, - structured logging, nullable reference types, and formatting rules. - Keywords: C# style, naming conventions, _camelCase, PascalCase, async suffix, - CancellationToken, nullable annotations, structured logging, ExceptionlessState +description: > + Use this skill when writing or reviewing C# code to follow project conventions. Covers + naming standards, async patterns, CancellationToken usage, structured logging, nullable + reference types, and formatting rules. Apply when authoring new C# classes, reviewing + code style, or ensuring consistency with existing patterns. --- # .NET Conventions diff --git a/.agents/skills/e2e-testing/SKILL.md b/.agents/skills/e2e-testing/SKILL.md index 6c3e278b70..9b05e26fa6 100644 --- a/.agents/skills/e2e-testing/SKILL.md +++ b/.agents/skills/e2e-testing/SKILL.md @@ -1,10 +1,10 @@ --- name: e2e-testing -description: | - End-to-end frontend testing with Playwright. Page Object Model, selectors, fixtures, - accessibility audits. Limited E2E coverage currently - area for improvement. - Keywords: Playwright, E2E, Page Object Model, POM, data-testid, getByRole, getByLabel, - getByText, fixtures, axe-playwright, frontend testing +description: > + Use this skill when writing or running end-to-end browser tests with Playwright. Covers + Page Object Model patterns, selector strategies (data-testid, getByRole, getByLabel), + fixtures, and accessibility audits with axe-playwright. Apply when adding E2E test coverage, + debugging flaky tests, or testing user flows through the browser. --- # E2E Testing (Frontend) @@ -24,7 +24,7 @@ Create page objects for reusable page interactions: ```typescript // e2e/pages/login-page.ts -import { type Page, type Locator, expect } from '@playwright/test'; +import { type Page, type Locator, expect } from "@playwright/test"; export class LoginPage { readonly page: Page; @@ -35,14 +35,14 @@ export class LoginPage { constructor(page: Page) { this.page = page; - this.emailInput = page.getByLabel('Email'); - this.passwordInput = page.getByLabel('Password'); - this.submitButton = page.getByRole('button', { name: /log in/i }); - this.errorMessage = page.getByRole('alert'); + this.emailInput = page.getByLabel("Email"); + this.passwordInput = page.getByLabel("Password"); + this.submitButton = page.getByRole("button", { name: /log in/i }); + this.errorMessage = page.getByRole("alert"); } async goto() { - await this.page.goto('/login'); + await this.page.goto("/login"); } async login(email: string, password: string) { @@ -61,26 +61,26 @@ export class LoginPage { ```typescript // e2e/auth/login.spec.ts -import { test, expect } from '@playwright/test'; -import { LoginPage } from '../pages/login-page'; +import { test, expect } from "@playwright/test"; +import { LoginPage } from "../pages/login-page"; -test.describe('Login', () => { - test('successful login redirects to dashboard', async ({ page }) => { +test.describe("Login", () => { + test("successful login redirects to dashboard", async ({ page }) => { const loginPage = new LoginPage(page); await loginPage.goto(); - await loginPage.login('user@example.com', 'password123'); + await loginPage.login("user@example.com", "password123"); - await expect(page).toHaveURL('/'); + await expect(page).toHaveURL("/"); }); - test('invalid credentials shows error', async ({ page }) => { + test("invalid credentials shows error", async ({ page }) => { const loginPage = new LoginPage(page); await loginPage.goto(); - await loginPage.login('wrong@example.com', 'wrongpassword'); + await loginPage.login("wrong@example.com", "wrongpassword"); - await loginPage.expectError('Invalid email or password'); + await loginPage.expectError("Invalid email or password"); }); }); ``` @@ -89,24 +89,24 @@ test.describe('Login', () => { 1. **Semantic selectors first**: - ```typescript - page.getByRole('button', { name: /submit/i }); - page.getByLabel('Email address'); - page.getByText('Welcome back'); - ``` + ```typescript + page.getByRole("button", { name: /submit/i }); + page.getByLabel("Email address"); + page.getByText("Welcome back"); + ``` 2. **Fallback to test IDs**: - ```typescript - page.getByTestId('stack-trace'); - ``` + ```typescript + page.getByTestId("stack-trace"); + ``` 3. **Avoid implementation details**: - ```typescript - // ❌ Avoid CSS classes and IDs - page.locator('.btn-primary'); - ``` + ```typescript + // ❌ Avoid CSS classes and IDs + page.locator(".btn-primary"); + ``` ## Backend Data Setup @@ -122,13 +122,13 @@ For tests requiring specific data, consider: ```typescript test.beforeEach(async ({ request }) => { // Set up test data via API - await request.post('/api/test/seed', { - data: { scenario: 'events-with-errors' } + await request.post("/api/test/seed", { + data: { scenario: "events-with-errors" }, }); }); test.afterEach(async ({ request }) => { - await request.delete('/api/test/cleanup'); + await request.delete("/api/test/cleanup"); }); ``` @@ -137,11 +137,11 @@ test.afterEach(async ({ request }) => { ## Accessibility Audits ```typescript -import { test, expect } from '@playwright/test'; -import AxeBuilder from '@axe-core/playwright'; +import { test, expect } from "@playwright/test"; +import AxeBuilder from "@axe-core/playwright"; -test('login page has no accessibility violations', async ({ page }) => { - await page.goto('/login'); +test("login page has no accessibility violations", async ({ page }) => { + await page.goto("/login"); const results = await new AxeBuilder({ page }).analyze(); expect(results.violations).toEqual([]); diff --git a/.agents/skills/foundatio/SKILL.md b/.agents/skills/foundatio/SKILL.md index e93930f4d1..72ed37ec8e 100644 --- a/.agents/skills/foundatio/SKILL.md +++ b/.agents/skills/foundatio/SKILL.md @@ -1,10 +1,10 @@ --- name: foundatio -description: | - Foundatio infrastructure abstractions for caching, queuing, messaging, file storage, - locking, jobs, and resilience. Use context7 for complete API documentation. - Keywords: Foundatio, ICacheClient, IQueue, IMessageBus, IFileStorage, ILockProvider, - IJob, QueueJobBase, resilience, retry, Redis, Elasticsearch +description: > + Use this skill when working with Foundatio infrastructure abstractions — caching, queuing, + messaging, file storage, locking, or background jobs. Apply when using ICacheClient, IQueue, + IMessageBus, IFileStorage, ILockProvider, or IJob, or when implementing retry/resilience + patterns. Covers both in-memory and production (Redis, Elasticsearch) implementations. --- # Foundatio diff --git a/.agents/skills/frontend-architecture/SKILL.md b/.agents/skills/frontend-architecture/SKILL.md index b6609daa03..3fb9a9ae60 100644 --- a/.agents/skills/frontend-architecture/SKILL.md +++ b/.agents/skills/frontend-architecture/SKILL.md @@ -1,10 +1,10 @@ --- name: frontend-architecture -description: | - Svelte SPA architecture for Exceptionless. Route groups, lib structure, API client, - feature slices, and barrel exports. - Keywords: route groups, $lib, feature slices, api-client, barrel exports, index.ts, - vertical slices, shared components, generated models, ClientApp structure +description: > + Use this skill when working on the Svelte SPA's project structure — adding routes, creating + feature slices, organizing shared components, or understanding the ClientApp directory layout. + Covers route groups, $lib conventions, barrel exports, API client organization, and vertical + slice architecture. Apply when deciding where to place new files or components. --- # Frontend Architecture @@ -80,20 +80,25 @@ Centralize API calls per feature: ```typescript // features/organizations/api.svelte.ts -import { createQuery, createMutation, useQueryClient } from '@tanstack/svelte-query'; -import { useFetchClient } from '@exceptionless/fetchclient'; -import type { Organization, CreateOrganizationRequest } from './models'; +import { + createQuery, + createMutation, + useQueryClient, +} from "@tanstack/svelte-query"; +import { useFetchClient } from "@exceptionless/fetchclient"; +import type { Organization, CreateOrganizationRequest } from "./models"; export function getOrganizationsQuery() { const client = useFetchClient(); return createQuery(() => ({ - queryKey: ['organizations'], + queryKey: ["organizations"], queryFn: async () => { - const response = await client.getJSON('/organizations'); + const response = + await client.getJSON("/organizations"); if (!response.ok) throw response.problem; return response.data!; - } + }, })); } @@ -103,13 +108,16 @@ export function postOrganizationMutation() { return createMutation(() => ({ mutationFn: async (data: CreateOrganizationRequest) => { - const response = await client.postJSON('/organizations', data); + const response = await client.postJSON( + "/organizations", + data, + ); if (!response.ok) throw response.problem; return response.data!; }, onSuccess: () => { - queryClient.invalidateQueries({ queryKey: ['organizations'] }); - } + queryClient.invalidateQueries({ queryKey: ["organizations"] }); + }, })); } ``` @@ -123,8 +131,8 @@ Re-export generated models through feature model folders: export type { Organization, CreateOrganizationRequest, - UpdateOrganizationRequest -} from '$lib/generated'; + UpdateOrganizationRequest, +} from "$lib/generated"; // Add feature-specific types export interface OrganizationWithStats extends Organization { @@ -139,9 +147,9 @@ Use `index.ts` for clean imports: ```typescript // features/organizations/index.ts -export { getOrganizationsQuery, postOrganizationMutation } from './api.svelte'; -export type { Organization, CreateOrganizationRequest } from './models'; -export { organizationSchema } from './schemas'; +export { getOrganizationsQuery, postOrganizationMutation } from "./api.svelte"; +export type { Organization, CreateOrganizationRequest } from "./models"; +export { organizationSchema } from "./schemas"; ``` ## Shared Components @@ -176,9 +184,9 @@ Prefer regeneration over hand-writing DTOs. Generated types live in `$lib/genera ```typescript // Configured in svelte.config.js -import { Button } from '$comp/ui/button'; // $lib/components -import { User } from '$features/users/models'; // $lib/features -import { formatDate } from '$shared/formatters'; // $lib/features/shared +import { Button } from "$comp/ui/button"; // $lib/components +import { User } from "$features/users/models"; // $lib/features +import { formatDate } from "$shared/formatters"; // $lib/features/shared ``` ## Composite Component Pattern diff --git a/.agents/skills/frontend-testing/SKILL.md b/.agents/skills/frontend-testing/SKILL.md index 96c81daa18..760982642a 100644 --- a/.agents/skills/frontend-testing/SKILL.md +++ b/.agents/skills/frontend-testing/SKILL.md @@ -1,9 +1,10 @@ --- name: frontend-testing -description: | - Unit and component testing for the frontend with Vitest and Testing Library. - Keywords: Vitest, @testing-library/svelte, component tests, vi.mock, render, screen, - fireEvent, userEvent, test.ts, spec.ts, describe, it, AAA pattern +description: > + Use this skill when writing or running frontend unit and component tests with Vitest and + Testing Library. Covers render/screen/fireEvent patterns, vi.mock for mocking, and the + AAA (Arrange-Act-Assert) test structure. Apply when adding test coverage for Svelte + components, debugging test failures, or setting up test utilities. --- # Frontend Testing @@ -27,10 +28,10 @@ npm run test:unit Use explicit Arrange, Act, Assert regions: ```typescript -import { describe, expect, it } from 'vitest'; +import { describe, expect, it } from "vitest"; -describe('Calculator', () => { - it('should add two numbers correctly', () => { +describe("Calculator", () => { + it("should add two numbers correctly", () => { // Arrange const a = 5; const b = 3; @@ -42,7 +43,7 @@ describe('Calculator', () => { expect(result).toBe(8); }); - it('should handle negative numbers', () => { + it("should handle negative numbers", () => { // Arrange const a = -5; const b = 3; @@ -63,11 +64,11 @@ describe('Calculator', () => { From [dates.test.ts](src/Exceptionless.Web/ClientApp/src/lib/features/shared/dates.test.ts): ```typescript -import { describe, expect, it } from 'vitest'; -import { getDifferenceInSeconds, getRelativeTimeFormatUnit } from './dates'; +import { describe, expect, it } from "vitest"; +import { getDifferenceInSeconds, getRelativeTimeFormatUnit } from "./dates"; -describe('getDifferenceInSeconds', () => { - it('should calculate difference in seconds correctly', () => { +describe("getDifferenceInSeconds", () => { + it("should calculate difference in seconds correctly", () => { // Arrange const now = new Date(); const past = new Date(now.getTime() - 5000); @@ -80,18 +81,18 @@ describe('getDifferenceInSeconds', () => { }); }); -describe('getRelativeTimeFormatUnit', () => { - it('should return correct unit for given seconds', () => { +describe("getRelativeTimeFormatUnit", () => { + it("should return correct unit for given seconds", () => { // Arrange & Act & Assert (simple value tests) - expect(getRelativeTimeFormatUnit(30)).toBe('seconds'); - expect(getRelativeTimeFormatUnit(1800)).toBe('minutes'); - expect(getRelativeTimeFormatUnit(7200)).toBe('hours'); + expect(getRelativeTimeFormatUnit(30)).toBe("seconds"); + expect(getRelativeTimeFormatUnit(1800)).toBe("minutes"); + expect(getRelativeTimeFormatUnit(7200)).toBe("hours"); }); - it('should handle boundary cases correctly', () => { + it("should handle boundary cases correctly", () => { // Arrange & Act & Assert - expect(getRelativeTimeFormatUnit(59)).toBe('seconds'); - expect(getRelativeTimeFormatUnit(60)).toBe('minutes'); + expect(getRelativeTimeFormatUnit(59)).toBe("seconds"); + expect(getRelativeTimeFormatUnit(60)).toBe("minutes"); }); }); ``` @@ -101,27 +102,27 @@ describe('getRelativeTimeFormatUnit', () => { From [cached-persisted-state.svelte.test.ts](src/Exceptionless.Web/ClientApp/src/lib/features/shared/utils/cached-persisted-state.svelte.test.ts): ```typescript -import { beforeEach, describe, expect, it, vi } from 'vitest'; -import { CachedPersistedState } from './cached-persisted-state.svelte'; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { CachedPersistedState } from "./cached-persisted-state.svelte"; -describe('CachedPersistedState', () => { +describe("CachedPersistedState", () => { beforeEach(() => { vi.clearAllMocks(); }); - it('should initialize with default value when storage is empty', () => { + it("should initialize with default value when storage is empty", () => { // Arrange & Act - const state = new CachedPersistedState('test-key', 'default'); + const state = new CachedPersistedState("test-key", "default"); // Assert - expect(state.current).toBe('default'); + expect(state.current).toBe("default"); }); - it('should return cached value without reading storage repeatedly', () => { + it("should return cached value without reading storage repeatedly", () => { // Arrange - const getItemSpy = vi.spyOn(Storage.prototype, 'getItem'); - localStorage.setItem('test-key', 'value1'); - const state = new CachedPersistedState('test-key', 'default'); + const getItemSpy = vi.spyOn(Storage.prototype, "getItem"); + localStorage.setItem("test-key", "value1"); + const state = new CachedPersistedState("test-key", "default"); getItemSpy.mockClear(); // Act @@ -129,8 +130,8 @@ describe('CachedPersistedState', () => { const val2 = state.current; // Assert - expect(val1).toBe('value1'); - expect(val2).toBe('value1'); + expect(val1).toBe("value1"); + expect(val2).toBe("value1"); expect(getItemSpy).not.toHaveBeenCalled(); }); }); @@ -141,26 +142,44 @@ describe('CachedPersistedState', () => { From [helpers.svelte.test.ts](src/Exceptionless.Web/ClientApp/src/lib/features/events/components/filters/helpers.svelte.test.ts): ```typescript -import { describe, expect, it } from 'vitest'; -import { quoteIfSpecialCharacters } from './helpers.svelte'; +import { describe, expect, it } from "vitest"; +import { quoteIfSpecialCharacters } from "./helpers.svelte"; -describe('helpers.svelte', () => { - it('quoteIfSpecialCharacters handles tabs and newlines', () => { +describe("helpers.svelte", () => { + it("quoteIfSpecialCharacters handles tabs and newlines", () => { // Arrange & Act & Assert - expect(quoteIfSpecialCharacters('foo\tbar')).toBe('"foo\tbar"'); - expect(quoteIfSpecialCharacters('foo\nbar')).toBe('"foo\nbar"'); + expect(quoteIfSpecialCharacters("foo\tbar")).toBe('"foo\tbar"'); + expect(quoteIfSpecialCharacters("foo\nbar")).toBe('"foo\nbar"'); }); - it('quoteIfSpecialCharacters handles empty string and undefined/null', () => { + it("quoteIfSpecialCharacters handles empty string and undefined/null", () => { // Arrange & Act & Assert - expect(quoteIfSpecialCharacters('')).toBe(''); + expect(quoteIfSpecialCharacters("")).toBe(""); expect(quoteIfSpecialCharacters(undefined)).toBeUndefined(); expect(quoteIfSpecialCharacters(null)).toBeNull(); }); - it('quoteIfSpecialCharacters quotes all Lucene special characters', () => { + it("quoteIfSpecialCharacters quotes all Lucene special characters", () => { // Arrange - const luceneSpecials = ['+', '-', '!', '(', ')', '{', '}', '[', ']', '^', '"', '~', '*', '?', ':', '\\', '/']; + const luceneSpecials = [ + "+", + "-", + "!", + "(", + ")", + "{", + "}", + "[", + "]", + "^", + '"', + "~", + "*", + "?", + ":", + "\\", + "/", + ]; // Act & Assert for (const char of luceneSpecials) { @@ -176,50 +195,50 @@ Use accessible queries (not implementation details): ```typescript // ✅ Role-based -screen.getByRole('button', { name: /submit/i }); -screen.getByRole('textbox', { name: /email/i }); +screen.getByRole("button", { name: /submit/i }); +screen.getByRole("textbox", { name: /email/i }); // ✅ Label-based -screen.getByLabelText('Email address'); +screen.getByLabelText("Email address"); // ✅ Text-based -screen.getByText('Welcome back'); +screen.getByText("Welcome back"); // ⚠️ Fallback: Test ID -screen.getByTestId('complex-chart'); +screen.getByTestId("complex-chart"); // ❌ Avoid: Implementation details -screen.getByClassName('btn-primary'); +screen.getByClassName("btn-primary"); ``` ## Mocking Modules ```typescript -import { vi, describe, it, beforeEach, expect } from 'vitest'; -import { render, screen } from '@testing-library/svelte'; +import { vi, describe, it, beforeEach, expect } from "vitest"; +import { render, screen } from "@testing-library/svelte"; -vi.mock('$lib/api/organizations', () => ({ - getOrganizations: vi.fn() +vi.mock("$lib/api/organizations", () => ({ + getOrganizations: vi.fn(), })); -import { getOrganizations } from '$lib/api/organizations'; -import OrganizationList from './organization-list.svelte'; +import { getOrganizations } from "$lib/api/organizations"; +import OrganizationList from "./organization-list.svelte"; -describe('OrganizationList', () => { +describe("OrganizationList", () => { beforeEach(() => { vi.clearAllMocks(); }); - it('displays organizations from API', async () => { + it("displays organizations from API", async () => { // Arrange - const mockOrganizations = [{ id: '1', name: 'Org One' }]; + const mockOrganizations = [{ id: "1", name: "Org One" }]; vi.mocked(getOrganizations).mockResolvedValue(mockOrganizations); // Act render(OrganizationList); // Assert - expect(await screen.findByText('Org One')).toBeInTheDocument(); + expect(await screen.findByText("Org One")).toBeInTheDocument(); }); }); ``` @@ -227,7 +246,7 @@ describe('OrganizationList', () => { ## Snapshot Testing (Use Sparingly) ```typescript -it('matches snapshot', () => { +it("matches snapshot", () => { // Arrange & Act const { container } = render(StaticComponent); diff --git a/.agents/skills/security-principles/SKILL.md b/.agents/skills/security-principles/SKILL.md index 2e56a1e0f4..2fbb49b1cb 100644 --- a/.agents/skills/security-principles/SKILL.md +++ b/.agents/skills/security-principles/SKILL.md @@ -1,10 +1,10 @@ --- name: security-principles -description: | - Security best practices for the Exceptionless codebase. Secrets management, input validation, - secure defaults, and avoiding common vulnerabilities. - Keywords: security, secrets, encryption, PII, logging, input validation, secure defaults, - environment variables, OWASP, cryptography +description: > + Use this skill when handling secrets, credentials, PII, input validation, or any + security-sensitive code. Covers secrets management, secure defaults, encryption, logging + safety, and common vulnerability prevention. Apply when adding authentication, configuring + environment variables, reviewing code for security issues, or working with sensitive data. --- # Security Principles diff --git a/.agents/skills/shadcn-svelte/SKILL.md b/.agents/skills/shadcn-svelte/SKILL.md index 95cbe8f603..8e7cc0fe6a 100644 --- a/.agents/skills/shadcn-svelte/SKILL.md +++ b/.agents/skills/shadcn-svelte/SKILL.md @@ -1,10 +1,10 @@ --- name: shadcn-svelte -description: | - UI components with shadcn-svelte and bits-ui. Component patterns, trigger snippets, - dialog handling, and accessibility. - Keywords: shadcn-svelte, bits-ui, Button, Dialog, Sheet, Popover, DropdownMenu, - Tooltip, Form, Input, Select, child snippet, trigger pattern, cn utility +description: > + Use this skill when building UI with shadcn-svelte or bits-ui components — buttons, dialogs, + sheets, popovers, dropdowns, tooltips, forms, inputs, or selects. Covers import patterns, + trigger snippets, child snippet composition, and the cn utility. Apply when adding or + customizing any shadcn-svelte component in the frontend. --- # shadcn-svelte Components @@ -173,18 +173,18 @@ When using trigger components with custom elements like Button, **always use the ```typescript // options.ts -import type { DropdownItem } from '$shared/types'; +import type { DropdownItem } from "$shared/types"; export enum Status { - Active = 'active', - Inactive = 'inactive', - Pending = 'pending' + Active = "active", + Inactive = "inactive", + Pending = "pending", } export const statusOptions: DropdownItem[] = [ - { value: Status.Active, label: 'Active' }, - { value: Status.Inactive, label: 'Inactive' }, - { value: Status.Pending, label: 'Pending' } + { value: Status.Active, label: "Active" }, + { value: Status.Inactive, label: "Inactive" }, + { value: Status.Pending, label: "Pending" }, ]; ``` diff --git a/.agents/skills/storybook/SKILL.md b/.agents/skills/storybook/SKILL.md index b44ec8689f..8b7d786eee 100644 --- a/.agents/skills/storybook/SKILL.md +++ b/.agents/skills/storybook/SKILL.md @@ -1,9 +1,10 @@ --- name: storybook -description: | - Component stories using Storybook with Svelte CSF. Story patterns, defineMeta, argTypes, - snippet-based customization, and visual testing. - Keywords: storybook, stories.svelte, defineMeta, Story, args, argTypes, autodocs +description: > + Use this skill when creating or updating Storybook stories for Svelte components. Covers + Svelte CSF story format, defineMeta, argTypes, snippet-based customization, and autodocs. + Apply when adding visual documentation for components, setting up story files, or running + Storybook for development. --- # Storybook diff --git a/.agents/skills/svelte-components/SKILL.md b/.agents/skills/svelte-components/SKILL.md index b57d36ebd7..f838e69017 100644 --- a/.agents/skills/svelte-components/SKILL.md +++ b/.agents/skills/svelte-components/SKILL.md @@ -1,10 +1,10 @@ --- name: svelte-components -description: | - Svelte 5 component patterns for the Exceptionless SPA. Runes, reactivity, props, - events, snippets, component organization, and shadcn-svelte integration. - Keywords: Svelte 5, $state, $derived, $effect, $props, runes, onclick, snippets, - {@render}, reactive, component composition, shadcn-svelte +description: > + Use this skill when writing Svelte 5 components — using runes ($state, $derived, $effect, + $props), handling events, composing with snippets ({@render}), or integrating with + shadcn-svelte. Apply when creating new components, refactoring to Svelte 5 patterns, + or debugging reactivity issues in the frontend. --- # Svelte Components @@ -146,7 +146,7 @@ Use `onclick` instead of `on:click`: ## Snippets (Content Projection) -Replace `` with snippets. From [login/+page.svelte](src/Exceptionless.Web/ClientApp/src/routes/(auth)/login/+page.svelte): +Replace `` with snippets. From [login/+page.svelte](): ```svelte state.errors}> diff --git a/.agents/skills/tanstack-form/SKILL.md b/.agents/skills/tanstack-form/SKILL.md index 831a55d51a..d7011192c3 100644 --- a/.agents/skills/tanstack-form/SKILL.md +++ b/.agents/skills/tanstack-form/SKILL.md @@ -1,10 +1,10 @@ --- name: tanstack-form -description: | - TanStack Form with Zod validation in Svelte 5. Form state management, field validation, - error handling, and ProblemDetails integration. - Keywords: TanStack Form, createForm, Field, form validation, zod schema, form errors, - onSubmit, onSubmitAsync, problemDetailsToFormErrors +description: > + Use this skill when building or modifying forms with TanStack Form and Zod validation. + Covers createForm, field-level validation, error handling, and mapping ProblemDetails + API errors to form fields. Apply when adding new forms, implementing validation logic, + or handling form submission in the Svelte frontend. --- # TanStack Form @@ -24,23 +24,23 @@ export type LoginFormData = Infer; // Extended in feature schemas.ts // From src/lib/features/auth/schemas.ts -import { ChangePasswordModelSchema } from '$generated/schemas'; +import { ChangePasswordModelSchema } from "$generated/schemas"; export const ChangePasswordSchema = ChangePasswordModelSchema.extend({ - confirm_password: string().min(6).max(100) + confirm_password: string().min(6).max(100), }).refine((data) => data.password === data.confirm_password, { - message: 'Passwords do not match', - path: ['confirm_password'] + message: "Passwords do not match", + path: ["confirm_password"], }); export type ChangePasswordFormData = Infer; // Re-export generated schemas -export { LoginSchema, type LoginFormData } from '$generated/schemas'; +export { LoginSchema, type LoginFormData } from "$generated/schemas"; ``` ## Basic Form Pattern -From [login/+page.svelte](src/Exceptionless.Web/ClientApp/src/routes/(auth)/login/+page.svelte): +From [login/+page.svelte](): ```svelte