[DevPortal] Fixed failing test cases#2752
Conversation
📝 WalkthroughWalkthroughThe integration test harness now provisions host-mounted TLS certificates, supports templated server and database settings, serializes REST API tests, and corrects Cypress API routing. View deletion now blocks views with API workflows and transactionally removes dependent records. ChangesIntegration test harness
View deletion safety
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
portals/developer-portal/it/Makefile (1)
19-20: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove
allfrom.PHONYor define it.The
alltarget is declared in the.PHONYlist but is not defined in the Makefile. Runningmake allwill result in a "No rule to make target" error. Consider removing it from.PHONYor defining it (e.g.,all: test test-postgres).🧹 Proposed fix
-.PHONY: all test test-postgres test-rest-api test-rest-api-postgres open clean deps ensure-test-tag ensure-certs +.PHONY: test test-postgres test-rest-api test-rest-api-postgres open clean deps ensure-test-tag ensure-certs🤖 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 `@portals/developer-portal/it/Makefile` around lines 19 - 20, Remove the undefined all target from the .PHONY declaration in the Makefile, or define an all target that aggregates the intended test targets; keep the remaining phony targets unchanged.Source: Linters/SAST tools
🤖 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 `@portals/developer-portal/src/dao/viewDao.js`:
- Around line 80-107: Update the caller of viewDao.delete in
apiMetadataService.js so it passes the surrounding sequelize transaction as the
third argument, matching deleteView(orgId, handle, t). Make the transaction
parameter required in the DAO if deleteView is intended to be transaction-only,
and ensure ViewLabels.destroy, OrgContent.destroy, and View.destroy all use that
same transaction.
---
Nitpick comments:
In `@portals/developer-portal/it/Makefile`:
- Around line 19-20: Remove the undefined all target from the .PHONY declaration
in the Makefile, or define an all target that aggregates the intended test
targets; keep the remaining phony targets unchanged.
🪄 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: 18c8ae79-2e3b-4fa6-93ee-b100f28fd98f
📒 Files selected for processing (11)
portals/developer-portal/.dockerignoreportals/developer-portal/it/.gitignoreportals/developer-portal/it/Makefileportals/developer-portal/it/docker-compose.test.postgres.yamlportals/developer-portal/it/docker-compose.test.yamlportals/developer-portal/it/rest-api/jest.config.jsportals/developer-portal/it/test-config.tomlportals/developer-portal/it/ui/cypress/support/e2e.jsportals/developer-portal/src/dao/viewDao.jsportals/developer-portal/src/services/apiMetadataService.jsportals/developer-portal/src/utils/constants.js
Purpose
$subject
Security checks