Skip to content

fix(auth): fix Discord OAuth mobile login and add password login/register toggle - #3250

Closed
aliceric27 wants to merge 5401 commits into
QuantumNous:mainfrom
aliceric27:feature/custom-settings
Closed

fix(auth): fix Discord OAuth mobile login and add password login/register toggle#3250
aliceric27 wants to merge 5401 commits into
QuantumNous:mainfrom
aliceric27:feature/custom-settings

Conversation

@aliceric27

@aliceric27 aliceric27 commented Mar 13, 2026

Copy link
Copy Markdown

This PR introduces password login/registration control settings, fixes Discord OAuth on mobile devices, and adds
optional Cloudflare Tunnel support to the Docker Compose configuration.

本 PR 新增密碼登入/註冊的開關設定、修復手機上的 Discord OAuth 登入問題,並在 Docker Compose 中加入可選的 Cloudflare Tunnel 支援。


Changes / 變更內容

✨ feat: Password Login / Register Toggle

  • Backend (controller/misc.go): Exposes password_login_enabled and password_register_enabled fields in the /api/status
    response, reading from common.PasswordLoginEnabled and common.PasswordRegisterEnabled.
  • Frontend (LoginForm.jsx, RegisterForm.jsx): Conditionally renders password login/register UI based on the above
    settings. When disabled, shows only OAuth options or a "login disabled" message.
  • 後端 (controller/misc.go):在 /api/status 回應中新增 password_login_enabled 和 password_register_enabled 欄位。
  • 前端 (LoginForm.jsx, RegisterForm.jsx):依據上述設定動態顯示或隱藏密碼登入/註冊區塊;停用時僅顯示 OAuth
    選項或「登入已停用」提示。

🐛 fix: Discord OAuth Mobile Login

  • web/src/helpers/api.js: Changed window.open() to window.location.href for the Discord OAuth redirect. window.open()
    is frequently blocked by mobile popup blockers or opens in a separate tab, causing the OAuth state session to be lost
    and leaving the user appearing not logged in after authorization.
  • web/src/helpers/api.js:將 Discord OAuth 的跳轉從 window.open() 改為 window.location.href。window.open()
    在手機上常被彈出視窗封鎖,或在新分頁完成登入後,原分頁仍顯示未登入狀態。

🔧 chore: Docker Compose & Environment

  • docker-compose.yml: Adds Cloudflare Tunnel service as a commented-out template with setup instructions. Default
    image restored to calciumion/new-api:latest. Placeholder credentials restored.
  • .env.example: Documents the TUNNEL_TOKEN environment variable required for Cloudflare Tunnel.
  • .gitignore: Adds .omc/ to ignore local OMC orchestration state files.
  • docker-compose.yml:將 Cloudflare Tunnel service 以註解方式附上說明,方便需要時啟用。image 恢復為上游預設值,預設憑證恢復為佔位符。
  • .env.example:新增 TUNNEL_TOKEN 說明,描述 Cloudflare Tunnel 的設定方式。
  • .gitignore:加入 .omc/,避免本地 OMC 狀態檔案被追蹤。

Testing / 測試

  • Discord OAuth login works on mobile (same-tab redirect, no popup blocker)
  • When password_login_enabled = false, password login UI is hidden; OAuth options shown
  • When password_register_enabled = false, registration button is hidden
  • Docker Compose starts correctly with default config
  • 手機 Discord OAuth 登入正常(同分頁 redirect,不被彈出視窗封鎖)
  • password_login_enabled = false 時,密碼登入 UI 隱藏,僅顯示 OAuth
  • password_register_enabled = false 時,註冊按鈕隱藏
  • Docker Compose 以預設設定可正常啟動

Summary by CodeRabbit

  • New Features

    • Password login and registration can be enabled or disabled independently; the UI adapts to those toggles.
  • Improvements

    • OAuth flows now open in the current tab instead of a popup.
    • Server status now reports password-login and password-register availability so the UI reflects configured toggles.
  • Chores

    • Added Cloudflare Tunnel configuration templates to examples and compose, and added .omc/ to .gitignore.

MUTED64 and others added 30 commits February 6, 2026 21:22
fix: /v1/chat/completions -> /v1/responses json_schema
将散落在多个文件中的预扣费/结算/退款逻辑抽象为统一的 BillingSession 生命周期管理:

- 新增 BillingSettler 接口 (relay/common/billing.go) 避免循环引用
- 新增 FundingSource 接口 + WalletFunding / SubscriptionFunding 实现 (service/funding_source.go)
- 新增 BillingSession 封装预扣/结算/退款原子操作 (service/billing_session.go)
- 新增 SettleBilling 统一结算辅助函数,替换各 handler 中的 quotaDelta 模式
- 重写 PreConsumeBilling 为 BillingSession 工厂入口
- controller/relay.go 退款守卫改用 BillingSession.Refund()

修复的 Bug:
- 令牌额度泄漏:PreConsumeTokenQuota 成功但 DecreaseUserQuota 失败时未回滚
- 订阅退款遗漏:FinalPreConsumedQuota=0 但 SubscriptionPreConsumed>0 时跳过退款
- 订阅多扣费:subConsume 强制为 1 但 FinalPreConsumedQuota 不同步
- 退款路径不统一:钱包/订阅退款逻辑现统一由 FundingSource.Refund 分派
- Settle 部分失败保护:新增 fundingSettled 标记,资金来源提交后
  令牌调整失败不再导致 Refund 误退已结算的资金
- 订阅多扣费修复:trySubscription 传 subConsume 而非 preConsumedQuota
  给 preConsume,保证三者(amount/preConsume/FinalPreConsumedQuota)一致
- 令牌回滚错误记录:preConsume 中 funding 失败时令牌回滚错误不再丢弃
- 移除钱包路径死代码:用户额度不足的 strings.Contains 匹配不可能命中
- WalletFunding.Refund 不重试:IncreaseUserQuota 非幂等,重试会多退
…e recharge card tabs

- Defaulting to subscriptions when available and avoiding initial flash when no plans exist.
- Adjust the wide-screen layout to place wallet and invite sections side by side, simplify the subscription header and controls, and add padding to prevent card borders from clipping.
- Update related i18n strings by adding the new tab label and removing the obsolete subscription blurb.
…iption-card-when-no-plans

✨ refactor(wallet): Top-up layout to embed subscription plans into the recharge card tabs
…-session

refactor: 抽象统一计费会话 BillingSession
Add a lightweight active-subscription check to skip subscription pre-consume when none exist, reducing unnecessary transactions and locks. In the subscription UI, disable subscription-first options when no active plan is available, show the effective fallback to wallet with a clear notice, and distinguish “invalidated” from “expired” states. Update i18n strings across supported locales to reflect the new messages and status labels.
Aligns the error variable types in the subscription-first path so that quota fallback checks use the correct NewAPIError.
This prevents build failures and preserves the intended wallet fallback when subscription pre-consume returns an insufficient quota error.
Routes quota alerts through a subscription-specific check when billing from subscriptions, preventing wallet-based thresholds from triggering false warnings.
Updates the notification settings description and localization keys to clarify that both wallet and subscription balances are monitored.
…n-quota-notify

🔔 feat: Add subscription-aware quota notifications and update UI copy
…-preference-fallback

✨ chore: Improve subscription billing fallback and UI states
…tumNous#2881)

当上游为 AWS Bedrock 时,message_delta 的 usage 可能缺少 input_tokens、
cache_creation_input_tokens、cache_read_input_tokens 等字段,导致与原生
Anthropic 格式不一致。从 message_start 积累的 claudeInfo 中补全这些字段后
重新序列化,确保客户端收到一致的 usage 格式。
Modified the formatUserLogs function to include a startIdx parameter, allowing for more flexible log ID assignment. Updated calls to this function in GetLogByTokenId and GetUserLogs to pass the appropriate starting index.
feat: add Codex channel disclaimer (i18n, OpenAI terms)
feat: Force beta=true parameter for Anthropic channel
feat(oauth): implement custom OAuth provider
fix: Claude stream block index/type transitions
fix: add paragraph breaks between reasoning summary chunks
# Conflicts:
#	service/openaicompat/chat_to_responses.go
…t-stream

feat: channel test with stream=true
…fo-input-token

fix: 使用openai兼容接口调用部分渠道在最终端点为claude原生端点下还是走了openai扣减input_token的逻辑
seefs001 and others added 16 commits March 9, 2026 00:12
…4f8a4248b0ab3b03ba703796ea3

fix: kling risk fail return openAIVideo error
…ride-beta-header-append

