fix(CVEDetails): RHINENG-19856 - Update remediation section #2423
+81
−48
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Reviewer's GuideThis PR refactors the remediation section in CsawRuleBox by extracting display logic into a new RemediationPlanSplitItem component, integrating it into the main component, simplifying the reboot requirement to an icon-based yes/no label, and updating tests to match the new UI structure. Class diagram for CsawRuleBox and RemediationPlanSplitItem refactorclassDiagram
class CsawRuleBox {
+rules
+synopsis
+intl
+setHeaderFilters
+headerFilters
+render()
}
class RemediationPlanSplitItem {
+rule
+render()
}
CsawRuleBox --> RemediationPlanSplitItem: uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
bastilian
force-pushed
the
RHINENG-19856
branch
4 times, most recently
from
94f90a7 to
7a02634
Compare
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- Use intl.formatMessage and message definitions for all new user-facing text (e.g. “Remediation plan”, “Available”, “Not available” and tooltip content) to maintain localization support.
- Add aria-labels or accessible text to the icon elements in RemediationPlanSplitItem so screen readers can convey their purpose.
- Consider extracting the reboot-required UI into its own small component to mirror RemediationPlanSplitItem and keep CsawRuleBox concerns separated.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Use intl.formatMessage and message definitions for all new user-facing text (e.g. “Remediation plan”, “Available”, “Not available” and tooltip content) to maintain localization support.
- Add aria-labels or accessible text to the icon elements in RemediationPlanSplitItem so screen readers can convey their purpose.
- Consider extracting the reboot-required UI into its own small component to mirror RemediationPlanSplitItem and keep CsawRuleBox concerns separated.
## Individual Comments
### Comment 1
<location> `src/Components/PresentationalComponents/CsawRuleBox/CsawRuleBox.js:141` </location>
<code_context>
<SplitItem>
<Label className="label pf-v5-u-mb-xs">
- {intl.formatMessage(messages.remediationLabel)}
+ Remediation requires reboot
</Label>
<Split>
</code_context>
<issue_to_address>
**suggestion:** The label 'Remediation requires reboot' may be misleading if reboot is not always required.
If reboot is only needed in certain cases, update the label to indicate when a reboot is actually required.
Suggested implementation:
```javascript
<Label className="label pf-v5-u-mb-xs">
{rule.requires_reboot
? 'Remediation requires reboot'
: 'Remediation does not require reboot'}
</Label>
```
If the `rule` object does not have a `requires_reboot` property, you will need to add or derive this property based on your application's logic.
</issue_to_address>
### Comment 2
<location> `src/Components/PresentationalComponents/CsawRuleBox/components/RemediationPlanSplitItem.js:22` </location>
<code_context>
+ </Label>
+ <Split>
+ <SplitItem data-testid="remediation-plan">
+ {rule?.playbook_count === 0
+ ? (
+ <>
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring the repeated status rendering logic into a reusable StatusIndicator component with a lookup map.
You can collapse all of that nested JSX and duplicated fragments into a small, self-contained `StatusIndicator` component and a simple lookup map. For example:
```jsx
// StatusIndicator.js
import React from 'react';
import PropTypes from 'prop-types';
import { Tooltip, Icon } from '@patternfly/react-core';
import { CheckCircleIcon, TimesIcon } from '@patternfly/react-icons';
const STATUS_MAP = {
available: {
IconComponent: CheckCircleIcon,
tooltip: 'You can create an Ansible playbook to remediate your systems.',
label: 'Available',
iconClass: 'pf-v5-u-mr-xs',
},
unavailable: {
IconComponent: TimesIcon,
tooltip: 'No Ansible playbooks are available, manual remediation is required for patching.',
label: 'Not available',
iconClass: 'pf-v5-u-mr-xs',
},
};
const StatusIndicator = ({ status }) => {
const { IconComponent, tooltip, label, iconClass } = STATUS_MAP[status];
return (
<>
<Tooltip content={tooltip}>
<Icon>
<IconComponent className={iconClass} />
</Icon>
</Tooltip>
{label}
</>
);
};
StatusIndicator.propTypes = {
status: PropTypes.oneOf(['available', 'unavailable']).isRequired,
};
export default StatusIndicator;
```
Then simplify your main component to:
```jsx
import React from 'react';
import PropTypes from 'prop-types';
import { Split, SplitItem } from '@patternfly/react-core';
import Label from '../../Snippets/Label';
import StatusIndicator from './StatusIndicator';
const RemediationPlanSplitItem = ({ rule }) => {
const status = rule?.playbook_count > 0 ? 'available' : 'unavailable';
return (
<SplitItem>
<Label className="label pf-v5-u-mb-xs">Remediation plan</Label>
<Split>
<SplitItem data-testid="remediation-plan">
<StatusIndicator status={status} />
</SplitItem>
</Split>
</SplitItem>
);
};
RemediationPlanSplitItem.propTypes = {
rule: PropTypes.shape({
playbook_count: PropTypes.number,
}),
};
export default RemediationPlanSplitItem;
```
This removes the inline fragments, duplicate tooltips/icons, and makes it trivial to extend or tweak in the future.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| </Label> | ||
| <Split> | ||
| <SplitItem data-testid="remediation-plan"> | ||
| {rule?.playbook_count === 0 |
There was a problem hiding this comment.
issue (complexity): Consider refactoring the repeated status rendering logic into a reusable StatusIndicator component with a lookup map.
You can collapse all of that nested JSX and duplicated fragments into a small, self-contained StatusIndicator component and a simple lookup map. For example:
// StatusIndicator.js
import React from 'react';
import PropTypes from 'prop-types';
import { Tooltip, Icon } from '@patternfly/react-core';
import { CheckCircleIcon, TimesIcon } from '@patternfly/react-icons';
const STATUS_MAP = {
available: {
IconComponent: CheckCircleIcon,
tooltip: 'You can create an Ansible playbook to remediate your systems.',
label: 'Available',
iconClass: 'pf-v5-u-mr-xs',
},
unavailable: {
IconComponent: TimesIcon,
tooltip: 'No Ansible playbooks are available, manual remediation is required for patching.',
label: 'Not available',
iconClass: 'pf-v5-u-mr-xs',
},
};
const StatusIndicator = ({ status }) => {
const { IconComponent, tooltip, label, iconClass } = STATUS_MAP[status];
return (
<>
<Tooltip content={tooltip}>
<Icon>
<IconComponent className={iconClass} />
</Icon>
</Tooltip>
{label}
</>
);
};
StatusIndicator.propTypes = {
status: PropTypes.oneOf(['available', 'unavailable']).isRequired,
};
export default StatusIndicator;Then simplify your main component to:
import React from 'react';
import PropTypes from 'prop-types';
import { Split, SplitItem } from '@patternfly/react-core';
import Label from '../../Snippets/Label';
import StatusIndicator from './StatusIndicator';
const RemediationPlanSplitItem = ({ rule }) => {
const status = rule?.playbook_count > 0 ? 'available' : 'unavailable';
return (
<SplitItem>
<Label className="label pf-v5-u-mb-xs">Remediation plan</Label>
<Split>
<SplitItem data-testid="remediation-plan">
<StatusIndicator status={status} />
</SplitItem>
</Split>
</SplitItem>
);
};
RemediationPlanSplitItem.propTypes = {
rule: PropTypes.shape({
playbook_count: PropTypes.number,
}),
};
export default RemediationPlanSplitItem;This removes the inline fragments, duplicate tooltips/icons, and makes it trivial to extend or tweak in the future.
Contributor
|
In your PR the text is still the normal redhat font/color. In the mocks its lighter/ the 'disabled' font : To the CheckCircleIcon you may need to add status="success" in order to make it green. |
Member
Author
|
@adonispuente I think the font weight difference may just be a rendering thing or just accidental, at least I think there is no reason that this would have a different weight. The lack of colour when it's "Not available"/"No" is mentioned in the mocks as somewhat intentional only the "Availbal"/"Yes" states have "color": 
|
bastilian
force-pushed
the
RHINENG-19856
branch
from
7a02634 to
73f9f8c
Compare
bastilian
force-pushed
the
RHINENG-19856
branch
from
73f9f8c to
20d1b58
Compare
LightOfHeaven1994
approved these changes
Dec 4, 2025
Contributor
LightOfHeaven1994
left a comment
LightOfHeaven1994
left a comment
There was a problem hiding this comment.
LGTM, good work!
bastilian
force-pushed
the
RHINENG-19856
branch
from
20d1b58 to
c8bc01d
Compare
|
Referenced Jiras: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This updates the "remediation section" on the CVE details page.
Before:
After:
How to test:
Summary by Sourcery
Update the remediation section in CsawRuleBox by introducing a dedicated component for remediation plan availability and streamlining the reboot requirement display
Enhancements:
Summary by Sourcery
Update the remediation section in CsawRuleBox by extracting remediation plan availability into its own component and simplifying the reboot requirement display
New Features:
Enhancements:
Tests: