diff --git a/Core/Resgrid.Services/Records/RecordsBulkPacketService.cs b/Core/Resgrid.Services/Records/RecordsBulkPacketService.cs index 63704dca9..ae8de7a89 100644 --- a/Core/Resgrid.Services/Records/RecordsBulkPacketService.cs +++ b/Core/Resgrid.Services/Records/RecordsBulkPacketService.cs @@ -167,6 +167,9 @@ public async Task AssignForReviewAsync(int departmentId, stri throw new UnauthorizedAccessException("Bulk assign-for-review requires the ReviewRecords permission."); if (!await _authorization.IsActiveMemberAsync(request.ReviewerUserId, departmentId)) throw new ArgumentException("The reviewer is not an active member of this department.", nameof(request)); + // Checked once here: the per-record check would otherwise reject every row for the same reviewer-level reason. + if (!await _authorization.HasPermissionAsync(request.ReviewerUserId, departmentId, PermissionTypes.ReviewRecords)) + throw new ArgumentException("The chosen reviewer does not hold the ReviewRecords permission.", nameof(request)); var result = new RecordsBulkResult(); foreach (var id in ids) @@ -179,7 +182,11 @@ public async Task AssignForReviewAsync(int departmentId, stri } catch (RecordTransitionException) { Skip(result, id, "not_awaiting_review"); } catch (UnauthorizedAccessException) { Skip(result, id, "not_visible"); } - catch (ArgumentException) { Skip(result, id, "not_found"); } + catch (KeyNotFoundException) { Skip(result, id, "not_found"); } + // Someone else wrote the row between load and save; skip it rather than abort the rows after it. + catch (RecordConcurrencyException) { Skip(result, id, "conflict"); } + // The reviewer lost ReviewRecords after the precheck; earlier rows are already committed, so keep the batch result. + catch (ArgumentException) { Skip(result, id, "reviewer_not_eligible"); } } return result; } diff --git a/Providers/Resgrid.Providers.Migrations/Migrations/M0231_FixDepartmentProfilesLogoGooglePlusColumns.cs b/Providers/Resgrid.Providers.Migrations/Migrations/M0231_FixDepartmentProfilesLogoGooglePlusColumns.cs new file mode 100644 index 000000000..ad6decf70 --- /dev/null +++ b/Providers/Resgrid.Providers.Migrations/Migrations/M0231_FixDepartmentProfilesLogoGooglePlusColumns.cs @@ -0,0 +1,36 @@ +using FluentMigrator; + +namespace Resgrid.Providers.Migrations.Migrations +{ + /// + /// The initial schema created DepartmentProfiles with misspelled "Lo" and "oglePlus" columns (the same + /// dropped-"go" damage M0113 repaired on Notes and Documents) while the entity, and so every + /// Dapper-generated INSERT/UPDATE, uses "Logo" and "GooglePlus" — no department profile could ever be + /// saved against a database built from M0001, and the Department Profile page failed on first load when + /// it tried to create the row. Renames each column where the typo exists; guarded so databases that + /// already have the correct column (every database that predates M0001, or one hand-fixed) are untouched. + /// + /// No data moves: because every save failed, a database carrying the typo holds no DepartmentProfiles + /// rows, so M0172's one-time legacy logo copy (which it skipped here, finding no Logo column) has + /// nothing to pick up. + /// + [Migration(231)] + public class M0231_FixDepartmentProfilesLogoGooglePlusColumns : Migration + { + public override void Up() + { + Execute.Sql(@" +IF COL_LENGTH('dbo.DepartmentProfiles', 'Lo') IS NOT NULL AND COL_LENGTH('dbo.DepartmentProfiles', 'Logo') IS NULL + EXEC sp_rename 'dbo.DepartmentProfiles.Lo', 'Logo', 'COLUMN';"); + + Execute.Sql(@" +IF COL_LENGTH('dbo.DepartmentProfiles', 'oglePlus') IS NOT NULL AND COL_LENGTH('dbo.DepartmentProfiles', 'GooglePlus') IS NULL + EXEC sp_rename 'dbo.DepartmentProfiles.oglePlus', 'GooglePlus', 'COLUMN';"); + } + + public override void Down() + { + // One-way typo fix; nothing to restore. + } + } +} diff --git a/Providers/Resgrid.Providers.Migrations/Sql/M0001_InitialMigration.sql b/Providers/Resgrid.Providers.Migrations/Sql/M0001_InitialMigration.sql index 25c4f74fd..f15ff796b 100644 Binary files a/Providers/Resgrid.Providers.Migrations/Sql/M0001_InitialMigration.sql and b/Providers/Resgrid.Providers.Migrations/Sql/M0001_InitialMigration.sql differ diff --git a/Providers/Resgrid.Providers.MigrationsPg/Migrations/M0231_FixDepartmentProfilesLogoGooglePlusColumnsPg.cs b/Providers/Resgrid.Providers.MigrationsPg/Migrations/M0231_FixDepartmentProfilesLogoGooglePlusColumnsPg.cs new file mode 100644 index 000000000..8708ddf25 --- /dev/null +++ b/Providers/Resgrid.Providers.MigrationsPg/Migrations/M0231_FixDepartmentProfilesLogoGooglePlusColumnsPg.cs @@ -0,0 +1,43 @@ +using FluentMigrator; + +namespace Resgrid.Providers.MigrationsPg.Migrations +{ + /// + /// The initial schema created departmentprofiles with misspelled "lo" and "ogleplus" columns (the same + /// dropped-"go" damage M0113 repaired on notes and documents) while the entity, and so every + /// Dapper-generated INSERT/UPDATE, uses "logo" and "googleplus" — every save against a database built + /// from M0001 failed with 42703 "column googleplus of relation departmentprofiles does not exist", which + /// broke the Department Profile page on first load when it tried to create the row. Renames each column + /// where the typo exists; guarded so databases that already have the correct column (or were hand-fixed) + /// are untouched. + /// + /// No data moves: because every save failed, a database carrying the typo holds no departmentprofiles + /// rows, so M0172's one-time legacy logo copy (which it skipped here, finding no logo column) has + /// nothing to pick up. + /// + [Migration(231)] + public class M0231_FixDepartmentProfilesLogoGooglePlusColumnsPg : Migration + { + public override void Up() + { + Execute.Sql(@" +DO $$ +BEGIN + IF EXISTS (SELECT 1 FROM information_schema.columns WHERE table_schema = 'public' AND table_name = 'departmentprofiles' AND column_name = 'lo') + AND NOT EXISTS (SELECT 1 FROM information_schema.columns WHERE table_schema = 'public' AND table_name = 'departmentprofiles' AND column_name = 'logo') THEN + ALTER TABLE public.departmentprofiles RENAME COLUMN lo TO logo; + END IF; + + IF EXISTS (SELECT 1 FROM information_schema.columns WHERE table_schema = 'public' AND table_name = 'departmentprofiles' AND column_name = 'ogleplus') + AND NOT EXISTS (SELECT 1 FROM information_schema.columns WHERE table_schema = 'public' AND table_name = 'departmentprofiles' AND column_name = 'googleplus') THEN + ALTER TABLE public.departmentprofiles RENAME COLUMN ogleplus TO googleplus; + END IF; +END $$;"); + } + + public override void Down() + { + // One-way typo fix; nothing to restore. + } + } +} diff --git a/Providers/Resgrid.Providers.MigrationsPg/Sql/M0001_InitialMigration.sql b/Providers/Resgrid.Providers.MigrationsPg/Sql/M0001_InitialMigration.sql index d1c04dfe5..ea686861f 100644 Binary files a/Providers/Resgrid.Providers.MigrationsPg/Sql/M0001_InitialMigration.sql and b/Providers/Resgrid.Providers.MigrationsPg/Sql/M0001_InitialMigration.sql differ diff --git a/Tests/Resgrid.Tests/Rms/RecordsBulkPacketServiceTests.cs b/Tests/Resgrid.Tests/Rms/RecordsBulkPacketServiceTests.cs index 9ac4b3e25..129d8c9bc 100644 --- a/Tests/Resgrid.Tests/Rms/RecordsBulkPacketServiceTests.cs +++ b/Tests/Resgrid.Tests/Rms/RecordsBulkPacketServiceTests.cs @@ -150,16 +150,37 @@ public async Task Assign_for_review_touches_only_records_awaiting_review_and_rep _records.Setup(r => r.AssignReviewerAsync(Dept, Exporter, "r1", "reviewer", "rotation", It.IsAny())).ReturnsAsync(new RecordAggregate()); _records.Setup(r => r.AssignReviewerAsync(Dept, Exporter, "draft", "reviewer", "rotation", It.IsAny())).ThrowsAsync(new RecordTransitionException("draft", RmsRecordState.Draft, RmsRecordState.Draft, "only a Record awaiting review can be assigned a reviewer")); _records.Setup(r => r.AssignReviewerAsync(Dept, Exporter, "hidden", "reviewer", "rotation", It.IsAny())).ThrowsAsync(new UnauthorizedAccessException()); + // An id with no operational Record (deleted since the list rendered, or another kind's id) must not abort the batch. + _records.Setup(r => r.AssignReviewerAsync(Dept, Exporter, "missing", "reviewer", "rotation", It.IsAny())).ThrowsAsync(new KeyNotFoundException("Record missing does not exist in this department.")); + _records.Setup(r => r.AssignReviewerAsync(Dept, Exporter, "r2", "reviewer", "rotation", It.IsAny())).ReturnsAsync(new RecordAggregate()); + _authorization.Setup(a => a.HasPermissionAsync("reviewer", Dept, PermissionTypes.ReviewRecords)).ReturnsAsync(true); - var result = await _service.AssignForReviewAsync(Dept, Exporter, new RecordsBulkAssignRequest { RecordIds = new List { "r1", "draft", "hidden" }, ReviewerUserId = "reviewer", Reason = "rotation" }); + var result = await _service.AssignForReviewAsync(Dept, Exporter, new RecordsBulkAssignRequest { RecordIds = new List { "r1", "draft", "missing", "hidden", "r2" }, ReviewerUserId = "reviewer", Reason = "rotation" }); - result.Processed.Should().Be(1); - result.Skips.Select(s => s.RecordId + ":" + s.Reason).Should().BeEquivalentTo("draft:not_awaiting_review", "hidden:not_visible"); + result.Processed.Should().Be(2); + result.Skips.Select(s => s.RecordId + ":" + s.Reason).Should().BeEquivalentTo("draft:not_awaiting_review", "missing:not_found", "hidden:not_visible"); result.Run.Should().BeNull(); _authorization.Setup(a => a.IsActiveMemberAsync("gone", Dept)).ReturnsAsync(false); Func inactive = () => _service.AssignForReviewAsync(Dept, Exporter, new RecordsBulkAssignRequest { RecordIds = new List { "r1" }, ReviewerUserId = "gone" }); await inactive.Should().ThrowAsync(); + + // A reviewer without ReviewRecords fails the whole request up front rather than skipping every row. + Func notReviewer = () => _service.AssignForReviewAsync(Dept, Exporter, new RecordsBulkAssignRequest { RecordIds = new List { "r1" }, ReviewerUserId = "member" }); + (await notReviewer.Should().ThrowAsync()).Which.Message.Should().Contain("ReviewRecords"); + + // Permission revoked mid-batch: the rows already assigned are still reported rather than lost behind an error. + _records.Setup(r => r.AssignReviewerAsync(Dept, Exporter, "r2", "reviewer", "rotation", It.IsAny())).ThrowsAsync(new ArgumentException("The chosen reviewer does not hold the ReviewRecords permission.")); + var partial = await _service.AssignForReviewAsync(Dept, Exporter, new RecordsBulkAssignRequest { RecordIds = new List { "r1", "r2" }, ReviewerUserId = "reviewer", Reason = "rotation" }); + partial.Processed.Should().Be(1); + partial.Skips.Select(s => s.RecordId + ":" + s.Reason).Should().BeEquivalentTo("r2:reviewer_not_eligible"); + + // A concurrent write on one row is skipped; the rows after it are still assigned. + _records.Setup(r => r.AssignReviewerAsync(Dept, Exporter, "busy", "reviewer", "rotation", It.IsAny())).ThrowsAsync(new RecordConcurrencyException("busy", 3, 4)); + _records.Setup(r => r.AssignReviewerAsync(Dept, Exporter, "r3", "reviewer", "rotation", It.IsAny())).ReturnsAsync(new RecordAggregate()); + var conflicted = await _service.AssignForReviewAsync(Dept, Exporter, new RecordsBulkAssignRequest { RecordIds = new List { "r1", "busy", "r3" }, ReviewerUserId = "reviewer", Reason = "rotation" }); + conflicted.Processed.Should().Be(2); + conflicted.Skips.Select(s => s.RecordId + ":" + s.Reason).Should().BeEquivalentTo("busy:conflict"); } } } diff --git a/Web/Resgrid.Web/Areas/User/Views/Records/Index.cshtml b/Web/Resgrid.Web/Areas/User/Views/Records/Index.cshtml index 92a424618..44a3c66fa 100644 --- a/Web/Resgrid.Web/Areas/User/Views/Records/Index.cshtml +++ b/Web/Resgrid.Web/Areas/User/Views/Records/Index.cshtml @@ -235,7 +235,8 @@ @foreach (var r in Model.Records) { - @if (bulk) { } + @* Bulk assign and packets operate on operational Records only; Incident Reports have their own workflow. *@ + @if (bulk) { @if (r.RecordKind == (int)RmsRecordKind.Operational) { } } @if (!string.IsNullOrWhiteSpace(r.RecordNumber)) {