feat(operations): add read-only cases API foundation#386
Open
Mix13131 wants to merge 1 commit into
Open
Conversation
📝 WalkthroughWalkthroughThree PostgreSQL migration files establish ChangesOperational Cases Feature
Sequence Diagram(s)sequenceDiagram
participant Client as Frontend Client
participant opsService as operationsApiService
participant Express as casesApiRouter (Express)
participant Controller as CasesController
participant DB as PostgreSQL
Client->>opsService: getCases({ case_type, status, search })
opsService->>opsService: build URLSearchParams, skip 'all'/unset
opsService->>Express: GET /cases?case_type=&status=
Express->>Controller: safeControllerFunction → get(req, res)
Controller->>Controller: whitelist sortField, build WHERE clauses
Controller->>DB: parameterized SELECT with JOINs + jsonb_build_object
DB-->>Controller: rows { cases[], total }
Controller-->>Express: res.status(200) ServerResponse
Express-->>opsService: IServerResponse<IOperationalCasesResponse>
opsService-->>Client: response.data
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
|
|
Author
|
Implementation note for reviewers: This PR intentionally includes only the read-only foundation for operational cases. Out of scope:
Those should be reviewed in separate PRs. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@worklenz-backend/database/migrations/20260615000000-operational-cases.sql`:
- Around line 96-97: The foreign key constraints at lines 96-97 for
counterparty_id and asset_id only reference the id column in their respective
tables, creating a tenant isolation vulnerability where a case can reference
entities from another team. To fix this, modify the add_constraint calls for
both operational_cases_counterparty_id_fk and operational_cases_asset_id_fk to
include team_id in the foreign key constraint, ensuring that references are
scoped to the same team by matching both team_id and id columns in the
referenced tables (counterparties and assets).
In `@worklenz-backend/src/controllers/cases-controller.ts`:
- Around line 105-106: The ORDER BY clause at lines 105-106 sorts only by the
orderBy variable, which causes non-deterministic ordering when multiple rows
share the same sort value. This can result in duplicate or missing records
across paginated results due to shifting page boundaries. Add a deterministic
tie-breaker by appending a unique identifier column (typically id) as a
secondary sort criterion to the ORDER BY clause, ensuring consistent ordering
regardless of the primary sort field's values.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 86603257-585d-4b66-85b3-0f341004a1bf
📒 Files selected for processing (8)
worklenz-backend/database/migrations/20260615000000-operational-cases.sqlworklenz-backend/database/migrations/20260616000000-operational-case-next-action.sqlworklenz-backend/database/migrations/20260619000000-operational-order-number.sqlworklenz-backend/src/controllers/cases-controller.tsworklenz-backend/src/routes/apis/cases-api-router.tsworklenz-backend/src/routes/apis/index.tsworklenz-frontend/src/api/operations/operations.api.service.tsworklenz-frontend/src/types/operations/operations.types.ts
Comment on lines
+96
to
+97
| CALL pg_temp.add_constraint('operational_cases', 'operational_cases_counterparty_id_fk', 'FOREIGN KEY (counterparty_id) REFERENCES counterparties ON DELETE SET NULL'); | ||
| CALL pg_temp.add_constraint('operational_cases', 'operational_cases_asset_id_fk', 'FOREIGN KEY (asset_id) REFERENCES assets ON DELETE SET NULL'); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Enforce tenant-safe foreign keys for related entities.
At Line 96-97, counterparty_id and asset_id only reference id, so a case can point to another team’s records. The read query later joins on both id and team_id (worklenz-backend/src/controllers/cases-controller.ts Line 101-103), which hides the mismatch as nulls instead of preventing bad writes.
Suggested schema fix
-- counterparties / assets: add composite uniqueness to support composite FK
+CREATE UNIQUE INDEX IF NOT EXISTS counterparties_id_team_uindex
+ ON counterparties (id, team_id);
+
+CREATE UNIQUE INDEX IF NOT EXISTS assets_id_team_uindex
+ ON assets (id, team_id);
-- operational_cases: replace single-column FKs with tenant-safe composite FKs
-CALL pg_temp.add_constraint('operational_cases', 'operational_cases_counterparty_id_fk', 'FOREIGN KEY (counterparty_id) REFERENCES counterparties ON DELETE SET NULL');
-CALL pg_temp.add_constraint('operational_cases', 'operational_cases_asset_id_fk', 'FOREIGN KEY (asset_id) REFERENCES assets ON DELETE SET NULL');
+CALL pg_temp.add_constraint(
+ 'operational_cases',
+ 'operational_cases_counterparty_team_fk',
+ 'FOREIGN KEY (counterparty_id, team_id) REFERENCES counterparties(id, team_id) ON DELETE SET NULL'
+);
+CALL pg_temp.add_constraint(
+ 'operational_cases',
+ 'operational_cases_asset_team_fk',
+ 'FOREIGN KEY (asset_id, team_id) REFERENCES assets(id, team_id) ON DELETE SET NULL'
+);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@worklenz-backend/database/migrations/20260615000000-operational-cases.sql`
around lines 96 - 97, The foreign key constraints at lines 96-97 for
counterparty_id and asset_id only reference the id column in their respective
tables, creating a tenant isolation vulnerability where a case can reference
entities from another team. To fix this, modify the add_constraint calls for
both operational_cases_counterparty_id_fk and operational_cases_asset_id_fk to
include team_id in the foreign key constraint, ensuring that references are
scoped to the same team by matching both team_id and id columns in the
referenced tables (counterparties and assets).
Comment on lines
+105
to
+106
| ORDER BY ${orderBy} ${sortOrder} | ||
| LIMIT $2 OFFSET $3 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Add a deterministic tie-breaker to paginated sorting.
At Line 105-106, offset pagination sorts only by the selected field. When many rows share that value, page boundaries can shift and return duplicates/missing records.
Suggested fix
- ORDER BY ${orderBy} ${sortOrder}
+ ORDER BY ${orderBy} ${sortOrder}, oc.id ${sortOrder}
LIMIT $2 OFFSET $3🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@worklenz-backend/src/controllers/cases-controller.ts` around lines 105 - 106,
The ORDER BY clause at lines 105-106 sorts only by the orderBy variable, which
causes non-deterministic ordering when multiple rows share the same sort value.
This can result in duplicate or missing records across paginated results due to
shifting page boundaries. Add a deterministic tie-breaker by appending a unique
identifier column (typically id) as a secondary sort criterion to the ORDER BY
clause, ensuring consistent ordering regardless of the primary sort field's
values.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a minimal read-only Operations Cases foundation.
What changed
GET /cases.operationsApiService.getCases().Safety
team_idfrom authenticated user context.QA
git diff --check: pass./api/v1/cases: returns 401 and no data.Known notes
message: "Internal Server Error"with 401 from the shared error handler. This is existing auth/error-response debt, not specific to/cases.WHERE oc.team_id = req.user.team_id.Summary by CodeRabbit