-
Notifications
You must be signed in to change notification settings - Fork 55
fix: add exact match flag to prevent duplicate app entries #1339
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.
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
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis pull request enhances the duplicate detection logic in the application model by updating the match() call to use an exact match flag and limit results, preventing partial desktop ID matches from causing duplicate entries. Sequence diagram for exact duplicate detection on InterfacesAddedsequenceDiagram
participant ObjectManager
participant AMAppItemModel
ObjectManager->>AMAppItemModel: InterfacesAdded(objPath, interfacesAndProperties)
AMAppItemModel->>AMAppItemModel: DUtil::unescapeFromObjectPath(objPath)
AMAppItemModel->>AMAppItemModel: match(index, DesktopIdRole, desktopId, 1, MatchExactly)
AMAppItemModel-->>ObjectManager: (if match found) return (do not add duplicate)
AMAppItemModel->>AMAppItemModel: (if no match) proceed to add new app entry
Class diagram for updated duplicate detection in AMAppItemModelclassDiagram
class AMAppItemModel {
+QObject *parent
+ObjectManager *m_manager
+AMAppItemModel(QObject *parent)
+match(QModelIndex, int role, QVariant value, int hits = 1, Qt::MatchFlag flag = Qt::MatchExactly)
}
AMAppItemModel --> ObjectManager
AMAppItemModel : connect(m_manager, InterfacesAdded, lambda)
AMAppItemModel : match(index(0, 0), AppItemModel_DesktopIdRole, desktopId, 1, Qt_MatchExactly)
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
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:
- Consider adding Qt::MatchRecursive to the match flags so the search spans the entire model rather than just the starting row.
- Replace the literal ‘1’ hits parameter with a named constant or helper method to clarify its role as a match count limit.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider adding Qt::MatchRecursive to the match flags so the search spans the entire model rather than just the starting row.
- Replace the literal ‘1’ hits parameter with a named constant or helper method to clarify its role as a match count limit.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Added Qt::MatchExactly flag to the match() function call to ensure proper duplicate detection This prevents multiple instances of the same application from being added to the model The previous implementation could incorrectly match partial desktop IDs, leading to duplicates fix: 添加精确匹配标志以防止重复应用条目 在 match() 函数调用中添加了 Qt::MatchExactly 标志以确保正确的重复检测 这可以防止同一应用程序的多个实例被添加到模型中 之前的实现可能会错误地匹配部分桌面 ID,导致重复条目 PMS: BUG-339005
b9b68d3 to
37f339c
Compare
deepin pr auto review我来对这个git diff进行代码审查:
总结:
建议:
|
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: BLumia, wjyrich The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Added Qt::MatchExactly flag to the match() function call to ensure proper duplicate detection
This prevents multiple instances of the same application from being added to the model
The previous implementation could incorrectly match partial desktop IDs, leading to duplicates
fix: 添加精确匹配标志以防止重复应用条目
在 match() 函数调用中添加了 Qt::MatchExactly 标志以确保正确的重复检测
这可以防止同一应用程序的多个实例被添加到模型中
之前的实现可能会错误地匹配部分桌面 ID,导致重复条目
PMS: BUG-339003 BUG-339005
Summary by Sourcery
Bug Fixes: