feat: block component deploy when the project is not deployed#677
Conversation
|
Warning Review limit reached
Next review available in: 20 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (27)
📝 WalkthroughWalkthroughAdds derived ChangesProject Deployment Status Feature
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant EnvironmentInfoService
participant ProjectReleaseBindingAPI
participant deriveProjectDeploymentStatus
participant Environment
EnvironmentInfoService->>ProjectReleaseBindingAPI: fetch project release bindings
ProjectReleaseBindingAPI-->>EnvironmentInfoService: bindings or fail-open null
EnvironmentInfoService->>deriveProjectDeploymentStatus: derive status per environment
deriveProjectDeploymentStatus-->>EnvironmentInfoService: ready, pending, or not-deployed
EnvironmentInfoService->>Environment: populate projectDeploymentStatus
sequenceDiagram
participant User
participant ProjectEnvironmentsList
participant EnvironmentDetailPanel
participant usePromotionAction
participant ProjectNotDeployedCallout
User->>ProjectEnvironmentsList: open deploy deep link
ProjectEnvironmentsList->>ProjectEnvironmentsList: select environment and show arrival hint
User->>EnvironmentDetailPanel: request promotion
EnvironmentDetailPanel->>usePromotionAction: resolve target project status
usePromotionAction-->>EnvironmentDetailPanel: disabled target and blockedReason
EnvironmentDetailPanel->>ProjectNotDeployedCallout: render deploy-project remediation
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
VajiraPrabuddhaka
requested review from
Mirage20,
kaviththiranga,
sameerajayasoma and
stefinie123
as code owners
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
plugins/openchoreo/src/components/Environments/Environments.tsx (1)
94-108: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valuePotential double refetch when returning to the tab.
Both
window.focusanddocument.visibilitychangefire when switching back to the browser tab. SinceonVisiblecallsrefetch()for both events, the function executes twice in quick succession. Additionally,window.focusfires when regaining focus from another application window (not just tab switches), which may trigger unnecessary refetches.Consider either dropping the
focuslistener (sincevisibilitychangealready covers tab returns) or guarding with a short debounce/dedup to avoid redundant network calls.♻️ Optional: rely on visibilitychange only
useEffect(() => { const onVisible = () => { if (document.visibilityState === 'visible') refetch(); }; - window.addEventListener('focus', onVisible); document.addEventListener('visibilitychange', onVisible); return () => { - window.removeEventListener('focus', onVisible); document.removeEventListener('visibilitychange', onVisible); }; }, [refetch]);If
focusis needed for cases where the tab is visible but the window loses focus to another app, a simple dedup guard works:useEffect(() => { + let lastFetch = 0; const onVisible = () => { - if (document.visibilityState === 'visible') refetch(); + if (document.visibilityState === 'visible') { + const now = Date.now(); + if (now - lastFetch > 2000) { + lastFetch = now; + refetch(); + } + } }; window.addEventListener('focus', onVisible); document.addEventListener('visibilitychange', onVisible); return () => { window.removeEventListener('focus', onVisible); document.removeEventListener('visibilitychange', onVisible); }; }, [refetch]);🤖 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 `@plugins/openchoreo/src/components/Environments/Environments.tsx` around lines 94 - 108, The refetch logic in Environments.tsx can fire twice because both the focus listener and visibilitychange listener call onVisible, so update the effect to avoid redundant refetch calls. Prefer keeping only document.visibilitychange for tab վերադարձ handling, or add a short dedup/debounce guard inside onVisible so refetch runs once per return; use the existing useEffect, onVisible, and refetch symbols to locate the change.plugins/openchoreo/src/components/ProjectEnvironments/ProjectEnvironmentsList.test.tsx (1)
137-168: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider adding a test for hint dismissal.
The tests cover preselection and hint visibility well, but the
onClosedismiss behavior (clicking the alert's close button hides it permanently) is untested. A simple test rendering withintent=deploy, asserting the hint, clicking close, and asserting it disappears would guard against regressions.🤖 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 `@plugins/openchoreo/src/components/ProjectEnvironments/ProjectEnvironmentsList.test.tsx` around lines 137 - 168, Add a test in ProjectEnvironmentsList.test.tsx to cover the hint dismissal flow for the arrival alert. Use ProjectEnvironmentsList with mockSearchParams including env and intent=deploy, assert the “return to your component tab to continue” hint is shown, click the alert’s close control, and verify the hint disappears and stays hidden. Reuse the existing deep-link setup in the ProjectEnvironmentsList / alert rendering path so the new test validates the onClose behavior.plugins/openchoreo-backend/src/services/EnvironmentService/EnvironmentInfoService.ts (1)
273-332: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winSerial project-bindings fetch adds latency to every environment-info request.
fetchProjectBindingsForStatusonly needsnamespaceName/projectName, both known upfront, yet it's awaited after thePromise.all([environmentsPromise, bindingsPromise, pipelinePromise])batch instead of inside it. This adds one full network round-trip to everyfetchDeploymentInfocall (a hot path rendering the environments/component page), even though the fetch is fail-open and independent of the other three.Folding it into the existing
Promise.allwould remove that added latency in the common case. Trade-off: it would also fire this request in the rare case wheredeploymentPipelineturns out to be missing (currently skipped via the earlyPipelineUnavailableErrorthrow) — likely an acceptable cost for the latency win. Note existing tests mock GETs in strict sequential order, so parallelizing would require adjustingEnvironmentInfoService.test.ts's mock queue order/expectations.♻️ Illustrative refactor sketch
+ const projectBindingsPromise = this.fetchProjectBindingsForStatus( + client, + request.namespaceName, + request.projectName, + ); + const fetchStart = Date.now(); - const [environmentsResult, bindingsResult, pipelineResult] = - await Promise.all([ - environmentsPromise, - bindingsPromise, - pipelinePromise, - ]); + const [ + environmentsResult, + bindingsResult, + pipelineResult, + projectBindings, + ] = await Promise.all([ + environmentsPromise, + bindingsPromise, + pipelinePromise, + projectBindingsPromise, + ]);Then drop the later standalone
await this.fetchProjectBindingsForStatus(...)call and passprojectBindingsstraight through.🤖 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 `@plugins/openchoreo-backend/src/services/EnvironmentService/EnvironmentInfoService.ts` around lines 273 - 332, The `fetchDeploymentInfo` flow in `EnvironmentInfoService` is doing `fetchProjectBindingsForStatus` as a separate await after the main `Promise.all`, which adds an unnecessary round-trip on the hot path. Move `fetchProjectBindingsForStatus(client, request.namespaceName, request.projectName)` into the existing parallel batch alongside the other promises, then use the returned `projectBindings` directly in `transformEnvironmentDataWithBindings`. Update `EnvironmentInfoService.test.ts` mocks/expectations to match the new parallel request order.
🤖 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 `@plugins/openchoreo/src/components/Environments/utils/projectDeployment.ts`:
- Around line 62-71: Update the explanatory comment in
attributeToProjectNotDeployed to match the actual fail-open behavior: it should
state that a ResourceApplyFailed plus namespace-not-found match is sufficient
even when env.projectDeploymentStatus is absent, not only when the project is
explicitly known to be not deployed or pending. Keep the implementation
unchanged and adjust the wording near the attributeToProjectNotDeployed logic so
future readers understand that undefined status is intentionally treated as
eligible for attribution.
---
Nitpick comments:
In
`@plugins/openchoreo-backend/src/services/EnvironmentService/EnvironmentInfoService.ts`:
- Around line 273-332: The `fetchDeploymentInfo` flow in
`EnvironmentInfoService` is doing `fetchProjectBindingsForStatus` as a separate
await after the main `Promise.all`, which adds an unnecessary round-trip on the
hot path. Move `fetchProjectBindingsForStatus(client, request.namespaceName,
request.projectName)` into the existing parallel batch alongside the other
promises, then use the returned `projectBindings` directly in
`transformEnvironmentDataWithBindings`. Update `EnvironmentInfoService.test.ts`
mocks/expectations to match the new parallel request order.
In `@plugins/openchoreo/src/components/Environments/Environments.tsx`:
- Around line 94-108: The refetch logic in Environments.tsx can fire twice
because both the focus listener and visibilitychange listener call onVisible, so
update the effect to avoid redundant refetch calls. Prefer keeping only
document.visibilitychange for tab վերադարձ handling, or add a short
dedup/debounce guard inside onVisible so refetch runs once per return; use the
existing useEffect, onVisible, and refetch symbols to locate the change.
In
`@plugins/openchoreo/src/components/ProjectEnvironments/ProjectEnvironmentsList.test.tsx`:
- Around line 137-168: Add a test in ProjectEnvironmentsList.test.tsx to cover
the hint dismissal flow for the arrival alert. Use ProjectEnvironmentsList with
mockSearchParams including env and intent=deploy, assert the “return to your
component tab to continue” hint is shown, click the alert’s close control, and
verify the hint disappears and stays hidden. Reuse the existing deep-link setup
in the ProjectEnvironmentsList / alert rendering path so the new test validates
the onClose behavior.
🪄 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: 63964b20-88bb-41cf-8ff9-2ae4d3f95f21
📒 Files selected for processing (27)
.changeset/component-deploy-project-prereq.mdplugins/openchoreo-backend/src/services/EnvironmentService/EnvironmentInfoService.test.tsplugins/openchoreo-backend/src/services/EnvironmentService/EnvironmentInfoService.tsplugins/openchoreo-backend/src/services/transformers/project-release-binding.test.tsplugins/openchoreo-backend/src/services/transformers/project-release-binding.tsplugins/openchoreo-backend/src/types.tsplugins/openchoreo/src/components/Environments/Environments.tsxplugins/openchoreo/src/components/Environments/PipelineDAG/DeployFlowCanvas.tsxplugins/openchoreo/src/components/Environments/components/DeploymentErrorDetailsDialog.test.tsxplugins/openchoreo/src/components/Environments/components/DeploymentErrorDetailsDialog.tsxplugins/openchoreo/src/components/Environments/components/DeploymentFailureBanner.test.tsxplugins/openchoreo/src/components/Environments/components/DeploymentFailureBanner.tsxplugins/openchoreo/src/components/Environments/components/EnvironmentDetailPanel.test.tsxplugins/openchoreo/src/components/Environments/components/EnvironmentDetailPanel.tsxplugins/openchoreo/src/components/Environments/components/MiniEnvironmentNode.tsxplugins/openchoreo/src/components/Environments/components/ProjectNotDeployedCallout.test.tsxplugins/openchoreo/src/components/Environments/components/ProjectNotDeployedCallout.tsxplugins/openchoreo/src/components/Environments/components/PromotePrimaryAction.tsxplugins/openchoreo/src/components/Environments/components/SetupDetailPane.test.tsxplugins/openchoreo/src/components/Environments/components/SetupDetailPane.tsxplugins/openchoreo/src/components/Environments/hooks/useEnvironmentData.tsplugins/openchoreo/src/components/Environments/hooks/usePromotionAction.test.tsplugins/openchoreo/src/components/Environments/hooks/usePromotionAction.tsplugins/openchoreo/src/components/Environments/utils/projectDeployment.test.tsplugins/openchoreo/src/components/Environments/utils/projectDeployment.tsplugins/openchoreo/src/components/ProjectEnvironments/ProjectEnvironmentsList.test.tsxplugins/openchoreo/src/components/ProjectEnvironments/ProjectEnvironmentsList.tsx
VajiraPrabuddhaka
force-pushed
the
feat/component-deploy-project-prereq
branch
2 times, most recently
from
9de70c4 to
dd671d6
Compare
Signed-off-by: VajiraPrabuddhaka <vajiraprabuddhaka@gmail.com>
VajiraPrabuddhaka
force-pushed
the
feat/component-deploy-project-prereq
branch
from
dd671d6 to
fe1119a
Compare
Signed-off-by: VajiraPrabuddhaka <vajiraprabuddhaka@gmail.com>
VajiraPrabuddhaka
force-pushed
the
feat/component-deploy-project-prereq
branch
from
fe1119a to
db3792c
Compare
3 participants
Purpose
Since project creation has an Auto Deploy option, a project can exist without being deployed to any environment. Deploying a component into such an environment used to fail asynchronously with a raw error (
namespaces "dp-..." not found) that didn't tell the user the cause or the fix. This blocks the action up front and guides the user to deploy the project first.Approach
projectDeploymentStatusper env (ready/pending/not-deployed), derived from the project's ProjectReleaseBindingNamespaceReadycondition. Fetched fail-open so a transient error never blocks a deploy.pendingshows an info line instead of blocking.?env=&intent=deploy), which pre-selects that env and shows a hint; returning to the component tab refetches and unblocks.Related Issues
Quick Demo
Without auto-deploy for component
Screen.Recording.2026-07-10.at.10.44.36.mov
With auto-deploy enabled for component
Screen.Recording.2026-07-10.at.10.50.29.mov
Checklist
Remarks
Ships after the Auto Deploy option (#668). The controller reporting a first-class
ProjectNotDeployedreason is a follow-up; the UI works today on a heuristic.Summary by CodeRabbit