Skip to content

fix: allow clearing channel remark#4886

Open
yyhhyyyyyy wants to merge 1 commit into
QuantumNous:mainfrom
yyhhyyyyyy:fix/channel-remark-clear
Open

fix: allow clearing channel remark#4886
yyhhyyyyyy wants to merge 1 commit into
QuantumNous:mainfrom
yyhhyyyyyy:fix/channel-remark-clear

Conversation

@yyhhyyyyyy
Copy link
Copy Markdown
Contributor

@yyhhyyyyyy yyhhyyyyyy commented May 15, 2026

⚠️ 提交说明 / PR Notice

Important

  • 请提供人工撰写的简洁摘要,避免直接粘贴未经整理的 AI 输出。

📝 变更描述 / Description

(简述:做了什么?为什么这样改能生效?请基于你对代码逻辑的理解来写,避免粘贴未经整理的内容)
修复 new ui 编辑渠道时,清空渠道备注后无法保存的问题。

🚀 变更类型 / Type of change

  • 🐛 Bug 修复 (Bug fix) - 请关联对应 Issue,避免将设计取舍、理解偏差或预期不一致直接归类为 bug
  • ✨ 新功能 (New feature) - 重大特性建议先通过 Issue 沟通
  • ⚡ 性能优化 / 重构 (Refactor)
  • 📝 文档更新 (Documentation)

🔗 关联任务 / Related Issue

✅ 提交前检查项 / Checklist

  • 人工确认: 我已亲自整理并撰写此描述,没有直接粘贴未经处理的 AI 输出。
  • 非重复提交: 我已搜索现有的 IssuesPRs,确认不是重复提交。
  • Bug fix 说明: 若此 PR 标记为 Bug fix,我已提交或关联对应 Issue,且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。
  • 变更理解: 我已理解这些更改的工作原理及可能影响。
  • 范围聚焦: 本 PR 未包含任何与当前任务无关的代码改动。
  • 本地验证: 已在本地运行并通过测试或手动验证,维护者可以据此复核结果。
  • 安全合规: 代码中无敏感凭据,且符合项目代码规范。

📸 运行证明 / Proof of Work

(请在此粘贴截图、关键日志或测试报告,以证明变更生效)

iShot_2026-05-15_15.47.22.mp4

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • Fixed channel configuration form submission. Clearing base URL, organization, test model, tag, remarks, priority, and weight fields now properly saves these values as empty, instead of retaining previous entries.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 15, 2026

Walkthrough

This PR modifies the channel form's payload transformation function to properly handle field clearing. It changes numeric field coercion from logical OR to nullish coalescing operators and adds explicit empty string assignments for text fields, enabling the backend to clear previously set values during channel updates.

Changes

Form Payload Transformation

Layer / File(s) Summary
Numeric defaults and empty string assignments
web/default/src/features/channels/lib/channel-form.ts
transformFormDataToUpdatePayload switches priority and weight from || null to ?? 0 to preserve zero values, and explicitly assigns empty strings to text fields (base_url, openai_organization, test_model, tag, remark) when form values are empty, allowing the backend to clear these fields during updates.

🎯 2 (Simple) | ⏱️ ~8 minutes

A rabbit hops through the field form,
Where zeros now stand tall and clear,
And empty strings erase old marks—
The channel edits hold their power,
No more lost notes in the update! 🐰✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: allow clearing channel remark' directly addresses the primary bug fix: enabling users to clear and persist channel remarks in the new UI.
Linked Issues check ✅ Passed The code changes implement the fix for Issue #4874 by forcing nullable remark/text fields to empty strings instead of null, enabling the backend to persist field clearing.
Out of Scope Changes check ✅ Passed All changes are directly related to the bug fix objective; modifications to priority, weight, and other nullable fields are necessary to support clearing remark and other text fields via the update payload.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ 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.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
web/default/src/features/channels/lib/channel-form.ts (1)

481-492: ⚡ Quick win

Refactor to avoid redundant null-to-empty-string conversions.

The current logic converts empty strings to null (lines 481-485), then immediately converts several fields back to empty strings (lines 488-492). This pattern is inefficient and makes the code harder to understand.

Consider refactoring to either:

  1. Skip certain fields in the cleanup loop by maintaining an allowlist of fields that should remain as empty strings
  2. Set the explicit empty strings before the cleanup loop and exclude those fields from iteration
♻️ Proposed refactor option 1: Skip fields in cleanup loop
  // Clean up empty strings to null for optional fields
+ const keepEmptyStringFields = new Set([
+   'base_url', 'openai_organization', 'test_model', 'tag', 'remark',
+   'model_mapping', 'status_code_mapping', 'param_override', 'header_override'
+ ])
  Object.keys(payload).forEach((key) => {
-   if (payload[key as keyof typeof payload] === '') {
+   if (payload[key as keyof typeof payload] === '' && !keepEmptyStringFields.has(key)) {
      ;(payload as Record<string, unknown>)[key] = null
    }
  })

- // Send explicit empty strings for nullable fields so GORM updates can clear them.
- payload.base_url = formData.base_url || ''
- payload.openai_organization = formData.openai_organization || ''
- payload.test_model = formData.test_model || ''
- payload.tag = formData.tag || ''
- payload.remark = formData.remark || ''
- payload.model_mapping = formData.model_mapping || ''
- payload.status_code_mapping = formData.status_code_mapping || ''
- payload.param_override = formData.param_override || ''
- payload.header_override = formData.header_override || ''
🤖 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 `@web/default/src/features/channels/lib/channel-form.ts` around lines 481 -
492, The cleanup loop currently turns all empty-string values to null then you
immediately overwrite specific fields back to empty strings; fix by making an
allowlist of fields that should remain empty strings (e.g., "base_url",
"openai_organization", "test_model", "tag", "remark") and skip those keys inside
the Object.keys(payload).forEach loop, so other keys are converted to null while
those allowlisted keys are left untouched; after the loop, continue to set
payload.base_url = formData.base_url || '' etc. so the intent and order are
clear and the redundant conversions are removed.
🤖 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.

Inline comments:
In `@web/default/src/features/channels/lib/channel-form.ts`:
- Around line 460-461: The create payload transforms use the `|| null` pattern
which treats 0 as falsy; update uses `?? 0`, causing inconsistency for numeric
fields. In the create payload transformation (where `formData.priority || null`
and `formData.weight || null` are used), replace those with the nullish
coalescing operator `formData.priority ?? 0` and `formData.weight ?? 0` so both
create and update payloads consistently preserve explicit 0 values when building
the request.

---

Nitpick comments:
In `@web/default/src/features/channels/lib/channel-form.ts`:
- Around line 481-492: The cleanup loop currently turns all empty-string values
to null then you immediately overwrite specific fields back to empty strings;
fix by making an allowlist of fields that should remain empty strings (e.g.,
"base_url", "openai_organization", "test_model", "tag", "remark") and skip those
keys inside the Object.keys(payload).forEach loop, so other keys are converted
to null while those allowlisted keys are left untouched; after the loop,
continue to set payload.base_url = formData.base_url || '' etc. so the intent
and order are clear and the redundant conversions are removed.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d1d2a0c3-9f80-4cad-94d0-1ddaf6872c7f

📥 Commits

Reviewing files that changed from the base of the PR and between 18282e6 and 7359e4a.

📒 Files selected for processing (1)
  • web/default/src/features/channels/lib/channel-form.ts

Comment on lines +460 to +461
priority: formData.priority ?? 0,
weight: formData.weight ?? 0,
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.

⚠️ Potential issue | 🟠 Major | ⚖️ Poor tradeoff

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for priority/weight field usage and validation in backend or API contracts

rg -n -C3 --type=go 'priority|weight' --glob '*channel*' --glob '!*.md'

Repository: QuantumNous/new-api

Length of output: 34051


🏁 Script executed:

#!/bin/bash
# Look at the context around the field-clearing logic and verify if priority/weight changes are intentional
grep -n -A 10 -B 10 "lines 481-492\|base_url\|openai_organization" web/default/src/features/channels/lib/channel-form.ts | head -100

Repository: QuantumNous/new-api

Length of output: 3721


🏁 Script executed:

#!/bin/bash
# Check if create and update have different intended behaviors by looking at their signatures and comments
grep -n -B 5 "transformFormDataToCreatePayload\|transformFormDataToUpdatePayload" web/default/src/features/channels/lib/channel-form.ts | head -60

Repository: QuantumNous/new-api

Length of output: 385


🏁 Script executed:

#!/bin/bash
# Get the complete transformFormDataToUpdatePayload function to see the full priority/weight handling
sed -n '447,510p' web/default/src/features/channels/lib/channel-form.ts

Repository: QuantumNous/new-api

Length of output: 2323


🏁 Script executed:

#!/bin/bash
# Check if there are any comments or git history that explain the priority/weight handling difference
git log -p --follow -S "priority.*??" -- web/default/src/features/channels/lib/channel-form.ts | head -100

Repository: QuantumNous/new-api

Length of output: 45


🏁 Script executed:

#!/bin/bash
# Check the backend model to see if priority/weight have special update behavior
grep -n -A 5 -B 5 "priority\|weight" service/channel.go | grep -A 5 -B 5 "Update\|Save" | head -60

Repository: QuantumNous/new-api

Length of output: 45


🏁 Script executed:

#!/bin/bash
# Check if there are recent changes that explain this pattern by looking at file history
git log --oneline -20 -- web/default/src/features/channels/lib/channel-form.ts

Repository: QuantumNous/new-api

Length of output: 107


🏁 Script executed:

#!/bin/bash
# Look for any TODOs or comments around the priority/weight fields
grep -n -C 5 "TODO\|FIXME\|BUG\|NOTE" web/default/src/features/channels/lib/channel-form.ts | grep -C 5 "priority\|weight"

Repository: QuantumNous/new-api

Length of output: 45


🏁 Script executed:

#!/bin/bash
# Let's check the git diff for the recent "fix: allow clearing channel remark" commit
git show 7359e4a1 -- web/default/src/features/channels/lib/channel-form.ts | head -200

Repository: QuantumNous/new-api

Length of output: 7697


🏁 Script executed:

#!/bin/bash
# Check if priority/weight defaulting to 0 is intentional by looking at the field-clearing comment section
sed -n '480,505p' web/default/src/features/channels/lib/channel-form.ts

Repository: QuantumNous/new-api

Length of output: 1052


🏁 Script executed:

#!/bin/bash
# Verify the complete field cleanup logic to understand the intent
sed -n '475,510p' web/default/src/features/channels/lib/channel-form.ts

Repository: QuantumNous/new-api

Length of output: 1325


🏁 Script executed:

#!/bin/bash
# Check if priority and weight are intentionally excluded from the field-clearing pattern
# by comparing what fields get the explicit empty string treatment
echo "=== Fields with explicit empty string assignment (for clearing) ===" && \
sed -n '487,496p' web/default/src/features/channels/lib/channel-form.ts | grep "payload\." | sed 's/.*payload\.\([a-z_]*\).*/\1/' | sort

echo ""
echo "=== Fields in the initial payload assignment (before cleanup) ===" && \
sed -n '447,475p' web/default/src/features/channels/lib/channel-form.ts | grep ":" | grep -oE '[a-z_]+:' | sed 's/://' | sort

Repository: QuantumNous/new-api

Length of output: 525


Align create and update payload transformation for numeric fields.

Lines 412-413 and 460-461 use different operators for priority and weight:

  • Create: formData.priority || null (treats 0 as falsy)
  • Update: formData.priority ?? 0 (preserves 0)

This creates inconsistent backend payloads for the same user input. When a user sets priority to 0:

  • Creating a channel sends null
  • Updating a channel sends 0

If the backend distinguishes these values (e.g., null = "use default", 0 = "lowest priority"), this causes unexpected behavior. Align both functions by using ?? 0 for numeric fields in the create function.

🤖 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 `@web/default/src/features/channels/lib/channel-form.ts` around lines 460 -
461, The create payload transforms use the `|| null` pattern which treats 0 as
falsy; update uses `?? 0`, causing inconsistency for numeric fields. In the
create payload transformation (where `formData.priority || null` and
`formData.weight || null` are used), replace those with the nullish coalescing
operator `formData.priority ?? 0` and `formData.weight ?? 0` so both create and
update payloads consistently preserve explicit 0 values when building the
request.

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.

v1.0.0-rc.6 新版UI,备注删除不成功,旧版无此问题

1 participant