MGMT-20893: Remove Remediation operators#3014
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@danmanor: This pull request references MGMT-20893 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.20.0" version, but no target version was set. 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. |
danmanor
requested review from
ammont82 and
rawagner
and removed request for
ammont82
openshift-ci
bot
added
the
size/M
WalkthroughThis update removes the "Remediation" category and its related operators from the operator specifications, including all references, constants, and descriptions. Additionally, the operator service now filters supported operators to return only those recognized in the current operator specifications, ensuring consistency between API results and defined operators. Changes
Sequence Diagram(s)sequenceDiagram
participant UI
participant OperatorsService
participant API
UI->>OperatorsService: getSupportedOperators()
OperatorsService->>API: fetchOperators()
API-->>OperatorsService: operatorsList
OperatorsService->>OperatorsService: filter operators by getOperatorSpecs()
OperatorsService-->>UI: filteredOperatorsList
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
libs/ui-lib/lib/ocm/services/OperatorsService.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the config "@openshift-assisted/eslint-config" to extend from. Please check that the name of the config is correct. The config "@openshift-assisted/eslint-config" was referenced from the config file in "/libs/ui-lib/.eslintrc.cjs". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. ✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@danmanor: This pull request references MGMT-20893 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.20.0" version, but no target version was set. 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. |
danmanor
force-pushed
the
remove-remediation-operators
branch
from
d43765d to
6310ce3
Compare
|
@danmanor: This pull request references MGMT-20893 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.20.0" version, but no target version was set. 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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
libs/ui-lib/lib/ocm/components/clusterWizard/OperatorsSelect.tsx (1)
66-69: LGTM! Consider memoizing for performance optimization.The logic correctly counts only the operators that are actually displayed, which fixes the count accuracy after removing the Remediation category from operator specs. The filtering logic is consistent with the rendering logic on line 88.
Consider memoizing this computation to avoid recalculating on every render:
- const totalDisplayedOperators = Object.values(byCategory).reduce((total, specs) => { - const displayedCount = specs.filter(spec => operators.includes(spec.operatorKey)).length; - return total + displayedCount; - }, 0); + const totalDisplayedOperators = React.useMemo(() => { + return Object.values(byCategory).reduce((total, specs) => { + const displayedCount = specs.filter(spec => operators.includes(spec.operatorKey)).length; + return total + displayedCount; + }, 0); + }, [byCategory, operators]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
libs/ui-lib/lib/common/components/operators/operatorSpecs.tsx(0 hunks)libs/ui-lib/lib/ocm/components/clusterWizard/OperatorsSelect.tsx(2 hunks)
💤 Files with no reviewable changes (1)
- libs/ui-lib/lib/common/components/operators/operatorSpecs.tsx
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: circular-deps
- GitHub Check: format
- GitHub Check: translation-files
- GitHub Check: tests
- GitHub Check: lint
- GitHub Check: unit-tests
🔇 Additional comments (1)
libs/ui-lib/lib/ocm/components/clusterWizard/OperatorsSelect.tsx (1)
81-81: Count accuracy improvement aligns with PR objectives.The use of
totalDisplayedOperatorsinstead ofoperators.lengthensures the toggle text shows the correct count of displayed operators, which is essential after removing the Remediation operators from the specs.
danmanor
force-pushed
the
remove-remediation-operators
branch
from
6310ce3 to
51b7a5e
Compare
|
@danmanor: This pull request references MGMT-20893 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.20.0" version, but no target version was set. 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. |
libs/ui-lib/lib/ocm/components/clusterWizard/OperatorsSelect.tsx
Outdated
Show resolved
Hide resolved
danmanor
force-pushed
the
remove-remediation-operators
branch
from
51b7a5e to
c80ced4
Compare
|
@danmanor: This pull request references MGMT-20893 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.20.0" version, but no target version was set. 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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
libs/ui-lib/lib/ocm/services/OperatorsService.tsx (1)
16-22: Filtering logic successfully implements the PR requirement.The modification to
getSupportedOperatorseffectively filters the API response to only include operators that are defined in the current specifications. This ensures consistency between the API results and the defined operators, which aligns perfectly with the goal of hiding Remediation operators without deleting their code.Consider caching the result of
getAllKnownOperatorKeys()if this method is called frequently, as the operator specifications are unlikely to change during runtime.+const knownOperatorKeysCache = new Set<string>(); +let cacheInitialized = false; const getAllKnownOperatorKeys = (): Set<string> => { + if (cacheInitialized) { + return knownOperatorKeysCache; + } + const allSpecsByCategory = getOperatorSpecs(() => undefined); const allKeys = Object.values(allSpecsByCategory) .flat() .map((spec) => spec.operatorKey); - return new Set(allKeys); + knownOperatorKeysCache.clear(); + allKeys.forEach(key => knownOperatorKeysCache.add(key)); + cacheInitialized = true; + + return knownOperatorKeysCache; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
libs/ui-lib/lib/common/components/operators/operatorSpecs.tsx(0 hunks)libs/ui-lib/lib/ocm/services/OperatorsService.tsx(1 hunks)
💤 Files with no reviewable changes (1)
- libs/ui-lib/lib/common/components/operators/operatorSpecs.tsx
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: tests
- GitHub Check: translation-files
- GitHub Check: format
- GitHub Check: unit-tests
- GitHub Check: lint
- GitHub Check: circular-deps
🔇 Additional comments (2)
libs/ui-lib/lib/ocm/services/OperatorsService.tsx (2)
2-2: Import addition looks good.The import of
getOperatorSpecsis correctly added to support the new filtering functionality.
4-12: Helper function implementation is solid.The
getAllKnownOperatorKeysfunction correctly extracts all operator keys from the specifications and returns them as a Set for efficient lookup. The implementation properly handles the flattening of nested operator categories.
…ight add them back soon. We just don't want them to be displayed
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: celdrake, danmanor 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 |
openshift-ci
bot
added
the
approved
|
@celdrake I can merge, is it OK with you ? |
openshift-merge-bot
bot
merged commit 0fecffc
into
openshift-assisted:master
…ight add them back soon. We just don't want them to be displayed (openshift-assisted#3014)
3 participants
We keep their code as we might add them back soon. We just don't want them to be displayed
Summary by CodeRabbit