RG-T41 Fixing Postgres Profile error

Core/Resgrid.Services/Records/RecordsBulkPacketService.cs

@@ -167,6 +167,9 @@ public async Task<RecordsBulkResult> 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<RecordsBulkResult> 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;
 		}

Providers/Resgrid.Providers.Migrations/Migrations/M0231_FixDepartmentProfilesLogoGooglePlusColumns.cs

@@ -0,0 +1,36 @@
+using FluentMigrator;
+
+namespace Resgrid.Providers.Migrations.Migrations
+{
+	/// <summary>
+	/// 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.
+	/// </summary>
+	[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.
+		}
+	}
+}

Providers/Resgrid.Providers.MigrationsPg/Migrations/M0231_FixDepartmentProfilesLogoGooglePlusColumnsPg.cs

@@ -0,0 +1,43 @@
+using FluentMigrator;
+
+namespace Resgrid.Providers.MigrationsPg.Migrations
+{
+	/// <summary>
+	/// 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.
+	/// </summary>
+	[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.
+		}
+	}
+}

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<CancellationToken>())).ReturnsAsync(new RecordAggregate());
 			_records.Setup(r => r.AssignReviewerAsync(Dept, Exporter, "draft", "reviewer", "rotation", It.IsAny<CancellationToken>())).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<CancellationToken>())).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<CancellationToken>())).ThrowsAsync(new KeyNotFoundException("Record missing does not exist in this department."));
+			_records.Setup(r => r.AssignReviewerAsync(Dept, Exporter, "r2", "reviewer", "rotation", It.IsAny<CancellationToken>())).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<string> { "r1", "draft", "hidden" }, ReviewerUserId = "reviewer", Reason = "rotation" });
+			var result = await _service.AssignForReviewAsync(Dept, Exporter, new RecordsBulkAssignRequest { RecordIds = new List<string> { "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<Task> inactive = () => _service.AssignForReviewAsync(Dept, Exporter, new RecordsBulkAssignRequest { RecordIds = new List<string> { "r1" }, ReviewerUserId = "gone" });
 			await inactive.Should().ThrowAsync<ArgumentException>();
+
+			// A reviewer without ReviewRecords fails the whole request up front rather than skipping every row.
+			Func<Task> notReviewer = () => _service.AssignForReviewAsync(Dept, Exporter, new RecordsBulkAssignRequest { RecordIds = new List<string> { "r1" }, ReviewerUserId = "member" });
+			(await notReviewer.Should().ThrowAsync<ArgumentException>()).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<CancellationToken>())).ThrowsAsync(new ArgumentException("The chosen reviewer does not hold the ReviewRecords permission."));
+			var partial = await _service.AssignForReviewAsync(Dept, Exporter, new RecordsBulkAssignRequest { RecordIds = new List<string> { "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<CancellationToken>())).ThrowsAsync(new RecordConcurrencyException("busy", 3, 4));
+			_records.Setup(r => r.AssignReviewerAsync(Dept, Exporter, "r3", "reviewer", "rotation", It.IsAny<CancellationToken>())).ReturnsAsync(new RecordAggregate());
+			var conflicted = await _service.AssignForReviewAsync(Dept, Exporter, new RecordsBulkAssignRequest { RecordIds = new List<string> { "r1", "busy", "r3" }, ReviewerUserId = "reviewer", Reason = "rotation" });
+			conflicted.Processed.Should().Be(2);
+			conflicted.Skips.Select(s => s.RecordId + ":" + s.Reason).Should().BeEquivalentTo("busy:conflict");
 		}
 	}
 }

Web/Resgrid.Web/Areas/User/Views/Records/Index.cshtml

@@ -235,7 +235,8 @@
 									@foreach (var r in Model.Records)
 									{
 										<tr>
-											@if (bulk) { <td><input type="checkbox" name="ids" value="@r.RmsRecordSearchProjectionId" class="records-bulk-row" /></td> }
+											@* Bulk assign and packets operate on operational Records only; Incident Reports have their own workflow. *@
+											@if (bulk) { <td>@if (r.RecordKind == (int)RmsRecordKind.Operational) { <input type="checkbox" name="ids" value="@r.RmsRecordSearchProjectionId" class="records-bulk-row" /> }</td> }
 											<td>
 												@if (!string.IsNullOrWhiteSpace(r.RecordNumber))
 												{