Conversation
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
|
Thanks for opening this, but we'd appreciate a little more information. Could you update it with more details? |
📝 WalkthroughWalkthroughCommand lanes now support configurable amber and red work-time thresholds. Values flow from templates into command definitions and structure nodes, are persisted through SQL and PostgreSQL migrations, and round-trip through the v4 command API. ChangesLane threshold configuration and propagation
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Web/Resgrid.Web.Services/Models/v4/Commands/CommandModels.cs`:
- Around line 115-116: Update WorkTimeAmberMinutes and WorkTimeRedMinutes in the
command request model to nullable inputs, then adjust SaveCommand to retain each
stored threshold when its corresponding value is omitted while still persisting
an explicitly supplied 0 as the disable value.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 21d04f33-2967-498b-997c-c9a02d75d168
📒 Files selected for processing (10)
Core/Resgrid.Model/CommandBoards/CommandBoardTemplate.csCore/Resgrid.Model/CommandBoards/CommandBoardTemplateCatalog.csCore/Resgrid.Model/CommandBoards/CommandBoardTemplateLane.csCore/Resgrid.Model/CommandDefinitionRole.csCore/Resgrid.Model/IncidentCommand/CommandStructureNode.csCore/Resgrid.Services/IncidentCommandService.csProviders/Resgrid.Providers.Migrations/Migrations/M0103_AddLaneWorkTimeThresholds.csProviders/Resgrid.Providers.MigrationsPg/Migrations/M0103_AddLaneWorkTimeThresholdsPg.csWeb/Resgrid.Web.Services/Controllers/v4/CommandsController.csWeb/Resgrid.Web.Services/Models/v4/Commands/CommandModels.cs
| public int WorkTimeAmberMinutes { get; set; } | ||
| public int WorkTimeRedMinutes { get; set; } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Preserve omitted thresholds during updates.
Because these request fields are non-nullable, callers that omit the new JSON properties receive 0, and SaveCommand persists that value over existing thresholds. Since 0 disables the indicator, older or partial v4 clients can silently disable configured lane thresholds. Use nullable input fields and preserve stored values when they are omitted; reserve explicit 0 for disabling.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Web/Resgrid.Web.Services/Models/v4/Commands/CommandModels.cs` around lines
115 - 116, Update WorkTimeAmberMinutes and WorkTimeRedMinutes in the command
request model to nullable inputs, then adjust SaveCommand to retain each stored
threshold when its corresponding value is omitted while still persisting an
explicitly supplied 0 as the disable value.
| string[] unitTypes = null, string[] personnelRoles = null, bool forceRequirements = false, | ||
| int minUnits = 0, int maxUnits = 0, int minUnitPersonnel = 0, int maxUnitPersonnel = 0, int minTimeInRole = 0, int maxTimeInRole = 0) | ||
| int minUnits = 0, int maxUnits = 0, int minUnitPersonnel = 0, int maxUnitPersonnel = 0, int minTimeInRole = 0, int maxTimeInRole = 0, | ||
| int workTimeAmberMinutes = 20, int workTimeRedMinutes = 40) |
There was a problem hiding this comment.
Uninitialized property WorkTimeAmberMinutes in CommandModels.cs lacks an explicit default initializer, leaving the object in an ambiguous state per Rule [32]. Add an explicit initializer such as = 0 or set it in the constructor.
Kody rule violation: Replace magic numbers with named constants
Prompt for LLM
File Core/Resgrid.Model/CommandBoards/CommandBoardTemplateCatalog.cs:
Line 45:
Uninitialized property WorkTimeAmberMinutes in CommandModels.cs lacks an explicit default initializer, leaving the object in an ambiguous state per Rule [32]. Add an explicit initializer such as = 0 or set it in the constructor.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| { | ||
| if (Schema.Table("CommandStructureNodes").Exists()) | ||
| { | ||
| foreach (var column in new[] { "WorkTimeAmberMinutes", "WorkTimeRedMinutes" }) |
There was a problem hiding this comment.
WHAT: The column-name string literals are defined inline and will be repeated four times in this file. WHY: Repeating raw string literals for database identifiers risks typos and makes future renames error-prone. HOW: Extract a private static readonly string[] WorkTimeColumns = { "WorkTimeAmberMinutes", "WorkTimeRedMinutes" }; at class level and reference it in every loop.
Also found in:
Providers/Resgrid.Providers.Migrations/Migrations/M0103_AddLaneWorkTimeThresholds.cs:29-29Providers/Resgrid.Providers.Migrations/Migrations/M0103_AddLaneWorkTimeThresholds.cs:44-44Providers/Resgrid.Providers.Migrations/Migrations/M0103_AddLaneWorkTimeThresholds.cs:53-53
Kody rule violation: Centralize string constants
Prompt for LLM
File Providers/Resgrid.Providers.Migrations/Migrations/M0103_AddLaneWorkTimeThresholds.cs:
Line 17:
WHAT: The column-name string literals are defined inline and will be repeated four times in this file. WHY: Repeating raw string literals for database identifiers risks typos and makes future renames error-prone. HOW: Extract a private static readonly string[] WorkTimeColumns = { "WorkTimeAmberMinutes", "WorkTimeRedMinutes" }; at class level and reference it in every loop.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| { | ||
| if (Schema.Table("commandstructurenodes").Exists()) | ||
| { | ||
| foreach (var column in new[] { "worktimeamberminutes", "worktimeredminutes" }) |
There was a problem hiding this comment.
WHAT: The column-name array literal new[] { "worktimeamberminutes", "worktimeredminutes" } is repeated on lines 17, 29, 44, and 53. WHY: Duplicated literals risk divergence if a column name is renamed or added. HOW: Declare a private static readonly string[] WorkTimeColumns at class level and reference it in all four loops; additionally, factor the repeated Alter/Table-check and Delete/Table-check blocks into helper methods.
Also found in:
Providers/Resgrid.Providers.Migrations/Migrations/M0103_AddLaneWorkTimeThresholds.cs:29-29Providers/Resgrid.Providers.Migrations/Migrations/M0103_AddLaneWorkTimeThresholds.cs:53-53Providers/Resgrid.Providers.MigrationsPg/Migrations/M0103_AddLaneWorkTimeThresholdsPg.cs:29-29Providers/Resgrid.Providers.MigrationsPg/Migrations/M0103_AddLaneWorkTimeThresholdsPg.cs:53-53Providers/Resgrid.Providers.MigrationsPg/Migrations/M0103_AddLaneWorkTimeThresholdsPg.cs:44-44
Kody rule violation: Extract duplicated logic into functions
Prompt for LLM
File Providers/Resgrid.Providers.MigrationsPg/Migrations/M0103_AddLaneWorkTimeThresholdsPg.cs:
Line 17:
WHAT: The column-name array literal new[] { "worktimeamberminutes", "worktimeredminutes" } is repeated on lines 17, 29, 44, and 53. WHY: Duplicated literals risk divergence if a column name is renamed or added. HOW: Declare a private static readonly string[] WorkTimeColumns at class level and reference it in all four loops; additionally, factor the repeated Alter/Table-check and Delete/Table-check blocks into helper methods.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| public int MaxUnits { get; set; } | ||
| public int MinTimeInRole { get; set; } | ||
| public int MaxTimeInRole { get; set; } | ||
| public int WorkTimeAmberMinutes { get; set; } |
There was a problem hiding this comment.
WHAT: The new int property WorkTimeAmberMinutes has no explicit default initializer. WHY: Rule [32] requires properties to have sensible defaults or be initialized via constructors so the object is never in an ambiguous state. Although int defaults to 0 in C#, making the default explicit communicates intent (e.g., = 0 or a meaningful value). HOW: Add an explicit initializer such as public int WorkTimeAmberMinutes { get; set; } = 0; or set it in the constructor.
Also found in:
Web/Resgrid.Web.Services/Models/v4/Commands/CommandModels.cs:50-50Web/Resgrid.Web.Services/Models/v4/Commands/CommandModels.cs:115-115Web/Resgrid.Web.Services/Models/v4/Commands/CommandModels.cs:116-116
Kody rule violation: Initialize properties with default values
Prompt for LLM
File Web/Resgrid.Web.Services/Models/v4/Commands/CommandModels.cs:
Line 49:
WHAT: The new int property `WorkTimeAmberMinutes` has no explicit default initializer. WHY: Rule [32] requires properties to have sensible defaults or be initialized via constructors so the object is never in an ambiguous state. Although `int` defaults to `0` in C#, making the default explicit communicates intent (e.g., `= 0` or a meaningful value). HOW: Add an explicit initializer such as `public int WorkTimeAmberMinutes { get; set; } = 0;` or set it in the constructor.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
Approve |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
2 participants
Pull Request Description
This PR adds configurable per-lane work-time (crew fatigue) indicator thresholds to command board lanes. Each lane can now specify when its work-time indicator turns amber (warning) and red (critical) based on minutes resources have been assigned to that lane.
Changes
New data fields (
WorkTimeAmberMinutesandWorkTimeRedMinutes) added to:CommandBoardTemplateLane(template definition, with defaults of 20 and 40 minutes)CommandDefinitionRole(command definition role)CommandStructureNode(runtime node)CommandRoleResultDataandSaveCommandLaneInput)Data propagation: Updated the template-to-definition creation, command seeding service, and the save/convert logic in
CommandsControllerto flow these values through all layers.Database migrations (Migration 103) for both SQL Server and PostgreSQL, adding the two new integer columns to the
CommandStructureNodesandCommandDefinitionRolestables with a default value of 0 (disabled).A value of
0disables the indicator, allowing clients to fall back to their own defaults. These thresholds are denormalized onto runtime nodes from the template role at seeding, consistent with how the existing lane limits (e.g., min/max time in role) are handled.