Skip to content

fix: Change the app order in Privacy and Security#2825

Merged
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
pengfeixx:fix-303417
Dec 24, 2025
Merged

fix: Change the app order in Privacy and Security#2825
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
pengfeixx:fix-303417

Conversation

@pengfeixx

@pengfeixx pengfeixx commented Nov 20, 2025

Copy link
Copy Markdown
Contributor

Change the app order in Privacy and Security

Log: Change the app order in Privacy and Security
pms: BUG-303417

Summary by Sourcery

Fix app ordering in Privacy and Security by categorizing names and inserting apps at the correct sorted position.

Bug Fixes:

  • Prefix sort field with a category (digit, ASCII letter, or Chinese/other) before pinyin conversion to ensure correct ordering
  • Insert new applications at the computed sorted position instead of always appending to the end

@sourcery-ai

sourcery-ai Bot commented Nov 20, 2025

Copy link
Copy Markdown

Reviewer's Guide

This PR refactors the app ordering mechanism in Privacy and Security by enhancing the sort field to include a category prefix (digits, letters, others) and updating the model insertion logic to maintain sorted order based on that sort key.

Class diagram for updated ApplicationItem and AppsModel

classDiagram
    class ApplicationItem {
        - QString m_name
        - QString m_sortField
        + void onNameChanged(const QString &name)
        + QString sortField()
    }
    class AppsModel {
        - QList<ApplicationItem*> m_appItems
        + void appendItem(ApplicationItem *appItem)
    }
    ApplicationItem <.. AppsModel : uses
Loading

Flow diagram for new app insertion logic in AppsModel

flowchart TD
    A["New ApplicationItem created"] --> B["Calculate sortField (category + pinyin)"]
    B --> C["Find insert position in m_appItems based on sortField"]
    C --> D["Insert ApplicationItem at correct position"]
    D --> E["App list remains sorted"]
Loading

File-Level Changes

Change Details Files
Enhanced application sort key generation with category prefix
  • Extract uppercase name into a temporary variable
  • Determine category (digit, ASCII letter, other) from the first non-space character
  • Prefix the pinyin conversion with the category number when setting sortField
src/plugin-privacy/operation/applicationitem.cpp
Changed app list insertion to maintain sorted order by sortField
  • Compute insertPos by comparing the new item’s sortField against existing items
  • Use beginInsertRows with the computed insertPos
  • Insert the new item at insertPos instead of appending at the end
src/plugin-privacy/operation/privacysecuritymodel.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

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey there - I've reviewed your changes - here's some feedback:

  • appendItem uses a linear search for each insertion, leading to O(n²) behavior when resetting; consider sorting once and doing a bulk insert or using std::lower_bound for more efficient insertion.
  • Extract the category-plus-pinyin sortField construction into a shared helper so the sorting logic stays consistent and easier to maintain.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- appendItem uses a linear search for each insertion, leading to O(n²) behavior when resetting; consider sorting once and doing a bulk insert or using std::lower_bound for more efficient insertion.
- Extract the category-plus-pinyin sortField construction into a shared helper so the sorting logic stays consistent and easier to maintain.

## Individual Comments

### Comment 1
<location> `src/plugin-privacy/operation/applicationitem.cpp:120` </location>
<code_context>
+        break;
+    }
+
+    QString pinyin = DTK_CORE_NAMESPACE::Chinese2Pinyin(upperName);
+    m_sortField = QString::number(category) + pinyin;
     emitDataChanged();
 }
</code_context>

<issue_to_address>
**suggestion:** Consider adding a separator between category and pinyin for sortField.

Directly joining category and pinyin may cause ambiguity if pinyin begins with a digit. Adding a separator, such as an underscore, will clarify the sortField format.

```suggestion
    m_sortField = QString::number(category) + "_" + pinyin;
```
</issue_to_address>

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.

}

