50foreman - #2103
50foreman#2103
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdds local IOP (Insights on-prem) development mode support and aligns the frontend build configuration with newer frontend-components tooling, including an IOP-specific proxy start script and route configuration updates. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThe PR adds IOP pathways navigation and detail rendering, environment-aware providers and routing, normalized conditional filters, rule-change overview refetching, pathway remediation handling, and updated container, Tekton, dependency, and local development configuration. ChangesIOP application behavior
Build and delivery updates
Estimated code review effort: 4 (Complex) | ~60 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 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 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 9 security issues, 1 other issue, and left some high level feedback:
Security issues:
- FSL-1.1-MIT: Open-source license could not be identified (link)
- FSL-1.1-MIT: Open-source license could not be identified (link)
- FSL-1.1-MIT: Open-source license could not be identified (link)
- FSL-1.1-MIT: Open-source license could not be identified (link)
- FSL-1.1-MIT: Open-source license could not be identified (link)
- FSL-1.1-MIT: Open-source license could not be identified (link)
- FSL-1.1-MIT: Open-source license could not be identified (link)
- FSL-1.1-MIT: Open-source license could not be identified (link)
- FSL-1.1-MIT: Open-source license could not be identified (link)
General comments:
- The
start:proxy:iopnpm script relies on bash-specific features like$(pwd)and inline env vars, which may not work on Windows shells; consider using a more cross-platform approach (e.g.,cross-envand avoiding command substitution) if this script is expected to run outside POSIX environments.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `start:proxy:iop` npm script relies on bash-specific features like `$(pwd)` and inline env vars, which may not work on Windows shells; consider using a more cross-platform approach (e.g., `cross-env` and avoiding command substitution) if this script is expected to run outside POSIX environments.
## Individual Comments
### Comment 1
<location path="package.json" line_range="22" />
<code_context>
"server:ctr": "node src/server/generateServerKey.js",
"start": "fec dev",
"start:proxy": "PROXY=true fec dev",
+ "start:proxy:iop": "PROXY=true IOP=true HCC_ENV=iop HCC_ENV_URL=${IOP_URL} FEC_IOP_CUSTOM_ROUTES_PATH=$(pwd)/custom_routes.json fec dev-proxy --iop",
"static": "fec static",
"test": "jest --passWithNoTests",
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Consider shell portability and quoting in the `start:proxy:iop` script.
This script uses shell-specific syntax (`$(pwd)`, `${IOP_URL}`) and unquoted paths, which will break on Windows and other non-POSIX shells. If you need cross-platform support, consider using something like `cross-env` for env vars and avoid shell substitution in the npm script itself. At minimum, quote `$(pwd)/custom_routes.json` to handle spaces in the project path.
Suggested implementation:
```
"start:proxy": "PROXY=true fec dev",
"start:proxy:iop": "PROXY=true IOP=true HCC_ENV=iop HCC_ENV_URL=${IOP_URL} FEC_IOP_CUSTOM_ROUTES_PATH=\"./custom_routes.json\" fec dev-proxy --iop",
"static": "fec static",
```
For full cross-platform support (especially Windows), consider:
1. Using `cross-env` (or similar) for setting env vars (`PROXY`, `IOP`, `HCC_ENV`, `HCC_ENV_URL`, `FEC_IOP_CUSTOM_ROUTES_PATH`) instead of inline shell syntax.
2. Avoiding `${IOP_URL}` interpolation in the script; instead, read `IOP_URL` from `process.env` in your Node code and keep the script free of shell-specific variable expansion.
These broader refactors require checking how the app currently reads these env vars and aligning with existing tooling in the repo.
</issue_to_address>
### Comment 2
<location path="package-lock.json" line_range="7675-7683" />
<code_context>
</code_context>
<issue_to_address>
**security (license/@sentry/cli):** FSL-1.1-MIT: Open-source license could not be identified
The obligations of the `FSL-1.1-MIT` license for this code could not be determined automatically. Unknown licenses may carry obligations or restrictions and should be reviewed manually to ensure compliance
*Source: trivy*
</issue_to_address>
### Comment 3
<location path="package-lock.json" line_range="7705-7712" />
<code_context>
</code_context>
<issue_to_address>
**security (license/@sentry/cli-darwin):** FSL-1.1-MIT: Open-source license could not be identified
The obligations of the `FSL-1.1-MIT` license for this code could not be determined automatically. Unknown licenses may carry obligations or restrictions and should be reviewed manually to ensure compliance
*Source: trivy*
</issue_to_address>
### Comment 4
<location path="package-lock.json" line_range="7718-7735" />
<code_context>
</code_context>
<issue_to_address>
**security (license/@sentry/cli-linux-arm):** FSL-1.1-MIT: Open-source license could not be identified
The obligations of the `FSL-1.1-MIT` license for this code could not be determined automatically. Unknown licenses may carry obligations or restrictions and should be reviewed manually to ensure compliance
*Source: trivy*
</issue_to_address>
### Comment 5
<location path="package-lock.json" line_range="7736-7753" />
<code_context>
</code_context>
<issue_to_address>
**security (license/@sentry/cli-linux-arm64):** FSL-1.1-MIT: Open-source license could not be identified
The obligations of the `FSL-1.1-MIT` license for this code could not be determined automatically. Unknown licenses may carry obligations or restrictions and should be reviewed manually to ensure compliance
*Source: trivy*
</issue_to_address>
### Comment 6
<location path="package-lock.json" line_range="7754-7772" />
<code_context>
</code_context>
<issue_to_address>
**security (license/@sentry/cli-linux-i686):** FSL-1.1-MIT: Open-source license could not be identified
The obligations of the `FSL-1.1-MIT` license for this code could not be determined automatically. Unknown licenses may carry obligations or restrictions and should be reviewed manually to ensure compliance
*Source: trivy*
</issue_to_address>
### Comment 7
<location path="package-lock.json" line_range="7773-7790" />
<code_context>
</code_context>
<issue_to_address>
**security (license/@sentry/cli-linux-x64):** FSL-1.1-MIT: Open-source license could not be identified
The obligations of the `FSL-1.1-MIT` license for this code could not be determined automatically. Unknown licenses may carry obligations or restrictions and should be reviewed manually to ensure compliance
*Source: trivy*
</issue_to_address>
### Comment 8
<location path="package-lock.json" line_range="7791-7806" />
<code_context>
</code_context>
<issue_to_address>
**security (license/@sentry/cli-win32-arm64):** FSL-1.1-MIT: Open-source license could not be identified
The obligations of the `FSL-1.1-MIT` license for this code could not be determined automatically. Unknown licenses may carry obligations or restrictions and should be reviewed manually to ensure compliance
*Source: trivy*
</issue_to_address>
### Comment 9
<location path="package-lock.json" line_range="7807-7818" />
<code_context>
</code_context>
<issue_to_address>
**security (license/@sentry/cli-win32-i686):** FSL-1.1-MIT: Open-source license could not be identified
The obligations of the `FSL-1.1-MIT` license for this code could not be determined automatically. Unknown licenses may carry obligations or restrictions and should be reviewed manually to ensure compliance
*Source: trivy*
</issue_to_address>
### Comment 10
<location path="package-lock.json" line_range="7824-7834" />
<code_context>
</code_context>
<issue_to_address>
**security (license/@sentry/cli-win32-x64):** FSL-1.1-MIT: Open-source license could not be identified
The obligations of the `FSL-1.1-MIT` license for this code could not be determined automatically. Unknown licenses may carry obligations or restrictions and should be reviewed manually to ensure compliance
*Source: trivy*
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| "server:ctr": "node src/server/generateServerKey.js", | ||
| "start": "fec dev", | ||
| "start:proxy": "PROXY=true fec dev", | ||
| "start:proxy:iop": "PROXY=true IOP=true HCC_ENV=iop HCC_ENV_URL=${IOP_URL} FEC_IOP_CUSTOM_ROUTES_PATH=$(pwd)/custom_routes.json fec dev-proxy --iop", |
There was a problem hiding this comment.
suggestion (bug_risk): Consider shell portability and quoting in the start:proxy:iop script.
This script uses shell-specific syntax ($(pwd), ${IOP_URL}) and unquoted paths, which will break on Windows and other non-POSIX shells. If you need cross-platform support, consider using something like cross-env for env vars and avoid shell substitution in the npm script itself. At minimum, quote $(pwd)/custom_routes.json to handle spaces in the project path.
Suggested implementation:
"start:proxy": "PROXY=true fec dev",
"start:proxy:iop": "PROXY=true IOP=true HCC_ENV=iop HCC_ENV_URL=${IOP_URL} FEC_IOP_CUSTOM_ROUTES_PATH=\"./custom_routes.json\" fec dev-proxy --iop",
"static": "fec static",
For full cross-platform support (especially Windows), consider:
- Using
cross-env(or similar) for setting env vars (PROXY,IOP,HCC_ENV,HCC_ENV_URL,FEC_IOP_CUSTOM_ROUTES_PATH) instead of inline shell syntax. - Avoiding
${IOP_URL}interpolation in the script; instead, readIOP_URLfromprocess.envin your Node code and keep the script free of shell-specific variable expansion.
These broader refactors require checking how the app currently reads these env vars and aligning with existing tooling in the repo.
| "node_modules/@sentry/cli": { | ||
| "version": "2.32.1", | ||
| "resolved": "https://registry.npmjs.org/@sentry/cli/-/cli-2.32.1.tgz", | ||
| "integrity": "sha512-MWkbkzZfnlE7s2pPbg4VozRSAeMlIObfZlTIou9ye6XnPt6ZmmxCLOuOgSKMv4sXg6aeqKNzMNiadThxCWyvPg==", | ||
| "version": "2.58.6", | ||
| "resolved": "https://registry.npmjs.org/@sentry/cli/-/cli-2.58.6.tgz", | ||
| "integrity": "sha512-baBcNPLLfUi9WuL+Tpri9BFaAdvugZIKelC5X0tt0Zdy+K0K+PCVSrnNmwMWU/HyaF/SEv6b6UHnXIdqanBlcg==", | ||
| "hasInstallScript": true, | ||
| "license": "FSL-1.1-MIT", | ||
| "dependencies": { | ||
| "https-proxy-agent": "^5.0.0", | ||
| "node-fetch": "^2.6.7", |
There was a problem hiding this comment.
security (license/@sentry/cli): FSL-1.1-MIT: Open-source license could not be identified
The obligations of the FSL-1.1-MIT license for this code could not be determined automatically. Unknown licenses may carry obligations or restrictions and should be reviewed manually to ensure compliance
Source: trivy
| "node_modules/@sentry/cli-darwin": { | ||
| "version": "2.32.1", | ||
| "resolved": "https://registry.npmjs.org/@sentry/cli-darwin/-/cli-darwin-2.32.1.tgz", | ||
| "integrity": "sha512-z/lEwANTYPCzbWTZ2+eeeNYxRLllC8knd0h+vtAKlhmGw/fyc/N39cznIFyFu+dLJ6tTdjOWOeikHtKuS/7onw==", | ||
| "version": "2.58.6", | ||
| "resolved": "https://registry.npmjs.org/@sentry/cli-darwin/-/cli-darwin-2.58.6.tgz", | ||
| "integrity": "sha512-udAVvcyfNa0R+95GvPz/+43/N3TC0TYKdkQ7D7jhPSzbcMc7l2fxRNN5yB3UpCA5fWFnW4toeaqwDBhb/Wh3LA==", | ||
| "license": "FSL-1.1-MIT", | ||
| "optional": true, | ||
| "os": [ | ||
| "darwin" |
There was a problem hiding this comment.
security (license/@sentry/cli-darwin): FSL-1.1-MIT: Open-source license could not be identified
The obligations of the FSL-1.1-MIT license for this code could not be determined automatically. Unknown licenses may carry obligations or restrictions and should be reviewed manually to ensure compliance
Source: trivy
| "node_modules/@sentry/cli-linux-arm": { | ||
| "version": "2.32.1", | ||
| "resolved": "https://registry.npmjs.org/@sentry/cli-linux-arm/-/cli-linux-arm-2.32.1.tgz", | ||
| "integrity": "sha512-m0lHkn+o4YKBq8KptGZvpT64FAwSl9mYvHZO9/ChnEGIJ/WyJwiN1X1r9JHVaW4iT5lD0Y5FAyq3JLkk0m0XHg==", | ||
| "version": "2.58.6", | ||
| "resolved": "https://registry.npmjs.org/@sentry/cli-linux-arm/-/cli-linux-arm-2.58.6.tgz", | ||
| "integrity": "sha512-pD0LAt5PcUzAinBwvDqc66x9+2CabHEv486yP0gRjWO7SakbaxmfVq/EXd8VLq/Tzi39LAu422UYK1lpW3MILw==", | ||
| "cpu": [ | ||
| "arm" | ||
| ], | ||
| "license": "BSD-3-Clause", | ||
| "license": "FSL-1.1-MIT", | ||
| "optional": true, | ||
| "os": [ | ||
| "linux", | ||
| "freebsd" | ||
| "freebsd", | ||
| "android" | ||
| ], | ||
| "engines": { | ||
| "node": ">=10" | ||
| } | ||
| }, |
There was a problem hiding this comment.
security (license/@sentry/cli-linux-arm): FSL-1.1-MIT: Open-source license could not be identified
The obligations of the FSL-1.1-MIT license for this code could not be determined automatically. Unknown licenses may carry obligations or restrictions and should be reviewed manually to ensure compliance
Source: trivy
| "node_modules/@sentry/cli-linux-arm64": { | ||
| "version": "2.32.1", | ||
| "resolved": "https://registry.npmjs.org/@sentry/cli-linux-arm64/-/cli-linux-arm64-2.32.1.tgz", | ||
| "integrity": "sha512-hsGqHYuecUl1Yhq4MhiRejfh1gNlmhyNPcQEoO/DDRBnGnJyEAdiDpKXJcc2e/lT9k40B55Ob2CP1SeY040T2w==", | ||
| "version": "2.58.6", | ||
| "resolved": "https://registry.npmjs.org/@sentry/cli-linux-arm64/-/cli-linux-arm64-2.58.6.tgz", | ||
| "integrity": "sha512-q8mEcNNmeXMy5i+jWT30TVpH7LcP4HD21CD5XRSPAd/a912HF6EpK0ybf/1USO14WOhoXbAGi9txwaWabSe33g==", | ||
| "cpu": [ | ||
| "arm64" | ||
| ], | ||
| "license": "BSD-3-Clause", | ||
| "license": "FSL-1.1-MIT", | ||
| "optional": true, | ||
| "os": [ | ||
| "linux", | ||
| "freebsd" | ||
| "freebsd", | ||
| "android" | ||
| ], | ||
| "engines": { | ||
| "node": ">=10" | ||
| } | ||
| }, |
There was a problem hiding this comment.
security (license/@sentry/cli-linux-arm64): FSL-1.1-MIT: Open-source license could not be identified
The obligations of the FSL-1.1-MIT license for this code could not be determined automatically. Unknown licenses may carry obligations or restrictions and should be reviewed manually to ensure compliance
Source: trivy
| "node_modules/@sentry/cli-linux-i686": { | ||
| "version": "2.32.1", | ||
| "resolved": "https://registry.npmjs.org/@sentry/cli-linux-i686/-/cli-linux-i686-2.32.1.tgz", | ||
| "integrity": "sha512-SuMLN1/ceFd3Q/B0DVyh5igjetTAF423txiABAHASenEev0lG0vZkRDXFclfgDtDUKRPmOXW7VDMirM3yZWQHQ==", | ||
| "version": "2.58.6", | ||
| "resolved": "https://registry.npmjs.org/@sentry/cli-linux-i686/-/cli-linux-i686-2.58.6.tgz", | ||
| "integrity": "sha512-q8vNJi1eOV/4vxAFWBsEwLHoSYapaZHIf4j76KJGJXFKTkEbsjCOOsKbwUIBTQQhRgV4DFWh3ryfsPS/que4Kg==", | ||
| "cpu": [ | ||
| "x86", | ||
| "ia32" | ||
| ], | ||
| "license": "BSD-3-Clause", | ||
| "license": "FSL-1.1-MIT", | ||
| "optional": true, | ||
| "os": [ | ||
| "linux", | ||
| "freebsd" | ||
| "freebsd", | ||
| "android" | ||
| ], | ||
| "engines": { | ||
| "node": ">=10" | ||
| } | ||
| }, |
There was a problem hiding this comment.
security (license/@sentry/cli-linux-i686): FSL-1.1-MIT: Open-source license could not be identified
The obligations of the FSL-1.1-MIT license for this code could not be determined automatically. Unknown licenses may carry obligations or restrictions and should be reviewed manually to ensure compliance
Source: trivy
| "node_modules/@sentry/cli-linux-x64": { | ||
| "version": "2.32.1", | ||
| "resolved": "https://registry.npmjs.org/@sentry/cli-linux-x64/-/cli-linux-x64-2.32.1.tgz", | ||
| "integrity": "sha512-x4FGd6xgvFddz8V/dh6jii4wy9qjWyvYLBTz8Fhi9rIP+b8wQ3oxwHIdzntareetZP7C1ggx+hZheiYocNYVwA==", | ||
| "version": "2.58.6", | ||
| "resolved": "https://registry.npmjs.org/@sentry/cli-linux-x64/-/cli-linux-x64-2.58.6.tgz", | ||
| "integrity": "sha512-DZu956Mhi3ZRjTBe1WdbGV46ldVbA8d2rgp/fh51GsI25zjBHah4wZnPTSzpc+YqxU6pJpg579B/r3jrIK530Q==", | ||
| "cpu": [ | ||
| "x64" | ||
| ], | ||
| "license": "BSD-3-Clause", | ||
| "license": "FSL-1.1-MIT", | ||
| "optional": true, | ||
| "os": [ | ||
| "linux", | ||
| "freebsd" | ||
| "freebsd", | ||
| "android" | ||
| ], | ||
| "engines": { | ||
| "node": ">=10" | ||
| } | ||
| }, |
There was a problem hiding this comment.
security (license/@sentry/cli-linux-x64): FSL-1.1-MIT: Open-source license could not be identified
The obligations of the FSL-1.1-MIT license for this code could not be determined automatically. Unknown licenses may carry obligations or restrictions and should be reviewed manually to ensure compliance
Source: trivy
| "node_modules/@sentry/cli-win32-arm64": { | ||
| "version": "2.58.6", | ||
| "resolved": "https://registry.npmjs.org/@sentry/cli-win32-arm64/-/cli-win32-arm64-2.58.6.tgz", | ||
| "integrity": "sha512-nj0Ff/kmAB73EPDhR8B4O9r+NUHK5GkPCkGWC+kXVemqAJWL5jcJ5KdxG0l/S0z6RoEoltID8/43/B+TaMlT7A==", | ||
| "cpu": [ | ||
| "arm64" | ||
| ], | ||
| "license": "FSL-1.1-MIT", | ||
| "optional": true, | ||
| "os": [ | ||
| "win32" | ||
| ], | ||
| "engines": { | ||
| "node": ">=10" | ||
| } | ||
| }, |
There was a problem hiding this comment.
security (license/@sentry/cli-win32-arm64): FSL-1.1-MIT: Open-source license could not be identified
The obligations of the FSL-1.1-MIT license for this code could not be determined automatically. Unknown licenses may carry obligations or restrictions and should be reviewed manually to ensure compliance
Source: trivy
| "node_modules/@sentry/cli-win32-i686": { | ||
| "version": "2.32.1", | ||
| "resolved": "https://registry.npmjs.org/@sentry/cli-win32-i686/-/cli-win32-i686-2.32.1.tgz", | ||
| "integrity": "sha512-i6aZma9mFzR+hqMY5VliQZEX6ypP/zUjPK0VtIMYWs5cC6PsQLRmuoeJmy3Z7d4nlh0CdK5NPC813Ej6RY6/vg==", | ||
| "version": "2.58.6", | ||
| "resolved": "https://registry.npmjs.org/@sentry/cli-win32-i686/-/cli-win32-i686-2.58.6.tgz", | ||
| "integrity": "sha512-WNZiDzPbgsEMQWq4avsQ391v/xWKJDIWWWo9GYl+N/w5qcYKkoDW7wQG7T9FasI6ENn68phChTOAPXXxbfAdOg==", | ||
| "cpu": [ | ||
| "x86", | ||
| "ia32" | ||
| ], | ||
| "license": "BSD-3-Clause", | ||
| "license": "FSL-1.1-MIT", | ||
| "optional": true, | ||
| "os": [ | ||
| "win32" |
There was a problem hiding this comment.
security (license/@sentry/cli-win32-i686): FSL-1.1-MIT: Open-source license could not be identified
The obligations of the FSL-1.1-MIT license for this code could not be determined automatically. Unknown licenses may carry obligations or restrictions and should be reviewed manually to ensure compliance
Source: trivy
| "node_modules/@sentry/cli-win32-x64": { | ||
| "version": "2.32.1", | ||
| "resolved": "https://registry.npmjs.org/@sentry/cli-win32-x64/-/cli-win32-x64-2.32.1.tgz", | ||
| "integrity": "sha512-B58w/lRHLb4MUSjJNfMMw2cQykfimDCMLMmeK+1EiT2RmSeNQliwhhBxYcKk82a8kszH6zg3wT2vCea7LyPUyA==", | ||
| "version": "2.58.6", | ||
| "resolved": "https://registry.npmjs.org/@sentry/cli-win32-x64/-/cli-win32-x64-2.58.6.tgz", | ||
| "integrity": "sha512-R35WJ17oF4D2eqI1DR2sQQqr0fjRTt5xoP16WrTu91XM2lndRMFsnjh+/GttbxapLCBNlrjzia99MJ0PZHZpgA==", | ||
| "cpu": [ | ||
| "x64" | ||
| ], | ||
| "license": "BSD-3-Clause", | ||
| "license": "FSL-1.1-MIT", | ||
| "optional": true, | ||
| "os": [ | ||
| "win32" |
There was a problem hiding this comment.
security (license/@sentry/cli-win32-x64): FSL-1.1-MIT: Open-source license could not be identified
The obligations of the FSL-1.1-MIT license for this code could not be determined automatically. Unknown licenses may carry obligations or restrictions and should be reviewed manually to ensure compliance
Source: trivy
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/PresentationalComponents/SystemsTable/SystemsTable.js (1)
82-95: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winInherits the
normalizeFilterValuefalsy-value gap for theincidentfilter.If
filters.incidentcan ever be a scalarfalserather than an array, the checkbox will render unchecked despite an active filter. See root-cause comment onsrc/PresentationalComponents/helper.jslines 103-115.🤖 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 `@src/PresentationalComponents/SystemsTable/SystemsTable.js` around lines 82 - 95, Update the incident filter configuration in SystemsTable so filters.incident is normalized through the same false-value handling required by normalizeFilterValue before being passed to the checkbox value. Preserve the existing SFC.incident.urlParam and addFilterParam behavior, and ensure a scalar false represents an active checked filter rather than an unchecked state.src/Messages.js (1)
205-218: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRedundant phrasing in the merged disabled-rules messages.
Both messages repeat "no recommendations are disabled" back-to-back, e.g. "No recommendations are disabled, or no recommendations are disabled that match the applied filter settings." Consider tightening, e.g. "No recommendations are disabled, or none match the applied filter settings."
🤖 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 `@src/Messages.js` around lines 205 - 218, Update the defaultMessage values for rulesTableNoRuleHitsDisabledRulesBody and rulesTableNoRuleHitsRedHatDisabledRulesBody to remove the repeated “no recommendations are disabled” phrasing, using “or none match the applied filter settings” while preserving each message’s existing meaning and Red Hat-specific wording.src/SmartComponents/SystemAdvisor/SystemAdvisorAssets.js (1)
72-100: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winInherits the
normalizeFilterValuefalsy-value gap forhas_playbook.Same concern as flagged on
src/PresentationalComponents/helper.jslines 103-115: a scalarhas_playbook: falsewould render as an unchecked checkbox instead of a selected "false" filter.🤖 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 `@src/SmartComponents/SystemAdvisor/SystemAdvisorAssets.js` around lines 72 - 100, The has_playbook filter in the conditional-filter configuration must preserve scalar false values when normalizing filters. Update its value handling around normalizeFilterValue and the shared helper behavior so false becomes the selected “false” filter option rather than an empty or unchecked value, while retaining existing handling for other filter values.src/PresentationalComponents/RulesTable/helpers.js (1)
235-323: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winInherits the
normalizeFilterValuefalsy-value gap.
incident,has_playbook, andrebootare boolean-shaped filters; if any of them can carry a scalarfalse(not yet array-wrapped),normalizeFilterValuewill render the checkbox as unchecked even though the filter is active. See the root-cause comment onsrc/PresentationalComponents/helper.jslines 103-115.🤖 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 `@src/PresentationalComponents/RulesTable/helpers.js` around lines 235 - 323, Update the boolean-shaped filter handling in the filter configuration around FC.incident, FC.has_playbook, and FC.reboot so scalar false values remain represented as active selections when passed through normalizeFilterValue. Reuse the shared normalization fix identified in helper.js rather than adding filter-specific workarounds, while preserving existing array and truthy-value behavior.
🧹 Nitpick comments (6)
src/PresentationalComponents/Inventory/helpers.js (1)
34-53: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winSwallowed fetch errors are indistinguishable from an empty result set.
Returning
{ data: [], meta: { count: 0 } }makes the table render "no systems" on a backend outage, andallCurrentSystemIdswill silently resolve select-all to zero systems. Consider re-throwing (or surfacing a notification) so the caller can render an error state instead of a misleading empty table.🤖 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 `@src/PresentationalComponents/Inventory/helpers.js` around lines 34 - 53, Update the error handling around the systems fetch in the pathway/rule request to re-throw the caught error instead of returning an empty `{ data, meta }` result. Preserve the existing diagnostic logging, allowing the caller to surface an error state rather than treating backend failures as valid empty results.src/PresentationalComponents/Inventory/Inventory.js (2)
122-134: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
fetchSystemsmemo captures a stalehandleRefresh.
handleRefreshcloses overselectedTags(used byurlBuilder), butselectedTagsisn't in the dependency list, so tag changes won't be reflected in the URL built during subsequent fetches. WrappinghandleRefreshinuseCallback([pathway, selectedTags])and adding it here keeps the closure fresh. (fullFiltersis passed but never read insidegetEntities, so its omission is harmless — consider dropping the parameter.)🤖 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 `@src/PresentationalComponents/Inventory/Inventory.js` around lines 122 - 134, Update handleRefresh to use useCallback with pathway and selectedTags dependencies, then include the memoized handleRefresh in fetchSystems’ useMemo dependency list so URL generation reflects tag changes. Do not alter unrelated dependencies; optionally remove the unused fullFilters argument from getEntities and its call site if supported by the surrounding implementation.
559-593: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
chromelessExportConfigandstandardExportConfigare byte-identical.Collapse into a single
exportConfigmemo and reuse it in both branches.♻️ Proposed refactor
- const chromelessExportConfig = useMemo(() => permsExport && { + const exportConfig = useMemo(() => permsExport && { label: intl.formatMessage(messages.exportJson), onSelect: (_e, fileType) => downloadReport( exportTable, fileType, { rule_id: rule.rule_id, ...filters }, selectedTags, workloads, dispatch, envContext.BASE_URL, ), isDisabled: !permsExport || entities?.rows?.length === 0, tooltipText: permsExport ? intl.formatMessage(messages.exportData) : intl.formatMessage(messages.permsAction), }, [permsExport, filters, rule, selectedTags, workloads, entities?.rows?.length, envContext.BASE_URL]); - - const standardExportConfig = useMemo(() => permsExport && { - label: intl.formatMessage(messages.exportJson), - onSelect: (_e, fileType) => - downloadReport( - exportTable, - fileType, - { rule_id: rule.rule_id, ...filters }, - selectedTags, - workloads, - dispatch, - envContext.BASE_URL, - ), - isDisabled: !permsExport || entities?.rows?.length === 0, - tooltipText: permsExport - ? intl.formatMessage(messages.exportData) - : intl.formatMessage(messages.permsAction), - }, [permsExport, filters, rule, selectedTags, workloads, entities?.rows?.length, envContext.BASE_URL]);🤖 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 `@src/PresentationalComponents/Inventory/Inventory.js` around lines 559 - 593, Replace the duplicate chromelessExportConfig and standardExportConfig useMemo definitions with one shared exportConfig memo containing the existing export behavior and dependencies, then reuse exportConfig in both relevant branches.src/PresentationalComponents/SystemsTable/systemsFilters.test.js (1)
23-31: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate tautological assertions — doesn't test the real
normalizeFilterValue.Same pattern as
filterArraySafety.test.js: these two tests re-derive the value with an inline ternary instead of calling the importednormalizeFilterValue, so they don't actually validate the implementation.Also applies to: 59-67
🤖 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 `@src/PresentationalComponents/SystemsTable/systemsFilters.test.js` around lines 23 - 31, Update the tests in systemsFilters.test.js around the “should safely handle hits filter value” cases to call the imported normalizeFilterValue function instead of duplicating its ternary logic inline. Apply the same change to the tests around lines 59–67, preserving their expected normalized values so they validate the actual implementation.src/PresentationalComponents/RulesTable/filterArraySafety.test.js (1)
97-147: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win"CheckboxFilter Usage" tests don't exercise
normalizeFilterValue.These five tests re-implement the same ternary inline (
Array.isArray(...) ? ... : ... ? [String(...)] : []) rather than calling the importednormalizeFilterValue, so they're tautological — they'll always pass regardless of whether the real function is correct.🤖 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 `@src/PresentationalComponents/RulesTable/filterArraySafety.test.js` around lines 97 - 147, Update the tests in the “CheckboxFilter Usage” describe block to call the imported normalizeFilterValue function for each filter value instead of reimplementing its ternary logic inline. Preserve the existing test inputs and expected outputs so the cases validate string, boolean, number, undefined, and array normalization behavior.src/PresentationalComponents/PathwaysTable/PathwaysTable.js (1)
143-204: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the duplicated pathway-detail base path.
envContext?.pathwayDetailBasePath || '/insights/advisor/recommendations/pathways'is repeated verbatim at lines 169 and 193.♻️ Proposed refactor
+ const pathwayDetailBasePath = + envContext?.pathwayDetailBasePath || '/insights/advisor/recommendations/pathways'; + const rowBuilder = (pathways) => ... <Link key={key} - to={`${envContext?.pathwayDetailBasePath || '/insights/advisor/recommendations/pathways'}/${pathway.slug}`} + to={`${pathwayDetailBasePath}/${pathway.slug}`} > ... <Link key={key} - to={`${envContext?.pathwayDetailBasePath || '/insights/advisor/recommendations/pathways'}/${pathway.slug}`} + to={`${pathwayDetailBasePath}/${pathway.slug}`} >🤖 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 `@src/PresentationalComponents/PathwaysTable/PathwaysTable.js` around lines 143 - 204, Extract the repeated pathway-detail base path expression into a local variable near rowBuilder, using the existing envContext value with its current fallback. Update both Link destinations in rowBuilder to interpolate that variable while preserving the existing pathway slug path.
🤖 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 `@src/AppConstants.js`:
- Line 512: Update the IOP overview layout so IopOverviewDashbar’s mdSpan
matches the three cards it actually renders; do not derive that span from
displayRecPathways unless a corresponding pathways card is also rendered.
In `@src/PresentationalComponents/Inventory/Hooks/useBulkSelect/useBulkSelect.js`:
- Around line 46-48: The identifier option is misspelled at the Inventory call
site, while selectOne in useBulkSelect currently falls back to id and must
remain aligned with selectedIds matching item.id. Remove the unused identifier
option, or correct it together with every corresponding selection comparison so
the selected and deselected keys consistently use the same field.
In `@src/PresentationalComponents/Inventory/Inventory.js`:
- Around line 452-491: Add message definitions in Messages.js for the
remediation-unavailable tooltip, no-playbook tooltip, and “Plan remediation”
label, then update the corresponding Tooltip content and RemediationButton
children in the Inventory component to use intl.formatMessage with those
messages instead of hardcoded strings.
- Around line 137-147: Update the selection-driven effect at
src/PresentationalComponents/Inventory/Inventory.js lines 137-147 to include
pathwayRulesList, pathwayReportList, and rulesPlaybookCount in its dependencies
so checkRemediationButtonStatus reruns after asynchronous data loads. Also
update the resolutions payload effect at
src/PresentationalComponents/Inventory/Inventory.js lines 399-442 to depend on
rule, pathway, pathwayRulesList, pathwayReportList, and entities, ensuring it
rebuilds when those values become available.
In `@src/SmartComponents/Recs/DetailsPathways.js`:
- Line 206: Remove the redundant unconditional loading render at
src/SmartComponents/Recs/DetailsPathways.js:206-206. Keep
src/SmartComponents/Recs/DetailsPathways.js:163-165 as the single page-level
Loading indicator, and update
src/SmartComponents/Recs/DetailsPathways.js:221-227 to gate tab content on
!loading without rendering another per-tab Loading component.
- Around line 74-92: Update the useGetPathwayQuery handling in DetailsPathways
to destructure isError and stop treating failed requests as an indefinite
loading state. Render the existing ErrorState when isError is true, matching the
behavior used by RulesTable and PathwaysTable, while preserving the current
loading and successful pathway rendering paths.
In `@src/SmartComponents/Recs/ListIop.js`:
- Around line 117-148: Update the Recommendations `RulesTable` inside the `Tabs`
in `ListIop` to pass `isTabActive` based on the active tab, and replace the tab
magic numbers with the shared `RECOMMENDATIONS_TAB` and `PATHWAYS_TAB` constants
used by `List.js`. Preserve the existing inactive-tab rendering and non-pathways
fallback behavior.
In `@src/Utilities/Hooks.test.js`:
- Around line 213-282: Align the tests around useIopEnvironmentContext with the
hook’s intended contract: either implement permission loading and
permission-derived isLoading, isDisableRecEnabled, and isAllowedToViewRec
behavior in useIopEnvironmentContext, or replace the empty, pending, and
partial-permission expectations with the static policy values currently
returned. Keep the assertions consistent with one chosen behavior across all
four test cases.
---
Outside diff comments:
In `@src/Messages.js`:
- Around line 205-218: Update the defaultMessage values for
rulesTableNoRuleHitsDisabledRulesBody and
rulesTableNoRuleHitsRedHatDisabledRulesBody to remove the repeated “no
recommendations are disabled” phrasing, using “or none match the applied filter
settings” while preserving each message’s existing meaning and Red Hat-specific
wording.
In `@src/PresentationalComponents/RulesTable/helpers.js`:
- Around line 235-323: Update the boolean-shaped filter handling in the filter
configuration around FC.incident, FC.has_playbook, and FC.reboot so scalar false
values remain represented as active selections when passed through
normalizeFilterValue. Reuse the shared normalization fix identified in helper.js
rather than adding filter-specific workarounds, while preserving existing array
and truthy-value behavior.
In `@src/PresentationalComponents/SystemsTable/SystemsTable.js`:
- Around line 82-95: Update the incident filter configuration in SystemsTable so
filters.incident is normalized through the same false-value handling required by
normalizeFilterValue before being passed to the checkbox value. Preserve the
existing SFC.incident.urlParam and addFilterParam behavior, and ensure a scalar
false represents an active checked filter rather than an unchecked state.
In `@src/SmartComponents/SystemAdvisor/SystemAdvisorAssets.js`:
- Around line 72-100: The has_playbook filter in the conditional-filter
configuration must preserve scalar false values when normalizing filters. Update
its value handling around normalizeFilterValue and the shared helper behavior so
false becomes the selected “false” filter option rather than an empty or
unchecked value, while retaining existing handling for other filter values.
---
Nitpick comments:
In `@src/PresentationalComponents/Inventory/helpers.js`:
- Around line 34-53: Update the error handling around the systems fetch in the
pathway/rule request to re-throw the caught error instead of returning an empty
`{ data, meta }` result. Preserve the existing diagnostic logging, allowing the
caller to surface an error state rather than treating backend failures as valid
empty results.
In `@src/PresentationalComponents/Inventory/Inventory.js`:
- Around line 122-134: Update handleRefresh to use useCallback with pathway and
selectedTags dependencies, then include the memoized handleRefresh in
fetchSystems’ useMemo dependency list so URL generation reflects tag changes. Do
not alter unrelated dependencies; optionally remove the unused fullFilters
argument from getEntities and its call site if supported by the surrounding
implementation.
- Around line 559-593: Replace the duplicate chromelessExportConfig and
standardExportConfig useMemo definitions with one shared exportConfig memo
containing the existing export behavior and dependencies, then reuse
exportConfig in both relevant branches.
In `@src/PresentationalComponents/PathwaysTable/PathwaysTable.js`:
- Around line 143-204: Extract the repeated pathway-detail base path expression
into a local variable near rowBuilder, using the existing envContext value with
its current fallback. Update both Link destinations in rowBuilder to interpolate
that variable while preserving the existing pathway slug path.
In `@src/PresentationalComponents/RulesTable/filterArraySafety.test.js`:
- Around line 97-147: Update the tests in the “CheckboxFilter Usage” describe
block to call the imported normalizeFilterValue function for each filter value
instead of reimplementing its ternary logic inline. Preserve the existing test
inputs and expected outputs so the cases validate string, boolean, number,
undefined, and array normalization behavior.
In `@src/PresentationalComponents/SystemsTable/systemsFilters.test.js`:
- Around line 23-31: Update the tests in systemsFilters.test.js around the
“should safely handle hits filter value” cases to call the imported
normalizeFilterValue function instead of duplicating its ternary logic inline.
Apply the same change to the tests around lines 59–67, preserving their expected
normalized values so they validate the actual implementation.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 892ad6d8-ac86-4ee3-9d77-4616c3f9c419
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (48)
.github/workflows/container-publish.yaml.gitignore.tekton/advisor-frontend-hermetic-pull-request.yaml.tekton/advisor-frontend-hermetic-push.yaml.tekton/advisor-frontend-pull-request.yaml.tekton/advisor-frontend-push.yaml.tekton/iop-advisor-frontend-sat-6-19-pull-request.yaml.tekton/iop-advisor-frontend-sat-6-19-push.yamlREADME.mdbuild-toolscustom_routes.jsonfec.config.jslocales/translations.jsonpackage.jsonsrc/AppConstants.jssrc/Messages.jssrc/Modules/SystemDetailWrapped.jssrc/PresentationalComponents/Inventory/Hooks/useBulkSelect/useBulkSelect.jssrc/PresentationalComponents/Inventory/Inventory.jssrc/PresentationalComponents/Inventory/helpers.jssrc/PresentationalComponents/OverviewDashbar/Hooks/useOverviewData/useOverviewData.jssrc/PresentationalComponents/OverviewDashbar/IopOverviewDashbar.jssrc/PresentationalComponents/OverviewDashbar/OverviewDashbar.jssrc/PresentationalComponents/PathwaysTable/PathwaysTable.cy.jssrc/PresentationalComponents/PathwaysTable/PathwaysTable.jssrc/PresentationalComponents/RulesTable/Components/EmptyState.jssrc/PresentationalComponents/RulesTable/RulesTable.cy.jssrc/PresentationalComponents/RulesTable/RulesTable.jssrc/PresentationalComponents/RulesTable/RulesTableWrapped.jssrc/PresentationalComponents/RulesTable/filterArraySafety.test.jssrc/PresentationalComponents/RulesTable/helper.test.jssrc/PresentationalComponents/RulesTable/helpers.jssrc/PresentationalComponents/SystemsTable/SystemsTable.jssrc/PresentationalComponents/SystemsTable/systemsFilters.test.jssrc/PresentationalComponents/helper.jssrc/Services/Pathways.jssrc/SmartComponents/HybridInventoryTabs/ConventionalSystems/PathwaySystems.jssrc/SmartComponents/Recs/DetailsPathways.jssrc/SmartComponents/Recs/List.jssrc/SmartComponents/Recs/ListIop.jssrc/SmartComponents/Recs/ListWrapped.jssrc/SmartComponents/Recs/PathwayDetailsWrapped.jssrc/SmartComponents/Recs/RecommendationDetailsWrapped.jssrc/SmartComponents/SystemAdvisor/SystemAdvisorAssets.jssrc/SmartComponents/SystemAdvisor/SystemAdvisorWrapped.jssrc/Utilities/DownloadPlaybookButton.jssrc/Utilities/Hooks.jssrc/Utilities/Hooks.test.js
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/PresentationalComponents/SystemsTable/SystemsTable.js (1)
82-95: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winInherits the
normalizeFilterValuefalsy-value gap for theincidentfilter.If
filters.incidentcan ever be a scalarfalserather than an array, the checkbox will render unchecked despite an active filter. See root-cause comment onsrc/PresentationalComponents/helper.jslines 103-115.🤖 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 `@src/PresentationalComponents/SystemsTable/SystemsTable.js` around lines 82 - 95, Update the incident filter configuration in SystemsTable so filters.incident is normalized through the same false-value handling required by normalizeFilterValue before being passed to the checkbox value. Preserve the existing SFC.incident.urlParam and addFilterParam behavior, and ensure a scalar false represents an active checked filter rather than an unchecked state.src/Messages.js (1)
205-218: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRedundant phrasing in the merged disabled-rules messages.
Both messages repeat "no recommendations are disabled" back-to-back, e.g. "No recommendations are disabled, or no recommendations are disabled that match the applied filter settings." Consider tightening, e.g. "No recommendations are disabled, or none match the applied filter settings."
🤖 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 `@src/Messages.js` around lines 205 - 218, Update the defaultMessage values for rulesTableNoRuleHitsDisabledRulesBody and rulesTableNoRuleHitsRedHatDisabledRulesBody to remove the repeated “no recommendations are disabled” phrasing, using “or none match the applied filter settings” while preserving each message’s existing meaning and Red Hat-specific wording.src/SmartComponents/SystemAdvisor/SystemAdvisorAssets.js (1)
72-100: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winInherits the
normalizeFilterValuefalsy-value gap forhas_playbook.Same concern as flagged on
src/PresentationalComponents/helper.jslines 103-115: a scalarhas_playbook: falsewould render as an unchecked checkbox instead of a selected "false" filter.🤖 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 `@src/SmartComponents/SystemAdvisor/SystemAdvisorAssets.js` around lines 72 - 100, The has_playbook filter in the conditional-filter configuration must preserve scalar false values when normalizing filters. Update its value handling around normalizeFilterValue and the shared helper behavior so false becomes the selected “false” filter option rather than an empty or unchecked value, while retaining existing handling for other filter values.src/PresentationalComponents/RulesTable/helpers.js (1)
235-323: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winInherits the
normalizeFilterValuefalsy-value gap.
incident,has_playbook, andrebootare boolean-shaped filters; if any of them can carry a scalarfalse(not yet array-wrapped),normalizeFilterValuewill render the checkbox as unchecked even though the filter is active. See the root-cause comment onsrc/PresentationalComponents/helper.jslines 103-115.🤖 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 `@src/PresentationalComponents/RulesTable/helpers.js` around lines 235 - 323, Update the boolean-shaped filter handling in the filter configuration around FC.incident, FC.has_playbook, and FC.reboot so scalar false values remain represented as active selections when passed through normalizeFilterValue. Reuse the shared normalization fix identified in helper.js rather than adding filter-specific workarounds, while preserving existing array and truthy-value behavior.
🧹 Nitpick comments (6)
src/PresentationalComponents/Inventory/helpers.js (1)
34-53: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winSwallowed fetch errors are indistinguishable from an empty result set.
Returning
{ data: [], meta: { count: 0 } }makes the table render "no systems" on a backend outage, andallCurrentSystemIdswill silently resolve select-all to zero systems. Consider re-throwing (or surfacing a notification) so the caller can render an error state instead of a misleading empty table.🤖 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 `@src/PresentationalComponents/Inventory/helpers.js` around lines 34 - 53, Update the error handling around the systems fetch in the pathway/rule request to re-throw the caught error instead of returning an empty `{ data, meta }` result. Preserve the existing diagnostic logging, allowing the caller to surface an error state rather than treating backend failures as valid empty results.src/PresentationalComponents/Inventory/Inventory.js (2)
122-134: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
fetchSystemsmemo captures a stalehandleRefresh.
handleRefreshcloses overselectedTags(used byurlBuilder), butselectedTagsisn't in the dependency list, so tag changes won't be reflected in the URL built during subsequent fetches. WrappinghandleRefreshinuseCallback([pathway, selectedTags])and adding it here keeps the closure fresh. (fullFiltersis passed but never read insidegetEntities, so its omission is harmless — consider dropping the parameter.)🤖 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 `@src/PresentationalComponents/Inventory/Inventory.js` around lines 122 - 134, Update handleRefresh to use useCallback with pathway and selectedTags dependencies, then include the memoized handleRefresh in fetchSystems’ useMemo dependency list so URL generation reflects tag changes. Do not alter unrelated dependencies; optionally remove the unused fullFilters argument from getEntities and its call site if supported by the surrounding implementation.
559-593: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
chromelessExportConfigandstandardExportConfigare byte-identical.Collapse into a single
exportConfigmemo and reuse it in both branches.♻️ Proposed refactor
- const chromelessExportConfig = useMemo(() => permsExport && { + const exportConfig = useMemo(() => permsExport && { label: intl.formatMessage(messages.exportJson), onSelect: (_e, fileType) => downloadReport( exportTable, fileType, { rule_id: rule.rule_id, ...filters }, selectedTags, workloads, dispatch, envContext.BASE_URL, ), isDisabled: !permsExport || entities?.rows?.length === 0, tooltipText: permsExport ? intl.formatMessage(messages.exportData) : intl.formatMessage(messages.permsAction), }, [permsExport, filters, rule, selectedTags, workloads, entities?.rows?.length, envContext.BASE_URL]); - - const standardExportConfig = useMemo(() => permsExport && { - label: intl.formatMessage(messages.exportJson), - onSelect: (_e, fileType) => - downloadReport( - exportTable, - fileType, - { rule_id: rule.rule_id, ...filters }, - selectedTags, - workloads, - dispatch, - envContext.BASE_URL, - ), - isDisabled: !permsExport || entities?.rows?.length === 0, - tooltipText: permsExport - ? intl.formatMessage(messages.exportData) - : intl.formatMessage(messages.permsAction), - }, [permsExport, filters, rule, selectedTags, workloads, entities?.rows?.length, envContext.BASE_URL]);🤖 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 `@src/PresentationalComponents/Inventory/Inventory.js` around lines 559 - 593, Replace the duplicate chromelessExportConfig and standardExportConfig useMemo definitions with one shared exportConfig memo containing the existing export behavior and dependencies, then reuse exportConfig in both relevant branches.src/PresentationalComponents/SystemsTable/systemsFilters.test.js (1)
23-31: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate tautological assertions — doesn't test the real
normalizeFilterValue.Same pattern as
filterArraySafety.test.js: these two tests re-derive the value with an inline ternary instead of calling the importednormalizeFilterValue, so they don't actually validate the implementation.Also applies to: 59-67
🤖 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 `@src/PresentationalComponents/SystemsTable/systemsFilters.test.js` around lines 23 - 31, Update the tests in systemsFilters.test.js around the “should safely handle hits filter value” cases to call the imported normalizeFilterValue function instead of duplicating its ternary logic inline. Apply the same change to the tests around lines 59–67, preserving their expected normalized values so they validate the actual implementation.src/PresentationalComponents/RulesTable/filterArraySafety.test.js (1)
97-147: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win"CheckboxFilter Usage" tests don't exercise
normalizeFilterValue.These five tests re-implement the same ternary inline (
Array.isArray(...) ? ... : ... ? [String(...)] : []) rather than calling the importednormalizeFilterValue, so they're tautological — they'll always pass regardless of whether the real function is correct.🤖 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 `@src/PresentationalComponents/RulesTable/filterArraySafety.test.js` around lines 97 - 147, Update the tests in the “CheckboxFilter Usage” describe block to call the imported normalizeFilterValue function for each filter value instead of reimplementing its ternary logic inline. Preserve the existing test inputs and expected outputs so the cases validate string, boolean, number, undefined, and array normalization behavior.src/PresentationalComponents/PathwaysTable/PathwaysTable.js (1)
143-204: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the duplicated pathway-detail base path.
envContext?.pathwayDetailBasePath || '/insights/advisor/recommendations/pathways'is repeated verbatim at lines 169 and 193.♻️ Proposed refactor
+ const pathwayDetailBasePath = + envContext?.pathwayDetailBasePath || '/insights/advisor/recommendations/pathways'; + const rowBuilder = (pathways) => ... <Link key={key} - to={`${envContext?.pathwayDetailBasePath || '/insights/advisor/recommendations/pathways'}/${pathway.slug}`} + to={`${pathwayDetailBasePath}/${pathway.slug}`} > ... <Link key={key} - to={`${envContext?.pathwayDetailBasePath || '/insights/advisor/recommendations/pathways'}/${pathway.slug}`} + to={`${pathwayDetailBasePath}/${pathway.slug}`} >🤖 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 `@src/PresentationalComponents/PathwaysTable/PathwaysTable.js` around lines 143 - 204, Extract the repeated pathway-detail base path expression into a local variable near rowBuilder, using the existing envContext value with its current fallback. Update both Link destinations in rowBuilder to interpolate that variable while preserving the existing pathway slug path.
🤖 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 `@src/AppConstants.js`:
- Line 512: Update the IOP overview layout so IopOverviewDashbar’s mdSpan
matches the three cards it actually renders; do not derive that span from
displayRecPathways unless a corresponding pathways card is also rendered.
In `@src/PresentationalComponents/Inventory/Hooks/useBulkSelect/useBulkSelect.js`:
- Around line 46-48: The identifier option is misspelled at the Inventory call
site, while selectOne in useBulkSelect currently falls back to id and must
remain aligned with selectedIds matching item.id. Remove the unused identifier
option, or correct it together with every corresponding selection comparison so
the selected and deselected keys consistently use the same field.
In `@src/PresentationalComponents/Inventory/Inventory.js`:
- Around line 452-491: Add message definitions in Messages.js for the
remediation-unavailable tooltip, no-playbook tooltip, and “Plan remediation”
label, then update the corresponding Tooltip content and RemediationButton
children in the Inventory component to use intl.formatMessage with those
messages instead of hardcoded strings.
- Around line 137-147: Update the selection-driven effect at
src/PresentationalComponents/Inventory/Inventory.js lines 137-147 to include
pathwayRulesList, pathwayReportList, and rulesPlaybookCount in its dependencies
so checkRemediationButtonStatus reruns after asynchronous data loads. Also
update the resolutions payload effect at
src/PresentationalComponents/Inventory/Inventory.js lines 399-442 to depend on
rule, pathway, pathwayRulesList, pathwayReportList, and entities, ensuring it
rebuilds when those values become available.
In `@src/SmartComponents/Recs/DetailsPathways.js`:
- Line 206: Remove the redundant unconditional loading render at
src/SmartComponents/Recs/DetailsPathways.js:206-206. Keep
src/SmartComponents/Recs/DetailsPathways.js:163-165 as the single page-level
Loading indicator, and update
src/SmartComponents/Recs/DetailsPathways.js:221-227 to gate tab content on
!loading without rendering another per-tab Loading component.
- Around line 74-92: Update the useGetPathwayQuery handling in DetailsPathways
to destructure isError and stop treating failed requests as an indefinite
loading state. Render the existing ErrorState when isError is true, matching the
behavior used by RulesTable and PathwaysTable, while preserving the current
loading and successful pathway rendering paths.
In `@src/SmartComponents/Recs/ListIop.js`:
- Around line 117-148: Update the Recommendations `RulesTable` inside the `Tabs`
in `ListIop` to pass `isTabActive` based on the active tab, and replace the tab
magic numbers with the shared `RECOMMENDATIONS_TAB` and `PATHWAYS_TAB` constants
used by `List.js`. Preserve the existing inactive-tab rendering and non-pathways
fallback behavior.
In `@src/Utilities/Hooks.test.js`:
- Around line 213-282: Align the tests around useIopEnvironmentContext with the
hook’s intended contract: either implement permission loading and
permission-derived isLoading, isDisableRecEnabled, and isAllowedToViewRec
behavior in useIopEnvironmentContext, or replace the empty, pending, and
partial-permission expectations with the static policy values currently
returned. Keep the assertions consistent with one chosen behavior across all
four test cases.
---
Outside diff comments:
In `@src/Messages.js`:
- Around line 205-218: Update the defaultMessage values for
rulesTableNoRuleHitsDisabledRulesBody and
rulesTableNoRuleHitsRedHatDisabledRulesBody to remove the repeated “no
recommendations are disabled” phrasing, using “or none match the applied filter
settings” while preserving each message’s existing meaning and Red Hat-specific
wording.
In `@src/PresentationalComponents/RulesTable/helpers.js`:
- Around line 235-323: Update the boolean-shaped filter handling in the filter
configuration around FC.incident, FC.has_playbook, and FC.reboot so scalar false
values remain represented as active selections when passed through
normalizeFilterValue. Reuse the shared normalization fix identified in helper.js
rather than adding filter-specific workarounds, while preserving existing array
and truthy-value behavior.
In `@src/PresentationalComponents/SystemsTable/SystemsTable.js`:
- Around line 82-95: Update the incident filter configuration in SystemsTable so
filters.incident is normalized through the same false-value handling required by
normalizeFilterValue before being passed to the checkbox value. Preserve the
existing SFC.incident.urlParam and addFilterParam behavior, and ensure a scalar
false represents an active checked filter rather than an unchecked state.
In `@src/SmartComponents/SystemAdvisor/SystemAdvisorAssets.js`:
- Around line 72-100: The has_playbook filter in the conditional-filter
configuration must preserve scalar false values when normalizing filters. Update
its value handling around normalizeFilterValue and the shared helper behavior so
false becomes the selected “false” filter option rather than an empty or
unchecked value, while retaining existing handling for other filter values.
---
Nitpick comments:
In `@src/PresentationalComponents/Inventory/helpers.js`:
- Around line 34-53: Update the error handling around the systems fetch in the
pathway/rule request to re-throw the caught error instead of returning an empty
`{ data, meta }` result. Preserve the existing diagnostic logging, allowing the
caller to surface an error state rather than treating backend failures as valid
empty results.
In `@src/PresentationalComponents/Inventory/Inventory.js`:
- Around line 122-134: Update handleRefresh to use useCallback with pathway and
selectedTags dependencies, then include the memoized handleRefresh in
fetchSystems’ useMemo dependency list so URL generation reflects tag changes. Do
not alter unrelated dependencies; optionally remove the unused fullFilters
argument from getEntities and its call site if supported by the surrounding
implementation.
- Around line 559-593: Replace the duplicate chromelessExportConfig and
standardExportConfig useMemo definitions with one shared exportConfig memo
containing the existing export behavior and dependencies, then reuse
exportConfig in both relevant branches.
In `@src/PresentationalComponents/PathwaysTable/PathwaysTable.js`:
- Around line 143-204: Extract the repeated pathway-detail base path expression
into a local variable near rowBuilder, using the existing envContext value with
its current fallback. Update both Link destinations in rowBuilder to interpolate
that variable while preserving the existing pathway slug path.
In `@src/PresentationalComponents/RulesTable/filterArraySafety.test.js`:
- Around line 97-147: Update the tests in the “CheckboxFilter Usage” describe
block to call the imported normalizeFilterValue function for each filter value
instead of reimplementing its ternary logic inline. Preserve the existing test
inputs and expected outputs so the cases validate string, boolean, number,
undefined, and array normalization behavior.
In `@src/PresentationalComponents/SystemsTable/systemsFilters.test.js`:
- Around line 23-31: Update the tests in systemsFilters.test.js around the
“should safely handle hits filter value” cases to call the imported
normalizeFilterValue function instead of duplicating its ternary logic inline.
Apply the same change to the tests around lines 59–67, preserving their expected
normalized values so they validate the actual implementation.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 892ad6d8-ac86-4ee3-9d77-4616c3f9c419
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (48)
.github/workflows/container-publish.yaml.gitignore.tekton/advisor-frontend-hermetic-pull-request.yaml.tekton/advisor-frontend-hermetic-push.yaml.tekton/advisor-frontend-pull-request.yaml.tekton/advisor-frontend-push.yaml.tekton/iop-advisor-frontend-sat-6-19-pull-request.yaml.tekton/iop-advisor-frontend-sat-6-19-push.yamlREADME.mdbuild-toolscustom_routes.jsonfec.config.jslocales/translations.jsonpackage.jsonsrc/AppConstants.jssrc/Messages.jssrc/Modules/SystemDetailWrapped.jssrc/PresentationalComponents/Inventory/Hooks/useBulkSelect/useBulkSelect.jssrc/PresentationalComponents/Inventory/Inventory.jssrc/PresentationalComponents/Inventory/helpers.jssrc/PresentationalComponents/OverviewDashbar/Hooks/useOverviewData/useOverviewData.jssrc/PresentationalComponents/OverviewDashbar/IopOverviewDashbar.jssrc/PresentationalComponents/OverviewDashbar/OverviewDashbar.jssrc/PresentationalComponents/PathwaysTable/PathwaysTable.cy.jssrc/PresentationalComponents/PathwaysTable/PathwaysTable.jssrc/PresentationalComponents/RulesTable/Components/EmptyState.jssrc/PresentationalComponents/RulesTable/RulesTable.cy.jssrc/PresentationalComponents/RulesTable/RulesTable.jssrc/PresentationalComponents/RulesTable/RulesTableWrapped.jssrc/PresentationalComponents/RulesTable/filterArraySafety.test.jssrc/PresentationalComponents/RulesTable/helper.test.jssrc/PresentationalComponents/RulesTable/helpers.jssrc/PresentationalComponents/SystemsTable/SystemsTable.jssrc/PresentationalComponents/SystemsTable/systemsFilters.test.jssrc/PresentationalComponents/helper.jssrc/Services/Pathways.jssrc/SmartComponents/HybridInventoryTabs/ConventionalSystems/PathwaySystems.jssrc/SmartComponents/Recs/DetailsPathways.jssrc/SmartComponents/Recs/List.jssrc/SmartComponents/Recs/ListIop.jssrc/SmartComponents/Recs/ListWrapped.jssrc/SmartComponents/Recs/PathwayDetailsWrapped.jssrc/SmartComponents/Recs/RecommendationDetailsWrapped.jssrc/SmartComponents/SystemAdvisor/SystemAdvisorAssets.jssrc/SmartComponents/SystemAdvisor/SystemAdvisorWrapped.jssrc/Utilities/DownloadPlaybookButton.jssrc/Utilities/Hooks.jssrc/Utilities/Hooks.test.js
🛑 Comments failed to post (8)
src/AppConstants.js (1)
512-512: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Keep the IOP overview grid aligned with its rendered cards.
IopOverviewDashbarswitches tomdSpan = 3when this is true, but it renders only incidents, critical, and important cards—not a pathways card—leaving 25% of the grid empty. Decouple its span from the pathways-tab flag, or add the missing pathways card.🤖 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 `@src/AppConstants.js` at line 512, Update the IOP overview layout so IopOverviewDashbar’s mdSpan matches the three cards it actually renders; do not derive that span from displayRecPathways unless a corresponding pathways card is also rendered.src/PresentationalComponents/Inventory/Hooks/useBulkSelect/useBulkSelect.js (1)
46-48: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Heads-up on the
identifieroption while you're in here.The Inventory caller passes
identitfier: 'system_uuid'(misspelled), so this line actually resolvesrow['id']via the default. That happens to be what the rest of the flow expects (selected: selectedIds?.includes(item.id)insrc/PresentationalComponents/Inventory/helpers.js), so correcting the typo alone would silently switch selection tosystem_uuidand break selection matching. Either drop the dead option at the call site or fix both together.🤖 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 `@src/PresentationalComponents/Inventory/Hooks/useBulkSelect/useBulkSelect.js` around lines 46 - 48, The identifier option is misspelled at the Inventory call site, while selectOne in useBulkSelect currently falls back to id and must remain aligned with selectedIds matching item.id. Remove the unused identifier option, or correct it together with every corresponding selection comparison so the selected and deselected keys consistently use the same field.src/PresentationalComponents/Inventory/Inventory.js (2)
137-147: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Selection-driven effects only depend on
safeSelectedIdswhile the data they read loads asynchronously.pathwayRulesList,pathwayReportList,rulesPlaybookCount, andentitiesare all populated after mount bypathwayCheck/rulesCheck/the inventory fetch, so any selection made before those resolve leaves the derived state stale until the user re-selects.
src/PresentationalComponents/Inventory/Inventory.js#L137-L147: addpathwayRulesList,pathwayReportList, andrulesPlaybookCountto the dependency array socheckRemediationButtonStatusre-runs when the fetches land.src/PresentationalComponents/Inventory/Inventory.js#L399-L442: addrule,pathway,pathwayRulesList,pathwayReportList, andentitiesso the resolutions payload is rebuilt once pathway rules/reports and inventory rows are available.📍 Affects 1 file
src/PresentationalComponents/Inventory/Inventory.js#L137-L147(this comment)src/PresentationalComponents/Inventory/Inventory.js#L399-L442🤖 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 `@src/PresentationalComponents/Inventory/Inventory.js` around lines 137 - 147, Update the selection-driven effect at src/PresentationalComponents/Inventory/Inventory.js lines 137-147 to include pathwayRulesList, pathwayReportList, and rulesPlaybookCount in its dependencies so checkRemediationButtonStatus reruns after asynchronous data loads. Also update the resolutions payload effect at src/PresentationalComponents/Inventory/Inventory.js lines 399-442 to depend on rule, pathway, pathwayRulesList, pathwayReportList, and entities, ensuring it rebuilds when those values become available.
452-491: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Tooltip strings bypass
react-intl."Remediation is not available for the selected systems." and "No playbook available for the selected systems." (and
Plan remediationat Line 480) are hardcoded while the rest of the component usesintl.formatMessage(messages.*). Add them tosrc/Messages.jsso they're translatable.🤖 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 `@src/PresentationalComponents/Inventory/Inventory.js` around lines 452 - 491, Add message definitions in Messages.js for the remediation-unavailable tooltip, no-playbook tooltip, and “Plan remediation” label, then update the corresponding Tooltip content and RemediationButton children in the Inventory component to use intl.formatMessage with those messages instead of hardcoded strings.src/SmartComponents/Recs/DetailsPathways.js (2)
74-92: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Missing
isErrorhandling causes an infinite loading state on API failure.
loadingis derived fromisFetching || isLoading || Object.keys(pathway).length === 0, butisErroris never destructured fromuseGetPathwayQuery. If the pathway request fails,pathwaystays{}(the default), soloadingremainstrueindefinitely and the page never shows an error — unlikeRulesTable/PathwaysTable, which both render<ErrorState />onisError.🐛 Proposed fix
- const { data: pathway = {}, isFetching, isLoading } = useGetPathwayQuery({ + const { data: pathway = {}, isFetching, isLoading, isError } = useGetPathwayQuery({ ...options, slug: pathwayName, }); const loading = isFetching || isLoading || Object.keys(pathway).length === 0; + // render <ErrorState /> when isError is true, similar to RulesTable/PathwaysTable🤖 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 `@src/SmartComponents/Recs/DetailsPathways.js` around lines 74 - 92, Update the useGetPathwayQuery handling in DetailsPathways to destructure isError and stop treating failed requests as an indefinite loading state. Render the existing ErrorState when isError is true, matching the behavior used by RulesTable and PathwaysTable, while preserving the current loading and successful pathway rendering paths.
206-206: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Stacked duplicate
<Loading />renders whilepathwayis loading. Three separate checks all gate on the sameloadingboolean without coordinating, so multiple loading skeletons render simultaneously.
src/SmartComponents/Recs/DetailsPathways.js#L206-L206: remove this redundant unconditional{loading && <Loading />}— the header block above (L163-L165) already renders<Loading />for the same condition.src/SmartComponents/Recs/DetailsPathways.js#L163-L165: keep as the single top-level loading indicator for the page.src/SmartComponents/Recs/DetailsPathways.js#L221-L227: since the page-levelloadingcheck already covers this state, consider gating the tab content on!loadingonly once the top-level Loading has resolved, rather than repeating a separate<Loading />per tab.📍 Affects 1 file
src/SmartComponents/Recs/DetailsPathways.js#L206-L206(this comment)src/SmartComponents/Recs/DetailsPathways.js#L163-L165src/SmartComponents/Recs/DetailsPathways.js#L221-L227🤖 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 `@src/SmartComponents/Recs/DetailsPathways.js` at line 206, Remove the redundant unconditional loading render at src/SmartComponents/Recs/DetailsPathways.js:206-206. Keep src/SmartComponents/Recs/DetailsPathways.js:163-165 as the single page-level Loading indicator, and update src/SmartComponents/Recs/DetailsPathways.js:221-227 to gate tab content on !loading without rendering another per-tab Loading component.src/SmartComponents/Recs/ListIop.js (1)
117-148: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Missing
isTabActiveon the RecommendationsRulesTablebreaks URL filter hydration.Unlike
List.js, which passesisTabActive={activeTab === RECOMMENDATIONS_TAB}toRulesTableinside the Tabs, this Recommendations tab only passesonRuleChange.RulesTable's URL-filter-building effect is gated onisTabActive && filterBuilding, but it unconditionally setsfilterBuildingtofalseon the same run — so whenisTabActiveis falsy (undefined here), the filters encoded in the URL are silently never applied, on the very first render.Also,
eventKey={0}/eventKey={1}are magic numbers;List.jsuses the exportedRECOMMENDATIONS_TAB/PATHWAYS_TABconstants for the same purpose.🐛 Proposed fix
<Tab eventKey={0} title={<TabTitleText>Recommendations</TabTitleText>} > - <RulesTable onRuleChange={handleRuleChange} /> + <RulesTable + isTabActive={activeTab === 0} + onRuleChange={handleRuleChange} + /> </Tab>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.<IopOverviewDashbar changeTab={changeTab} onRefetchReady={handleOverviewRefetchReady} /> </StackItem> <StackItem> {envContext.displayRecPathways ? ( <Tabs className="adv__background--global-100" activeKey={activeTab} onSelect={(_e, tab) => changeTab(tab)} > <Tab eventKey={0} title={<TabTitleText>Recommendations</TabTitleText>} > <RulesTable isTabActive={activeTab === 0} onRuleChange={handleRuleChange} /> </Tab> <Tab eventKey={1} title={<TabTitleText>Pathways</TabTitleText>} > {activeTab === 1 && ( <Suspense fallback={<Loading />}> <PathwaysTable isTabActive={activeTab === 1} /> </Suspense> )} </Tab> </Tabs> ) : ( <RulesTable onRuleChange={handleRuleChange} /> )}🤖 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 `@src/SmartComponents/Recs/ListIop.js` around lines 117 - 148, Update the Recommendations `RulesTable` inside the `Tabs` in `ListIop` to pass `isTabActive` based on the active tab, and replace the tab magic numbers with the shared `RECOMMENDATIONS_TAB` and `PATHWAYS_TAB` constants used by `List.js`. Preserve the existing inactive-tab rendering and non-pathways fallback behavior.src/Utilities/Hooks.test.js (1)
213-282: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Align these expectations with
useIopEnvironmentContext.
src/Utilities/Hooks.js:128-138currently never callsuseChromeor reads permissions; it always returnsisLoading: false,isDisableRecEnabled: true, andisAllowedToViewRec: true. Therefore the empty, pending, and partial-permission tests fail. Implement the permission-based hook behavior or adjust these tests to the intended static IOP policy.🤖 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 `@src/Utilities/Hooks.test.js` around lines 213 - 282, Align the tests around useIopEnvironmentContext with the hook’s intended contract: either implement permission loading and permission-derived isLoading, isDisableRecEnabled, and isAllowedToViewRec behavior in useIopEnvironmentContext, or replace the empty, pending, and partial-permission expectations with the static policy values currently returned. Keep the assertions consistent with one chosen behavior across all four test cases.
2851542
into
RedHatInsights:foreman-5.0
2 participants
To test this PR, all you need to do:
Run npm i
export IOP_URL={current IOP instance, DM if you need a url} npm run start:proxy:iop
verify everything works as expected after navigating to iop.foo.redhat.com and logging in
Theres currently an issue where when trying to kill the applicaiton, it doesnt always kill webpack instances. If it hangs up, try
ps aux | grep webpack | grep -v grep
and then kill -9 whatever port is running
Theres a currently a PR in FEC to fix this: RedHatInsights/frontend-components#2397
Summary by Sourcery
Add support and configuration updates for running Advisor via the IOP development proxy.
New Features:
Enhancements:
Build:
Documentation:
Chores: