Skip to content

fix(web/default): api-info save silently fails when color is invalid or result unchecked - #4823

Merged
Calcium-Ion merged 1 commit into
QuantumNous:mainfrom
Micah-Zheng:fix/api-info-save-validation
May 19, 2026
Merged

fix(web/default): api-info save silently fails when color is invalid or result unchecked#4823
Calcium-Ion merged 1 commit into
QuantumNous:mainfrom
Micah-Zheng:fix/api-info-save-validation

Conversation

@Micah-Zheng

@Micah-Zheng Micah-Zheng commented May 13, 2026

Copy link
Copy Markdown
Contributor

问题描述

系统管理 → 内容 → API 地址页面,添加条目后点击「保存设置」,提示保存成功,但刷新页面后新添加的条目消失。

根本原因

问题一:前端不检查后端返回值

handleSaveAll 使用 mutateAsync 但不检查返回的 success 字段:

// 修复前:无论后端返回 success: true 还是 false,都执行 setHasChanges(false) 和 toast.success
await updateOption.mutateAsync({ key: 'console_setting.api_info', value: ... })
setHasChanges(false)
toast.success(t('API info saved successfully'))  // 无条件显示成功

当后端返回 { success: false, message: ... } 时(HTTP 200),mutateAsync 不会 throw,catch 块不触发,用户看到保存成功但数据实际未写入。

问题二:slate 颜色不在后端白名单

前端颜色选项包含 slate,但后端 validColors 白名单不包含 slate,导致选择 slate 颜色时后端校验失败返回 success: false,触发上述问题一。

修复方案

  1. handleSaveAll 检查 result.success,只有成功时才清除 hasChanges 标记(错误提示已由 useUpdateOptiononSuccess 统一处理)
  2. 后端 validColors 白名单加入 slate,与前端颜色选项保持一致

影响范围

  • 仅影响 API 地址设置页面的保存逻辑
  • 不影响其他设置项

Signed-off-by: Micah-Zheng 102610064+Micah-Zheng@users.noreply.github.com

Summary by CodeRabbit

  • New Features

    • Added support for "slate" color in console settings.
  • Bug Fixes

    • Updated success notification handling for API settings updates.

Review Change Stack

Signed-off-by: Micah-Zheng <102610064+Micah-Zheng@users.noreply.github.com>
@coderabbitai

coderabbitai Bot commented May 13, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f5fe2161-ebf6-487e-9a90-9cbc472363b3

📥 Commits

Reviewing files that changed from the base of the PR and between aa56667 and 707eea4.

📒 Files selected for processing (2)
  • setting/console_setting/validation.go
  • web/default/src/features/system-settings/content/api-info-section.tsx

Walkthrough

The PR adds slate as a valid console color in validation logic and refactors the API settings save handler to conditionally reset state based on mutation success, removing its success toast notification in favor of error-only toast behavior.

Changes

Settings Updates

Layer / File(s) Summary
Console color validation
setting/console_setting/validation.go
The validColors map accepts "slate" as an additional valid console color value.
API settings save handler
web/default/src/features/system-settings/content/api-info-section.tsx
handleSaveAll now awaits the mutation result and only clears hasChanges when result.success is true, removing unconditional success handling and success toast emission.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A slate hue joins the color palette bright,
While save handlers now check success with care,
State resets only when the result is right,
Settings now whisper changes through the air!

🚥 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
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the primary fix: ensuring the API info save operation properly validates results and handles invalid colors, which directly addresses both root causes identified in the PR objectives.
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.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with 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.

❤️ Share

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

@Calcium-Ion Calcium-Ion left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Approving. Two correct fixes:

  1. handleSaveAll now checks result.success from mutateAsync before clearing dirty state and showing the success toast, so backend { success: false, ... } responses no longer silently look successful.
  2. Backend setting/console_setting/validation.go gains slate in validColors, matching the frontend color options (also added in #4824).

Recommend landing after #4824 so the frontend and backend slate additions ship together.

@Calcium-Ion
Calcium-Ion merged commit 032993e into QuantumNous:main May 19, 2026
2 checks passed
reggie-lula pushed a commit to WhaleCrane/new-api-hz that referenced this pull request May 21, 2026
…uantumNous#4823)

Signed-off-by: Micah-Zheng <102610064+Micah-Zheng@users.noreply.github.com>
xyfacai pushed a commit to xyfacai/new-api that referenced this pull request May 30, 2026
…uantumNous#4823)

Signed-off-by: Micah-Zheng <102610064+Micah-Zheng@users.noreply.github.com>
isboyjc added a commit to isboyjc/amux-api that referenced this pull request May 31, 2026
Sync upstream QuantumNous/new-api, backend parts only:
- QuantumNous#4871 (b397c58): add register_enabled/password_register_enabled to
  GetStatus so the login page can react to admin registration toggle
- QuantumNous#4823 (032993e): add "slate" to validColors so it is not rejected
Skipped the web/default (new UI) frontend parts of both commits.
SamuelSxy pushed a commit to SamuelSxy/new-api-rh that referenced this pull request Jun 7, 2026
…uantumNous#4823)

Signed-off-by: Micah-Zheng <102610064+Micah-Zheng@users.noreply.github.com>
fx247562340 pushed a commit to fx247562340/vancine-platform that referenced this pull request Jun 11, 2026
…uantumNous#4823)

Signed-off-by: Micah-Zheng <102610064+Micah-Zheng@users.noreply.github.com>
330079598 pushed a commit to 330079598/new-api that referenced this pull request Aug 19, 2026
…uantumNous#4823)

Signed-off-by: Micah-Zheng <102610064+Micah-Zheng@users.noreply.github.com>
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