Conversation
|
Thanks for opening this, but we'd appreciate a little more information. Could you update it with more details? |
Contributor
📝 WalkthroughWalkthroughAdds server-side geocoding endpoints and configuration for OSM/OSRM, replaces Google Maps with Leaflet/OSRM across UI and JS, introduces broad localization, migrates CDN assets to local libs, updates route/indoor-map controllers and client-side route/stop/geofence interactions, and adjusts permission/claims logic. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant GeocodingController
participant IGeoLocationProvider
participant ExternalService as Nominatim/OSRM
rect rgba(173, 216, 230, 0.5)
Note over Client,ExternalService: Forward Geocoding Flow
Client->>GeocodingController: GET /api/v4/Geocoding/ForwardGeocode?address=...
activate GeocodingController
GeocodingController->>IGeoLocationProvider: GetLatLonFromAddress(address)
activate IGeoLocationProvider
IGeoLocationProvider->>ExternalService: HTTP Request (Nominatim)
activate ExternalService
ExternalService-->>IGeoLocationProvider: Return lat,lon
deactivate ExternalService
IGeoLocationProvider-->>GeocodingController: Return "lat,lng"
deactivate IGeoLocationProvider
GeocodingController->>GeocodingController: Parse lat, lon
GeocodingController-->>Client: 200 OK with ForwardGeocodeResult
deactivate GeocodingController
end
rect rgba(255, 182, 193, 0.5)
Note over Client,ExternalService: Reverse Geocoding Flow
Client->>GeocodingController: GET /api/v4/Geocoding/ReverseGeocode?lat=...&lon=...
activate GeocodingController
GeocodingController->>IGeoLocationProvider: GetAddressFromLatLong(lat,lon)
activate IGeoLocationProvider
IGeoLocationProvider->>ExternalService: HTTP Request (Nominatim)
activate ExternalService
ExternalService-->>IGeoLocationProvider: Return address string
deactivate ExternalService
IGeoLocationProvider-->>GeocodingController: Return address
deactivate IGeoLocationProvider
GeocodingController-->>Client: 200 OK with ReverseGeocodeResult
deactivate GeocodingController
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 14
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
Web/Resgrid.Web/Areas/User/Views/Subscription/ViewInvoice.cshtml (1)
18-22:⚠️ Potential issue | 🟡 MinorRedundant fallback configuration.
Both
hrefandasp-fallback-hrefpoint to the same local path (~/lib/bootstrap/dist/css/bootstrap.min.css). This defeats the purpose of the fallback mechanism—if the primary source fails, the fallback will also fail. Consider pointingasp-fallback-hrefto a CDN (e.g., the original MaxCDN/BootstrapCDN URL) to provide actual resilience.Additionally,
crossorigin="anonymous"is unnecessary for same-origin local resources.Suggested fix
- <link rel="stylesheet" href="~/lib/bootstrap/dist/css/bootstrap.min.css" - crossorigin="anonymous" - asp-fallback-href="~/lib/bootstrap/dist/css/bootstrap.min.css" + <link rel="stylesheet" href="~/lib/bootstrap/dist/css/bootstrap.min.css" + asp-fallback-href="https://cdn.jsdelivr.net/npm/bootstrap@3.4.1/dist/css/bootstrap.min.css" + asp-fallback-test-class="hidden" + asp-fallback-test-property="visibility" asp-fallback-test-value="hidden" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Web/Resgrid.Web/Areas/User/Views/Subscription/ViewInvoice.cshtml` around lines 18 - 22, The <link> element in ViewInvoice.cshtml currently uses the same local path for both href and asp-fallback-href and includes crossorigin="anonymous"; update the tag so the asp-fallback-href points to a public CDN URL for Bootstrap (e.g., the official BootstrapCDN URL for the same version used locally) to provide a real fallback, and remove the crossorigin="anonymous" attribute since the primary resource is served from the same origin; keep the existing asp-fallback-test-* attributes and tests intact to preserve the fallback detection logic.Web/Resgrid.Web/Areas/User/Views/Dispatch/Dashboard.cshtml (1)
105-105:⚠️ Potential issue | 🟠 MajorRemove orphaned Google Maps dependency.
The
markerwithlabel.jsscript (line 105) is a Google Maps library that depends on thegoogle.mapsnamespace. However, the Google Maps JavaScript API is not loaded in this view, and themarkerwithlabellibrary is not referenced anywhere in the dispatch JavaScript code.Since the dashboard now uses the Leaflet-based
rg-mapcomponent (line 72), this Google Maps-specific script should be removed as dead code.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Web/Resgrid.Web/Areas/User/Views/Dispatch/Dashboard.cshtml` at line 105, Remove the orphaned Google Maps dependency by deleting the markerwithlabel script include (the line referencing "markerwithlabel/markerwithlabel.js") from the Dispatch Dashboard view; confirm there are no remaining references to the Google Maps namespace or the markerwithlabel library in the dispatch JS and that the view continues to use the Leaflet-based rg-map component (the rg-map usage around line 72) so removing this dead Google Maps script is safe.Web/Resgrid.Web/Areas/User/Views/Reports/StaffingReport.cshtml (1)
18-22:⚠️ Potential issue | 🟡 MinorSame redundant fallback configuration issue.
As with
_StylePartial.cshtml, thehrefandasp-fallback-hrefboth point to the same local path, making the fallback test ineffective.🔧 Suggested fix: Remove redundant fallback attributes
- <link rel="stylesheet" href="~/lib/bootstrap/dist/css/bootstrap.min.css" - crossorigin="anonymous" - asp-fallback-href="~/lib/bootstrap/dist/css/bootstrap.min.css" - asp-fallback-test-class="hidden" - asp-fallback-test-property="visibility" asp-fallback-test-value="hidden" /> + <link rel="stylesheet" href="~/lib/bootstrap/dist/css/bootstrap.min.css" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Web/Resgrid.Web/Areas/User/Views/Reports/StaffingReport.cshtml` around lines 18 - 22, The link element in StaffingReport.cshtml repeats the same local path in href and asp-fallback-href, making the fallback test ineffective; update the <link> for bootstrap (the tag in StaffingReport.cshtml) by removing the redundant asp-fallback-href and related asp-fallback-test-* attributes (or alternatively replace asp-fallback-href with a true CDN fallback URL) so the fallback mechanism is meaningful and not pointing to the identical local file.Web/Resgrid.Web/Views/Shared/_StylePartial.cshtml (1)
1-5:⚠️ Potential issue | 🟡 MinorRedundant fallback configuration.
The
hrefandasp-fallback-hrefnow point to the same local path (~/lib/bootstrap/dist/css/bootstrap.min.css). This renders the fallback mechanism ineffective since both will fail together if the local file is unavailable.Consider either:
- Remove the
asp-fallback-*attributes entirely since they provide no benefit when primary and fallback are identical- Keep a CDN as primary with local fallback, or vice versa
🔧 Suggested fix: Remove redundant fallback attributes
-<link rel="stylesheet" href="~/lib/bootstrap/dist/css/bootstrap.min.css" - asp-fallback-href="~/lib/bootstrap/dist/css/bootstrap.min.css" - asp-fallback-test-class="sr-only" - asp-fallback-test-property="position" - asp-fallback-test-value="absolute" /> +<link rel="stylesheet" href="~/lib/bootstrap/dist/css/bootstrap.min.css" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Web/Resgrid.Web/Views/Shared/_StylePartial.cshtml` around lines 1 - 5, The link tag in _StylePartial.cshtml uses the same local path for href and asp-fallback-href, making the asp-fallback-* attributes redundant; either remove the asp-fallback-href, asp-fallback-test-class, asp-fallback-test-property, and asp-fallback-test-value attributes from the <link> element to eliminate the ineffective fallback, or change href to a CDN URL (keeping asp-fallback-href pointing to the local ~/lib/bootstrap/dist/css/bootstrap.min.css) so the fallback mechanism functions correctly.Web/Resgrid.Web/Views/Shared/_ScriptsPartial.cshtml (1)
7-12:⚠️ Potential issue | 🟡 MinorBootstrap fallback path directory mismatch.
The main
srcpoints to~/lib/bootstrap/dist/js/bootstrap.min.jsbut theasp-fallback-srcpoints to~/lib/bootstrap/js/bootstrap.min.js(missingdist/in the path). These should be consistent.Proposed fix
<script src="~/lib/bootstrap/dist/js/bootstrap.min.js" - asp-fallback-src="~/lib/bootstrap/js/bootstrap.min.js" + asp-fallback-src="~/lib/bootstrap/dist/js/bootstrap.min.js" asp-fallback-test="window.jQuery && window.jQuery.fn && window.jQuery.fn.modal"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Web/Resgrid.Web/Views/Shared/_ScriptsPartial.cshtml` around lines 7 - 12, The asp-fallback-src value in the <script> tag in _ScriptsPartial.cshtml mismatches the main src (missing "dist/"); update the asp-fallback-src attribute to use the same path as src (~/lib/bootstrap/dist/js/bootstrap.min.js) so both src and asp-fallback-src reference the identical bootstrap file and ensure asp-fallback-test remains unchanged.Web/Resgrid.Web/Areas/User/Views/Mapping/EditLayer.cshtml (1)
148-152:⚠️ Potential issue | 🔴 CriticalAvoid raw GeoJSON injection in the
<script>block.Line 151 and Line 181 both write
Model.GeoJsondirectly into JavaScript. That payload includes user-controlled layer metadata from this editor, so a crafted value can break out of the script tag and become stored XSS. Serialize it as a JS string andJSON.parseit client-side instead.Proposed fix
- var geoJson = '@(Model.GeoJson)'; + var geoJson = `@Html.Raw`(Newtonsoft.Json.JsonConvert.SerializeObject(Model.GeoJson));- const collection = L.geoJson(`@Html.Raw`(Model.GeoJson), { + const collection = L.geoJson(JSON.parse(geoJson), { });Also applies to: 181-185, 201-230
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Web/Resgrid.Web/Areas/User/Views/Mapping/EditLayer.cshtml` around lines 148 - 152, The view currently injects raw Model.GeoJson into JavaScript (assigned to the geoJson variable and used elsewhere), which risks stored XSS; instead serialize the server value into a JavaScript string literal and parse it client-side — e.g., replace direct uses of Model.GeoJson in the script blocks (the geoJson assignment and the other occurrences around lines referenced) with a safe serialized expression such as Html.Raw(Json.Encode(Model.GeoJson)) (so the output is a quoted/escaped JSON string) and then call JSON.parse(geoJson) in client script to obtain the object; update every location that writes Model.GeoJson directly (including the geoJson variable and the other blocks noted) to follow this pattern.Web/Resgrid.Web/Areas/User/Views/Dispatch/NewCall.cshtml (1)
543-547:⚠️ Potential issue | 🟡 MinorDuplicate declaration of
newCallFormDatavariable.
newCallFormDatais already declared at line 534. This conditional block (lines 543-547) re-declares it at line 546, which is redundant and potentially confusing. If the intent is to override the value conditionally, it should be an assignment rather than a new declaration.🐛 Proposed fix - remove duplicate declaration
`@if` (Model.CenterCoordinates != null && Model.CenterCoordinates.Latitude.HasValue && Model.CenterCoordinates.Longitude.HasValue) { <script type="text/javascript"> var centerLat = @(Model.CenterCoordinates.Latitude.Value); var centerLng = @(Model.CenterCoordinates.Longitude.Value); - var newCallFormData = '@Html.Raw(Model.NewCallFormData)'; </script> }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Web/Resgrid.Web/Areas/User/Views/Dispatch/NewCall.cshtml` around lines 543 - 547, There is a duplicate declaration of newCallFormData in the script block that redeclares the variable already created earlier; update the block so it does not re-declare newCallFormData (change "var newCallFormData = '@Html.Raw(Model.NewCallFormData)';" to an assignment "newCallFormData = '@Html.Raw(Model.NewCallFormData)';" or remove the line entirely) while leaving centerLat and centerLng assignments intact so Model.NewCallFormData is only assigned, not redeclared.Web/Resgrid.Web/Areas/User/Views/IndoorMaps/ZoneEditor.cshtml (1)
107-114:⚠️ Potential issue | 🟠 MajorFix culture-sensitive decimal rendering and JSON-serialize all JavaScript variable assignments in this script block.
Lines 107-114 have two issues:
- Lines 111-114 (boundsNE, boundsSW, centerLat, centerLon) directly interpolate decimal values without
.ToString("G", System.Globalization.CultureInfo.InvariantCulture). In non-en-US locales that use comma decimals, this produces invalid JavaScript array syntax.- Lines 107-110 (osmTileUrl, osmTileAttribution, floorImageUrl) interpolate strings without JSON serialization. While these specific values are typically safe, using
@Html.Raw(Newtonsoft.Json.JsonConvert.SerializeObject(...))ensures consistent protection and follows the pattern already established at line 115.The CustomMaps/RegionEditor.cshtml view demonstrates the correct numeric pattern with
.ToString("G", InvariantCulture). Apply the same fix to ZoneEditor.cshtml, and extend it to other IndoorMaps and mapping views that lack InvariantCulture on their numeric bounds and coordinates.Fix pattern
- var osmTileUrl = '@Resgrid.Config.MappingConfig.GetWebsiteOSMUrl()'; - var osmTileAttribution = '@Resgrid.Config.MappingConfig.LeafletAttribution'; - var floorImageUrl = '@Url.Action("GetFloorImage", "IndoorMaps", new { id = Model.Floor.IndoorMapFloorId, area = "User" })'; - var boundsNE = [@(Model.Floor.BoundsNELat ?? Model.IndoorMap.BoundsNELat), @(Model.Floor.BoundsNELon ?? Model.IndoorMap.BoundsNELon)]; - var boundsSW = [@(Model.Floor.BoundsSWLat ?? Model.IndoorMap.BoundsSWLat), @(Model.Floor.BoundsSWLon ?? Model.IndoorMap.BoundsSWLon)]; - var centerLat = `@Model.IndoorMap.CenterLatitude`; - var centerLon = `@Model.IndoorMap.CenterLongitude`; + var osmTileUrl = `@Html.Raw`(Newtonsoft.Json.JsonConvert.SerializeObject(Resgrid.Config.MappingConfig.GetWebsiteOSMUrl())); + var osmTileAttribution = `@Html.Raw`(Newtonsoft.Json.JsonConvert.SerializeObject(Resgrid.Config.MappingConfig.LeafletAttribution)); + var floorImageUrl = `@Html.Raw`(Newtonsoft.Json.JsonConvert.SerializeObject(Url.Action("GetFloorImage", "IndoorMaps", new { id = Model.Floor.IndoorMapFloorId, area = "User" }))); + var boundsNE = [@((Model.Floor.BoundsNELat ?? Model.IndoorMap.BoundsNELat).ToString("G", System.Globalization.CultureInfo.InvariantCulture)), @((Model.Floor.BoundsNELon ?? Model.IndoorMap.BoundsNELon).ToString("G", System.Globalization.CultureInfo.InvariantCulture))]; + var boundsSW = [@((Model.Floor.BoundsSWLat ?? Model.IndoorMap.BoundsSWLat).ToString("G", System.Globalization.CultureInfo.InvariantCulture)), @((Model.Floor.BoundsSWLon ?? Model.IndoorMap.BoundsSWLon).ToString("G", System.Globalization.CultureInfo.InvariantCulture))]; + var centerLat = `@Model.IndoorMap.CenterLatitude.ToString`("G", System.Globalization.CultureInfo.InvariantCulture); + var centerLon = `@Model.IndoorMap.CenterLongitude.ToString`("G", System.Globalization.CultureInfo.InvariantCulture);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Web/Resgrid.Web/Areas/User/Views/IndoorMaps/ZoneEditor.cshtml` around lines 107 - 114, Update the ZoneEditor.cshtml script block so all JS variable assignments are JSON-serialized and all numeric values are rendered with invariant culture: wrap osmTileUrl, osmTileAttribution and floorImageUrl with `@Html.Raw`(Newtonsoft.Json.JsonConvert.SerializeObject(...)) and render boundsNE, boundsSW, centerLat and centerLon using .ToString("G", System.Globalization.CultureInfo.InvariantCulture) (or serialize the numeric values via JsonConvert.SerializeObject) to prevent culture-specific decimal commas and ensure valid JavaScript; follow the same pattern used in CustomMaps/RegionEditor.cshtml and apply the same fixes to other IndoorMaps/mapping views that currently interpolate raw decimals or strings.
🟡 Minor comments (16)
Web/Resgrid.Web/Views/Account/ForgotPassword.cshtml-68-69 (1)
68-69:⚠️ Potential issue | 🟡 MinorBroken jQuery fallback path.
The primary script path on line 68 points to
~/lib/jquery/dist/jquery.min.js, which exists. However, the fallback on line 69 references~/lib/jquery/jquery.min.js, which does not exist in the repository. Only thedist/subdirectory contains jQuery files. Update the fallback to use the correct path:Fix
<script src="~/lib/jquery/dist/jquery.min.js"></script> -<script>window.jQuery || document.write('<script src="~/lib/jquery/jquery.min.js"><\/script>')</script> +<script>window.jQuery || document.write('<script src="~/lib/jquery/dist/jquery.min.js"><\/script>')</script>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Web/Resgrid.Web/Views/Account/ForgotPassword.cshtml` around lines 68 - 69, The fallback document.write path in the inline script that checks window.jQuery (the line containing "window.jQuery || document.write('<script src=...')") is pointing to a non-existent "~/lib/jquery/jquery.min.js"; change that fallback string to reference the correct existing file in the dist folder (use "~/lib/jquery/dist/jquery.min.js") so the fallback loads the proper jQuery file when window.jQuery is undefined.Web/Resgrid.Web/Views/Account/Register.cshtml-143-144 (1)
143-144:⚠️ Potential issue | 🟡 MinorFix the jQuery fallback URL; it references a file that does not exist and uses unresolved tilde syntax.
Primary script loads
~/lib/jquery/dist/jquery.min.js(dist folder), but fallback still references~/lib/jquery/jquery.min.js(non-dist). Repository contains no jquery.min.js file in the non-dist location. Additionally, the~/token in the raw JavaScript string will not be resolved by the server—the browser receives it literally and cannot load the resource.Update the fallback to use
@Url.Content()to ensure proper path resolution:Suggested fix
- <script>window.jQuery || document.write('<script src="~/lib/jquery/jquery.min.js"><\/script>')</script> + <script>window.jQuery || document.write('<script src="@Url.Content("~/lib/jquery/dist/jquery.min.js")"><\/script>')</script>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Web/Resgrid.Web/Views/Account/Register.cshtml` around lines 143 - 144, The jQuery fallback uses a nonexistent path and a raw '~/' token that the browser can't resolve; update the inline fallback in Register.cshtml so it writes the same dist path as the primary script and uses Razor path resolution (Url.Content) to emit a correct URL. Replace the document.write fallback that currently references ~/lib/jquery/jquery.min.js with one that emits the resolved path for ~/lib/jquery/dist/jquery.min.js via `@Url.Content` (keep the existing existence check window.jQuery || document.write(...) logic and ensure proper escaping of the closing script tag).Web/Resgrid.Web/Areas/User/Controllers/RoutesController.cs-170-176 (1)
170-176:⚠️ Potential issue | 🟡 MinorUse
DistinctBy(c => c.CallId)to ensure proper deduplication.The
Callclass does not overrideEquals()orGetHashCode(), soDistinct()uses reference equality. If both queries return different object instances for the same call, duplicates won't be removed. SinceCallIdis the unique identifier, useDistinctBy()instead:Suggested change
-var all = calls.Union(active).Distinct().OrderBy(c => c.Name).Select(c => new { id = c.CallId, name = c.Name, address = c.Address }).ToList(); +var all = calls.Union(active).DistinctBy(c => c.CallId).OrderBy(c => c.Name).Select(c => new { id = c.CallId, name = c.Name, address = c.Address }).ToList();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Web/Resgrid.Web/Areas/User/Controllers/RoutesController.cs` around lines 170 - 176, GetCallsForLinking currently uses Union().Distinct(), which relies on reference equality for Call objects; replace the union+distinct logic with concatenation followed by DistinctBy(c => c.CallId) so deduplication is based on the unique CallId (adjust the pipeline in GetCallsForLinking that calls _callsService.GetAllNonDispatchedScheduledCallsByDepartmentIdAsync and _callsService.GetActiveCallsByDepartmentAsync to use calls.Concat(active).DistinctBy(c => c.CallId).OrderBy(...).Select(...).ToList()); ensure System.Linq is available if DistinctBy is not in scope.Web/Resgrid.Web/Views/Account/AffiliateRegister.cshtml-86-87 (1)
86-87:⚠️ Potential issue | 🟡 MinorjQuery version mismatch between primary and fallback.
The primary script loads jQuery from the local library (version 2.2.4 per libman.json), but the fallback references jQuery 1.8.2. This significant version gap (1.8.2 vs 2.2.4) could cause compatibility issues with dependent scripts (jquery-ui.custom, jquery.validate, etc.) if the fallback is triggered.
Consider updating the fallback to reference a local copy of the same jQuery version used in the primary load.
Proposed fix
<script src="~/lib/jquery/dist/jquery.min.js"></script> - <script> window.jQuery || document.write('<script src="@Url.Content("~/Scripts/libs/jquery-1.8.2.min.js")"><\/script>')</script> + <script> window.jQuery || document.write('<script src="@Url.Content("~/lib/jquery/dist/jquery.min.js")"><\/script>')</script>Note: If a true fallback is needed, ensure both paths point to the same jQuery version.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Web/Resgrid.Web/Views/Account/AffiliateRegister.cshtml` around lines 86 - 87, The fallback for jQuery uses an older 1.8.2 file, causing a version mismatch with the primary jQuery (2.2.4); update the fallback reference so it points to the same jQuery build/version as the primary (e.g., the same local ~/lib/jquery/dist/jquery.min.js or a matching 2.2.4 file) so that the fallback and primary are identical and dependent scripts (jquery-ui.custom, jquery.validate) remain compatible.Providers/Resgrid.Providers.Geo/GeoLocationProvider.cs-110-110 (1)
110-110:⚠️ Potential issue | 🟡 MinorDebug artifact: unused exception variable.
The exception is captured into
testbut never used, which appears to be leftover debug code. This is inconsistent with other catch blocks in this file that use empty catches.🧹 Suggested fix: Remove unused variable or keep consistent with other catch blocks
- catch (Exception ex) { var test = ex; /* Don't report on GeoLocation failures */ } + catch { /* Don't report on GeoLocation failures */ }Alternatively, if exception logging is desired, consider actually logging the exception rather than assigning it to an unused variable.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Providers/Resgrid.Providers.Geo/GeoLocationProvider.cs` at line 110, Remove the leftover debug artifact in the catch block that assigns the exception to an unused variable; in the catch (Exception ex) block within the GeoLocationProvider class/method, either delete the "var test = ex" assignment so the catch is empty like the others, or replace it with proper logging if you want to record failures (e.g., call the existing logger); ensure the final catch matches the file's pattern for handling GeoLocation failures.Web/Resgrid.Web/Areas/User/Views/CustomMaps/Layers.cshtml-102-102 (1)
102-102:⚠️ Potential issue | 🟡 MinorPotential JavaScript injection if localized string contains quotes.
The localized string is inserted directly into a JavaScript string literal. If
DeleteLayerConfirmcontains single quotes in any locale, theonclickhandler will break.🔧 Suggested fix: Use JavaScript-safe encoding
-<a class="btn btn-danger btn-xs" asp-controller="CustomMaps" asp-action="DeleteLayer" asp-route-id="@layer.IndoorMapFloorId" asp-route-area="User" onclick="return confirm('@localizer["DeleteLayerConfirm"]');"><i class="fa fa-trash"></i> `@localizer`["Delete"]</a> +<a class="btn btn-danger btn-xs" asp-controller="CustomMaps" asp-action="DeleteLayer" asp-route-id="@layer.IndoorMapFloorId" asp-route-area="User" onclick="return confirm('@Html.Raw(System.Text.Encodings.Web.JavaScriptEncoder.Default.Encode(localizer["DeleteLayerConfirm"].Value))');"><i class="fa fa-trash"></i> `@localizer`["Delete"]</a>Alternatively, use a data attribute and attach the handler via JavaScript.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Web/Resgrid.Web/Areas/User/Views/CustomMaps/Layers.cshtml` at line 102, The inline onclick uses the localized DeleteLayerConfirm directly which can break if the string contains quotes; change the anchor (in Layers.cshtml) to remove the onclick and instead add a data-confirm="@localizer["DeleteLayerConfirm"]" attribute (Razor will HTML-encode the attribute), then add a small client-side script that binds to the DeleteLayer anchor(s) (e.g., querySelectorAll on links with asp-action="DeleteLayer" or a CSS class) and calls window.confirm(this.dataset.confirm) before allowing navigation; this avoids injecting the localized text into a JS string literal and prevents JavaScript injection issues.Web/Resgrid.Web/Views/Shared/_ScriptsPartial.cshtml-1-6 (1)
1-6:⚠️ Potential issue | 🟡 MinorjQuery fallback path is inconsistent with main source.
The
asp-fallback-srcon line 2 referenceslib/jquery/jquery-1.12.3.min.jswhich:
- Is missing the
~/prefix for consistent path resolution- References a specific old version (1.12.3) that may differ from the version in
~/lib/jquery/dist/jquery.min.jsThe fallback should mirror the main source path.
Proposed fix
<script src="~/lib/jquery/dist/jquery.min.js" - asp-fallback-src="lib/jquery/jquery-1.12.3.min.js" + asp-fallback-src="~/lib/jquery/dist/jquery.min.js" asp-fallback-test="window.jQuery"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Web/Resgrid.Web/Views/Shared/_ScriptsPartial.cshtml` around lines 1 - 6, Update the jQuery fallback path in _ScriptsPartial.cshtml so it mirrors the main src and uses a consistent path and version: change the asp-fallback-src on the <script> tag (asp-fallback-src) to use the same ~/lib/jquery/dist/ directory and filename (matching jquery.min.js) rather than the relative lib/jquery/jquery-1.12.3.min.js reference, ensuring the fallback path includes the ~/ prefix and matches the deployed jquery.min.js version.Web/Resgrid.Web/Areas/User/Views/Routes/InstanceDetail.cshtml-33-34 (1)
33-34:⚠️ Potential issue | 🟡 MinorLocalize the instance status badge too.
Line 34 still renders
RouteInstanceStatusdirectly, while Line 60 already maps status text through the localizer. That leaves mixed-language status text on the same page. Reuse a localized mapping for the summary badge as well.Also applies to: 59-60
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Web/Resgrid.Web/Areas/User/Views/Routes/InstanceDetail.cshtml` around lines 33 - 34, The status badge currently prints the enum value via @((Resgrid.Model.RouteInstanceStatus)Model.Instance.Status) which causes mixed-language output; change the badge to use the same localized mapping used for the summary (the existing localizer-based mapping at lines 59-60) so the badge reads localizer[...] for Model.Instance.Status instead of the raw enum; locate the code that renders the summary status text (where localizer is used for the RouteInstanceStatus) and reuse that same expression or helper for the <dd><span class="label label-primary"> so the badge shows the localized string for Model.Instance.Status.Web/Resgrid.Web/Areas/User/Views/Routes/Edit.cshtml-110-115 (1)
110-115:⚠️ Potential issue | 🟡 Minor"Add Stop" button text not localized.
The button text "Add Stop" on line 113 should use the localizer for consistency.
🌐 Proposed fix
- <button type="button" class="btn btn-sm btn-primary pull-right" data-toggle="modal" data-target="#addStopModal"> - <i class="fa fa-plus"></i> Add Stop - </button> + <button type="button" class="btn btn-sm btn-primary pull-right" data-toggle="modal" data-target="#addStopModal"> + <i class="fa fa-plus"></i> `@localizer`["AddStop"] + </button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Web/Resgrid.Web/Areas/User/Views/Routes/Edit.cshtml` around lines 110 - 115, The "Add Stop" button text is not localized; update the button's inner text in the Edit.cshtml view (the <button ... data-target="#addStopModal"> element) to use the localizer (e.g., replace the literal "Add Stop" with `@localizer`["Add Stop"] or a chosen resource key like `@localizer`["AddStop"]) and ensure the corresponding key/value is added to the resource file(s) used by this view so the translated string appears correctly.Web/Resgrid.Web/Areas/User/Views/Routes/Edit.cshtml-182-186 (1)
182-186:⚠️ Potential issue | 🟡 MinorAdd the missing "Station" option to the stop type dropdown.
The
RouteStopTypeenum includes aStationvalue (2), but the dropdown at lines 183–186 only includes Manual (0), Scheduled Call (1), and Waypoint (3). Add the Station option:Suggested fix
<select id="stopType" class="form-control"> <option value="0">Manual</option> <option value="1">Scheduled Call</option> <option value="2">Station</option> <option value="3">Waypoint</option> </select>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Web/Resgrid.Web/Areas/User/Views/Routes/Edit.cshtml` around lines 182 - 186, The stop type dropdown (select with id "stopType") is missing the RouteStopType.Station option (value 2); update the select in Edit.cshtml to include an <option value="2">Station</option> between the Scheduled Call (value 1) and Waypoint (value 3) entries so the UI matches the RouteStopType enum.Web/Resgrid.Web.Services/Controllers/v4/GeocodingController.cs-81-97 (1)
81-97:⚠️ Potential issue | 🟡 MinorMissing coordinate range validation for ReverseGeocode.
The endpoint accepts any double values for
latandlonwithout validating they're within valid geographic ranges (-90 to 90 for latitude, -180 to 180 for longitude). Invalid coordinates could cause provider errors or unexpected behavior.🔧 Proposed fix to add validation
public async Task<ActionResult<ReverseGeocodeResult>> ReverseGeocode([FromQuery] double lat, [FromQuery] double lon) { + if (lat < -90 || lat > 90 || lon < -180 || lon > 180) + return BadRequest("Invalid coordinate range"); + var result = new ReverseGeocodeResult();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Web/Resgrid.Web.Services/Controllers/v4/GeocodingController.cs` around lines 81 - 97, In ReverseGeocode (GeocodingController.ReverseGeocode) add validation for the lat and lon query params before calling _geoLocationProvider: verify lat is between -90 and 90 and lon between -180 and 180, and if validation fails set result.Status to an appropriate error (e.g., ResponseHelper.InvalidRequest or a 400 response), populate result with a descriptive error message and return BadRequest(result) (or the project’s standard V4 error response) instead of calling GetAddressFromLatLong; keep the existing try/catch for provider calls and ensure ResponseHelper.PopulateV4ResponseData is still called on success paths.Web/Resgrid.Web/Areas/User/Views/Mapping/StationRouting.cshtml-93-99 (1)
93-99:⚠️ Potential issue | 🟡 MinorInconsistent localization: hard-coded "onto" in step instructions.
Same issue as in CallExportEx.cshtml - the word "onto" is hard-coded in English.
🌐 Proposed fix
- html += '<li>' + s.maneuver.type + (s.name ? ' onto ' + s.name : '') + + html += '<li>' + s.maneuver.type + (s.name ? ' `@localizer`["OntoDirection"] ' + s.name : '') + ' (' + (s.distance / 1000).toFixed(1) + ' km)</li>';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Web/Resgrid.Web/Areas/User/Views/Mapping/StationRouting.cshtml` around lines 93 - 99, Replace the hard-coded English " onto " string in the JavaScript string concatenation inside the for-loop (variable s in StationRouting.cshtml) with a server-side localized resource value (e.g., inject the localized string into the JS using your view/localizer like `@Localizer`["Onto"] or the project’s equivalent) and preserve spacing and concatenation so the expression becomes: s.maneuver.type + (s.name ? ' ' + localizedOnto + ' ' + s.name : '') ; also apply the same localization approach used in CallExportEx.cshtml to keep translations consistent and ensure proper HTML escaping when embedding the localized value into the script.Web/Resgrid.Web/Areas/User/Views/Dispatch/CallExportEx.cshtml-601-610 (1)
601-610:⚠️ Potential issue | 🟡 MinorInconsistent localization: hard-coded "onto" in step instructions.
The step instruction text contains the hard-coded English word "onto" which won't be translated.
🌐 Proposed fix
Consider either:
- Moving this string to a localizer key
- Or accepting that OSRM maneuver types themselves are English and documenting this limitation
- html += '<li>' + s.maneuver.type + (s.name ? ' onto ' + s.name : '') + + html += '<li>' + s.maneuver.type + (s.name ? ' `@localizer`["OntoDirection"] ' + s.name : '') + ' (' + (s.distance / 1000).toFixed(1) + ' km)</li>';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Web/Resgrid.Web/Areas/User/Views/Dispatch/CallExportEx.cshtml` around lines 601 - 610, The step rendering concatenates a hard-coded English " onto " when building maneuver text (in the loop over steps, using variables steps, s, s.maneuver.type and document.getElementById('dvPanel')), which prevents localization; replace that literal with a localized string pulled from your resource/localizer (e.g., emit a JS variable from the view containing the localized "onto" or call a client-side i18n function) and use that variable when concatenating the label, or alternatively document the OSRM-language limitation if you choose not to localize; ensure the change is applied in the CallExportEx.cshtml HTML construction block so all rendered instructions use the localized token.Web/Resgrid.Web/Areas/User/Views/Routes/Edit.cshtml-165-278 (1)
165-278:⚠️ Potential issue | 🟡 MinorAdd Stop modal contains many hard-coded English strings.
The modal has numerous unlocalizable strings while the main form uses
@localizer. This breaks i18n consistency. Examples include:
- "Add Stop" (title and button, lines 171, 274)
- "Name", "Type", "Priority", "Address", etc. (form labels)
- "Manual", "Scheduled Call", "Waypoint" (dropdown options)
- "Cancel" (line 273)
🌐 Proposed fix pattern
Apply the same localization pattern used in the main form:
- <h4 class="modal-title" id="addStopModalLabel">Add Stop</h4> + <h4 class="modal-title" id="addStopModalLabel">@localizer["AddStop"]</h4>- <label class="col-sm-3 control-label">Name <span class="text-danger">*</span></label> + <label class="col-sm-3 control-label">@localizer["Name"] <span class="text-danger">*</span></label>Apply similar changes to all form labels and button text in the modal.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Web/Resgrid.Web/Areas/User/Views/Routes/Edit.cshtml` around lines 165 - 278, The Add Stop modal contains hard-coded UI strings; update all visible text (modal title, button labels, form labels, select option text, help text, placeholders) to use the same localization pattern as the main form (i.e. replace literals with `@localizer`["KeyName"] or the project's equivalent) for elements inside the addStopModal (references: addStopModal, addStopModalLabel, saveStopBtn, geocodeStopBtn, findStopW3WBtn, stopName, stopType, linkedCallGroup/linkedCallId, stopPriority, stopAddress, stopW3W, stopPickerMap, stopLat, stopLng, stopPlannedArrival, stopPlannedDeparture, stopDwellMinutes, stopContactName, stopContactNumber, stopNotes and the Cancel button); ensure new resource keys are added for "Add Stop", "Cancel", "Name", "Type", "Manual", "Scheduled Call", "Waypoint", "Priority", "Address", "What3Words", "Find", label help text, placeholders and any other literal so the modal matches the main form's localization usage.Web/Resgrid.Web/Areas/User/Views/IndoorMaps/Index.cshtml-53-53 (1)
53-53:⚠️ Potential issue | 🟡 MinorPotential JavaScript injection if localized string contains quotes.
The delete confirmation uses an inline
onclickhandler with a localized string. IfDeleteIndoorMapConfirmcontains an apostrophe (e.g., "Are you sure you want to delete this map?"), it will break the JavaScript.🔧 Proposed fix using JavaScript encoding
-<a class="btn btn-danger btn-xs" asp-controller="IndoorMaps" asp-action="Delete" asp-route-id="@map.IndoorMapId" asp-route-area="User" onclick="return confirm('@localizer["DeleteIndoorMapConfirm"]');"><i class="fa fa-trash"></i> `@localizer`["Delete"]</a> +<a class="btn btn-danger btn-xs" asp-controller="IndoorMaps" asp-action="Delete" asp-route-id="@map.IndoorMapId" asp-route-area="User" onclick="return confirm('@Html.Raw(System.Text.Encodings.Web.JavaScriptEncoder.Default.Encode(localizer["DeleteIndoorMapConfirm"].Value))');"><i class="fa fa-trash"></i> `@localizer`["Delete"]</a>Alternatively, move the confirmation to an external JS file using data attributes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Web/Resgrid.Web/Areas/User/Views/IndoorMaps/Index.cshtml` at line 53, The inline onclick handler with localizer["DeleteIndoorMapConfirm"] can break when the localized string contains quotes; remove the inline onclick and instead add a data-confirm attribute to the <a> (the anchor with asp-controller="IndoorMaps" / asp-action="Delete" / asp-route-id and the Delete text) carrying the raw localized string, and implement a small external JS handler that listens for clicks on anchors with data-confirm (e.g., selector 'a[data-confirm]'), calls window.confirm using the data-confirm value, and only proceeds with the navigation if confirmed; alternatively, if you must keep inline JS, encode the localized string using HttpUtility.JavaScriptStringEncode(localizer["DeleteIndoorMapConfirm"]) before embedding it in the onclick to prevent injection.Web/Resgrid.Web/Areas/User/Views/Dispatch/CallExportEx.cshtml-594-597 (1)
594-597:⚠️ Potential issue | 🟡 MinorInconsistent localization: hard-coded strings in routing display.
The distance and duration labels are hard-coded in English while the rest of the view uses
@localizer. This breaks i18n consistency.🌐 Proposed fix to use localized strings
- var dvDistance = document.getElementById('dvDistance'); - dvDistance.innerHTML = 'Distance: ' + distKm + ' km<br />Duration: ' + durationMin + ' min'; + var dvDistance = document.getElementById('dvDistance'); + dvDistance.innerHTML = '@localizer["DistanceLabel"] ' + distKm + ' km<br />@localizer["DurationLabel"] ' + durationMin + ' min';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Web/Resgrid.Web/Areas/User/Views/Dispatch/CallExportEx.cshtml` around lines 594 - 597, The hard-coded labels in the routing display (the string concatenation that sets dvDistance.innerHTML using 'Distance:' and 'Duration:') break localization; replace those literal English labels by injecting localized strings into the JS (e.g., render server-side/localizer values into JS variables or data attributes) and use those variables when building the innerHTML for distKm and durationMin; update the code that computes distKm, durationMin and writes to element with id 'dvDistance' to use the localized label variables instead of the hard-coded 'Distance:' and 'Duration:' and 'min'.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b9acba2b-bbed-46c2-a21d-a1aa608642cb
⛔ Files ignored due to path filters (4)
Core/Resgrid.Localization/Areas/User/CustomMaps/CustomMaps.en.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/IndoorMaps/IndoorMaps.en.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Mapping/Mapping.en.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Routes/Routes.en.resxis excluded by!**/*.resx
📒 Files selected for processing (98)
Core/Resgrid.Config/MappingConfig.csCore/Resgrid.Localization/Areas/User/CustomMaps/CustomMaps.csCore/Resgrid.Localization/Areas/User/IndoorMaps/IndoorMaps.csCore/Resgrid.Localization/Areas/User/Mapping/Mapping.csCore/Resgrid.Localization/Areas/User/Routes/Routes.csCore/Resgrid.Model/PermissionTypes.csProviders/Resgrid.Providers.Claims/ClaimsLogic.csProviders/Resgrid.Providers.Geo/GeoLocationProvider.csWeb/Resgrid.Web.Services/Controllers/v4/GeocodingController.csWeb/Resgrid.Web.Services/Models/v4/Geocoding/GeocodingResults.csWeb/Resgrid.Web.Services/Resgrid.Web.Services.xmlWeb/Resgrid.Web.Services/appsettings.jsonWeb/Resgrid.Web/Areas/User/Controllers/CustomMapsController.csWeb/Resgrid.Web/Areas/User/Controllers/DispatchController.csWeb/Resgrid.Web/Areas/User/Controllers/IndoorMapsController.csWeb/Resgrid.Web/Areas/User/Controllers/RoutesController.csWeb/Resgrid.Web/Areas/User/Views/Calendar/Edit.cshtmlWeb/Resgrid.Web/Areas/User/Views/Calendar/New.cshtmlWeb/Resgrid.Web/Areas/User/Views/CustomMaps/Edit.cshtmlWeb/Resgrid.Web/Areas/User/Views/CustomMaps/Import.cshtmlWeb/Resgrid.Web/Areas/User/Views/CustomMaps/Index.cshtmlWeb/Resgrid.Web/Areas/User/Views/CustomMaps/Layers.cshtmlWeb/Resgrid.Web/Areas/User/Views/CustomMaps/New.cshtmlWeb/Resgrid.Web/Areas/User/Views/CustomMaps/RegionEditor.cshtmlWeb/Resgrid.Web/Areas/User/Views/Dispatch/AddArchivedCall.cshtmlWeb/Resgrid.Web/Areas/User/Views/Dispatch/CallExport.cshtmlWeb/Resgrid.Web/Areas/User/Views/Dispatch/CallExportEx.cshtmlWeb/Resgrid.Web/Areas/User/Views/Dispatch/Dashboard.cshtmlWeb/Resgrid.Web/Areas/User/Views/Dispatch/NewCall.cshtmlWeb/Resgrid.Web/Areas/User/Views/Dispatch/UpdateCall.cshtmlWeb/Resgrid.Web/Areas/User/Views/Dispatch/ViewCall.cshtmlWeb/Resgrid.Web/Areas/User/Views/Groups/Geofence.cshtmlWeb/Resgrid.Web/Areas/User/Views/IndoorMaps/Edit.cshtmlWeb/Resgrid.Web/Areas/User/Views/IndoorMaps/Floors.cshtmlWeb/Resgrid.Web/Areas/User/Views/IndoorMaps/Index.cshtmlWeb/Resgrid.Web/Areas/User/Views/IndoorMaps/New.cshtmlWeb/Resgrid.Web/Areas/User/Views/IndoorMaps/ZoneEditor.cshtmlWeb/Resgrid.Web/Areas/User/Views/Logs/LogExport.cshtmlWeb/Resgrid.Web/Areas/User/Views/Mapping/AddPOI.cshtmlWeb/Resgrid.Web/Areas/User/Views/Mapping/AddPOIType.cshtmlWeb/Resgrid.Web/Areas/User/Views/Mapping/EditLayer.cshtmlWeb/Resgrid.Web/Areas/User/Views/Mapping/ImportPOIs.cshtmlWeb/Resgrid.Web/Areas/User/Views/Mapping/Index.cshtmlWeb/Resgrid.Web/Areas/User/Views/Mapping/Layers.cshtmlWeb/Resgrid.Web/Areas/User/Views/Mapping/LiveRouting.cshtmlWeb/Resgrid.Web/Areas/User/Views/Mapping/NewLayer.cshtmlWeb/Resgrid.Web/Areas/User/Views/Mapping/POIs.cshtmlWeb/Resgrid.Web/Areas/User/Views/Mapping/StationRouting.cshtmlWeb/Resgrid.Web/Areas/User/Views/Mapping/ViewType.cshtmlWeb/Resgrid.Web/Areas/User/Views/Reports/ActionLogs.cshtmlWeb/Resgrid.Web/Areas/User/Views/Reports/ActiveCallsResourcesReport.cshtmlWeb/Resgrid.Web/Areas/User/Views/Reports/CallSummaryReport.cshtmlWeb/Resgrid.Web/Areas/User/Views/Reports/CertificationsReport.cshtmlWeb/Resgrid.Web/Areas/User/Views/Reports/DepartmentActivityReport.cshtmlWeb/Resgrid.Web/Areas/User/Views/Reports/FlaggedCallNotesReport.cshtmlWeb/Resgrid.Web/Areas/User/Views/Reports/LogReport.cshtmlWeb/Resgrid.Web/Areas/User/Views/Reports/PersonnelEventsReport.cshtmlWeb/Resgrid.Web/Areas/User/Views/Reports/PersonnelHoursDetailReport.cshtmlWeb/Resgrid.Web/Areas/User/Views/Reports/PersonnelHoursReport.cshtmlWeb/Resgrid.Web/Areas/User/Views/Reports/PersonnelReport.cshtmlWeb/Resgrid.Web/Areas/User/Views/Reports/PersonnelStaffingHistoryReport.cshtmlWeb/Resgrid.Web/Areas/User/Views/Reports/StaffingReport.cshtmlWeb/Resgrid.Web/Areas/User/Views/Reports/UnitEventsReport.cshtmlWeb/Resgrid.Web/Areas/User/Views/Reports/UnitStateHistoryReport.cshtmlWeb/Resgrid.Web/Areas/User/Views/Reports/UpcomingShiftReadinessReport.cshtmlWeb/Resgrid.Web/Areas/User/Views/Routes/ActiveRoutes.cshtmlWeb/Resgrid.Web/Areas/User/Views/Routes/Edit.cshtmlWeb/Resgrid.Web/Areas/User/Views/Routes/Index.cshtmlWeb/Resgrid.Web/Areas/User/Views/Routes/InstanceDetail.cshtmlWeb/Resgrid.Web/Areas/User/Views/Routes/Instances.cshtmlWeb/Resgrid.Web/Areas/User/Views/Routes/New.cshtmlWeb/Resgrid.Web/Areas/User/Views/Routes/View.cshtmlWeb/Resgrid.Web/Areas/User/Views/Shared/_ChatWidget.cshtmlWeb/Resgrid.Web/Areas/User/Views/Subscription/ViewInvoice.cshtmlWeb/Resgrid.Web/Views/Account/AffiliateRegister.cshtmlWeb/Resgrid.Web/Views/Account/CompletedInvite.cshtmlWeb/Resgrid.Web/Views/Account/ForgotPassword.cshtmlWeb/Resgrid.Web/Views/Account/Lockout.cshtmlWeb/Resgrid.Web/Views/Account/MissingCode.cshtmlWeb/Resgrid.Web/Views/Account/MissingInvite.cshtmlWeb/Resgrid.Web/Views/Account/Register.cshtmlWeb/Resgrid.Web/Views/Shared/Error.cshtmlWeb/Resgrid.Web/Views/Shared/Unauthorized.cshtmlWeb/Resgrid.Web/Views/Shared/_ScriptsPartial.cshtmlWeb/Resgrid.Web/Views/Shared/_StylePartial.cshtmlWeb/Resgrid.Web/libman.jsonWeb/Resgrid.Web/wwwroot/js/app/internal/custommaps/resgrid.custommaps.editor.jsWeb/Resgrid.Web/wwwroot/js/app/internal/custommaps/resgrid.custommaps.regioneditor.jsWeb/Resgrid.Web/wwwroot/js/app/internal/dispatch/resgrid.dispatch.addArchivedCall.jsWeb/Resgrid.Web/wwwroot/js/app/internal/dispatch/resgrid.dispatch.editcall.jsWeb/Resgrid.Web/wwwroot/js/app/internal/dispatch/resgrid.dispatch.newcall.jsWeb/Resgrid.Web/wwwroot/js/app/internal/groups/resgrid.groups.geofence.jsWeb/Resgrid.Web/wwwroot/js/app/internal/indoormaps/resgrid.indoormaps.editor.jsWeb/Resgrid.Web/wwwroot/js/app/internal/indoormaps/resgrid.indoormaps.zoneeditor.jsWeb/Resgrid.Web/wwwroot/js/app/internal/mapping/resgrid.mapping.viewType.jsWeb/Resgrid.Web/wwwroot/js/app/internal/routes/resgrid.routes.edit.jsWeb/Resgrid.Web/wwwroot/js/app/internal/routes/resgrid.routes.instancedetail.jsWeb/Resgrid.Web/wwwroot/js/app/internal/routes/resgrid.routes.view.js
Comment on lines
+62
to
+63
| } | ||
| catch { /* provider errors are non-fatal */ } |
Contributor
There was a problem hiding this comment.
Silent exception swallowing loses valuable debugging information.
The empty catch block discards all exceptions, making it impossible to diagnose provider failures. Consider logging the exception even if it's non-fatal for the request.
🔧 Proposed fix to log exceptions
+ private readonly ILogger<GeocodingController> _logger;
+
- public GeocodingController(IGeoLocationProvider geoLocationProvider)
+ public GeocodingController(IGeoLocationProvider geoLocationProvider, ILogger<GeocodingController> logger)
{
_geoLocationProvider = geoLocationProvider;
+ _logger = logger;
}
// In ForwardGeocode:
- catch { /* provider errors are non-fatal */ }
+ catch (Exception ex)
+ {
+ _logger.LogWarning(ex, "Forward geocoding failed for address: {Address}", address);
+ }As per coding guidelines, design for testability and avoid hidden dependencies - silent failures make debugging and testing difficult.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Web/Resgrid.Web.Services/Controllers/v4/GeocodingController.cs` around lines
62 - 63, The empty catch in GeocodingController silently swallows provider
exceptions; change it to catch Exception (e.g., catch (Exception ex)) and log
the exception using the controller's logger or a new injected
ILogger<GeocodingController> (e.g., LogWarning/LogError with context like
"Geocoding provider failed") while preserving the non-fatal behavior (do not
rethrow), so provider errors are recorded for debugging and tests.
Comment on lines
+65
to
+66
| var endLat = @Model.EndLat; | ||
| var endLon = @Model.EndLon; |
Contributor
There was a problem hiding this comment.
Serialize coordinates as JSON/invariant numbers.
Direct Razor numeric output can be culture-formatted (comma decimal) and break JavaScript numeric parsing.
🌍 Proposed fix
- var endLat = `@Model.EndLat`;
- var endLon = `@Model.EndLon`;
+ var endLat = `@Html.Raw`(System.Text.Json.JsonSerializer.Serialize(Model.EndLat));
+ var endLon = `@Html.Raw`(System.Text.Json.JsonSerializer.Serialize(Model.EndLon));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Web/Resgrid.Web/Areas/User/Views/Mapping/LiveRouting.cshtml` around lines 65
- 66, The numeric Razor outputs endLat and endLon are rendered using server
culture and may produce comma decimals; change their rendering to
culture-invariant JSON numbers by serializing the values (e.g., use a JSON
serializer or call ToString(System.Globalization.CultureInfo.InvariantCulture)
and write as raw output) so the JavaScript receives valid numeric literals;
update the endLat and endLon usages in LiveRouting.cshtml to emit
invariant/JSON-formatted numbers instead of direct Razor numeric interpolation.
Comment on lines
+1
to
+2
| <script src="~/lib/twilio/twilio-common.min.js"></script> | ||
| <script src="~/lib/twilio/twilio-chat.min.js"></script> |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify that the Twilio library files exist in the expected location
# Search for Twilio library files in the repository
fd -t f -e js "twilio" --full-path
# Also check if there's a lib/twilio directory structure
fd -t d "twilio" --full-pathRepository: Resgrid/Core
Length of output: 74
🏁 Script executed:
# Check the Web/Resgrid.Web structure for wwwroot and lib directories
find Web/Resgrid.Web/wwwroot -type d -name "lib" 2>/dev/null | head -20
# Check if the lib/twilio directory exists
find Web/Resgrid.Web/wwwroot -type d -name "twilio" 2>/dev/null | head -20
# List all files in the References/Twilio directory
find References/Twilio -type f 2>/dev/null | head -30
# Check what's in the lib directory if it exists
ls -la Web/Resgrid.Web/wwwroot/lib/ 2>/dev/null | head -50
# Check specifically for the script files mentioned in the view
find . -name "twilio-common.min.js" -o -name "twilio-chat.min.js" 2>/dev/nullRepository: Resgrid/Core
Length of output: 3665
🏁 Script executed:
# Check if the file exists and read its content
cat -n Web/Resgrid.Web/Areas/User/Views/Shared/_ChatWidget.cshtml 2>/dev/null | head -30
# Check the file size to understand how much content we have
wc -l Web/Resgrid.Web/Areas/User/Views/Shared/_ChatWidget.cshtml
# Double-check for any twilio files in the entire repo, including subdirectories
find . -path "*/node_modules" -prune -o -name "*twilio*" -type f -print 2>/dev/nullRepository: Resgrid/Core
Length of output: 972
Missing Twilio JavaScript libraries—chat widget will fail to load.
The referenced files twilio-common.min.js and twilio-chat.min.js do not exist in Web/Resgrid.Web/wwwroot/lib/twilio/. The application will fail at runtime when attempting to load these dependencies. Either add the missing Twilio JavaScript SDK files to the repository or revert these script references if this change was unintended.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Web/Resgrid.Web/Areas/User/Views/Shared/_ChatWidget.cshtml` around lines 1 -
2, In _ChatWidget.cshtml remove or fix the broken Twilio script references:
either add the missing Twilio SDK files (twilio-common.min.js and
twilio-chat.min.js) into wwwroot/lib/twilio/ so the existing <script> tags load
correctly, or revert/update the two <script
src="~/lib/twilio/twilio-common.min.js"> and <script
src="~/lib/twilio/twilio-chat.min.js"> lines to point to valid local paths or
official CDN URLs; ensure the referenced filenames and paths exactly match the
files you add or the CDN spec so the chat widget can load.
Web/Resgrid.Web/wwwroot/js/app/internal/dispatch/resgrid.dispatch.editcall.js
Show resolved
Hide resolved
Web/Resgrid.Web/wwwroot/js/app/internal/dispatch/resgrid.dispatch.newcall.js
Show resolved
Hide resolved
Comment on lines
+147
to
+148
| if (result && result.Data && result.Data.Latitude && result.Data.Longitude) { | ||
| setPickerLocation(result.Data.Latitude, result.Data.Longitude, false); |
Contributor
There was a problem hiding this comment.
Valid 0 coordinates are currently treated as missing.
Checks like if (Latitude && Longitude) fail at the equator/prime meridian. Use numeric finite checks instead.
🧭 Proposed fix
- if (result && result.Data && result.Data.Latitude && result.Data.Longitude) {
- setPickerLocation(result.Data.Latitude, result.Data.Longitude, false);
+ var lat = Number(result && result.Data ? result.Data.Latitude : NaN);
+ var lng = Number(result && result.Data ? result.Data.Longitude : NaN);
+ if (Number.isFinite(lat) && Number.isFinite(lng)) {
+ setPickerLocation(lat, lng, false);
} else {
alert('Address not found.');
}
@@
- if (data && data.Latitude && data.Longitude) {
- setPickerLocation(data.Latitude, data.Longitude, true);
+ var lat = Number(data ? data.Latitude : NaN);
+ var lng = Number(data ? data.Longitude : NaN);
+ if (Number.isFinite(lat) && Number.isFinite(lng)) {
+ setPickerLocation(lat, lng, true);
} else {
alert('What3Words was unable to find a location for those words. Ensure it is 3 words separated by periods.');
}Also applies to: 170-171
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Web/Resgrid.Web/wwwroot/js/app/internal/routes/resgrid.routes.edit.js` around
lines 147 - 148, The current truthy checks treat 0 coordinates as missing;
replace checks like "if (result && result.Data && result.Data.Latitude &&
result.Data.Longitude)" with numeric finite checks (e.g., use Number.isFinite or
isFinite and ensure values are not null/undefined) before calling
setPickerLocation(result.Data.Latitude, result.Data.Longitude, false); apply the
same change to the other occurrence that checks Latitude/Longitude (the one
around setPickerLocation and the check at the later block referencing
result.Data.Latitude/Longitude).
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| var success = await _indoorMapService.DeleteFloorAsync(id, cancellationToken); | ||
| if (!success) | ||
| { | ||
| _logger.LogWarning("DeleteFloor failed for floor {FloorId} in department {DepartmentId}.", id, DepartmentId); |
Check failure
Code scanning / CodeQL
Log entries created from user input High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 10 days ago
In general, to fix log-forging issues, any user-controlled data passed to log methods should be sanitized to remove or neutralize characters that can affect the structure of the logs, especially newline and carriage return characters. In .NET, this can be done with string.Replace (for example, removing \r and \n) or by otherwise constraining the logged representation of the user input.
For this specific case, the simplest and least intrusive fix is to create a sanitized version of id just before logging and use that sanitized value in the log call. For example, we can create var safeId = id?.Replace("\r", "").Replace("\n", ""); and then pass safeId into _logger.LogWarning instead of id. This changes only the logged representation (removing control characters) and does not affect any other logic. The change should be made in Web/Resgrid.Web/Areas/User/Controllers/IndoorMapsController.cs, within the DeleteFloor method, around line 94–99, where the logging occurs. No new imports or external methods are needed; we only use string.Replace, which is already available.
Suggested changeset
1
Web/Resgrid.Web/Areas/User/Controllers/IndoorMapsController.cs
| @@ -94,7 +94,8 @@ | ||
| var success = await _indoorMapService.DeleteFloorAsync(id, cancellationToken); | ||
| if (!success) | ||
| { | ||
| _logger.LogWarning("DeleteFloor failed for floor {FloorId} in department {DepartmentId}.", id, DepartmentId); | ||
| var safeId = id?.Replace("\r", string.Empty).Replace("\n", string.Empty); | ||
| _logger.LogWarning("DeleteFloor failed for floor {FloorId} in department {DepartmentId}.", safeId, DepartmentId); | ||
| TempData["ErrorMessage"] = "Failed to delete the floor. Please try again."; | ||
| } | ||
|
|
Copilot is powered by AI and may make mistakes. Always verify output.
Unable to commit as this autofix suggestion is now outdated
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
Web/Resgrid.Web/Areas/User/Views/Mapping/EditLayer.cshtml (2)
195-227:⚠️ Potential issue | 🟡 MinorRemove
e.preventDefault()andthis.submit()to allow normal form submission.With
e.preventDefault()followed bythis.submit(), the form bypasses any client-side validation hooks. Since the GeoJson serialization is synchronous, populate#GeoJsonand let the form submit normally so that validation can run.🛠️ Suggested change
$('#newLayerForm').on('submit', function (e) { - e.preventDefault(); let layers = map.pm.getGeomanLayers(); @@ $('#GeoJson').val(JSON.stringify(featureCollection)); - - this.submit(); });Additionally, consider including
<partial name="_ValidationScriptsPartial" />in the Scripts section to enable client-side validation, as other views in this area do.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Web/Resgrid.Web/Areas/User/Views/Mapping/EditLayer.cshtml` around lines 195 - 227, In the submit handler for '#newLayerForm' remove the call to e.preventDefault() and the explicit this.submit() so the handler only serializes the map into '#GeoJson' and then returns, allowing normal form submission and client-side validation to run; keep the layers -> features mapping and the $('#GeoJson').val(JSON.stringify(featureCollection)) logic intact inside the handler. Also add the view-level Scripts partial inclusion <partial name="_ValidationScriptsPartial" /> (in the Scripts section of this view) to enable client-side validation scripts.
202-203:⚠️ Potential issue | 🔴 CriticalRemove user-controlled strings from the GeoJSON payload rendered with
@Html.Raw.At lines 202–203 and 212,
color,name, andtextare read directly from user input and stored in GeoJSON properties. This view later renders the stored GeoJSON with@Html.Raw(Model.GeoJson)at line 181 inside a script block. A user can inject</script>to break out of the script context and execute arbitrary code when the layer is reopened.Either sanitize these fields before persistence, store them outside the GeoJSON payload, or use a safe JSON deserialization approach on the client side instead of
@Html.Raw.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Web/Resgrid.Web/Areas/User/Views/Mapping/EditLayer.cshtml` around lines 202 - 203, The view is injecting user-controlled strings (geo.properties.color, geo.properties.name, geo.properties.text) into Model.GeoJson and rendering it with `@Html.Raw`(Model.GeoJson), allowing a script injection; fix by ensuring these fields are validated/sanitized on the server before they are included in the GeoJSON (e.g., enforce a strict color whitelist/hex regex for color and strip/encode HTML/JS from name/text or store them outside the GeoJSON), and stop rendering raw user strings client-side by emitting the GeoJSON via a safe JSON serializer (server-side JSON encode/serialize) instead of `@Html.Raw`(Model.GeoJson).
🧹 Nitpick comments (4)
Web/Resgrid.Web/wwwroot/js/app/internal/dispatch/resgrid.dispatch.editcall.js (1)
354-364: Consider extracting shared reverse geocoding logic.The fetch logic in
geocodeCoordinates(lines 355-362) is nearly identical tofindLocation(lines 321-328). You could extract a helper to reduce duplication:♻️ Proposed refactor to extract shared helper
+ function reverseGeocodeAndUpdateAddress(lat, lng) { + fetch(resgrid.absoluteApiBaseUrl + '/api/v4/Geocoding/ReverseGeocode?lat=' + lat + '&lon=' + lng, { headers: { 'Authorization': 'Bearer ' + localStorage.getItem('RgWebApp.auth-tokens') } }) + .then(function(r) { return r.json(); }) + .then(function(result) { + if (result && result.Data && result.Data.Address) { + $("#Call_Address").val(result.Data.Address); + } + }) + .catch(function(err) { console.error("Reverse geocode error:", err); }); + } + editcall.reverseGeocodeAndUpdateAddress = reverseGeocodeAndUpdateAddress; + function findLocation(pos) { - fetch(resgrid.absoluteApiBaseUrl + '/api/v4/Geocoding/ReverseGeocode?lat=' + pos.lat + '&lon=' + pos.lng, { headers: { 'Authorization': 'Bearer ' + localStorage.getItem('RgWebApp.auth-tokens') } }) - .then(function(r) { return r.json(); }) - .then(function(result) { - if (result && result.Data && result.Data.Address) { - $("#Call_Address").val(result.Data.Address); - } - }) - .catch(function(err) { console.error("Reverse geocode error:", err); }); + reverseGeocodeAndUpdateAddress(pos.lat, pos.lng); $("#Latitude").val(pos.lat.toString()); $("#Longitude").val(pos.lng.toString()); }Then update
geocodeCoordinatesto callreverseGeocodeAndUpdateAddress(lat, lng)as well.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Web/Resgrid.Web/wwwroot/js/app/internal/dispatch/resgrid.dispatch.editcall.js` around lines 354 - 364, Extract the duplicated fetch/reverse-geocoding logic used in findLocation and geocodeCoordinates into a single helper (e.g., reverseGeocodeAndUpdateAddress) that accepts lat/lng, performs the fetch to '/api/v4/Geocoding/ReverseGeocode', handles the Authorization header, parses JSON, checks result.Data.Address, and updates $("#Call_Address").val(...); then replace the bodies of findLocation and geocodeCoordinates so they simply call reverseGeocodeAndUpdateAddress(lat, lng) and keep editcall.geocodeCoordinates assignment unchanged.Web/Resgrid.Web/Views/Account/LogOn.cshtml (1)
104-132: Consider centralizing locale-to-flag metadata to prevent drift.The locale list and
flagMapare now repeated in multiple views/scripts. A shared source (server-provided JSON or one shared JS module/partial) would reduce future mismatch risk.Also applies to: 175-185
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Web/Resgrid.Web/Views/Account/LogOn.cshtml` around lines 104 - 132, Duplicate locale-to-flag mappings are hard-coded in this view (the <ul class="dropdown-menu"> entries) and a repeated flagMap elsewhere; extract the locale/flag metadata into a single shared source (e.g., a server-provided JSON endpoint or a shared JS module/partial such as a _LocaleFlags partial or locales.json) and replace the hard-coded <a class="langDropdownSelection" data-locale="..."> items with a loop that renders entries from that shared data; update the other occurrence noted (lines 175-185 / the other flagMap usage) to read from the same shared module/endpoint so all views/scripts use one canonical locale-to-flag mapping.Web/Resgrid.Web/Areas/User/Controllers/IndoorMapsController.cs (1)
97-97: Localize the user-facing failure message.Line 97 introduces hardcoded English text, which will bypass localization in localized UI flows.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Web/Resgrid.Web/Areas/User/Controllers/IndoorMapsController.cs` at line 97, Replace the hardcoded TempData["ErrorMessage"] = "Failed to delete the floor. Please try again."; with a localized string lookup so the UI uses resource translations; locate the assignment in IndoorMapsController (the DeleteFloor / floor deletion action) and change it to set TempData["ErrorMessage"] = _localizer["FailedToDeleteFloor"] (or Resource.Messages.FailedToDeleteFloor) using the controller's IStringLocalizer or existing resource class, adding the "FailedToDeleteFloor" key to the resource files for each supported culture.Web/Resgrid.Web/wwwroot/js/app/internal/dispatch/resgrid.dispatch.newcall.js (1)
365-372: Same missing HTTP status check pattern.This fetch call has the same issue as the forward geocoding call. Consider extracting a helper function for authenticated API calls that handles status checks consistently:
Example helper function
function fetchApi(url) { return fetch(url, { headers: { 'Authorization': 'Bearer ' + localStorage.getItem('RgWebApp.auth-tokens') } }) .then(function(r) { if (!r.ok) throw new Error('HTTP ' + r.status); return r.json(); }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Web/Resgrid.Web/wwwroot/js/app/internal/dispatch/resgrid.dispatch.newcall.js` around lines 365 - 372, The reverse-geocode fetch lacks an HTTP status check; extract a helper like fetchApi that wraps fetch with the Authorization header, checks r.ok and throws on non-2xx (e.g., throw new Error('HTTP ' + r.status)), then returns r.json(); replace the inline fetch call in resgrid.dispatch.newcall.js with fetchApi('/api/v4/Geocoding/ReverseGeocode?lat=' + lat + '&lon=' + lng) and handle .then(result) and .catch(err) the same way so non-OK responses are surfaced as errors before JSON processing (refer to the existing Authorization header, userSuppliedAddress check, and $("#Call_Address").val usage to locate where to change).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Web/Resgrid.Web/Areas/User/Controllers/IndoorMapsController.cs`:
- Around line 27-34: The Select on zones can NRE if SearchZonesAsync returns
null; in IndoorMapsController (the action where zones is assigned from
_indoorMapService.SearchZonesAsync), guard zones before projecting by replacing
the null with an empty enumerable or returning an appropriate empty result.
Locate the variable zones and the projection into results (zones.Select(...))
and change the code to use a null-coalescing fallback (e.g., zones ??
Enumerable.Empty<YourZoneType>()) or check for null and set results to an empty
sequence so results.Select never runs on null.
In `@Web/Resgrid.Web/Areas/User/Views/IndoorMaps/Floors.cshtml`:
- Around line 78-81: Add an anti-forgery token to the delete form used for the
DeleteFloor action and ensure the confirm dialog string is safely escaped:
inside the form that posts to IndoorMapsController.DeleteFloor (the form that
posts the hidden id floor.IndoorMapFloorId), call Html.AntiForgeryToken() so
[ValidateAntiForgeryToken] won’t 400; for the onclick confirm use a
JSON-encoded/JS-escaped string (e.g., serialize localizer["DeleteFloorConfirm"]
with Json.Serialize or output via Html.Raw(Json.Serialize(...))) or move the
confirmation to a data-confirm attribute and handle with JavaScript so
apostrophes in localizer["DeleteFloorConfirm"] won’t break the JS string.
In
`@Web/Resgrid.Web/wwwroot/js/app/internal/dispatch/resgrid.dispatch.newcall.js`:
- Around line 114-128: The fetch call that starts with
fetch(resgrid.absoluteApiBaseUrl + '/api/v4/Geocoding/ForwardGeocode?address=' +
...) does not check the HTTP response status before calling r.json(), so 4xx/5xx
responses (e.g. 401) will be parsed as JSON and treated as successful; update
the promise chain to first check the Response object (use response.ok or
response.status) and if not ok throw or handle a clear error (include
response.status/statusText), only call r.json() when response.ok, and in the
catch/logging include the HTTP status and message; keep existing success
handling that uses result.Data and
resgrid.dispatch.newcall.setMarkerLocation(lat, lng).
---
Outside diff comments:
In `@Web/Resgrid.Web/Areas/User/Views/Mapping/EditLayer.cshtml`:
- Around line 195-227: In the submit handler for '#newLayerForm' remove the call
to e.preventDefault() and the explicit this.submit() so the handler only
serializes the map into '#GeoJson' and then returns, allowing normal form
submission and client-side validation to run; keep the layers -> features
mapping and the $('#GeoJson').val(JSON.stringify(featureCollection)) logic
intact inside the handler. Also add the view-level Scripts partial inclusion
<partial name="_ValidationScriptsPartial" /> (in the Scripts section of this
view) to enable client-side validation scripts.
- Around line 202-203: The view is injecting user-controlled strings
(geo.properties.color, geo.properties.name, geo.properties.text) into
Model.GeoJson and rendering it with `@Html.Raw`(Model.GeoJson), allowing a script
injection; fix by ensuring these fields are validated/sanitized on the server
before they are included in the GeoJSON (e.g., enforce a strict color
whitelist/hex regex for color and strip/encode HTML/JS from name/text or store
them outside the GeoJSON), and stop rendering raw user strings client-side by
emitting the GeoJSON via a safe JSON serializer (server-side JSON
encode/serialize) instead of `@Html.Raw`(Model.GeoJson).
---
Nitpick comments:
In `@Web/Resgrid.Web/Areas/User/Controllers/IndoorMapsController.cs`:
- Line 97: Replace the hardcoded TempData["ErrorMessage"] = "Failed to delete
the floor. Please try again."; with a localized string lookup so the UI uses
resource translations; locate the assignment in IndoorMapsController (the
DeleteFloor / floor deletion action) and change it to set
TempData["ErrorMessage"] = _localizer["FailedToDeleteFloor"] (or
Resource.Messages.FailedToDeleteFloor) using the controller's IStringLocalizer
or existing resource class, adding the "FailedToDeleteFloor" key to the resource
files for each supported culture.
In `@Web/Resgrid.Web/Views/Account/LogOn.cshtml`:
- Around line 104-132: Duplicate locale-to-flag mappings are hard-coded in this
view (the <ul class="dropdown-menu"> entries) and a repeated flagMap elsewhere;
extract the locale/flag metadata into a single shared source (e.g., a
server-provided JSON endpoint or a shared JS module/partial such as a
_LocaleFlags partial or locales.json) and replace the hard-coded <a
class="langDropdownSelection" data-locale="..."> items with a loop that renders
entries from that shared data; update the other occurrence noted (lines 175-185
/ the other flagMap usage) to read from the same shared module/endpoint so all
views/scripts use one canonical locale-to-flag mapping.
In
`@Web/Resgrid.Web/wwwroot/js/app/internal/dispatch/resgrid.dispatch.editcall.js`:
- Around line 354-364: Extract the duplicated fetch/reverse-geocoding logic used
in findLocation and geocodeCoordinates into a single helper (e.g.,
reverseGeocodeAndUpdateAddress) that accepts lat/lng, performs the fetch to
'/api/v4/Geocoding/ReverseGeocode', handles the Authorization header, parses
JSON, checks result.Data.Address, and updates $("#Call_Address").val(...); then
replace the bodies of findLocation and geocodeCoordinates so they simply call
reverseGeocodeAndUpdateAddress(lat, lng) and keep editcall.geocodeCoordinates
assignment unchanged.
In
`@Web/Resgrid.Web/wwwroot/js/app/internal/dispatch/resgrid.dispatch.newcall.js`:
- Around line 365-372: The reverse-geocode fetch lacks an HTTP status check;
extract a helper like fetchApi that wraps fetch with the Authorization header,
checks r.ok and throws on non-2xx (e.g., throw new Error('HTTP ' + r.status)),
then returns r.json(); replace the inline fetch call in
resgrid.dispatch.newcall.js with
fetchApi('/api/v4/Geocoding/ReverseGeocode?lat=' + lat + '&lon=' + lng) and
handle .then(result) and .catch(err) the same way so non-OK responses are
surfaced as errors before JSON processing (refer to the existing Authorization
header, userSuppliedAddress check, and $("#Call_Address").val usage to locate
where to change).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: dcaf0a09-228d-4339-b068-e23140bef684
⛔ Files ignored due to path filters (34)
Core/Resgrid.Localization/Account/Login.ar.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Account/DeleteAccount.ar.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Account/ForcePasswordChange.ar.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Calendar/Calendar.ar.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Contacts/Contacts.ar.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/CustomMaps/CustomMaps.ar.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/CustomStatuses/CustomStatuses.ar.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Department/Department.ar.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Department/DepartmentTypes.ar.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Dispatch/Call.ar.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Dispatch/Dashboard.ar.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Documents/Documents.ar.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Home/EditProfile.ar.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Home/HomeDashboard.ar.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/IndoorMaps/IndoorMaps.ar.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Inventory/Inventory.ar.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Logs/Logs.ar.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Mapping/Mapping.ar.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Messages/Messages.ar.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Notes/Note.ar.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Personnel/Person.ar.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Profile/Profile.ar.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Reports/FlaggedReport.ar.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Routes/Routes.ar.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Security/Security.ar.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Shifts/Shifts.ar.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Subscription/Subscription.ar.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Templates/Templates.ar.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/TwoFactor/TwoFactor.ar.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Units/Units.ar.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/UserDefinedFields/UserDefinedFields.ar.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Voice/Voice.ar.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Workflows/Workflows.ar.resxis excluded by!**/*.resxCore/Resgrid.Localization/Common.ar.resxis excluded by!**/*.resx
📒 Files selected for processing (10)
Core/Resgrid.Localization/SupportedLocales.csWeb/Resgrid.Web/Areas/User/Controllers/IndoorMapsController.csWeb/Resgrid.Web/Areas/User/Views/CustomMaps/Index.cshtmlWeb/Resgrid.Web/Areas/User/Views/IndoorMaps/Floors.cshtmlWeb/Resgrid.Web/Areas/User/Views/Mapping/EditLayer.cshtmlWeb/Resgrid.Web/Areas/User/Views/Shared/_TopNavbar.cshtmlWeb/Resgrid.Web/Views/Account/LogOn.cshtmlWeb/Resgrid.Web/wwwroot/js/app/internal/dispatch/resgrid.dispatch.editcall.jsWeb/Resgrid.Web/wwwroot/js/app/internal/dispatch/resgrid.dispatch.newcall.jsWeb/Resgrid.Web/wwwroot/js/app/internal/resgrid.user.js
🚧 Files skipped from review as they are similar to previous changes (1)
- Web/Resgrid.Web/Areas/User/Views/CustomMaps/Index.cshtml
Comment on lines
+27
to
+34
| var zones = await _indoorMapService.SearchZonesAsync(DepartmentId, term); | ||
|
|
||
| var results = zones.Select(z => new | ||
| { | ||
| id = z.IndoorMapZoneId, | ||
| text = z.Name, | ||
| floorId = z.IndoorMapFloorId | ||
| }); |
Contributor
There was a problem hiding this comment.
Guard zones before Select to avoid a possible null-reference.
If SearchZonesAsync returns null, Line 29 will throw and return a 500 from this endpoint.
🛠️ Proposed fix
var zones = await _indoorMapService.SearchZonesAsync(DepartmentId, term);
+if (zones == null)
+ return Json(new { results = System.Array.Empty<object>() });
var results = zones.Select(z => new
{
id = z.IndoorMapZoneId,
text = z.Name,
floorId = z.IndoorMapFloorId
});📝 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
| var zones = await _indoorMapService.SearchZonesAsync(DepartmentId, term); | |
| var results = zones.Select(z => new | |
| { | |
| id = z.IndoorMapZoneId, | |
| text = z.Name, | |
| floorId = z.IndoorMapFloorId | |
| }); | |
| var zones = await _indoorMapService.SearchZonesAsync(DepartmentId, term); | |
| if (zones == null) | |
| return Json(new { results = System.Array.Empty<object>() }); | |
| var results = zones.Select(z => new | |
| { | |
| id = z.IndoorMapZoneId, | |
| text = z.Name, | |
| floorId = z.IndoorMapFloorId | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Web/Resgrid.Web/Areas/User/Controllers/IndoorMapsController.cs` around lines
27 - 34, The Select on zones can NRE if SearchZonesAsync returns null; in
IndoorMapsController (the action where zones is assigned from
_indoorMapService.SearchZonesAsync), guard zones before projecting by replacing
the null with an empty enumerable or returning an appropriate empty result.
Locate the variable zones and the projection into results (zones.Select(...))
and change the code to use a null-coalescing fallback (e.g., zones ??
Enumerable.Empty<YourZoneType>()) or check for null and set results to an empty
sequence so results.Select never runs on null.
Comment on lines
+78
to
+81
| <form asp-controller="IndoorMaps" asp-action="DeleteFloor" asp-route-area="User" method="post" style="display:inline;"> | ||
| <input type="hidden" name="id" value="@floor.IndoorMapFloorId" /> | ||
| <button type="submit" class="btn btn-danger btn-xs" onclick="return confirm('@localizer["DeleteFloorConfirm"]');"><i class="fa fa-trash"></i> @localizer["Delete"]</button> | ||
| </form> |
Contributor
There was a problem hiding this comment.
Missing anti-forgery token in delete form.
The delete form is missing @Html.AntiForgeryToken(), unlike the NewFloor form on line 27. If the controller action has [ValidateAntiForgeryToken], this will cause a 400 error. If not, this is a CSRF vulnerability.
Additionally, the confirm() dialog on line 80 could break if the localized string contains apostrophes (e.g., "Are you sure you'd like to delete this floor?").
🛡️ Proposed fix
<form asp-controller="IndoorMaps" asp-action="DeleteFloor" asp-route-area="User" method="post" style="display:inline;">
+ `@Html.AntiForgeryToken`()
<input type="hidden" name="id" value="@floor.IndoorMapFloorId" />
- <button type="submit" class="btn btn-danger btn-xs" onclick="return confirm('@localizer["DeleteFloorConfirm"]');"><i class="fa fa-trash"></i> `@localizer`["Delete"]</button>
+ <button type="submit" class="btn btn-danger btn-xs" onclick="return confirm('@Html.Raw(System.Text.Encodings.Web.JavaScriptEncoder.Default.Encode(localizer["DeleteFloorConfirm"].Value))');"><i class="fa fa-trash"></i> `@localizer`["Delete"]</button>
</form>Or use @Json.Serialize() for proper JavaScript escaping:
- <button type="submit" class="btn btn-danger btn-xs" onclick="return confirm('@localizer["DeleteFloorConfirm"]');"><i class="fa fa-trash"></i> `@localizer`["Delete"]</button>
+ <button type="submit" class="btn btn-danger btn-xs" onclick="return confirm(`@Json.Serialize`(localizer["DeleteFloorConfirm"].Value));"><i class="fa fa-trash"></i> `@localizer`["Delete"]</button>📝 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
| <form asp-controller="IndoorMaps" asp-action="DeleteFloor" asp-route-area="User" method="post" style="display:inline;"> | |
| <input type="hidden" name="id" value="@floor.IndoorMapFloorId" /> | |
| <button type="submit" class="btn btn-danger btn-xs" onclick="return confirm('@localizer["DeleteFloorConfirm"]');"><i class="fa fa-trash"></i> @localizer["Delete"]</button> | |
| </form> | |
| <form asp-controller="IndoorMaps" asp-action="DeleteFloor" asp-route-area="User" method="post" style="display:inline;"> | |
| `@Html.AntiForgeryToken`() | |
| <input type="hidden" name="id" value="@floor.IndoorMapFloorId" /> | |
| <button type="submit" class="btn btn-danger btn-xs" onclick="return confirm('@Html.Raw(System.Text.Encodings.Web.JavaScriptEncoder.Default.Encode(localizer["DeleteFloorConfirm"].Value))');"><i class="fa fa-trash"></i> `@localizer`["Delete"]</button> | |
| </form> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Web/Resgrid.Web/Areas/User/Views/IndoorMaps/Floors.cshtml` around lines 78 -
81, Add an anti-forgery token to the delete form used for the DeleteFloor action
and ensure the confirm dialog string is safely escaped: inside the form that
posts to IndoorMapsController.DeleteFloor (the form that posts the hidden id
floor.IndoorMapFloorId), call Html.AntiForgeryToken() so
[ValidateAntiForgeryToken] won’t 400; for the onclick confirm use a
JSON-encoded/JS-escaped string (e.g., serialize localizer["DeleteFloorConfirm"]
with Json.Serialize or output via Html.Raw(Json.Serialize(...))) or move the
confirmation to a data-confirm attribute and handle with JavaScript so
apostrophes in localizer["DeleteFloorConfirm"] won’t break the JS string.
Comment on lines
+114
to
+128
| fetch(resgrid.absoluteApiBaseUrl + '/api/v4/Geocoding/ForwardGeocode?address=' + encodeURIComponent(where), { headers: { 'Authorization': 'Bearer ' + localStorage.getItem('RgWebApp.auth-tokens') } }) | ||
| .then(function(r) { return r.json(); }) | ||
| .then(function(result) { | ||
| if (result && result.Data && result.Data.Latitude != null && result.Data.Longitude != null) { | ||
| var lat = result.Data.Latitude; | ||
| var lng = result.Data.Longitude; | ||
| map.panTo(new L.LatLng(lat, lng)); | ||
| $("#Latitude").val(lat.toString()); | ||
| $("#Longitude").val(lng.toString()); | ||
| resgrid.dispatch.newcall.setMarkerLocation(lat, lng); | ||
| } else { | ||
| console.log("Geocode returned no results for: " + where); | ||
| } | ||
| }) | ||
| .catch(function(err) { console.error("Geocode error:", err); }); |
Contributor
There was a problem hiding this comment.
Missing HTTP response status check before parsing JSON.
fetch() only rejects on network errors, not HTTP 4xx/5xx responses. If the server returns 401 (expired/invalid token) or 400, the code attempts to parse the error response as JSON and proceeds with potentially malformed data.
Proposed fix to check response status
fetch(resgrid.absoluteApiBaseUrl + '/api/v4/Geocoding/ForwardGeocode?address=' + encodeURIComponent(where), { headers: { 'Authorization': 'Bearer ' + localStorage.getItem('RgWebApp.auth-tokens') } })
- .then(function(r) { return r.json(); })
+ .then(function(r) {
+ if (!r.ok) throw new Error('HTTP ' + r.status);
+ return r.json();
+ })
.then(function(result) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Web/Resgrid.Web/wwwroot/js/app/internal/dispatch/resgrid.dispatch.newcall.js`
around lines 114 - 128, The fetch call that starts with
fetch(resgrid.absoluteApiBaseUrl + '/api/v4/Geocoding/ForwardGeocode?address=' +
...) does not check the HTTP response status before calling r.json(), so 4xx/5xx
responses (e.g. 401) will be parsed as JSON and treated as successful; update
the promise chain to first check the Response object (use response.ok or
response.status) and if not ok throw or handle a clear error (include
response.status/statusText), only call r.json() when response.ok, and in the
catch/logging include the HTTP status and message; keep existing success
handling that uses result.Data and
resgrid.dispatch.newcall.setMarkerLocation(lat, lng).
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
Web/Resgrid.Web/Areas/User/Controllers/IndoorMapsController.cs (1)
28-35:⚠️ Potential issue | 🟡 MinorGuard against null return from
SearchZonesAsync.If
SearchZonesAsyncreturnsnull, theSelecton line 30 will throw aNullReferenceException, resulting in a 500 response.🛡️ Proposed fix
var zones = await _indoorMapService.SearchZonesAsync(DepartmentId, term); + if (zones == null) + return Json(new { results = System.Array.Empty<object>() }); var results = zones.Select(z => new🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Web/Resgrid.Web/Areas/User/Controllers/IndoorMapsController.cs` around lines 28 - 35, SearchZonesAsync may return null so the LINQ Select on zones can throw; in IndoorMapsController where you call _indoorMapService.SearchZonesAsync(DepartmentId, term) check for null before projecting (e.g., treat null as an empty collection) and build results from an empty sequence when zones is null; update the code that defines zones and results to null-guard the projection (using a conditional, null-coalescing to Enumerable.Empty<...>(), or an early return) so Select is never invoked on a null reference.
🧹 Nitpick comments (1)
Web/Resgrid.Web/Areas/User/Controllers/IndoorMapsController.cs (1)
13-14: Redundant fully-qualified namespace onILogger.Line 5 already imports
Microsoft.Extensions.Logging, so the field declaration can simply useILogger<IndoorMapsController>.♻️ Proposed simplification
private readonly IIndoorMapService _indoorMapService; - private readonly Microsoft.Extensions.Logging.ILogger<IndoorMapsController> _logger; + private readonly ILogger<IndoorMapsController> _logger;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Web/Resgrid.Web/Areas/User/Controllers/IndoorMapsController.cs` around lines 13 - 14, The field declaration for the logger in IndoorMapsController uses a redundant fully-qualified name; update the private field declaration named _logger to use the imported ILogger generic (ILogger<IndoorMapsController>) instead of Microsoft.Extensions.Logging.ILogger<IndoorMapsController>; verify the class still compiles and that other references to _logger and the IIndoorMapService field remain unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@Web/Resgrid.Web/Areas/User/Controllers/IndoorMapsController.cs`:
- Around line 28-35: SearchZonesAsync may return null so the LINQ Select on
zones can throw; in IndoorMapsController where you call
_indoorMapService.SearchZonesAsync(DepartmentId, term) check for null before
projecting (e.g., treat null as an empty collection) and build results from an
empty sequence when zones is null; update the code that defines zones and
results to null-guard the projection (using a conditional, null-coalescing to
Enumerable.Empty<...>(), or an early return) so Select is never invoked on a
null reference.
---
Nitpick comments:
In `@Web/Resgrid.Web/Areas/User/Controllers/IndoorMapsController.cs`:
- Around line 13-14: The field declaration for the logger in
IndoorMapsController uses a redundant fully-qualified name; update the private
field declaration named _logger to use the imported ILogger generic
(ILogger<IndoorMapsController>) instead of
Microsoft.Extensions.Logging.ILogger<IndoorMapsController>; verify the class
still compiles and that other references to _logger and the IIndoorMapService
field remain unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e50aa1d0-4918-4560-bb5d-e11b1e9414c7
📒 Files selected for processing (2)
Web/Resgrid.Web/Areas/User/Controllers/IndoorMapsController.csWeb/Resgrid.Web/Areas/User/Views/Dispatch/Dashboard.cshtml
Member
Author
|
Approve |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
This was referenced Mar 18, 2026
Merged
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
1 participant
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
New Features
Refactor
Localization