fix: DSL connection deletion signal processing error#376
fix: DSL connection deletion signal processing error#376caixr23 merged 1 commit intolinuxdeepin:masterfrom
Conversation
Reviewer's GuideThis PR corrects DSL connection signal handling by replacing incorrect addition signals with removal signals, consolidates DSL item addition emissions within the controller, and improves QML null-safety for proxy properties, along with general whitespace and formatting cleanups. Sequence diagram for corrected DSL connection removal signalsequenceDiagram
participant DSLController_NM
participant NetManagerThreadPrivate
participant QML_UI
DSLController_NM->>NetManagerThreadPrivate: Q_EMIT itemRemoved(connection->path())
NetManagerThreadPrivate->>QML_UI: (UI updates to reflect removal)
Class diagram for updated DSL connection signal handlingclassDiagram
class DSLController_NM {
+void connectionAppeared(WiredDevice::Ptr, QList<Connection::Ptr>)
+DSLItem* addPppoeConnection(WiredDevice::Ptr, Connection::Ptr)
+signal itemAdded(QList<DSLItem*>)
}
class NetManagerThreadPrivate {
+void onDSLRemoved(QList<DSLItem*>)
+signal itemRemoved(QString)
}
DSLController_NM --|> NetManagerThreadPrivate : emits signals to
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 @caixr23 - I've reviewed your changes - here's some feedback:
- Verify that onDSLRemoved’s new single-parameter signal provides enough context to clean up the original NetConnectionItemPrivate instance instead of only emitting its path—otherwise stale objects may linger.
- Ensure that moving the itemAdded emit into addPppoeConnection (and removing the conditional emit in connectionAppeared) doesn’t result in missing or duplicated DSL itemAdded signals in edge cases.
- Consider separating the whitespace-only reformatting from the functional fixes into its own commit to make the actual logic changes easier to review.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Verify that onDSLRemoved’s new single-parameter signal provides enough context to clean up the original NetConnectionItemPrivate instance instead of only emitting its path—otherwise stale objects may linger.
- Ensure that moving the itemAdded emit into addPppoeConnection (and removing the conditional emit in connectionAppeared) doesn’t result in missing or duplicated DSL itemAdded signals in edge cases.
- Consider separating the whitespace-only reformatting from the functional fixes into its own commit to make the actual logic changes easier to review.
## Individual Comments
### Comment 1
<location> `src/impl/networkmanager/dslcontrollernm.cpp:189` </location>
<code_context>
return item1->connection()->id() < item2->connection()->id();
});
-
+ Q_EMIT itemAdded({item});
return item;
}
</code_context>
<issue_to_address>
Emitting 'itemAdded' unconditionally may emit null items.
Previously, 'itemAdded' was only emitted for non-null DSLItems. Now, emitting unconditionally could add null or invalid items. Please restore the null check or ensure 'item' is always valid before emitting.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| return item1->connection()->id() < item2->connection()->id(); | ||
| }); | ||
|
|
||
| Q_EMIT itemAdded({item}); |
There was a problem hiding this comment.
issue (bug_risk): Emitting 'itemAdded' unconditionally may emit null items.
Previously, 'itemAdded' was only emitted for non-null DSLItems. Now, emitting unconditionally could add null or invalid items. Please restore the null check or ensure 'item' is always valid before emitting.
DSL connection deletion signal processing error pms: BUG-308553
deepin pr auto review代码审查意见：
总体来说，代码的修改是合理的，没有发现明显的语法或逻辑错误。但是，建议进行进一步的代码重构，以减少代码重复和提高代码的可维护性。 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: caixr23, robertkill 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 |
3 participants
DSL connection deletion signal processing error
pms: BUG-308553
Summary by Sourcery
Fix errors in DSL connection signal handling and improve property safety in QML.
Bug Fixes:
Enhancements: