Skip to content

Conversation

@18202781743
Copy link
Contributor

@18202781743 18202781743 commented Oct 27, 2025

Fixed incorrect bubble ID comparison in removeById method that was
causing bubble deletion failures. The method was comparing against
bubbleId() instead of id(), preventing proper bubble removal. Removed
the unused getBubbleIdByStorageId method and simplified the closeBubble
logic by directly using the provided ID without unnecessary entity
lookups and ID conversions.

The issue occurred because bubbleId and storageId were being confused
in the deletion process. The removeById method was incorrectly checking
bubbleId() instead of the actual item id(), making it impossible to find
and remove bubbles by their correct identifier. This also eliminated the
need for the complex ID mapping logic in closeBubble.

Influence:

  1. Test bubble deletion functionality by closing notification bubbles
  2. Verify that bubbles are properly removed from the interface when
    closed
  3. Check that no orphaned bubbles remain after deletion attempts
  4. Test with multiple bubbles to ensure correct individual bubble
    removal
  5. Verify bubble count decreases appropriately after deletions

fix: 修正通知系统中气泡ID处理问题

修复了removeById方法中错误的气泡ID比较问题,该问题导致气泡删除失败。方
法之前比较的是bubbleId()而不是id(),阻止了正确的气泡移除。移除了未使用
的getBubbleIdByStorageId方法,并通过直接使用提供的ID简化了closeBubble逻
辑,无需不必要的实体查找和ID转换。

该问题的发生是因为在删除过程中混淆了bubbleId和storageId。removeById方法
错误地检查bubbleId()而不是实际的item id(),导致无法通过正确的标识符找到
和移除气泡。这也消除了closeBubble中复杂的ID映射逻辑的需求。

Influence:

  1. 测试气泡删除功能,关闭通知气泡
  2. 验证关闭时气泡是否正确从界面移除
  3. 检查删除尝试后是否没有残留的孤立气泡
  4. 使用多个气泡测试,确保正确移除单个气泡
  5. 验证删除后气泡计数是否适当减少

PMS: BUG-337395

Summary by Sourcery

Fix bubble deletion by correcting ID comparison and streamlining closeBubble logic.

Bug Fixes:

  • Compare against item->id() instead of bubbleId() in removeById to enable proper bubble removal.

Enhancements:

  • Remove unused getBubbleIdByStorageId method and simplify closeBubble by directly using the provided ID and eliminating entity lookups and conversions.

@sourcery-ai
Copy link

sourcery-ai bot commented Oct 27, 2025

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

This PR corrects bubble deletion by replacing an incorrect ID comparison in removeById, removes an unused ID‐mapping helper, and streamlines the closeBubble flow to use the supplied ID directly, removing unnecessary lookups and conversions.

Sequence diagram for simplified bubble closing process

sequenceDiagram
participant BubblePanel
participant BubbleModel
actor User
User->>BubblePanel: Request to close bubble (id)
BubblePanel->>BubbleModel: removeById(id)
BubbleModel->>BubblePanel: Bubble removed (if id matches)
BubblePanel->>User: Bubble closed (UI updated)
Loading

Class diagram for updated BubbleModel and BubblePanel

classDiagram
class BubbleModel {
  - QList<BubbleItem*> m_bubbles
  - QList<qint64> m_delayBubbles
  + void remove(int index)
  + void remove(const BubbleItem *bubble)
  + BubbleItem *removeById(qint64 id) // ID comparison fixed
  + void clear()
  + BubbleItem *bubbleItem(int bubbleIndex) const
}
class BubblePanel {
  - BubbleModel *m_bubbles
  - Accessor *m_accessor
  + void addBubble(qint64 id)
  + void closeBubble(qint64 id) // Simplified logic
}
BubblePanel --> BubbleModel
Loading

File-Level Changes

Change Details Files
Correct ID comparison in removeById
  • Replaced item->bubbleId() check with item->id()
  • Removed outdated getBubbleIdByStorageId dependency in deletion
panels/notification/bubble/bubblemodel.cpp
Remove unused getBubbleIdByStorageId method
  • Deleted getBubbleIdByStorageId definition from .cpp
  • Removed its declaration from .h
