fix(vnote): keep note order stable after pinning#374
Conversation
Use note ID as a tie-breaker when modified times are equal. 当修改时间相同时使用笔记ID作为兜底排序条件。 Log: 修复置顶笔记后同秒笔记顺序变化问题 PMS: BUG-366799 Influence: 修复后置顶笔记不会打乱同秒创建的其他笔记顺序,列表展示更稳定。
Reviewer's guide (collapsed on small PRs)Reviewer's GuideEnsures stable note ordering by refining the comparator used for sorting notes: notes are still ordered by pin (top) status and modification time, but now use note ID as a final tie-breaker when modification times are equal, and ensures the updated ordering is applied when updating top status. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Note
详情{
"src/common/VNoteMainManager.cpp": [
{
"line": " url = \"https://www.deepin.org/zh/agreement/privacy/\";",
"line_number": 1363,
"rule": "S35",
"reason": "Url link | 20e2eab189"
},
{
"line": " url = \"https://www.uniontech.com/agreement/privacy-cn\";",
"line_number": 1365,
"rule": "S35",
"reason": "Url link | 4850a00dd7"
},
{
"line": " url = \"https://www.deepin.org/en/agreement/privacy/\";",
"line_number": 1369,
"rule": "S35",
"reason": "Url link | 38d42f63bf"
},
{
"line": " url = \"https://www.uniontech.com/agreement/privacy-en\";",
"line_number": 1371,
"rule": "S35",
"reason": "Url link | f82409d3b5"
}
]
} |
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The comparator reparses NOTE_MODIFY_TIME_KEY into QDateTime on every comparison, which can be expensive during sort; consider precomputing and caching parsed timestamps (e.g., in the QVariantMap or a parallel structure) before calling std::sort.
- In updateTop, the find_if still uses the raw "noteId" string while the comparator uses NOTE_ID_KEY; aligning these to consistently use the same key constant would reduce the risk of subtle key mismatches.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The comparator reparses NOTE_MODIFY_TIME_KEY into QDateTime on every comparison, which can be expensive during sort; consider precomputing and caching parsed timestamps (e.g., in the QVariantMap or a parallel structure) before calling std::sort.
- In updateTop, the find_if still uses the raw "noteId" string while the comparator uses NOTE_ID_KEY; aligning these to consistently use the same key constant would reduce the risk of subtle key mismatches.
## Individual Comments
### Comment 1
<location path="src/common/VNoteMainManager.cpp" line_range="859-861" />
<code_context>
notesDataList.append(data);
}
+
std::sort(notesDataList.begin(), notesDataList.end(), NoteCompare());
+
auto it = std::find_if(notesDataList.begin(), notesDataList.end(), [id](const QVariantMap &item)->bool {
return item.value("noteId").toInt() == id;
});
</code_context>
<issue_to_address>
**issue (bug_risk):** Use NOTE_ID_KEY instead of the literal "noteId" to keep the key usage consistent.
Immediately after inserting `NOTE_ID_KEY` into `data`, the predicate still uses the literal "noteId". If `NOTE_ID_KEY` ever changes, `find_if` will stop finding the correct entry. Use `NOTE_ID_KEY` in the predicate as well:
```cpp
auto it = std::find_if(notesDataList.begin(), notesDataList.end(), [id](const QVariantMap &item)->bool {
return item.value(NOTE_ID_KEY).toInt() == id;
});
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: dengzhongyuan365-dev, lzwind 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 |
|
/forcemerge |
|
This pr force merged! (status: unstable) |
1941317
into
linuxdeepin:develop/snipe
deepin pr auto review★ 总体评分:100分■ 【总体评价】
■ 【详细分析】
■ 【改进建议代码示例】 // 虽然当前代码已非常优秀,但可进一步增加对异常时间数据的容错处理,提升健壮性
struct NoteCompare {
bool operator()(const QVariantMap &a, const QVariantMap &b) const {
// 置顶笔记排在普通笔记前面
bool aIsTop = a.value(NOTE_ISTOP_KEY).toBool();
bool bIsTop = b.value(NOTE_ISTOP_KEY).toBool();
if (aIsTop != bIsTop) {
return aIsTop > bIsTop;
}
// 相同置顶状态下,按最近修改时间降序排列
QDateTime aModifyTime = QDateTime::fromString(a.value(NOTE_MODIFY_TIME_KEY).toString(), "yyyy-MM-dd hh:mm:ss");
QDateTime bModifyTime = QDateTime::fromString(b.value(NOTE_MODIFY_TIME_KEY).toString(), "yyyy-MM-dd hh:mm:ss");
// 增加时间有效性检查,防止底层数据损坏导致解析出无效时间
if (aModifyTime.isValid() && bModifyTime.isValid()) {
if (aModifyTime != bModifyTime) {
return aModifyTime > bModifyTime;
}
} else if (aModifyTime.isValid() != bModifyTime.isValid()) {
// 有效时间优先于无效时间
return aModifyTime.isValid();
}
// 修改时间相同或无效时,按笔记ID降序避免同秒创建的笔记顺序不稳定
return a.value(NOTE_ID_KEY).toInt() > b.value(NOTE_ID_KEY).toInt();
}
}; |
Use note ID as a tie-breaker when modified times are equal.
当修改时间相同时使用笔记ID作为兜底排序条件。
Log: 修复置顶笔记后同秒笔记顺序变化问题
PMS: BUG-366799
Influence: 修复后置顶笔记不会打乱同秒创建的其他笔记顺序,列表展示更稳定。
Summary by Sourcery
Bug Fixes: