Skip to content

fix: system email setting #7746

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
fit2cloudwxx merged 1 commit into from
Nov 9, 2026
Merged

fix: system email setting #7746

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion ui/src/views/system/settings/email/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,12 @@ const rules: FormRules<EmailSettingPayload> = {
function loadEmailSetting() {
loading.value = true
return EmailSettingApi.getEmailSetting()
.then((setting) => Object.assign(emailSetting, defaultEmailSetting, setting))
.then((setting) => {
// 兼容接口返回数字端口，统一使用字符串进行回显、校验和提交。
Object.assign(emailSetting, defaultEmailSetting, setting, {
email_port: String(setting.email_port ?? ''),
})
})
.finally(() => {
loading.value = false
})

@shaohuzhang1 shaohuzhang1 Nov 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The intent is reasonable, but the change introduces a few behavioral risks.

Potential issues

  1. setting may be null/undefined

    setting.email_port

    The old code passed setting directly to Object.assign, where null/undefined sources are ignored. The new code will throw if the API returns no setting object.

    • Use setting?.email_port and setting ?? {}.

  2. Default email_port can be lost

    email_port: String(setting.email_port ?? '')

    If setting does not contain email_port, this always overwrites the value from defaultEmailSetting with ''.

    • If the default port should be preserved, fall back to defaultEmailSetting.email_port.

  3. The promise return value changed
    The old arrow function returned:

    Object.assign(emailSetting, defaultEmailSetting, setting)

    The new block-bodied arrow function returns undefined:

    .then((setting) => {
  Object.assign(...)
})

    If any caller relies on loadEmailSetting().then(...) receiving emailSetting, that behavior is now broken.

    • Restore return Object.assign(...) if the return value matters.

  4. Normalization only happens on load
    The comment says the port is unified as a string for display, validation, and submission, but this diff only normalizes on load. Make sure the submit payload is also converted consistently. If the API expects a number on submit, converting everything to a string could break the request.

  5. String(...) is permissive
    String will produce "NaN", "true", or "[object Object]" for unexpected values. Validation should reject these, or the normalization should be stricter.

Suggested patch

.then((setting) => {
  const source = setting ?? {}
  const port = source.email_port ?? defaultEmailSetting.email_port ?? ''

  return Object.assign(emailSetting, defaultEmailSetting, source, {
    email_port: String(port),
  })
})

If null from the API is intended to mean “clear the port” rather than “use the default,” adjust the fallback logic accordingly:

const port = source.email_port === undefined
  ? defaultEmailSetting.email_port
  : source.email_port

email_port: port == null ? '' : String(port)

Minor optimization

Using async/await would make the loading/error flow clearer, but it is not required:

async function loadEmailSetting() {
  loading.value = true
  try {
    const setting = await EmailSettingApi.getEmailSetting()
    const source = setting ?? {}
    const port = source.email_port ?? defaultEmailSetting.email_port ?? ''

    return Object.assign(emailSetting, defaultEmailSetting, source, {
      email_port: String(port),
    })
  } finally {
    loading.value = false
  }
}

Overall: no syntax problem, but check the null safety, default-port fallback, and the changed promise return value.

Expand Down
Loading