Conversation
|
Thanks for opening this, but we'd appreciate a little more information. Could you update it with more details? |
Contributor
📝 WalkthroughWalkthroughThe pull request adds incident notes, attachments, public sharing, weather retrieval, reporting, voice transmission logging, workflow integration, stronger SSO validation and SAML relay handling, plus command-board and unit-role template catalogs with editable assignment controls. ChangesIncident Command Operations
SSO Validation and Relay Security
Command Board Templates and Assignment Editing
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Commander
participant IncidentCommandController
participant IncidentCommandService
participant WeatherProvider
participant NWS
Commander->>IncidentCommandController: request incident weather
IncidentCommandController->>IncidentCommandService: GetWeatherForIncidentAsync
IncidentCommandService->>WeatherProvider: GetWeatherAsync
WeatherProvider->>NWS: fetch point, forecast, and observation data
NWS-->>WeatherProvider: return weather JSON
WeatherProvider-->>IncidentCommandService: return IncidentWeather
IncidentCommandService-->>IncidentCommandController: return weather result
IncidentCommandController-->>Commander: return forecast and observations
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| return BadRequest(new { error = "invalid_request", error_description = "provider must be 'saml2' or 'oidc'." }); | ||
| } | ||
|
|
||
| if (providerType == SsoProviderType.Saml2 && external_token.StartsWith(SamlRelayTokenPrefix, StringComparison.Ordinal)) |
| [ProducesResponseType(StatusCodes.Status200OK)] | ||
| [Authorize(Policy = ResgridResources.Command_Update)] | ||
| [RequiresIncidentCapability(IncidentCapabilities.ManageNotes)] | ||
| public async Task<ActionResult<ICModels.IncidentNoteResult>> AddNote([FromBody] ICModels.AddIncidentNoteInput input) |
| [ProducesResponseType(StatusCodes.Status200OK)] | ||
| [Authorize(Policy = ResgridResources.Command_Update)] | ||
| [RequiresIncidentCapability(IncidentCapabilities.ManageDocuments)] | ||
| public async Task<ActionResult<ICModels.IncidentAttachmentResult>> AddAttachment([FromForm] ICModels.AddIncidentAttachmentInput input, CancellationToken cancellationToken) |
| [HttpPost("EnablePublicSharing/{incidentCommandId}")] | ||
| [Authorize(Policy = ResgridResources.Command_Update)] | ||
| [RequiresIncidentCapability(IncidentCapabilities.ManagePublicInformation)] | ||
| public async Task<ActionResult<ICModels.IncidentCommandResult>> EnablePublicSharing(string incidentCommandId) |
| [HttpPost("DisablePublicSharing/{incidentCommandId}")] | ||
| [Authorize(Policy = ResgridResources.Command_Update)] | ||
| [RequiresIncidentCapability(IncidentCapabilities.ManagePublicInformation)] | ||
| public async Task<ActionResult<ICModels.IncidentCommandResult>> DisablePublicSharing(string incidentCommandId) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (5)
Providers/Resgrid.Providers.MigrationsPg/Migrations/M0091_AddUniqueDepartmentSsoProviderPg.cs (1)
12-22: 🗄️ Data Integrity & Integration | 🔵 TrivialSee consolidated migration note below regarding pre-existing duplicate rows.
🤖 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 `@Providers/Resgrid.Providers.MigrationsPg/Migrations/M0091_AddUniqueDepartmentSsoProviderPg.cs` around lines 12 - 22, Update the migration’s Up method to handle pre-existing duplicate rows before creating the unique index IndexName on TableName; consolidate or remove duplicates according to the migration requirements, then retain the conditional index creation so the migration remains safe when the table or index already exists.Providers/Resgrid.Providers.Migrations/Migrations/M0091_AddUniqueDepartmentSsoProvider.cs (1)
12-22: 🗄️ Data Integrity & Integration | 🔵 TrivialSee consolidated migration note below regarding pre-existing duplicate rows.
🤖 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 `@Providers/Resgrid.Providers.Migrations/Migrations/M0091_AddUniqueDepartmentSsoProvider.cs` around lines 12 - 22, Update the Up method to handle pre-existing duplicate DepartmentId and SsoProviderType rows before creating the unique IndexName index. Ensure duplicates are consolidated or removed according to the migration’s established data policy, then retain the existing table and index existence checks before applying the unique constraint.Core/Resgrid.Model/CommandBoards/CommandBoardTemplate.cs (1)
11-98: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider separating state from behavior per coding guidelines.
CommandBoardTemplatemixes plain state (Id,Name,Lanes, etc.) with domain behavior (CreateDefinition,SearchText,MatchesAny). Per coding guidelines, state should live in a record/POCO with operations extracted into a separate static class (or extension methods), which also improves testability of the matching logic independent of the data.As per coding guidelines, "Separate state from behavior (e.g., use records for state and place operations in separate static classes)."
♻️ Suggested direction
- public class CommandBoardTemplate - { - ... - public CommandDefinition CreateDefinition(IEnumerable<UnitType> unitTypes, IEnumerable<PersonnelRole> personnelRoles) { ... } - private static bool MatchesAny(string value, IEnumerable<string> suggestions) { ... } - } + public class CommandBoardTemplate + { + // state only: Id, Name, Category, Description, Keywords, Timer, TimerMinutes, Lanes, LaneCount, SearchText + } + + public static class CommandBoardTemplateOperations + { + public static CommandDefinition CreateDefinition(this CommandBoardTemplate template, + IEnumerable<UnitType> unitTypes, IEnumerable<PersonnelRole> personnelRoles) { ... } + }🤖 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/CommandBoards/CommandBoardTemplate.cs` around lines 11 - 98, Separate the data properties in CommandBoardTemplate from its behavior by keeping the template as a state-only record/POCO and moving SearchText and CreateDefinition into a dedicated static helper or extension class. Move the associated MatchesAny logic with the definition-building behavior, preserve the existing search and assignment results, and update callers to use the extracted operations.Source: Coding guidelines
Core/Resgrid.Model/UnitRoles/UnitRoleTemplateCatalog.cs (1)
167-180: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueInconsistent id prefixes vs. reorganized categories.
The new "Emergency Management" group mixes
er-,dr-, andem-prefixed ids (lines 167, 172, 177), anddr-eoc-teamcollides in meaning with the separate "Disaster Response" category which also usesdr-. Similarly,sec-event-team(line 215) is the lonesec-id in the otherwiseevent--prefixed "Event Medical / Security" group. Align prefixes with category for consistency as the catalog grows.✏️ Suggested renames
- T("er-incident-command", "Incident Command Team", "Emergency Management", + T("em-incident-command", "Incident Command Team", "Emergency Management", ... - T("dr-eoc-team", "Emergency Operations Center", "Emergency Management", + T("em-eoc-team", "Emergency Operations Center", "Emergency Management", ... - T("sec-event-team", "Event Security Team", "Event Medical / Security", + T("event-security-team", "Event Security Team", "Event Medical / Security",Also applies to: 215-218
🤖 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/UnitRoles/UnitRoleTemplateCatalog.cs` around lines 167 - 180, Rename the Emergency Management template IDs in the catalog to use a consistent category-specific prefix, replacing the mixed er-, dr-, and em- prefixes while preserving each template’s content. Update the EOC template so it no longer uses the dr- prefix associated with Disaster Response, and rename sec-event-team to use the event- prefix used by the Event Medical / Security group; update all references to these IDs if present.Providers/Resgrid.Providers.Migrations/Migrations/M0090_AddIncidentContentAndPublicSharing.cs (1)
11-21: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winConsider a filtered unique index instead of a plain non-unique index.
The comment correctly notes SQL Server only allows one NULL in a plain unique index, but the chosen workaround (a non-unique index) means token collisions are only prevented by the random-generation entropy, not the database. FluentMigrator's SQL Server extension supports filtered indexes via
.Filter(...), which lets you keep the index unique for non-null tokens while still allowing multiple legacy NULL rows.🔒 Proposed fix using a SQL Server filtered unique index
- if (!Schema.Table("IncidentCommands").Index("IX_IncidentCommands_PublicShareToken").Exists()) - Create.Index("IX_IncidentCommands_PublicShareToken").OnTable("IncidentCommands").OnColumn("PublicShareToken").Ascending(); + if (!Schema.Table("IncidentCommands").Index("IX_IncidentCommands_PublicShareToken").Exists()) + Create.Index("IX_IncidentCommands_PublicShareToken").OnTable("IncidentCommands").OnColumn("PublicShareToken").Ascending() + .WithOptions().Unique().Filter("[PublicShareToken] IS NOT NULL");🤖 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 `@Providers/Resgrid.Providers.Migrations/Migrations/M0090_AddIncidentContentAndPublicSharing.cs` around lines 11 - 21, Update the PublicShareToken index creation in the IncidentCommands migration to be unique while filtering out NULL values via FluentMigrator’s SQL Server filter support. Preserve the existing index name and idempotent existence check so multiple legacy NULL tokens remain allowed and non-null tokens are database-enforced as unique.
🤖 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.Services/DepartmentSsoService.cs`:
- Around line 726-736: Update MarkSamlAssertionConsumedAsync to extend the
replay-marker TTL by TokenClockSkew when calculating remainingLifetime, so it
remains valid through the full assertion acceptance window. Preserve the
existing expired-assertion rejection and single-use IncrementAsync behavior.
In `@Core/Resgrid.Services/WorkflowSampleDataGenerator.cs`:
- Around line 452-462: Update the sample sha256_hash value assigned in the
incident object within WorkflowSampleDataGenerator so it is a complete
64-character hexadecimal SHA-256 digest, preserving the existing sample-data
structure and field.
In `@Providers/Resgrid.Providers.Weather/NwsIncidentWeatherProvider.cs`:
- Around line 39-97: The caching in
Providers/Resgrid.Providers.Weather/NwsIncidentWeatherProvider.cs lines 39-97
must use ICacheProvider.RetrieveAsync<T>() with the HTTP weather retrieval
placed in its fallback function; preserve coordinate validation, forecast
processing, and cache expiration behavior. Remove the static
ConcurrentDictionary at
Providers/Resgrid.Providers.Weather/NwsIncidentWeatherProvider.cs line 26 and
update the provider’s cache dependency accordingly.
- Around line 86-91: Update the exception filter catch block in NWS incident
weather retrieval to call Resgrid.Framework.Logging.LogException with the caught
exception before setting weather.Current to null. Preserve the existing
graceful-degradation behavior and exception filter.
---
Nitpick comments:
In `@Core/Resgrid.Model/CommandBoards/CommandBoardTemplate.cs`:
- Around line 11-98: Separate the data properties in CommandBoardTemplate from
its behavior by keeping the template as a state-only record/POCO and moving
SearchText and CreateDefinition into a dedicated static helper or extension
class. Move the associated MatchesAny logic with the definition-building
behavior, preserve the existing search and assignment results, and update
callers to use the extracted operations.
In `@Core/Resgrid.Model/UnitRoles/UnitRoleTemplateCatalog.cs`:
- Around line 167-180: Rename the Emergency Management template IDs in the
catalog to use a consistent category-specific prefix, replacing the mixed er-,
dr-, and em- prefixes while preserving each template’s content. Update the EOC
template so it no longer uses the dr- prefix associated with Disaster Response,
and rename sec-event-team to use the event- prefix used by the Event Medical /
Security group; update all references to these IDs if present.
In
`@Providers/Resgrid.Providers.Migrations/Migrations/M0090_AddIncidentContentAndPublicSharing.cs`:
- Around line 11-21: Update the PublicShareToken index creation in the
IncidentCommands migration to be unique while filtering out NULL values via
FluentMigrator’s SQL Server filter support. Preserve the existing index name and
idempotent existence check so multiple legacy NULL tokens remain allowed and
non-null tokens are database-enforced as unique.
In
`@Providers/Resgrid.Providers.Migrations/Migrations/M0091_AddUniqueDepartmentSsoProvider.cs`:
- Around line 12-22: Update the Up method to handle pre-existing duplicate
DepartmentId and SsoProviderType rows before creating the unique IndexName
index. Ensure duplicates are consolidated or removed according to the
migration’s established data policy, then retain the existing table and index
existence checks before applying the unique constraint.
In
`@Providers/Resgrid.Providers.MigrationsPg/Migrations/M0091_AddUniqueDepartmentSsoProviderPg.cs`:
- Around line 12-22: Update the migration’s Up method to handle pre-existing
duplicate rows before creating the unique index IndexName on TableName;
consolidate or remove duplicates according to the migration requirements, then
retain the conditional index creation so the migration remains safe when the
table or index already exists.
🪄 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: f855c404-8720-4884-be69-a580a86bc89d
⛔ Files ignored due to path filters (7)
Tests/Resgrid.Tests/Providers/NwsIncidentWeatherProviderTests.csis excluded by!**/Tests/**Tests/Resgrid.Tests/Services/DepartmentSsoServiceTests.csis excluded by!**/Tests/**Tests/Resgrid.Tests/Services/IncidentCommandContentServiceTests.csis excluded by!**/Tests/**Tests/Resgrid.Tests/Services/IncidentCommandServiceParTests.csis excluded by!**/Tests/**Tests/Resgrid.Tests/Services/TemplateCatalogTests.csis excluded by!**/Tests/**Tests/Resgrid.Tests/Services/WorkflowTemplateContextBuilderTests.csis excluded by!**/Tests/**Tests/Resgrid.Tests/Web/Services/ConnectControllerSsoTests.csis excluded by!**/Tests/**
📒 Files selected for processing (44)
Core/Resgrid.Config/IncidentCommandConfig.csCore/Resgrid.Model/CommandBoards/CommandBoardTemplate.csCore/Resgrid.Model/CommandBoards/CommandBoardTemplateCatalog.csCore/Resgrid.Model/CommandBoards/CommandBoardTemplateLane.csCore/Resgrid.Model/Events/IncidentCommandEvents.csCore/Resgrid.Model/IncidentCommand/IncidentCommand.csCore/Resgrid.Model/IncidentCommand/IncidentCommandBoard.csCore/Resgrid.Model/IncidentCommand/IncidentCommandChanges.csCore/Resgrid.Model/IncidentCommand/IncidentCommandEnums.csCore/Resgrid.Model/IncidentCommand/IncidentContent.csCore/Resgrid.Model/IncidentCommand/IncidentRole.csCore/Resgrid.Model/IncidentCommand/IncidentWeather.csCore/Resgrid.Model/Providers/IIncidentWeatherProvider.csCore/Resgrid.Model/Repositories/IIncidentCommandRepositories.csCore/Resgrid.Model/Services/IIncidentCommandService.csCore/Resgrid.Model/UnitRoles/UnitRoleTemplateCatalog.csCore/Resgrid.Model/WorkflowTemplateVariableCatalog.csCore/Resgrid.Model/WorkflowTriggerEventType.csCore/Resgrid.Services/DepartmentSsoService.csCore/Resgrid.Services/IncidentCommandService.csCore/Resgrid.Services/WorkflowSampleDataGenerator.csCore/Resgrid.Services/WorkflowTemplateContextBuilder.csProviders/Resgrid.Providers.Bus/WorkflowEventProvider.csProviders/Resgrid.Providers.Migrations/Migrations/M0090_AddIncidentContentAndPublicSharing.csProviders/Resgrid.Providers.Migrations/Migrations/M0091_AddUniqueDepartmentSsoProvider.csProviders/Resgrid.Providers.MigrationsPg/Migrations/M0090_AddIncidentContentAndPublicSharingPg.csProviders/Resgrid.Providers.MigrationsPg/Migrations/M0091_AddUniqueDepartmentSsoProviderPg.csProviders/Resgrid.Providers.Weather/NwsIncidentWeatherProvider.csProviders/Resgrid.Providers.Weather/WeatherProviderModule.csRepositories/Resgrid.Repositories.DataRepository/IncidentCommandRepositories.csRepositories/Resgrid.Repositories.DataRepository/Modules/DataModule.csWeb/Resgrid.Web.Services/Controllers/v4/ConnectController.csWeb/Resgrid.Web.Services/Controllers/v4/IncidentCommandController.csWeb/Resgrid.Web.Services/Controllers/v4/PublicIncidentController.csWeb/Resgrid.Web.Services/Controllers/v4/SsoAdminController.csWeb/Resgrid.Web.Services/Models/v4/IncidentCommand/IncidentCommandModels.csWeb/Resgrid.Web/Areas/User/Controllers/CommandController.csWeb/Resgrid.Web/Areas/User/Controllers/SecurityController.csWeb/Resgrid.Web/Areas/User/Views/Command/Index.cshtmlWeb/Resgrid.Web/Areas/User/Views/Command/New.cshtmlWeb/Resgrid.Web/Areas/User/Views/Command/Templates.cshtmlWeb/Resgrid.Web/Areas/User/Views/Command/_CommandForm.cshtmlWeb/Resgrid.Web/Areas/User/Views/Units/_UnitRoleTemplatesModal.cshtmlWeb/Resgrid.Web/wwwroot/js/app/internal/command/resgrid.commands.newcommand.js
Comment on lines
+86
to
+91
| catch (Exception ex) when (ex is HttpRequestException || ex is JsonException || ex is InvalidOperationException) | ||
| { | ||
| // A station observation can be delayed or temporarily unavailable. Forecast/radar data remains useful, | ||
| // so degrade only the current-conditions portion instead of failing the commander weather panel. | ||
| weather.Current = null; | ||
| } |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Log the caught exception.
As per coding guidelines, use Resgrid.Framework.Logging.LogException when catching exceptions, even if the error is swallowed to allow graceful degradation.
📝 Proposed fix to add logging
- catch (Exception ex) when (ex is HttpRequestException || ex is JsonException || ex is InvalidOperationException)
- {
- // A station observation can be delayed or temporarily unavailable. Forecast/radar data remains useful,
- // so degrade only the current-conditions portion instead of failing the commander weather panel.
- weather.Current = null;
- }
+ catch (Exception ex) when (ex is HttpRequestException || ex is JsonException || ex is InvalidOperationException)
+ {
+ Resgrid.Framework.Logging.LogException(ex, "Failed to get latest weather observation");
+ // A station observation can be delayed or temporarily unavailable. Forecast/radar data remains useful,
+ // so degrade only the current-conditions portion instead of failing the commander weather panel.
+ weather.Current = null;
+ }📝 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.
Suggested change
| catch (Exception ex) when (ex is HttpRequestException || ex is JsonException || ex is InvalidOperationException) | |
| { | |
| // A station observation can be delayed or temporarily unavailable. Forecast/radar data remains useful, | |
| // so degrade only the current-conditions portion instead of failing the commander weather panel. | |
| weather.Current = null; | |
| } | |
| catch (Exception ex) when (ex is HttpRequestException || ex is JsonException || ex is InvalidOperationException) | |
| { | |
| Resgrid.Framework.Logging.LogException(ex, "Failed to get latest weather observation"); | |
| // A station observation can be delayed or temporarily unavailable. Forecast/radar data remains useful, | |
| // so degrade only the current-conditions portion instead of failing the commander weather panel. | |
| weather.Current = null; | |
| } |
🤖 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 `@Providers/Resgrid.Providers.Weather/NwsIncidentWeatherProvider.cs` around
lines 86 - 91, Update the exception filter catch block in NWS incident weather
retrieval to call Resgrid.Framework.Logging.LogException with the caught
exception before setting weather.Current to null. Preserve the existing
graceful-degradation behavior and exception filter.
Source: Coding guidelines
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Providers/Resgrid.Providers.Weather/NwsIncidentWeatherProvider.cs (1)
20-20: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winRename the provider implementation to match its interface.
As per coding guidelines, provider implementations must be named
{Name}Providerwhere the interface isI{Name}Provider. Since the interface isIIncidentWeatherProvider, this class should be namedIncidentWeatherProviderto comply with the pattern.♻️ Proposed fix
- public class NwsIncidentWeatherProvider : IIncidentWeatherProvider + public class IncidentWeatherProvider : IIncidentWeatherProvider🤖 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 `@Providers/Resgrid.Providers.Weather/NwsIncidentWeatherProvider.cs` at line 20, Rename the NwsIncidentWeatherProvider class to IncidentWeatherProvider so it matches the IIncidentWeatherProvider naming convention, and update any references to the implementation accordingly.Source: Coding guidelines
🧹 Nitpick comments (1)
Providers/Resgrid.Providers.Weather/NwsIncidentWeatherProvider.cs (1)
146-147: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider parsing JSON directly from the response stream.
To improve performance and avoid allocating a large string for the entire JSON payload, read directly from the response stream using
JsonDocument.ParseAsync().♻️ Proposed fix
- var json = await response.Content.ReadAsStringAsync(cancellationToken); - return JsonDocument.Parse(json); + using var stream = await response.Content.ReadAsStreamAsync(cancellationToken); + return await JsonDocument.ParseAsync(stream, default, cancellationToken);🤖 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 `@Providers/Resgrid.Providers.Weather/NwsIncidentWeatherProvider.cs` around lines 146 - 147, Update the response parsing in the weather provider to obtain the HTTP content stream and use JsonDocument.ParseAsync with the existing cancellationToken, replacing ReadAsStringAsync and JsonDocument.Parse while preserving the returned JsonDocument behavior.
🤖 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 `@Providers/Resgrid.Providers.Weather/NwsIncidentWeatherProvider.cs`:
- Around line 29-38: Update the parameterless NwsIncidentWeatherProvider
constructor to resolve ICacheProvider through
Bootstrapper.GetKernel().Resolve<ICacheProvider>() instead of receiving it via
constructor injection, while preserving the existing HttpClient-based
constructor for deterministic tests and its null validation.
---
Outside diff comments:
In `@Providers/Resgrid.Providers.Weather/NwsIncidentWeatherProvider.cs`:
- Line 20: Rename the NwsIncidentWeatherProvider class to
IncidentWeatherProvider so it matches the IIncidentWeatherProvider naming
convention, and update any references to the implementation accordingly.
---
Nitpick comments:
In `@Providers/Resgrid.Providers.Weather/NwsIncidentWeatherProvider.cs`:
- Around line 146-147: Update the response parsing in the weather provider to
obtain the HTTP content stream and use JsonDocument.ParseAsync with the existing
cancellationToken, replacing ReadAsStringAsync and JsonDocument.Parse while
preserving the returned JsonDocument behavior.
🪄 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: 9a0f2836-3794-4591-8c36-072c483ed8f5
⛔ Files ignored due to path filters (4)
Tests/Resgrid.Tests/Providers/NwsIncidentWeatherProviderTests.csis excluded by!**/Tests/**Tests/Resgrid.Tests/Services/DepartmentSsoServiceTests.csis excluded by!**/Tests/**Tests/Resgrid.Tests/Services/IncidentCommandContentServiceTests.csis excluded by!**/Tests/**Tests/Resgrid.Tests/Services/WorkflowSampleDataGeneratorTests.csis excluded by!**/Tests/**
📒 Files selected for processing (4)
Core/Resgrid.Services/DepartmentSsoService.csCore/Resgrid.Services/IncidentCommandService.csCore/Resgrid.Services/WorkflowSampleDataGenerator.csProviders/Resgrid.Providers.Weather/NwsIncidentWeatherProvider.cs
🚧 Files skipped from review as they are similar to previous changes (3)
- Core/Resgrid.Services/WorkflowSampleDataGenerator.cs
- Core/Resgrid.Services/DepartmentSsoService.cs
- Core/Resgrid.Services/IncidentCommandService.cs
Comment on lines
+29
to
+38
| public NwsIncidentWeatherProvider(ICacheProvider cacheProvider) : this(SharedHttpClient, cacheProvider) | ||
| { | ||
| } | ||
|
|
||
| /// <summary>Constructor exposed for deterministic provider tests with a stubbed HTTP handler.</summary> | ||
| public NwsIncidentWeatherProvider(HttpClient httpClient, ICacheProvider cacheProvider) | ||
| { | ||
| _httpClient = httpClient ?? throw new ArgumentNullException(nameof(httpClient)); | ||
| _cacheProvider = cacheProvider ?? throw new ArgumentNullException(nameof(cacheProvider)); | ||
| } |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Use Service Locator to resolve dependencies explicitly.
As per coding guidelines, use the Service Locator pattern via Bootstrapper.GetKernel().Resolve<T>() to resolve dependencies explicitly in constructors rather than using constructor injection.
♻️ Proposed fix
- public NwsIncidentWeatherProvider(ICacheProvider cacheProvider) : this(SharedHttpClient, cacheProvider)
+ public NwsIncidentWeatherProvider() : this(SharedHttpClient)
{
}
/// <summary>Constructor exposed for deterministic provider tests with a stubbed HTTP handler.</summary>
- public NwsIncidentWeatherProvider(HttpClient httpClient, ICacheProvider cacheProvider)
+ public NwsIncidentWeatherProvider(HttpClient httpClient)
{
_httpClient = httpClient ?? throw new ArgumentNullException(nameof(httpClient));
- _cacheProvider = cacheProvider ?? throw new ArgumentNullException(nameof(cacheProvider));
+ _cacheProvider = Bootstrapper.GetKernel().Resolve<ICacheProvider>();
}📝 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.
Suggested change
| public NwsIncidentWeatherProvider(ICacheProvider cacheProvider) : this(SharedHttpClient, cacheProvider) | |
| { | |
| } | |
| /// <summary>Constructor exposed for deterministic provider tests with a stubbed HTTP handler.</summary> | |
| public NwsIncidentWeatherProvider(HttpClient httpClient, ICacheProvider cacheProvider) | |
| { | |
| _httpClient = httpClient ?? throw new ArgumentNullException(nameof(httpClient)); | |
| _cacheProvider = cacheProvider ?? throw new ArgumentNullException(nameof(cacheProvider)); | |
| } | |
| public NwsIncidentWeatherProvider() : this(SharedHttpClient) | |
| { | |
| } | |
| /// <summary>Constructor exposed for deterministic provider tests with a stubbed HTTP handler.</summary> | |
| public NwsIncidentWeatherProvider(HttpClient httpClient) | |
| { | |
| _httpClient = httpClient ?? throw new ArgumentNullException(nameof(httpClient)); | |
| _cacheProvider = Bootstrapper.GetKernel().Resolve<ICacheProvider>(); | |
| } |
🤖 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 `@Providers/Resgrid.Providers.Weather/NwsIncidentWeatherProvider.cs` around
lines 29 - 38, Update the parameterless NwsIncidentWeatherProvider constructor
to resolve ICacheProvider through
Bootstrapper.GetKernel().Resolve<ICacheProvider>() instead of receiving it via
constructor injection, while preserving the existing HttpClient-based
constructor for deterministic tests and its null validation.
Source: Coding guidelines
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
Web/Resgrid.Web.Services/Controllers/v4/CommandsController.cs (1)
168-168: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winValidate lane color hex values on API requests.
The API currently maps the
Colorstring directly without verifying its hex format, unlike the MVC form parsing which enforces a strict regex (^#[0-9a-fA-F]{3,8}$). Unsanitized color strings could cause rendering bugs when injected into frontend map markers or lane display styles.Apply the identical validation and fallback to
nullto ensure data consistency between the API and the web interface.🛡️ Proposed fix to ensure safe color mapping
- Color = lane.Color, + Color = !string.IsNullOrWhiteSpace(lane.Color) && System.Text.RegularExpressions.Regex.IsMatch(lane.Color.Trim(), "^#[0-9a-fA-F]{3,8}$") ? lane.Color.Trim() : null,🤖 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/CommandsController.cs` at line 168, Update the lane mapping near Color = lane.Color to validate the color against the same strict ^#[0-9a-fA-F]{3,8}$ format used by MVC parsing, and assign null for invalid or unsanitized values. Preserve valid color strings unchanged so API and web-interface behavior remain consistent.Web/Resgrid.Web/Areas/User/Views/Command/_AssignmentModal.cshtml (1)
50-64: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
NewCommandView.LaneColorsto generate the color options dynamically.The lane color options are manually duplicated here in the markup instead of utilizing the newly introduced
NewCommandView.LaneColorspalette collection. Iterating over the centralized list adheres to the DRY principle and ensures the UI automatically adopts any future palette adjustments.♻️ Proposed refactor to dynamically populate options
<div class="form-group"> <label for="assignment-color" class="control-label">Lane Color:</label> <select class="form-control" id="assignment-color"> <option value="">None</option> - <option value="`#e74c3c`" style="background-color:`#e74c3c`;color:`#fff`;">Red</option> - <option value="`#e67e22`" style="background-color:`#e67e22`;color:`#fff`;">Orange</option> - <option value="`#f1c40f`" style="background-color:`#f1c40f`;">Yellow</option> - <option value="`#2ecc71`" style="background-color:`#2ecc71`;color:`#fff`;">Green</option> - <option value="`#1abc9c`" style="background-color:`#1abc9c`;color:`#fff`;">Teal</option> - <option value="`#3498db`" style="background-color:`#3498db`;color:`#fff`;">Blue</option> - <option value="`#9b59b6`" style="background-color:`#9b59b6`;color:`#fff`;">Purple</option> - <option value="`#7f8c8d`" style="background-color:`#7f8c8d`;color:`#fff`;">Gray</option> + `@foreach` (var color in Resgrid.Web.Areas.User.Models.Command.NewCommandView.LaneColors) + { + var textColor = color.Name == "Yellow" ? "" : "color:`#fff`;"; + <option value="`@color.Value`" style="background-color:`@color.Value`;`@textColor`">`@color.Name`</option> + } </select> <span class="help-block m-b-none">Identification color for this lane — board lanes and the map markers of assigned units/personnel are tinted with it.</span> </div>🤖 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/Areas/User/Views/Command/_AssignmentModal.cshtml` around lines 50 - 64, Replace the hardcoded options in the assignment-color select with an iteration over NewCommandView.LaneColors, preserving the empty “None” option and each option’s value, label, and color styling. Use the palette collection as the sole source for lane colors so future palette changes are reflected automatically.
🤖 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/IIncidentReportingService.cs`:
- Around line 16-22: Add an optional bool bypassCache = false parameter to all
listed retrieval methods and matching interface declarations:
IIncidentReportingService methods GetIncidentTimesReportAsync,
GetResourceUtilizationReportAsync, and ExportAfterActionCsvAsync;
IIncidentVoiceService.GetTransmissionLogForCallAsync; and the corresponding
implementations in IncidentReportingService, IncidentVoiceService, and
IncidentCommandService for GetNotesForCallAsync, GetAttachmentsForCallAsync,
GetAttachmentAsync, GetPublicInformationAsync, GetPublicAttachmentAsync, and
GetWeatherForIncidentAsync. Preserve existing behavior while allowing callers to
request fresh data.
In `@Core/Resgrid.Services/IncidentReportingService.cs`:
- Around line 21-26: Refactor the constructors to use the Service Locator
pattern for the newly added dependencies: in
Core/Resgrid.Services/IncidentReportingService.cs lines 21-26, resolve
ICallsService and IIncidentResourcesService via
Bootstrapper.GetKernel().Resolve<T>() instead of constructor parameters; in
Core/Resgrid.Services/IncidentVoiceService.cs lines 29-45, resolve
IVoiceTransmissionLogRepository the same way; and in
Core/Resgrid.Services/IncidentCommandService.cs lines 46-88, resolve
IIncidentNoteRepository, IIncidentAttachmentRepository, and
IIncidentWeatherProvider via the bootstrapper, removing them from constructor
injection while preserving existing assignments and behavior.
In `@Web/Resgrid.Web.Services/Controllers/v4/IncidentReportingController.cs`:
- Around line 134-143: Validate the result of
_incidentReportingService.ExportAfterActionCsvAsync in ExportAfterAction before
calling Encoding.UTF8.GetBytes; return NotFound when csv is null, and preserve
the existing file response for non-null reports.
---
Nitpick comments:
In `@Web/Resgrid.Web.Services/Controllers/v4/CommandsController.cs`:
- Line 168: Update the lane mapping near Color = lane.Color to validate the
color against the same strict ^#[0-9a-fA-F]{3,8}$ format used by MVC parsing,
and assign null for invalid or unsanitized values. Preserve valid color strings
unchanged so API and web-interface behavior remain consistent.
In `@Web/Resgrid.Web/Areas/User/Views/Command/_AssignmentModal.cshtml`:
- Around line 50-64: Replace the hardcoded options in the assignment-color
select with an iteration over NewCommandView.LaneColors, preserving the empty
“None” option and each option’s value, label, and color styling. Use the palette
collection as the sole source for lane colors so future palette changes are
reflected automatically.
🪄 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: 661605f3-b5d7-40ec-b9ab-45286c2d18d2
⛔ Files ignored due to path filters (3)
Tests/Resgrid.Tests/Services/IncidentCommandServiceParTests.csis excluded by!**/Tests/**Tests/Resgrid.Tests/Services/IncidentExportTests.csis excluded by!**/Tests/**Tests/Resgrid.Tests/Services/IncidentVoiceServiceTests.csis excluded by!**/Tests/**
📒 Files selected for processing (31)
Core/Resgrid.Model/CommandBoards/CommandBoardTemplate.csCore/Resgrid.Model/CommandBoards/CommandBoardTemplateCatalog.csCore/Resgrid.Model/CommandBoards/CommandBoardTemplateLane.csCore/Resgrid.Model/CommandDefinitionRole.csCore/Resgrid.Model/IncidentCommand/CommandStructureNode.csCore/Resgrid.Model/IncidentCommand/IncidentReport.csCore/Resgrid.Model/IncidentCommand/VoiceTransmissionLog.csCore/Resgrid.Model/Repositories/IIncidentCommandRepositories.csCore/Resgrid.Model/Services/IIncidentReportingService.csCore/Resgrid.Model/Services/IIncidentVoiceService.csCore/Resgrid.Services/IncidentCommandService.csCore/Resgrid.Services/IncidentReportingService.csCore/Resgrid.Services/IncidentVoiceService.csProviders/Resgrid.Providers.Migrations/Migrations/M0092_AddLaneColorAndVoiceTransmissionLog.csProviders/Resgrid.Providers.MigrationsPg/Migrations/M0092_AddLaneColorAndVoiceTransmissionLogPg.csRepositories/Resgrid.Repositories.DataRepository/IncidentCommandRepositories.csRepositories/Resgrid.Repositories.DataRepository/Modules/ApiDataModule.csRepositories/Resgrid.Repositories.DataRepository/Modules/DataModule.csWeb/Resgrid.Web.Services/Controllers/v4/CommandsController.csWeb/Resgrid.Web.Services/Controllers/v4/IncidentReportingController.csWeb/Resgrid.Web.Services/Controllers/v4/IncidentVoiceController.csWeb/Resgrid.Web.Services/Models/v4/Commands/CommandModels.csWeb/Resgrid.Web.Services/Models/v4/IncidentCommand/IncidentCommandModels.csWeb/Resgrid.Web.Services/Resgrid.Web.Services.xmlWeb/Resgrid.Web/Areas/User/Controllers/CommandController.csWeb/Resgrid.Web/Areas/User/Models/Command/NewCommandView.csWeb/Resgrid.Web/Areas/User/Views/Command/Templates.cshtmlWeb/Resgrid.Web/Areas/User/Views/Command/View.cshtmlWeb/Resgrid.Web/Areas/User/Views/Command/_AssignmentModal.cshtmlWeb/Resgrid.Web/Areas/User/Views/Command/_CommandForm.cshtmlWeb/Resgrid.Web/wwwroot/js/app/internal/command/resgrid.commands.newcommand.js
🚧 Files skipped from review as they are similar to previous changes (8)
- Web/Resgrid.Web/Areas/User/Views/Command/_CommandForm.cshtml
- Core/Resgrid.Model/Repositories/IIncidentCommandRepositories.cs
- Web/Resgrid.Web/Areas/User/Views/Command/Templates.cshtml
- Core/Resgrid.Model/CommandBoards/CommandBoardTemplate.cs
- Web/Resgrid.Web/wwwroot/js/app/internal/command/resgrid.commands.newcommand.js
- Web/Resgrid.Web/Areas/User/Controllers/CommandController.cs
- Core/Resgrid.Model/CommandBoards/CommandBoardTemplateCatalog.cs
- Repositories/Resgrid.Repositories.DataRepository/IncidentCommandRepositories.cs
Comment on lines
+16
to
+22
| Task<IncidentTimesReport> GetIncidentTimesReportAsync(int departmentId, int callId); | ||
|
|
||
| /// <summary>Per-resource lane utilization (which lanes, how long) across the incident.</summary> | ||
| Task<ResourceUtilizationReport> GetResourceUtilizationReportAsync(int departmentId, int callId); | ||
|
|
||
| /// <summary>Full after-action export as a multi-section CSV (summary, times, roles, lanes, utilization, timeline).</summary> | ||
| Task<string> ExportAfterActionCsvAsync(int departmentId, int callId); |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add missing bypassCache parameter to new service retrieval methods.
As per coding guidelines, service methods within Core/Resgrid.Services/**/*.cs must include a bypassCache parameter (default: false) to allow callers to skip cache retrieval when fresh data is required. Please update the signatures for these newly added retrieval methods and their interface definitions:
Core/Resgrid.Model/Services/IIncidentReportingService.cs#L16-L22: Addbool bypassCache = falseparameter to the newly added interface methods.Core/Resgrid.Model/Services/IIncidentVoiceService.cs#L29-L30: Addbool bypassCache = falseparameter toGetTransmissionLogForCallAsyncin the interface.Core/Resgrid.Services/IncidentReportingService.cs#L95-L95: Addbool bypassCache = falseparameter toGetIncidentTimesReportAsync.Core/Resgrid.Services/IncidentReportingService.cs#L146-L146: Addbool bypassCache = falseparameter toGetResourceUtilizationReportAsync.Core/Resgrid.Services/IncidentReportingService.cs#L172-L172: Addbool bypassCache = falseparameter toExportAfterActionCsvAsync.Core/Resgrid.Services/IncidentVoiceService.cs#L130-L130: Addbool bypassCache = falseparameter toGetTransmissionLogForCallAsync.Core/Resgrid.Services/IncidentCommandService.cs#L686-L686: Addbool bypassCache = falseparameter toGetNotesForCallAsync.Core/Resgrid.Services/IncidentCommandService.cs#L775-L775: Addbool bypassCache = falseparameter toGetAttachmentsForCallAsync.Core/Resgrid.Services/IncidentCommandService.cs#L787-L787: Addbool bypassCache = falseparameter toGetAttachmentAsync.Core/Resgrid.Services/IncidentCommandService.cs#L865-L865: Addbool bypassCache = falseparameter toGetPublicInformationAsync.Core/Resgrid.Services/IncidentCommandService.cs#L911-L911: Addbool bypassCache = falseparameter toGetPublicAttachmentAsync.Core/Resgrid.Services/IncidentCommandService.cs#L926-L926: Addbool bypassCache = falseparameter toGetWeatherForIncidentAsync.
📍 Affects 5 files
Core/Resgrid.Model/Services/IIncidentReportingService.cs#L16-L22(this comment)Core/Resgrid.Model/Services/IIncidentVoiceService.cs#L29-L30Core/Resgrid.Services/IncidentReportingService.cs#L95-L95Core/Resgrid.Services/IncidentReportingService.cs#L146-L146Core/Resgrid.Services/IncidentReportingService.cs#L172-L172Core/Resgrid.Services/IncidentVoiceService.cs#L130-L130Core/Resgrid.Services/IncidentCommandService.cs#L686-L686Core/Resgrid.Services/IncidentCommandService.cs#L775-L775Core/Resgrid.Services/IncidentCommandService.cs#L787-L787Core/Resgrid.Services/IncidentCommandService.cs#L865-L865Core/Resgrid.Services/IncidentCommandService.cs#L911-L911Core/Resgrid.Services/IncidentCommandService.cs#L926-L926
🤖 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/IIncidentReportingService.cs` around lines 16 -
22, Add an optional bool bypassCache = false parameter to all listed retrieval
methods and matching interface declarations: IIncidentReportingService methods
GetIncidentTimesReportAsync, GetResourceUtilizationReportAsync, and
ExportAfterActionCsvAsync; IIncidentVoiceService.GetTransmissionLogForCallAsync;
and the corresponding implementations in IncidentReportingService,
IncidentVoiceService, and IncidentCommandService for GetNotesForCallAsync,
GetAttachmentsForCallAsync, GetAttachmentAsync, GetPublicInformationAsync,
GetPublicAttachmentAsync, and GetWeatherForIncidentAsync. Preserve existing
behavior while allowing callers to request fresh data.
Source: Coding guidelines
Comment on lines
+21
to
26
| public IncidentReportingService(IIncidentCommandService incidentCommandService, ICallsService callsService, IIncidentResourcesService incidentResourcesService) | ||
| { | ||
| _incidentCommandService = incidentCommandService; | ||
| _callsService = callsService; | ||
| _incidentResourcesService = incidentResourcesService; | ||
| } |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Use Service Locator pattern for constructor dependencies.
As per coding guidelines, dependencies should be resolved explicitly in constructors using the Service Locator pattern (Bootstrapper.GetKernel().Resolve<T>()) to minimize constructor injection size. Please refactor the newly expanded constructors:
Core/Resgrid.Services/IncidentReportingService.cs#L21-L26: Refactor the constructor to resolve dependencies viaBootstrapper.GetKernel().Resolve<T>()rather than injectingICallsServiceandIIncidentResourcesService.Core/Resgrid.Services/IncidentVoiceService.cs#L29-L45: Resolve the injectedIVoiceTransmissionLogRepositoryvia the bootstrapper instead of adding it to the constructor parameters.Core/Resgrid.Services/IncidentCommandService.cs#L46-L88: Resolve the newly added dependencies (IIncidentNoteRepository,IIncidentAttachmentRepository,IIncidentWeatherProvider) via the bootstrapper rather than expanding constructor injection further.
📍 Affects 3 files
Core/Resgrid.Services/IncidentReportingService.cs#L21-L26(this comment)Core/Resgrid.Services/IncidentVoiceService.cs#L29-L45Core/Resgrid.Services/IncidentCommandService.cs#L46-L88
🤖 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/IncidentReportingService.cs` around lines 21 - 26,
Refactor the constructors to use the Service Locator pattern for the newly added
dependencies: in Core/Resgrid.Services/IncidentReportingService.cs lines 21-26,
resolve ICallsService and IIncidentResourcesService via
Bootstrapper.GetKernel().Resolve<T>() instead of constructor parameters; in
Core/Resgrid.Services/IncidentVoiceService.cs lines 29-45, resolve
IVoiceTransmissionLogRepository the same way; and in
Core/Resgrid.Services/IncidentCommandService.cs lines 46-88, resolve
IIncidentNoteRepository, IIncidentAttachmentRepository, and
IIncidentWeatherProvider via the bootstrapper, removing them from constructor
injection while preserving existing assignments and behavior.
Source: Coding guidelines
| [HttpPost("LogTransmission")] | ||
| [ProducesResponseType(StatusCodes.Status200OK)] | ||
| [Authorize(Policy = ResgridResources.Command_View)] | ||
| public async Task<ActionResult<ICModels.VoiceTransmissionLogResult>> LogTransmission([FromBody] ICModels.LogTransmissionInput input) |
Contributor
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 (1)
Web/Resgrid.Web.Services/Resgrid.Web.Services.xml (1)
1086-1086: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winEscape the XML doc comment in
Web/Resgrid.Web.Services/Controllers/v4/IncidentReportingController.cs:4-7
Per-incident reporting & analyticsshould use&so the class documentation is generated correctly.🤖 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/Resgrid.Web.Services.xml` at line 1086, Update the XML documentation comment for the IncidentReportingController class to escape the ampersand in “Per-incident reporting & analytics” as “&”.
🤖 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 `@Web/Resgrid.Web.Services/Resgrid.Web.Services.xml`:
- Line 1086: Update the XML documentation comment for the
IncidentReportingController class to escape the ampersand in “Per-incident
reporting & analytics” as “&”.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d9360e58-3801-49e8-b4a9-2fb101516bf1
📒 Files selected for processing (2)
Web/Resgrid.Web.Services/Controllers/v4/IncidentReportingController.csWeb/Resgrid.Web.Services/Resgrid.Web.Services.xml
🚧 Files skipped from review as they are similar to previous changes (1)
- Web/Resgrid.Web.Services/Controllers/v4/IncidentReportingController.cs
Member
Author
|
Approve |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit