sync: from linuxdeepin/dde-session-shell

[deepin-ci-robot]

Synchronize source files from linuxdeepin/dde-session-shell.

Source-pull-request: linuxdeepin/dde-session-shell#60

Summary by Sourcery

Enhancements:

• Use org.deepin.dde.control-center as the notification app identifier when DSS snipe support is enabled, while preserving the existing behavior otherwise.

[sourcery-ai]

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Adjusts the notification parameters used by UpdateWorker::doUpdate based on the ENABLE_DSS_SNIPE build flag to align with upstream dde-session-shell behavior.

Sequence diagram for UpdateWorker doUpdate notification parameters

sequenceDiagram
    participant UpdateWorker
    participant NotificationsService as org_freedesktop_Notifications

    UpdateWorker->>NotificationsService: Notify(app_name, replaces_id, app_icon, summary, body, actions, hints, expire_timeout)

    alt ENABLE_DSS_SNIPE disabled
        Note over UpdateWorker,NotificationsService: Build without ENABLE_DSS_SNIPE
        UpdateWorker->>NotificationsService: Notify(app_name = Update, replaces_id = 0, app_icon = package-updated-failed, summary = , body = Please_plug_in_and_then_install_updates, actions = [], hints = {}, expire_timeout = default)
    else ENABLE_DSS_SNIPE enabled
        Note over UpdateWorker,NotificationsService: Build with ENABLE_DSS_SNIPE
        UpdateWorker->>NotificationsService: Notify(app_name = org.deepin.dde.control-center, replaces_id = 0, app_icon = , summary = , body = Please_plug_in_and_then_install_updates, actions = [], hints = {}, expire_timeout = default)
    end

Loading

Class diagram for UpdateWorker doUpdate change

classDiagram
    class UpdateWorker {
        +doUpdate(powerOff bool) void
    }

Loading

File-Level Changes

Change	Details	Files
Gate notification metadata arguments in UpdateWorker::doUpdate on ENABLE_DSS_SNIPE to use different app id and icon for the low-power update warning.	Wrap the initial Notify() arguments in an ENABLE_DSS_SNIPE preprocessor conditional. For non-ENABLE_DSS_SNIPE builds, keep using translated "Update" summary with icon "package-updated-failed" and numeric id 0. For ENABLE_DSS_SNIPE builds, switch to app id "org.deepin.dde.control-center", keep numeric id 0, and pass an empty icon string before the rest of the existing notification arguments.	src/dde-lock/updateworker.cpp

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Hey - I've left some high level feedback:

• Consider moving the shared .arg(static_cast<uint>(0)) call outside the #ifndef/#else block so the conditional compilation only wraps the differing arguments, which will make the code easier to read and maintain.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- Consider moving the shared `.arg(static_cast<uint>(0))` call outside the `#ifndef/#else` block so the conditional compilation only wraps the differing arguments, which will make the code easier to read and maintain.

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[deepin-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: deepin-ci-robot, yixinshark

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

Details

Needs approval from an approver in each of these files:

• debian/deepin/OWNERS

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

[deepin-ci-robot]

deepin pr auto review

这段代码是一个 git diff 输出，主要涉及对 UpdateWorker::doUpdate 函数中 D-Bus 通知调用参数的修改。以下是对该代码变更的审查意见：

1. 语法逻辑

• 审查结果：通过。
• 分析：代码使用了预处理器宏 #ifndef ENABLE_DSS_SNIPE 来区分不同构建配置下的参数。这是 C/C++ 中标准的条件编译语法，逻辑上是正确的。它确保了在不同配置下，D-Bus 方法调用 Notify 能接收到正确数量和类型的参数。

2. 代码质量

• 审查结果：良好，但有改进空间。
• 分析：
  • 可读性：使用宏定义来区分不同版本的功能是常见的做法，但过多的条件编译可能会降低代码的可读性。建议在宏定义处添加注释，说明 ENABLE_DSS_SNIPE 的具体含义或应用场景（例如：是否用于特定硬件或特定发行版）。
  • 硬编码字符串：org.deepin.dde.control-center 和 package-updated-failed 是硬编码的字符串。虽然这在 D-Bus 调用中很常见，但如果这些字符串在其他地方也有使用，建议将其提取为静态常量或定义在头文件中，以便于统一管理和修改。
  • 空字符串：代码中使用了 QString() 和 QString("")。在 Qt 中，QString() 默认构造即为空字符串，性能略优于 QString("")（后者涉及构造临时对象再赋值），虽然在这里影响微乎其微，但保持风格统一更好。

3. 代码性能

• 审查结果：无明显影响。
• 分析：这段代码主要涉及 D-Bus 异步调用，性能瓶颈主要在于 IPC 通信本身，而不是参数构造。tr() 函数用于国际化，QString 的构造开销在 D-Bus 调用的开销面前可以忽略不计。因此，这部分修改不会对程序性能产生负面影响。

4. 代码安全

• 审查结果：安全。
• 分析：
  • 版权年份：将版权年份从 2022 更新至 2026 是合规操作，符合开源许可证维护的要求。
  • D-Bus 参数注入：这里传入的参数都是字面量或通过 tr() 处理的固定字符串，没有直接拼接用户输入，因此不存在 D-Bus 参数注入或格式化字符串的风险。
  • 空指针/崩溃风险：QString() 的使用是安全的，不会导致空指针解引用。

改进建议

1. 添加注释：
   建议在 #ifndef ENABLE_DSS_SNIPE 附近添加注释，解释为什么需要更改应用名称和图标。例如：

   // 在特定项目配置下，通知需要显示为控制中心，且不使用默认图标
   #ifndef ENABLE_DSS_SNIPE

2. 统一空字符串风格：
   原代码中混用了 QString() 和 QString("")。建议统一使用 QString()，因为它是构造空字符串的规范写法，且意图更明确（构造一个空对象，而非构造一个包含空字符的对象）。

3. 提取常量（可选）：
   如果 org.deepin.dde.control-center 或 package-updated-failed 在代码库中多次出现，建议定义在类的头文件或公共常量文件中：

   // 在头文件中
   static const constexpr char* const CONTROL_CENTER_APP_ID = "org.deepin.dde.control-center";

修改后的代码示例（结合建议）：

// SPDX-FileCopyrightText: 2022 - 2026 UnionTech Software Technology Co., Ltd.
//
// SPDX-License-Identifier: GPL-3.0-or-later

// ... (其他代码)

void UpdateWorker::doUpdate(bool powerOff)
{
    // ... (前置代码)

    QDBusMessage::createMethodCall("org.freedesktop.Notifications",
            "/org/freedesktop/Notifications",
            "org.freedesktop.Notifications",
            "Notify")
#ifndef ENABLE_DSS_SNIPE
        // 标准模式：使用默认应用名称和更新失败图标
        .arg(tr("Update"))
        .arg(static_cast<uint>(0))
        .arg("package-updated-failed")
#else
        // 特定模式：使用控制中心作为应用名称，且不显示图标
        .arg("org.deepin.dde.control-center")
        .arg(static_cast<uint>(0))
        .arg(QString()) // 统一使用 QString() 表示无图标
#endif
        .arg(QString()) // 统一使用 QString() 代替 ""
        .arg(tr("Please plug in and then install updates."))
        .arg(QStringList());
    // ... (后续代码)
}

总体而言，这段代码的修改是为了适配不同的构建环境，逻辑清晰，没有明显的安全隐患。采纳上述关于注释和代码风格的建议后，代码的可维护性将得到进一步提升。