Repository navigation
Conversation
|
Thanks for opening this, but we'd appreciate a little more information. Could you update it with more details? |
Contributor
|
Warning Review limit reached
This review includes 294 billable files and costs up to $73.50. View limit details
|
This comment has been minimized.
This comment has been minimized.
Resgrid-Bot
reviewed
Nov 1, 2026
| return value switch | ||
| { | ||
| int i => i, | ||
| long l => (int)l, |
There was a problem hiding this comment.
DashboardWidgetContext.cs and DashboardRepositories.cs cast long values directly to int, which can overflow or truncate values outside the Int32 range. Check that each value is between int.MinValue and int.MaxValue before casting, or preserve it as a long.
Kody rule violation: Prevent Numeric Overflow in Calculations
long l when l >= int.MinValue && l <= int.MaxValue => (int)l,Prompt for LLM
File Core/Resgrid.Model/Dashboard/DashboardWidgetContext.cs:
Line 105:
DashboardWidgetContext.cs and DashboardRepositories.cs cast long values directly to int, which can overflow or truncate values outside the Int32 range. Check that each value is between int.MinValue and int.MaxValue before casting, or preserve it as a long.
Suggested Code:
long l when l >= int.MinValue && l <= int.MaxValue => (int)l,
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
Resgrid-Bot
reviewed
Nov 1, 2026
| await RevealAsync(actor, schedule, display: true); | ||
| var metadata = JsonConvert.DeserializeObject<ChecklistOccurrence>(JsonConvert.SerializeObject(occurrence)); metadata.Content = null; | ||
| views.Add(new ChecklistOccurrenceView { Occurrence = metadata, Name = Decode<ChecklistScheduleContent>(schedule.Content).Name, Target = target, | ||
| views.Add(new ChecklistOccurrenceView { Occurrence = metadata, Name = Decode<ChecklistScheduleContent>(schedule.Content).Name, Target = target, Assigned = assigned, |
There was a problem hiding this comment.
ChecklistsScheduling.cs accesses Name on the result of Decode(schedule.Content) without handling a null decoded value, which can cause a NullReferenceException. Use null propagation and default Name to string.Empty.
Kody rule violation: Add null checks to prevent NullReferenceException
Name = Decode<ChecklistScheduleContent>(schedule.Content)?.Name ?? string.EmptyPrompt for LLM
File Core/Resgrid.Services/ChecklistsScheduling.cs:
Line 93:
ChecklistsScheduling.cs accesses Name on the result of Decode<ChecklistScheduleContent>(schedule.Content) without handling a null decoded value, which can cause a NullReferenceException. Use null propagation and default Name to string.Empty.
Suggested Code:
Name = Decode<ChecklistScheduleContent>(schedule.Content)?.Name ?? string.Empty
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
Resgrid-Bot
reviewed
Nov 1, 2026
Comment on lines
+229
to
+234
| row.LayoutJson = DashboardLayoutValidator.Serialize(resolved.Layout); | ||
| row.TemplateId = resolved.TemplateId; | ||
| row.UpdatedOn = DateTime.UtcNow; | ||
| await _userDashboardsRepository.UpdateWithRowVersionAsync(row, row.RowVersion, CancellationToken.None); | ||
| await InvalidateUserAsync(departmentId, userId); | ||
| return ToBoard(row); |
There was a problem hiding this comment.
ResetBoardAsync returns the old RowVersion after UpdateWithRowVersionAsync increments the stored version, causing the client’s next save to fail with a concurrency conflict despite no changes from another browser. Reload the row after the update or use the incremented RowVersion returned by the repository before constructing the result.
var updated = await _userDashboardsRepository.UpdateWithRowVersionAsync(row, row.RowVersion, CancellationToken.None);
if (!updated)
return await GetBoardAsync(departmentId, userId, viewer, boardId);
row = await _userDashboardsRepository.GetByIdForUserAsync(row.UserDashboardId, departmentId, userId);
await InvalidateUserAsync(departmentId, userId);
return ToBoard(row);Prompt for LLM
File Core/Resgrid.Services/Dashboard/DashboardService.cs:
Line 229 to 234:
ResetBoardAsync returns the old RowVersion after UpdateWithRowVersionAsync increments the stored version, causing the client’s next save to fail with a concurrency conflict despite no changes from another browser. Reload the row after the update or use the incremented RowVersion returned by the repository before constructing the result.
Suggested Code:
var updated = await _userDashboardsRepository.UpdateWithRowVersionAsync(row, row.RowVersion, CancellationToken.None);
if (!updated)
return await GetBoardAsync(departmentId, userId, viewer, boardId);
row = await _userDashboardsRepository.GetByIdForUserAsync(row.UserDashboardId, departmentId, userId);
await InvalidateUserAsync(departmentId, userId);
return ToBoard(row);
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
Resgrid-Bot
reviewed
Nov 1, 2026
Comment on lines
+26
to
+30
| Settings = new[] | ||
| { | ||
| Number("days", 7, 1, 14), | ||
| MultiFrom("types", PlanningOptionSources.CalendarTypes), | ||
| Number("maxItems", 10, 1, 25) |
There was a problem hiding this comment.
Planning widget settings reference DashboardOptionSources constants without registered IDashboardOptionSource implementations, leaving the Calendar type, document category, note category, and incident activation selectors without options and allowing posted values outside the advertised bootstrap choices. Add keyed providers for every referenced source, or remove those sources from DashboardWidgetCatalog.Planning.cs until providers exist.
// Register keyed IDashboardOptionSource implementations for CalendarTypes,
// DocumentCategories, NoteCategories, and IncidentActivations before exposing
// these settings in the catalog.Prompt for LLM
File Core/Resgrid.Services/Dashboard/DashboardWidgetCatalog.Planning.cs:
Line 26 to 30:
Planning widget settings reference DashboardOptionSources constants without registered IDashboardOptionSource implementations, leaving the Calendar type, document category, note category, and incident activation selectors without options and allowing posted values outside the advertised bootstrap choices. Add keyed providers for every referenced source, or remove those sources from DashboardWidgetCatalog.Planning.cs until providers exist.
Suggested Code:
// Register keyed IDashboardOptionSource implementations for CalendarTypes,
// DocumentCategories, NoteCategories, and IncidentActivations before exposing
// these settings in the catalog.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
Resgrid-Bot
reviewed
Nov 1, 2026
| if (context.Viewer?.IsDepartmentAdmin != true) | ||
| return data; | ||
|
|
||
| var latest = runs.FirstOrDefault(r => r.Status != (int)CommunicationTestRunStatus.Pending) ?? runs[0]; |
There was a problem hiding this comment.
CommunicationTestsWidgetProvider.cs has already established that runs is non-empty, so FirstOrDefault followed by runs[0] obscures the invariant and preserves an unnecessary fallback. Use First with the non-pending predicate, or document and explicitly retain the non-empty fallback semantics.
Kody rule violation: Use `First`/`Single` Instead of `FirstOrDefault`/`SingleOrDefault` for Non-Empty Collections
CommunicationTestRun latest = runs.First(r => r.Status != (int)CommunicationTestRunStatus.Pending);Prompt for LLM
File Core/Resgrid.Services/Dashboard/Widgets/Communications/CommunicationTestsWidgetProvider.cs:
Line 70:
CommunicationTestsWidgetProvider.cs has already established that runs is non-empty, so FirstOrDefault followed by runs[0] obscures the invariant and preserves an unnecessary fallback. Use First with the non-pending predicate, or document and explicitly retain the non-empty fallback semantics.
Suggested Code:
CommunicationTestRun latest = runs.First(r => r.Status != (int)CommunicationTestRunStatus.Pending);
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
Resgrid-Bot
reviewed
Nov 1, 2026
| var visible = new List<UnitState>(); | ||
| foreach (var state in states) | ||
| { | ||
| if (await authorizationService.CanUserViewUnitViaMatrixAsync(state.UnitId, userId, departmentId)) |
There was a problem hiding this comment.
DashboardUnitReads.cs calls CanUserViewUnitViaMatrixAsync once per state inside the loop, creating one authorization request per unit. Use a batch authorization API or retrieve the permitted unit IDs once and filter locally.
Kody rule violation: Detect N+1 style queries and suggest batching
Prompt for LLM
File Core/Resgrid.Services/Dashboard/Widgets/Operations/DashboardUnitReads.cs:
Line 32:
DashboardUnitReads.cs calls CanUserViewUnitViaMatrixAsync once per state inside the loop, creating one authorization request per unit. Use a batch authorization API or retrieve the permitted unit IDs once and filter locally.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
Resgrid-Bot
reviewed
Nov 1, 2026
| public const int MaxLabelLength = 100; | ||
|
|
||
| /// <summary>"[label |] http(s)://…" — the URL is the line's last token, so a label may itself contain "|".</summary> | ||
| private static readonly Regex LinePattern = new Regex(@"^(?:(?<label>.*?)\s*\|\s*)?(?<url>https?://\S+)$", |
There was a problem hiding this comment.
QuickLinksWidgetProvider.cs and DashboardCatalogGuardTests.cs construct Regex instances without a timeout, allowing malicious input to consume excessive processing time. Define an explicit Regex timeout for every affected pattern.
Kody rule violation: Specify Timeout for Regular Expressions
Prompt for LLM
File Core/Resgrid.Services/Dashboard/Widgets/Planning/QuickLinksWidgetProvider.cs:
Line 29:
QuickLinksWidgetProvider.cs and DashboardCatalogGuardTests.cs construct Regex instances without a timeout, allowing malicious input to consume excessive processing time. Define an explicit Regex timeout for every affected pattern.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
Resgrid-Bot
reviewed
Nov 1, 2026
Comment on lines
+36
to
+47
| OutOfServiceHydrants = (summary.OutOfServiceHydrants ?? new List<RmsHydrant>()) | ||
| .Where(h => h != null && h.DepartmentId == context.DepartmentId && h.DeletedOn == null && !h.InService) | ||
| .Take(maxRows) | ||
| .Select(h => new RecordsHydrantItem | ||
| { | ||
| HydrantId = h.RmsHydrantId, | ||
| Number = RecordsWidgetSupport.SafeText(h.HydrantNumber), | ||
| Type = h.Type, | ||
| Latitude = h.Latitude, | ||
| Longitude = h.Longitude, | ||
| OutOfServiceSince = h.OutOfServiceSince | ||
| }).ToList() |
There was a problem hiding this comment.
RecordsHydrantsWidgetProvider.cs and the listed dashboard providers chain filtering, limiting, projection, and materialization in a single LINQ expression, making each stage harder to inspect and maintain. Store the source sequence, filtered and limited sequence, and projected materialized results in named intermediate variables.
Kody rule violation: Limit Lengthy LINQ Chains
var hydrants = summary.OutOfServiceHydrants ?? new List<RmsHydrant>();
var outOfServiceHydrants = hydrants.Where(h => h != null && h.DepartmentId == context.DepartmentId && h.DeletedOn == null && !h.InService).Take(maxRows);
var hydrantItems = outOfServiceHydrants.Select(h => new RecordsHydrantItem
{
HydrantId = h.RmsHydrantId,
Number = RecordsWidgetSupport.SafeText(h.HydrantNumber),
Type = h.Type,
Latitude = h.Latitude,
Longitude = h.Longitude,
OutOfServiceSince = h.OutOfServiceSince
}).ToList();Prompt for LLM
File Core/Resgrid.Services/Dashboard/Widgets/Records/RecordsHydrantsWidgetProvider.cs:
Line 36 to 47:
RecordsHydrantsWidgetProvider.cs and the listed dashboard providers chain filtering, limiting, projection, and materialization in a single LINQ expression, making each stage harder to inspect and maintain. Store the source sequence, filtered and limited sequence, and projected materialized results in named intermediate variables.
Suggested Code:
var hydrants = summary.OutOfServiceHydrants ?? new List<RmsHydrant>();
var outOfServiceHydrants = hydrants.Where(h => h != null && h.DepartmentId == context.DepartmentId && h.DeletedOn == null && !h.InService).Take(maxRows);
var hydrantItems = outOfServiceHydrants.Select(h => new RecordsHydrantItem
{
HydrantId = h.RmsHydrantId,
Number = RecordsWidgetSupport.SafeText(h.HydrantNumber),
Type = h.Type,
Latitude = h.Latitude,
Longitude = h.Longitude,
OutOfServiceSince = h.OutOfServiceSince
}).ToList();
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
Resgrid-Bot
reviewed
Nov 1, 2026
| builder.RegisterAssemblyTypes(assembly) | ||
| .Where(t => typeof(IDashboardWidgetDataProvider).IsAssignableFrom(t) && !t.IsAbstract && | ||
| System.Reflection.CustomAttributeExtensions.GetCustomAttribute<Resgrid.Model.Dashboard.DashboardWidgetAttribute>(t) != null) | ||
| .Keyed<IDashboardWidgetDataProvider>(t => System.Reflection.CustomAttributeExtensions.GetCustomAttribute<Resgrid.Model.Dashboard.DashboardWidgetAttribute>(t).Key) |
There was a problem hiding this comment.
ServicesModule.cs accesses DashboardWidgetAttribute.Key through a separate reflection lookup that can return null despite the earlier predicate. Use null propagation with an explicit InvalidOperationException fallback when the DashboardWidgetAttribute is absent.
Kody rule violation: Add null checks before accessing properties
.Keyed<IDashboardWidgetDataProvider>(t => System.Reflection.CustomAttributeExtensions.GetCustomAttribute<Resgrid.Model.Dashboard.DashboardWidgetAttribute>(t)?.Key ?? throw new InvalidOperationException("Dashboard widget attribute is required."))Prompt for LLM
File Core/Resgrid.Services/ServicesModule.cs:
Line 601:
ServicesModule.cs accesses DashboardWidgetAttribute.Key through a separate reflection lookup that can return null despite the earlier predicate. Use null propagation with an explicit InvalidOperationException fallback when the DashboardWidgetAttribute is absent.
Suggested Code:
.Keyed<IDashboardWidgetDataProvider>(t => System.Reflection.CustomAttributeExtensions.GetCustomAttribute<Resgrid.Model.Dashboard.DashboardWidgetAttribute>(t)?.Key ?? throw new InvalidOperationException("Dashboard widget attribute is required."))
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
Resgrid-Bot
reviewed
Nov 1, 2026
| // minimum; the questions are loaded for that case alone. | ||
| if (assignment.Score < training.MinimumScore) | ||
| { | ||
| var questions = await _trainingQuestionRepository.GetTrainingQuestionsByTrainingIdAsync(training.TrainingId); |
There was a problem hiding this comment.
TrainingService.cs queries _trainingQuestionRepository once per training inside the loop, causing an avoidable N+1 query pattern. Batch-load questions with GetTrainingQuestionsByTrainingIdsAsync for the relevant training IDs or eager-load them with the initial query.
Kody rule violation: Optimize database queries with JOINs
var trainingIds = trainings.Where(t => t != null && t.Score < t.MinimumScore).Select(t => t.TrainingId).ToList();
var questionsByTrainingId = await _trainingQuestionRepository.GetTrainingQuestionsByTrainingIdsAsync(trainingIds);Prompt for LLM
File Core/Resgrid.Services/TrainingService.cs:
Line 374:
TrainingService.cs queries _trainingQuestionRepository once per training inside the loop, causing an avoidable N+1 query pattern. Batch-load questions with GetTrainingQuestionsByTrainingIdsAsync for the relevant training IDs or eager-load them with the initial query.
Suggested Code:
var trainingIds = trainings.Where(t => t != null && t.Score < t.MinimumScore).Select(t => t.TrainingId).ToList();
var questionsByTrainingId = await _trainingQuestionRepository.GetTrainingQuestionsByTrainingIdsAsync(trainingIds);
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
Resgrid-Bot
reviewed
Nov 1, 2026
| // Seeded OFF: every member keeps the classic dashboard until the flag is enabled globally or per department. | ||
| Execute.Sql( | ||
| "IF NOT EXISTS (SELECT 1 FROM [FeatureFlags] WHERE [FlagKey] = '" + FlagKey + "') " + | ||
| "INSERT INTO [FeatureFlags] ([FlagKey], [Name], [Description], [Category], [IsEnabledGlobally]) " + |
There was a problem hiding this comment.
The SQL statements in M0307_AddHomeDashboards.cs, M0307_AddHomeDashboardsPg.cs, CallsRepository.Dashboard.cs, DocumentRepository.cs, RouteInstancesRepository.cs, and the referenced tests must not interpolate unsanitized input into queries. Use parameterized queries for values supplied by users or external input.
Kody rule violation: Prevent SQL Injection in Queries
Prompt for LLM
File Providers/Resgrid.Providers.Migrations/Migrations/M0307_AddHomeDashboards.cs:
Line 60:
The SQL statements in M0307_AddHomeDashboards.cs, M0307_AddHomeDashboardsPg.cs, CallsRepository.Dashboard.cs, DocumentRepository.cs, RouteInstancesRepository.cs, and the referenced tests must not interpolate unsanitized input into queries. Use parameterized queries for values supplied by users or external input.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
Resgrid-Bot
reviewed
Nov 1, 2026
| .WithColumn("UpdatedByUserId").AsString(128).Nullable() | ||
| .WithColumn("UpdatedOn").AsDateTime2().NotNullable(); | ||
| Execute.Sql("CREATE NONCLUSTERED INDEX IX_DepartmentDashboardTemplates_Department ON DepartmentDashboardTemplates (DepartmentId);"); | ||
| Execute.Sql("CREATE UNIQUE NONCLUSTERED INDEX IX_DepartmentDashboardTemplates_Default ON DepartmentDashboardTemplates (DepartmentId) WHERE IsDefault = 1;"); |
There was a problem hiding this comment.
The filtered unique index in M0307_AddHomeDashboards.cs can acquire significant locks during migration, and the target database may not support online creation for filtered indexes. Use an online, low-locking strategy where supported, verify compatibility, and document the rollback plan.
Kody rule violation: Block risky database migrations (locking ops, downtime risk)
Execute.Sql("CREATE UNIQUE NONCLUSTERED INDEX IX_DepartmentDashboardTemplates_Default ON DepartmentDashboardTemplates (DepartmentId) WHERE IsDefault = 1 WITH (ONLINE = ON);");Prompt for LLM
File Providers/Resgrid.Providers.Migrations/Migrations/M0307_AddHomeDashboards.cs:
Line 54:
The filtered unique index in M0307_AddHomeDashboards.cs can acquire significant locks during migration, and the target database may not support online creation for filtered indexes. Use an online, low-locking strategy where supported, verify compatibility, and document the rollback plan.
Suggested Code:
Execute.Sql("CREATE UNIQUE NONCLUSTERED INDEX IX_DepartmentDashboardTemplates_Default ON DepartmentDashboardTemplates (DepartmentId) WHERE IsDefault = 1 WITH (ONLINE = ON);");
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
Resgrid-Bot
reviewed
Nov 1, 2026
| { | ||
| throw; | ||
| } | ||
| catch (Exception ex) |
There was a problem hiding this comment.
CallsRepository.Dashboard.cs and the listed repositories catch every Exception uniformly, preventing safe retries for transient database failures and losing specific handling for unrecoverable errors. Catch DbException selectively with IsTransient for safe, idempotent retries, and map non-transient failures with operation context.
Kody rule violation: Implement proper database error checking
catch (DbException ex) when (IsTransient(ex))
{
// Retry only safe, idempotent reads.
}
catch (DbException ex)
{
// Map non-transient database failures with operation context.
}Prompt for LLM
File Repositories/Resgrid.Repositories.DataRepository/CallsRepository.Dashboard.cs:
Line 51:
CallsRepository.Dashboard.cs and the listed repositories catch every Exception uniformly, preventing safe retries for transient database failures and losing specific handling for unrecoverable errors. Catch DbException selectively with IsTransient for safe, idempotent retries, and map non-transient failures with operation context.
Suggested Code:
catch (DbException ex) when (IsTransient(ex))
{
// Retry only safe, idempotent reads.
}
catch (DbException ex)
{
// Map non-transient database failures with operation context.
}
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
Resgrid-Bot
reviewed
Nov 1, 2026
| var p = RmsPreventionSql.Paged(departmentId, 0, take, 200); | ||
| p.Add("Off", false); | ||
| // NULL ordering differs by dialect (SQL Server first, PostgreSQL last), so an unknown start date is placed explicitly. | ||
| return QueryAsync<RmsHydrant>($"SELECT * FROM {Tbl("RmsHydrants")} WHERE {Col("DepartmentId")} = {P}DepartmentId AND {Col("DeletedOn")} IS NULL AND {Col("InService")} = {P}Off ORDER BY CASE WHEN {Col("OutOfServiceSince")} IS NULL THEN 1 ELSE 0 END, {Col("OutOfServiceSince")}, {Col("HydrantNumber")}, {Col("RmsHydrantId")} {Paging()}", p); |
There was a problem hiding this comment.
RmsPreventionRepositories.cs and the listed repository call sites allow asynchronous database failures to escape without operation or department context. Make the method async, await QueryAsync, catch the database failure, and rethrow an application-level exception such as DataException with the department context while passing parameters rather than the existing p variable.
Kody rule violation: Add try-catch blocks for external calls
try
{
return await QueryAsync<RmsHydrant>($"SELECT * FROM {Tbl("RmsHydrants")} WHERE {Col("DepartmentId")} = {P}DepartmentId AND {Col("DeletedOn")} IS NULL AND {Col("InService")} = {P}Off ORDER BY CASE WHEN {Col("OutOfServiceSince")} IS NULL THEN 1 ELSE 0 END, {Col("OutOfServiceSince")}, {Col("HydrantNumber")}, {Col("RmsHydrantId")} {Paging()}", parameters);
}
catch (Exception ex)
{
throw new DataException($"Failed to retrieve out-of-service hydrants for department {departmentId}.", ex);
}Prompt for LLM
File Repositories/Resgrid.Repositories.DataRepository/RmsPreventionRepositories.cs:
Line 444:
RmsPreventionRepositories.cs and the listed repository call sites allow asynchronous database failures to escape without operation or department context. Make the method async, await QueryAsync, catch the database failure, and rethrow an application-level exception such as DataException with the department context while passing parameters rather than the existing p variable.
Suggested Code:
try
{
return await QueryAsync<RmsHydrant>($"SELECT * FROM {Tbl("RmsHydrants")} WHERE {Col("DepartmentId")} = {P}DepartmentId AND {Col("DeletedOn")} IS NULL AND {Col("InService")} = {P}Off ORDER BY CASE WHEN {Col("OutOfServiceSince")} IS NULL THEN 1 ELSE 0 END, {Col("OutOfServiceSince")}, {Col("HydrantNumber")}, {Col("RmsHydrantId")} {Paging()}", parameters);
}
catch (Exception ex)
{
throw new DataException($"Failed to retrieve out-of-service hydrants for department {departmentId}.", ex);
}
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
Resgrid-Bot
reviewed
Nov 1, 2026
| // stylesheet, and require every cell above the last card's bottom edge to be filled. | ||
| var css = System.IO.File.ReadAllText(Path.Combine(RepositoryRoot(), "Web", "Resgrid.Web", "Areas", "User", "Apps", "src", "components", "dashboard", "dashboard.css")); | ||
| var spans = Regex.Matches(css, @"\.rg-w\[data-size='(\w+)'\]\s*\{\s*grid-column:\s*span (\d+);\s*grid-row:\s*span (\d+);") | ||
| .GroupBy(m => m.Groups[1].Value).ToDictionary(g => g.Key, g => (Columns: int.Parse(g.First().Groups[2].Value), Rows: int.Parse(g.First().Groups[3].Value))); |
There was a problem hiding this comment.
DashboardCatalogGuardTests.cs uses int.Parse on regex-captured strings, so malformed or unexpected input can throw a FormatException. Use an int.TryParse-style conversion with explicit culture and format validation before constructing the Columns and Rows values.
Kody rule violation: Use TryParse for string conversions
Prompt for LLM
File Tests/Resgrid.Tests/Dashboard/DashboardCatalogGuardTests.cs:
Line 157:
DashboardCatalogGuardTests.cs uses int.Parse on regex-captured strings, so malformed or unexpected input can throw a FormatException. Use an int.TryParse-style conversion with explicit culture and format validation before constructing the Columns and Rows values.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
Resgrid-Bot
reviewed
Nov 1, 2026
| var data = (MapVisualizationWidgetData)await provider.GetDataAsync(For("map.visualization"), CancellationToken.None); | ||
|
|
||
| data.Configured.Should().BeFalse(); | ||
| data.Result.Should().BeNull(); |
There was a problem hiding this comment.
MapVisualizationWidgetProviderTests.cs blocks on asynchronous work through data.Result, and the related MapVisualizationWidget.tsx paths contain the same blocking pattern. Replace .Result and .Wait() usage with await so asynchronous execution cannot deadlock.
Kody rule violation: Avoid Blocking Calls to Async Methods
Prompt for LLM
File Tests/Resgrid.Tests/Dashboard/Widgets/MapVisualizationWidgetProviderTests.cs:
Line 27:
MapVisualizationWidgetProviderTests.cs blocks on asynchronous work through data.Result, and the related MapVisualizationWidget.tsx paths contain the same blocking pattern. Replace .Result and .Wait() usage with await so asynchronous execution cannot deadlock.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
Resgrid-Bot
reviewed
Nov 1, 2026
| var data = (MapVisualizationWidgetData)await provider.GetDataAsync(For("map.visualization"), CancellationToken.None); | ||
|
|
||
| data.Configured.Should().BeFalse(); | ||
| data.Result.Should().BeNull(); |
There was a problem hiding this comment.
MapVisualizationWidgetProviderTests.cs blocks on the asynchronous result through data.Result, and the related MapVisualizationWidget.tsx paths have the same synchronous access pattern. Await the underlying Tasks end to end and use configure awaits appropriately instead of blocking.
Kody rule violation: Await async operations properly
Prompt for LLM
File Tests/Resgrid.Tests/Dashboard/Widgets/MapVisualizationWidgetProviderTests.cs:
Line 27:
MapVisualizationWidgetProviderTests.cs blocks on the asynchronous result through data.Result, and the related MapVisualizationWidget.tsx paths have the same synchronous access pattern. Await the underlying Tasks end to end and use configure awaits appropriately instead of blocking.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
Resgrid-Bot
reviewed
Nov 1, 2026
| departments.Setup(x => x.GetDepartmentByIdAsync(DepartmentId, It.IsAny<bool>())).ReturnsAsync(new Department { DepartmentId = DepartmentId }); | ||
| var geo = new Mock<IGeoLocationProvider>(MockBehavior.Strict); | ||
|
|
||
| var provider = new WeatherCurrentWidgetProvider(new HttpClient(handler), cache.Object, settings.Object, departments.Object, geo.Object); |
There was a problem hiding this comment.
WeatherCurrentWidgetProviderTests.cs creates an HttpClient without clearly disposing its disposable resource, which can leak handlers across tests. Dispose the HttpClient deterministically with using var or make its ownership explicit.
Kody rule violation: Use using statements for disposable resources
using var client = new HttpClient(handler);
var provider = new WeatherCurrentWidgetProvider(client, cache.Object, settings.Object, departments.Object, geo.Object);Prompt for LLM
File Tests/Resgrid.Tests/Dashboard/Widgets/WeatherCurrentWidgetProviderTests.cs:
Line 96:
WeatherCurrentWidgetProviderTests.cs creates an HttpClient without clearly disposing its disposable resource, which can leak handlers across tests. Dispose the HttpClient deterministically with using var or make its ownership explicit.
Suggested Code:
using var client = new HttpClient(handler);
var provider = new WeatherCurrentWidgetProvider(client, cache.Object, settings.Object, departments.Object, geo.Object);
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
Resgrid-Bot
reviewed
Nov 1, 2026
| if (type == DatabaseTypes.Postgres) r.AddPostgres(); else r.AddSqlServer(); | ||
| r.WithGlobalConnectionString(_connection); | ||
| }).AddSingleton(source.Object).BuildServiceProvider(); | ||
| _runner.GetRequiredService<IMigrationRunner>().MigrateUp(); |
There was a problem hiding this comment.
DashboardRepositoriesDatabaseTests.cs blocks the async migration setup by calling MigrateUp synchronously. Use the migration runner’s awaitable MigrateUpAsync API inside the async setup method.
Kody rule violation: Use Awaitable Methods in Async Code
await _runner.GetRequiredService<IMigrationRunner>().MigrateUpAsync();Prompt for LLM
File Tests/Resgrid.Tests/Repositories/DashboardRepositoriesDatabaseTests.cs:
Line 89:
DashboardRepositoriesDatabaseTests.cs blocks the async migration setup by calling MigrateUp synchronously. Use the migration runner’s awaitable MigrateUpAsync API inside the async setup method.
Suggested Code:
await _runner.GetRequiredService<IMigrationRunner>().MigrateUpAsync();
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
Resgrid-Bot
reviewed
Nov 1, 2026
| <div className="rg-dash" aria-busy="true"> | ||
| <div className="rg-grid rg-grid-loading"> | ||
| {['s', 's', 's', 's', 'l', 'l', 'full'].map((size, index) => ( | ||
| <div key={index} className="rg-w rg-w-placeholder" data-size={size}> |
There was a problem hiding this comment.
DashboardElement.tsx and numbers.tsx use array indexes as React keys, which can associate state with the wrong element when dashboard items reorder or change. Use stable, unique identifiers for each list item.
Kody rule violation: Avoid array indexes as keys in React lists
Prompt for LLM
File Web/Resgrid.Web/Areas/User/Apps/src/components/dashboard/DashboardElement.tsx:
Line 946:
DashboardElement.tsx and numbers.tsx use array indexes as React keys, which can associate state with the wrong element when dashboard items reorder or change. Use stable, unique identifiers for each list item.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
Resgrid-Bot
reviewed
Nov 1, 2026
| <i className="fa fa-th-large" aria-hidden="true" /> | ||
| <b>{editing ? t('Dash.EmptyBoardEditing') : t('Dash.EmptyBoard')}</b> | ||
| {editing ? ( | ||
| <button type="button" className="btn btn-sm btn-primary" onClick={() => setGalleryOpen(true)}> |
There was a problem hiding this comment.
DashboardElement.tsx and the listed dashboard components create inline arrow functions or use .bind() in JSX props, allocating new callbacks on every render and potentially increasing child re-renders. Define stable handlers outside JSX, using memoization where parameters are required.
Kody rule violation: Avoid using .bind() or arrow functions in JSX props
Prompt for LLM
File Web/Resgrid.Web/Areas/User/Apps/src/components/dashboard/DashboardElement.tsx:
Line 969:
DashboardElement.tsx and the listed dashboard components create inline arrow functions or use .bind() in JSX props, allocating new callbacks on every render and potentially increasing child re-renders. Define stable handlers outside JSX, using memoization where parameters are required.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
Resgrid-Bot
reviewed
Nov 1, 2026
| const loadInitial = useCallback(() => { | ||
| setBootError(false); | ||
| loadBootstrap(null, 'initial').catch((error: unknown) => { | ||
| console.error('Dashboard: the bootstrap failed to load.', error); |
There was a problem hiding this comment.
DashboardElement.tsx logs bootstrap failures with console.error without structured operation context, making failures difficult to correlate across dashboard instances. Replace it with a structured error log such as logger.error('dashboard bootstrap failed', { operation: 'bootstrap', boardId: null, templateId: props.templateId, error }), and apply the same approach to the listed call sites.
Kody rule violation: Include error context in structured logs
Prompt for LLM
File Web/Resgrid.Web/Areas/User/Apps/src/components/dashboard/DashboardElement.tsx:
Line 236:
DashboardElement.tsx logs bootstrap failures with console.error without structured operation context, making failures difficult to correlate across dashboard instances. Replace it with a structured error log such as logger.error('dashboard bootstrap failed', { operation: 'bootstrap', boardId: null, templateId: props.templateId, error }), and apply the same approach to the listed call sites.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
Resgrid-Bot
reviewed
Nov 1, 2026
| 'incidentCommand', | ||
| ] as const; | ||
|
|
||
| const RETRY_DELAYS_MS = [2000, 5000, 10000, 30000, 60000]; |
There was a problem hiding this comment.
dashboardBus.ts exposes RETRY_DELAYS_MS as a mutable array, allowing retry timing to change at runtime. Mark the collection as immutable with as const.
Kody rule violation: Use `readonly` or `const` for Immutable Data
const RETRY_DELAYS_MS = [2000, 5000, 10000, 30000, 60000] as const;Prompt for LLM
File Web/Resgrid.Web/Areas/User/Apps/src/components/dashboard/data/dashboardBus.ts:
Line 26:
dashboardBus.ts exposes RETRY_DELAYS_MS as a mutable array, allowing retry timing to change at runtime. Mark the collection as immutable with as const.
Suggested Code:
const RETRY_DELAYS_MS = [2000, 5000, 10000, 30000, 60000] as const;
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
Resgrid-Bot
reviewed
Nov 1, 2026
| } | ||
| pumpScheduled = true; | ||
| // A macrotask, not requestAnimationFrame: a hidden tab or preview pane stalls rAF. | ||
| setTimeout(pump, 0); |
There was a problem hiding this comment.
scheduler.ts schedules pump with setTimeout without retaining the timeout handle, so scheduled work can survive scheduler teardown. Store the handle and call clearTimeout(timeoutId) during cleanup.
Kody rule violation: Clear timers on teardown/unmount
const timeoutId = setTimeout(pump, 0);
// Expose or invoke clearTimeout(timeoutId) during scheduler teardown.Prompt for LLM
File Web/Resgrid.Web/Areas/User/Apps/src/components/dashboard/data/scheduler.ts:
Line 56:
scheduler.ts schedules pump with setTimeout without retaining the timeout handle, so scheduled work can survive scheduler teardown. Store the handle and call clearTimeout(timeoutId) during cleanup.
Suggested Code:
const timeoutId = setTimeout(pump, 0);
// Expose or invoke clearTimeout(timeoutId) during scheduler teardown.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
Resgrid-Bot
reviewed
Nov 1, 2026
Comment on lines
+260
to
+266
| if (shownMarkers.length > 0) { | ||
| if (fit) { | ||
| key = `m:${markers | ||
| .map((marker) => marker.id) | ||
| .sort() | ||
| .join('|')}`; | ||
| target = { bounds: L.latLngBounds(shownMarkers.map((marker) => [marker.lat, marker.lng] as [number, number])).pad(0.15) }; |
There was a problem hiding this comment.
MiniMapView.tsx derives the auto-fit guard key from all markers while deriving bounds from shownMarkers, so layer toggles can leave the key unchanged and prevent applyFit from updating the viewport. Build the key from shownMarkers, including the visible marker IDs used for the bounds.
key = `m:${shownMarkers
.map((marker) => marker.id)
.sort()
.join('|')}`;
target = { bounds: L.latLngBounds(shownMarkers.map((marker) => [marker.lat, marker.lng] as [number, number])).pad(0.15) };Prompt for LLM
File Web/Resgrid.Web/Areas/User/Apps/src/components/dashboard/primitives/MiniMapView.tsx:
Line 260 to 266:
MiniMapView.tsx derives the auto-fit guard key from all markers while deriving bounds from shownMarkers, so layer toggles can leave the key unchanged and prevent applyFit from updating the viewport. Build the key from shownMarkers, including the visible marker IDs used for the bounds.
Suggested Code:
key = `m:${shownMarkers
.map((marker) => marker.id)
.sort()
.join('|')}`;
target = { bounds: L.latLngBounds(shownMarkers.map((marker) => [marker.lat, marker.lng] as [number, number])).pad(0.15) };
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
Resgrid-Bot
reviewed
Nov 1, 2026
| import type { Tone } from '../../primitives'; | ||
| import type { WidgetProps } from '../types'; | ||
| import { ActivationHeader, deskUrl, type IncidentArrivalWindow, type IncidentWidgetDataBase, NoActivation, ReadOnlyNote, compactCurrency, wholeCurrency } from './iboShared'; | ||
| import './ibo.css'; |
There was a problem hiding this comment.
IncidentDeskWidget.tsx and the listed non-layout dashboard components import global styles such as ibo.css, allowing selectors to leak into unrelated components. Use a component-scoped stylesheet such as ibo.module.css with CSS Modules or equivalent scoped/BEM classes.
Kody rule violation: Use component-scoped styling
import styles from './ibo.module.css';Prompt for LLM
File Web/Resgrid.Web/Areas/User/Apps/src/components/dashboard/widgets/incidentBackOffice/IncidentDeskWidget.tsx:
Line 5:
IncidentDeskWidget.tsx and the listed non-layout dashboard components import global styles such as ibo.css, allowing selectors to leak into unrelated components. Use a component-scoped stylesheet such as ibo.module.css with CSS Modules or equivalent scoped/BEM classes.
Suggested Code:
import styles from './ibo.module.css';
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
Resgrid-Bot
reviewed
Nov 1, 2026
| @@ -110,6 +120,23 @@ export async function connectEventingHub(departmentId: number, handlers: Eventin | |||
| connection.on('callAdded', (id: unknown) => handlers.onCallsChanged?.(asId(id))); | |||
| connection.on('callClosed', (id: unknown) => handlers.onCallsChanged?.(asId(id))); | |||
|
|
|||
| // Registered only when asked for, so elements that predate these events see no change in hub traffic handling. | |||
| if (handlers.onPersonnelStaffingUpdated) { | |||
| connection.on('personnelStaffingUpdated', (id: unknown) => handlers.onPersonnelStaffingUpdated?.(asId(id))); | |||
There was a problem hiding this comment.
signalr.ts registers the personnelStaffingUpdated listener without an error-handling path or deterministic cleanup, so the callback can remain attached after the connection is torn down. Store the handler, remove it with connection.off during cleanup, and register connection.onclose(onError).
Kody rule violation: Provide error handlers to subscription/listener APIs
const onPersonnelStaffingUpdated = (id: unknown) => handlers.onPersonnelStaffingUpdated?.(asId(id));
connection.on('personnelStaffingUpdated', onPersonnelStaffingUpdated);
cleanup.push(() => connection.off('personnelStaffingUpdated', onPersonnelStaffingUpdated));
connection.onclose(onError);Prompt for LLM
File Web/Resgrid.Web/Areas/User/Apps/src/runtime/signalr.ts:
Line 125:
signalr.ts registers the personnelStaffingUpdated listener without an error-handling path or deterministic cleanup, so the callback can remain attached after the connection is torn down. Store the handler, remove it with connection.off during cleanup, and register connection.onclose(onError).
Suggested Code:
const onPersonnelStaffingUpdated = (id: unknown) => handlers.onPersonnelStaffingUpdated?.(asId(id));
connection.on('personnelStaffingUpdated', onPersonnelStaffingUpdated);
cleanup.push(() => connection.off('personnelStaffingUpdated', onPersonnelStaffingUpdated));
connection.onclose(onError);
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
Resgrid-Bot
reviewed
Nov 1, 2026
| { | ||
| // The widget-based dashboard (home-dashboard-redesign-plan.md D10): behind Dashboard.Modern, unless the member chose | ||
| // the classic page (or asked for it once with ?classic=true). | ||
| var modernEnabled = await _featureToggleService.IsEnabledAsync(FeatureFlagKeys.DashboardModern, DepartmentId); |
There was a problem hiding this comment.
HomeController.cs and the listed feature-toggle call sites allow IsEnabledAsync and other awaited operations to propagate service failures without logging their operation context or applying a fallback. Wrap the awaited operation in try/catch, log the relevant context, and apply an appropriate fallback or application-level error.
Kody rule violation: Handle async operations with proper error handling
bool modernEnabled;
try
{
modernEnabled = await _featureToggleService.IsEnabledAsync(FeatureFlagKeys.DashboardModern, DepartmentId);
}
catch (Exception ex)
{
// Log and apply the appropriate fallback.
throw;
}Prompt for LLM
File Web/Resgrid.Web/Areas/User/Controllers/HomeController.cs:
Line 182:
HomeController.cs and the listed feature-toggle call sites allow IsEnabledAsync and other awaited operations to propagate service failures without logging their operation context or applying a fallback. Wrap the awaited operation in try/catch, log the relevant context, and apply an appropriate fallback or application-level error.
Suggested Code:
bool modernEnabled;
try
{
modernEnabled = await _featureToggleService.IsEnabledAsync(FeatureFlagKeys.DashboardModern, DepartmentId);
}
catch (Exception ex)
{
// Log and apply the appropriate fallback.
throw;
}
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
Resgrid-Bot
reviewed
Nov 1, 2026
| data: JSON.stringify({ UseClassic: false }), | ||
| headers: { 'RequestVerificationToken': getAntiForgeryToken() } | ||
| }).done(function (result) { | ||
| window.location.href = (result && result.Redirect) ? result.Redirect : '/User/Home/Dashboard'; |
There was a problem hiding this comment.
resgrid.home.dashboard.js and DashboardElement.tsx assign the server-provided Redirect directly to window.location.href, allowing an open redirect if the value targets an external origin. Validate the redirect against an allowlist or same-origin path before assigning it, and fall back to /User/Home/Dashboard.
Kody rule violation: Avoid unprotected HTTP request redirections
const redirect = result?.Redirect;
const target = isAllowedSameOriginPath(redirect) ? redirect : '/User/Home/Dashboard';
window.location.href = target;Prompt for LLM
File Web/Resgrid.Web/wwwroot/js/app/internal/home/resgrid.home.dashboard.js:
Line 62:
resgrid.home.dashboard.js and DashboardElement.tsx assign the server-provided Redirect directly to window.location.href, allowing an open redirect if the value targets an external origin. Validate the redirect against an allowlist or same-origin path before assigning it, and fall back to /User/Home/Dashboard.
Suggested Code:
const redirect = result?.Redirect;
const target = isAllowedSameOriginPath(redirect) ? redirect : '/User/Home/Dashboard';
window.location.href = target;
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
Kody Review CompleteGreat news! 🎉 Keep up the excellent work! 🚀 Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
Member
Author
|
Approve |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Resgrid-Bot
approved these changes
Nov 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements RG-T151, introducing a feature-flagged modern home dashboard while preserving the existing classic dashboard experience.
Modern dashboard
Dashboard.Modernfeature flag, seeded off by default and evaluated per department.Dashboard templates
DashboardTemplateChangedsystem audit events.Widget catalog and module coverage
Adds a catalog and provider model for dashboard widgets across Operations, Personnel, Map & Weather, Communications, Planning, Readiness, Records, Workforce & Business, Incident Back Office, Intelligence, and Administration.
Widgets expose module-consistent data and access behavior for areas including:
Security and entitlement handling
Supporting services and persistence
UserDashboardsandDepartmentDashboardTemplatespersistence models, repositories, and SQL Server/PostgreSQL migrations.Additional fix
Testing
Adds coverage for dashboard catalog validation, entitlement and add-on gating, layout normalization, board/template resolution, concurrency, persistence, widget providers, protected data behavior, localization completeness, controller security attributes, and route lookup caching.