panels/notification/bubble/bubblemodel.cpp
panels/notification/bubble/bubblemodel.h
Simplify closeBubble logic
  • Eliminated fetchEntity lookup and ID conversion
  • Dropped getBubbleIdByStorageId call
  • Updated warning log to reflect direct ID usage
panels/notification/bubble/bubblepanel.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

Copy link

@sourcery-ai sourcery-ai bot left a 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 and they look great!


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.

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 18202781743, wjyrich, yixinshark

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

Fixed incorrect bubble ID comparison in removeById method that was
causing bubble deletion failures. The method was comparing against
bubbleId() instead of id(), preventing proper bubble removal. Removed
the unused getBubbleIdByStorageId method and simplified the closeBubble
logic by directly using the provided ID without unnecessary entity
lookups and ID conversions.

The issue occurred because bubbleId and storageId were being confused
in the deletion process. The removeById method was incorrectly checking
bubbleId() instead of the actual item id(), making it impossible to find
and remove bubbles by their correct identifier. This also eliminated the
need for the complex ID mapping logic in closeBubble.

Influence:
1. Test bubble deletion functionality by closing notification bubbles
2. Verify that bubbles are properly removed from the interface when
closed
3. Check that no orphaned bubbles remain after deletion attempts
4. Test with multiple bubbles to ensure correct individual bubble
removal
5. Verify bubble count decreases appropriately after deletions

fix: 修正通知系统中气泡ID处理问题

修复了removeById方法中错误的气泡ID比较问题,该问题导致气泡删除失败。方
法之前比较的是bubbleId()而不是id(),阻止了正确的气泡移除。移除了未使用
的getBubbleIdByStorageId方法,并通过直接使用提供的ID简化了closeBubble逻
辑,无需不必要的实体查找和ID转换。

该问题的发生是因为在删除过程中混淆了bubbleId和storageId。removeById方法
错误地检查bubbleId()而不是实际的item id(),导致无法通过正确的标识符找到
和移除气泡。这也消除了closeBubble中复杂的ID映射逻辑的需求。

Influence:
1. 测试气泡删除功能,关闭通知气泡
2. 验证关闭时气泡是否正确从界面移除
3. 检查删除尝试后是否没有残留的孤立气泡
4. 使用多个气泡测试,确保正确移除单个气泡
5. 验证删除后气泡计数是否适当减少

PMS: BUG-337395
@deepin-ci-robot
Copy link

deepin pr auto review

我来对这个git diff进行代码审查:

  1. 代码逻辑改进:
  • 删除了冗余的getBubbleIdByStorageId函数,这个函数的功能与item->bubbleId()重复,简化了代码结构。
  • 修改了removeById函数中的判断条件,从item->bubbleId() == id改为item->id() == id,使ID的判断更加直接和统一。
  • 简化了closeBubble函数的实现,移除了多余的错误处理逻辑,使代码更加简洁。
  1. 代码质量改进:
  • 移除了未使用的函数getBubbleIdByStorageId,提高了代码的可维护性。
  • 统一了ID的获取方式,避免了不同类型ID混用可能带来的混淆。
  • 简化了错误处理流程,使代码逻辑更加清晰。
  1. 代码性能改进:
  • removeById函数中,减少了不必要的ID转换操作,直接使用item->id()进行比较,提高了查找效率。
  • 移除了closeBubble中多余的实体查找操作,减少了系统开销。
  1. 代码安全改进:
  • 简化了ID的处理逻辑,减少了ID转换过程中可能出现的安全隐患。
  • 保持了基本的ID有效性检查(id > 0),确保不会处理无效ID。

建议:

  1. 建议在代码注释中说明id的具体含义和用途,以便其他开发者更好地理解代码。
  2. 可以考虑在removeById函数中添加对空指针的检查,提高代码的健壮性。
  3. 建议对clearInvalidBubbles()函数的实现进行审查,确保它能够正确处理所有异常情况。

总体来说,这次代码改动是积极的,提高了代码的可读性和可维护性,同时保持了原有功能的完整性。

@18202781743 18202781743 merged commit 8dcf20e into linuxdeepin:master Oct 28, 2025
10 of 11 checks passed
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.

4 participants