Conversation
|
Thanks for opening this, but we'd appreciate a little more information. Could you update it with more details? |
📝 WalkthroughWalkthroughAdds a call video feeds feature: new CallVideoFeed entity and enums, DB migrations for CallVideoFeeds, repository and service methods, v4 API controller and models for CRUD, UI view/tab to display feeds, DI registrations, SQL queries/config, and unit tests. Changes
Sequence DiagramsequenceDiagram
participant Client as Client
participant Controller as CallVideoFeedsController
participant Service as CallsService
participant Repo as CallVideoFeedRepository
participant DB as Database
Client->>Controller: POST /api/v4/CallVideoFeeds (SaveCallVideoFeedInput)
Controller->>Service: SaveCallVideoFeedAsync(feed)
Service->>Repo: SaveOrUpdateAsync(feed)
Repo->>DB: INSERT/UPDATE CallVideoFeeds
DB-->>Repo: Success
Repo-->>Service: saved feed
Service-->>Controller: saved feed
Controller-->>Client: 201 Created {Id}
Client->>Controller: GET /api/v4/CallVideoFeeds?callId=X
Controller->>Service: GetCallVideoFeedsByCallIdAsync(X)
Service->>Repo: GetByCallIdAsync(X)
Repo->>DB: SELECT FROM CallVideoFeeds WHERE CallId=@CallId AND IsDeleted=0
DB-->>Repo: feeds
Repo-->>Service: feeds
Service-->>Controller: feeds
Controller->>Controller: filter deleted, resolve names
Controller-->>Client: 200 OK {Data: [feeds]}
Client->>Controller: PUT /api/v4/CallVideoFeeds (EditCallVideoFeedInput)
Controller->>Service: SaveCallVideoFeedAsync(updated feed)
Service->>Repo: SaveOrUpdateAsync(feed)
Repo->>DB: UPDATE CallVideoFeeds
DB-->>Repo: Success
Repo-->>Service: updated feed
Service-->>Controller: updated feed
Controller-->>Client: 200 OK
Client->>Controller: DELETE /api/v4/CallVideoFeeds/{id}
Controller->>Service: DeleteCallVideoFeedAsync(feed, userId)
Service->>Service: set IsDeleted, DeletedByUserId, DeletedOn
Service->>Repo: SaveOrUpdateAsync(feed)
Repo->>DB: UPDATE (soft delete)
DB-->>Repo: Success
Repo-->>Service: true
Service-->>Controller: true
Controller-->>Client: 200 OK
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| [ProducesResponseType(StatusCodes.Status201Created)] | ||
| [ProducesResponseType(StatusCodes.Status400BadRequest)] | ||
| [Authorize(Policy = ResgridResources.Call_View)] | ||
| public async Task<ActionResult<SaveCallVideoFeedResult>> SaveCallVideoFeed(SaveCallVideoFeedInput input, CancellationToken cancellationToken) |
Check failure
Code scanning / CodeQL
Missing cross-site request forgery token validation High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 1 day ago
In general, to fix missing CSRF validation on an ASP.NET Core controller action that handles POST requests, you add an antiforgery validation attribute so the framework validates a token sent by the client (typically in a header or form field) on each request. In ASP.NET Core MVC, the standard way is to decorate the action (or controller) with [ValidateAntiForgeryToken] or use [AutoValidateAntiforgeryToken] globally. This ensures that every POST, PUT, DELETE, etc. from a browser includes a valid antiforgery token, mitigating CSRF.
For this specific method, the least invasive and clearest fix is to add the [ValidateAntiForgeryToken] attribute directly above SaveCallVideoFeed, alongside the existing HTTP method and authorization attributes. This does not change any business logic, return types, or routing; it only adds a framework-level check that will reject requests missing or having an invalid antiforgery token. All other functionality, including model validation, authorization policy checks, entity creation, and persistence, remains unchanged. No new imports are required because Microsoft.AspNetCore.Mvc is already imported, and that namespace provides ValidateAntiForgeryTokenAttribute.
Concretely: in Web/Resgrid.Web.Services/Controllers/v4/CallVideoFeedsController.cs, locate the SaveCallVideoFeed method and, between the existing [Authorize(Policy = ResgridResources.Call_Create)] attribute and the method signature, insert the [ValidateAntiForgeryToken] attribute. No other edits are necessary in the shown code.
| @@ -99,6 +99,7 @@ | ||
| [ProducesResponseType(StatusCodes.Status201Created)] | ||
| [ProducesResponseType(StatusCodes.Status400BadRequest)] | ||
| [Authorize(Policy = ResgridResources.Call_Create)] | ||
| [ValidateAntiForgeryToken] | ||
| public async Task<ActionResult<SaveCallVideoFeedResult>> SaveCallVideoFeed(SaveCallVideoFeedInput input, CancellationToken cancellationToken) | ||
| { | ||
| if (!ModelState.IsValid) |
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (1)
Core/Resgrid.Model/Services/ICallsService.cs (1)
422-422: Replace the boolean-flag matrix with an options object.This contract is up to ten switches now, and existing callers already pass them positionally. Another addition makes it very easy to request the wrong related data by accident. A small
PopulateCallDataOptionstype would make call sites self-describing and future additions safer.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Core/Resgrid.Model/Services/ICallsService.cs` at line 422, The PopulateCallData method on ICallsService currently accepts a long positional boolean-flag matrix which is error-prone; replace it with a single options object (e.g., PopulateCallDataOptions) that exposes named bool properties like GetDispatches, GetAttachments, GetNotes, GetGroupDispatches, GetUnitDispatches, GetRoleDispatches, GetProtocols, GetReferences, GetContacts, GetVideoFeeds (with sensible defaults). Change the interface signature from Task<Call> PopulateCallData(Call call, bool..., bool) to Task<Call> PopulateCallData(Call call, PopulateCallDataOptions options) and update all callers to construct and pass a PopulateCallDataOptions instance with named properties instead of positional booleans; ensure any implementations of PopulateCallData read the new options object and preserve previous behavior when properties are omitted (defaults).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Core/Resgrid.Services/CallsService.cs`:
- Around line 590-597: When populating call.VideoFeeds from the repository (the
await _callVideoFeedRepository.GetByCallIdAsync(call.CallId) path) ensure the
returned CallVideoFeed list is ordered by its SortOrder before assigning to
call.VideoFeeds (and likewise in the other occurrence around the 1014–1019
block); change the assignment so that if items != null you set call.VideoFeeds =
items.OrderBy(f => f.SortOrder).ToList() (and when creating an empty collection
keep it as new List<CallVideoFeed>()), ensuring CallVideoFeed.SortOrder is the
stable sort key used in both places.
In
`@Providers/Resgrid.Providers.MigrationsPg/Migrations/M0061_AddingCallVideoFeedsPg.cs`:
- Around line 11-31: The migrations use the PostgreSQL citext type (seen in
M0061_AddingCallVideoFeedsPg.cs columns like callvideofeedid, name,
addedbyuserid, deletedbyuserid, flaggedbyuserid) but no migration creates the
citext extension; add a new initial migration (e.g.,
M0000_CreateCitextExtension) that executes CREATE EXTENSION IF NOT EXISTS
"citext" as its first step (or ensure the DB runs that SQL before migrations
start) so migrations such as M0001_InitialMigration.sql and
M0061_AddingCallVideoFeedsPg.cs won’t fail with "type 'citext' does not exist".
In
`@Repositories/Resgrid.Repositories.DataRepository/Servers/PostgreSql/PostgreSqlConfiguration.cs`:
- Around line 1117-1119: The PostgreSQL query templates for CallVideoFeeds
return soft-deleted rows; update SelectCallVideoFeedsByCallIdQuery and
SelectCallVideoFeedsByDepartmentIdQuery (related to CallVideoFeedsTable) to
filter out deleted rows by adding a condition that IsDeleted = false to the
WHERE clause (matching the SQL Server templates' active-feed behavior) so only
non-deleted feeds are returned.
In
`@Repositories/Resgrid.Repositories.DataRepository/Servers/SqlServer/SqlServerConfiguration.cs`:
- Around line 1081-1083: The SELECT templates for CallVideoFeeds currently
return soft-deleted rows; update SelectCallVideoFeedsByCallIdQuery and
SelectCallVideoFeedsByDepartmentIdQuery to filter out soft-deleted records by
adding an IsDeleted check (e.g. append "AND [IsDeleted] = 0" to the existing
WHERE clauses) while keeping the existing placeholders and CallVideoFeedsTable
unchanged so queries like SelectCallVideoFeedsByCallIdQuery and
SelectCallVideoFeedsByDepartmentIdQuery exclude deleted feeds by default.
In `@Tests/Resgrid.Tests/Services/CallVideoFeedTests.cs`:
- Around line 121-136: The test
GetCallVideoFeedsByCallIdAsync_ShouldNotReturnDeletedFeeds has a mismatched
assertion: it sets up _callVideoFeedRepo to return two feeds (one IsDeleted =
true) and then asserts result.Should().HaveCount(2) while the name implies
deleted feeds should be excluded; either rename the test to reflect the current
service contract (e.g.,
GetCallVideoFeedsByCallIdAsync_ReturnsAllFeedsIncludingDeleted) or change the
assertion to expect only non-deleted feeds and update the setup/verification
accordingly (filtering logic would be exercised against
_service.GetCallVideoFeedsByCallIdAsync and the CallVideoFeed.IsDeleted flag).
In `@Web/Resgrid.Web.Services/Controllers/v4/CallVideoFeedsController.cs`:
- Around line 96-100: The controller currently uses the read-only policy for
mutating endpoints; change the Authorize attribute on the mutating actions in
CallVideoFeedsController (e.g., the SaveCallVideoFeed POST, the
UpdateCallVideoFeed PUT, and the DeleteCallVideoFeed DELETE methods) to the call
mutating policy instead of ResgridResources.Call_View — replace the policy value
with the registered mutating policy for calls (for example
ResgridResources.Call_Edit or the project’s Call_Modify policy) on each of those
methods so view-only callers cannot perform create/update/delete operations.
- Line 54: Validate and reject invalid numeric input instead of using Parse: in
GetCallVideoFeeds validate the incoming callId string once using int.TryParse
(prefer the overload with System.Globalization.NumberStyles.Integer and
System.Globalization.CultureInfo.InvariantCulture) and return BadRequest() on
failure before calling _callsService.GetCallByIdAsync; in SaveCallVideoFeed
validate input.CallId with int.TryParse before any use and return BadRequest()
on failure; for all decimal parsing in SaveCallVideoFeed and EditCallVideoFeed
(the latitude/longitude parses) replace decimal.Parse with decimal.TryParse
using System.Globalization.NumberStyles.Number and
System.Globalization.CultureInfo.InvariantCulture and return BadRequest() when
parsing fails so malformed coordinates produce a 400 instead of an exception.
In `@Web/Resgrid.Web.Services/Models/v4/CallVideoFeeds/EditCallVideoFeedInput.cs`:
- Around line 10-31: Change the loose string/nullable properties on
EditCallVideoFeedInput to enforce the persisted schema: make CallId an int (not
string) with [Required] and [Range(1, int.MaxValue)], constrain Name and Url
with [Required] and appropriate [StringLength(max)] limits, add
[StringLength(max)] to Description, Latitude and Longitude (or use a regex for
coordinate format if required), and add validation for FeedType/FeedFormat
(e.g., non-nullable enums or [Range] if they are numeric) and for Status and
SortOrder (use [Required] and [Range] to match allowed values). Update the
EditCallVideoFeedInput class property types/attributes (CallId, Name, Url,
Description, Latitude, Longitude, FeedType, FeedFormat, Status, SortOrder) so
model binding rejects invalid formats/over-length values at the API boundary.
In `@Web/Resgrid.Web.Services/Models/v4/CallVideoFeeds/SaveCallVideoFeedInput.cs`:
- Around line 7-26: In SaveCallVideoFeedInput, tighten model validation to
reflect storage constraints: change CallId from string to Guid (or Guid?) and
keep [Required] so invalid GUIDs fail model binding; add appropriate
[MaxLength]/[StringLength] annotations to Name, Url, and Description to match DB
column lengths; add [Url] (or a regex) to Url and consider adding range/regex
validation for Latitude and Longitude or convert them to decimal?double to match
storage; ensure SortOrder remains int and mark nullable only if DB allows nulls.
Reference properties: SaveCallVideoFeedInput.CallId, .Name, .Url, .Description,
.Latitude, .Longitude, .SortOrder.
In `@Web/Resgrid.Web/Areas/User/Models/Calls/ViewCallView.cs`:
- Line 30: The VideoFeeds property on the ViewCallView model can be null and
cause runtime errors when views enumerate it; update the ViewCallView class so
VideoFeeds is initialized defensively (e.g., assign a new List<CallVideoFeed>()
as the property default or initialize it inside the ViewCallView constructor) so
the property is never null at render time.
In `@Web/Resgrid.Web/Areas/User/Views/Dispatch/ViewCall.cshtml`:
- Line 764: The timestamp rendering uses feed.AddedOn.ToString("g"), which
ignores the department timezone/localization; replace that call with the
department-aware formatting helper used elsewhere on this view (i.e. use the
same method/extension that converts datetimes to the department local time and
formats them consistently) so that feed.AddedOn is converted to the department
timezone/localization before being rendered.
- Line 733: The anchor renders feed.Url with target="_blank" without rel or
scheme checks; update the view so the href uses only validated/normalized URLs
(accept only http or https via server-side Uri.TryCreate/validation for feed.Url
or a sanitized property) and add rel="noopener noreferrer" to the <a> element to
prevent reverse-tabnabbing; if the URL is unsafe or has a non-http(s) scheme,
render it as plain text or replace with a safe placeholder instead of linking.
---
Nitpick comments:
In `@Core/Resgrid.Model/Services/ICallsService.cs`:
- Line 422: The PopulateCallData method on ICallsService currently accepts a
long positional boolean-flag matrix which is error-prone; replace it with a
single options object (e.g., PopulateCallDataOptions) that exposes named bool
properties like GetDispatches, GetAttachments, GetNotes, GetGroupDispatches,
GetUnitDispatches, GetRoleDispatches, GetProtocols, GetReferences, GetContacts,
GetVideoFeeds (with sensible defaults). Change the interface signature from
Task<Call> PopulateCallData(Call call, bool..., bool) to Task<Call>
PopulateCallData(Call call, PopulateCallDataOptions options) and update all
callers to construct and pass a PopulateCallDataOptions instance with named
properties instead of positional booleans; ensure any implementations of
PopulateCallData read the new options object and preserve previous behavior when
properties are omitted (defaults).
🪄 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: dc0f19bd-3ce8-4897-b84d-ee13afa6b80b
⛔ Files ignored due to path filters (9)
Core/Resgrid.Localization/Areas/User/Dispatch/Call.ar.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Dispatch/Call.de.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Dispatch/Call.en.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Dispatch/Call.es.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Dispatch/Call.fr.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Dispatch/Call.it.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Dispatch/Call.pl.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Dispatch/Call.sv.resxis excluded by!**/*.resxCore/Resgrid.Localization/Areas/User/Dispatch/Call.uk.resxis excluded by!**/*.resx
📒 Files selected for processing (31)
Core/Resgrid.Model/Call.csCore/Resgrid.Model/CallVideoFeed.csCore/Resgrid.Model/CallVideoFeedFormats.csCore/Resgrid.Model/CallVideoFeedStatuses.csCore/Resgrid.Model/CallVideoFeedTypes.csCore/Resgrid.Model/Repositories/ICallVideoFeedRepository.csCore/Resgrid.Model/Services/ICallsService.csCore/Resgrid.Services/CallsService.csProviders/Resgrid.Providers.Migrations/Migrations/M0061_AddingCallVideoFeeds.csProviders/Resgrid.Providers.MigrationsPg/Migrations/M0061_AddingCallVideoFeedsPg.csRepositories/Resgrid.Repositories.DataRepository/CallVideoFeedRepository.csRepositories/Resgrid.Repositories.DataRepository/Configs/SqlConfiguration.csRepositories/Resgrid.Repositories.DataRepository/Modules/ApiDataModule.csRepositories/Resgrid.Repositories.DataRepository/Modules/DataModule.csRepositories/Resgrid.Repositories.DataRepository/Modules/NonWebDataModule.csRepositories/Resgrid.Repositories.DataRepository/Modules/TestingDataModule.csRepositories/Resgrid.Repositories.DataRepository/Queries/CallVideoFeeds/SelectCallVideoFeedsByCallIdQuery.csRepositories/Resgrid.Repositories.DataRepository/Queries/CallVideoFeeds/SelectCallVideoFeedsByDepartmentIdQuery.csRepositories/Resgrid.Repositories.DataRepository/Servers/PostgreSql/PostgreSqlConfiguration.csRepositories/Resgrid.Repositories.DataRepository/Servers/SqlServer/SqlServerConfiguration.csTests/Resgrid.Tests/Services/CallVideoFeedTests.csWeb/Resgrid.Web.Services/Controllers/v4/CallVideoFeedsController.csWeb/Resgrid.Web.Services/Models/v4/CallVideoFeeds/CallVideoFeedsResult.csWeb/Resgrid.Web.Services/Models/v4/CallVideoFeeds/DeleteCallVideoFeedResult.csWeb/Resgrid.Web.Services/Models/v4/CallVideoFeeds/EditCallVideoFeedInput.csWeb/Resgrid.Web.Services/Models/v4/CallVideoFeeds/SaveCallVideoFeedInput.csWeb/Resgrid.Web.Services/Models/v4/CallVideoFeeds/SaveCallVideoFeedResult.csWeb/Resgrid.Web.Services/Resgrid.Web.Services.xmlWeb/Resgrid.Web/Areas/User/Controllers/DispatchController.csWeb/Resgrid.Web/Areas/User/Models/Calls/ViewCallView.csWeb/Resgrid.Web/Areas/User/Views/Dispatch/ViewCall.cshtml
| .WithColumn("callvideofeedid").AsCustom("citext").NotNullable().PrimaryKey() | ||
| .WithColumn("callid").AsInt32().NotNullable() | ||
| .WithColumn("departmentid").AsInt32().NotNullable() | ||
| .WithColumn("name").AsCustom("citext").NotNullable() | ||
| .WithColumn("url").AsCustom("text").NotNullable() | ||
| .WithColumn("feedtype").AsInt32().Nullable() | ||
| .WithColumn("feedformat").AsInt32().Nullable() | ||
| .WithColumn("description").AsCustom("text").Nullable() | ||
| .WithColumn("status").AsInt32().NotNullable().WithDefaultValue(0) | ||
| .WithColumn("latitude").AsDecimal(10, 7).Nullable() | ||
| .WithColumn("longitude").AsDecimal(10, 7).Nullable() | ||
| .WithColumn("addedbyuserid").AsCustom("citext").NotNullable() | ||
| .WithColumn("addedon").AsDateTime().NotNullable() | ||
| .WithColumn("updatedon").AsDateTime().Nullable() | ||
| .WithColumn("sortorder").AsInt32().NotNullable().WithDefaultValue(0) | ||
| .WithColumn("isdeleted").AsBoolean().NotNullable().WithDefaultValue(false) | ||
| .WithColumn("deletedbyuserid").AsCustom("citext").Nullable() | ||
| .WithColumn("deletedon").AsDateTime().Nullable() | ||
| .WithColumn("isflagged").AsBoolean().NotNullable().WithDefaultValue(false) | ||
| .WithColumn("flaggedreason").AsCustom("text").Nullable() | ||
| .WithColumn("flaggedbyuserid").AsCustom("citext").Nullable() |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Look for any migration/bootstrap step enabling citext.
rg -n --type=cs "create extension.*citext|citext.*extension|AsCustom\\(\"citext\"\\)" Providers
# Narrow to Postgres migrations for explicit extension setup.
rg -n --type=cs "create extension.*citext|Execute\\.Sql\\(\"CREATE EXTENSION.*citext" Providers/Resgrid.Providers.MigrationsPgRepository: Resgrid/Core
Length of output: 50368
🏁 Script executed:
# Search for CREATE EXTENSION in all file types, not just C#
find Providers/Resgrid.Providers.MigrationsPg -type f -exec grep -l "CREATE EXTENSION" {} \;
# Also search for bootstrap/initialization SQL files
find . -type f \( -name "*.sql" -o -name "*bootstrap*" -o -name "*init*" \) | head -20
# Check if there's a setup or database initialization in the migration runner
rg -i "create extension|citext" --type=sql --type=txt --type=md Providers/Resgrid.Providers.MigrationsPgRepository: Resgrid/Core
Length of output: 40583
🏁 Script executed:
# Check the beginning of M0001_InitialMigration.sql for CREATE EXTENSION
head -50 Providers/Resgrid.Providers.MigrationsPg/Sql/M0001_InitialMigration.sql
# Also search for CREATE EXTENSION specifically in that file
grep -n "CREATE EXTENSION" Providers/Resgrid.Providers.MigrationsPg/Sql/M0001_InitialMigration.sqlRepository: Resgrid/Core
Length of output: 3080
🏁 Script executed:
# Check file encoding
file Providers/Resgrid.Providers.MigrationsPg/Sql/M0001_InitialMigration.sql
# Try with iconv to convert if needed
iconv -f UTF-16 -t UTF-8 Providers/Resgrid.Providers.MigrationsPg/Sql/M0001_InitialMigration.sql 2>/dev/null | head -100 | grep -i "CREATE EXTENSION" || echo "Not found in first 100 lines"
# Also check the C# migration classes for explicit extension creation
rg -A 5 "Execute\.Sql.*CREATE EXTENSION" Providers/Resgrid.Providers.MigrationsPg/Migrations/Repository: Resgrid/Core
Length of output: 144
🏁 Script executed:
# Check the Docker database initialization files
head -50 Docker/db/create-databases.sql
# Check the initialization shell scripts
cat Docker/db/initdbs.sh
# Check for CREATE EXTENSION in any SQL file type
find Providers/Resgrid.Providers.MigrationsPg -type f -name "*.sql" -exec grep -l "CREATE EXTENSION" {} \;
find Docker -type f -name "*.sql" -exec grep -l "CREATE EXTENSION" {} \;Repository: Resgrid/Core
Length of output: 546
🏁 Script executed:
# Check M0001 migration C# code for extension creation
cat Providers/Resgrid.Providers.MigrationsPg/Migrations/M0001_InitialMigration.cs | head -80
# Search for Execute.Sql that might create extension in all M0001 files
rg -i "execute|extension" Providers/Resgrid.Providers.MigrationsPg/Migrations/M0001*.cs -A 2 | head -100Repository: Resgrid/Core
Length of output: 279
🏁 Script executed:
# Find the actual M0001 migration file
find Providers/Resgrid.Providers.MigrationsPg/Migrations -name "*M0001*" -type f
# Read the first migration C# file
find Providers/Resgrid.Providers.MigrationsPg/Migrations -name "*001*" -type f -exec cat {} \;
# Check if citext extension is enabled in PostgreSQL Docker setup
find Docker -type f \( -name "*.sh" -o -name "*.sql" -o -name "Dockerfile*" \) -exec grep -l "citext\|CREATE EXTENSION" {} \;Repository: Resgrid/Core
Length of output: 11076
🏁 Script executed:
# The M0001 migration uses an embedded SQL script. Let's check if citext extension is created in that script
# Try different encoding approaches
python3 << 'EOF'
import os
file_path = "Providers/Resgrid.Providers.MigrationsPg/Sql/M0001_InitialMigration.sql"
try:
with open(file_path, 'rb') as f:
content = f.read()
# Try different decodings
for encoding in ['utf-16', 'utf-16-le', 'utf-8', 'latin1']:
try:
text = content.decode(encoding)
if 'CREATE EXTENSION' in text.upper():
print(f"Found CREATE EXTENSION in {encoding} encoding")
# Print first occurrence
idx = text.upper().find('CREATE EXTENSION')
print(text[max(0, idx-50):min(len(text), idx+200)])
break
except:
continue
else:
print("CREATE EXTENSION not found in any encoding")
except Exception as e:
print(f"Error: {e}")
EOF
# Also check what PostgreSQL initialization files exist
find . -type f -path "*/postgres*" -o -path "*/pg*" | grep -i "init\|docker\|sql" | head -20Repository: Resgrid/Core
Length of output: 98
The citext extension prerequisite must be created before any PostgreSQL migrations execute.
The database extension citext is used extensively throughout the migration chain starting with M0001_InitialMigration.sql, yet no migration explicitly creates it. If the extension is not pre-enabled on the target PostgreSQL database, migration execution will fail with a "type "citext" does not exist" error.
Ensure one of the following:
- The PostgreSQL database has
CREATE EXTENSION IF NOT EXISTS "citext"executed before running migrations, or - Add an explicit migration (M0000 or equivalent) that creates the extension as its first step
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@Providers/Resgrid.Providers.MigrationsPg/Migrations/M0061_AddingCallVideoFeedsPg.cs`
around lines 11 - 31, The migrations use the PostgreSQL citext type (seen in
M0061_AddingCallVideoFeedsPg.cs columns like callvideofeedid, name,
addedbyuserid, deletedbyuserid, flaggedbyuserid) but no migration creates the
citext extension; add a new initial migration (e.g.,
M0000_CreateCitextExtension) that executes CREATE EXTENSION IF NOT EXISTS
"citext" as its first step (or ensure the DB runs that SQL before migrations
start) so migrations such as M0001_InitialMigration.sql and
M0061_AddingCallVideoFeedsPg.cs won’t fail with "type 'citext' does not exist".
Repositories/Resgrid.Repositories.DataRepository/Servers/PostgreSql/PostgreSqlConfiguration.cs
Outdated
Show resolved
Hide resolved
Repositories/Resgrid.Repositories.DataRepository/Servers/SqlServer/SqlServerConfiguration.cs
Outdated
Show resolved
Hide resolved
| [Test] | ||
| public async Task GetCallVideoFeedsByCallIdAsync_ShouldNotReturnDeletedFeeds() | ||
| { | ||
| var feeds = new List<CallVideoFeed> | ||
| { | ||
| new CallVideoFeed { CallVideoFeedId = "feed1", CallId = 1, Name = "Feed 1", Url = "http://example.com/1", IsDeleted = false }, | ||
| new CallVideoFeed { CallVideoFeedId = "feed2", CallId = 1, Name = "Feed 2", Url = "http://example.com/2", IsDeleted = true } | ||
| }; | ||
|
|
||
| _callVideoFeedRepo.Setup(x => x.GetByCallIdAsync(1)).ReturnsAsync(feeds); | ||
|
|
||
| var result = await _service.GetCallVideoFeedsByCallIdAsync(1); | ||
|
|
||
| // The service returns all feeds from repo; filtering is done at the API layer | ||
| result.Should().HaveCount(2); | ||
| } |
There was a problem hiding this comment.
Align the test name with the asserted contract.
The name says deleted feeds should be excluded, but the body asserts both rows are returned and even documents caller-side filtering. As written, the test communicates the opposite of what it checks. Rename it to reflect the current service contract, or change the assertion if deleted feeds are supposed to be hidden here.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Tests/Resgrid.Tests/Services/CallVideoFeedTests.cs` around lines 121 - 136,
The test GetCallVideoFeedsByCallIdAsync_ShouldNotReturnDeletedFeeds has a
mismatched assertion: it sets up _callVideoFeedRepo to return two feeds (one
IsDeleted = true) and then asserts result.Should().HaveCount(2) while the name
implies deleted feeds should be excluded; either rename the test to reflect the
current service contract (e.g.,
GetCallVideoFeedsByCallIdAsync_ReturnsAllFeedsIncludingDeleted) or change the
assertion to expect only non-deleted feeds and update the setup/verification
accordingly (filtering logic would be exercised against
_service.GetCallVideoFeedsByCallIdAsync and the CallVideoFeed.IsDeleted flag).
| [Required] | ||
| public string CallId { get; set; } | ||
|
|
||
| [Required] | ||
| public string Name { get; set; } | ||
|
|
||
| [Required] | ||
| public string Url { get; set; } | ||
|
|
||
| public int? FeedType { get; set; } | ||
|
|
||
| public int? FeedFormat { get; set; } | ||
|
|
||
| public string Description { get; set; } | ||
|
|
||
| public int Status { get; set; } | ||
|
|
||
| public string Latitude { get; set; } | ||
|
|
||
| public string Longitude { get; set; } | ||
|
|
||
| public int SortOrder { get; set; } |
There was a problem hiding this comment.
Align edit input contract with persisted schema constraints.
As written, model binding accepts values that can fail later (non-numeric CallId, over-length text fields). Tightening this at the API boundary will prevent downstream errors.
Suggested fix
[Required]
- public string CallId { get; set; }
+ public int CallId { get; set; }
[Required]
+ [StringLength(500)]
public string Name { get; set; }
[Required]
+ [StringLength(2000)]
public string Url { get; set; }
+ [StringLength(4000)]
public string Description { get; set; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Web/Resgrid.Web.Services/Models/v4/CallVideoFeeds/EditCallVideoFeedInput.cs`
around lines 10 - 31, Change the loose string/nullable properties on
EditCallVideoFeedInput to enforce the persisted schema: make CallId an int (not
string) with [Required] and [Range(1, int.MaxValue)], constrain Name and Url
with [Required] and appropriate [StringLength(max)] limits, add
[StringLength(max)] to Description, Latitude and Longitude (or use a regex for
coordinate format if required), and add validation for FeedType/FeedFormat
(e.g., non-nullable enums or [Range] if they are numeric) and for Status and
SortOrder (use [Required] and [Range] to match allowed values). Update the
EditCallVideoFeedInput class property types/attributes (CallId, Name, Url,
Description, Latitude, Longitude, FeedType, FeedFormat, Status, SortOrder) so
model binding rejects invalid formats/over-length values at the API boundary.
| [Required] | ||
| public string CallId { get; set; } | ||
|
|
||
| [Required] | ||
| public string Name { get; set; } | ||
|
|
||
| [Required] | ||
| public string Url { get; set; } | ||
|
|
||
| public int? FeedType { get; set; } | ||
|
|
||
| public int? FeedFormat { get; set; } | ||
|
|
||
| public string Description { get; set; } | ||
|
|
||
| public string Latitude { get; set; } | ||
|
|
||
| public string Longitude { get; set; } | ||
|
|
||
| public int SortOrder { get; set; } |
There was a problem hiding this comment.
Harden API input validation to match storage constraints.
CallId as string and missing length caps for Name/Url/Description allow invalid payloads through model binding, pushing failures downstream.
Suggested fix
public class SaveCallVideoFeedInput
{
[Required]
- public string CallId { get; set; }
+ public int CallId { get; set; }
[Required]
+ [StringLength(500)]
public string Name { get; set; }
[Required]
+ [StringLength(2000)]
public string Url { get; set; }
public int? FeedType { get; set; }
public int? FeedFormat { get; set; }
+ [StringLength(4000)]
public string Description { get; set; }📝 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.
| [Required] | |
| public string CallId { get; set; } | |
| [Required] | |
| public string Name { get; set; } | |
| [Required] | |
| public string Url { get; set; } | |
| public int? FeedType { get; set; } | |
| public int? FeedFormat { get; set; } | |
| public string Description { get; set; } | |
| public string Latitude { get; set; } | |
| public string Longitude { get; set; } | |
| public int SortOrder { get; set; } | |
| [Required] | |
| public int CallId { get; set; } | |
| [Required] | |
| [StringLength(500)] | |
| public string Name { get; set; } | |
| [Required] | |
| [StringLength(2000)] | |
| public string Url { get; set; } | |
| public int? FeedType { get; set; } | |
| public int? FeedFormat { get; set; } | |
| [StringLength(4000)] | |
| public string Description { get; set; } | |
| public string Latitude { get; set; } | |
| public string Longitude { get; set; } | |
| public int SortOrder { get; set; } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Web/Resgrid.Web.Services/Models/v4/CallVideoFeeds/SaveCallVideoFeedInput.cs`
around lines 7 - 26, In SaveCallVideoFeedInput, tighten model validation to
reflect storage constraints: change CallId from string to Guid (or Guid?) and
keep [Required] so invalid GUIDs fail model binding; add appropriate
[MaxLength]/[StringLength] annotations to Name, Url, and Description to match DB
column lengths; add [Url] (or a regex) to Url and consider adding range/regex
validation for Latitude and Longitude or convert them to decimal?double to match
storage; ensure SortOrder remains int and mark nullable only if DB allows nulls.
Reference properties: SaveCallVideoFeedInput.CallId, .Name, .Url, .Description,
.Latitude, .Longitude, .SortOrder.
| { | ||
| <tr> | ||
| <td>@feed.Name</td> | ||
| <td><a href="@feed.Url" target="_blank">@feed.Url</a></td> |
There was a problem hiding this comment.
Harden video feed links against unsafe schemes and tabnabbing.
Line 733 renders feed.Url directly in href with target="_blank" and no rel safeguards. If URL values are user-provided, this allows unsafe schemes and reverse-tabnabbing.
🔐 Proposed hardening
- <td><a href="@feed.Url" target="_blank">@feed.Url</a></td>
+ @{
+ var isSafeUrl = Uri.TryCreate(feed.Url, UriKind.Absolute, out var parsedFeedUrl) &&
+ (parsedFeedUrl.Scheme == Uri.UriSchemeHttp || parsedFeedUrl.Scheme == Uri.UriSchemeHttps);
+ }
+ <td>
+ `@if` (isSafeUrl)
+ {
+ <a href="@feed.Url" target="_blank" rel="noopener noreferrer">@feed.Url</a>
+ }
+ else
+ {
+ <span>@feed.Url</span>
+ }
+ </td>📝 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.
| <td><a href="@feed.Url" target="_blank">@feed.Url</a></td> | |
| @{ | |
| var isSafeUrl = Uri.TryCreate(feed.Url, UriKind.Absolute, out var parsedFeedUrl) && | |
| (parsedFeedUrl.Scheme == Uri.UriSchemeHttp || parsedFeedUrl.Scheme == Uri.UriSchemeHttps); | |
| } | |
| <td> | |
| `@if` (isSafeUrl) | |
| { | |
| <a href="@feed.Url" target="_blank" rel="noopener noreferrer">@feed.Url</a> | |
| } | |
| else | |
| { | |
| <span>@feed.Url</span> | |
| } | |
| </td> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Web/Resgrid.Web/Areas/User/Views/Dispatch/ViewCall.cshtml` at line 733, The
anchor renders feed.Url with target="_blank" without rel or scheme checks;
update the view so the href uses only validated/normalized URLs (accept only
http or https via server-side Uri.TryCreate/validation for feed.Url or a
sanitized property) and add rel="noopener noreferrer" to the <a> element to
prevent reverse-tabnabbing; if the URL is unsafe or has a non-http(s) scheme,
render it as plain text or replace with a safe placeholder instead of linking.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Web/Resgrid.Web.Services/Controllers/v4/CallVideoFeedsController.cs (1)
243-263: Consider converting to an extension method.Per coding guidelines, extension methods are appropriate for domain-specific operations. This mapper could be an extension method on
CallVideoFeed:♻️ Suggested refactor
-public static CallVideoFeedResultData ConvertCallVideoFeed(CallVideoFeed feed, string fullName, Department department) +public static CallVideoFeedResultData ToResultData(this CallVideoFeed feed, string fullName, Department department)Usage would then read more fluently:
result.Data.Add(feed.ToResultData(fullName, department));As per coding guidelines: "Use extension methods appropriately for domain-specific operations."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Web/Resgrid.Web.Services/Controllers/v4/CallVideoFeedsController.cs` around lines 243 - 263, ConvertCallVideoFeed currently lives as a static mapper but should be an extension on CallVideoFeed for domain-specific fluency: create a static class (e.g., CallVideoFeedExtensions) and add a public static method ToResultData(this CallVideoFeed feed, string fullName, Department department) that returns CallVideoFeedResultData and contains the same mapping logic from ConvertCallVideoFeed (populate CallVideoFeedResultData properties including AddedOn formatting via feed.AddedOn.TimeConverter(department).FormatForDepartment(department)). Replace callers that invoke ConvertCallVideoFeed(...) with feed.ToResultData(fullName, department) and remove or mark the old ConvertCallVideoFeed method as obsolete.
🤖 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.Services/Controllers/v4/CallVideoFeedsController.cs`:
- Around line 69-75: The loop causes an N+1 lookup by calling
UserHelper.GetFullNameForUser for each feed; change to collect non-deleted feeds
(feeds.Where(f => !f.IsDeleted)), extract distinct AddedByUserId values, call a
batch helper like UserHelper.GetFullNamesForUsers(userIds) to retrieve a
dictionary/map of userId->fullName, then iterate nonDeletedFeeds and call
ConvertCallVideoFeed(feed, userNames[feed.AddedByUserId], department) to add to
result.Data. Ensure you add/implement the batch method on UserHelper and
preserve existing behavior when a userId is missing (e.g., fallback to empty
string).
---
Nitpick comments:
In `@Web/Resgrid.Web.Services/Controllers/v4/CallVideoFeedsController.cs`:
- Around line 243-263: ConvertCallVideoFeed currently lives as a static mapper
but should be an extension on CallVideoFeed for domain-specific fluency: create
a static class (e.g., CallVideoFeedExtensions) and add a public static method
ToResultData(this CallVideoFeed feed, string fullName, Department department)
that returns CallVideoFeedResultData and contains the same mapping logic from
ConvertCallVideoFeed (populate CallVideoFeedResultData properties including
AddedOn formatting via
feed.AddedOn.TimeConverter(department).FormatForDepartment(department)). Replace
callers that invoke ConvertCallVideoFeed(...) with feed.ToResultData(fullName,
department) and remove or mark the old ConvertCallVideoFeed method as obsolete.
🪄 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: 5629d559-3f7e-467a-bd8f-fa2dfbf6dc2a
📒 Files selected for processing (8)
Core/Resgrid.Services/CallsService.csRepositories/Resgrid.Repositories.DataRepository/Servers/PostgreSql/PostgreSqlConfiguration.csRepositories/Resgrid.Repositories.DataRepository/Servers/SqlServer/SqlServerConfiguration.csWeb/Resgrid.Web.Services/Controllers/v4/CallVideoFeedsController.csWeb/Resgrid.Web.Services/Models/v4/CallVideoFeeds/EditCallVideoFeedInput.csWeb/Resgrid.Web.Services/Models/v4/CallVideoFeeds/SaveCallVideoFeedInput.csWeb/Resgrid.Web/Areas/User/Models/Calls/ViewCallView.csWeb/Resgrid.Web/Areas/User/Views/Dispatch/ViewCall.cshtml
✅ Files skipped from review due to trivial changes (3)
- Web/Resgrid.Web/Areas/User/Views/Dispatch/ViewCall.cshtml
- Web/Resgrid.Web.Services/Models/v4/CallVideoFeeds/SaveCallVideoFeedInput.cs
- Web/Resgrid.Web.Services/Models/v4/CallVideoFeeds/EditCallVideoFeedInput.cs
🚧 Files skipped from review as they are similar to previous changes (4)
- Web/Resgrid.Web/Areas/User/Models/Calls/ViewCallView.cs
- Repositories/Resgrid.Repositories.DataRepository/Servers/PostgreSql/PostgreSqlConfiguration.cs
- Core/Resgrid.Services/CallsService.cs
- Repositories/Resgrid.Repositories.DataRepository/Servers/SqlServer/SqlServerConfiguration.cs
| if (feeds != null && feeds.Any()) | ||
| { | ||
| foreach (var feed in feeds.Where(f => !f.IsDeleted)) | ||
| { | ||
| var fullName = await UserHelper.GetFullNameForUser(feed.AddedByUserId); | ||
| result.Data.Add(ConvertCallVideoFeed(feed, fullName, department)); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify if GetCallVideoFeedsByCallIdAsync already filters IsDeleted
ast-grep --pattern 'GetCallVideoFeedsByCallIdAsync($$$)'
rg -n "IsDeleted" --type cs -C 3 -g '*CallVideoFeed*Repository*'Repository: Resgrid/Core
Length of output: 38
🏁 Script executed:
# Locate the CallVideoFeedsController.cs file
find . -name "*CallVideoFeedsController*" -type fRepository: Resgrid/Core
Length of output: 125
🏁 Script executed:
# Search for GetCallVideoFeedsByCallIdAsync method definition
rg -n "GetCallVideoFeedsByCallIdAsync" --type cs -A 5Repository: Resgrid/Core
Length of output: 4127
🏁 Script executed:
# Locate UserHelper and GetFullNameForUser method
fd -i "UserHelper" --type f
rg -n "GetFullNameForUser" --type cs -A 3 -B 1Repository: Resgrid/Core
Length of output: 21955
Refactor to batch-fetch user names and eliminate N+1 query pattern.
Each video feed in the loop triggers UserHelper.GetFullNameForUser, causing a separate database lookup per feed. Instead, collect all user IDs upfront and fetch names in a single batch operation:
Suggested refactoring
if (feeds != null && feeds.Any())
{
var nonDeletedFeeds = feeds.Where(f => !f.IsDeleted).ToList();
var userIds = nonDeletedFeeds.Select(f => f.AddedByUserId).Distinct();
var userNames = await UserHelper.GetFullNamesForUsers(userIds); // batch method
foreach (var feed in nonDeletedFeeds)
{
var fullName = userNames[feed.AddedByUserId];
result.Data.Add(ConvertCallVideoFeed(feed, fullName, department));
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Web/Resgrid.Web.Services/Controllers/v4/CallVideoFeedsController.cs` around
lines 69 - 75, The loop causes an N+1 lookup by calling
UserHelper.GetFullNameForUser for each feed; change to collect non-deleted feeds
(feeds.Where(f => !f.IsDeleted)), extract distinct AddedByUserId values, call a
batch helper like UserHelper.GetFullNamesForUsers(userIds) to retrieve a
dictionary/map of userId->fullName, then iterate nonDeletedFeeds and call
ConvertCallVideoFeed(feed, userNames[feed.AddedByUserId], department) to add to
result.Data. Ensure you add/implement the batch method on UserHelper and
preserve existing behavior when a userId is missing (e.g., fallback to empty
string).
| [Authorize(Policy = ResgridResources.Call_View)] | ||
| public async Task<ActionResult<CallVideoFeedsResult>> GetCallVideoFeeds(string callId) | ||
| { | ||
| if (String.IsNullOrWhiteSpace(callId) || !int.TryParse(callId, out var cId)) |
Check failure
Code scanning / CodeQL
User-controlled bypass of sensitive method High
Copilot Autofix
AI 1 day ago
Copilot could not generate an autofix suggestion
Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.
| [Authorize(Policy = ResgridResources.Call_View)] | ||
| public async Task<ActionResult<CallVideoFeedsResult>> GetCallVideoFeeds(string callId) | ||
| { | ||
| if (String.IsNullOrWhiteSpace(callId) || !int.TryParse(callId, out var cId)) |
Check failure
Code scanning / CodeQL
User-controlled bypass of sensitive method High
Copilot Autofix
AI 1 day ago
Copilot could not generate an autofix suggestion
Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.
| [Authorize(Policy = ResgridResources.Call_View)] | ||
| public async Task<ActionResult<CallVideoFeedsResult>> GetCallVideoFeeds(string callId) | ||
| { | ||
| if (String.IsNullOrWhiteSpace(callId) || !int.TryParse(callId, out var cId)) |
Check failure
Code scanning / CodeQL
User-controlled bypass of sensitive method High
Copilot Autofix
AI 1 day ago
Copilot could not generate an autofix suggestion
Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.
| if (!ModelState.IsValid) | ||
| return BadRequest(); | ||
|
|
||
| if (!int.TryParse(input.CallId, out var parsedCallId)) |
Check failure
Code scanning / CodeQL
User-controlled bypass of sensitive method High
Copilot Autofix
AI 1 day ago
Copilot could not generate an autofix suggestion
Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.
| if (!ModelState.IsValid) | ||
| return BadRequest(); | ||
|
|
||
| if (!int.TryParse(input.CallId, out var parsedCallId)) |
Check failure
Code scanning / CodeQL
User-controlled bypass of sensitive method High
Copilot Autofix
AI 1 day ago
Copilot could not generate an autofix suggestion
Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.
Summary by CodeRabbit
New Features
Tests