move contingencies table to a tab in sa parameters - #1243
Conversation
|
Warning Review limit reached
Next review available in: 45 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (13)
📝 WalkthroughWalkthroughChangesThe security analysis form delegates contingency-table rendering to the selector. The selector now manages the expanded tab set, tab selection behavior, contingency-table props, and panel rendering. Security analysis tabs
Sequence Diagram(s)sequenceDiagram
participant SecurityAnalysisParametersForm
participant SecurityAnalysisParametersSelector
participant ContingencyTable
participant fetchContingencyCount
SecurityAnalysisParametersForm->>SecurityAnalysisParametersSelector: pass contingency props
SecurityAnalysisParametersSelector->>ContingencyTable: render Contingencies panel
ContingencyTable->>fetchContingencyCount: fetch contingency count
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/features/parameters/security-analysis/security-analysis-parameters-selector.tsx (1)
70-110: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDecouple tab values from array indices.
Relying on the array
.mapindex for theTabandTabPanelvalues is fragile. If a tab is ever filtered out ofvisibleTabs(e.g., viadeveloperModeOnly), the remaining array indices will shift and immediately desync from yourTabValuesenum.Additionally, evaluating the active
tabValueinside theTabPanelchildren means every mapped panel processes the same active condition, needlessly creating React element objects for the active tab inside every panel iteration.Use the tab's enum value directly to determine its identity, value, and rendered content.
♻️ Proposed refactor
- <Tabs value={tabValue} onChange={handleTabChange}> - {visibleTabs.map( - (tab, index) => + <Tabs value={tabValue} onChange={handleTabChange}> + {visibleTabs.map((tab) => { + const tabEnumValue = TabValues[tab.label as keyof typeof TabValues]; + return ( (tab.label !== TabValues[TabValues.LimitReductions] || (currentProvider === PARAM_PROVIDER_OPENLOADFLOW && params?.limitReductions)) && ( <Tab key={tab.label} label={<FormattedMessage id={tab.label} />} - value={index} + value={tabEnumValue} sx={{ fontSize: 17, fontWeight: 'bold', }} /> ) - )} + ); + })} </Tabs> - {visibleTabs.map((tab, index) => ( - <TabPanel key={tab.label} value={tabValue} index={index}> - {tabValue === TabValues.Contingencies && ( + {visibleTabs.map((tab) => { + const tabEnumValue = TabValues[tab.label as keyof typeof TabValues]; + return ( + <TabPanel key={tab.label} value={tabValue} index={tabEnumValue}> + {tabEnumValue === TabValues.Contingencies && ( <ContingencyTable name={CONTINGENCY_LISTS_INFOS} showContingencyCount={showContingencyCount} fetchContingencyCount={fetchContingencyCount} isBuiltCurrentNode={isBuiltCurrentNode} ref={contingencyTableApiRef} /> )} - {tabValue === TabValues.General && <ViolationsHidingParameters />} + {tabEnumValue === TabValues.General && <ViolationsHidingParameters />} - {tabValue === TabValues.LimitReductions && + {tabEnumValue === TabValues.LimitReductions && currentProvider === PARAM_PROVIDER_OPENLOADFLOW && params?.limitReductions && ( <Grid sx={{ width: '100%' }}> <LimitReductionsTableForm limits={params?.limitReductions ?? defaultLimitReductions} /> </Grid> )} </TabPanel> - ))} + ); + })}🤖 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/features/parameters/security-analysis/security-analysis-parameters-selector.tsx` around lines 70 - 110, Update the tab rendering in the security-analysis parameters selector to use each tab’s enum value rather than the visibleTabs map index for Tab value, TabPanel index, and active-content selection. Render each tab’s content based on that tab identity, preserving the existing visibility conditions and component behavior while preventing filtered tabs from desynchronizing the active value.
🤖 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/features/parameters/security-analysis/security-analysis-parameters-selector.tsx`:
- Line 13: Convert the UUID imports to type-only imports so the Node.js crypto
module is erased from client-side bundles: update
src/features/parameters/security-analysis/security-analysis-parameters-selector.tsx
lines 13-13 and
src/features/parameters/security-analysis/security-analysis-parameters-form.tsx
lines 9-9, preserving their type annotation usage.
---
Outside diff comments:
In
`@src/features/parameters/security-analysis/security-analysis-parameters-selector.tsx`:
- Around line 70-110: Update the tab rendering in the security-analysis
parameters selector to use each tab’s enum value rather than the visibleTabs map
index for Tab value, TabPanel index, and active-content selection. Render each
tab’s content based on that tab identity, preserving the existing visibility
conditions and component behavior while preventing filtered tabs from
desynchronizing the active value.
🪄 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
Run ID: 098416aa-79c8-46c3-80ea-cda89e17478c
📒 Files selected for processing (3)
src/features/parameters/common/limitreductions/columns-definitions.tssrc/features/parameters/security-analysis/security-analysis-parameters-form.tsxsrc/features/parameters/security-analysis/security-analysis-parameters-selector.tsx
ccb37fd to
1e59a65
Compare
49f29b8 to
951229a
Compare
antoinebhs
left a comment
antoinebhs
left a comment
There was a problem hiding this comment.
LGTM
would like to make it consistent with other params #1247
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/features/parameters/common/limitreductions/columns-definitions.ts`:
- Around line 44-46: Add the missing Contingencies translation key to both the
English and French parameters translation definitions, using the existing
translation structure and appropriate localized labels. Ensure the
TabValues.Contingencies message ID resolves correctly for the TAB_INFO entry.
🪄 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
Run ID: 74ebc447-6a62-4396-8b3c-b97769d95cef
📒 Files selected for processing (5)
src/features/parameters/common/limitreductions/columns-definitions.tssrc/features/parameters/security-analysis/security-analysis-parameters-form.tsxsrc/features/parameters/security-analysis/security-analysis-parameters-selector.tsxsrc/translations/en/parameters.tssrc/translations/fr/parameters.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/features/parameters/security-analysis/security-analysis-parameters-form.tsx
- src/features/parameters/security-analysis/security-analysis-parameters-selector.tsx
| export const TAB_INFO = [ | ||
| { label: TabValues[TabValues.General], developerModeOnly: false }, | ||
| { label: TabValues[TabValues.Contingencies], developerModeOnly: false }, | ||
| { label: TabValues[TabValues.Aggravation], developerModeOnly: false }, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Add the missing Contingencies translation key.
TabValues[TabValues.Contingencies] produces the message ID Contingencies, but neither src/translations/en/parameters.ts nor src/translations/fr/parameters.ts defines that key. The new tab will therefore render a missing-translation fallback instead of its label.
🤖 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/features/parameters/common/limitreductions/columns-definitions.ts` around
lines 44 - 46, Add the missing Contingencies translation key to both the English
and French parameters translation definitions, using the existing translation
structure and appropriate localized labels. Ensure the TabValues.Contingencies
message ID resolves correctly for the TAB_INFO entry.
719bd5a to
0dee063
Compare
* adress reviews * keep contingencies tab mounted to avoid fetching data on every tab selection * Simplify AS tabs handling * Simplify AS tabs handling * Simplify AS tabs handling * Simplify AS tabs handling * Simplify AS tabs handling * Remove unnecessary Grid * Lint --------- Co-authored-by: Radouane Khouadri <redouane.khouadri_externe@rte-france.com>
|
2 participants
new SA params :