Conversation
|
Thanks for opening this, but we'd appreciate a little more information. Could you update it with more details? |
📝 WalkthroughWalkthroughThis PR introduces a comprehensive check-in timer feature for Resgrid, enabling departments to configure and track timed check-ins for personnel, unit types, and incident commanders during active calls. The implementation includes database schema additions, service layer logic for timer resolution with priority-based overrides, API endpoints for configuration and check-in recording, web UI for management, and real-time status tracking with color-coded indicators. Changes
Sequence Diagram(s)sequenceDiagram
participant User as Dispatch User
participant Controller as DispatchController
participant Service as CheckInTimerService
participant ConfigRepo as CheckInTimerConfigRepository
participant OverrideRepo as CheckInTimerOverrideRepository
participant DB as Database
User->>Controller: Request resolved timers for call
Controller->>Service: ResolveAllTimersForCallAsync(call)
Service->>ConfigRepo: GetByDepartmentIdAsync(deptId)
ConfigRepo->>DB: SELECT timer configs by department
DB-->>ConfigRepo: Return default configs
ConfigRepo-->>Service: Return configs
Service->>OverrideRepo: GetMatchingOverridesAsync(deptId, callType, priority)
OverrideRepo->>DB: SELECT matching overrides with priority filtering
DB-->>OverrideRepo: Return priority-matched overrides
OverrideRepo-->>Service: Return overrides
Service->>Service: Apply priority matching logic<br/>(type+priority > type-only > priority-only)
Service->>Service: Seed resolved timers from defaults<br/>and apply best-matching overrides
Service-->>Controller: Return List<ResolvedCheckInTimer>
Controller-->>User: Display resolved timers
sequenceDiagram
participant User as Call Participant
participant JS as Client Script
participant Controller as DispatchController
participant Service as CheckInTimerService
participant CheckInRepo as CheckInRecordRepository
participant DB as Database
User->>JS: Click "Perform Check-In"
JS->>JS: Request browser geolocation
JS->>JS: Build check-in payload<br/>(callId, type, location, note)
JS->>Controller: POST PerformCheckIn(input)
Controller->>Service: PerformCheckInAsync(CheckInRecord)
Service->>Service: Set Timestamp to DateTime.UtcNow
Service->>CheckInRepo: SaveAsync(record)
CheckInRepo->>DB: INSERT CheckInRecord
DB-->>CheckInRepo: Return saved record
CheckInRepo-->>Service: Return CheckInRecord
Service-->>Controller: Return saved CheckInRecord
Controller->>Controller: Publish SignalR event<br/>CheckInPerformed
Controller-->>JS: Return success with record ID
JS->>JS: Update timer statuses and check-in history tables<br/>via polling endpoints
JS-->>User: Confirm check-in recorded
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| [ProducesResponseType(StatusCodes.Status200OK)] | ||
| [ProducesResponseType(StatusCodes.Status400BadRequest)] | ||
| [Authorize(Policy = ResgridResources.Department_Update)] | ||
| public async Task<ActionResult<SaveCheckInTimerConfigResult>> SaveTimerConfig([FromBody] CheckInTimerConfigInput input, CancellationToken cancellationToken) |
Check failure
Code scanning / CodeQL
Missing cross-site request forgery token validation High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 11 hours ago
In general, to fix missing CSRF validation in ASP.NET Core MVC or Web API controllers that rely on cookie-based authentication, each mutating POST action (and other state-changing verbs like PUT/DELETE) should either (a) be protected with an anti-forgery token validated by [ValidateAntiForgeryToken] or (b) be excluded from browser-based cookie auth and instead require a non-cookie token (like a bearer token) that is not automatically sent by browsers. Since we must not change broader application behavior, the minimal fix is to add the standard antiforgery validation attribute to this POST action.
Concretely, in Web/Resgrid.Web.Services/Controllers/v4/CheckInTimersController.cs, on the SaveTimerConfig action (line 80), add the [ValidateAntiForgeryToken] attribute in the attribute list above the method. ASP.NET Core’s antiforgery attribute type is Microsoft.AspNetCore.Mvc.ValidateAntiForgeryTokenAttribute, which is brought into scope via using Microsoft.AspNetCore.Mvc; already present at the top, so no new using directive is required. No changes to the method body are necessary. This will cause ASP.NET Core to validate the antiforgery token submitted with the request (e.g., from a hidden form field or header) before executing the action, mitigating CSRF for this operation while preserving existing business logic.
| @@ -73,6 +73,7 @@ | ||
| /// Creates or updates a timer configuration | ||
| /// </summary> | ||
| [HttpPost("SaveTimerConfig")] | ||
| [ValidateAntiForgeryToken] | ||
| [Consumes(MediaTypeNames.Application.Json)] | ||
| [ProducesResponseType(StatusCodes.Status200OK)] | ||
| [ProducesResponseType(StatusCodes.Status400BadRequest)] |
| [ProducesResponseType(StatusCodes.Status200OK)] | ||
| [ProducesResponseType(StatusCodes.Status400BadRequest)] | ||
| [Authorize(Policy = ResgridResources.Department_Update)] | ||
| public async Task<ActionResult<SaveCheckInTimerOverrideResult>> SaveTimerOverride([FromBody] CheckInTimerOverrideInput input, CancellationToken cancellationToken) |
Check failure
Code scanning / CodeQL
Missing cross-site request forgery token validation High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 11 hours ago
In general, state-changing POST (and other unsafe) actions that are authenticated via cookies should validate an anti-forgery token on every request. In ASP.NET Core MVC, this is typically done by decorating the action with [ValidateAntiForgeryToken] and ensuring clients send a valid antiforgery token with the request. For APIs, some teams instead use [AutoValidateAntiforgeryToken] on controllers or globally.
For this specific method, the minimal, targeted fix that does not change existing logic is to add the [ValidateAntiForgeryToken] attribute to SaveTimerOverride. Since the file already imports Microsoft.AspNetCore.Mvc, no additional using directives are needed; ValidateAntiForgeryTokenAttribute resides in that namespace. The change is confined to Web/Resgrid.Web.Services/Controllers/v4/CheckInTimersController.cs, on the SaveTimerOverride action’s attribute list. No other methods or logic need to be modified for this fix.
Concretely:
- In
CheckInTimersController.cs, locate theSaveTimerOverrideaction at lines 170–176. - Insert
[ValidateAntiForgeryToken]among the existing attributes (e.g., between[ProducesResponseType(StatusCodes.Status400BadRequest)]and[Authorize(...)], or just before the method signature). - No new methods or imports are required, because
Microsoft.AspNetCore.Mvcis already included.
| @@ -171,6 +171,7 @@ | ||
| [Consumes(MediaTypeNames.Application.Json)] | ||
| [ProducesResponseType(StatusCodes.Status200OK)] | ||
| [ProducesResponseType(StatusCodes.Status400BadRequest)] | ||
| [ValidateAntiForgeryToken] | ||
| [Authorize(Policy = ResgridResources.Department_Update)] | ||
| public async Task<ActionResult<SaveCheckInTimerOverrideResult>> SaveTimerOverride([FromBody] CheckInTimerOverrideInput input, CancellationToken cancellationToken) | ||
| { |
| [ProducesResponseType(StatusCodes.Status400BadRequest)] | ||
| [ProducesResponseType(StatusCodes.Status404NotFound)] | ||
| [Authorize(Policy = ResgridResources.Call_Update)] | ||
| public async Task<ActionResult<PerformCheckInResult>> PerformCheckIn([FromBody] PerformCheckInInput input, CancellationToken cancellationToken) |
Check failure
Code scanning / CodeQL
Missing cross-site request forgery token validation High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 11 hours ago
In general, for ASP.NET Core MVC controllers that handle state-changing POST requests and can be called from a browser with cookie-based authentication, you should enable antiforgery token validation using [ValidateAntiForgeryToken] or [AutoValidateAntiforgeryToken]. This ensures that POSTs must include a valid antiforgery token (typically emitted in a hidden form field or header), preventing cross-site sites from forcing authenticated users to perform unintended actions.
The best minimal fix here is to decorate the PerformCheckIn action with [ValidateAntiForgeryToken]. This aligns with the CodeQL recommendation, adds explicit CSRF protection only for this endpoint, and does not change any business logic or response formats. Since the project is using ASP.NET Core, the attribute is available from Microsoft.AspNetCore.Mvc, which is already imported (line 3), so no new using directives are needed. Concretely, in Web/Resgrid.Web.Services/Controllers/v4/CheckInTimersController.cs, just above the PerformCheckIn method declaration at line 317, add [ValidateAntiForgeryToken] beneath the existing attributes.
No additional methods or helpers are required in this snippet, and no other endpoints need to be changed for this specific fix.
| @@ -314,6 +314,7 @@ | ||
| [ProducesResponseType(StatusCodes.Status400BadRequest)] | ||
| [ProducesResponseType(StatusCodes.Status404NotFound)] | ||
| [Authorize(Policy = ResgridResources.Call_Update)] | ||
| [ValidateAntiForgeryToken] | ||
| public async Task<ActionResult<PerformCheckInResult>> PerformCheckIn([FromBody] PerformCheckInInput input, CancellationToken cancellationToken) | ||
| { | ||
| var result = new PerformCheckInResult(); |
| } | ||
|
|
||
| [HttpPost] | ||
| public async Task<IActionResult> PerformCheckIn([FromBody] PerformCheckInInput input, CancellationToken cancellationToken) |
Check failure
Code scanning / CodeQL
Missing cross-site request forgery token validation High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 11 hours ago
In general, to fix missing CSRF protection on ASP.NET Core MVC POST actions that use cookie-based authentication, you should: (1) ensure that the client sends an anti-forgery token with the request (e.g., via a hidden form field or a request header), and (2) annotate the action (or apply a global filter) with [ValidateAntiForgeryToken] (or [AutoValidateAntiforgeryToken]) so the framework validates that token on each request.
For this specific PerformCheckIn action in Web/Resgrid.Web/Areas/User/Controllers/DispatchController.cs, the minimal change that adds CSRF protection without altering business logic is to decorate the method with [ValidateAntiForgeryToken], immediately above the existing [HttpPost] or above the method signature. ASP.NET Core already brings ValidateAntiForgeryTokenAttribute into scope via Microsoft.AspNetCore.Mvc, which is present in the using statements at the top of the file, so no new imports are needed. The resulting method will still accept the same parameters and perform the same operations, but any request lacking a valid anti-forgery token will be rejected by the framework. If the client is a JavaScript front end posting JSON, it must be updated (outside this snippet) to include the anti-forgery token (for example, by reading it from a cookie or meta tag and sending it in the RequestVerificationToken header), but that is out of scope for this code-only fix.
Concretely, in DispatchController.cs, around line 1751 where PerformCheckIn is defined, add [ValidateAntiForgeryToken] directly above the [HttpPost] attribute (or above the method if preferred). No other changes to method body, parameters, or return type are required.
| @@ -1749,6 +1749,7 @@ | ||
| } | ||
|
|
||
| [HttpPost] | ||
| [ValidateAntiForgeryToken] | ||
| public async Task<IActionResult> PerformCheckIn([FromBody] PerformCheckInInput input, CancellationToken cancellationToken) | ||
| { | ||
| if (input == null || input.CallId <= 0) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
🧹 Nitpick comments (11)
Core/Resgrid.Model/ResolvedCheckInTimer.cs (1)
3-18: Consider making this state model immutable (record/init-only).This is a pure data carrier, so immutability would reduce accidental mutation and simplify reasoning.
♻️ Suggested refactor
-public class ResolvedCheckInTimer -{ - public int TargetType { get; set; } - public int? UnitTypeId { get; set; } - public string TargetEntityId { get; set; } - public string TargetName { get; set; } - public int DurationMinutes { get; set; } - public int WarningThresholdMinutes { get; set; } - public bool IsFromOverride { get; set; } -} +public record ResolvedCheckInTimer +{ + public int TargetType { get; init; } + public int? UnitTypeId { get; init; } + public string TargetEntityId { get; init; } + public string TargetName { get; init; } + public int DurationMinutes { get; init; } + public int WarningThresholdMinutes { get; init; } + public bool IsFromOverride { get; init; } +}As per coding guidelines, "Prefer functional patterns and immutable data where appropriate in C#" and "Separate state from behavior (e.g., use records for state and place operations in separate static classes)".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Core/Resgrid.Model/ResolvedCheckInTimer.cs` around lines 3 - 18, Convert the mutable POCO ResolvedCheckInTimer into an immutable state record by replacing the class with a record type (keep the name ResolvedCheckInTimer) and turn all properties (TargetType, UnitTypeId, TargetEntityId, TargetName, DurationMinutes, WarningThresholdMinutes, IsFromOverride) into init-only properties (or positional record parameters) so instances are immutable after construction; update any call sites that assign to these properties to construct a new record instance instead.Core/Resgrid.Model/CheckInTimerTargetType.cs (1)
7-8: Consider expanding abbreviated enum members for long-term clarity.
ICandPARare terse in a public contract; consider fully descriptive names (or at least XML docs) before wider usage spreads.As per coding guidelines, "Use meaningful, descriptive names for types, methods, and parameters; avoid unclear abbreviations".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Core/Resgrid.Model/CheckInTimerTargetType.cs` around lines 7 - 8, The enum CheckInTimerTargetType contains terse members IC and PAR which are unclear; rename these to descriptive identifiers (for example IC -> IndividualCheckIn or IncidentCheckIn and PAR -> Paramedic or Participant) throughout the codebase (search/replace occurrences of CheckInTimerTargetType.IC and CheckInTimerTargetType.PAR) and update any XML docs or comments for the enum and its members to describe their purpose; ensure serialization names or mappings (if used) are preserved or updated (e.g., add EnumMember attributes or maintain backward-compatible numeric values 2 and 3) so external consumers are not broken.Core/Resgrid.Model/CheckInTimerStatus.cs (1)
5-24: Consider using arecordfor this state-holding type.Per coding guidelines, records are preferred for state representation. Additionally, the
Statusproperty could benefit from an enum (e.g.,CheckInTimerState) instead of magic strings like"Green","Warning","Critical"to improve type safety and prevent typos.♻️ Suggested refactor using record and enum
public enum CheckInTimerState { Green, Warning, Critical } public record CheckInTimerStatus( int TargetType, string TargetEntityId, string TargetName, int? UnitId, DateTime? LastCheckIn, int DurationMinutes, int WarningThresholdMinutes, double ElapsedMinutes, CheckInTimerState Status );As per coding guidelines: "Separate state from behavior (e.g., use records for state and place operations in separate static classes)".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Core/Resgrid.Model/CheckInTimerStatus.cs` around lines 5 - 24, Replace the mutable CheckInTimerStatus class with an immutable record to represent state and add a CheckInTimerState enum for Status to avoid magic strings; specifically change the type named CheckInTimerStatus into a record (positional or read-only properties) and introduce an enum CheckInTimerState with values like Green, Warning, Critical, then update the Status property/type to use CheckInTimerState across usages to preserve type safety and prevent string typos.Web/Resgrid.Web/Areas/User/Controllers/DispatchController.cs (3)
1649-1711: Missing[Authorize]attribute onGetCheckInTimerStatusesendpoint.The
GetCheckInTimerStatusesendpoint checkscall.DepartmentId != DepartmentIdbut lacks an explicit[Authorize(Policy = ...)]attribute like other endpoints in this controller. WhileSecureBaseControllerlikely provides base authentication, consider adding the policy attribute for consistency with other call-related endpoints (e.g.,GetCallNotesuses[Authorize(Policy = ResgridResources.Call_View)]).♻️ Add authorization attribute
[HttpGet] +[Authorize(Policy = ResgridResources.Call_View)] public async Task<IActionResult> GetCheckInTimerStatuses(int callId)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Web/Resgrid.Web/Areas/User/Controllers/DispatchController.cs` around lines 1649 - 1711, The GetCheckInTimerStatuses action is missing an explicit authorization attribute; add the same policy used by other call endpoints by decorating the GetCheckInTimerStatuses method with [Authorize(Policy = ResgridResources.Call_View)] so it enforces the Call_View permission (keep the existing DepartmentId check and method logic intact).
1751-1778: Missing[Authorize]attribute onPerformCheckInendpoint.The
PerformCheckInendpoint modifies data (creates a check-in record) but lacks an explicit authorization policy. Consider adding[Authorize(Policy = ResgridResources.Call_View)]or a more specific policy.♻️ Add authorization attribute
[HttpPost] +[Authorize(Policy = ResgridResources.Call_View)] public async Task<IActionResult> PerformCheckIn([FromBody] PerformCheckInInput input, CancellationToken cancellationToken)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Web/Resgrid.Web/Areas/User/Controllers/DispatchController.cs` around lines 1751 - 1778, The PerformCheckIn action lacks an authorization attribute; add an authorization policy attribute (e.g. [Authorize(Policy = ResgridResources.Call_View)] or a more restrictive policy you have for modifying calls) to the PerformCheckIn method so only authorized users can create check-in records; locate the PerformCheckIn method in DispatchController and apply the attribute directly above the method signature to enforce the policy.
1713-1749: Missing[Authorize]attribute onGetCheckInHistoryendpoint.Same as
GetCheckInTimerStatuses- consider adding[Authorize(Policy = ResgridResources.Call_View)]for consistency.♻️ Add authorization attribute
[HttpGet] +[Authorize(Policy = ResgridResources.Call_View)] public async Task<IActionResult> GetCheckInHistory(int callId)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Web/Resgrid.Web/Areas/User/Controllers/DispatchController.cs` around lines 1713 - 1749, Add the missing authorization attribute to the GetCheckInHistory action: apply [Authorize(Policy = ResgridResources.Call_View)] to the GetCheckInHistory method (same as used on GetCheckInTimerStatuses) so access is restricted consistently; update the method signature above public async Task<IActionResult> GetCheckInHistory(int callId) to include the attribute and run tests to verify authorization behavior.Web/Resgrid.Web/wwwroot/js/app/internal/dispatch/resgrid.dispatch.checkin-timers.js (2)
7-14: Consider clearing the timer interval when the call is no longer active.The
timerIntervalis set on page load but never cleared. If the call state changes (e.g., closed via another action), the interval continues running unnecessarily. Consider adding cleanup logic when the component is destroyed or call state changes.♻️ Suggested cleanup approach
// Add a function to stop the timer function stopTimerInterval() { if (timerInterval) { clearInterval(timerInterval); timerInterval = null; } } // Call stopTimerInterval when navigating away or when call state changes $(window).on('beforeunload', stopTimerInterval);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Web/Resgrid.Web/wwwroot/js/app/internal/dispatch/resgrid.dispatch.checkin-timers.js` around lines 7 - 14, The interval set by timerInterval in the document ready block (where loadTimerStatuses() and setInterval(updateCountdowns, 1000) are called) is never cleared; add cleanup logic to stop the interval when the call is no longer active or the page unloads by implementing a stopTimerInterval function that checks timerInterval, calls clearInterval(timerInterval), and nulls it, and invoke stopTimerInterval when callState changes or on window beforeunload/unload to ensure updateCountdowns is not left running unnecessarily.
16-27: Consider adding error handling for AJAX requests.The
loadTimerStatusesandloadCheckInHistoryfunctions don't handle AJAX failures. Users won't be informed if the data fails to load.♻️ Add .fail() handlers
function loadTimerStatuses() { $.ajax({ url: resgrid.absoluteBaseUrl + '/User/Dispatch/GetCheckInTimerStatuses?callId=' + callId, contentType: 'application/json; charset=utf-8', type: 'GET' }).done(function (result) { if (result) { timerStatuses = result; renderTimerStatuses(); } + }).fail(function () { + console.warn('Failed to load timer statuses'); }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Web/Resgrid.Web/wwwroot/js/app/internal/dispatch/resgrid.dispatch.checkin-timers.js` around lines 16 - 27, Add error handling for the AJAX calls in the loadTimerStatuses and loadCheckInHistory functions: attach .fail(...) handlers to the $.ajax(...) chains used in loadTimerStatuses and loadCheckInHistory, log the error (e.g., console.error or existing logger) and provide user feedback (e.g., show a toast/alert or display an error message in the UI where renderTimerStatuses or history rendering would normally go), and optionally retry or disable UI elements until a successful load; ensure you reference the existing timerStatuses and renderTimerStatuses usage so failures keep the UI consistent.Repositories/Resgrid.Repositories.DataRepository/CheckInTimerOverrideRepository.cs (1)
48-61: Refactor for clarity: make transaction handling explicit and eliminate code duplication.The pattern of passing
_unitOfWork.Transaction(null when using standalone connections) is intentional and consistent across the codebase. However, this implicit behavior can be made more explicit. Additionally, theselectFunctionpattern is duplicated in both methods—consider extracting a shared helper method to reduce duplication and improve maintainability.Example improvement:
private async Task<IEnumerable<CheckInTimerOverride>> ExecuteSelectAsync( Func<DbConnection, DbTransaction, Task<IEnumerable<CheckInTimerOverride>>> selectFunction) { if (_unitOfWork?.Connection == null) { using (var conn = _connectionProvider.Create()) { await conn.OpenAsync(); return await selectFunction(conn, null); } } else { var conn = _unitOfWork.CreateOrGetConnection(); return await selectFunction(conn, _unitOfWork.Transaction); } }Then update methods to accept transaction parameter explicitly in the selectFunction delegate.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Repositories/Resgrid.Repositories.DataRepository/CheckInTimerOverrideRepository.cs` around lines 48 - 61, Refactor the duplicated connection/transaction handling in CheckInTimerOverrideRepository by extracting a shared helper (e.g., ExecuteSelectAsync) that takes a delegate Func<DbConnection, DbTransaction, Task<IEnumerable<CheckInTimerOverride>>>; inside the helper, if _unitOfWork?.Connection is null create and open a using connection from _connectionProvider.Create() and pass null for the transaction, otherwise get the connection via _unitOfWork.CreateOrGetConnection() and pass _unitOfWork.Transaction; update existing methods to call ExecuteSelectAsync and change their selectFunction delegates to accept the explicit DbTransaction parameter to eliminate duplication and make transaction handling explicit.Web/Resgrid.Web.Services/Resgrid.Web.Services.xml (1)
311-365: Add contract-level XML docs for the new check-in timer endpoints.These actions are discoverable, but the generated XML only has summaries here, and this file has no
Resgrid.Web.Services.Models.v4.CheckInTimers.*members elsewhere. That leaves the new v4 surface without field or return descriptions for its request/response payloads. Please add source XML comments for the action parameters/returns and theCheckInTimersDTO properties, then regenerate this file.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Web/Resgrid.Web.Services/Resgrid.Web.Services.xml` around lines 311 - 365, The generated XML lacks parameter/return and DTO member docs for the new CheckInTimers v4 API; open the controller methods (GetTimerConfigs, SaveTimerConfig, DeleteTimerConfig, GetTimerOverrides, SaveTimerOverride, DeleteTimerOverride, GetTimersForCall, GetTimerStatuses, PerformCheckIn, GetCheckInHistory, ToggleCallTimers) and add C# XML comments for each action's parameters and returns, then add XML comments on all DTO properties under Resgrid.Web.Services.Models.v4.CheckInTimers (e.g., CheckInTimerConfigInput, CheckInTimerOverrideInput, PerformCheckInInput and any response models) describing fields and expected values, rebuild to regenerate Resgrid.Web.Services.xml so the contract includes parameter/return descriptions and DTO member documentation.Web/Resgrid.Web/Areas/User/Controllers/DepartmentController.cs (1)
64-72: Split the new timer settings surface out ofDepartmentController.Adding
ICheckInTimerServicehere pushes an already very large constructor further out of shape and makes this controller harder to test. The new check-in timer CRUD/actions look substantial enough for a dedicated controller or composed settings coordinator instead of continuing to grow this one.As per coding guidelines "Minimize constructor injection; keep the number of injected dependencies small".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Web/Resgrid.Web/Areas/User/Controllers/DepartmentController.cs` around lines 64 - 72, The DepartmentController constructor has grown too large by adding ICheckInTimerService; extract the check‑in timer functionality into its own controller or coordinator to reduce constructor injection and improve testability: remove ICheckInTimerService from DepartmentController's ctor and move related CRUD/actions into a new CheckInTimerController (or CheckInTimerSettingsCoordinator) that depends only on ICheckInTimerService and any minimal collaborators; update routing/DI to register the new controller/service and adjust any calls from DepartmentController to delegate to the new component (reference symbols: DepartmentController, ICheckInTimerService, and the check‑in timer CRUD/action methods).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.gitignore:
- Line 276: Remove the duplicate `.claude/settings.local.json` entry from
.gitignore by deleting the redundant line that repeats the same pattern; look
for the string ".claude/settings.local.json" (it appears twice) and remove the
second occurrence so the file contains only a single entry for that path.
In `@Core/Resgrid.Services/CheckInTimerService.cs`:
- Around line 36-52: When updating or deleting timers, first verify the existing
entity's DepartmentId matches the caller's department before persisting or
removing it: in SaveTimerConfigAsync, when config.CheckInTimerConfigId is
present, load the existing entity via _configRepository.GetByIdAsync(existingId)
and compare existing.DepartmentId to the caller's department (either
config.DepartmentId or a new callerDepartmentId parameter); if they differ,
reject (throw UnauthorizedAccessException or return an error) instead of saving;
similarly in DeleteTimerConfigAsync, after loading config from
_configRepository.GetByIdAsync(configId) verify config.DepartmentId matches the
caller's department and return false or throw when it does not before calling
_configRepository.DeleteAsync.
- Around line 88-152: ResolveAllTimersForCallAsync collapses different concrete
targets into a single TimerTargetType/UnitTypeId bucket; change the resolution
to carry a concrete target identity through and use that same identity when
computing statuses. Specifically: add a TargetId (or ConcreteTargetId) property
to ResolvedCheckInTimer and populate it when building defaults and when applying
overrides in ResolveAllTimersForCallAsync (use the config/override field that
identifies the specific unit/person/team), change the dictionary key to include
TargetId (e.g., $"{def.TimerTargetType}_{def.UnitTypeId}_{def.TargetId}"),
preserve IsFromOverride, then update GetActiveTimerStatusesForCallAsync to match
check-ins using the identical composite key (Timer/CheckIn type + UnitTypeId +
TargetId) when selecting latest records; also fix the reporting bug by returning
UnitId where Line 222 currently emits UnitTypeId.
- Around line 101-149: The override application currently materializes all
overrides into resolved regardless of o.IsEnabled; update the loop that
processes scoredOverrides in CheckInTimerService (the block using
scoredOverrides, o, key, and resolved) to skip any override where o.IsEnabled is
false (i.e., continue to next scored item) so disabled overrides are not applied
and therefore cannot suppress or replace defaults; ensure you only construct a
ResolvedCheckInTimer and set IsFromOverride = true when o.IsEnabled == true.
In `@Core/Resgrid.Services/ServicesModule.cs`:
- Line 89: The registration for CheckInTimerService currently uses
SingleInstance() and creates a captive dependency with its scoped constructor
dependencies; change the Autofac registration of
CheckInTimerService.As<ICheckInTimerService>() to use InstancePerLifetimeScope()
(instead of SingleInstance()) so the service shares the same lifetime as its
InstancePerLifetimeScope() dependencies and avoids elevating scoped repositories
to singletons; verify no other registrations reference CheckInTimerService as a
singleton.
In
`@Providers/Resgrid.Providers.Migrations/Migrations/M0056_AddingCheckInTimers.cs`:
- Around line 31-33: The UNIQUE constraints
UQ_CheckInTimerConfigs_Dept_Target_Unit (on CheckInTimerConfigs: DepartmentId,
TimerTargetType, UnitTypeId) and UQ_CheckInTimerOverrides (on
CheckInTimerOverrides: DepartmentId, TimerTargetType, CallTypeId, CallPriority,
UnitTypeId) include nullable columns and will prevent multiple NULLs; replace
them with SQL Server filtered unique nonclustered indexes that enforce
uniqueness only when those nullable columns are NOT NULL (e.g., create unique
nonclustered index for CheckInTimerConfigs on DepartmentId, TimerTargetType,
UnitTypeId WHERE UnitTypeId IS NOT NULL, and similarly for CheckInTimerOverrides
include a WHERE clause excluding rows where CallTypeId IS NULL OR CallPriority
IS NULL OR UnitTypeId IS NULL as appropriate) so multiple NULL-valued rows are
allowed.
In
`@Providers/Resgrid.Providers.MigrationsPg/Migrations/M0056_AddingCheckInTimersPg.cs`:
- Around line 83-90: The migration currently creates ix_checkinrecords_callid
and ix_checkinrecords_departmentid_timestamp but does not support the hot
"latest check-in" lookups by (callid, userid) and (callid, unitid); update the
migration to add compound indexes that match those access patterns by creating
indexes on table "checkinrecords" for (callid, userid, timestamp DESC) and
(callid, unitid, timestamp DESC) so queries seeking the latest record per
user/unit under a call can use the index rather than scanning/sorting—place
these alongside the existing Create.Index calls (refer to the existing
Create.Index("ix_checkinrecords_callid") and
Create.Index("ix_checkinrecords_departmentid_timestamp") usages to locate where
to add the new index creations).
- Around line 31-33: The migration's current unique constraint
Create.UniqueConstraint("uq_checkintimerconfigs_dept_target_unit") on
checkintimerconfigs still allows duplicate "Any/None" rules because PostgreSQL
treats NULLs as distinct; update the migration to enforce uniqueness for
nullable override columns by either (a) using a NULLS NOT DISTINCT unique index
if targeting PostgreSQL 15+ (replace the Create.UniqueConstraint with a CREATE
UNIQUE INDEX ... ON checkintimeroverrides (...) NULLS NOT DISTINCT), or (b) add
partial unique indexes that cover only non-NULL combinations for the nullable
override columns (e.g., unique partial indexes on checkintimeroverrides for
(departmentid, timertargettype, calltypeid) WHERE calltypeid IS NOT NULL,
similarly for callpriority and unittypeid) so that duplicates for "Any/None"
cannot be inserted; ensure you reference and update the constraint name
("uq_checkintimerconfigs_dept_target_unit") or create appropriate index names to
avoid conflicts.
In
`@Repositories/Resgrid.Repositories.DataRepository/Queries/CheckIns/SelectLastCheckInForUnitOnCallQuery.cs`:
- Around line 28-30: The generic GetQuery<TEntity>() in the check-in query
classes currently throws NotImplementedException; update each of the eight
classes (SelectCheckInRecordsByCallIdQuery,
SelectCheckInRecordsByDepartmentIdAndDateRangeQuery,
SelectCheckInTimerConfigByDepartmentAndTargetQuery,
SelectCheckInTimerConfigsByDepartmentIdQuery,
SelectCheckInTimerOverridesByDepartmentIdQuery,
SelectLastCheckInForUnitOnCallQuery, SelectLastCheckInForUserOnCallQuery,
SelectMatchingCheckInTimerOverridesQuery) so the generic overload simply
delegates to the non-generic GetQuery() (i.e., implement GetQuery<TEntity>() to
return GetQuery()) to avoid runtime failures when _queryFactory.GetQuery<T>() is
called.
In `@Web/Resgrid.Web.Services/Hubs/EventingHub.cs`:
- Around line 101-115: The CheckInPerformed and CheckInTimersUpdated hub methods
(and Connect()) currently accept caller-supplied departmentId on an
[AllowAnonymous] hub, allowing unauthenticated clients to broadcast to arbitrary
groups; fix by restricting access or validating membership: either add
[Authorize] to the hub class (or the specific methods) to require authenticated
callers, or perform server-side validation inside CheckInPerformed,
CheckInTimersUpdated and Connect() by verifying the caller's identity/claims
(e.g., user id and department membership from Context.User) matches the supplied
departmentId before calling Clients.Group/departments operations; if these are
intended for server-only use, remove the public hub methods and invoke
broadcasts via IHubContext instead.
In `@Web/Resgrid.Web.Services/Models/v4/CheckInTimers/CheckInTimerModels.cs`:
- Around line 32-41: The DTOs accept zero/negative defaults because non-nullable
ints bind to 0 and [Required] doesn't reject that; update the three input models
to enforce positive values by adding a Range(1, int.MaxValue) constraint to the
specified properties: in CheckInTimerConfigInput add Range to TimerTargetType,
DurationMinutes, WarningThresholdMinutes; in CheckInTimerOverrideInput add Range
to TimerTargetType, DurationMinutes, WarningThresholdMinutes; and in
PerformCheckInInput add Range to CallId and CheckInType so invalid (0 or
negative) values are rejected at model validation.
In `@Web/Resgrid.Web/Areas/User/Models/Dispatch/PerformCheckInInput.cs`:
- Around line 5-10: The PerformCheckInInput DTO accepts unconstrained values;
add DataAnnotations to validate at the boundary: mark CallId and CheckInType
with [Required] and [Range(1, int.MaxValue)] (or an appropriate enum range) to
ensure positive IDs, add [Required] plus a numeric/coordinate regex via
[RegularExpression(@"^-?\d+(\.\d+)?$")] (or a more specific lat/lon regex) to
Latitude and Longitude to ensure valid decimal coordinates, mark UnitId as
nullable but add [Range(1, int.MaxValue)] to prevent zero/negative IDs, and
constrain Note with [StringLength(1000)] (or desired max) to limit payload size;
apply these attributes to the properties CallId, CheckInType, Latitude,
Longitude, UnitId, and Note in the PerformCheckInInput class so malformed
check-ins are rejected before business logic runs.
In `@Web/Resgrid.Web/Areas/User/Views/Department/DispatchSettings.cshtml`:
- Around line 163-166: Wrap the localized string used inside the onclick confirm
call with Html.Raw so Razor doesn't HTML-encode the already JavaScript-encoded
value: generate a JavaScript-safe string for
localizer["CheckInTimerDeleteConfirm"] (using the same JS-encoding helper
pattern used elsewhere in the project) and then output it with `@Html.Raw`(...)
inside the single-quoted confirm(...) literal; apply the same change to the
other occurrences mentioned (the block around lines 278-281) so all confirm(...)
usages use Html.Raw around the JS-encoded localized text.
In `@Web/Resgrid.Web/Areas/User/Views/Dispatch/CallExport.cshtml`:
- Around line 5-6: Remove the duplicate Razor injection: there are two `@inject`
declarations creating the same property localizer of type
IStringLocalizer<Resgrid.Localization.Areas.User.Dispatch.Call>; keep a single
`@inject` IStringLocalizer<Resgrid.Localization.Areas.User.Dispatch.Call>
localizer and delete the other so the compiler does not generate duplicate
property declarations (search for the duplicate `@inject` lines referencing
"localizer" to locate them).
In `@Web/Resgrid.Web/Areas/User/Views/Dispatch/NewCall.cshtml`:
- Around line 410-412: Replace the raw checkbox input with the MVC tag helper
bound to the model property (use asp-for="Call.CheckInTimersEnabled") so ASP.NET
Core will render the checked state on re-render and include the hidden false
input for reliable posting; keep the surrounding label and the
`@localizer`["EnableCheckInTimers"] text but change the <input> to use asp-for on
the Call.CheckInTimersEnabled property to ensure model binding preserves state.
In `@Web/Resgrid.Web/Areas/User/Views/Dispatch/ViewCall.cshtml`:
- Around line 774-833: The PerformCheckIn POST currently sends JSON from
resgrid.dispatch.checkin-timers.js without an antiforgery token and the
corresponding controller action in DispatchController (PerformCheckIn) lacks
[ValidateAntiForgeryToken]; fix by adding antiforgery protection: update the
client (resgrid.dispatch.checkin-timers.js -> PerformCheckIn request) to include
the antiforgery token (e.g., read the token from the page meta/cookie/header)
and modify the server action (DispatchController.PerformCheckIn) to validate the
token—either add [ValidateAntiForgeryToken] and accept the token in a header (or
change to a form-bound DTO) or call IAntiforgery.ValidateRequestAsync inside the
action if you must keep JSON binding; ensure token name used on client matches
what the server expects.
In `@Web/Resgrid.Web/Areas/User/Views/Templates/Edit.cshtml`:
- Around line 96-103: Replace the manual checkbox input for
Model.Template.CheckInTimersEnabled with the helper that emits the hidden
fallback input so false values are posted; locate the checkbox using the symbol
Model.Template.CheckInTimersEnabled and change the markup to use
`@Html.CheckBoxFor`(m => m.Template.CheckInTimersEnabled) (keeping the surrounding
label and layout) to ensure proper model binding of both true and false.
In
`@Web/Resgrid.Web/wwwroot/js/app/internal/dispatch/resgrid.dispatch.newcall.js`:
- Around line 322-326: The checkbox is left in a stale state when
data.CheckInTimersEnabled is null/undefined; replace the if/else block with a
single deterministic assignment for $('input[name="Call.CheckInTimersEnabled"]')
using a boolean conversion of data.CheckInTimersEnabled (e.g., use
Boolean(data.CheckInTimersEnabled) or !!data.CheckInTimersEnabled) so the
checkbox is explicitly set to checked/unchecked every time; update the code
around the existing input selector and the data.CheckInTimersEnabled reference
accordingly.
---
Nitpick comments:
In `@Core/Resgrid.Model/CheckInTimerStatus.cs`:
- Around line 5-24: Replace the mutable CheckInTimerStatus class with an
immutable record to represent state and add a CheckInTimerState enum for Status
to avoid magic strings; specifically change the type named CheckInTimerStatus
into a record (positional or read-only properties) and introduce an enum
CheckInTimerState with values like Green, Warning, Critical, then update the
Status property/type to use CheckInTimerState across usages to preserve type
safety and prevent string typos.
In `@Core/Resgrid.Model/CheckInTimerTargetType.cs`:
- Around line 7-8: The enum CheckInTimerTargetType contains terse members IC and
PAR which are unclear; rename these to descriptive identifiers (for example IC
-> IndividualCheckIn or IncidentCheckIn and PAR -> Paramedic or Participant)
throughout the codebase (search/replace occurrences of CheckInTimerTargetType.IC
and CheckInTimerTargetType.PAR) and update any XML docs or comments for the enum
and its members to describe their purpose; ensure serialization names or
mappings (if used) are preserved or updated (e.g., add EnumMember attributes or
maintain backward-compatible numeric values 2 and 3) so external consumers are
not broken.
In `@Core/Resgrid.Model/ResolvedCheckInTimer.cs`:
- Around line 3-18: Convert the mutable POCO ResolvedCheckInTimer into an
immutable state record by replacing the class with a record type (keep the name
ResolvedCheckInTimer) and turn all properties (TargetType, UnitTypeId,
TargetEntityId, TargetName, DurationMinutes, WarningThresholdMinutes,
IsFromOverride) into init-only properties (or positional record parameters) so
instances are immutable after construction; update any call sites that assign to
these properties to construct a new record instance instead.
In
`@Repositories/Resgrid.Repositories.DataRepository/CheckInTimerOverrideRepository.cs`:
- Around line 48-61: Refactor the duplicated connection/transaction handling in
CheckInTimerOverrideRepository by extracting a shared helper (e.g.,
ExecuteSelectAsync) that takes a delegate Func<DbConnection, DbTransaction,
Task<IEnumerable<CheckInTimerOverride>>>; inside the helper, if
_unitOfWork?.Connection is null create and open a using connection from
_connectionProvider.Create() and pass null for the transaction, otherwise get
the connection via _unitOfWork.CreateOrGetConnection() and pass
_unitOfWork.Transaction; update existing methods to call ExecuteSelectAsync and
change their selectFunction delegates to accept the explicit DbTransaction
parameter to eliminate duplication and make transaction handling explicit.
In `@Web/Resgrid.Web.Services/Resgrid.Web.Services.xml`:
- Around line 311-365: The generated XML lacks parameter/return and DTO member
docs for the new CheckInTimers v4 API; open the controller methods
(GetTimerConfigs, SaveTimerConfig, DeleteTimerConfig, GetTimerOverrides,
SaveTimerOverride, DeleteTimerOverride, GetTimersForCall, GetTimerStatuses,
PerformCheckIn, GetCheckInHistory, ToggleCallTimers) and add C# XML comments for
each action's parameters and returns, then add XML comments on all DTO
properties under Resgrid.Web.Services.Models.v4.CheckInTimers (e.g.,
CheckInTimerConfigInput, CheckInTimerOverrideInput, PerformCheckInInput and any
response models) describing fields and expected values, rebuild to regenerate
Resgrid.Web.Services.xml so the contract includes parameter/return descriptions
and DTO member documentation.
In `@Web/Resgrid.Web/Areas/User/Controllers/DepartmentController.cs`:
- Around line 64-72: The DepartmentController constructor has grown too large by
adding ICheckInTimerService; extract the check‑in timer functionality into its
own controller or coordinator to reduce constructor injection and improve
testability: remove ICheckInTimerService from DepartmentController's ctor and
move related CRUD/actions into a new CheckInTimerController (or
CheckInTimerSettingsCoordinator) that depends only on ICheckInTimerService and
any minimal collaborators; update routing/DI to register the new
controller/service and adjust any calls from DepartmentController to delegate to
the new component (reference symbols: DepartmentController,
ICheckInTimerService, and the check‑in timer CRUD/action methods).
In `@Web/Resgrid.Web/Areas/User/Controllers/DispatchController.cs`:
- Around line 1649-1711: The GetCheckInTimerStatuses action is missing an
explicit authorization attribute; add the same policy used by other call
endpoints by decorating the GetCheckInTimerStatuses method with
[Authorize(Policy = ResgridResources.Call_View)] so it enforces the Call_View
permission (keep the existing DepartmentId check and method logic intact).
- Around line 1751-1778: The PerformCheckIn action lacks an authorization
attribute; add an authorization policy attribute (e.g. [Authorize(Policy =
ResgridResources.Call_View)] or a more restrictive policy you have for modifying
calls) to the PerformCheckIn method so only authorized users can create check-in
records; locate the PerformCheckIn method in DispatchController and apply the
attribute directly above the method signature to enforce the policy.
- Around line 1713-1749: Add the missing authorization attribute to the
GetCheckInHistory action: apply [Authorize(Policy = ResgridResources.Call_View)]
to the GetCheckInHistory method (same as used on GetCheckInTimerStatuses) so
access is restricted consistently; update the method signature above public
async Task<IActionResult> GetCheckInHistory(int callId) to include the attribute
and run tests to verify authorization behavior.
In
`@Web/Resgrid.Web/wwwroot/js/app/internal/dispatch/resgrid.dispatch.checkin-timers.js`:
- Around line 7-14: The interval set by timerInterval in the document ready
block (where loadTimerStatuses() and setInterval(updateCountdowns, 1000) are
called) is never cleared; add cleanup logic to stop the interval when the call
is no longer active or the page unloads by implementing a stopTimerInterval
function that checks timerInterval, calls clearInterval(timerInterval), and
nulls it, and invoke stopTimerInterval when callState changes or on window
beforeunload/unload to ensure updateCountdowns is not left running
unnecessarily.
- Around line 16-27: Add error handling for the AJAX calls in the
loadTimerStatuses and loadCheckInHistory functions: attach .fail(...) handlers
to the $.ajax(...) chains used in loadTimerStatuses and loadCheckInHistory, log
the error (e.g., console.error or existing logger) and provide user feedback
(e.g., show a toast/alert or display an error message in the UI where
renderTimerStatuses or history rendering would normally go), and optionally
retry or disable UI elements until a successful load; ensure you reference the
existing timerStatuses and renderTimerStatuses usage so failures keep the UI
consistent.
🪄 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
Run ID: a43aff96-8354-480c-a2f0-07054022c380
⛔ Files ignored due to path filters (28)
Core/Resgrid.Localization/Areas/User/Department/Department.ar.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Department/Department.de.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Department/Department.en.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Department/Department.es.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Department/Department.fr.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Department/Department.it.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Department/Department.pl.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Department/Department.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Department/Department.sv.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Department/Department.uk.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Dispatch/Call.ar.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Dispatch/Call.de.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Dispatch/Call.en.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Dispatch/Call.es.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Dispatch/Call.fr.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Dispatch/Call.it.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Dispatch/Call.pl.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Dispatch/Call.sv.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Dispatch/Call.uk.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Templates/Templates.ar.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Templates/Templates.de.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Templates/Templates.en.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Templates/Templates.es.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Templates/Templates.fr.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Templates/Templates.it.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Templates/Templates.pl.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Templates/Templates.sv.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Templates/Templates.uk.resxis excluded by!**/*.resx
📒 Files selected for processing (62)
.gitignoreCore/Resgrid.Model/AuditLogTypes.csCore/Resgrid.Model/Call.csCore/Resgrid.Model/CallQuickTemplate.csCore/Resgrid.Model/CheckInRecord.csCore/Resgrid.Model/CheckInTimerConfig.csCore/Resgrid.Model/CheckInTimerOverride.csCore/Resgrid.Model/CheckInTimerStatus.csCore/Resgrid.Model/CheckInTimerTargetType.csCore/Resgrid.Model/DepartmentSettingTypes.csCore/Resgrid.Model/Repositories/ICheckInRecordRepository.csCore/Resgrid.Model/Repositories/ICheckInTimerConfigRepository.csCore/Resgrid.Model/Repositories/ICheckInTimerOverrideRepository.csCore/Resgrid.Model/ResolvedCheckInTimer.csCore/Resgrid.Model/Services/ICheckInTimerService.csCore/Resgrid.Model/Services/IDepartmentSettingsService.csCore/Resgrid.Services/CheckInTimerService.csCore/Resgrid.Services/DepartmentSettingsService.csCore/Resgrid.Services/ServicesModule.csProviders/Resgrid.Providers.Migrations/Migrations/M0056_AddingCheckInTimers.csProviders/Resgrid.Providers.MigrationsPg/Migrations/M0056_AddingCheckInTimersPg.csRepositories/Resgrid.Repositories.DataRepository/CheckInRecordRepository.csRepositories/Resgrid.Repositories.DataRepository/CheckInTimerConfigRepository.csRepositories/Resgrid.Repositories.DataRepository/CheckInTimerOverrideRepository.csRepositories/Resgrid.Repositories.DataRepository/Configs/SqlConfiguration.csRepositories/Resgrid.Repositories.DataRepository/Modules/ApiDataModule.csRepositories/Resgrid.Repositories.DataRepository/Modules/DataModule.csRepositories/Resgrid.Repositories.DataRepository/Modules/NonWebDataModule.csRepositories/Resgrid.Repositories.DataRepository/Modules/TestingDataModule.csRepositories/Resgrid.Repositories.DataRepository/Queries/CheckIns/SelectCheckInRecordsByCallIdQuery.csRepositories/Resgrid.Repositories.DataRepository/Queries/CheckIns/SelectCheckInRecordsByDepartmentIdAndDateRangeQuery.csRepositories/Resgrid.Repositories.DataRepository/Queries/CheckIns/SelectCheckInTimerConfigByDepartmentAndTargetQuery.csRepositories/Resgrid.Repositories.DataRepository/Queries/CheckIns/SelectCheckInTimerConfigsByDepartmentIdQuery.csRepositories/Resgrid.Repositories.DataRepository/Queries/CheckIns/SelectCheckInTimerOverridesByDepartmentIdQuery.csRepositories/Resgrid.Repositories.DataRepository/Queries/CheckIns/SelectLastCheckInForUnitOnCallQuery.csRepositories/Resgrid.Repositories.DataRepository/Queries/CheckIns/SelectLastCheckInForUserOnCallQuery.csRepositories/Resgrid.Repositories.DataRepository/Queries/CheckIns/SelectMatchingCheckInTimerOverridesQuery.csRepositories/Resgrid.Repositories.DataRepository/Servers/PostgreSql/PostgreSqlConfiguration.csRepositories/Resgrid.Repositories.DataRepository/Servers/SqlServer/SqlServerConfiguration.csTests/Resgrid.Tests/Services/CheckInTimerServiceTests.csWeb/Resgrid.Web.Services/Controllers/v4/CallsController.csWeb/Resgrid.Web.Services/Controllers/v4/CheckInTimersController.csWeb/Resgrid.Web.Services/Hubs/EventingHub.csWeb/Resgrid.Web.Services/Models/v4/Calls/CallResult.csWeb/Resgrid.Web.Services/Models/v4/Calls/NewCallInput.csWeb/Resgrid.Web.Services/Models/v4/CheckInTimers/CheckInTimerModels.csWeb/Resgrid.Web.Services/Resgrid.Web.Services.xmlWeb/Resgrid.Web/Areas/User/Controllers/DepartmentController.csWeb/Resgrid.Web/Areas/User/Controllers/DispatchController.csWeb/Resgrid.Web/Areas/User/Models/Departments/DispatchSettingsView.csWeb/Resgrid.Web/Areas/User/Models/Dispatch/CallExportView.csWeb/Resgrid.Web/Areas/User/Models/Dispatch/PerformCheckInInput.csWeb/Resgrid.Web/Areas/User/Views/Department/DispatchSettings.cshtmlWeb/Resgrid.Web/Areas/User/Views/Department/Settings.cshtmlWeb/Resgrid.Web/Areas/User/Views/Dispatch/CallExport.cshtmlWeb/Resgrid.Web/Areas/User/Views/Dispatch/NewCall.cshtmlWeb/Resgrid.Web/Areas/User/Views/Dispatch/UpdateCall.cshtmlWeb/Resgrid.Web/Areas/User/Views/Dispatch/ViewCall.cshtmlWeb/Resgrid.Web/Areas/User/Views/Templates/Edit.cshtmlWeb/Resgrid.Web/Areas/User/Views/Templates/New.cshtmlWeb/Resgrid.Web/wwwroot/js/app/internal/dispatch/resgrid.dispatch.checkin-timers.jsWeb/Resgrid.Web/wwwroot/js/app/internal/dispatch/resgrid.dispatch.newcall.js
| /Web/Resgrid.Web/wwwroot/lib | ||
| .dual-graph/ | ||
| .claude/settings.local.json | ||
| .claude/settings.local.json |
There was a problem hiding this comment.
Remove duplicate .gitignore entry.
The entry .claude/settings.local.json already exists at line 275. This duplicate adds no value and should be removed to keep the configuration clean.
🧹 Proposed fix
-.claude/settings.local.json📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .claude/settings.local.json |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.gitignore at line 276, Remove the duplicate `.claude/settings.local.json`
entry from .gitignore by deleting the redundant line that repeats the same
pattern; look for the string ".claude/settings.local.json" (it appears twice)
and remove the second occurrence so the file contains only a single entry for
that path.
| public int DurationMinutes { get; set; } | ||
|
|
||
| public int WarningThresholdMinutes { get; set; } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether timer invariants are enforced in service logic and migrations.
# 1) Find check-in timer service implementations.
fd -i 'CheckInTimerService.cs' | xargs -r rg -nP 'DurationMinutes|WarningThresholdMinutes|<=\s*0|<\s*0|>\s*.*DurationMinutes|throw|Validation|Argument'
# 2) Find migrations creating CheckInTimerConfigs and inspect constraints.
fd -i 'M0056*CheckInTimers*.cs' | xargs -r rg -nP 'CheckInTimerConfigs|DurationMinutes|WarningThresholdMinutes|CHECK|constraint|Unique|Index'
# 3) Broader search for DB check constraints on these columns.
rg -nP 'CheckInTimerConfigs|DurationMinutes|WarningThresholdMinutes|CHECK\s*\('Repository: Resgrid/Core
Length of output: 14477
Add validation to enforce timer invariant constraints at service and database boundaries.
Invalid timer configurations can persist and cause broken timer behavior. Currently, neither the service layer nor the database schema enforce constraints: DurationMinutes must be > 0, WarningThresholdMinutes must be ≥ 0, and WarningThresholdMinutes must be ≤ DurationMinutes. The service logic at CheckInTimerService (line 212) performs unchecked arithmetic (DurationMinutes + WarningThresholdMinutes), and the migration (M0056_AddingCheckInTimers.cs) defines columns as Int32.NotNullable() without CHECK constraints.
Add explicit validation in the controller/service before persistence and add database-level CHECK constraints in the migration to prevent invalid state.
| public async Task<CheckInTimerConfig> SaveTimerConfigAsync(CheckInTimerConfig config, CancellationToken cancellationToken = default) | ||
| { | ||
| if (string.IsNullOrWhiteSpace(config.CheckInTimerConfigId)) | ||
| config.CreatedOn = DateTime.UtcNow; | ||
| else | ||
| config.UpdatedOn = DateTime.UtcNow; | ||
|
|
||
| return await _configRepository.SaveOrUpdateAsync(config, cancellationToken); | ||
| } | ||
|
|
||
| public async Task<bool> DeleteTimerConfigAsync(string configId, CancellationToken cancellationToken = default) | ||
| { | ||
| var config = await _configRepository.GetByIdAsync(configId); | ||
| if (config == null) | ||
| return false; | ||
|
|
||
| return await _configRepository.DeleteAsync(config, cancellationToken); |
There was a problem hiding this comment.
Verify ownership before updating or deleting an existing timer record.
These paths trust the caller-supplied config/override ID. If a request submits an ID from another department, the service will update or delete that row without checking ownership, and both the MVC and v4 API controllers flow through here. Load the existing entity first and reject when its DepartmentId differs from the caller’s department before persisting or deleting.
Also applies to: 65-81
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Core/Resgrid.Services/CheckInTimerService.cs` around lines 36 - 52, When
updating or deleting timers, first verify the existing entity's DepartmentId
matches the caller's department before persisting or removing it: in
SaveTimerConfigAsync, when config.CheckInTimerConfigId is present, load the
existing entity via _configRepository.GetByIdAsync(existingId) and compare
existing.DepartmentId to the caller's department (either config.DepartmentId or
a new callerDepartmentId parameter); if they differ, reject (throw
UnauthorizedAccessException or return an error) instead of saving; similarly in
DeleteTimerConfigAsync, after loading config from
_configRepository.GetByIdAsync(configId) verify config.DepartmentId matches the
caller's department and return false or throw when it does not before calling
_configRepository.DeleteAsync.
| public async Task<List<ResolvedCheckInTimer>> ResolveAllTimersForCallAsync(Call call) | ||
| { | ||
| if (call == null || !call.CheckInTimersEnabled) | ||
| return new List<ResolvedCheckInTimer>(); | ||
|
|
||
| var defaults = await _configRepository.GetByDepartmentIdAsync(call.DepartmentId); | ||
| var defaultList = defaults?.Where(c => c.IsEnabled).ToList() ?? new List<CheckInTimerConfig>(); | ||
|
|
||
| // Parse call type as int for override matching | ||
| int? callTypeId = null; | ||
| if (!string.IsNullOrWhiteSpace(call.Type) && int.TryParse(call.Type, out int parsedType)) | ||
| callTypeId = parsedType; | ||
|
|
||
| var overrides = await _overrideRepository.GetMatchingOverridesAsync(call.DepartmentId, callTypeId, call.Priority); | ||
| var overrideList = overrides?.ToList() ?? new List<CheckInTimerOverride>(); | ||
|
|
||
| var resolved = new Dictionary<string, ResolvedCheckInTimer>(); | ||
|
|
||
| // First, populate from defaults | ||
| foreach (var def in defaultList) | ||
| { | ||
| var key = $"{def.TimerTargetType}_{def.UnitTypeId}"; | ||
| resolved[key] = new ResolvedCheckInTimer | ||
| { | ||
| TargetType = def.TimerTargetType, | ||
| UnitTypeId = def.UnitTypeId, | ||
| DurationMinutes = def.DurationMinutes, | ||
| WarningThresholdMinutes = def.WarningThresholdMinutes, | ||
| IsFromOverride = false | ||
| }; | ||
| } | ||
|
|
||
| // Then, apply overrides with scoring: type+priority=3, type-only=2, priority-only=1 | ||
| var scoredOverrides = overrideList | ||
| .Select(o => new | ||
| { | ||
| Override = o, | ||
| Score = (o.CallTypeId.HasValue && o.CallPriority.HasValue) ? 3 | ||
| : o.CallTypeId.HasValue ? 2 | ||
| : o.CallPriority.HasValue ? 1 | ||
| : 0 | ||
| }) | ||
| .OrderByDescending(x => x.Score) | ||
| .ToList(); | ||
|
|
||
| foreach (var scored in scoredOverrides) | ||
| { | ||
| var o = scored.Override; | ||
| var key = $"{o.TimerTargetType}_{o.UnitTypeId}"; | ||
|
|
||
| // Only apply if this is the best scoring override for this key | ||
| if (!resolved.ContainsKey(key) || !resolved[key].IsFromOverride) | ||
| { | ||
| resolved[key] = new ResolvedCheckInTimer | ||
| { | ||
| TargetType = o.TimerTargetType, | ||
| UnitTypeId = o.UnitTypeId, | ||
| DurationMinutes = o.DurationMinutes, | ||
| WarningThresholdMinutes = o.WarningThresholdMinutes, | ||
| IsFromOverride = true | ||
| }; | ||
| } | ||
| } | ||
|
|
||
| return resolved.Values.ToList(); |
There was a problem hiding this comment.
Status calculation collapses unrelated timers together.
ResolveAllTimersForCallAsync only produces one entry per TimerTargetType/UnitTypeId and never carries a concrete target identity through the pipeline, then GetActiveTimerStatusesForCallAsync selects the latest record using only CheckInType. That lets one unit/person check-in reset every timer with the same target type, and Line 222 currently reports UnitTypeId as UnitId. The resolution/status path needs to resolve concrete call targets and match check-ins on that same entity key.
Also applies to: 198-223
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Core/Resgrid.Services/CheckInTimerService.cs` around lines 88 - 152,
ResolveAllTimersForCallAsync collapses different concrete targets into a single
TimerTargetType/UnitTypeId bucket; change the resolution to carry a concrete
target identity through and use that same identity when computing statuses.
Specifically: add a TargetId (or ConcreteTargetId) property to
ResolvedCheckInTimer and populate it when building defaults and when applying
overrides in ResolveAllTimersForCallAsync (use the config/override field that
identifies the specific unit/person/team), change the dictionary key to include
TargetId (e.g., $"{def.TimerTargetType}_{def.UnitTypeId}_{def.TargetId}"),
preserve IsFromOverride, then update GetActiveTimerStatusesForCallAsync to match
check-ins using the identical composite key (Timer/CheckIn type + UnitTypeId +
TargetId) when selecting latest records; also fix the reporting bug by returning
UnitId where Line 222 currently emits UnitTypeId.
| var overrides = await _overrideRepository.GetMatchingOverridesAsync(call.DepartmentId, callTypeId, call.Priority); | ||
| var overrideList = overrides?.ToList() ?? new List<CheckInTimerOverride>(); | ||
|
|
||
| var resolved = new Dictionary<string, ResolvedCheckInTimer>(); | ||
|
|
||
| // First, populate from defaults | ||
| foreach (var def in defaultList) | ||
| { | ||
| var key = $"{def.TimerTargetType}_{def.UnitTypeId}"; | ||
| resolved[key] = new ResolvedCheckInTimer | ||
| { | ||
| TargetType = def.TimerTargetType, | ||
| UnitTypeId = def.UnitTypeId, | ||
| DurationMinutes = def.DurationMinutes, | ||
| WarningThresholdMinutes = def.WarningThresholdMinutes, | ||
| IsFromOverride = false | ||
| }; | ||
| } | ||
|
|
||
| // Then, apply overrides with scoring: type+priority=3, type-only=2, priority-only=1 | ||
| var scoredOverrides = overrideList | ||
| .Select(o => new | ||
| { | ||
| Override = o, | ||
| Score = (o.CallTypeId.HasValue && o.CallPriority.HasValue) ? 3 | ||
| : o.CallTypeId.HasValue ? 2 | ||
| : o.CallPriority.HasValue ? 1 | ||
| : 0 | ||
| }) | ||
| .OrderByDescending(x => x.Score) | ||
| .ToList(); | ||
|
|
||
| foreach (var scored in scoredOverrides) | ||
| { | ||
| var o = scored.Override; | ||
| var key = $"{o.TimerTargetType}_{o.UnitTypeId}"; | ||
|
|
||
| // Only apply if this is the best scoring override for this key | ||
| if (!resolved.ContainsKey(key) || !resolved[key].IsFromOverride) | ||
| { | ||
| resolved[key] = new ResolvedCheckInTimer | ||
| { | ||
| TargetType = o.TimerTargetType, | ||
| UnitTypeId = o.UnitTypeId, | ||
| DurationMinutes = o.DurationMinutes, | ||
| WarningThresholdMinutes = o.WarningThresholdMinutes, | ||
| IsFromOverride = true | ||
| }; | ||
| } |
There was a problem hiding this comment.
Disabled overrides still resolve as active timers.
Defaults are filtered by IsEnabled, but matching overrides are always materialized into resolved regardless of o.IsEnabled. A disabled override therefore cannot suppress a default timer and will still show up in resolved timers/statuses.
🛠️ Proposed fix
foreach (var scored in scoredOverrides)
{
var o = scored.Override;
var key = $"{o.TimerTargetType}_{o.UnitTypeId}";
- // Only apply if this is the best scoring override for this key
- if (!resolved.ContainsKey(key) || !resolved[key].IsFromOverride)
- {
- resolved[key] = new ResolvedCheckInTimer
- {
- TargetType = o.TimerTargetType,
- UnitTypeId = o.UnitTypeId,
- DurationMinutes = o.DurationMinutes,
- WarningThresholdMinutes = o.WarningThresholdMinutes,
- IsFromOverride = true
- };
- }
+ if (resolved.ContainsKey(key) && resolved[key].IsFromOverride)
+ continue;
+
+ if (!o.IsEnabled)
+ {
+ resolved.Remove(key);
+ continue;
+ }
+
+ resolved[key] = new ResolvedCheckInTimer
+ {
+ TargetType = o.TimerTargetType,
+ UnitTypeId = o.UnitTypeId,
+ DurationMinutes = o.DurationMinutes,
+ WarningThresholdMinutes = o.WarningThresholdMinutes,
+ IsFromOverride = true
+ };
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Core/Resgrid.Services/CheckInTimerService.cs` around lines 101 - 149, The
override application currently materializes all overrides into resolved
regardless of o.IsEnabled; update the loop that processes scoredOverrides in
CheckInTimerService (the block using scoredOverrides, o, key, and resolved) to
skip any override where o.IsEnabled is false (i.e., continue to next scored
item) so disabled overrides are not applied and therefore cannot suppress or
replace defaults; ensure you only construct a ResolvedCheckInTimer and set
IsFromOverride = true when o.IsEnabled == true.
| @using Microsoft.Extensions.Localization | ||
| @inject IStringLocalizer<Resgrid.Localization.Areas.User.Dispatch.Call> localizer |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n Web/Resgrid.Web/Areas/User/Views/Dispatch/CallExport.cshtml | head -30Repository: Resgrid/Core
Length of output: 1250
🌐 Web query:
Razor @Inject duplicate property same name compilation error ASP.NET Core
💡 Result:
The compilation error "duplicate property same name" in ASP.NET Core Razor (typically Blazor components) when using @inject occurs when the @inject directive generates a property that conflicts with an existing property or field declaration of the same name in the same component class. @inject Type PropertyName generates C# code equivalent to: [Inject] public Type PropertyName { get; set; } = default!; If you manually declare a field or property with the same name (e.g., private Type PropertyName; in @code {}), the Razor compiler produces duplicate members, causing CS0542: 'Class': member names cannot be the same as their enclosing type or CS0111/CS0102 duplicate member errors. Solution: Rename either the @inject property name or the manual declaration to avoid name collision. Use distinct names like MyServiceInstance for injected properties. Common causes: - Manual field in @code block matching @inject name (11,14,19,21). - @inject in _Imports.razor combined with per-component @inject or manual prop (11). - Inherited base classes or partial class mismatches declaring same prop (28). Official docs confirm @inject creates backing properties; avoid redeclaration (11,22). To debug: Enable true in .csproj to inspect generated C# (1).
Citations:
- 1: https://stackoverflow.com/questions/79753134/why-does-this-razor-code-compile-even-though-i-reuse-the-same-loop-variable-name
- 2: https://learn.microsoft.com/en-us/aspnet/core/blazor/fundamentals/dependency-injection?view=aspnetcore-7.0
- 3: https://stackoverflow.com/questions/79771871/avoiding-null-warnings-in-blazor-default-or-required-per-inject-service-depe
- 4: Blazor compiler error if there are two field definitions in an @code block dotnet/razor#8593
- 5: Cannot use properties in razor pages. dotnet/razor#8263
- 6: https://stackoverflow.com/questions/65629065/c-sharp-asp-net-core-inject-attribute-usage-for-dependency-injection
- 7: https://stackoverflow.com/questions/74122958/blazor-page-partial-declarations-of-type-must-not-specify-different-base-class
Remove the duplicate @inject for localizer on line 6.
Lines 6 and 8 both inject the same localizer property. The Razor compiler generates duplicate property declarations from both @inject directives, causing a CS0111 compilation error.
Proposed fix
`@using` Resgrid.Web
`@using` Resgrid.Web.Helpers
`@using` Microsoft.Extensions.Localization
-@inject IStringLocalizer<Resgrid.Localization.Areas.User.Dispatch.Call> localizer
`@model` Resgrid.Web.Areas.User.Models.Dispatch.CallExportView
`@inject` IStringLocalizer<Resgrid.Localization.Areas.User.Dispatch.Call> localizer🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Web/Resgrid.Web/Areas/User/Views/Dispatch/CallExport.cshtml` around lines 5 -
6, Remove the duplicate Razor injection: there are two `@inject` declarations
creating the same property localizer of type
IStringLocalizer<Resgrid.Localization.Areas.User.Dispatch.Call>; keep a single
`@inject` IStringLocalizer<Resgrid.Localization.Areas.User.Dispatch.Call>
localizer and delete the other so the compiler does not generate duplicate
property declarations (search for the duplicate `@inject` lines referencing
"localizer" to locate them).
| <label> | ||
| <input type="checkbox" name="Call.CheckInTimersEnabled" value="true"> @localizer["EnableCheckInTimers"] | ||
| </label> |
There was a problem hiding this comment.
Bind the checkbox to the model to preserve state and reliable posting.
The current raw checkbox won’t automatically preserve Call.CheckInTimersEnabled on postback/re-render. Use asp-for so MVC handles checked state and hidden false submission consistently.
Proposed fix
<div class="checkbox">
<label>
- <input type="checkbox" name="Call.CheckInTimersEnabled" value="true"> `@localizer`["EnableCheckInTimers"]
+ <input asp-for="Call.CheckInTimersEnabled" type="checkbox" /> `@localizer`["EnableCheckInTimers"]
</label>
</div>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <label> | |
| <input type="checkbox" name="Call.CheckInTimersEnabled" value="true"> @localizer["EnableCheckInTimers"] | |
| </label> | |
| <div class="checkbox"> | |
| <label> | |
| <input asp-for="Call.CheckInTimersEnabled" type="checkbox" /> `@localizer`["EnableCheckInTimers"] | |
| </label> | |
| </div> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Web/Resgrid.Web/Areas/User/Views/Dispatch/NewCall.cshtml` around lines 410 -
412, Replace the raw checkbox input with the MVC tag helper bound to the model
property (use asp-for="Call.CheckInTimersEnabled") so ASP.NET Core will render
the checked state on re-render and include the hidden false input for reliable
posting; keep the surrounding label and the `@localizer`["EnableCheckInTimers"]
text but change the <input> to use asp-for on the Call.CheckInTimersEnabled
property to ensure model binding preserves state.
| @if (Model.Call.CheckInTimersEnabled) | ||
| { | ||
| <div class="ibox float-e-margins"> | ||
| <div class="ibox-title"> | ||
| <h5>@localizer["CheckInTimersHeader"]</h5> | ||
| </div> | ||
| <div class="ibox-content"> | ||
| @if (Model.Call.State == (int)CallStates.Active) | ||
| { | ||
| <div id="checkInTimersPanel"> | ||
| <table class="table table-striped table-hover table-condensed" id="checkInTimersTable"> | ||
| <thead> | ||
| <tr> | ||
| <th>@localizer["CheckInTimerTarget"]</th> | ||
| <th>@localizer["CheckInTimerTimeRemaining"]</th> | ||
| <th>@localizer["CheckInTimerStatus"]</th> | ||
| <th></th> | ||
| </tr> | ||
| </thead> | ||
| <tbody id="checkInTimersBody"> | ||
| <tr><td colspan="4">...</td></tr> | ||
| </tbody> | ||
| </table> | ||
| </div> | ||
| <div class="m-t-sm"> | ||
| <a href="#checkInHistory" data-toggle="collapse" class="btn btn-xs btn-default">@localizer["CheckInTimerShowHistory"]</a> | ||
| <div id="checkInHistory" class="collapse m-t-sm"> | ||
| <table class="table table-condensed" id="checkInHistoryTable"> | ||
| <thead> | ||
| <tr> | ||
| <th>@localizer["CheckInTimerHistoryTimestamp"]</th> | ||
| <th>@localizer["CheckInTimerHistoryWho"]</th> | ||
| <th>@localizer["CheckInTimerHistoryType"]</th> | ||
| <th>@localizer["CheckInTimerHistoryNote"]</th> | ||
| </tr> | ||
| </thead> | ||
| <tbody id="checkInHistoryBody"> | ||
| </tbody> | ||
| </table> | ||
| </div> | ||
| </div> | ||
| } | ||
| else | ||
| { | ||
| <table class="table table-condensed" id="checkInHistoryTable"> | ||
| <thead> | ||
| <tr> | ||
| <th>@localizer["CheckInTimerHistoryTimestamp"]</th> | ||
| <th>@localizer["CheckInTimerHistoryWho"]</th> | ||
| <th>@localizer["CheckInTimerHistoryType"]</th> | ||
| <th>@localizer["CheckInTimerHistoryNote"]</th> | ||
| </tr> | ||
| </thead> | ||
| <tbody id="checkInHistoryBody"> | ||
| </tbody> | ||
| </table> | ||
| } | ||
| </div> | ||
| </div> | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
js=$(fd -g 'resgrid.dispatch.checkin-timers.js' | head -n 1)
layout=$(fd -g '_UserLayout.cshtml' | head -n 1)
controller='Web/Resgrid.Web/Areas/User/Controllers/DispatchController.cs'
echo "# JS initializer: $js"
sed -n '1,260p' "$js"
echo
echo "# Guard clauses / token handling in JS"
rg -n -C3 'callState|checkInModal|checkInSubmitBtn|checkInTimersBody|checkInTimersPanel|RequestVerificationToken|__RequestVerificationToken' "$js"
echo
echo "# Antiforgery setup in layout"
rg -n -C3 'RequestVerificationToken|__RequestVerificationToken|AntiForgery' "$layout"
echo
echo "# Check-in endpoints / antiforgery on controller"
rg -n -C3 'CheckIn|ValidateAntiForgeryToken|IgnoreAntiforgeryToken|AutoValidateAntiforgeryToken' "$controller"Repository: Resgrid/Core
Length of output: 7700
🏁 Script executed:
#!/bin/bash
# Check PerformCheckIn action in controller
echo "=== PerformCheckIn action in DispatchController ==="
rg -n -A 10 'PerformCheckIn' 'Web/Resgrid.Web/Areas/User/Controllers/DispatchController.cs' | head -n 50
echo
echo "=== All antiforgery attributes in DispatchController ==="
rg -n 'ValidateAntiForgeryToken|IgnoreAntiforgeryToken|AutoValidateAntiforgeryToken' 'Web/Resgrid.Web/Areas/User/Controllers/DispatchController.cs'
echo
echo "=== Check ViewCall.cshtml for antiforgery token ==="
rg -n 'RequestVerificationToken|AntiForgery' 'Web/Resgrid.Web/Areas/User/Views/Dispatch/ViewCall.cshtml'
echo
echo "=== Check if modal form includes hidden token fields ==="
rg -n -B5 -A5 'checkInModal\|checkInTargetType\|checkInUnitId\|checkInNote' 'Web/Resgrid.Web/Areas/User/Views/Dispatch/ViewCall.cshtml' | head -n 100Repository: Resgrid/Core
Length of output: 1309
🏁 Script executed:
#!/bin/bash
# Check what actions have ValidateAntiForgeryToken at those lines
echo "=== ValidateAntiForgeryToken attributes context ==="
rg -n -B2 'ValidateAntiForgeryToken' 'Web/Resgrid.Web/Areas/User/Controllers/DispatchController.cs' | head -n 30
echo
echo "=== Confirm PerformCheckIn has no antiforgery attribute (full method signature) ==="
sed -n '1750,1780p' 'Web/Resgrid.Web/Areas/User/Controllers/DispatchController.cs'
echo
echo "=== Check other POST endpoints in same controller for pattern ==="
rg -n -B3 'HttpPost|HttpPut' 'Web/Resgrid.Web/Areas/User/Controllers/DispatchController.cs' | head -n 60Repository: Resgrid/Core
Length of output: 2002
🏁 Script executed:
#!/bin/bash
# Check if there's a global antiforgery filter or controller-level attribute
echo "=== Check DispatchController class declaration and attributes ==="
sed -n '1,100p' 'Web/Resgrid.Web/Areas/User/Controllers/DispatchController.cs' | grep -n -A10 'public class DispatchController'
echo
echo "=== Check Startup/Program configuration for global antiforgery ==="
find . -name 'Startup.cs' -o -name 'Program.cs' | head -n 5 | while read f; do
echo "=== $f ==="
rg -n 'AutoValidateAntiforgeryToken|ValidateAntiForgeryToken' "$f" | head -n 20
done
echo
echo "=== Check if PerformCheckIn needs token - verify it handles JSON POST ==="
rg -n -B5 'PerformCheckInInput' 'Web/Resgrid.Web/Areas/User/Controllers/DispatchController.cs' | head -n 20Repository: Resgrid/Core
Length of output: 1311
🏁 Script executed:
#!/bin/bash
# Find the main Startup or Program file for the web project
echo "=== Find Startup/Program for Web project ==="
fd -g 'Startup.cs' 'Web/Resgrid.Web/' | head -n 3
echo
echo "=== Check for AutoValidateAntiforgeryToken in web project ==="
rg -n 'AutoValidateAntiforgeryToken|antiforgery' 'Web/Resgrid.Web/' --max-count=20
echo
echo "=== Verify PerformCheckInInput is JSON-bound and no token field ==="
rg -n -A15 'class PerformCheckInInput' 'Web/Resgrid.Web/'Repository: Resgrid/Core
Length of output: 1206
🏁 Script executed:
#!/bin/bash
# Confirm Startup.cs antiforgery configuration
echo "=== Antiforgery setup in Startup.cs ==="
rg -n -B5 -A5 'antiforgery|Antiforgery' 'Web/Resgrid.Web/Startup.cs'
echo
echo "=== Check if there's a base controller with antiforgery handling ==="
rg -n -B3 -A10 'class SecureBaseController' 'Web/Resgrid.Web/'Repository: Resgrid/Core
Length of output: 1348
The guard clause correctly short-circuits on inactive calls, but the POST endpoint lacks antiforgery protection.
The JavaScript properly checks callState === 0 before binding timers and polling (line 9 of resgrid.dispatch.checkin-timers.js), so the timer panel will not execute when the call is inactive—verified.
However, the PerformCheckIn POST request (line 187 in the JS) sends JSON without an antiforgery token, and the controller action (line 1751 in DispatchController.cs) has no [ValidateAntiForgeryToken] attribute. While this follows the ASP.NET Core MVC convention of exempting [FromBody] JSON endpoints from CSRF validation, it still represents a security gap: the endpoint accepts state-changing operations without CSRF protection. Consider either adding an antiforgery check to the controller action or documenting why this endpoint is intentionally exempt.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Web/Resgrid.Web/Areas/User/Views/Dispatch/ViewCall.cshtml` around lines 774 -
833, The PerformCheckIn POST currently sends JSON from
resgrid.dispatch.checkin-timers.js without an antiforgery token and the
corresponding controller action in DispatchController (PerformCheckIn) lacks
[ValidateAntiForgeryToken]; fix by adding antiforgery protection: update the
client (resgrid.dispatch.checkin-timers.js -> PerformCheckIn request) to include
the antiforgery token (e.g., read the token from the page meta/cookie/header)
and modify the server action (DispatchController.PerformCheckIn) to validate the
token—either add [ValidateAntiForgeryToken] and accept the token in a header (or
change to a form-bound DTO) or call IAntiforgery.ValidateRequestAsync inside the
action if you must keep JSON binding; ensure token name used on client matches
what the server expects.
| <div class="form-group"> | ||
| <label class="col-sm-2 control-label"> | ||
| @localizer["EnableCheckInTimersLabel"] | ||
| </label> | ||
| <div class="col-sm-10"> | ||
| <div class="col-md-10"> | ||
| <input type="checkbox" name="Template.CheckInTimersEnabled" value="true" @(Model.Template.CheckInTimersEnabled == true ? "checked" : "") /> | ||
| </div> |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In ASP.NET Core MVC, when posting a form, does an unchecked submit any value, and is a hidden input required for reliablefalse binding?
💡 Result:
No, an unchecked does not submit any value to the server. This is standard HTML form behavior: checkboxes only include their name-value pair in the form data if checked. For reliable false binding in ASP.NET Core MVC model binding to a bool property: - Use the asp-for tag helper: . This automatically renders a checkbox (value="true") + hidden input (value="false", same name). On post: - Checked: submits true,false; binder takes first (true). - Unchecked: submits only false; binder sets false. - Manual HTML requires explicit hidden input before checkbox: . This ensures consistent true/false binding without custom logic. Raw HTML checkbox alone binds true if checked, default false if absent (reliable for bool defaults), but tag helper guarantees explicit false value.
Citations:
- 1: https://stackoverflow.com/questions/1809494/post-unchecked-html-checkboxes
- 2: https://stackoverflow.com/questions/77181652/checkbox-values-not-returning-to-controller-asp-net-core-7-mvc
- 3: https://learn.microsoft.com/en-us/answers/questions/1179859/dont-see-the-value-of-the-checkboxes-in-the-http-p
- 4: https://stackoverflow.com/questions/57079165/why-do-my-checkboxes-not-pass-their-value-to-my-controller-in-asp-net-core/57087930
- 5: https://stackoverflow.com/questions/50513690/asp-net-core-tag-helper-checkbox-does-not-save-unchecked-value/50514167
- 6: https://www.learnrazorpages.com/razor-pages/forms/checkboxes
- 7: https://stackoverflow.com/questions/50513690/asp-net-core-tag-helper-checkbox-does-not-save-unchecked-value
- 8: https://stackoverflow.com/questions/57079165/why-do-my-checkboxes-not-pass-their-value-to-my-controller-in-asp-net-core
🏁 Script executed:
# Check if the file exists and read the relevant section
if [ -f "Web/Resgrid.Web/Areas/User/Views/Templates/Edit.cshtml" ]; then
echo "=== File exists ==="
wc -l "Web/Resgrid.Web/Areas/User/Views/Templates/Edit.cshtml"
echo ""
echo "=== Lines 96-110 ==="
sed -n '96,110p' "Web/Resgrid.Web/Areas/User/Views/Templates/Edit.cshtml" | cat -n
else
echo "File not found at that path"
fd -t f "Edit.cshtml" | head -20
fiRepository: Resgrid/Core
Length of output: 1301
🏁 Script executed:
# Search for other manual checkbox instances in the same file
echo "=== All checkboxes in the file ==="
rg -n 'type="checkbox"' "Web/Resgrid.Web/Areas/User/Views/Templates/Edit.cshtml" -A 2 -B 1Repository: Resgrid/Core
Length of output: 412
🏁 Script executed:
# Find the controller handling this Edit action
fd -t f "TemplatesController.cs" Web/Repository: Resgrid/Core
Length of output: 180
🏁 Script executed:
# Search for the Edit action in Templates controller
rg -n "public.*Edit" --type cs "Web/Resgrid.Web/Areas/User/Controllers/" | grep -i templateRepository: Resgrid/Core
Length of output: 636
🏁 Script executed:
# Examine the POST Edit action to see how binding and model update work
sed -n '123,180p' "Web/Resgrid.Web/Areas/User/Controllers/TemplatesController.cs" | cat -nRepository: Resgrid/Core
Length of output: 2236
Use @Html.CheckBoxFor() to reliably persist checkbox state.
Line 102's manual checkbox markup does not emit a hidden input, which is the standard ASP.NET Core approach for ensuring unchecked values are posted. While default model binding typically handles missing bool values as false, relying on this implicit behavior is fragile. An edit flow that preserves prior state could leave stale values if form data is incomplete or custom binding logic exists. Use @Html.CheckBoxFor(m => m.Template.CheckInTimersEnabled) to emit the proper hidden fallback input that guarantees both true and false states are reliably captured.
Suggested change
- <input type="checkbox" name="Template.CheckInTimersEnabled" value="true" @(Model.Template.CheckInTimersEnabled == true ? "checked" : "") />
+ `@Html.CheckBoxFor`(m => m.Template.CheckInTimersEnabled)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Web/Resgrid.Web/Areas/User/Views/Templates/Edit.cshtml` around lines 96 -
103, Replace the manual checkbox input for Model.Template.CheckInTimersEnabled
with the helper that emits the hidden fallback input so false values are posted;
locate the checkbox using the symbol Model.Template.CheckInTimersEnabled and
change the markup to use `@Html.CheckBoxFor`(m => m.Template.CheckInTimersEnabled)
(keeping the surrounding label and layout) to ensure proper model binding of
both true and false.
| if (data.CheckInTimersEnabled === true) { | ||
| $('input[name="Call.CheckInTimersEnabled"]').prop('checked', true); | ||
| } else if (data.CheckInTimersEnabled === false) { | ||
| $('input[name="Call.CheckInTimersEnabled"]').prop('checked', false); | ||
| } |
There was a problem hiding this comment.
Handle missing CheckInTimersEnabled deterministically to avoid stale checkbox state.
If data.CheckInTimersEnabled is null/undefined, current logic keeps the previous checkbox state. Set it in one expression so template switches are deterministic.
Suggested change
- if (data.CheckInTimersEnabled === true) {
- $('input[name="Call.CheckInTimersEnabled"]').prop('checked', true);
- } else if (data.CheckInTimersEnabled === false) {
- $('input[name="Call.CheckInTimersEnabled"]').prop('checked', false);
- }
+ $('input[name="Call.CheckInTimersEnabled"]').prop('checked', data.CheckInTimersEnabled === true);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Web/Resgrid.Web/wwwroot/js/app/internal/dispatch/resgrid.dispatch.newcall.js`
around lines 322 - 326, The checkbox is left in a stale state when
data.CheckInTimersEnabled is null/undefined; replace the if/else block with a
single deterministic assignment for $('input[name="Call.CheckInTimersEnabled"]')
using a boolean conversion of data.CheckInTimersEnabled (e.g., use
Boolean(data.CheckInTimersEnabled) or !!data.CheckInTimersEnabled) so the
checkbox is explicitly set to checked/unchecked every time; update the code
around the existing input selector and the data.CheckInTimersEnabled reference
accordingly.
1 participant
Summary by CodeRabbit
Release Notes