Demo pr ia cursor#176
Conversation
There was a problem hiding this comment.
Code Review Summary
This PR introduces a Cursor CLI code-review step into the CI workflow and modifies RegionRoutes.cs. The workflow additions look structurally sound, but RegionRoutes.cs contains several critical bugs — hardcoded IDs and broken error-handling logic — that would break the API in production. These must be fixed before merging.
Issues found: 3 🚨 Critical, 2
| return Results.BadRequest(); | ||
| } | ||
|
|
||
| id = 1; //testing with item 1 |
There was a problem hiding this comment.
🚨 Critical: This hardcodes the region ID to 1, completely ignoring the id parameter from the request. Every call to this endpoint will return Region 1 regardless of what the caller asks for. Remove this line.
| return Results.NotFound(); | ||
| return | ||
|
|
||
| Results.NotFound(); |
There was a problem hiding this comment.
✨ Improvement: Splitting return Results.NotFound() across three lines with a blank line in the middle hurts readability for no benefit. Keep it as a single-line return statement.
| var region = await db.Regions.Include(p => p.Departments) | ||
| .SingleOrDefaultAsync(p => p.Id == id); | ||
| .SingleOrDefaultAsync(p => p.Id == 0); | ||
|
|
There was a problem hiding this comment.
🚨 Critical: The query is hardcoded to p.Id == 0, so this will always return null and the endpoint will always 404 — the actual id parameter is completely ignored. This should be p.Id == id.
| { | ||
| return Results.BadRequest(RequestMessages.BadRequest); | ||
| if (region is not null) | ||
| { |
There was a problem hiding this comment.
Results.BadRequest are unnecessary and introduce a silent failure path. If region were null (it can't be — it was checked above) or db were null, the invalid sort would be silently ignored and the request would proceed. Revert to the original direct return Results.BadRequest(...).
| - name: Build with dotnet | ||
| run: dotnet build --configuration Release --no-restore | ||
|
|
||
| - name: Checkout repository |
There was a problem hiding this comment.
actions/checkout that overwrites the working tree after dotnet build. The subsequent dotnet test --no-build relies on existing build artifacts, which this checkout may invalidate. Move this checkout + code-review step either before the build or into a separate job.
No description provided.