NO-ISSUE: Return self-support when date is before all lifecycle phases#16699
NO-ISSUE: Return self-support when date is before all lifecycle phases#16699joelanford wants to merge 1 commit into
Conversation
|
@joelanford: This pull request explicitly references no jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: openshift/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Walkthrough
ChangesSupport phase fallback
Estimated code review effort: 1 (Trivial) | ~3 minutes 🚥 Pre-merge checks | ✅ 14 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (14 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
frontend/packages/operator-lifecycle-manager/src/components/operator-lifecycle-status.tsx (1)
141-146: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRedundant conditional now that both branches return the same result.
With the fallback changed to
SelfSupport, theif (now > parseLocalEndOfDay(lastPhase.endDate))branch (line 142-144) and the final fallback (line 146) both return the identical{ status: SupportPhaseStatus.SelfSupport, allPhases }. The condition can be removed for clarity.♻️ Proposed simplification
- const lastPhase = allPhases[allPhases.length - 1]; - if (now > parseLocalEndOfDay(lastPhase.endDate)) { - return { status: SupportPhaseStatus.SelfSupport, allPhases }; - } - return { status: SupportPhaseStatus.SelfSupport, allPhases };🤖 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 `@frontend/packages/operator-lifecycle-manager/src/components/operator-lifecycle-status.tsx` around lines 141 - 146, The fallback logic in operator-lifecycle-status.tsx is redundant because the `if` branch and the final return in the status calculation both return the same `SelfSupport` result. Remove the unnecessary `now > parseLocalEndOfDay(lastPhase.endDate)` conditional in the function that builds `{ status, allPhases }`, and keep a single fallback return so the `SupportPhaseStatus` flow is clearer and easier to maintain.
🤖 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.
Nitpick comments:
In
`@frontend/packages/operator-lifecycle-manager/src/components/operator-lifecycle-status.tsx`:
- Around line 141-146: The fallback logic in operator-lifecycle-status.tsx is
redundant because the `if` branch and the final return in the status calculation
both return the same `SelfSupport` result. Remove the unnecessary `now >
parseLocalEndOfDay(lastPhase.endDate)` conditional in the function that builds
`{ status, allPhases }`, and keep a single fallback return so the
`SupportPhaseStatus` flow is clearer and easier to maintain.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 105764c1-9b79-487c-b075-eb3454d2eec2
📒 Files selected for processing (2)
frontend/packages/operator-lifecycle-manager/src/components/__tests__/operator-lifecycle-status.spec.tsxfrontend/packages/operator-lifecycle-manager/src/components/operator-lifecycle-status.tsx
a1fb1ab to
c83d7c6
Compare
|
/test backend |
|
/lgtm |
|
Docs Approver: PX Approver: |
|
/label px-approved |
jseseCCS
left a comment
There was a problem hiding this comment.
one lil comment! conditionally approving...
|
/label docs-approved |
|
/hold to address docs review comments |
getSupportPhase was returning Active with the first phase when the current date preceded all phases. If you're outside the supported window — whether before or after — the status should be the same: self-support. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
c83d7c6 to
7238147
Compare
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: joelanford, jseseCCS, perdasilva The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/retest |
|
@joelanford: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Analysis / Root cause:
getSupportPhasewas returningActivewith the first phase when the current date was before all lifecycle phases had started. This was misleading — being outside the supported window (whether before or after all phases) should be treated the same way.Solution description:
Changed the fallback in
getSupportPhase(inoperator-lifecycle-status.tsx) to returnSelfSupportinstead ofActivewhen the current date precedes all phases. Updated the corresponding test case inoperator-lifecycle-status.spec.tsx.Screenshots / screen recording:
N/A — logic-only change. The UI already renders
SelfSupportas a "Self-support" badge; this change ensures the badge appears when the date is before all phases (previously it incorrectly showed the first phase name as if it were active).Test setup:
No special setup required.
Test cases:
Browser conformance:
Additional info:
N/A
Generated with Claude Code
Summary by CodeRabbit