QString pinyin = DTK_CORE_NAMESPACE::Chinese2Pinyin(upperName);
m_sortField = QString::number(category) + pinyin;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

suggestion: Consider adding a separator between category and pinyin for sortField.

Directly joining category and pinyin may cause ambiguity if pinyin begins with a digit. Adding a separator, such as an underscore, will clarify the sortField format.

Suggested change
m_sortField = QString::number(category) + pinyin;
m_sortField = QString::number(category) + "_" + pinyin;

@deepin-bot

deepin-bot Bot commented Nov 27, 2025

Copy link
Copy Markdown

TAG Bot

New tag: 6.1.59
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #2843

@deepin-bot

deepin-bot Bot commented Nov 28, 2025

Copy link
Copy Markdown

TAG Bot

New tag: 6.1.60
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #2844

@deepin-bot

deepin-bot Bot commented Dec 4, 2025

Copy link
Copy Markdown

TAG Bot

New tag: 6.1.61
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #2854

@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: caixr23, pengfeixx

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

@deepin-bot

deepin-bot Bot commented Dec 11, 2025

Copy link
Copy Markdown

TAG Bot

New tag: 6.1.62
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #2874

@deepin-bot

deepin-bot Bot commented Dec 18, 2025

Copy link
Copy Markdown

TAG Bot

New tag: 6.1.63
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #2888

Change the app order in Privacy and Security

Log: Change the app order in Privacy and Security
pms: BUG-303417
@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

我来对这段代码的修改进行审查:

  1. 代码逻辑改进:
  • 在 ApplicationItem::onNameChanged() 中,新增了分类逻辑,将应用名按首字符分为数字(0)、字母(1)和其他(2)三类,这是一个合理的分类方式。
  • 在 AppsModel::appendItem() 中,改为插入排序而不是简单的追加,这样可以保持列表的有序性。
  1. 代码质量改进:
  • 变量命名清晰,如 category、insertPos 等都有明确的含义
  • 代码结构清晰,逻辑分层合理
  • 使用了 Qt 的标准命名规范
  1. 性能优化建议:
  • 在 ApplicationItem::onNameChanged() 中,可以提前返回,避免不必要的转换:
if (m_name == name) {
    return;
}
  • 在 AppsModel::appendItem() 中,可以考虑使用二分查找来优化插入位置的查找,特别是当列表很大时:
int left = 0, right = m_appItems.size();
while (left < right) {
    int mid = (left + right) / 2;
    if (appItem->sortField() < m_appItems.at(mid)->sortField()) {
        right = mid;
    } else {
        left = mid + 1;
    }
}
insertPos = left;
  1. 安全性考虑:
  • 代码中使用了 Qt 的字符串处理函数,这些函数都有良好的边界检查,安全性较好
  • 建议在 ApplicationItem::onNameChanged() 中添加对空字符串的处理:
if (name.isEmpty()) {
    return;
}
  1. 其他建议:
  • 可以考虑将分类逻辑提取为一个单独的函数,提高代码的可维护性:
int getCategory(const QString& name) {
    QString upperName = name.toUpper();
    for (const QChar &ch : upperName) {
        if (ch.isSpace()) continue;
        if (ch.isDigit()) return 0;
        if (ch.isLetter() && ch.unicode() < 128) return 1;
        return 2;
    }
    return 2;
}

总体来说,这个修改是合理的,提高了应用列表的排序效果,但还可以在性能和代码组织方面做进一步优化。

@pengfeixx

Copy link
Copy Markdown
Contributor Author

/forcemerge

@deepin-bot

deepin-bot Bot commented Dec 24, 2025

Copy link
Copy Markdown

This pr force merged! (status: blocked)

@deepin-bot deepin-bot Bot merged commit 65dbdfe into linuxdeepin:master Dec 24, 2025
16 of 18 checks passed
@pengfeixx pengfeixx deleted the fix-303417 branch December 24, 2025 01:16
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