Skip to content

[FIX] Force trigger fetching contingency count from button action (using API Ref)#1140

Open
thangqp wants to merge 6 commits into
mainfrom
fix_api_force_refresh_contingency_count_from_buttons
Open

[FIX] Force trigger fetching contingency count from button action (using API Ref)#1140
thangqp wants to merge 6 commits into
mainfrom
fix_api_force_refresh_contingency_count_from_buttons

Conversation

@thangqp
Copy link
Copy Markdown
Contributor

@thangqp thangqp commented May 21, 2026

PR Summary

Expose an API Ref for ContingencyTable to control the trigger from an ancestor parent.

thangqp added 2 commits May 20, 2026 20:29
Signed-off-by: Thang PHAM <phamthang37@gmail.com>
…ing API Ref)

Signed-off-by: Thang PHAM <phamthang37@gmail.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 21, 2026

Review Change Stack

📝 Walkthrough

Walkthrough

Adds an imperative ref API to ContingencyTable (reset and refresh methods), exports formal types, and threads the ref through SecurityAnalysisParametersForm and SecurityAnalysisParametersInline to coordinate contingency count resets and refreshes.

Changes

ContingencyTable imperative ref API

Layer / File(s) Summary
ContingencyTable ref API definition and implementation
src/components/parameters/common/contingency-table/contingency-table.tsx
New exported types ContingencyTableApi and ContingencyTableProps. Component refactored to internal ContingencyTableWithApiRef and exported via forwardRef, exposing resetSimulatedContingencyCount() and triggerContingencyCountRefresh() through useImperativeHandle. columnsDefinition useMemo relocated inside the internal component.
SecurityAnalysisParametersForm ref acceptance
src/components/parameters/security-analysis/security-analysis-parameters-form.tsx
Adds and exports SecurityAnalysisParametersFormProps with optional contingencyTableApiRef?: ForwardedRef<ContingencyTableApi>; component now accepts the typed prop and forwards it into ContingencyTable via ref.
SecurityAnalysisParametersInline orchestration
src/components/parameters/security-analysis/security-analysis-parameters-inline.tsx
Creates contingencyTableApiRef via useRef<ContingencyTableApi>, passes it to SecurityAnalysisParametersForm, calls resetSimulatedContingencyCount() after parameter resets, and calls triggerContingencyCountRefresh() after loading new parameters.

Suggested reviewers

  • ayolab
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: exposing an API Ref for ContingencyTable to force trigger contingency count fetching from parent components.
Description check ✅ Passed The description directly relates to the changeset, explaining that an API Ref is exposed to allow parent components to trigger contingency count refresh.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/components/parameters/common/contingency-table/contingency-table.tsx (1)

37-40: 💤 Low value

Consider using React 19's "ref as prop" pattern instead of forwardRef.

React 19 supports ref as a regular prop, making forwardRef unnecessary. This would simplify the component definition.

