Conversation
|
Thanks for opening this, but we'd appreciate a little more information. Could you update it with more details? |
This comment has been minimized.
This comment has been minimized.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis change adds command and dispatch access controls, incident chat provisioning and lifecycle handling, automatic-event notification suppression, caller-specific incident view data, paged system-audit retrieval, and shared CORS origin validation. ChangesIncident command and chat access
Automatic event notification handling
Paged system audit retrieval
Shared CORS origin validation
Estimated code review effort: 5 (Critical) | ~90 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant IncidentCommandController
participant CommandAccessService
participant IncidentCommandService
participant ChatChannelService
Client->>IncidentCommandController: request command or board operation
IncidentCommandController->>CommandAccessService: check department access
CommandAccessService-->>IncidentCommandController: authorization result
IncidentCommandController->>IncidentCommandService: process authorized request
IncidentCommandService->>ChatChannelService: backfill incident channels
ChatChannelService-->>IncidentCommandService: channel state
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 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: 15
🧹 Nitpick comments (2)
Core/Resgrid.Services/ChatMessageService.cs (1)
264-291: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winRedundant channel re-fetch across four chat-mutation paths.
IsChannelFrozenAsyncfetches the channel bychatChannelIdto checkIsArchived. Each of its four callers then re-fetches the same channel later, purely to build thePublishEventpayload — one extra avoidable DB round trip per edit, delete, add-reaction, and remove-reaction call.
Core/Resgrid.Services/ChatMessageService.cs#L264-L291: changeIsChannelFrozenAsyncto return the fetchedChatChannel(ornullwhen missing/archived) instead of abool, soEditMessageAsynccan reuse it instead of calling_chatChannelRepository.GetByIdAsync(message.ChatChannelId)again at line 309.Core/Resgrid.Services/ChatMessageService.cs#L326-L331: reuse the channel returned by the updated frozen-check call instead of re-fetching it at line 344 forPublishEvent.Core/Resgrid.Services/ChatMessageService.cs#L367-L369: reuse the channel returned by the updated frozen-check call instead of re-fetching it at line 411 forPublishEvent.Core/Resgrid.Services/ChatMessageService.cs#L423-L425: reuse the channel returned by the updated frozen-check call instead of re-fetching it at line 431 forPublishEvent.🤖 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 `@Core/Resgrid.Services/ChatMessageService.cs` around lines 264 - 291, Eliminate redundant channel lookups in ChatMessageService.cs at lines 264-291, 326-331, 367-369, and 423-425 by changing IsChannelFrozenAsync to return the fetched ChatChannel only when present and not archived, otherwise null. Update EditMessageAsync, the delete path, add-reaction path, and remove-reaction path to retain and reuse that returned channel for PublishEvent instead of calling GetByIdAsync again; preserve fail-closed behavior for missing, blank, or archived channels.Web/Resgrid.Web.Services/Controllers/v4/IncidentCommandController.cs (1)
35-42: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse the required dependency resolution pattern.
Resolve
ICommandAccessServicewithBootstrapper.GetKernel().Resolve<ICommandAccessService>()in the constructor. Do not add constructor injection for this dependency.As per coding guidelines, use
Service Locatorpattern viaBootstrapper.GetKernel().Resolve<T>()to resolve dependencies explicitly in constructors, rather than constructor injection.🤖 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/Controllers/v4/IncidentCommandController.cs` around lines 35 - 42, Update IncidentCommandController’s constructor to remove the ICommandAccessService parameter and resolve it through Bootstrapper.GetKernel().Resolve<ICommandAccessService>() when assigning _commandAccessService; retain constructor injection for the other dependencies.Source: Coding guidelines
🤖 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 `@Core/Resgrid.Model/Services/IActionLogsService.cs`:
- Line 88: Propagate the autoGenerated flag through the single-log path by
adding it to the relevant SetUserActionAsync and
ActionLogsService.SaveActionLogAsync overloads, then assign it to
UserStatusEvent.AutoGenerated. Pass true from
CallDispatchStatusService.ApplyPersonnelStatusesAsync and add a regression test
verifying automated personnel status changes preserve the flag and avoid
outbound notifications.
- Line 88: Propagate the autoGenerated value from SaveActionLogAsync through
both SetUserActionAsync overloads into UserStatusEvent.AutoGenerated, preserving
false only when the argument is omitted. Update SaveAllActionLogsAsync to
document the autoGenerated parameter and ensure its calls pass that value
through the action-log save flow.
In `@Core/Resgrid.Services/ActionLogsService.cs`:
- Around line 348-350: Update the bulk status reset flow around
SaveAllActionLogsAsync so manual department-wide reset operations do not pass
autoGenerated: true and continue producing per-user notifications. Restrict the
autoGenerated flag to scheduled resets, or introduce a separate
notification-suppression parameter while preserving suppression for scheduled
resets.
In `@Core/Resgrid.Services/ChatChannelService.cs`:
- Around line 751-812: The channel provisioning flow in
EnsureIncidentChannelsAsync must rebind reused command, leads, and dispatch
channels to the current command. Update EnsureCommandChannelAsync,
EnsureLeadsChannelAsync, and EnsureDispatchChannelAsync so reused channels set
IncidentCommandId to the new command, clear IsArchived and ArchivedOn, and
invalidate the associated permission cache before returning.
In `@Core/Resgrid.Services/CommandAccessService.cs`:
- Around line 12-18: Update CommandAccessService’s constructor to remove its
injected service parameters and resolve IPermissionsService,
IDepartmentsService, IDepartmentGroupsService, IPersonnelRolesService, and
ICacheProvider via Bootstrapper.GetKernel().Resolve<T>(), passing the resolved
instances to the base constructor.
In `@Core/Resgrid.Services/IncidentCommandService.cs`:
- Line 482: Replace the new ServiceLocator.Current.GetInstance calls in
Core/Resgrid.Services/IncidentCommandService.cs at lines 482, 1075, and
1116-1117 with Bootstrapper.GetKernel().Resolve<T>() for ICommandAccessService,
IChatChannelService, and IChatChannelRepository respectively; preserve the
surrounding command logic.
- Around line 1090-1108: Scope incident nodes and roles to
command.IncidentCommandId throughout IncidentCommandService.cs: in lines
1090-1108 filter before building contacts and calculating isCommandStaff or
isLaneLead; in lines 528-531 and 1058-1059 pass only matching nodes to the
backfill; in lines 1074-1075 filter fallback nodes before
EnsureIncidentChannelsAsync. Preserve the existing command-specific channel and
authorization behavior.
In `@Core/Resgrid.Services/PermissionGateServiceBase.cs`:
- Around line 147-153: Update the permission flow in the relevant method of
PermissionGateServiceBase so GetDepartmentMemberAsync validates department
membership before handling a null permission. Return false when membership is
absent, and only return the default true for a missing permission after
membership has been confirmed; preserve the existing permission evaluation for
non-null permissions.
- Around line 59-75: Update EvaluateAsync and the permission paths used by
CanUseDispatchAsync and CanUseCommandAsync so a missing permission row returns
true only after GetDepartmentMemberAsync confirms department membership. Replace
the direct _cacheProvider GetStringAsync/SetStringAsync calls with
ICacheProvider.RetrieveAsync<string>() and a local fallback function that
evaluates and caches the permission result, preserving cache-outage fallback
behavior.
In
`@Repositories/Resgrid.Repositories.DataRepository/Queries/SystemAudits/SelectSystemAuditsByTypePagedQuery.cs`:
- Line 16: The paged audit queries need a deterministic secondary sort. In
SelectSystemAuditsByTypePagedQuery.cs at lines 16-16 and 18-18, update each
ORDER BY clause to sort by the unique system-audit key after loggedon DESC,
preserving the existing pagination parameters and primary ordering.
In `@Repositories/Resgrid.Repositories.DataRepository/SystemAuditRepository.cs`:
- Around line 136-139: Update the pagination parameter handling around safePage
and safePageSize to enforce a defined maximum page-size constant, clamping valid
pageSize values to that bound while retaining the minimum of 1. Compute the
Offset multiplication using long arithmetic after normalization, and pass the
bounded safePageSize to the database.
In `@Web/Resgrid.Web.Services/Controllers/v4/IncidentCommandController.cs`:
- Around line 49-56: Apply CanCommandAsync as an authorization guard in
ReopenCommand, GetCommandForCall, and SendMessageToCommand before their existing
operation logic, matching EstablishCommand and the board-read routes. Preserve
the current capability-filter checks and return the established unauthorized
response when the command gate denies access.
In `@Web/Resgrid.Web.Services/Filters/RequiresIncidentCapabilityAttribute.cs`:
- Around line 62-70: The RequiresIncidentCapabilityAttribute filter must fail
closed when ICommandAccessService cannot be resolved. Update the commandAccess
handling before CanUseCommandAsync so a null service returns a server error or
denies the request, while preserving the existing unauthorized response when
CanUseCommandAsync returns false.
In `@Web/Resgrid.Web/Areas/User/Views/Security/Index.cshtml`:
- Around line 326-345: Replace the hard-coded labels and descriptions in the
Dispatch App Login and Command App Login rows with appropriate localizer
lookups, and add matching resource keys for each label and description in the
localization resources. Preserve the existing permission controls and localized
PermissionNA/PermissionNoRoles content.
In
`@Web/Resgrid.Web/wwwroot/js/app/internal/security/resgrid.security.permissions.js`:
- Around line 787-801: Change the permission updates in the DispatchAppLogin and
corresponding Command access handlers to use POST requests, and update the
SetPermission and SetPermissionData actions to require anti-forgery validation.
Include the page’s verification token in both AJAX request payloads while
preserving the existing permission type and value submission.
---
Nitpick comments:
In `@Core/Resgrid.Services/ChatMessageService.cs`:
- Around line 264-291: Eliminate redundant channel lookups in
ChatMessageService.cs at lines 264-291, 326-331, 367-369, and 423-425 by
changing IsChannelFrozenAsync to return the fetched ChatChannel only when
present and not archived, otherwise null. Update EditMessageAsync, the delete
path, add-reaction path, and remove-reaction path to retain and reuse that
returned channel for PublishEvent instead of calling GetByIdAsync again;
preserve fail-closed behavior for missing, blank, or archived channels.
In `@Web/Resgrid.Web.Services/Controllers/v4/IncidentCommandController.cs`:
- Around line 35-42: Update IncidentCommandController’s constructor to remove
the ICommandAccessService parameter and resolve it through
Bootstrapper.GetKernel().Resolve<ICommandAccessService>() when assigning
_commandAccessService; retain constructor injection for the other dependencies.
🪄 Autofix
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: a6221ec6-0c4a-4c0d-94bb-cf7ee93d5343
⛔ Files ignored due to path filters (7)
.claude/settings.local.jsonis excluded by!**/.claude/**Tests/Resgrid.Tests/Services/CallDispatchStatusServiceTests.csis excluded by!**/Tests/**Tests/Resgrid.Tests/Services/ChatFrozenChannelTests.csis excluded by!**/Tests/**Tests/Resgrid.Tests/Services/ChatIncidentBackfillTests.csis excluded by!**/Tests/**Tests/Resgrid.Tests/Services/ChatPermissionServiceTests.csis excluded by!**/Tests/**Tests/Resgrid.Tests/Services/DispatchAccessServiceTests.csis excluded by!**/Tests/**Tests/Resgrid.Tests/Web/Services/RequiresIncidentCapabilityFilterTests.csis excluded by!**/Tests/**
📒 Files selected for processing (43)
Core/Resgrid.Model/Chat/ChatEnums.csCore/Resgrid.Model/Events/UnitStatusEvent.csCore/Resgrid.Model/Events/UserStaffingEvent.csCore/Resgrid.Model/Events/UserStatusEvent.csCore/Resgrid.Model/IncidentCommand/IncidentRole.csCore/Resgrid.Model/IncidentCommand/ResourceIncidentView.csCore/Resgrid.Model/PermissionTypes.csCore/Resgrid.Model/Repositories/IChatRepositories.csCore/Resgrid.Model/Repositories/ISystemAuditsRepository.csCore/Resgrid.Model/Services/IActionLogsService.csCore/Resgrid.Model/Services/IChatServices.csCore/Resgrid.Model/Services/ICommandAccessService.csCore/Resgrid.Model/Services/IDispatchAccessService.csCore/Resgrid.Model/Services/IUnitsService.csCore/Resgrid.Model/Services/IUserStateService.csCore/Resgrid.Services/ActionLogsService.csCore/Resgrid.Services/CallDispatchStatusService.csCore/Resgrid.Services/ChatChannelService.csCore/Resgrid.Services/ChatMessageService.csCore/Resgrid.Services/ChatPermissionService.csCore/Resgrid.Services/ChatProvisioningEventService.csCore/Resgrid.Services/CommandAccessService.csCore/Resgrid.Services/DispatchAccessService.csCore/Resgrid.Services/IncidentCommandService.csCore/Resgrid.Services/PermissionGateServiceBase.csCore/Resgrid.Services/ServicesModule.csCore/Resgrid.Services/UnitsService.csCore/Resgrid.Services/UserStateService.csProviders/Resgrid.Providers.Bus/OutboundEventProvider.csRepositories/Resgrid.Repositories.DataRepository/ChatRepositories.csRepositories/Resgrid.Repositories.DataRepository/Queries/SystemAudits/SelectSystemAuditsByTypePagedQuery.csRepositories/Resgrid.Repositories.DataRepository/SystemAuditRepository.csWeb/Resgrid.Web.Services/Controllers/v4/ChatController.csWeb/Resgrid.Web.Services/Controllers/v4/IncidentCommandController.csWeb/Resgrid.Web.Services/Controllers/v4/SecurityController.csWeb/Resgrid.Web.Services/Filters/RequiresIncidentCapabilityAttribute.csWeb/Resgrid.Web.Services/Models/v4/Security/DepartmentRightsResult.csWeb/Resgrid.Web.Services/Resgrid.Web.Services.xmlWeb/Resgrid.Web/Areas/User/Controllers/SecurityController.csWeb/Resgrid.Web/Areas/User/Models/Security/PermissionsView.csWeb/Resgrid.Web/Areas/User/Views/Security/Index.cshtmlWeb/Resgrid.Web/wwwroot/js/app/internal/security/resgrid.security.permissions.jsWorkers/Resgrid.Workers.Framework/Logic/StaffingScheduleLogic.cs
| /// <param name="cancellationToken">The cancellation token that can be used by other objects or threads to receive notice of cancellation.</param> | ||
| /// <returns>Task<System.Boolean>.</returns> | ||
| Task<bool> SaveAllActionLogsAsync(List<ActionLog> actionLogs, CancellationToken cancellationToken = default(CancellationToken)); | ||
| Task<bool> SaveAllActionLogsAsync(List<ActionLog> actionLogs, CancellationToken cancellationToken = default(CancellationToken), bool autoGenerated = false); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Propagate autoGenerated through single action-log updates.
The new flag is exposed only by SaveAllActionLogsAsync. CallDispatchStatusService.ApplyPersonnelStatusesAsync uses SetUserActionAsync, which reaches ActionLogsService.SaveActionLogAsync. That event does not set UserStatusEvent.AutoGenerated, so the value remains false. OutboundEventProvider.personnelStatusChangedHandler then enqueues notifications for automated personnel status changes.
Propagate the flag through the relevant SetUserActionAsync and SaveActionLogAsync overloads. Pass true from the call-dispatch personnel path. Add a regression test for this flow.
🤖 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 `@Core/Resgrid.Model/Services/IActionLogsService.cs` at line 88, Propagate the
autoGenerated flag through the single-log path by adding it to the relevant
SetUserActionAsync and ActionLogsService.SaveActionLogAsync overloads, then
assign it to UserStatusEvent.AutoGenerated. Pass true from
CallDispatchStatusService.ApplyPersonnelStatusesAsync and add a regression test
verifying automated personnel status changes preserve the flag and avoid
outbound notifications.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
file="Core/Resgrid.Model/Services/IActionLogsService.cs"
printf '%s\n' '--- file outline ---'
ast-grep outline "$file" || true
printf '%s\n' '--- interface section ---'
sed -n '60,105p' "$file"
printf '%s\n' '--- declarations and usages ---'
rg -n --glob '*.cs' 'SaveAllActionLogsAsync|SaveActionLogAsync|SetUserActionAsync|autoGenerated|AutoGenerated' CoreRepository: Resgrid/Core
Length of output: 11132
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- ActionLogsService implementation ---'
sed -n '175,340p' Core/Resgrid.Services/ActionLogsService.cs
printf '%s\n' '--- call-dispatch implementation ---'
sed -n '65,135p' Core/Resgrid.Services/CallDispatchStatusService.cs
printf '%s\n' '--- action-log event and notification references ---'
rg -n --glob '*.cs' 'ActionLogEvent|UserStatusEvent|ActionLog.*Event|AutoGenerated' Core/Resgrid.Model Core/Resgrid.Services Core/Resgrid.Messaging Core/Resgrid.Notifications 2>/dev/null || trueRepository: Resgrid/Core
Length of output: 11073
🏁 Script executed:
#!/bin/bash
set -e
python3 - <<'PY'
from pathlib import Path
service = Path("Core/Resgrid.Services/ActionLogsService.cs").read_text()
dispatch = Path("Core/Resgrid.Services/CallDispatchStatusService.cs").read_text()
checks = {
"SaveAllActionLogsAsync writes AutoGenerated": "AutoGenerated = autoGenerated" in service,
"SaveActionLogAsync sends UserStatusEvent without AutoGenerated": (
"SaveActionLogAsync" in service
and "Status = saved,\n\t\t\t\tPreviousStatus = previousStatus\n\t\t\t});" in service
),
"SetUserActionAsync delegates to SaveActionLogAsync": "return await SaveActionLogAsync(al, cancellationToken);" in service,
"call dispatch personnel uses SetUserActionAsync": "_actionLogsService.SetUserActionAsync(userId, call.DepartmentId, statusToSet, null, call.CallId, cancellationToken)" in dispatch,
"call dispatch unit path passes autoGenerated": "_unitsService.SetUnitStateAsync(state, call.DepartmentId, cancellationToken, autoGenerated: true)" in dispatch,
}
for name, result in checks.items():
print(f"{name}: {'yes' if result else 'no'}")
PY
printf '%s\n' '--- all UserStatusEvent consumers ---'
rg -n --glob '*.cs' 'UserStatusEvent|AutoGenerated' .Repository: Resgrid/Core
Length of output: 3705
🏁 Script executed:
#!/bin/bash
set -e
sed -n '70,95p' Providers/Resgrid.Providers.Bus/OutboundEventProvider.cs
sed -n '225,245p' Providers/Resgrid.Providers.Bus/OutboundEventProvider.csRepository: Resgrid/Core
Length of output: 1902
Propagate autoGenerated to personnel status updates.
SaveActionLogAsync always publishes UserStatusEvent.AutoGenerated = false. Call-dispatch personnel updates use SetUserActionAsync, so they still enqueue user notifications. Propagate autoGenerated through the SetUserActionAsync overloads and document the new parameter on SaveAllActionLogsAsync.
🤖 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 `@Core/Resgrid.Model/Services/IActionLogsService.cs` at line 88, Propagate the
autoGenerated value from SaveActionLogAsync through both SetUserActionAsync
overloads into UserStatusEvent.AutoGenerated, preserving false only when the
argument is omitted. Update SaveAllActionLogsAsync to document the autoGenerated
parameter and ensure its calls pass that value through the action-log save flow.
| // Bulk status operations (scheduled resets, manual department-wide resets) never generate | ||
| // per-user notifications, otherwise every member change fans out to every subscriber. | ||
| return await SaveAllActionLogsAsync(logs, autoGenerated: true); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 5 \
'SetActionForEntireDepartmentAsync|SetActionForDepartmentGroupAsync' \
--glob '*.cs'Repository: Resgrid/Core
Length of output: 150
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="$(fd -t f 'ActionLogsService\.cs$' . | head -n 1)"
printf '%s\n' "FILE=$file"
test -n "$file"
ast-grep outline "$file" --match 'SetActionForEntireDepartmentAsync' --view expanded || true
ast-grep outline "$file" --match 'SetActionForDepartmentGroupAsync' --view expanded || true
rg -n -C 8 'SetActionFor|autoGenerated|UserStatusEvent|SaveAllActionLogsAsync' "$file"
printf '\nCALL SITES\n'
rg -n -C 4 'SetActionForEntireDepartmentAsync|SetActionForDepartmentGroupAsync' --glob '*.cs' . || trueRepository: Resgrid/Core
Length of output: 10662
🏁 Script executed:
#!/bin/bash
set -euo pipefail
service="./Core/Resgrid.Services/ActionLogsService.cs"
cat -n "$service" | sed -n '180,240p;315,385p'
printf '\nAUTO-GENERATED DEFINITIONS AND CONSUMERS\n'
rg -n -C 6 'AutoGenerated|UserStatusEvent|Status.*Notification|Notification.*Status' \
--glob '*.cs' Core Web Workers Tests | head -n 500
printf '\nCONTROLLER ACTIONS\n'
cat -n ./Web/Resgrid.Web/Areas/User/Controllers/HomeController.cs | sed -n '1035,1078p'
printf '\nSCHEDULED CALLER\n'
cat -n ./Workers/Resgrid.Workers.Framework/Logic/StatusScheduleLogic.cs | sed -n '20,55p'Repository: Resgrid/Core
Length of output: 25622
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' 'USER STATUS EVENT REFERENCES'
rg -n -C 10 'UserStatusEvent' --glob '*.cs' .
printf '\nAUTOGENERATED CONDITION REFERENCES\n'
rg -n -C 8 'AutoGenerated\s*(==|!=|&&|\|\||\)|\.)|\.AutoGenerated' --glob '*.cs' Core Web Workers Tests | head -n 800
printf '\nRELEVANT ACTIONS AND ROUTES\n'
rg -n -C 5 'ResetAllToStandingBy|ResetGroupToStandingBy|DepartmentStatusReset|SetActionForDepartmentGroupAsync' \
--glob '*.cs' Web Workers Core TestsRepository: Resgrid/Core
Length of output: 39788
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' 'USER STATUS EVENT REFERENCES'
rg -n -C 10 'UserStatusEvent' --glob '*.cs' .
printf '\nAUTOGENERATED CONDITION REFERENCES\n'
rg -n -C 8 'AutoGenerated\s*(==|!=|&&|\|\||\)|\.)|\.AutoGenerated' \
--glob '*.cs' Core Web Workers Tests | head -n 800
printf '\nRELEVANT ACTIONS AND ROUTES\n'
rg -n -C 5 'ResetAllToStandingBy|ResetGroupToStandingBy|DepartmentStatusReset|SetActionForDepartmentGroupAsync' \
--glob '*.cs' Web Workers Core TestsRepository: Resgrid/Core
Length of output: 39788
🏁 Script executed:
#!/bin/bash
set -euo pipefail
provider="./Providers/Resgrid.Providers.Bus/OutboundEventProvider.cs"
cat -n "$provider" | sed -n '218,270p'
printf '\nLISTENER REGISTRATION\n'
rg -n -C 6 'personnelStatusChangedHandler|PersonnelStatusChanged|AddListener<UserStatusEvent>|UserStatusEvent' \
"$provider" ./Providers/Resgrid.Providers.Bus.Rabbit ./Core/Resgrid.Model
printf '\nALL AUTO-GENERATED STATUS HANDLING\n'
rg -n -C 5 'message\.AutoGenerated|AutoGenerated.*message|AutoGenerated.*UserStatusEvent|UserStatusEvent.*AutoGenerated' \
--glob '*.cs' . || trueRepository: Resgrid/Core
Length of output: 30926
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
service = Path("Core/Resgrid.Services/ActionLogsService.cs").read_text()
controller = Path("Web/Resgrid.Web/Areas/User/Controllers/HomeController.cs").read_text()
worker = Path("Workers/Resgrid.Workers.Framework/Logic/StatusScheduleLogic.cs").read_text()
provider = Path("Providers/Resgrid.Providers.Bus/OutboundEventProvider.cs").read_text()
checks = {
"department bulk method suppresses": re.search(
r"SetActionForEntireDepartmentAsync.*?SaveAllActionLogsAsync\(logs,\s*autoGenerated:\s*true\)",
service, re.S),
"group bulk method suppresses": re.search(
r"SetActionForDepartmentGroupAsync.*?SaveAllActionLogsAsync\(logs,\s*autoGenerated:\s*true\)",
service, re.S),
"manual department endpoint calls bulk method": re.search(
r"ResetAllToStandingBy\s*\(\).*?SetActionForEntireDepartmentAsync",
controller, re.S),
"manual group endpoint calls bulk method": re.search(
r"ResetGroupToStandingBy\s*\(int groupId\).*?SetActionForDepartmentGroupAsync",
controller, re.S),
"scheduled worker calls department method": re.search(
r"DepartmentStatusReset.*?SetActionForEntireDepartmentAsync",
worker, re.S),
"outbound handler suppresses auto-generated notifications": re.search(
r"personnelStatusChangedHandler.*?if\s*\(!message\.AutoGenerated\)\s*"
r"\s*await\s+_outboundQueueProvider\.EnqueueNotification",
provider, re.S),
}
for name, matched in checks.items():
print(f"{name}: {'YES' if matched else 'NO'}")
if not all(checks.values()):
raise SystemExit(1)
PYRepository: Resgrid/Core
Length of output: 426
Preserve notifications for manual bulk status resets. Both manual reset endpoints pass autoGenerated: true, so OutboundEventProvider skips their user notifications. Pass this flag only for scheduled resets, or add a separate suppression parameter.
🤖 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 `@Core/Resgrid.Services/ActionLogsService.cs` around lines 348 - 350, Update
the bulk status reset flow around SaveAllActionLogsAsync so manual
department-wide reset operations do not pass autoGenerated: true and continue
producing per-user notifications. Restrict the autoGenerated flag to scheduled
resets, or introduce a separate notification-suppression parameter while
preserving suppression for scheduled resets.
| public CommandAccessService( | ||
| IPermissionsService permissionsService, | ||
| IDepartmentsService departmentsService, | ||
| IDepartmentGroupsService departmentGroupsService, | ||
| IPersonnelRolesService personnelRolesService, | ||
| ICacheProvider cacheProvider) | ||
| : base(permissionsService, departmentsService, departmentGroupsService, personnelRolesService, cacheProvider) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Resolve dependencies in the constructor through the configured service locator.
This constructor uses dependency injection for all service dependencies. Resolve these dependencies with Bootstrapper.GetKernel().Resolve<T>() and pass them to the base constructor.
As per coding guidelines, use Bootstrapper.GetKernel().Resolve<T>() to resolve dependencies explicitly in constructors, rather than constructor injection.
🤖 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 `@Core/Resgrid.Services/CommandAccessService.cs` around lines 12 - 18, Update
CommandAccessService’s constructor to remove its injected service parameters and
resolve IPermissionsService, IDepartmentsService, IDepartmentGroupsService,
IPersonnelRolesService, and ICacheProvider via
Bootstrapper.GetKernel().Resolve<T>(), passing the resolved instances to the
base constructor.
Source: Coding guidelines
| // permission side, which has no dependency on this service, does not close a DI cycle. | ||
| try | ||
| { | ||
| if (await ServiceLocator.Current.GetInstance<ICommandAccessService>().CanAssistWithCommandAsync(departmentId, userId)) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Use the required service resolver.
Replace ServiceLocator.Current.GetInstance<T>() with Bootstrapper.GetKernel().Resolve<T>() at each new dependency-resolution site.
Core/Resgrid.Services/IncidentCommandService.cs#L482-L482: ResolveICommandAccessServicethroughBootstrapper.GetKernel().Resolve<T>().Core/Resgrid.Services/IncidentCommandService.cs#L1075-L1075: ResolveIChatChannelServicethroughBootstrapper.GetKernel().Resolve<T>().Core/Resgrid.Services/IncidentCommandService.cs#L1116-L1117: ResolveIChatChannelRepositorythroughBootstrapper.GetKernel().Resolve<T>().
As per coding guidelines, use the Service Locator pattern via Bootstrapper.GetKernel().Resolve<T>().
📍 Affects 1 file
Core/Resgrid.Services/IncidentCommandService.cs#L482-L482(this comment)Core/Resgrid.Services/IncidentCommandService.cs#L1075-L1075Core/Resgrid.Services/IncidentCommandService.cs#L1116-L1117
🤖 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 `@Core/Resgrid.Services/IncidentCommandService.cs` at line 482, Replace the new
ServiceLocator.Current.GetInstance calls in
Core/Resgrid.Services/IncidentCommandService.cs at lines 482, 1075, and
1116-1117 with Bootstrapper.GetKernel().Resolve<T>() for ICommandAccessService,
IChatChannelService, and IChatChannelRepository respectively; preserve the
surrounding command logic.
Source: Coding guidelines
| private Task<bool> CanCommandAsync() => _commandAccessService.CanUseCommandAsync(DepartmentId, UserId); | ||
|
|
||
| /// <summary> | ||
| /// Whether the caller may READ command boards — the same commander gate as everything else on this | ||
| /// surface. A dispatcher who needs to work boards is given the command permission too (it grants | ||
| /// the assist capability set); dispatch authorization on its own is not a way in. | ||
| /// </summary> | ||
| private Task<bool> CanReadBoardsAsync() => CanCommandAsync(); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Apply the command gate to every command route without a capability filter.
CanCommandAsync() is only called by EstablishCommand and three board-read routes. A user denied CommandAppLogin can still call ReopenCommand, GetCommandForCall, and SendMessageToCommand when they hold the broad plan-level policy. Apply the department command gate consistently before these operations.
🤖 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/Controllers/v4/IncidentCommandController.cs` around
lines 49 - 56, Apply CanCommandAsync as an authorization guard in ReopenCommand,
GetCommandForCall, and SendMessageToCommand before their existing operation
logic, matching EstablishCommand and the board-read routes. Preserve the current
capability-filter checks and return the established unauthorized response when
the command gate denies access.
| $('#DispatchAppLogin').change(function () { | ||
| var val = this.value; | ||
| $.ajax({ | ||
| url: resgrid.absoluteBaseUrl + '/User/Security/SetPermission?type=29&perm=' + val, | ||
| type: 'GET' | ||
| }).done(function (results) { | ||
| }); | ||
| if ($("#DispatchAppLogin").val() === "2") { | ||
| $('#dispatchAppLoginNoRolesSpan').hide(); | ||
| $('#dispatchAppLoginRolesDiv').show(); | ||
| } else { | ||
| $('#dispatchAppLoginNoRolesSpan').show(); | ||
| $('#dispatchAppLoginRolesDiv').hide(); | ||
| } | ||
| }); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Do not change access permissions with GET requests.
SetPermission changes department authorization through a query-string GET request. A malicious site can cause an authenticated administrator to request type=29 or type=30 and weaken Dispatch or Command access. Change SetPermission and SetPermissionData to anti-forgery-protected POST actions. Send the verification token from these AJAX calls.
Also applies to: 814-828
🤖 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/wwwroot/js/app/internal/security/resgrid.security.permissions.js`
around lines 787 - 801, Change the permission updates in the DispatchAppLogin
and corresponding Command access handlers to use POST requests, and update the
SetPermission and SetPermissionData actions to require anti-forgery validation.
Include the page’s verification token in both AJAX request payloads while
preserving the existing permission type and value submission.
| Task<bool> CanUseCommandAsync(int departmentId, string userId); | ||
|
|
||
| /// <summary>Every user in the department who may act as a commander.</summary> | ||
| Task<List<string>> GetCommandUserIdsAsync(int departmentId); |
There was a problem hiding this comment.
Mutable collection exposure in ICommandAccessService.cs allows callers to alter the returned List<string>, violating encapsulation. Change the return type of GetCommandUserIdsAsync to Task<IReadOnlyList<string>> to enforce read-only results.
Kody rule violation: Use IReadOnlyList for immutable collections
Prompt for LLM
File Core/Resgrid.Model/Services/ICommandAccessService.cs:
Line 23:
Mutable collection exposure in ICommandAccessService.cs allows callers to alter the returned `List<string>`, violating encapsulation. Change the return type of GetCommandUserIdsAsync to `Task<IReadOnlyList<string>>` to enforce read-only results.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| catch (Exception ex) | ||
| { | ||
| // A cache outage must not stop the backfill — worst case it runs again on the next read. | ||
| Logging.LogException(ex); |
There was a problem hiding this comment.
Insufficient error logging in ChatChannelService.cs omits operation names and identifiers during cache-read failures. Include structured context, such as command.IncidentCommandId and markerKey, within the Logging.LogException call to satisfy Rule [3] and improve diagnosability.
Kody rule violation: Include error context in structured logs
Prompt for LLM
File Core/Resgrid.Services/ChatChannelService.cs:
Line 771:
Insufficient error logging in ChatChannelService.cs omits operation names and identifiers during cache-read failures. Include structured context, such as `command.IncidentCommandId` and `markerKey`, within the `Logging.LogException` call to satisfy Rule [3] and improve diagnosability.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| if (string.IsNullOrWhiteSpace(incidentCommandId)) | ||
| return false; | ||
|
|
||
| var affected = await _chatChannelRepository.SetArchivedByIncidentCommandIdAsync(incidentCommandId, archived, archived ? DateTime.UtcNow : (DateTime?)null); |
There was a problem hiding this comment.
Unsafe type casting violates the 'Use safe type casting with as operator' team rule. Implement the as operator or pattern matching to guard against null results before usage.
Prompt for LLM
File Core/Resgrid.Services/ChatChannelService.cs:
Line 862:
Unsafe type casting violates the 'Use safe type casting with as operator' team rule. Implement the `as` operator or pattern matching to guard against null results before usage.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| if (command.Status != (int)IncidentCommandStatus.Active) | ||
| return; | ||
|
|
||
| var markerKey = $"chat:incidentbackfill:{command.IncidentCommandId}"; |
There was a problem hiding this comment.
Hardcoded cache key prefix in ChatChannelService.cs introduces typo risks and violates Rule [6]. Extract 'chat:incidentbackfill:' into a private constant string to ensure consistency across cache operations.
Kody rule violation: Centralize string constants
Prompt for LLM
File Core/Resgrid.Services/ChatChannelService.cs:
Line 761:
Hardcoded cache key prefix in ChatChannelService.cs introduces typo risks and violates Rule [6]. Extract `'chat:incidentbackfill:'` into a private constant string to ensure consistency across cache operations.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| foreach (var dispatcherId in await _dispatchAccessService.GetDispatchUserIdsAsync(channel.DepartmentId)) | ||
| AddIfSet(userIds, dispatcherId); |
There was a problem hiding this comment.
Performance bottleneck in ResolveChannelAudienceUserIdsAsync triggers a full department member load with N+1 user lookups via GetDispatchUserIdsAsync on every message send. This hot path scales with department size and requires caching or batching to avoid database exhaustion.
Prompt for LLM
File Core/Resgrid.Services/ChatPermissionService.cs:
Line 199 to 200:
Performance bottleneck in ResolveChannelAudienceUserIdsAsync triggers a full department member load with N+1 user lookups via GetDispatchUserIdsAsync on every message send. This hot path scales with department size and requires caching or batching to avoid database exhaustion.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| /// Deliberately derived from the lanes on each check rather than stored as membership — a lead who | ||
| /// is replaced on the board loses the channel without anyone having to remember to remove them. | ||
| /// </summary> | ||
| private async Task<bool> IsLaneLeadOrCommanderAsync(int departmentId, int callId, string userId) |
There was a problem hiding this comment.
DRY violation in ChatPermissionService.cs duplicates the fetch-command and node-filtering pattern across IsLaneLeadOrCommanderAsync and AddLaneLeadsAsync. Extract a helper method, such as GetLaneLeadUserIdsAsync, to centralize the enumeration of commander and lead user IDs.
Kody rule violation: Extract duplicated logic into functions
Prompt for LLM
File Core/Resgrid.Services/ChatPermissionService.cs:
Line 541:
DRY violation in ChatPermissionService.cs duplicates the fetch-command and node-filtering pattern across IsLaneLeadOrCommanderAsync and AddLaneLeadsAsync. Extract a helper method, such as GetLaneLeadUserIdsAsync, to centralize the enumeration of commander and lead user IDs.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| return nodes.Any(n => !n.DeletedOn.HasValue && | ||
| (string.Equals(n.PrimaryLeadUserId, userId, StringComparison.OrdinalIgnoreCase) || | ||
| string.Equals(n.SecondaryLeadUserId, userId, StringComparison.OrdinalIgnoreCase))); |
There was a problem hiding this comment.
Repeated LINQ predicates in ChatPermissionService.cs scatter the definition of an active node across multiple calls. Factor the !n.DeletedOn.HasValue check into a reusable helper or expression to maintain a single source of truth.
Kody rule violation: Extract common query logic
Prompt for LLM
File Core/Resgrid.Services/ChatPermissionService.cs:
Line 556 to 558:
Repeated LINQ predicates in ChatPermissionService.cs scatter the definition of an active node across multiple calls. Factor the `!n.DeletedOn.HasValue` check into a reusable helper or expression to maintain a single source of truth.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| foreach (var member in active) | ||
| { | ||
| if (await IsAllowedAsync(departmentId, member.UserId)) | ||
| allowed.Add(member.UserId); | ||
| } |
There was a problem hiding this comment.
N+1 query bottleneck in PermissionGateServiceBase.cs issues 5 separate database queries per active member during IsAllowedAsync evaluation. Implement a bulk evaluation method or Task.WhenAll batching to fetch all permissions, memberships, groups, and roles simultaneously.
Kody rule violation: Detect N+1 style queries and suggest batching
Prompt for LLM
File Core/Resgrid.Services/PermissionGateServiceBase.cs:
Line 129 to 133:
N+1 query bottleneck in PermissionGateServiceBase.cs issues 5 separate database queries per active member during IsAllowedAsync evaluation. Implement a bulk evaluation method or `Task.WhenAll` batching to fetch all permissions, memberships, groups, and roles simultaneously.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| foreach (var member in active) | ||
| { | ||
| if (await IsAllowedAsync(departmentId, member.UserId)) | ||
| allowed.Add(member.UserId); | ||
| } |
There was a problem hiding this comment.
Excessive database round-trips in PermissionGateServiceBase.cs stem from per-member loops triggering individual lookups for permissions, memberships, groups, and roles. Eager-load the department data in bulk queries and evaluate eligibility in-memory to eliminate the N+1 query issue.
Kody rule violation: Optimize database queries with JOINs
Prompt for LLM
File Core/Resgrid.Services/PermissionGateServiceBase.cs:
Line 129 to 133:
Excessive database round-trips in PermissionGateServiceBase.cs stem from per-member loops triggering individual lookups for permissions, memberships, groups, and roles. Eager-load the department data in bulk queries and evaluate eligibility in-memory to eliminate the N+1 query issue.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
|
||
| return await x.QueryAsync<SystemAudit>(sql: query, | ||
| param: dynamicParameters, | ||
| transaction: _unitOfWork.Transaction); |
There was a problem hiding this comment.
NullReferenceException risk in SystemAuditRepository.cs arises when line 149 evaluates _unitOfWork as null, causing the selectFunction lambda to dereference _unitOfWork.Transaction unsafely. Apply a null-conditional operator to the transaction access or ensure the _unitOfWork instance is guaranteed non-null.
Kody rule violation: Add null checks to prevent NullReferenceException
Prompt for LLM
File Repositories/Resgrid.Repositories.DataRepository/SystemAuditRepository.cs:
Line 145:
NullReferenceException risk in SystemAuditRepository.cs arises when line 149 evaluates `_unitOfWork` as null, causing the `selectFunction` lambda to dereference `_unitOfWork.Transaction` unsafely. Apply a null-conditional operator to the transaction access or ensure the `_unitOfWork` instance is guaranteed non-null.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| => _channelRepository.Setup(x => x.GetByCallIdAsync(CallId)).ReturnsAsync(new List<ChatChannel>(channels)); | ||
|
|
||
| [Test] | ||
| public async Task an_incident_with_no_channels_gets_the_full_set() |
There was a problem hiding this comment.
Naming convention violation in ChatIncidentBackfillTests.cs uses snake_case for test methods. Rename methods like an_incident_with_no_channels_gets_the_full_set to PascalCase to comply with Rule [7].
Kody rule violation: Use proper naming conventions
Prompt for LLM
File Tests/Resgrid.Tests/Services/ChatIncidentBackfillTests.cs:
Line 92:
Naming convention violation in ChatIncidentBackfillTests.cs uses snake_case for test methods. Rename methods like `an_incident_with_no_channels_gets_the_full_set` to PascalCase to comply with Rule [7].
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| var nextCalled = await Invoke(filter, context); | ||
|
|
||
| nextCalled.Should().BeFalse(); | ||
| context.Result.Should().BeOfType<ObjectResult>() |
There was a problem hiding this comment.
Async method blocking violates the 'Avoid Blocking Calls to Async Methods' team rule. Replace .Result or .Wait() with await to prevent deadlocks and ensure proper asynchronous execution.
Prompt for LLM
File Tests/Resgrid.Tests/Web/Services/RequiresIncidentCapabilityFilterTests.cs:
Line 294:
Async method blocking violates the 'Avoid Blocking Calls to Async Methods' team rule. Replace `.Result` or `.Wait()` with `await` to prevent deadlocks and ensure proper asynchronous execution.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| var nextCalled = await Invoke(filter, context); | ||
|
|
||
| nextCalled.Should().BeFalse(); | ||
| context.Result.Should().BeOfType<ObjectResult>() |
There was a problem hiding this comment.
Async operation blocking violates the 'Await async operations properly' team rule. Await Tasks end-to-end instead of using .Result or .Wait() to prevent deadlocks and ensure efficient asynchronous execution.
Prompt for LLM
File Tests/Resgrid.Tests/Web/Services/RequiresIncidentCapabilityFilterTests.cs:
Line 294:
Async operation blocking violates the 'Await async operations properly' team rule. Await Tasks end-to-end instead of using `.Result` or `.Wait()` to prevent deadlocks and ensure efficient asynchronous execution.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| if (input == null || input.CallId <= 0) | ||
| return BadRequest(); | ||
|
|
||
| if (!await CanCommandAsync()) |
There was a problem hiding this comment.
Unhandled exception risk in IncidentCommandController.cs occurs when the external CanCommandAsync() database call rejects without a try/catch block. Wrap the authorization check to log contextual identifiers like DepartmentId and return a structured error response instead of an opaque 500.
Kody rule violation: Handle async operations with proper error handling
Prompt for LLM
File Web/Resgrid.Web.Services/Controllers/v4/IncidentCommandController.cs:
Line 74:
Unhandled exception risk in IncidentCommandController.cs occurs when the external CanCommandAsync() database call rejects without a try/catch block. Wrap the authorization check to log contextual identifiers like `DepartmentId` and return a structured error response instead of an opaque 500.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| if (!await CanCommandAsync()) | ||
| return Unauthorized(); |
There was a problem hiding this comment.
Conflicting HTTP status codes occur when EstablishCommand returns 401 Unauthorized for an authenticated user lacking commander permissions, whereas RequiresIncidentCapabilityAttribute correctly returns 403 Forbidden. Return Forbid() to align with the capability filter and accurately indicate an authorization failure.
if (!await CanCommandAsync())
return Forbid();Prompt for LLM
File Web/Resgrid.Web.Services/Controllers/v4/IncidentCommandController.cs:
Line 74 to 75:
Conflicting HTTP status codes occur when EstablishCommand returns 401 Unauthorized for an authenticated user lacking commander permissions, whereas RequiresIncidentCapabilityAttribute correctly returns 403 Forbidden. Return Forbid() to align with the capability filter and accurately indicate an authorization failure.
Suggested Code:
if (!await CanCommandAsync())
return Forbid();
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| // because a member the department hasn't authorized as a commander shouldn't reach the board | ||
| // surface whatever ICS role happens to be recorded against them. | ||
| var commandAccess = context.HttpContext.RequestServices?.GetService(typeof(ICommandAccessService)) as ICommandAccessService; | ||
| if (commandAccess != null && !await commandAccess.CanUseCommandAsync(departmentId, userId)) |
There was a problem hiding this comment.
Unhandled external call in RequiresIncidentCapabilityAttribute.cs bypasses Rule [27] by omitting a try/catch around CanUseCommandAsync. Wrap the database call to map failures to application-level errors and log structured context such as departmentId and userId.
Kody rule violation: Add try-catch blocks for external calls
Prompt for LLM
File Web/Resgrid.Web.Services/Filters/RequiresIncidentCapabilityAttribute.cs:
Line 63:
Unhandled external call in RequiresIncidentCapabilityAttribute.cs bypasses Rule [27] by omitting a try/catch around CanUseCommandAsync. Wrap the database call to map failures to application-level errors and log structured context such as `departmentId` and `userId`.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| if (permissions.Any(x => x.PermissionType == (int)PermissionTypes.DispatchAppLogin)) | ||
| model.DispatchAppLogin = permissions.First(x => x.PermissionType == (int)PermissionTypes.DispatchAppLogin).Action; | ||
| else | ||
| model.DispatchAppLogin = 3; |
There was a problem hiding this comment.
Magic number duplication in SecurityController.cs uses unexplained literal 3 for default 'Everyone' permissions. Extract a named constant or reuse the mapped enum value to prevent error-prone changes.
Kody rule violation: Replace magic numbers with named constants
Prompt for LLM
File Web/Resgrid.Web/Areas/User/Controllers/SecurityController.cs:
Line 401:
Magic number duplication in SecurityController.cs uses unexplained literal `3` for default 'Everyone' permissions. Extract a named constant or reuse the mapped enum value to prevent error-prone changes.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| model.DispatchAppLogin = 3; | ||
|
|
||
| var dispatchAppLoginPermissions = new List<dynamic>(); | ||
| dispatchAppLoginPermissions.Add(new { Id = 3, Name = "Everyone" }); |
There was a problem hiding this comment.
Magic string repetition in SecurityController.cs hardcodes the 'Everyone' permission level across multiple code blocks. Define a constant or enum for permission level names to eliminate duplication and reduce error risks.
Kody rule violation: Use enums instead of magic strings
Prompt for LLM
File Web/Resgrid.Web/Areas/User/Controllers/SecurityController.cs:
Line 404:
Magic string repetition in SecurityController.cs hardcodes the `'Everyone'` permission level across multiple code blocks. Define a constant or enum for permission level names to eliminate duplication and reduce error risks.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| public SelectList UseCalendarSyncPermissions { get; set; } | ||
|
|
||
| public int DispatchAppLogin { get; set; } | ||
| public SelectList DispatchAppLoginPermissions { get; set; } |
There was a problem hiding this comment.
Uninitialized SelectList property in PermissionsView.cs risks null reference exceptions if used before population. Initialize DispatchAppLoginPermissions with an empty SelectList or assign it within the constructor to comply with Rule [31].
Kody rule violation: Initialize properties with default values
Prompt for LLM
File Web/Resgrid.Web/Areas/User/Models/Security/PermissionsView.cs:
Line 98:
Uninitialized `SelectList` property in PermissionsView.cs risks null reference exceptions if used before population. Initialize DispatchAppLoginPermissions with an empty `SelectList` or assign it within the constructor to comply with Rule [31].
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| </td> | ||
| </tr> | ||
| <tr> | ||
| <td>Dispatch App Login</td> |
There was a problem hiding this comment.
Internationalization bypass in Index.cshtml hardcodes the "Dispatch App Login" label instead of utilizing the localization pipeline. Apply a resource key using @localizer to comply with Rule [112] and match existing localized strings like PermissionNA.
Kody rule violation: Internationalize user-facing text with next-intl or next-i18next
Prompt for LLM
File Web/Resgrid.Web/Areas/User/Views/Security/Index.cshtml:
Line 327:
Internationalization bypass in Index.cshtml hardcodes the "Dispatch App Login" label instead of utilizing the localization pipeline. Apply a resource key using `@localizer` to comply with Rule [112] and match existing localized strings like `PermissionNA`.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| $('#DispatchAppLogin').change(function () { | ||
| var val = this.value; | ||
| $.ajax({ | ||
| url: resgrid.absoluteBaseUrl + '/User/Security/SetPermission?type=29&perm=' + val, |
There was a problem hiding this comment.
String concatenation violates the 'Use Template Literals Instead of String Concatenation' team rule. Implement template literals to construct the url string, improving readability and reducing error-prone syntax.
Prompt for LLM
File Web/Resgrid.Web/wwwroot/js/app/internal/security/resgrid.security.permissions.js:
Line 790:
String concatenation violates the 'Use Template Literals Instead of String Concatenation' team rule. Implement template literals to construct the `url` string, improving readability and reducing error-prone syntax.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| // Dispatch App Login | ||
| //////////////////////////////////////////////////////// | ||
| $('#DispatchAppLogin').change(function () { | ||
| var val = this.value; |
There was a problem hiding this comment.
Unsafe variable scoping in resgrid.security.permissions.js uses var instead of block-scoped declarations. Replace var with const for unassigned variables to adhere to Rule [37] and prevent subtle scoping bugs.
Kody rule violation: Always use const and let
Prompt for LLM
File Web/Resgrid.Web/wwwroot/js/app/internal/security/resgrid.security.permissions.js:
Line 788:
Unsafe variable scoping in resgrid.security.permissions.js uses `var` instead of block-scoped declarations. Replace `var` with `const` for unassigned variables to adhere to Rule [37] and prevent subtle scoping bugs.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| if (item.ScheduledTask.TaskType == (int)TaskTypes.UserStaffingLevel) | ||
| { | ||
| await _userStateService.CreateUserState(item.ScheduledTask.UserId, item.ScheduledTask.DepartmentId, int.Parse(item.ScheduledTask.Data), item.ScheduledTask.Note); | ||
| await _userStateService.CreateUserState(item.ScheduledTask.UserId, item.ScheduledTask.DepartmentId, int.Parse(item.ScheduledTask.Data), item.ScheduledTask.Note, autoGenerated: true); |
There was a problem hiding this comment.
Unsafe string conversion violates the 'Use TryParse for string conversions' team rule. Utilize int.TryParse for item.ScheduledTask.Data to handle invalid user or IO input safely and validate the format.
Prompt for LLM
File Workers/Resgrid.Workers.Framework/Logic/StaffingScheduleLogic.cs:
Line 36:
Unsafe string conversion violates the 'Use TryParse for string conversions' team rule. Utilize `int.TryParse` for `item.ScheduledTask.Data` to handle invalid user or IO input safely and validate the format.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
Core/Resgrid.Services/ChatChannelService.cs (2)
709-748: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winInclude the new channel types in implicit-audience channel discovery.
GetChannelsForUserAsynconly evaluatesIncident,IncidentLane, andIncidentCommandchannel types. It skipsIncidentLeadsandIncidentDispatch. These provisioning methods do not add explicit members, so eligible leads and dispatch users will not receive the new channels in their normal channel list.Proposed fix
- if (type != ChatChannelType.CustomLocked && type != ChatChannelType.Incident && - type != ChatChannelType.IncidentLane && type != ChatChannelType.IncidentCommand) + if (type != ChatChannelType.CustomLocked && type != ChatChannelType.Incident && + type != ChatChannelType.IncidentLane && type != ChatChannelType.IncidentCommand && + type != ChatChannelType.IncidentLeads && type != ChatChannelType.IncidentDispatch) continue;🤖 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 `@Core/Resgrid.Services/ChatChannelService.cs` around lines 709 - 748, Update GetChannelsForUserAsync to include ChatChannelType.IncidentLeads and ChatChannelType.IncidentDispatch in its implicit-audience channel discovery alongside the existing Incident, IncidentLane, and IncidentCommand types, so eligible users receive channels provisioned by EnsureLeadsChannelAsync and EnsureDispatchChannelAsync.
781-846: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winUse cache-aside retrieval and support cache bypass for incident backfill.
GetStringAsyncandSetStringAsyncbypass the requiredRetrieveAsync<T>()cache-aside flow. The method also has nobypassCacheparameter and still accesses the cache when caching is disabled. Move provisioning into a local fallback function, call it throughRetrieveAsync, and execute the fallback directly whenbypassCacheis true or caching is disabled.As per coding guidelines, “All caching operations must go through
ICacheProvider.Retrieve<T>()orICacheProvider.RetrieveAsync<T>()using the cache-aside pattern with fallback functions,” and service methods should includebypassCachewith defaultfalse.🤖 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 `@Core/Resgrid.Services/ChatChannelService.cs` around lines 781 - 846, Update EnsureIncidentChannelsAsync to accept a bypassCache parameter defaulting to false, move the channel provisioning work into a local fallback function, and invoke it through ICacheProvider.RetrieveAsync<T>() when caching is enabled and bypassCache is false. Execute the fallback directly when bypassCache is true or caching is disabled, and remove the direct GetStringAsync/SetStringAsync marker operations while preserving the existing provisioning behavior.Source: Coding guidelines
Repositories/Resgrid.Repositories.DataRepository/SystemAuditRepository.cs (1)
128-174:⚠️ Potential issue | 🟠 MajorRemove the remaining null dereference.
GetByTypePagedAsyncenters the standalone-connection path when_unitOfWorkis null, butQueryAsyncstill reads_unitOfWork.Transaction. This throwsNullReferenceExceptionbefore the query runs. Use_unitOfWork?.Transaction, or reject nullunitOfWorkin the constructor and remove the nullable path. Apply the same contract to all paged methods.🤖 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 `@Repositories/Resgrid.Repositories.DataRepository/SystemAuditRepository.cs` around lines 128 - 174, Update GetByTypePagedAsync and every other paged repository method to pass _unitOfWork?.Transaction to QueryAsync when supporting standalone connections, preventing dereferences when _unitOfWork is null. Alternatively, enforce a non-null unit-of-work contract in the constructor and remove the nullable connection path consistently across all paged methods.
🧹 Nitpick comments (1)
Web/Resgrid.Web.Services/Controllers/v4/IncidentCommandController.cs (1)
35-41: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winResolve
ICommandAccessServicethrough the required service locator.Remove
ICommandAccessServicefrom the constructor parameters. Resolve it withBootstrapper.GetKernel().Resolve<ICommandAccessService>()in the constructor.As per coding guidelines, use “
Service Locatorpattern viaBootstrapper.GetKernel().Resolve<T>()to resolve dependencies explicitly in constructors, rather than constructor injection.”🤖 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/Controllers/v4/IncidentCommandController.cs` around lines 35 - 41, Update IncidentCommandController by removing ICommandAccessService from the constructor parameters and resolving it inside the constructor via Bootstrapper.GetKernel().Resolve<ICommandAccessService>(), assigning the result to _commandAccessService while leaving the other injected services unchanged.Source: Coding guidelines
🤖 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.
Outside diff comments:
In `@Core/Resgrid.Services/ChatChannelService.cs`:
- Around line 709-748: Update GetChannelsForUserAsync to include
ChatChannelType.IncidentLeads and ChatChannelType.IncidentDispatch in its
implicit-audience channel discovery alongside the existing Incident,
IncidentLane, and IncidentCommand types, so eligible users receive channels
provisioned by EnsureLeadsChannelAsync and EnsureDispatchChannelAsync.
- Around line 781-846: Update EnsureIncidentChannelsAsync to accept a
bypassCache parameter defaulting to false, move the channel provisioning work
into a local fallback function, and invoke it through
ICacheProvider.RetrieveAsync<T>() when caching is enabled and bypassCache is
false. Execute the fallback directly when bypassCache is true or caching is
disabled, and remove the direct GetStringAsync/SetStringAsync marker operations
while preserving the existing provisioning behavior.
In `@Repositories/Resgrid.Repositories.DataRepository/SystemAuditRepository.cs`:
- Around line 128-174: Update GetByTypePagedAsync and every other paged
repository method to pass _unitOfWork?.Transaction to QueryAsync when supporting
standalone connections, preventing dereferences when _unitOfWork is null.
Alternatively, enforce a non-null unit-of-work contract in the constructor and
remove the nullable connection path consistently across all paged methods.
---
Nitpick comments:
In `@Web/Resgrid.Web.Services/Controllers/v4/IncidentCommandController.cs`:
- Around line 35-41: Update IncidentCommandController by removing
ICommandAccessService from the constructor parameters and resolving it inside
the constructor via Bootstrapper.GetKernel().Resolve<ICommandAccessService>(),
assigning the result to _commandAccessService while leaving the other injected
services unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 4d58d7bd-d969-421a-9ecb-e3d39f9b1716
⛔ Files ignored due to path filters (12)
Core/Resgrid.Localization/Areas/User/Security/Security.ar.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Security/Security.de.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Security/Security.en.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Security/Security.es.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Security/Security.fr.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Security/Security.it.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Security/Security.pl.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Security/Security.sv.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Security/Security.uk.resxis excluded by!**/*.resxTests/Resgrid.Tests/Services/ChatIncidentBackfillTests.csis excluded by!**/Tests/**Tests/Resgrid.Tests/Services/DispatchAccessServiceTests.csis excluded by!**/Tests/**Tests/Resgrid.Tests/Web/Services/RequiresIncidentCapabilityFilterTests.csis excluded by!**/Tests/**
📒 Files selected for processing (12)
Core/Resgrid.Model/Repositories/IChatRepositories.csCore/Resgrid.Model/Services/IActionLogsService.csCore/Resgrid.Services/ChatChannelService.csCore/Resgrid.Services/PermissionGateServiceBase.csRepositories/Resgrid.Repositories.DataRepository/ChatRepositories.csRepositories/Resgrid.Repositories.DataRepository/Queries/SystemAudits/SelectSystemAuditsByDepartmentIdPagedQuery.csRepositories/Resgrid.Repositories.DataRepository/Queries/SystemAudits/SelectSystemAuditsByTypePagedQuery.csRepositories/Resgrid.Repositories.DataRepository/Queries/SystemAudits/SelectSystemAuditsByUserIdPagedQuery.csRepositories/Resgrid.Repositories.DataRepository/SystemAuditRepository.csWeb/Resgrid.Web.Services/Controllers/v4/IncidentCommandController.csWeb/Resgrid.Web.Services/Filters/RequiresIncidentCapabilityAttribute.csWeb/Resgrid.Web/Areas/User/Views/Security/Index.cshtml
🚧 Files skipped from review as they are similar to previous changes (5)
- Web/Resgrid.Web.Services/Filters/RequiresIncidentCapabilityAttribute.cs
- Core/Resgrid.Services/PermissionGateServiceBase.cs
- Repositories/Resgrid.Repositories.DataRepository/Queries/SystemAudits/SelectSystemAuditsByTypePagedQuery.cs
- Core/Resgrid.Model/Services/IActionLogsService.cs
- Web/Resgrid.Web/Areas/User/Views/Security/Index.cshtml
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 `@Core/Resgrid.Config/CorsHelper.cs`:
- Around line 127-134: Replace the partial _publicRegistrySuffixes check in the
parent-domain widening loop with a maintained Public Suffix List implementation
that includes private suffixes, or remove automatic parent widening and require
sibling origins through ApiConfig.CorsAllowedOrigins. Ensure shared-hosting
domains such as github.io cannot widen to an attacker-controlled sibling, and
add regression coverage for these suffixes.
🪄 Autofix
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: 1b280f7c-5e8c-40e1-8b22-d3ac8e845f67
⛔ Files ignored due to path filters (1)
Tests/Resgrid.Tests/Config/CorsHelperTests.csis excluded by!**/Tests/**
📒 Files selected for processing (4)
Core/Resgrid.Config/ApiConfig.csCore/Resgrid.Config/CorsHelper.csWeb/Resgrid.Web.Eventing/Startup.csWeb/Resgrid.Web.Services/Startup.cs
| } | ||
| catch (Exception ex) | ||
| { | ||
| Logging.LogException(ex); |
There was a problem hiding this comment.
Insufficient error logging in the catch block. The call to Logging.LogException(ex) lacks the operation name and relevant identifiers (chatChannelId, incidentCommandId) as structured fields, making failures untraceable. Pass structured context, e.g. Logging.LogException(ex, new { op = "RebindToIncidentCommand", chatChannelId, incidentCommandId }).
Kody rule violation: Include error context in structured logs
Prompt for LLM
File Repositories/Resgrid.Repositories.DataRepository/ChatRepositories.cs:
Line 563:
Insufficient error logging in the catch block. The call to `Logging.LogException(ex)` lacks the operation name and relevant identifiers (`chatChannelId`, `incidentCommandId`) as structured fields, making failures untraceable. Pass structured context, e.g. `Logging.LogException(ex, new { op = "RebindToIncidentCommand", chatChannelId, incidentCommandId })`.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| // this gate is asked about a channel's department, not necessarily the caller's own. | ||
| GivenPermission(null); | ||
| _departmentsService.Setup(x => x.GetDepartmentMemberAsync("stranger", DepartmentId, It.IsAny<bool>())) | ||
| .ReturnsAsync((DepartmentMember)null); |
There was a problem hiding this comment.
Unsafe type casting detected. Use the as operator or pattern matching for safe casts and guard null results before usage.
Kody rule violation: Use safe type casting with as operator
Prompt for LLM
File Tests/Resgrid.Tests/Services/DispatchAccessServiceTests.cs:
Line 120:
Unsafe type casting detected. Use the `as` operator or pattern matching for safe casts and guard null results before usage.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| var nextCalled = await Invoke(filter, context); | ||
|
|
||
| nextCalled.Should().BeFalse(); | ||
| context.Result.Should().BeOfType<StatusCodeResult>() |
There was a problem hiding this comment.
Blocking call to async method identified. Blocking async methods with .Result or .Wait() can cause deadlocks and prevent efficient asynchronous execution; use await instead.
Kody rule violation: Avoid Blocking Calls to Async Methods
Prompt for LLM
File Tests/Resgrid.Tests/Web/Services/RequiresIncidentCapabilityFilterTests.cs:
Line 293:
Blocking call to async method identified. Blocking async methods with `.Result` or `.Wait()` can cause deadlocks and prevent efficient asynchronous execution; use `await` instead.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| var nextCalled = await Invoke(filter, context); | ||
|
|
||
| nextCalled.Should().BeFalse(); | ||
| context.Result.Should().BeOfType<StatusCodeResult>() |
There was a problem hiding this comment.
Blocking async operation found. Await Tasks instead of blocking with .Result or .Wait(), and prefer async/await end-to-end with appropriate await configuration.
Kody rule violation: Await async operations properly
Prompt for LLM
File Tests/Resgrid.Tests/Web/Services/RequiresIncidentCapabilityFilterTests.cs:
Line 293:
Blocking async operation found. Await Tasks instead of blocking with `.Result` or `.Wait()`, and prefer async/await end-to-end with appropriate await configuration.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| return; | ||
| } | ||
|
|
||
| if (!await commandAccess.CanUseCommandAsync(departmentId, userId)) |
There was a problem hiding this comment.
Unhandled exception risk in the awaited call commandAccess.CanUseCommandAsync(departmentId, userId). If the service throws a database timeout or connection failure, the exception propagates unhandled and surfaces a raw 500 without context. Wrap the await in try/catch, log with structured context (departmentId, userId), and set an appropriate error result.
Kody rule violation: Handle async operations with proper error handling
Prompt for LLM
File Web/Resgrid.Web.Services/Filters/RequiresIncidentCapabilityAttribute.cs:
Line 72:
Unhandled exception risk in the awaited call `commandAccess.CanUseCommandAsync(departmentId, userId)`. If the service throws a database timeout or connection failure, the exception propagates unhandled and surfaces a raw 500 without context. Wrap the await in try/catch, log with structured context (`departmentId`, `userId`), and set an appropriate error result.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| return; | ||
| } | ||
|
|
||
| if (!await commandAccess.CanUseCommandAsync(departmentId, userId)) |
There was a problem hiding this comment.
Missing error boundary around the external service call CanUseCommandAsync. Transient database failures surface as unstructured exceptions rather than mapped application-level errors. Wrap the call in try/catch, add operation context (departmentId, userId) to the log, and return a mapped error result on failure.
Kody rule violation: Add try-catch blocks for external calls
Prompt for LLM
File Web/Resgrid.Web.Services/Filters/RequiresIncidentCapabilityAttribute.cs:
Line 72:
Missing error boundary around the external service call `CanUseCommandAsync`. Transient database failures surface as unstructured exceptions rather than mapped application-level errors. Wrap the call in try/catch, add operation context (`departmentId`, `userId`) to the log, and return a mapped error result on failure.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
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:
|
| } | ||
|
|
||
| [Test] | ||
| public void should_not_widen_the_parent_domain_past_a_shared_hosting_suffix() |
There was a problem hiding this comment.
Naming convention violation in Tests/Resgrid.Tests/Config/CorsHelperTests.cs where the test method uses snake_case instead of standard .NET PascalCase. Rename the method to ShouldNotWidenTheParentDomainPastASharedHostingSuffix.
Kody rule violation: Use proper naming conventions
Prompt for LLM
File Tests/Resgrid.Tests/Config/CorsHelperTests.cs:
Line 87:
Naming convention violation in `Tests/Resgrid.Tests/Config/CorsHelperTests.cs` where the test method uses snake_case instead of standard .NET PascalCase. Rename the method to `ShouldNotWidenTheParentDomainPastASharedHostingSuffix`.
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
Incident Chat Channels, App-Level Access Gates, and Automated Notification Suppression
This PR introduces several major features across the Resgrid Core platform:
New Incident Chat Channels
Two new chat channel types have been added to improve incident communication:
A lazy backfill mechanism provisions any missing channels (incident, command, leads, dispatch, and lane channels) the first time someone opens an incident board or responder view, eliminating the need for a data migration. Channel provisioning is idempotent and cache-guarded.
App-Level Access Control
Two new permissions gate access to the specialized apps:
Both default to "Everyone" so existing departments are unaffected on upgrade. New
PermissionGateServiceBaseand corresponding service implementations provide cached, fail-closed evaluation. When a department narrows the command permission, those authorized users also gain aCommandAssistCapabilitiesset allowing them to assist on boards (assign/move resources, manage timers/accountability) without holding an ICS role.Frozen/Archived Channel Enforcement
Archived channels (from closed incidents or commands) are now truly immutable. Message edits, author deletions, and reactions are blocked on frozen channels, while moderator deletes still function for content moderation. This applies at both the command-close level (leaving the call's own channel live) and the call-close level.
Automated Change Notification Suppression
Status and staffing events now carry an
AutoGeneratedflag. Automated changes — such as scheduled department resets, group resets, and call dispatch auto-status — no longer trigger user notifications while still firing SignalR updates to keep clients in sync.Additional Changes
includeArchivedparameter for viewing closed incident chat history.Summary by CodeRabbit
New Features
Improvements