fix(notification): adjust notification sound playback timing#1532
Merged
deepin-bot[bot] merged 2 commits intolinuxdeepin:masterfrom Mar 25, 2026
Merged
fix(notification): adjust notification sound playback timing#1532deepin-bot[bot] merged 2 commits intolinuxdeepin:masterfrom
deepin-bot[bot] merged 2 commits intolinuxdeepin:masterfrom
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdjusts notification replacement handling so that missing replacesId no longer causes failure, instead logging and treating it as a new notification, in line with the freedesktop spec. Sequence diagram for handling notifications with missing replacesIdsequenceDiagram
actor App
participant NotificationManager
participant Persistence
App->>NotificationManager: sendNotification(summary, body, replacesId)
alt has replacesId
NotificationManager->>Persistence: findEntityById(replacesId)
alt entity exists
Persistence-->>NotificationManager: lastEntity
NotificationManager->>Persistence: replaceEntity(lastEntity.id, newEntity)
Persistence-->>NotificationManager: newId
NotificationManager-->>App: success(newId)
else entity does not exist
Note over NotificationManager: Log: Not exist notification to replace
NotificationManager->>Persistence: saveEntityAsNew(newEntity)
Persistence-->>NotificationManager: newId
NotificationManager-->>App: success(newId)
end
else no replacesId
NotificationManager->>Persistence: saveEntityAsNew(newEntity)
Persistence-->>NotificationManager: newId
NotificationManager-->>App: success(newId)
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Consider updating the
recordNotificationcontract (e.g., method comment or callers’ assumptions) now that an invalidreplacesIdno longer causes afalsereturn, to avoid confusing semantics around when this function can fail. - Since the situation is now treated as a normal flow rather than an error, you might want to downgrade the log level from
qWarningto something less severe (e.g., info/debug) to reduce noise in logs.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider updating the `recordNotification` contract (e.g., method comment or callers’ assumptions) now that an invalid `replacesId` no longer causes a `false` return, to avoid confusing semantics around when this function can fail.
- Since the situation is now treated as a normal flow rather than an error, you might want to downgrade the log level from `qWarning` to something less severe (e.g., info/debug) to reduce noise in logs.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
18202781743
reviewed
Mar 25, 2026
Move the sound playback to the end of Notify function, after all notification display logic has been processed. This ensures sound is played at the appropriate time in the notification flow. 将音效播放移到Notify函数末尾,在所有通知显示逻辑处理完成后。 这确保音效在通知流程的适当时机播放。 Log: 调整通知音效播放时机 PMS: BUG-354203 Influence: 调整音效播放时机到通知处理流程的最后阶段, 确保音效播放的时机更加合理。
18202781743
approved these changes
Mar 25, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, add-uos 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 |
|
This pr force merged! (status: blocked) |
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
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.
Move the sound playback to the end of Notify function, after all
notification display logic has been processed. This ensures sound is
played at the appropriate time in the notification flow.
将音效播放移到Notify函数末尾,在所有通知显示逻辑处理完成后。
这确保音效在通知流程的适当时机播放。
Log: 调整通知音效播放时机
PMS: BUG-354203
Influence: 调整音效播放时机到通知处理流程的最后阶段,
确保音效播放的时机更加合理。