Fix rules-of-hooks violation in getFilePicker

[KethniiImasha]

Purpose

Fixes the react-hooks/rules-of-hooks ESLint violation in getFilePicker function inside form-fields-helper.tsx by replacing the useState hook with a plain variable.

Related Issues

• fixes [React Doctor] rules-of-hooks: React Hook useState is called in function getFilePicker that is neither a... (1 occurrence) product-is#27958

Related PRs

• N/A

Checklist

• e2e cypress tests locally verified. (for internal contributers)
• Manual test round performed and verified.
• UX/UI review done on the final implementation.
• Documentation provided. (Add links if there are any)
• Relevant backend changes deployed and verified
• Unit tests provided. (Add links if there are any)
• Integration tests provided. (Add links if there are any)

Security checks

• Followed secure coding standards in http://wso2.com/technical-reports/wso2-secure-engineering-guidelines
• Ran FindSecurityBugs plugin and verified report
• Confirmed that this PR doesn't commit any keys, passwords, tokens, usernames, or other secrets

Developer Checklist (Mandatory)

• Complete the Developer Checklist in the related product-is issue to track any behavioral change or migration impact.

Screen.Recording.mov

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR fixes a React rules-of-hooks violation in the getFilePicker helper function. The useState import was removed, and the helper now derives its data value directly from eachProp?.value instead of maintaining component-level state, eliminating the hook violation and simplifying the callback flow.

Changes

Hook violation fix in getFilePicker

Layer / File(s)	Summary
Remove local state and useState hook from getFilePicker features/admin.connections.v1/components/edit/forms/helpers/form-fields-helper.tsx, .changeset/solid-pigs-laugh.md	React import updated to remove useState; getFilePicker refactored to compute data directly from eachProp?.value instead of maintaining internal state, Pkcs12FileField handler no longer calls setData, and a changeset documents the patch fix.

Possibly related issues:

• #27958 — rules-of-hooks violation in getFilePicker (this PR removes useState from the helper to address that report).

🚥 Pre-merge checks | ✅ 6

✅ Passed checks (6 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main change: fixing a rules-of-hooks violation in the getFilePicker function, which matches the core objective of the pull request.
Linked Issues check	✅ Passed	The code changes directly address the rules-of-hooks violation by removing useState from getFilePicker, which was the primary objective of issue #27958.
Out of Scope Changes check	✅ Passed	All changes are within scope: the changeset documents a patch fix, and the code changes remove the problematic useState hook and adjust the data flow in getFilePicker accordingly.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Changeset Required	✅ Passed	Changeset file .changeset/solid-pigs-laugh.md exists in PR with @wso2is/admin.connections.v1 marked as patch version, meeting the requirement for at least one valid changeset.
Description check	✅ Passed	The pull request description includes the Purpose, Related Issues, Related PRs, and both checklists (general and developer). Most sections are adequately filled, with a security check explicitly marked as completed.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

• 🛠️ create changeset

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Caution

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

⚠️ Outside diff range comments (1)

features/admin.connections.v1/components/edit/forms/helpers/form-fields-helper.tsx (1)

593-596: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Critical bug: callback ignores newData and passes stale value.

The onCertificateChange callback receives newData as its parameter but ignores it, passing the stale data constant instead. Since data is derived once from the initial eachProp?.value, it will always contain the original certificate value, not the newly selected one. This breaks the certificate change notification flow.

🐛 Proposed fix

                    <Pkcs12FileField
                        onCertificateChange={ (newData: string) =>
                        {
-                           onCertificateChange(data);
+                           onCertificateChange(newData);
                        } }
                    />

🤖 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
`@features/admin.connections.v1/components/edit/forms/helpers/form-fields-helper.tsx`
around lines 593 - 596, The callback passed to the onCertificateChange prop is
using the stale local variable `data` instead of the new value parameter
`newData`, so update the JSX handler (the anonymous arrow passed to
onCertificateChange) to forward `newData` to the parent handler (call
onCertificateChange(newData) instead of onCertificateChange(data)); locate the
occurrence in the form-fields helper where onCertificateChange is assigned and
replace the argument accordingly so the new certificate value is propagated.

🤖 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
`@features/admin.connections.v1/components/edit/forms/helpers/form-fields-helper.tsx`:
- Around line 593-596: The callback passed to the onCertificateChange prop is
using the stale local variable `data` instead of the new value parameter
`newData`, so update the JSX handler (the anonymous arrow passed to
onCertificateChange) to forward `newData` to the parent handler (call
onCertificateChange(newData) instead of onCertificateChange(data)); locate the
occurrence in the form-fields helper where onCertificateChange is assigned and
replace the argument accordingly so the new certificate value is propagated.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: eab31155-fafe-40ea-8ec7-68686f062374

📥 Commits

Reviewing files that changed from the base of the PR and between 60793a9 and e624daa.

📒 Files selected for processing (2)

• .changeset/solid-pigs-laugh.md
• features/admin.connections.v1/components/edit/forms/helpers/form-fields-helper.tsx

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.79%. Comparing base (3b0beb0) to head (ff52f4f).
⚠️ Report is 224 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master   #10370   +/-   ##
=======================================
  Coverage   98.79%   98.79%           
=======================================
  Files         165      165           
  Lines       51763    51764    +1     
  Branches      165      165           
=======================================
+ Hits        51137    51138    +1     
  Misses        626      626           

Flag	Coverage Δ
@wso2is/core	58.95% <ø> (ø)
@wso2is/i18n	100.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.
see 3 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[pavinduLakshan]

Were you able to validate this fix?

This component seems to be used when submitting the certificate while editing IDP settings. Since the onCertificateChange method no longer updates the data variable with the changes in this PR, this could cause issues when the certificate is submitted.

PS: just noticed coderabbit has also pointed this out. see #10370 (review)

[KethniiImasha]

Were you able to validate this fix?

This component seems to be used when submitting the certificate while editing IDP settings. Since the onCertificateChange method no longer updates the data variable with the changes in this PR, this could cause issues when the certificate is submitted.

PS: just noticed coderabbit has also pointed this out. see #10370 (review)

Hi @pavinduLakshan, you're right, removing useState means data is no longer reactive, which could break the certificate submission flow.

I think the cleanest fix would be to convert getFilePicker into a proper React component by renaming it to GetFilePicker and updating the call site at line 764 accordingly. This keeps useState valid without cascading into a large refactor. Would you be okay with that approach?

[pavinduLakshan]

I think the cleanest fix would be to convert getFilePicker into a proper React component by renaming it to GetFilePicker and updating the call site at line 764 accordingly. This keeps useState valid without cascading into a large refactor. Would you be okay with that approach?

please see the suggested solution in the bot comment. any reason why we can't proceed with just that?

[KethniiImasha]

I think the cleanest fix would be to convert getFilePicker into a proper React component by renaming it to GetFilePicker and updating the call site at line 764 accordingly. This keeps useState valid without cascading into a large refactor. Would you be okay with that approach?

please see the suggested solution in the bot comment. any reason why we can't proceed with just that?

Got it! I'll proceed with CodeRabbit's suggested fix and update onCertificateChange(data) to onCertificateChange(newData). Thanks!

[KethniiImasha]

Hi @pavinduLakshan, I've addressed the earlier feedback. Happy to make any further changes if needed...thanks!

[pavinduLakshan]

Hi @pavinduLakshan, I've addressed the earlier feedback. Happy to make any further changes if needed...thanks!

Thanks, could you please add a screen recording of the functionality of the certificate picker to the PR description as well?

[KethniiImasha]

Hi @pavinduLakshan, I've updated the PR description with a screen recording demonstrating the Private Key field (certificate picker) in the Google Outbound Provisioning connector. The value updates correctly after save and page reload.