Make service page Instances, Providers, and Microsegmentation configurable#3278
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces granular visibility controls for Instances, Providers, and Microsegmentation by adding specific sub-flags to the feature flag configuration. The changes span Redux selectors, UI components, and the backend API handler to ensure these elements are conditionally rendered. Feedback identifies a bug in Tabs.js where mutating an instance property during render could cause duplicate tabs, and suggests consistently checking the global featureFlag alongside the new specific flags in ServiceList.js and ServiceRow.js to maintain logic consistency.
| if (this.props.featureFlag && this.props.showMicrosegmentation) { | ||
| this.TABS.push(microSeg); | ||
| } |
There was a problem hiding this comment.
Mutating this.TABS inside the render method is problematic in React. Since this.TABS is an instance property (defined on line 30), every re-render will push another microSeg object into the array if the condition is met, leading to duplicate tabs in the UI. Furthermore, if showMicrosegmentation or featureFlag becomes false after a previous render where they were true, the tab will not be removed from the array.
Instead of mutating the instance property, you should construct the list of tabs to be rendered locally within the render method. For example:
const tabs = this.props.featureFlag && this.props.showMicrosegmentation
? [...this.TABS, microSeg]
: this.TABS;Then use this local tabs variable in the TabGroup component.
| {this.props.showMicrosegmentation ? ( | ||
| <TableHeadStyled align={center}> | ||
| MSD Policies | ||
| </TableHeadStyled> | ||
| ) : null} |
There was a problem hiding this comment.
For consistency with the logic used for the Instances column (lines 243-244) and the tab visibility in ServiceTabs.js and Tabs.js, the MSD Policies column should also be gated by this.props.featureFlag.
| {this.props.showMicrosegmentation ? ( | |
| <TableHeadStyled align={center}> | |
| MSD Policies | |
| </TableHeadStyled> | |
| ) : null} | |
| {this.props.featureFlag && | |
| this.props.showMicrosegmentation ? ( | |
| <TableHeadStyled align={center}> | |
| MSD Policies | |
| </TableHeadStyled> | |
| ) : null} |
| {this.props.showMicrosegmentation ? ( | ||
| <TdStyled color={color} align={center}> | ||
| <Icon | ||
| icon={'list-check'} | ||
| onClick={this.clickMicrosegmentation} | ||
| color={colors.icons} | ||
| isLink | ||
| size={'1.25em'} | ||
| verticalAlign={'text-bottom'} | ||
| /> | ||
| </TdStyled> | ||
| ) : null} |
There was a problem hiding this comment.
For consistency with the headers in ServiceList.js and the tab visibility logic, the microsegmentation icon should also check this.props.featureFlag before rendering.
| {this.props.showMicrosegmentation ? ( | |
| <TdStyled color={color} align={center}> | |
| <Icon | |
| icon={'list-check'} | |
| onClick={this.clickMicrosegmentation} | |
| color={colors.icons} | |
| isLink | |
| size={'1.25em'} | |
| verticalAlign={'text-bottom'} | |
| /> | |
| </TdStyled> | |
| ) : null} | |
| {this.props.featureFlag && this.props.showMicrosegmentation ? ( | |
| <TdStyled color={color} align={center}> | |
| <Icon | |
| icon={'list-check'} | |
| onClick={this.clickMicrosegmentation} | |
| color={colors.icons} | |
| isLink | |
| size={'1.25em'} | |
| verticalAlign={'text-bottom'} | |
| /> | |
| </TdStyled> | |
| ) : null} |
7fb6d2e to
817d6a8
Compare
…rable Add three boolean flags to servicePageConfig (all default true for backward compatibility): - showInstances: controls Instances column and Instance tabs - showProviders: controls Providers column - showMicrosegmentation: controls MSD Policies column and Microsegmentation tabs The flags are exposed via the feature-flag Fetchr service and consumed through new Redux selectors. Deployments that override config via extended-config.js can set any of these to false to hide the corresponding UI features. Made-with: Cursor Signed-off-by: dma <dma@yahooinc.com> Made-with: Cursor
817d6a8 to
505a171
Compare
Add three boolean flags to servicePageConfig (all default true for backward compatibility):
The flags are exposed via the feature-flag Fetchr service and consumed through new Redux selectors. Deployments that override config via extended-config.js can set any of these to false to hide the corresponding UI features.
Made-with: Cursor
Description
Contribution Checklist:
Attach Screenshots (Optional)