feat:support $keep_only_declared and deduped $append for header override
chore: update model lists for frequently used channels
- Comment out cloudflared service block with setup instructions
- Use image: new-api:local for local builds
- Restore SQL_DSN, TZ, POSTGRES_PASSWORD to default placeholder values
- Add TUNNEL_TOKEN entry to .env.example with description
- Restore image: calciumion/new-api:latest (local build is deployment-specific)
- Add .omc/ to .gitignore
@coderabbitai

coderabbitai Bot commented Mar 13, 2026

Copy link
Copy Markdown
Contributor

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

GetStatus adds password_login_enabled and password_register_enabled. Frontend gates login/register flows and may show OAuth-only or disabled messages; OAuth navigation now uses same-tab redirects. Cloudflare Tunnel example and TUNNEL_TOKEN added to env/docker templates; .omc/ added to .gitignore.

Changes

Cohort / File(s) Summary
Config & Docker
\.env.example, docker-compose.yml, \.gitignore
Adds commented Cloudflare Tunnel template and TUNNEL_TOKEN example in .env.example, a commented cloudflared service template in docker-compose.yml, and appends .omc/ to .gitignore.
Backend status endpoint
controller/misc.go
Adds password_login_enabled and password_register_enabled fields to the GetStatus response payload.
Frontend auth UI
web/src/components/auth/LoginForm.jsx, web/src/components/auth/RegisterForm.jsx
Introduces server-driven flags to gate email/password login and registration UI; conditionally renders email flows, OAuth-only flows, or “disabled” notices based on those flags.
Frontend OAuth navigation
web/src/helpers/api.js
Replaces window.open(...) popups with window.location.href = ... same-tab navigation for several OAuth providers.

Sequence Diagram(s)

sequenceDiagram
    participant Browser
    participant Frontend as "Web App (JS)"
    participant Backend as "API (GetStatus)"
    participant OAuth as "OAuth Provider"

    Browser->>Frontend: Load page
    Frontend->>Backend: GET /api/get_status
    Backend-->>Frontend: { ..., password_login_enabled, password_register_enabled }
    Frontend->>Browser: Render UI per flags
    alt User chooses OAuth
        Browser->>Frontend: Click OAuth button
        Frontend->>Browser: window.location.href = OAuthURL
        Browser->>OAuth: Redirect to provider
    else User chooses password (if enabled)
        Browser->>Frontend: Submit credentials
        Frontend->>Backend: POST /api/login
        Backend-->>Frontend: Auth result
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 I hopped through env and compose today,
I learned which doors the server lets stay,
If passwords sleep, OAuth leads the way,
Same-tab redirects make the path more gay,
A rabbit dances off to code and play.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the two main changes in the PR: fixing Discord OAuth mobile login and adding password login/register toggle. It directly reflects the core objectives and matches the substantial code changes across backend and frontend components.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
📝 Coding Plan
  • Generate coding plan for human review comments

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.

@aliceric27 aliceric27 changed the title Feature/custom settings fix(auth): fix Discord OAuth mobile login and add password login/register toggle Mar 13, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docker-compose.yml`:
- Around line 18-23: The comment about using Cloudflare Tunnel is incomplete
because the docker-compose service new-api still has a ports: 3000:3000 mapping;
update the docs to instruct users that for tunnel-only deployments they must
remove or comment out the ports: block (or replace it with expose: - "3000") on
the new-api service so the host port is not published, and mention doing this
alongside setting TUNNEL_TOKEN and uncommenting the cloudflared service.

In `@web/src/components/auth/LoginForm.jsx`:
- Around line 967-987: The current conditional in LoginForm.jsx that renders
only the "登录已停用" Card when !passwordLoginEnabled && !hasOAuthLoginOptions
creates a dead-end; update that branch to surface a registration CTA or perform
a redirect when registration is available (e.g. check your registration flag
like registrationEnabled / allowRegistration) instead of just showing the
message. Concretely, inside the block guarded by !passwordLoginEnabled &&
!hasOAuthLoginOptions replace or augment the Card content with a visible
Register button/link to "/register" (or call an existing renderRegisterCTA()
helper), or trigger a programmatic redirect to the registration route so
pre-auth users can reach the registration flow. Ensure this uses the same UI
pattern as renderEmailLoginForm()/renderOAuthOptions() so styling remains
consistent.

In `@web/src/components/auth/RegisterForm.jsx`:
- Around line 794-800: The branch that currently renders only the message when
!passwordRegisterEnabled && !hasOAuthRegisterOptions should instead provide a
way to reach the login flow; update the conditional in RegisterForm.jsx so that
when passwordRegisterEnabled is false and hasOAuthRegisterOptions is false you
either (a) render a login CTA (e.g., a Link or Button labeled "Go to
Login"/t('登录') that navigates to '/login') or (b) programmatically redirect to
'/login' using your router/navigation helper; adjust the branch around
passwordRegisterEnabled, hasOAuthRegisterOptions, showEmailRegister and the
renderOAuthOptions/renderEmailRegisterForm calls so the dead-end message
includes the CTA or performs the redirect.

In `@web/src/helpers/api.js`:
- Line 252: The GitHub, Linux DO, and custom OAuth click handlers still open
popups; change them to perform full-page redirects like the Discord flow so the
opener tab isn't left orphaned—update onGitHubOAuthClicked,
onLinuxDOOAuthClicked, and onCustomOAuthClicked to build the same authorize URL
they currently use for popups and assign it to window.location.href (preserving
client_id, redirect_uri, response_type, scope, state, etc.) so
OAuth2Callback.jsx and the callback routes in App.jsx receive the response in
the same tab; ensure any popup-specific cleanup code is removed and that
state/params are encoded the same way as the Discord implementation.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 81166fbb-d577-4ae1-8705-1227a8880589

📥 Commits

Reviewing files that changed from the base of the PR and between 4e1b05e and 924bad4.

📒 Files selected for processing (7)
  • .env.example
  • .gitignore
  • controller/misc.go
  • docker-compose.yml
  • web/src/components/auth/LoginForm.jsx
  • web/src/components/auth/RegisterForm.jsx
  • web/src/helpers/api.js

Comment thread docker-compose.yml
Comment thread web/src/components/auth/LoginForm.jsx Outdated
Comment thread web/src/components/auth/RegisterForm.jsx
Comment thread web/src/helpers/api.js
…irects

- docker-compose.yml: add step 4 to Cloudflare Tunnel instructions
  explaining that tunnel-only deployments must remove/replace the
  ports: 3000:3000 block with expose: - "3000"
- LoginForm.jsx: surface register CTA on the login-disabled dead-end
  screen when passwordRegisterEnabled && !self_use_mode_enabled
- RegisterForm.jsx: add login link on the register-disabled dead-end
  screen so users can reach the login flow
- api.js: convert onGitHubOAuthClicked, onLinuxDOOAuthClicked, and
  onCustomOAuthClicked from window.open() popups to window.location.href
  full-page redirects, consistent with the Discord OAuth flow
Comment thread web/src/components/auth/LoginForm.jsx Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@web/src/components/auth/LoginForm.jsx`:
- Around line 967-1000: The UI branch treats passkey-only setups as "登录已停用"
because the condition uses hasOAuthLoginOptions but excludes passkey; update the
conditional logic around passwordLoginEnabled to treat passkey as a valid
non-password login option (e.g. include passkeyEnabled in the check or create a
combined flag like hasNonPasswordLoginOptions = hasOAuthLoginOptions ||
passkeyEnabled) so that when passwordLoginEnabled is false but passkeyEnabled is
true the component calls renderOAuthOptions() or the appropriate passkey
renderer instead of showing the disabled card; adjust checks involving
renderOAuthOptions(), renderEmailLoginForm(), and showEmailLogin accordingly.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ce348ccb-78c5-4958-89a4-0af0bf2348cf

📥 Commits

Reviewing files that changed from the base of the PR and between 924bad4 and b144c62.

📒 Files selected for processing (4)
  • docker-compose.yml
  • web/src/components/auth/LoginForm.jsx
  • web/src/components/auth/RegisterForm.jsx
  • web/src/helpers/api.js
🚧 Files skipped from review as they are similar to previous changes (2)
  • web/src/helpers/api.js
  • docker-compose.yml

Comment thread web/src/components/auth/LoginForm.jsx Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 2

♻️ Duplicate comments (1)
web/src/components/auth/LoginForm.jsx (1)

967-1000: ⚠️ Potential issue | 🟠 Major

Passkey-only setups still incorrectly shown as "登录已停用".

The conditional logic at Line 967 uses hasOAuthLoginOptions, which excludes passkey. When password login is disabled and passkey is the only enabled method, users see the disabled screen instead of the passkey option.

The fix from the past review comment has not been applied. Include passkey in the non-password login options check:

💡 Proposed fix
   const hasOAuthLoginOptions = Boolean(
     status.github_oauth ||
       status.discord_oauth ||
       status.oidc_enabled ||
       status.wechat_login ||
       status.linuxdo_oauth ||
       status.telegram_oauth ||
       hasCustomOAuthProviders,
   );
+  const hasPasskeyLoginOption = Boolean(status.passkey_login && passkeySupported);
+  const hasAnyNonPasswordLoginOptions = hasOAuthLoginOptions || hasPasskeyLoginOption;
   const passwordLoginEnabled = status.password_login_enabled !== false;

Then update the conditional rendering:

-        {!passwordLoginEnabled && !hasOAuthLoginOptions
+        {!passwordLoginEnabled && !hasAnyNonPasswordLoginOptions
           ? (
             // ... disabled card
           )
-          : !passwordLoginEnabled && hasOAuthLoginOptions
+          : !passwordLoginEnabled && hasAnyNonPasswordLoginOptions
           ? renderOAuthOptions()
-          : showEmailLogin || !hasOAuthLoginOptions
+          : showEmailLogin || !hasAnyNonPasswordLoginOptions
           ? renderEmailLoginForm()
           : renderOAuthOptions()}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/src/components/auth/LoginForm.jsx` around lines 967 - 1000, The
conditional block treats "no password and hasOAuthLoginOptions" as the only
non-password path but excludes passkey-only setups; update the condition that
currently uses hasOAuthLoginOptions to also include the passkey flag (e.g.
hasOAuthLoginOptions || passkeyEnabled or introduce hasNonPasswordLoginOptions)
so when passwordLoginEnabled is false and passkey is available the code falls
through to renderOAuthOptions()/passkey UI instead of showing "登录已停用"; adjust
the ternary that references passwordLoginEnabled and hasOAuthLoginOptions
accordingly and ensure renderOAuthOptions() (or the passkey-render path) is
invoked when passkey is enabled.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@web/src/components/auth/RegisterForm.jsx`:
- Around line 723-735: The login link is currently only shown when
passwordLoginEnabled is true, hiding it when OAuth registration options exist;
update the conditional that renders the "已有账户? 登录" block to use
(passwordLoginEnabled || hasOAuthRegisterOptions) so the Link to '/login' is
available when either local password login or OAuth-based registration is
present; locate the JSX around the conditional that references
passwordLoginEnabled in RegisterForm.jsx and change that predicate accordingly
(matching the same pattern used in renderOAuthOptions()).
- Around line 545-557: The login link is currently only shown when
passwordLoginEnabled is true, which hides navigation to /login when only OAuth
is available; update the render condition in RegisterForm.jsx to show the login
link when either passwordLoginEnabled is true OR OAuth login is available (e.g.,
oauthEnabled or when oauthProviders.length > 0 — whichever variable is used in
this file). Locate the conditional that uses passwordLoginEnabled and change it
to use a logical OR with the OAuth flag/array so the block with the Text and
Link to '/login' always renders when any login method exists.

---

Duplicate comments:
In `@web/src/components/auth/LoginForm.jsx`:
- Around line 967-1000: The conditional block treats "no password and
hasOAuthLoginOptions" as the only non-password path but excludes passkey-only
setups; update the condition that currently uses hasOAuthLoginOptions to also
include the passkey flag (e.g. hasOAuthLoginOptions || passkeyEnabled or
introduce hasNonPasswordLoginOptions) so when passwordLoginEnabled is false and
passkey is available the code falls through to renderOAuthOptions()/passkey UI
instead of showing "登录已停用"; adjust the ternary that references
passwordLoginEnabled and hasOAuthLoginOptions accordingly and ensure
renderOAuthOptions() (or the passkey-render path) is invoked when passkey is
enabled.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 297f7545-df95-4fb1-92e0-8ff40b46692b

📥 Commits

Reviewing files that changed from the base of the PR and between b144c62 and 16230c4.

📒 Files selected for processing (2)
  • web/src/components/auth/LoginForm.jsx
  • web/src/components/auth/RegisterForm.jsx

Comment thread web/src/components/auth/RegisterForm.jsx Outdated
Comment thread web/src/components/auth/RegisterForm.jsx Outdated
aliceric27 added 2 commits March 14, 2026 11:21
When passwordLoginEnabled was false and no OAuth providers were
configured, the login page showed "登录已停用" even if passkey was
enabled. Include passkeyLoginAvailable in the conditional so passkey-only
setups correctly render the login UI instead of the disabled card.
The "已有账户?登录" link was gated by passwordLoginEnabled only, hiding
it when password login was disabled even if OAuth options existed on the
login page. Use (passwordLoginEnabled || hasOAuthRegisterOptions) so
users can navigate to /login whenever any login method is available.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
web/src/components/auth/LoginForm.jsx (1)

969-1002: Consider replacing the nested ternary with explicit branching for maintainability.

Behavior is correct, but this branch is getting hard to reason about and easy to break on future auth additions.

♻️ Refactor sketch
+  const renderLoginContent = () => {
+    if (!passwordLoginEnabled && !hasNonPasswordLoginOptions) {
+      return renderLoginDisabledCard();
+    }
+    if (!passwordLoginEnabled && hasNonPasswordLoginOptions) {
+      return renderOAuthOptions();
+    }
+    if (showEmailLogin || !hasOAuthLoginOptions) {
+      return renderEmailLoginForm();
+    }
+    return renderOAuthOptions();
+  };

@@
-        {!passwordLoginEnabled && !hasNonPasswordLoginOptions
-          ? ( ... )
-          : !passwordLoginEnabled && hasNonPasswordLoginOptions
-          ? renderOAuthOptions()
-          : showEmailLogin || !hasOAuthLoginOptions
-          ? renderEmailLoginForm()
-          : renderOAuthOptions()}
+        {renderLoginContent()}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/src/components/auth/LoginForm.jsx` around lines 969 - 1002, The nested
ternary inside the return block controlling login form rendering is hard to read
and maintain. Refactor the conditional logic in the render method of the
LoginForm component by replacing the nested ternaries with clear if-else
statements or switch-case blocks to explicitly branch based on the values of
passwordLoginEnabled, hasNonPasswordLoginOptions, showEmailLogin, and
hasOAuthLoginOptions. This will improve readability and prevent future mistakes
when modifying authentication options.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@web/src/components/auth/LoginForm.jsx`:
- Around line 969-1002: The nested ternary inside the return block controlling
login form rendering is hard to read and maintain. Refactor the conditional
logic in the render method of the LoginForm component by replacing the nested
ternaries with clear if-else statements or switch-case blocks to explicitly
branch based on the values of passwordLoginEnabled, hasNonPasswordLoginOptions,
showEmailLogin, and hasOAuthLoginOptions. This will improve readability and
prevent future mistakes when modifying authentication options.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c01d3c2a-eceb-4597-9854-925dee845bfb

📥 Commits

Reviewing files that changed from the base of the PR and between 16230c4 and 45d8bee.

📒 Files selected for processing (1)
  • web/src/components/auth/LoginForm.jsx

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
web/src/components/auth/RegisterForm.jsx (1)

545-557: Consider centralizing the login-CTA predicate to reduce drift.

The same condition is duplicated in two places. Defining a shared boolean (e.g., const showLoginLink = passwordLoginEnabled || hasOAuthRegisterOptions) will make future edits less error-prone.

Also applies to: 723-735

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/src/components/auth/RegisterForm.jsx` around lines 545 - 557, Introduce a
single shared boolean (e.g., showLoginLink) in the RegisterForm component and
use it wherever the current predicate appears instead of repeating
(passwordLoginEnabled || hasOAuthRegisterOptions); update both the block around
the existing snippet (lines shown) and the other occurrence mentioned (around
723-735) to reference showLoginLink, ensuring the variable is computed once near
the top of the component so future changes only require updating that single
symbol.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@web/src/components/auth/RegisterForm.jsx`:
- Around line 545-557: Introduce a single shared boolean (e.g., showLoginLink)
in the RegisterForm component and use it wherever the current predicate appears
instead of repeating (passwordLoginEnabled || hasOAuthRegisterOptions); update
both the block around the existing snippet (lines shown) and the other
occurrence mentioned (around 723-735) to reference showLoginLink, ensuring the
variable is computed once near the top of the component so future changes only
require updating that single symbol.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7c3e477f-faea-4235-b70a-299dbb3f88f3

📥 Commits

Reviewing files that changed from the base of the PR and between 45d8bee and bf84914.

📒 Files selected for processing (1)
  • web/src/components/auth/RegisterForm.jsx

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.