fix: support string list in notification hints #1135

18202781743 · 2025-05-28T07:26:02Z

Added LIST_VALUE_SEGMENT constant for string list serialization
Modified convertHintsToString to handle QStringList values by joining
with separator
Updated parseHint to reconstruct QStringList from serialized format
Improved action handling in NotificationManager to properly process
string list hints
Added deprecation warning for old string format hints

These changes were necessary to properly support complex hint values
in notifications, particularly for action parameters that may contain
multiple values. The previous implementation only supported simple
string values, which limited functionality.

fix: 支持通知提示中的字符串列表

添加 LIST_VALUE_SEGMENT 常量用于字符串列表序列化
修改 convertHintsToString 方法以处理 QStringList 值并使用分隔符连接
更新 parseHint 方法从序列化格式重建 QStringList
改进 NotificationManager 中的动作处理以正确处理字符串列表提示
为旧的字符串格式提示添加弃用警告

这些变更是为了在通知中正确支持复杂的提示值，特别是可能包含多个值的动作参
数。之前的实现仅支持简单字符串值，限制了功能。

pms: BUG-317649

Summary by Sourcery

Support and process QStringList values in notification hints by enhancing serialization/deserialization and action handling, and emit deprecation warnings for legacy formats.

New Features:

Enable serialization and reconstruction of QStringList as hint values in notifications.

Enhancements:

Add LIST_VALUE_SEGMENT delimiter for serializing list hints.
Extend convertHintsToString and parseHint to handle list-based hint values.
Update NotificationManager to process list-formatted action hints and warn on deprecated comma-separated formats.

1. Added LIST_VALUE_SEGMENT constant for string list serialization 2. Modified convertHintsToString to handle QStringList values by joining with separator 3. Updated parseHint to reconstruct QStringList from serialized format 4. Improved action handling in NotificationManager to properly process string list hints 5. Added deprecation warning for old string format hints These changes were necessary to properly support complex hint values in notifications, particularly for action parameters that may contain multiple values. The previous implementation only supported simple string values, which limited functionality. fix: 支持通知提示中的字符串列表 1. 添加 LIST_VALUE_SEGMENT 常量用于字符串列表序列化 2. 修改 convertHintsToString 方法以处理 QStringList 值并使用分隔符连接 3. 更新 parseHint 方法从序列化格式重建 QStringList 4. 改进 NotificationManager 中的动作处理以正确处理字符串列表提示 5. 为旧的字符串格式提示添加弃用警告这些变更是为了在通知中正确支持复杂的提示值，特别是可能包含多个值的动作参数。之前的实现仅支持简单字符串值，限制了功能。 pms: BUG-317649

deepin-ci-robot · 2025-05-28T07:26:22Z

deepin pr auto review

代码审查意见：

代码重复：
- 在NotifyEntity::convertHintsToString和NotifyEntity::parseHint函数中，QString value;的声明被重复了，建议将其移动到函数的开头，减少代码重复。
类型转换：
- 在NotifyEntity::parseHint函数中，QVariant::fromValue(listValue)将QStringList转换为QVariant，但QVariant的typeId()可能不会返回QMetaType::QStringList，因为QVariant的typeId()返回的是QMetaType::User。建议使用QVariant::type()来检查类型。
日志信息：
- 在NotificationManager::doActionInvoked函数中，当遇到不期望的hint格式时，使用了qDebug输出日志。建议使用qWarning或qCritical来输出警告或错误信息，以便于问题追踪。
字符串分割：
- 在NotificationManager::doActionInvoked函数中，当i.value().typeId() == QMetaType::QStringList时，直接将i.value().toString()转换为QStringList，这可能会导致类型不匹配的问题。建议先检查i.value()的类型，再进行转换。
代码可读性：
- 在NotificationManager::doActionInvoked函数中，qDebug输出的日志信息中，actionId和value的值应该使用<<操作符进行格式化输出，以提高日志的可读性。
错误处理：
- 在NotificationManager::doActionInvoked函数中，当args.isEmpty()时，没有进行任何处理。建议添加错误处理逻辑，例如记录日志或抛出异常。
注释：
- 在NotificationManager::doActionInvoked函数中，注释// 命令应该使用中文注释，以保持代码的一致性。

综合以上意见，建议对代码进行如下修改：

// 修改1：将QString value的声明移动到函数开头
QString value;

// 修改2：使用QVariant::type()来检查类型
if (it.value().type() == QVariant::StringList) {
    QStringList tmp = it.value().toStringList();
    value = tmp.join(LIST_VALUE_SEGMENT);
} else {
    value = it.value().toString();
}

// 修改3：使用qWarning输出警告信息
qWarning(notifyLog) << "Deprecate hint format, use string list instead of string."
                    << "actionId:" << actionId << ", value:" << i.value();

// 修改4：先检查类型再进行转换
if (i.value().type() == QVariant::StringList) {
    args = i.value().toStringList();
} else {
    args = i.value().toString().split(",");
}

// 修改5：使用<<操作符格式化输出日志
qDebug(notifyLog) << "Action invoked:" << actionId << ", args:" << args;

// 修改6：添加错误处理逻辑
if (args.isEmpty()) {
    qWarning(notifyLog) << "Empty args for action:" << actionId;
    // 或者抛出异常
}

// 修改7：使用中文注释
// 命令

sourcery-ai · 2025-05-28T07:27:19Z

Reviewer's Guide

This PR introduces a dedicated serialization segment for QStringList in notification hints, updates serialization/deserialization methods accordingly, and extends NotificationManager action handling to support list-based hint values while deprecating the old string format.

Class diagram for updated notification components

classDiagram
    class NotifyEntity {
        +convertHintsToString(const QVariantMap &map): QString
        +parseHint(const QString &hint): QVariantMap
    }
    class NotificationManager {
        +doActionInvoked(const NotifyEntity &entity, const QString &actionId): void
    }

Flow diagram for updated `parseHint` logic

graph TD
    A[Start parseHint] --> B[Split hint string by HINT_SEGMENT];
    B -- For each part --> C[Split part by KEY_VALUE_SEGMENT];
    C -- list.size() != 2 --> B;
    C -- list.size() == 2 --> D{Split value_string by LIST_VALUE_SEGMENT};
    D -- resultingList.size() > 1 --> E[parsed_value = QStringList from resultingList];
    D -- resultingList.size() <= 1 --> F[parsed_value = original value_string];
    E --> G[Insert key, parsed_value into map];
    F --> G;
    G --> B;
    B -- End of iteration --> H[Return QVariantMap];
    H --> I[End];

File-Level Changes

Change	Details	Files
Introduce dedicated serialization for string list hints	Add LIST_VALUE_SEGMENT constant Handle QStringList in convertHintsToString by joining segments Reconstruct QStringList in parseHint from serialized data	`panels/notification/common/notifyentity.cpp`
Extend NotificationManager to support list-based action hints	Branch on hint type to extract QStringList or fallback Split list values without relying on comma Log deprecation warning for old string format	`panels/notification/server/notificationmanager.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 @18202781743 - I've reviewed your changes and they look great!

Here's what I looked at during the review

🟡 General issues: 1 issue found
🟢 Security: all looks good
🟢 Testing: all looks good
🟢 Complexity: all looks good
🟢 Documentation: all looks good

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

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

sourcery-ai · 2025-05-28T07:29:48Z

panels/notification/common/notifyentity.cpp

+        QString value;
+        if (it.value().typeId() == QMetaType::QStringList) {
+            QStringList tmp = it.value().toStringList();
+            value = tmp.join(LIST_VALUE_SEGMENT);


issue (bug_risk): Delimiter may appear in data and break parsing

Consider escaping LIST_VALUE_SEGMENT in list elements or using a safer serialization method like JSON or base64 to prevent parsing errors.

deepin-ci-robot · 2025-05-28T07:54:04Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 18202781743, mhduiy

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

18202781743 · 2025-05-28T07:56:36Z

linuxdeepin/deepin-screen-recorder#669

18202781743 · 2025-05-28T10:29:08Z

/forcemerge

deepin-bot · 2025-05-28T10:29:31Z

This pr force merged! (status: blocked)

18202781743 requested review from BLumia, caixr23, fly602, mhduiy, robertkill and yixinshark May 28, 2025 07:26

sourcery-ai bot reviewed May 28, 2025

View reviewed changes

mhduiy approved these changes May 28, 2025

View reviewed changes

18202781743 mentioned this pull request May 28, 2025

fix: improve file manager command handling linuxdeepin/deepin-screen-recorder#669

Merged

deepin-bot bot merged commit f2f0ead into linuxdeepin:master May 28, 2025
8 of 11 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: support string list in notification hints #1135

fix: support string list in notification hints #1135

Uh oh!

18202781743 commented May 28, 2025 •

edited by sourcery-ai bot

Loading

Uh oh!

deepin-ci-robot commented May 28, 2025

Uh oh!

sourcery-ai bot commented May 28, 2025

Interacting with Sourcery

Customizing Your Experience

Getting Help

Uh oh!

sourcery-ai bot left a comment

Uh oh!

sourcery-ai bot May 28, 2025

Uh oh!

deepin-ci-robot commented May 28, 2025

Uh oh!

18202781743 commented May 28, 2025

Uh oh!

18202781743 commented May 28, 2025

Uh oh!

deepin-bot bot commented May 28, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

fix: support string list in notification hints #1135

fix: support string list in notification hints #1135

Uh oh!

Conversation

18202781743 commented May 28, 2025 • edited by sourcery-ai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by Sourcery

Uh oh!

deepin-ci-robot commented May 28, 2025

deepin pr auto review

Uh oh!

sourcery-ai bot commented May 28, 2025

Reviewer's Guide

Class diagram for updated notification components

Flow diagram for updated parseHint logic

File-Level Changes

Interacting with Sourcery

Customizing Your Experience

Getting Help

Uh oh!

sourcery-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

sourcery-ai bot May 28, 2025

Choose a reason for hiding this comment

Uh oh!

deepin-ci-robot commented May 28, 2025

Uh oh!

18202781743 commented May 28, 2025

Uh oh!

18202781743 commented May 28, 2025

Uh oh!

deepin-bot bot commented May 28, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

18202781743 commented May 28, 2025 •

edited by sourcery-ai bot

Loading

Flow diagram for updated `parseHint` logic