Fix third-party binding cards showing incorrect status when disabled - #1900
Fix third-party binding cards showing incorrect status when disabled#1900RedwindA wants to merge 2 commits into
Conversation
WalkthroughRefactors AccountManagement.jsx to centralize provider-binding UI logic: derives WeChat flags, adds helper functions for status/button text and generic provider rendering, and updates WeChat, GitHub, OIDC, Telegram, and LinuxDO sections to use these helpers. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as AccountManagement.jsx
participant State as status / userState
User->>UI: Open Account Management
UI->>State: Read provider configs and user bindings
Note over UI,State: derive wechatEnabled, isWeChatBound
UI->>UI: getBindingStatusText(enabled, bound)
UI->>UI: getBindingButtonText(enabled, bound)
UI->>UI: renderProviderBinding(enabled, accountId, label)
alt Provider enabled
UI->>User: Show account info / bind/unbind button
else Provider disabled
UI->>User: Show "未启用"
end
Note right of UI: WeChat button disabled state uses wechatEnabled
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
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: 4
🧹 Nitpick comments (2)
web/src/components/settings/personal/cards/AccountManagement.jsx (2)
89-108: Helpers are correct; consider hoisting to module scope.Pure helpers recreate every render. Hoist them outside the component (no props capture) to avoid churn and ease reuse across providers.
96-102: Optional: make button text helper reusable for OAuth providers.Add a
modifiable = trueflag so bound-but-not-editable providers show “已绑定” instead of “修改绑定”, and reuse everywhere for consistency.-const getBindingButtonText = (enabled, bound) => { - if (bound) { - return t('修改绑定'); - } - return enabled ? t('绑定') : t('未启用'); -}; +const getBindingButtonText = (enabled, bound, modifiable = true) => { + if (!enabled) return t('未启用'); + if (bound) return modifiable ? t('修改绑定') : t('已绑定'); + return t('绑定'); +};Example (GitHub) button label:
- {status.github_oauth ? t('绑定') : t('未启用')} + {getBindingButtonText(Boolean(status.github_oauth), Boolean(userState.user?.github_id), false)}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
web/src/components/settings/personal/cards/AccountManagement.jsx(8 hunks)
🔇 Additional comments (2)
web/src/components/settings/personal/cards/AccountManagement.jsx (2)
191-204: WeChat status/button now respect enablement and bind state.This meets the PR goal (“未启用” when disabled; “已/未绑定” otherwise) and disables actions when not enabled.
62-63: Confirmstatus.wechat_loginis the only WeChat enable flag. Verified thatstatus.wechat_loginis mapped fromcommon.WeChatAuthEnabledincontroller/misc.goand no otherwechat_oauthorwechat_enabledflags exist.
| {renderProviderBinding( | ||
| status.github_oauth, | ||
| userState.user?.github_id, | ||
| t('GitHub ID'), | ||
| )} |
There was a problem hiding this comment.
Guard against undefined IDs in button disable logic (GitHub).
Elsewhere you compare to '', which disables the button if the field is undefined. Use a boolean check.
Apply inline change near the button:
- disabled={
- (userState.user && userState.user.github_id !== '') ||
- !status.github_oauth
- }
+ disabled={Boolean(userState.user?.github_id) || !status.github_oauth}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In web/src/components/settings/personal/cards/AccountManagement.jsx around lines
224 to 228, the button disable logic currently compares the GitHub ID to an
empty string which fails when the ID is undefined; change the condition to a
boolean check so the button is disabled when the ID is falsy (e.g. use
!userState.user?.github_id or Boolean(userState.user?.github_id) accordingly)
and apply this inline where the button's disabled prop is set.
| {renderProviderBinding( | ||
| status.oidc_enabled, | ||
| userState.user?.oidc_id, | ||
| t('OIDC ID'), | ||
| )} |
There was a problem hiding this comment.
Same undefined check for OIDC button.
- disabled={
- (userState.user && userState.user.oidc_id !== '') ||
- !status.oidc_enabled
- }
+ disabled={Boolean(userState.user?.oidc_id) || !status.oidc_enabled}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In web/src/components/settings/personal/cards/AccountManagement.jsx around lines
266 to 270, the OIDC provider binding button is rendered without the same
undefined/user existence guard used for other provider buttons; update the code
to only call renderProviderBinding when status.oidc_enabled is true and
userState.user exists and userState.user.oidc_id is defined (mirror the same
conditional check pattern used for the other provider buttons) so the component
isn’t invoked with an undefined ID.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/src/components/settings/personal/cards/AccountManagement.jsx (1)
62-118: Replace direct string checks and unguarded user ID accesses with Boolean() + optional chaining
– InAccountManagement.jsx, the provider buttons still use patterns like(userState.user && userState.user.github_id !== '') userState.user.telegram_id !== '' (userState.user && userState.user.linux_do_id !== '')– Update each to use
Boolean(userState.user?.<provider>_id)instead, matching thewechatEnabled/isWeChatBoundpattern and removing fragile!== ''checks.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
web/src/components/settings/personal/cards/AccountManagement.jsx(8 hunks)
🔇 Additional comments (6)
web/src/components/settings/personal/cards/AccountManagement.jsx (6)
62-63: LGTM on normalized WeChat flags.Booleans read cleanly and align with the PR goal.
201-214: WeChat block: good use of helpers and disable logic.Status/label now consistent and button disables only when integration is off.
234-238: Fix undefined-ID check in GitHub button; consider unifying label via helper.
userState.user.github_id !== ''incorrectly disables when the ID isundefined. Use a boolean cast. Optionally, reusegetBindingButtonTextfor label consistency.- disabled={ - (userState.user && userState.user.github_id !== '') || - !status.github_oauth - } + disabled={ + Boolean(userState.user?.github_id) || !status.github_oauth + }Optional label unification:
- {status.github_oauth ? t('绑定') : t('未启用')} + {getBindingButtonText( + Boolean(status.github_oauth), + Boolean(userState.user?.github_id) + )}Also applies to: 251-256
276-280: Same undefined-ID guard and label consistency for OIDC.Mirror the GitHub fix.
- disabled={ - (userState.user && userState.user.oidc_id !== '') || - !status.oidc_enabled - } + disabled={ + Boolean(userState.user?.oidc_id) || !status.oidc_enabled + }Optional:
- {status.oidc_enabled ? t('绑定') : t('未启用')} + {getBindingButtonText( + Boolean(status.oidc_enabled), + Boolean(userState.user?.oidc_id) + )}Also applies to: 295-301
321-325: Telegram block can throw when userState.user is null.Direct access to
userState.user.telegram_idrisks a crash; use optional chaining + boolean.- {status.telegram_oauth ? ( - userState.user.telegram_id !== '' ? ( + {status.telegram_oauth ? ( + Boolean(userState.user?.telegram_id) ? ( <Button disabled={true} size='small'> {t('已绑定')} </Button> ) : (Also applies to: 330-347
367-371: Same undefined-ID guard and label consistency for LinuxDO.Mirror the GitHub/OIDC fixes.
- disabled={ - (userState.user && userState.user.linux_do_id !== '') || - !status.linuxdo_oauth - } + disabled={ + Boolean(userState.user?.linux_do_id) || !status.linuxdo_oauth + }Optional:
- {status.linuxdo_oauth ? t('绑定') : t('未启用')} + {getBindingButtonText( + Boolean(status.linuxdo_oauth), + Boolean(userState.user?.linux_do_id) + )}Also applies to: 383-389
| const getBindingStatusText = (enabled, bound) => { | ||
| if (bound) { | ||
| return t('已绑定'); | ||
| } | ||
| if (!enabled) { | ||
| return t('未启用'); | ||
| } | ||
| return t('未绑定'); | ||
| }; |
There was a problem hiding this comment.
Make “未启用” take precedence in status text.
When a provider is disabled, show “未启用” even if the user had bound previously (matches PR intent).
Apply:
- const getBindingStatusText = (enabled, bound) => {
- if (bound) {
- return t('已绑定');
- }
- if (!enabled) {
- return t('未启用');
- }
- return t('未绑定');
- };
+ const getBindingStatusText = (enabled, bound) => {
+ if (!enabled) return t('未启用');
+ return bound ? t('已绑定') : t('未绑定');
+ };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const getBindingStatusText = (enabled, bound) => { | |
| if (bound) { | |
| return t('已绑定'); | |
| } | |
| if (!enabled) { | |
| return t('未启用'); | |
| } | |
| return t('未绑定'); | |
| }; | |
| const getBindingStatusText = (enabled, bound) => { | |
| if (!enabled) return t('未启用'); | |
| return bound ? t('已绑定') : t('未绑定'); | |
| }; |
🤖 Prompt for AI Agents
In web/src/components/settings/personal/cards/AccountManagement.jsx around lines
89 to 97, the status text currently returns '已绑定' before checking whether the
provider is enabled; change the logic so the "not enabled" case takes precedence
by checking if (!enabled) first and returning t('未启用'), then check bound to
return t('已绑定') and finally return t('未绑定') as the fallback.
| const getBindingButtonText = (enabled, bound) => { | ||
| if (bound) { | ||
| return t('修改绑定'); | ||
| } | ||
| return enabled ? t('绑定') : t('未启用'); | ||
| }; | ||
|
|
There was a problem hiding this comment.
Button label should also prioritize “未启用”.
Avoid showing “修改绑定/绑定” when disabled.
- const getBindingButtonText = (enabled, bound) => {
- if (bound) {
- return t('修改绑定');
- }
- return enabled ? t('绑定') : t('未启用');
- };
+ const getBindingButtonText = (enabled, bound) => {
+ if (!enabled) return t('未启用');
+ return bound ? t('修改绑定') : t('绑定');
+ };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const getBindingButtonText = (enabled, bound) => { | |
| if (bound) { | |
| return t('修改绑定'); | |
| } | |
| return enabled ? t('绑定') : t('未启用'); | |
| }; | |
| const getBindingButtonText = (enabled, bound) => { | |
| if (!enabled) return t('未启用'); | |
| return bound ? t('修改绑定') : t('绑定'); | |
| }; |
🤖 Prompt for AI Agents
In web/src/components/settings/personal/cards/AccountManagement.jsx around lines
99 to 105, the getBindingButtonText currently returns '修改绑定' when bound even if
the feature is disabled; change the logic to first check if enabled is false and
return t('未启用'), otherwise if bound return t('修改绑定'), else return t('绑定') so the
button never shows bind/modify when the feature is disabled.
| const renderProviderBinding = (enabled, accountId, label) => { | ||
| const hasAccountId = Boolean(accountId && accountId !== ''); | ||
|
|
||
| if (hasAccountId) { | ||
| return renderAccountInfo(accountId, label); | ||
| } | ||
|
|
||
| if (!enabled) { | ||
| return <span className='text-gray-500'>{t('未启用')}</span>; | ||
| } | ||
|
|
||
| return renderAccountInfo(accountId, label); | ||
| }; |
There was a problem hiding this comment.
renderProviderBinding should show “未启用” even if bound.
Current check renders the account ID when disabled; flip the order.
const renderProviderBinding = (enabled, accountId, label) => {
const hasAccountId = Boolean(accountId && accountId !== '');
-
- if (hasAccountId) {
- return renderAccountInfo(accountId, label);
- }
-
- if (!enabled) {
- return <span className='text-gray-500'>{t('未启用')}</span>;
- }
-
- return renderAccountInfo(accountId, label);
+ if (!enabled) {
+ return <span className='text-gray-500'>{t('未启用')}</span>;
+ }
+ return renderAccountInfo(accountId, label);
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const renderProviderBinding = (enabled, accountId, label) => { | |
| const hasAccountId = Boolean(accountId && accountId !== ''); | |
| if (hasAccountId) { | |
| return renderAccountInfo(accountId, label); | |
| } | |
| if (!enabled) { | |
| return <span className='text-gray-500'>{t('未启用')}</span>; | |
| } | |
| return renderAccountInfo(accountId, label); | |
| }; | |
| const renderProviderBinding = (enabled, accountId, label) => { | |
| const hasAccountId = Boolean(accountId && accountId !== ''); | |
| if (!enabled) { | |
| return <span className='text-gray-500'>{t('未启用')}</span>; | |
| } | |
| return renderAccountInfo(accountId, label); | |
| }; |
🤖 Prompt for AI Agents
In web/src/components/settings/personal/cards/AccountManagement.jsx around lines
106 to 118, the rendering logic currently checks for accountId before enabled;
change the order so that if enabled is false you immediately return the "未启用"
span (even if accountId exists), otherwise proceed to check hasAccountId and
renderAccountInfo when present, falling back to your existing empty/default
rendering when enabled but no accountId. Ensure the enabled-check is evaluated
first so disabled providers always show "未启用".
1 participant
PR 类型
PR 是否包含破坏性更新？
PR 描述
provider enable flag and the user’s binding state
when rendering status and button text
providers show
未启用when the administrator hasdisabled the integration instead of reporting
未绑 定enablement checks to avoid misleading actions
效果
Before
After
Summary by CodeRabbit
Bug Fixes
Refactor