♻️ Optional simplification using React 19 ref-as-prop
-function ContingencyTableWithApiRef(
-    { name, showContingencyCount = false, fetchContingencyCount, isBuiltCurrentNode }: Readonly<ContingencyTableProps>,
-    apiRef: ForwardedRef<ContingencyTableApi>
-) {
+export function ContingencyTable({
+    name,
+    showContingencyCount = false,
+    fetchContingencyCount,
+    isBuiltCurrentNode,
+    ref: apiRef,
+}: Readonly<ContingencyTableProps & { ref?: ForwardedRef<ContingencyTableApi> }>) {

And remove the export wrapper:

-export const ContingencyTable = forwardRef(ContingencyTableWithApiRef);

Also applies to: 191-191

🤖 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/components/parameters/common/contingency-table/contingency-table.tsx`
around lines 37 - 40, The component currently uses forwardRef; update it to
React 19's "ref as prop" pattern by changing the function signature of
ContingencyTableWithApiRef to accept apiRef as a regular prop (extend
ContingencyTableProps to include apiRef?: ContingencyTableApi or appropriate
type) instead of the second ForwardedRef parameter, remove any ForwardedRef type
imports/usage, and delete the external forwardRef export wrapper (the export
that wraps ContingencyTableWithApiRef with forwardRef); keep internal logic the
same but reference apiRef from props. Ensure types (ContingencyTableProps and
ContingencyTableApi) are adjusted/imported accordingly and any callers/export
remain compatible with the new prop-based ref.
🤖 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.

Nitpick comments:
In `@src/components/parameters/common/contingency-table/contingency-table.tsx`:
- Around line 37-40: The component currently uses forwardRef; update it to React
19's "ref as prop" pattern by changing the function signature of
ContingencyTableWithApiRef to accept apiRef as a regular prop (extend
ContingencyTableProps to include apiRef?: ContingencyTableApi or appropriate
type) instead of the second ForwardedRef parameter, remove any ForwardedRef type
imports/usage, and delete the external forwardRef export wrapper (the export
that wraps ContingencyTableWithApiRef with forwardRef); keep internal logic the
same but reference apiRef from props. Ensure types (ContingencyTableProps and
ContingencyTableApi) are adjusted/imported accordingly and any callers/export
remain compatible with the new prop-based ref.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b2316059-e915-4c9c-8c7a-cb332491c6e1

📥 Commits

Reviewing files that changed from the base of the PR and between dd38611 and 2ef7fe4.

📒 Files selected for processing (3)
  • src/components/parameters/common/contingency-table/contingency-table.tsx
  • src/components/parameters/security-analysis/security-analysis-parameters-form.tsx
  • src/components/parameters/security-analysis/security-analysis-parameters-inline.tsx

@thangqp thangqp changed the title Fix api force refresh contingency count from buttons (using API Ref) [FIX] Force trigger fetching contingency count from button action (using API Ref) May 21, 2026
@sonarqubecloud
Copy link
Copy Markdown

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/components/parameters/common/contingency-table/contingency-table.tsx (1)

100-120: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Guard optional fetchContingencyCount before chaining promise handlers.

At Line 100, fetchContingencyCount?.(...) can evaluate to undefined, and Line 106 then calls .then(...) unconditionally. That can throw at runtime when showContingencyCount is true but no fetcher is passed.

💡 Proposed fix
+        if (!fetchContingencyCount) {
+            setIsLoading(false);
+            setSimulatedContingencyCount(null);
+            return () => {};
+        }
+
-        fetchContingencyCount?.(
+        fetchContingencyCount(
             contingencyListsInfos
                 .filter((lists) => lists[ACTIVATED])
                 .flatMap((lists) => lists[CONTINGENCY_LISTS]?.map((contingencyList) => contingencyList[ID])),
             abortSignal
         )
🤖 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/components/parameters/common/contingency-table/contingency-table.tsx`
around lines 100 - 120, The code calls fetchContingencyCount?.(...) but then
unconditionally chains .then/.catch which can throw if fetchContingencyCount is
undefined; change this to guard the chain by first capturing the result of
fetchContingencyCount (e.g. const p = fetchContingencyCount?.(...)) and only
call p.then(...).catch(...) when p is defined, otherwise
setSimulatedContingencyCount(null)/setIsLoading(false) or skip the fetch flow;
update the existing handlers that call setSimulatedContingencyCount,
setIsLoading and snackWithFallback so they remain in the same callbacks and
preserve the abortSignal/IGNORE_SIGNAL check used in the current catch block.
🤖 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.

Outside diff comments:
In `@src/components/parameters/common/contingency-table/contingency-table.tsx`:
- Around line 100-120: The code calls fetchContingencyCount?.(...) but then
unconditionally chains .then/.catch which can throw if fetchContingencyCount is
undefined; change this to guard the chain by first capturing the result of
fetchContingencyCount (e.g. const p = fetchContingencyCount?.(...)) and only
call p.then(...).catch(...) when p is defined, otherwise
setSimulatedContingencyCount(null)/setIsLoading(false) or skip the fetch flow;
update the existing handlers that call setSimulatedContingencyCount,
setIsLoading and snackWithFallback so they remain in the same callbacks and
preserve the abortSignal/IGNORE_SIGNAL check used in the current catch block.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 64433207-23a4-46b8-9589-b55058c84bf2

📥 Commits

Reviewing files that changed from the base of the PR and between 2ef7fe4 and a15fb8f.

📒 Files selected for processing (3)
  • src/components/parameters/common/contingency-table/contingency-table.tsx
  • src/components/parameters/security-analysis/security-analysis-parameters-form.tsx
  • src/components/parameters/security-analysis/security-analysis-parameters-inline.tsx

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants