bito-code-review[bot] commented on code in PR #40466:
URL: https://github.com/apache/superset/pull/40466#discussion_r3325685823
##########
superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/OAuth2ClientField.tsx:
##########
@@ -42,22 +46,51 @@ export const OAuth2ClientField = ({
changeMethods,
db,
default_value: defaultValue,
+ isPublic = true,
}: FieldPropTypes) => {
- const encryptedExtra = JSON.parse(db?.masked_encrypted_extra || '{}');
- const [oauth2ClientInfo, setOauth2ClientInfo] = useState<OAuth2ClientInfo>({
- id: encryptedExtra.oauth2_client_info?.id || '',
- secret: encryptedExtra.oauth2_client_info?.secret || '',
- authorization_request_uri:
- encryptedExtra.oauth2_client_info?.authorization_request_uri ||
- defaultValue?.authorization_request_uri ||
- '',
- token_request_uri:
- encryptedExtra.oauth2_client_info?.token_request_uri ||
- defaultValue?.token_request_uri ||
- '',
- scope:
- encryptedExtra.oauth2_client_info?.scope || defaultValue?.scope || '',
- });
+ const deriveOauth2ClientInfo = (): OAuth2ClientInfo => {
+ // `masked_encrypted_extra` is user/backend-supplied and historically
+ // sometimes the string "null" — JSON.parse('null') returns null, and
+ // malformed JSON throws. Defend against both so a single bad value
+ // can't crash the component.
+ let parsed: unknown;
+ try {
+ parsed = JSON.parse(db?.masked_encrypted_extra || '{}');
+ } catch {
+ parsed = {};
+ }
+ const encryptedExtra =
+ parsed && typeof parsed === 'object' && !Array.isArray(parsed)
+ ? (parsed as { oauth2_client_info?: Partial<OAuth2ClientInfo> })
+ : {};
+ const info = encryptedExtra.oauth2_client_info;
+ return {
+ id: info?.id || '',
+ secret: info?.secret || '',
+ authorization_request_uri:
+ info?.authorization_request_uri ||
+ defaultValue?.authorization_request_uri ||
+ '',
+ token_request_uri:
+ info?.token_request_uri || defaultValue?.token_request_uri || '',
+ scope: info?.scope || defaultValue?.scope || '',
+ };
+ };
+
+ const [oauth2ClientInfo, setOauth2ClientInfo] = useState<OAuth2ClientInfo>(
+ deriveOauth2ClientInfo,
+ );
+
+ // Re-sync local state when masked_encrypted_extra changes (e.g., when the
+ // gsheets dropdown toggles back to private after we cleared stored creds).
+ useEffect(() => {
+ setOauth2ClientInfo(deriveOauth2ClientInfo());
+ // eslint-disable-next-line react-hooks/exhaustive-deps -- depend only on
the serialized DB-side credentials
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>ESLint rule definition not found for react-hooks</b></div>
<div id="fix">
The `react-hooks/exhaustive-deps` ESLint rule is not recognized. Ensure the
`eslint-plugin-react-hooks` package is installed and properly configured in
your ESLint setup. This rule is essential for catching dependency array issues
in React hooks.
</div>
</div>
<small><i>Code Review Run #04e6ea</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
