diff --git a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/SettingsServiceTests.cs b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/SettingsServiceTests.cs index 359b1559..8f4dd13d 100644 --- a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/SettingsServiceTests.cs +++ b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/SettingsServiceTests.cs @@ -12,6 +12,7 @@ using NUnit.Framework; using Microting.EformAngularFrontendBase.Infrastructure.Data; using Microting.eFormApi.BasePn.Infrastructure.Database.Entities; +using Microting.eFormApi.BasePn.Infrastructure.Models.API; using TimePlanning.Pn.Infrastructure.Models.Settings; using TimePlanning.Pn.Services.TimePlanningLocalizationService; using TimePlanning.Pn.Services.TimePlanningSettingService; @@ -200,6 +201,75 @@ public async Task UpdateAssignedSite_UpdatesGpsEnabled_Successfully() Assert.That(updatedSite.SnapshotEnabled, Is.False); } + // A managing tag that was removed must be addable again: the unique index on + // (AssignedSiteId, TagId) also covers soft-deleted rows, so inserting a second + // row for the tag failed with a duplicate key and the settings save returned 500. + [Test] + public async Task UpdateAssignedSite_ManagingTags_RemovedTagCanBeAddedAgain() + { + var assignedSite = new AssignedSiteEntity + { + SiteId = 61, + IsManager = true, + UseGoogleSheetAsDefault = true, + CreatedByUserId = 1, + UpdatedByUserId = 1 + }; + await assignedSite.Create(TimePlanningPnDbContext); + + // Seeded by another user (2), so the saves below must stamp the acting user (1). + const int otherUserId = 2; + var removedTag = new AssignedSiteManagingTag + { + AssignedSiteId = assignedSite.Id, TagId = 3, CreatedByUserId = otherUserId, UpdatedByUserId = otherUserId + }; + await removedTag.Create(TimePlanningPnDbContext); + await removedTag.Delete(TimePlanningPnDbContext); + await new AssignedSiteManagingTag + { + AssignedSiteId = assignedSite.Id, TagId = 5, CreatedByUserId = otherUserId, UpdatedByUserId = otherUserId + }.Create(TimePlanningPnDbContext); + + Task Save(params int[] tagIds) => + _settingsService.UpdateAssignedSite(new AssignedSiteModel + { + Id = assignedSite.Id, + SiteId = 61, + IsManager = true, + UseGoogleSheetAsDefault = true, + ManagingTagIds = tagIds.ToList() + }); + + Task> Rows() => + TimePlanningPnDbContext.AssignedSiteManagingTags.AsNoTracking() + .Where(x => x.AssignedSiteId == assignedSite.Id) + .ToListAsync(); + + static int[] ActiveTagIds(List rows) => + rows.Where(x => x.WorkflowState != Constants.WorkflowStates.Removed) + .Select(x => x.TagId).OrderBy(x => x).ToArray(); + + // Give back the removed tag 3, keep 5, add the new tag 10 (twice in the request). + Assert.That((await Save(3, 5, 10, 10)).Success, Is.True); + var rows = await Rows(); + Assert.That(ActiveTagIds(rows), Is.EqualTo(new[] { 3, 5, 10 })); + Assert.That(rows.Single(x => x.TagId == 3).Id, Is.EqualTo(removedTag.Id), "tag 3 must reuse its old row"); + Assert.That(rows.Single(x => x.TagId == 3).UpdatedByUserId, Is.EqualTo(1), "reviving must record the acting user"); + Assert.That(rows.Single(x => x.TagId == 5).UpdatedByUserId, Is.EqualTo(otherUserId), "an untouched row keeps its updater"); + + // Remove 3 and 10 again (soft delete keeps the rows), then give 3 back once more. + Assert.That((await Save(5)).Success, Is.True); + rows = await Rows(); + Assert.That(ActiveTagIds(rows), Is.EqualTo(new[] { 5 })); + Assert.That(rows.Count, Is.EqualTo(3)); + + Assert.That((await Save(3)).Success, Is.True); + rows = await Rows(); + Assert.That(ActiveTagIds(rows), Is.EqualTo(new[] { 3 })); + Assert.That(rows.Single(x => x.TagId == 3).Id, Is.EqualTo(removedTag.Id)); + Assert.That(rows.Single(x => x.TagId == 5).UpdatedByUserId, Is.EqualTo(1), "removing must record the acting user"); + } + // #1740: every site runs on 1-minute intervals and the settings checkbox is // gone, so UpdateAssignedSite must ignore whatever a (stale) client sends for // UseOneMinuteIntervals — in both directions — and never touch the recorded diff --git a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Services/TimePlanningSettingService/TimeSettingService.cs b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Services/TimePlanningSettingService/TimeSettingService.cs index 5276042c..1171ff48 100644 --- a/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Services/TimePlanningSettingService/TimeSettingService.cs +++ b/eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Services/TimePlanningSettingService/TimeSettingService.cs @@ -1195,38 +1195,43 @@ await pushNotificationService.SendToSiteAsync( dbAssignedSite.SiteId); } - // Update managing tags - var existingManagingTags = await dbContext.AssignedSiteManagingTags + // Update managing tags. Removed rows are loaded too: the unique index on + // (AssignedSiteId, TagId) also covers soft-deleted rows, so a tag that is + // given back must revive its old row instead of inserting a new one. + var managingTagRows = await dbContext.AssignedSiteManagingTags .Where(x => x.AssignedSiteId == dbAssignedSite.Id) - .Where(x => x.WorkflowState != Constants.WorkflowStates.Removed) .ToListAsync(); - var managingTagIds = site.ManagingTagIds ?? new List(); + var managingTagIds = site.ManagingTagIds?.ToHashSet() ?? new HashSet(); - // Remove tags that are no longer in the list - foreach (var existingTag in existingManagingTags) + foreach (var row in managingTagRows) { - if (!managingTagIds.Contains(existingTag.TagId)) + var wanted = managingTagIds.Contains(row.TagId); + var removed = row.WorkflowState == Constants.WorkflowStates.Removed; + if (!wanted && !removed) + { + row.UpdatedByUserId = userService.UserId; + await row.Delete(dbContext); + } + else if (wanted && removed) { - await existingTag.Delete(dbContext); + row.WorkflowState = Constants.WorkflowStates.Created; + row.UpdatedByUserId = userService.UserId; + await row.Update(dbContext); } } - // Add new tags - var existingTagIds = existingManagingTags.Select(x => x.TagId).ToList(); - foreach (var tagId in managingTagIds) + var knownTagIds = managingTagRows.Select(x => x.TagId).ToHashSet(); + foreach (var tagId in managingTagIds.Except(knownTagIds)) { - if (!existingTagIds.Contains(tagId)) + var newManagingTag = new AssignedSiteManagingTag { - var newManagingTag = new AssignedSiteManagingTag - { - AssignedSiteId = dbAssignedSite.Id, - TagId = tagId, - CreatedByUserId = userService.UserId, - UpdatedByUserId = userService.UserId - }; - await newManagingTag.Create(dbContext); - } + AssignedSiteId = dbAssignedSite.Id, + TagId = tagId, + CreatedByUserId = userService.UserId, + UpdatedByUserId = userService.UserId + }; + await newManagingTag.Create(dbContext); } if (dbAssignedSite.UseGoogleSheetAsDefault)