fix: fix system and app proxy is hide#503
Open
ut003640 wants to merge 1 commit intolinuxdeepin:masterfrom
Open
fix: fix system and app proxy is hide#503ut003640 wants to merge 1 commit intolinuxdeepin:masterfrom
ut003640 wants to merge 1 commit intolinuxdeepin:masterfrom
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: ut003640 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 |
Reviewer's GuideEnsures system and app proxy controls are always visible in the control center by preventing their NetItem rows from being removed, filtering duplicate items in the proxy model, and making the System Proxy switches always enabled in QML. Sequence diagram for NetItem removal with preserved proxy itemssequenceDiagram
participant View
participant NetItemModel
participant ParentItem as NetItem_parent
participant ChildItem as NetItem_child
View->>NetItemModel: AboutToRemoveObject(parentItem, pos)
NetItemModel->>ParentItem: getChild(pos)
ParentItem-->>NetItemModel: childItem
NetItemModel->>NetItemModel: needRemoveNodeItem(childItem)
alt childItem is SystemProxyControlItem or AppProxyControlItem
NetItemModel-->>View: skip beginRemoveRows
else other item type
NetItemModel->>View: beginRemoveRows
end
View->>NetItemModel: removeObject(child)
NetItemModel->>NetItemModel: needRemoveNodeItem(child)
alt child is SystemProxyControlItem or AppProxyControlItem
NetItemModel-->>View: skip endRemoveRows
else other item type
NetItemModel->>View: endRemoveRows
end
Class diagram for updated NetItemModel proxy filtering and row removalclassDiagram
class NetItemModel {
+rootChanged(root NetItem)
+roleNames() QHash~int,QByteArray~
+lessThan(source_left QModelIndex, source_right QModelIndex) bool
+filterAcceptsRow(source_row int, source_parent QModelIndex) bool
-needRemoveNodeItem(childItem NetItem) bool
-AboutToRemoveObject(parentItem NetItem, pos int) void
-removeObject(child NetItem) void
-m_rowMap QMap~NetType_NetItemType,int~
}
class NetItem {
+itemType() NetType_NetItemType
+getChild(pos int) NetItem
}
class NetType_NetItemType {
<<enumeration>>
SystemProxyControlItem
AppProxyControlItem
otherTypes
}
NetItemModel --> NetItem : uses
NetItemModel --> NetType_NetItemType : uses
NetItem --> NetType_NetItemType : returns
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The
filterAcceptsRowimplementation keeps state inm_rowMapacross calls but never resets it, which can lead to incorrect filtering as the model changes over time; consider clearing or rebuilding this map when the source model changes or on each filter pass instead of relying on mutable state. - Using
NetType::NetItemTypeas the key inm_rowMapignores the parent index, so only one row per type will ever be shown even if logically distinct items exist under different parents; if that’s not intended, include the parent (or full index path) in the key. - In
filterAcceptsRow, you don't call the baseQSortFilterProxyModel::filterAcceptsRow, so any existing filtering behavior is bypassed; if additional conditions should be combined with the default logic, explicitly delegate to the base implementation.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `filterAcceptsRow` implementation keeps state in `m_rowMap` across calls but never resets it, which can lead to incorrect filtering as the model changes over time; consider clearing or rebuilding this map when the source model changes or on each filter pass instead of relying on mutable state.
- Using `NetType::NetItemType` as the key in `m_rowMap` ignores the parent index, so only one row per type will ever be shown even if logically distinct items exist under different parents; if that’s not intended, include the parent (or full index path) in the key.
- In `filterAcceptsRow`, you don't call the base `QSortFilterProxyModel::filterAcceptsRow`, so any existing filtering behavior is bypassed; if additional conditions should be combined with the default logic, explicitly delegate to the base implementation.
## Individual Comments
### Comment 1
<location> `dcc-network/operation/netitemmodel.cpp:233-238` </location>
<code_context>
+
bool NetItemModel::lessThan(const QModelIndex &source_left, const QModelIndex &source_right) const
{
NetItem *leftItem = static_cast<NetItem *>(source_left.internalPointer());
+ if (!leftItem)
+ return false;
+
NetItem *rightItem = static_cast<NetItem *>(source_right.internalPointer());
+ if (!rightItem)
+ return false;
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Handling null pointers in `lessThan` by returning `false` can violate strict weak ordering.
Returning `false` when either pointer is null means that for a null vs non-null pair, both `lessThan(left, right)` and `lessThan(right, left)` can be `false`, which violates strict weak ordering required by `QSortFilterProxyModel` and can cause unstable sorting. Consider defining a consistent ordering for nulls (e.g., always less or always greater than non-null) so comparisons remain deterministic.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
233
to
+238
| NetItem *leftItem = static_cast<NetItem *>(source_left.internalPointer()); | ||
| if (!leftItem) | ||
| return false; | ||
|
|
||
| NetItem *rightItem = static_cast<NetItem *>(source_right.internalPointer()); | ||
| if (!rightItem) |
There was a problem hiding this comment.
issue (bug_risk): Handling null pointers in lessThan by returning false can violate strict weak ordering.
Returning false when either pointer is null means that for a null vs non-null pair, both lessThan(left, right) and lessThan(right, left) can be false, which violates strict weak ordering required by QSortFilterProxyModel and can cause unstable sorting. Consider defining a consistent ordering for nulls (e.g., always less or always greater than non-null) so comparisons remain deterministic.
the system and app proxy must show on the control center PMS: BUG-350619
deepin pr auto review代码审查报告本次代码审查主要针对 1. 文件：
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
the system and app proxy must show on the control center
PMS: BUG-350619
Summary by Sourcery
Ensure system and app proxy controls remain visible and selectable in the network control center.
Bug Fixes: