Merged
Conversation
全屏不显示和数据库类型加载失败的的问题 Log: 全屏不显示和数据库类型加载失败的的问题
Reviewer's GuideThis PR addresses the missing full-screen preview and database schema loading failures by enhancing the DB migration process (adding a new ClassName column with version checks), refining the full-screen logic in the thumbnail tool to use model indexes, and propagating selected paths in the classification view. Sequence diagram for full-screen preview logic update in ThumbnailListViewTools.jssequenceDiagram
actor User
participant ThumbnailListViewTools
participant thumbnailModel
participant menuItemStates
User->>ThumbnailListViewTools: Trigger executeFullScreen()
ThumbnailListViewTools->>thumbnailModel: Get selectedIndexes
ThumbnailListViewTools->>thumbnailModel: Get allPictureUrls()
ThumbnailListViewTools->>thumbnailModel: data(indexes[0], "url")
ThumbnailListViewTools->>menuItemStates: executeFullScreen(selectedUrl, allUrls)
Sequence diagram for database schema upgrade to add ClassName columnsequenceDiagram
participant DBManager
participant Database
DBManager->>Database: SELECT * FROM AlbumVersion
alt AlbumVersion does not exist
DBManager->>Database: CREATE TABLE AlbumVersion
DBManager->>Database: INSERT version = "6.0"
DBManager->>Database: CREATE ImageTable3/TrashTable3 with ClassName
else AlbumVersion exists and version < "6.0"
DBManager->>Database: ALTER TABLE ImageTable3 ADD COLUMN ClassName
DBManager->>Database: ALTER TABLE TrashTable3 ADD COLUMN ClassName
DBManager->>Database: UPDATE AlbumVersion SET version = "6.0"
end
Entity Relationship diagram for updated database tables (ImageTable3 and TrashTable3)erDiagram
ImageTable3 {
TEXT PathHash PK
TEXT FileName
TEXT ChangeTime
TEXT ImportTime
INTEGER FileType
TEXT DataHash
TEXT UID PK
TEXT ClassName
}
TrashTable3 {
TEXT PathHash PK
TEXT FileName
TEXT ChangeTime
TEXT ImportTime
INTEGER FileType
TEXT UID PK
TEXT ClassName
}
Class diagram for updated DBManager classclassDiagram
class DBManager {
+void checkDatabase()
+void checkTimeColumn(tableName: QString)
+void checkClassNameColumn(tableName: QString)
+void insertSpUID(albumName: QString, astype: AlbumDBType, UID: SpUID)
static DBManager* m_dbManager
static once_flag instanceFlag
}
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 there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/src/dbmanager/dbmanager.cpp:1891-1896` </location>
<code_context>
checkTimeColumn("TrashTable3");
+ } else {
+ // 检查版本并升级（已存在的数据库）
+ if (m_query->next()) {
+ QString currentVersion = m_query->value(0).toString();
+ qDebug() << "Current database version:" << currentVersion;
+
+ // 如果是5.9或更早版本，升级到6.0
+ if (currentVersion == "5.9" || currentVersion < "6.0") {
+ qDebug() << "Upgrading database from" << currentVersion << "to 6.0";
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Version comparison logic may not behave as expected for non-numeric versions.
Lexicographical comparison may misorder versions like "6.0.1" and "6.0". Use a method that parses and compares version components numerically for accuracy.
</issue_to_address>
### Comment 2
<location> `src/src/dbmanager/dbmanager.cpp:1974-1983` </location>
<code_context>
+void DBManager::checkClassNameColumn(const QString &tableName)
</code_context>
<issue_to_address>
**suggestion:** Adding columns with ALTER TABLE may fail if the column already exists or if there are constraints.
Consider adding explicit handling or logging for cases where the column exists with a different type or the schema is out of sync, as SQLite's ALTER TABLE has limitations, especially with constraints.
</issue_to_address>
### Comment 3
<location> `src/qml/ClassificationView/ClassificationDetailView.qml:206-208` </location>
<code_context>
}
+ // 监听选中路径变化，更新全局选中状态
+ onSelectedPathsChanged: {
+ if (show) {
+ GStatus.selectedPaths = selectedPaths
+ }
+ }
</code_context>
<issue_to_address>
**suggestion:** Directly updating global state from a view may introduce tight coupling.
Consider updating global state via a controller or signal instead of directly in the view to improve maintainability and testability.
Suggested implementation:
```
// 监听选中路径变化，通过信号通知控制器更新全局选中状态
signal selectedPathsChangedSignal(var selectedPaths)
onSelectedPathsChanged: {
if (show) {
selectedPathsChangedSignal(selectedPaths)
}
}
```
You will need to:
1. Connect the `selectedPathsChangedSignal` to a slot or handler in your controller or parent component, where you update `GStatus.selectedPaths = selectedPaths`.
2. Remove any other direct references to `GStatus.selectedPaths` from this view to maintain separation of concerns.
</issue_to_address>
### Comment 4
<location> `src/qml/Control/ListView/ThumbnailListViewTools.js:51` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Use `const` or `let` instead of `var`. ([`avoid-using-var`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-using-var))
<details><summary>Explanation</summary>`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code).
`let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than
function-scoped.
From the [Airbnb JavaScript Style Guide](https://airbnb.io/javascript/#references--prefer-const)
</details>
</issue_to_address>
### Comment 5
<location> `src/qml/Control/ListView/ThumbnailListViewTools.js:59` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Use `const` or `let` instead of `var`. ([`avoid-using-var`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-using-var))
<details><summary>Explanation</summary>`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code).
`let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than
function-scoped.
From the [Airbnb JavaScript Style Guide](https://airbnb.io/javascript/#references--prefer-const)
</details>
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+1891
to
+1896
| if (m_query->next()) { | ||
| QString currentVersion = m_query->value(0).toString(); | ||
| qDebug() << "Current database version:" << currentVersion; | ||
|
|
||
| // 如果是5.9或更早版本，升级到6.0 | ||
| if (currentVersion == "5.9" || currentVersion < "6.0") { |
There was a problem hiding this comment.
issue (bug_risk): Version comparison logic may not behave as expected for non-numeric versions.
Lexicographical comparison may misorder versions like "6.0.1" and "6.0". Use a method that parses and compares version components numerically for accuracy.
Comment on lines
+1974
to
+1983
| void DBManager::checkClassNameColumn(const QString &tableName) | ||
| { | ||
| qDebug() << "DBManager::checkClassNameColumn - Entry"; | ||
| // 检查表中是否已存在 ClassName 字段 | ||
| QString checkSql = QString("SELECT sql FROM sqlite_master WHERE type='table' AND name='%1'").arg(tableName); | ||
| if (m_query->exec(checkSql) && m_query->next()) { | ||
| QString tableSql = m_query->value(0).toString(); | ||
| // 如果表定义中不包含 ClassName，则添加该字段 | ||
| if (!tableSql.contains("ClassName", Qt::CaseInsensitive)) { | ||
| qDebug() << "Adding ClassName column to" << tableName; |
There was a problem hiding this comment.
suggestion: Adding columns with ALTER TABLE may fail if the column already exists or if there are constraints.
Consider adding explicit handling or logging for cases where the column exists with a different type or the schema is out of sync, as SQLite's ALTER TABLE has limitations, especially with constraints.
Comment on lines
+206
to
+208
| onSelectedPathsChanged: { | ||
| if (show) { | ||
| GStatus.selectedPaths = selectedPaths |
There was a problem hiding this comment.
suggestion: Directly updating global state from a view may introduce tight coupling.
Consider updating global state via a controller or signal instead of directly in the view to improve maintainability and testability.
Suggested implementation:
// 监听选中路径变化，通过信号通知控制器更新全局选中状态
signal selectedPathsChangedSignal(var selectedPaths)
onSelectedPathsChanged: {
if (show) {
selectedPathsChangedSignal(selectedPaths)
}
}
You will need to:
- Connect the
selectedPathsChangedSignalto a slot or handler in your controller or parent component, where you updateGStatus.selectedPaths = selectedPaths. - Remove any other direct references to
GStatus.selectedPathsfrom this view to maintain separation of concerns.
| function executeFullScreen() { | ||
| if (window.visibility !== Window.FullScreen && selectedUrls.length > 0) { | ||
| // 使用 selectedIndexes 来判断，因为 selectedUrls 可能为空数组 | ||
| var indexes = thumbnailModel.selectedIndexes |
There was a problem hiding this comment.
issue (code-quality): Use const or let instead of var. (avoid-using-var)
Explanation
`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code). `let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than function-scoped.From the Airbnb JavaScript Style Guide
| menuItemStates.executeFullScreen(allUrls[indexes[0]], allUrls) | ||
| // 从模型中获取选中的URL，因为 indexes 是在完整模型中的索引 | ||
| // 而 allUrls 只包含图片，索引可能对不上 | ||
| var selectedUrl = thumbnailModel.data(indexes[0], "url").toString() |
There was a problem hiding this comment.
issue (code-quality): Use const or let instead of var. (avoid-using-var)
Explanation
`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code). `let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than function-scoped.From the Airbnb JavaScript Style Guide
deepin pr auto review我来对这段代码进行审查，主要从语法逻辑、代码质量、性能和安全几个方面来分析：
onSelectedPathsChanged: {
if (show) {
GStatus.selectedPaths = selectedPaths
}
}改进建议：
Binding {
target: GStatus
property: "selectedPaths"
value: selectedPaths
when: show
}
function executeFullScreen() {
var indexes = thumbnailModel.selectedIndexes
if (window.visibility !== Window.FullScreen && indexes.length > 0) {
var allUrls = thumbnailModel.allPictureUrls()
if (indexes.length > 0) {
var selectedUrl = thumbnailModel.data(indexes[0], "url").toString()
menuItemStates.executeFullScreen(selectedUrl, allUrls)
}
}
}改进建议：
function executeFullScreen() {
try {
var indexes = thumbnailModel.selectedIndexes
if (!indexes || !Array.isArray(indexes) || indexes.length === 0) {
return
}
if (window.visibility !== Window.FullScreen) {
var allUrls = thumbnailModel.allPictureUrls()
if (indexes.length > 0 && allUrls.length > 0) {
var selectedUrl = thumbnailModel.data(indexes[0], "url")
if (selectedUrl) {
menuItemStates.executeFullScreen(selectedUrl.toString(), allUrls)
}
}
}
} catch (e) {
console.error("Error in executeFullScreen:", e)
}
}
数据库升级相关代码： void DBManager::checkDatabase() {
// ... 其他代码 ...
if (!m_query->exec("SELECT * FROM AlbumVersion")) {
if (!m_query->exec("CREATE TABLE AlbumVersion (version TEXT primary key)")) {
}
if (!m_query->exec("INSERT INTO AlbumVersion (version) VALUES (\"6.0\")")) {
}
checkTimeColumn("ImageTable3");
checkTimeColumn("TrashTable3");
} else {
if (m_query->next()) {
QString currentVersion = m_query->value(0).toString();
if (currentVersion == "5.9" || currentVersion < "6.0") {
checkClassNameColumn("ImageTable3");
checkClassNameColumn("TrashTable3");
if (!m_query->exec("UPDATE AlbumVersion SET version = \"6.0\"")) {
qWarning() << "Failed to update version:" << m_query->lastError().text();
}
}
}
}
}改进建议：
void DBManager::checkDatabase() {
// ... 其他代码 ...
if (!m_query->exec("SELECT * FROM AlbumVersion")) {
// 开始事务
if (!m_query->exec("BEGIN TRANSACTION")) {
qCritical() << "Failed to begin transaction:" << m_query->lastError().text();
return;
}
try {
if (!m_query->exec("CREATE TABLE AlbumVersion (version TEXT primary key)")) {
throw std::runtime_error("Failed to create AlbumVersion table");
}
if (!m_query->exec("INSERT INTO AlbumVersion (version) VALUES (\"6.0\")")) {
throw std::runtime_error("Failed to insert initial version");
}
checkTimeColumn("ImageTable3");
checkTimeColumn("TrashTable3");
// 提交事务
if (!m_query->exec("COMMIT")) {
throw std::runtime_error("Failed to commit transaction");
}
} catch (const std::exception& e) {
// 回滚事务
m_query->exec("ROLLBACK");
qCritical() << "Database initialization failed:" << e.what();
return;
}
} else {
if (m_query->next()) {
QString currentVersion = m_query->value(0).toString();
QVersionNumber version = QVersionNumber::fromString(currentVersion);
QVersionNumber targetVersion = QVersionNumber::fromString("6.0");
if (version < targetVersion) {
qDebug() << "Upgrading database from" << currentVersion << "to 6.0";
// 开始事务
if (!m_query->exec("BEGIN TRANSACTION")) {
qCritical() << "Failed to begin upgrade transaction:" << m_query->lastError().text();
return;
}
try {
checkClassNameColumn("ImageTable3");
checkClassNameColumn("TrashTable3");
if (!m_query->exec("UPDATE AlbumVersion SET version = \"6.0\"")) {
throw std::runtime_error("Failed to update version");
}
// 提交事务
if (!m_query->exec("COMMIT")) {
throw std::runtime_error("Failed to commit upgrade transaction");
}
qDebug() << "Database upgraded to version 6.0 successfully";
} catch (const std::exception& e) {
// 回滚事务
m_query->exec("ROLLBACK");
qCritical() << "Database upgrade failed:" << e.what();
return;
}
}
}
}
}
void DBManager::checkClassNameColumn(const QString &tableName) {
// ... 现有实现 ...
}改进建议：
void DBManager::checkClassNameColumn(const QString &tableName) {
if (tableName.isEmpty()) {
qWarning() << "Table name is empty";
return;
}
qDebug() << "DBManager::checkClassNameColumn - Entry for table:" << tableName;
try {
// 使用预处理语句
QString checkSql = "SELECT sql FROM sqlite_master WHERE type='table' AND name=?";
m_query->prepare(checkSql);
m_query->addBindValue(tableName);
if (!m_query->exec()) {
throw std::runtime_error("Failed to check table structure");
}
if (m_query->next()) {
QString tableSql = m_query->value(0).toString();
if (!tableSql.contains("ClassName", Qt::CaseInsensitive)) {
QString alterSql = QString("ALTER TABLE %1 ADD COLUMN ClassName TEXT").arg(tableName);
if (!m_query->exec(alterSql)) {
throw std::runtime_error("Failed to add ClassName column");
}
qDebug() << "Successfully added ClassName column to" << tableName;
} else {
qDebug() << tableName << "already has ClassName column";
}
} else {
throw std::runtime_error("Table not found");
}
} catch (const std::exception& e) {
qCritical() << "Error in checkClassNameColumn:" << e.what();
}
qDebug() << "DBManager::checkClassNameColumn - Exit";
}总结：
这些改进将使代码更加健壮、安全和可维护。 |
wyu71
approved these changes
Oct 20, 2025
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: lzwind, wyu71 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 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.
全屏不显示和数据库类型加载失败的的问题
Log: 全屏不显示和数据库类型加载失败的的问题
Summary by Sourcery
Implement database schema migration to ensure ClassName column exists and bump album version to 6.0, and fix full-screen preview and classification selection behaviors.
New Features:
Bug Fixes:
Enhancements: