Skip to content

fix: Enhance the installation failure notification text#315

Merged
qiuzhiqian merged 1 commit intomasterfrom
fix_notify
Feb 5, 2026
Merged

fix: Enhance the installation failure notification text#315
qiuzhiqian merged 1 commit intomasterfrom
fix_notify

Conversation

@qiuzhiqian
Copy link

No description provided.

@github-actions
Copy link

github-actions bot commented Feb 5, 2026

CLA Assistant Lite bot:
提交邮箱中包含我们的合作伙伴,但您似乎并非合作伙伴的成员或对接人,请联系相关对接人将您添加至组织之中,或由其重新发起 Pull Request。
The commit email domain belongs to one of our partners, but it seems you are not yet a member of the current organization, please contact the contact person to add you to the organization or let them submit the Pull Request.

xml seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You can retrigger this bot by commenting recheck in this Pull Request

@deepin-ci-robot
Copy link

deepin pr auto review

这份代码审查主要针对 manager_upgrade.go 中的 preFailedHook 函数修改以及相关的本地化文件更新。以下是详细的审查意见:

1. 语法与逻辑审查

  • 逻辑清晰度:代码在处理错误通知时,引入了新的分支逻辑来区分“已备份”和“未备份”状态。整体逻辑是通顺的,但存在明显的代码重复。
    • 问题strings.Contains(errType, system.ErrorInsufficientSpace.String()) 分支和 else 分支(其他错误)中,处理“已备份”和“未备份”的逻辑几乎完全一致(仅仅是 msggettext.Tr 的内容不同)。
    • 建议:将“已备份”和“未备份”的处理逻辑提取出来,或者重构这部分代码,减少重复。例如,可以先根据备份状态决定 actionhints,再根据错误类型决定 msg

2. 代码质量审查

  • 代码重复:如上所述,hints 的定义和 sendNotify 的调用在代码中重复了多次。
    • 建议:可以将 hints 的定义提取为变量或常量。如果逻辑允许,可以将发送通知的流程封装成一个辅助函数。
  • 魔法值"reboot" 字符串在代码中出现了多次。
    • 建议:定义一个常量,例如 const actionReboot = "reboot",以提高可维护性。
  • 注释:代码中增加了中文注释(// 空间不足, // 已备份),这有助于理解,但建议保持代码风格统一,如果项目其他地方使用英文注释,这里也应统一。

3. 代码性能审查

  • 并发安全:代码中使用了 go m.sendNotify(...) 启动 goroutine。
    • 审查:这是合理的做法,因为发送通知通常不应阻塞主流程。
    • 注意:需确保 m.sendNotify 方法内部是并发安全的,或者 m 对象在调用该方法时不会发生竞态条件。从代码片段看,这似乎是现有的模式,暂未发现新引入的并发问题。

4. 代码安全审查

  • 本地化字符串拼接:代码直接使用 gettext.Tr 获取翻译后的字符串。
    • 审查:这部分看起来是安全的,因为翻译内容来自受控的 .pot.po 文件。
  • DBus 调用:在 hints 中硬编码了 DBus 调用命令:dbus-send,--session,--print-reply,--dest=org.deepin.dde.ShutdownFront1...
    • 风险:虽然这是通过通知系统的 hints 传递的,最终由前端(如 dde-session-ui)执行,但在后端代码中硬编码 DBus 调用命令字符串显得不够优雅且耦合度较高。
    • 建议:如果可能,建议通过定义明确的接口或常量来管理这些 Action,或者确保前端对这些命令有严格的校验。

5. 改进建议代码示例

针对代码重复问题,以下是一个重构建议:

func (m *Manager) preFailedHook(job *Job, mode system.UpdateType, uuid string) error {
    // ... 前面的代码 ...

    // 定义常量和变量
    const actionReboot = "reboot"
    var (
        msg    string
        action []string
        hints  map[string]dbus.Variant
    )

    // 辅助函数:设置已备份状态的通知参数
    setupBackedUpParams := func() {
        action = []string{actionReboot, gettext.Tr("Reboot")}
        hints = map[string]dbus.Variant{
            "x-deepin-action-" + actionReboot:      dbus.MakeVariant("dbus-send,--session,--print-reply,--dest=org.deepin.dde.ShutdownFront1,/org/deepin/dde/ShutdownFront1,org.deepin.dde.ShutdownFront1.Restart"),
            "x-deepin-NoAnimationActions":          dbus.MakeVariant(actionReboot),
        }
    }

    // 辅助函数:设置未备份状态的通知参数
    setupNotBackedUpParams := func() {
        action = []string{}
        hints = nil
    }

    // 根据错误类型和备份状态设置参数
    if strings.Contains(errType, system.ErrorInsufficientSpace.String()) {
        if m.statusManager.abStatus == system.HasBackedUp {
            msg = gettext.Tr("Updates failed: insufficient disk space. Please reboot to avoid the effect on your system.")
            setupBackedUpParams()
        } else {
            msg = gettext.Tr("Updates failed: insufficient disk space.")
            setupNotBackedUpParams()
        }
    } else {
        // 其他错误
        if m.statusManager.abStatus == system.HasBackedUp {
            msg = gettext.Tr("Updates failed. Please reboot to avoid the effect on your system.")
            setupBackedUpParams()
        } else {
            msg = gettext.Tr("Updates failed.")
            setupNotBackedUpParams()
        }
    }

    // 发送通知
    timeout := system.NotifyExpireTimeoutDefault
    if m.config.IntranetUpdate {
        timeout = system.NotifyExpireTimeoutPrivateLong
    }
    go m.sendNotify(updateNotifyShowOptional, 0, "preferences-system", "", msg, action, hints, timeout)

    // ... 后面的代码 ...
}

总结

这次修改主要目的是在更新失败且系统已备份的情况下,引导用户重启以恢复系统。逻辑上是正确的,但代码实现上存在较多重复。建议进行适当重构以提高可读性和可维护性。同时,本地化文件的更新与代码逻辑保持一致。

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: qiuzhiqian, zhaohuiw42

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@qiuzhiqian qiuzhiqian merged commit 047fc95 into master Feb 5, 2026
23 of 28 checks passed
@qiuzhiqian qiuzhiqian deleted the fix_notify branch February 5, 2026 02:32
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.

3 participants