fix: centralize relative time formatting logic#1559
Merged
Conversation
Reviewer's GuideCentralizes relative time formatting into a new ICU-based helper on NotifyEntity and updates both the notification center item and bubble model to use it, ensuring consistent, locale-aware time strings while simplifying their code and adjusting includes and copyrights. Sequence diagram for unified relative time formatting in notificationssequenceDiagram
actor User
participant UI as NotificationUI
participant Center as AppNotifyItem
participant Bubble as BubbleModel
participant Entity as NotifyEntity
User->>UI: Open notification center / see bubble
UI->>Center: Request display data
Center->>Entity: formatRelativeTime(m_entity.cTime())
Entity-->>Center: relative time string or empty
alt center got empty string
Center->>Center: validate QDateTime
Center->>Center: fallback to localized Just now
end
Center-->>UI: Formatted time for center item
UI->>Bubble: Request bubble updates
loop each bubble item
Bubble->>Entity: formatRelativeTime(item.ctime())
alt non empty result
Entity-->>Bubble: relative time string
Bubble->>Bubble: item.setTimeTip(timeTip)
else empty result
Entity-->>Bubble: empty (no update)
end
end
Bubble-->>UI: Updated bubble time tips
Class diagram for centralized relative time formatting logicclassDiagram
class NotifyEntity {
+qint64 cTime()
+QString bodyIcon()
+static QString formatRelativeTime(qint64 ctimeMs)
}
class AppNotifyItem {
-NotifyEntity m_entity
-QString m_time
+QString time()
+void updateTime()
}
class BubbleModel {
-QList~NotifyEntity*~ m_bubbles
+void updateBubbleTimeTip()
}
AppNotifyItem --> NotifyEntity : uses m_entity
BubbleModel --> NotifyEntity : calls formatRelativeTime
AppNotifyItem ..> NotifyEntity : calls formatRelativeTime
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:
- In
NotifyEntity::formatRelativeTime, the ICUUErrorCodevalues (cachedStatus,status,timeStatus) are never checked; consider handlingU_FAILUREto avoid using an invalid formatter and provide a deterministic fallback string when ICU operations fail. formatRelativeTimereturns an empty string for timestamps less than one minute old, which is treated as "Just now" only byAppNotifyItem::updateTimebut not byBubbleModel::updateBubbleTimeTip; please double-check whether this intentional divergence is desired or whether the "Just now" fallback should be handled consistently at the caller or callee level.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `NotifyEntity::formatRelativeTime`, the ICU `UErrorCode` values (`cachedStatus`, `status`, `timeStatus`) are never checked; consider handling `U_FAILURE` to avoid using an invalid formatter and provide a deterministic fallback string when ICU operations fail.
- `formatRelativeTime` returns an empty string for timestamps less than one minute old, which is treated as "Just now" only by `AppNotifyItem::updateTime` but not by `BubbleModel::updateBubbleTimeTip`; please double-check whether this intentional divergence is desired or whether the "Just now" fallback should be handled consistently at the caller or callee level.
## Individual Comments
### Comment 1
<location path="panels/notification/common/notifyentity.cpp" line_range="407-416" />
<code_context>
- formatter->format(hour, UDAT_DIRECTION_LAST, UDAT_RELATIVE_HOURS, result, status);
- ret = toQString(result);
- }
- } else if (elapsedDay >= 1 && elapsedDay < 2) {
- formatter->format(1, UDAT_DIRECTION_LAST, UDAT_RELATIVE_DAYS, result, status);
- UnicodeString combinedString;
</code_context>
<issue_to_address>
**suggestion:** Combining relative day text with a fixed "HH:mm" pattern ignores user’s time-format preferences.
In the 1‑day range you combine ICU’s relative day text with `SimpleDateFormat("HH:mm", ...)`, which forces a 24‑hour clock and ignores system/user time-format settings. Other branches already use `QLocale` to respect local conventions. Please derive the time pattern from `QLocale` (or a user preference) before passing it to `SimpleDateFormat`, or format the time via `QLocale` and use ICU only for the relative day text so the time format stays consistent with user expectations.
Suggested implementation:
```cpp
} else if (elapsedDay >= 1 && elapsedDay < 2) {
formatter->format(1, UDAT_DIRECTION_LAST, UDAT_RELATIVE_DAYS, result, status);
// Format the time according to the user's locale preferences
const auto locale = QLocale::system();
const QString localTimeString = locale.toString(time, QLocale::ShortFormat);
// Convert the localized time string to an ICU UnicodeString
UnicodeString icuTimeString = icu::UnicodeString::fromUTF8(localTimeString.toUtf8().constData());
UnicodeString combinedString;
formatter->combineDateAndTime(result, icuTimeString, combinedString, status);
return toQString(combinedString);
} else if (elapsedDay >= 2 && elapsedDay < 7) {
```
1. If not already available in this translation unit, ensure the necessary ICU headers are included for `icu::UnicodeString::fromUTF8` (typically `#include <unicode/unistr.h>` and `#include <unicode/stringpiece.h>`).
2. If the project already has a helper to convert `QString` to `UnicodeString` (for example a `toUnicodeString(const QString &)`), replace the direct `fromUTF8` call with that helper to stay consistent with existing conventions.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
18202781743
approved these changes
Apr 16, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, xionglinlin 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 |
Moved relative time formatting logic from BubbleModel and AppNotifyItem to a common static method in NotifyEntity to eliminate code duplication and fix incorrect time display issues. Previously, BubbleModel used a simple minutes calculation while AppNotifyItem used ICU library for locale-aware formatting, causing inconsistent time display between notification bubble and center. Now both components use the same unified formatting logic. The changes include: 1. Added static method NotifyEntity::formatRelativeTime() that handles all relative time formatting using ICU library 2. Removed QDateTime include from bubblemodel.cpp as it's no longer needed 3. Simplified updateBubbleTimeTip() in BubbleModel to use the new common method 4. Simplified updateTime() in AppNotifyItem to use the new common method with fallback to "Just now" 5. Updated copyright years to 2024-2026 in modified files Log: Fixed inconsistent time display between notification bubble and center Influence: 1. Test notification time display in both bubble and center views 2. Verify time formatting for different time intervals (just now, minutes, hours, yesterday, days, weeks) 3. Check locale-specific formatting works correctly 4. Verify time updates dynamically as notifications age 5. Test with notifications from different time periods fix: 集中相对时间格式化逻辑 将相对时间格式化逻辑从 BubbleModel 和 AppNotifyItem 移动到 NotifyEntity 的公共静态方法中，以消除代码重复并修复时间显示错误问题。之前， BubbleModel 使用简单的分钟计算，而 AppNotifyItem 使用 ICU 库进行本地化感 知的格式化，导致通知气泡和中心之间的时间显示不一致。现在两个组件都使用相 同的统一格式化逻辑。 变更包括： 1. 添加静态方法 NotifyEntity::formatRelativeTime()，使用 ICU 库处理所有 相对时间格式化 2. 从 bubblemodel.cpp 中移除不再需要的 QDateTime 包含 3. 简化 BubbleModel 中的 updateBubbleTimeTip() 以使用新的公共方法 4. 简化 AppNotifyItem 中的 updateTime() 以使用新的公共方法，并回退到"刚 刚" 5. 在修改的文件中更新版权年份为 2024-2026 Log: 修复通知气泡和中心之间时间显示不一致的问题 Influence: 1. 测试通知气泡和中心视图中的时间显示 2. 验证不同时间间隔（刚刚、分钟、小时、昨天、天数、周数）的时间格式化 3. 检查本地化特定的格式化是否正确工作 4. 验证时间随通知老化而动态更新 5. 测试来自不同时间段的通知 PMS: BUG-355185 Change-Id: I65ddbd0ea5ec943fd7b2c9aa55bc5ed29bd0522b
deepin pr auto review这段，这个 diff 主要进行了代码重构，将时间格式化的逻辑从 以下是对代码的详细审查意见： 1. 语法逻辑
2. 代码质量
3. 代码性能
4. 代码安全
改进建议
总结这次重构是一个正向的改动，显著提高了代码的模块化程度和可维护性，并通过复用 ICU formatter 对象优化了性能。主要的风险点在于 ICU 错误处理的缺失以及 |
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
3 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.
Moved relative time formatting logic from BubbleModel and AppNotifyItem to a common static method in NotifyEntity to eliminate code duplication and fix incorrect time display issues. Previously, BubbleModel used a simple minutes calculation while AppNotifyItem used ICU library for locale-aware formatting, causing inconsistent time display between notification bubble and center. Now both components use the same unified formatting logic.
The changes include:
Log: Fixed inconsistent time display between notification bubble and center
Influence:
fix: 集中相对时间格式化逻辑
将相对时间格式化逻辑从 BubbleModel 和 AppNotifyItem 移动到 NotifyEntity 的公共静态方法中，以消除代码重复并修复时间显示错误问题。之前，
BubbleModel 使用简单的分钟计算，而 AppNotifyItem 使用 ICU 库进行本地化感 知的格式化，导致通知气泡和中心之间的时间显示不一致。现在两个组件都使用相
同的统一格式化逻辑。
变更包括：
Log: 修复通知气泡和中心之间时间显示不一致的问题
Influence:
PMS: BUG-355185
Change-Id: I65ddbd0ea5ec943fd7b2c9aa55bc5ed29bd0522b
Summary by Sourcery
Centralize locale-aware relative time formatting for notifications and ensure consistent time display between the notification bubble and center.
Enhancements: