OCPBUGS-76986: above sea level QE test fix#3450
Conversation
|
@ElayAharoni: This pull request references Jira Issue OCPBUGS-76986, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
The bug has been updated to refer to the pull request using the external bug tracker. 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. |
📝 WalkthroughWalkthroughRefactors network validation to compute the network-wide schema at runtime via Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
|
@ElayAharoni: This pull request references Jira Issue OCPBUGS-76986, which is valid. 3 validation(s) were run on this bug
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. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
libs/ui-lib/lib/ocm/components/clusterConfiguration/staticIp/components/FormViewNetworkWide/formViewNetworkWideValidationSchema.tsx (1)
159-160: Redundant conditional check.
getRendezvousIpCoherenceTestalways returns aYup.object(), so the ternary on line 160 is unnecessary—rendezvousTestwill never be falsy.♻️ Simplify by removing the redundant conditional
- const rendezvousTest = getRendezvousIpCoherenceTest(rendezvousIp); - return rendezvousTest ? baseSchema.concat(rendezvousTest) : baseSchema; + return baseSchema.concat(getRendezvousIpCoherenceTest(rendezvousIp));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/ui-lib/lib/ocm/components/clusterConfiguration/staticIp/components/FormViewNetworkWide/formViewNetworkWideValidationSchema.tsx` around lines 159 - 160, The ternary is redundant because getRendezvousIpCoherenceTest(rendezvousIp) always returns a Yup.object(); replace the conditional return with a single concat call: always compute rendezvousTest via getRendezvousIpCoherenceTest(rendezvousIp) and return baseSchema.concat(rendezvousTest). Update the return in the function that builds the schema (references: getRendezvousIpCoherenceTest, rendezvousTest, baseSchema) and remove the unnecessary null/falsy branch.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@libs/ui-lib/lib/ocm/components/clusterConfiguration/staticIp/components/FormViewNetworkWide/formViewNetworkWideValidationSchema.tsx`:
- Around line 159-160: The ternary is redundant because
getRendezvousIpCoherenceTest(rendezvousIp) always returns a Yup.object();
replace the conditional return with a single concat call: always compute
rendezvousTest via getRendezvousIpCoherenceTest(rendezvousIp) and return
baseSchema.concat(rendezvousTest). Update the return in the function that builds
the schema (references: getRendezvousIpCoherenceTest, rendezvousTest,
baseSchema) and remove the unnecessary null/falsy branch.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ac2e6971-c291-4d11-9ffa-ca06e4c418c3
📒 Files selected for processing (3)
libs/ui-lib/lib/ocm/components/clusterConfiguration/staticIp/components/FormViewNetworkWide/FormViewNetworkWide.tsxlibs/ui-lib/lib/ocm/components/clusterConfiguration/staticIp/components/FormViewNetworkWide/formViewNetworkWideValidationSchema.tsxlibs/ui-lib/lib/ocm/components/clusterWizard/disconnected/ReviewStep.tsx
...onfiguration/staticIp/components/FormViewNetworkWide/formViewNetworkWideValidationSchema.tsx
Outdated
Show resolved
Hide resolved
Signed-off-by: Elay Aharoni <elayaha@gmail.com>
65d0ba4 to
65c3f38
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@libs/ui-lib/lib/ocm/components/clusterConfiguration/staticIp/components/FormViewNetworkWide/formViewNetworkWideValidationSchema.tsx`:
- Around line 143-153: The conditional Yup schema for vlanId uses object-style
when({...}) which triggers the noThenProperty lint; change it to callback-style
when by replacing Yup.mixed().when('useVlan', { is: ..., then: () => ... }) with
Yup.mixed().when('useVlan', (useVlan, schema) => useVlan ? schema.clone()... :
schema) and inside the true branch return the same number schema
(Yup.number().required(MUST_BE_A_NUMBER).min(1, ...).max(MAX_VLAN_ID,
...).test('not-number', MUST_BE_A_NUMBER, () =>
validateNumber(values.vlanId)).nullable().transform(transformNumber) ), keeping
references to vlanId, useVlan, validateNumber, transformNumber, MUST_BE_A_NUMBER
and MAX_VLAN_ID intact so lint passes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 622f7ba6-0fd3-4223-a04a-9ea375d0dc90
📒 Files selected for processing (3)
libs/ui-lib/lib/ocm/components/clusterConfiguration/staticIp/components/FormViewNetworkWide/FormViewNetworkWide.tsxlibs/ui-lib/lib/ocm/components/clusterConfiguration/staticIp/components/FormViewNetworkWide/formViewNetworkWideValidationSchema.tsxlibs/ui-lib/lib/ocm/components/clusterWizard/disconnected/ReviewStep.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- libs/ui-lib/lib/ocm/components/clusterWizard/disconnected/ReviewStep.tsx
- libs/ui-lib/lib/ocm/components/clusterConfiguration/staticIp/components/FormViewNetworkWide/FormViewNetworkWide.tsx
...onfiguration/staticIp/components/FormViewNetworkWide/formViewNetworkWideValidationSchema.tsx
Show resolved
Hide resolved
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ElayAharoni, rawagner The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
2dd860c
into
openshift-assisted:master
|
@ElayAharoni: Jira Issue OCPBUGS-76986: All pull requests linked via external trackers have merged: Jira Issue OCPBUGS-76986 has been moved to the MODIFIED state. 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. |
Signed-off-by: Elay Aharoni <elayaha@gmail.com>
https://issues.redhat.com/browse/OCPBUGS-76986
Summary by CodeRabbit
New Features
Bug Fixes
Documentation