From dbea6e40c369c7261d3bf7adbe7890bad54eda0c Mon Sep 17 00:00:00 2001 From: Wu JiangYu Date: Tue, 27 Oct 2026 13:20:40 +0800 Subject: [PATCH] refactor: fix app group management and folder IDs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. Fix folder ID parsing off-by-one bug and standardize canonical prefix from "internal/folder/" to "internal/folders/" 2. Support legacy ID formats ("internal/folder/", "internal/group/") during loading while always writing the canonical plural form 3. Decouple AppGroupManager from AMAppItemModel, using QAbstractItemModel allowing in-memory test sources 4. Add DConfig injection constructor for testability with memory backend 5. Coalesce reference model changes into a single refresh using a zero- delay timer, avoiding mutations during begin/endRemoveRows transactions 6. Guard against stale drag coordinates, out-of-range page indices, and nested folder drops 7. Handle last-child removal from a folder when dropped onto its own folder slot 8. Detect ready property on reference model to avoid pruning folders before app metadata is loaded 9. Migrate legacy launchpad INI only once with ArrangementMigrated flag, keeping INI as backup 10. Add QML-invokable APIs: itemPosition, folderInfo, pageCount, updateFolderName, createEmptyPage, removeEmptyPages, rearrange, bringToFront 11. Rename bringToFromt to bringToFront and accept .desktop suffix 12. Add comprehensive test suite appgroupmanagertests.cpp covering folder creation, renaming, moves, migration, and persistence 13. Fix ItemsPage firstNItems(0) and bounds handling in insertItem/ insertItemToPage/moveItemPosition Log: Fixed folder management stability, drag-drop edge cases, and legacy config migration Influence: 1. Verify folder creation by dragging one app onto another 2. Test folder rename and persistence across restarts 3. Test dragging apps in/out of folders at page boundaries 4. Verify last child app can be moved out of a folder without losing position 5. Test folder cannot be nested into another folder 6. Verify stale drag drops are ignored gracefully 7. Test launchpad INI migration runs only once and is preserved 8. Verify apps with .desktop suffix are handled correctly 9. Test empty page creation and removal 10. Verify top-level always exists even with empty app set 11. Run appgroupmanager_tests and itemspage_tests unit tests refactor: 修复应用分组管理和文件夹ID 1. 修复文件夹ID解析的偏移错误，并将标准前缀从 "internal/folder/" 改为 "internal/folders/" 2. 加载时支持旧格式ID（"internal/folder/"、"internal/group/"），但写入时 始终使用规范的复数形式 3. 将AppGroupManager与AMAppItemModel解耦，改用QAbstractItemModel以支持内 存测试源 4. 添加DConfig注入构造函数以便使用内存后端进行测试 5. 使用零延迟定时器合并参考模型变更，避免在begin/endRemoveRows事务期间修 改数据 6. 对过期的拖拽坐标、越界页码和嵌套文件夹拖放进行防护 7. 处理文件夹中最后一个子项被拖放到其自身文件夹位置的情况 8. 检测参考模型的ready属性，避免在应用元数据加载前清理文件夹 9. 使用ArrangementMigrated标志确保旧版启动器INI仅迁移一次，同时保留INI作 为备份 10. 添加QML可调用的API：itemPosition、folderInfo、pageCount、 updateFolderName、createEmptyPage、removeEmptyPages、rearrange、 bringToFront 11. 将bringToFromt重命名为bringToFront并支持.desktop后缀 12. 添加全面的测试套件appgroupmanagertests.cpp，覆盖文件夹创建、重命名、 移动、迁移和持久化 13. 修复ItemsPage firstNItems(0)以及insertItem/insertItemToPage/ moveItemPosition的边界处理 Log: 修复文件夹管理稳定性、拖放边界情况和旧版配置迁移 Influence: 1. 验证通过将一个应用拖到另一个应用上创建文件夹 2. 测试文件夹重命名及重启后的持久化 3. 测试在页面边界处将应用拖入/拖出文件夹 4. 验证最后一个子应用可以移出文件夹且不丢失位置 5. 测试文件夹不能嵌套到另一个文件夹中 6. 验证过期拖放操作被优雅忽略 7. 测试启动器INI迁移只运行一次并被保留 8. 验证带.desktop后缀的应用被正确处理 9. 测试空页面的创建和删除 10. 验证即使应用集为空，顶层也始终存在 11. 运行appgroupmanager_tests和itemspage_tests单元测试 PMS: TASK-396759 --- applets/dde-apps/appgroup.cpp | 25 +- applets/dde-apps/appgroupmanager.cpp | 344 ++++++++++++----- applets/dde-apps/appgroupmanager.h | 29 +- .../configs/org.deepin.ds.dde-apps.json | 9 + applets/dde-apps/itemspage.cpp | 23 +- tests/applets/CMakeLists.txt | 17 +- tests/applets/appgroupmanagertests.cpp | 361 ++++++++++++++++++ tests/applets/itemspagetests.cpp | 21 +- 8 files changed, 687 insertions(+), 142 deletions(-) create mode 100644 tests/applets/appgroupmanagertests.cpp diff --git a/applets/dde-apps/appgroup.cpp b/applets/dde-apps/appgroup.cpp index c37a6125f..5cc2e6c27 100644 --- a/applets/dde-apps/appgroup.cpp +++ b/applets/dde-apps/appgroup.cpp @@ -1,4 +1,4 @@ -// SPDX-FileCopyrightText: 2024 UnionTech Software Technology Co., Ltd. +// SPDX-FileCopyrightText: 2024-2026 UnionTech Software Technology Co., Ltd. // // SPDX-License-Identifier: GPL-3.0-or-later @@ -15,11 +15,11 @@ Q_LOGGING_CATEGORY(appGroupLog, "org.deepin.dde.shell.dde-apps.appgroup") namespace apps { AppGroup::AppGroup(const QString &groupId, const QString &name, const QList &appIDs) : AppItem(groupId, AppItemModel::FolderItemType) - , m_itemsPage(new ItemsPage(name, groupId == QStringLiteral("internal/folder/0") ? (4 * 8) : (3 * 4))) + , m_itemsPage(new ItemsPage(name, parseGroupId(groupId) == 0 ? (4 * 8) : (3 * 4))) { setItemsPerPage(m_itemsPage->maxItemCountPerPage()); setAppName(m_itemsPage->name()); - // folder id is a part of its groupId: "internal/folder/{folderId}" + // folder id is a part of its groupId: "internal/folders/{folderId}" setFolderId(parseGroupId(groupId)); for (const QStringList &items : appIDs) { @@ -49,19 +49,28 @@ ItemsPage *AppGroup::itemsPage() bool AppGroup::idIsFolder(const QString & id) { - return id.startsWith(QStringLiteral("internal/folder/")); + return id.startsWith(QStringLiteral("internal/folders/")) && parseGroupId(id) >= 0; } QString AppGroup::groupIdFromNumber(int groupId) { - return QStringLiteral("internal/folder/%1").arg(groupId); + return QStringLiteral("internal/folders/%1").arg(groupId); } int AppGroup::parseGroupId(const QString & id) { - using namespace std::string_view_literals; - constexpr size_t len = "internal/folder/"sv.size(); - return QStringView{id}.mid(len + 1).toInt(); + // Accept legacy IDs while loading, always write the canonical plural form. + const QStringList prefixes{QStringLiteral("internal/folders/"), QStringLiteral("internal/folder/"), QStringLiteral("internal/group/")}; + QString number = id; + for (const auto &prefix : prefixes) { + if (id.startsWith(prefix)) { + number = id.mid(prefix.size()); + break; + } + } + bool ok = false; + const int result = number.toInt(&ok); + return ok && result >= 0 ? result : -1; } void AppGroup::setItemsPerPage(int number) diff --git a/applets/dde-apps/appgroupmanager.cpp b/applets/dde-apps/appgroupmanager.cpp index 157fc1771..63ff9bc0a 100644 --- a/applets/dde-apps/appgroupmanager.cpp +++ b/applets/dde-apps/appgroupmanager.cpp @@ -5,49 +5,46 @@ #include "appgroupmanager.h" #include "appgroup.h" #include "itemspage.h" -#include "amappitemmodel.h" -#include "amappitem.h" +#include "appitemmodel.h" +#include +#include +#include +#include +#include #define TOPLEVEL_FOLDERID 0 namespace apps { -AppGroupManager::AppGroupManager(AMAppItemModel * referenceModel, QObject *parent) +AppGroupManager::AppGroupManager(QAbstractItemModel *referenceModel, QObject *parent) + : AppGroupManager(referenceModel, Dtk::Core::DConfig::create("org.deepin.dde.shell", "org.deepin.ds.dde-apps"), parent) +{ + m_config->setParent(this); +} + +AppGroupManager::AppGroupManager(QAbstractItemModel *referenceModel, Dtk::Core::DConfig *config, QObject *parent) : QStandardItemModel(parent) , m_referenceModel(referenceModel) - , m_dumpTimer(new QTimer(this)) - , m_config(Dtk::Core::DConfig::create("org.deepin.dde.shell", "org.deepin.ds.dde-apps", "", this)) + , m_config(config) + , m_referenceSyncTimer(this) { - m_dumpTimer->setSingleShot(true); - m_dumpTimer->setInterval(1000); - launchpadArrangementConfigMigration(); loadAppGroupInfo(); - connect(m_referenceModel, &AMAppItemModel::rowsInserted, this, [this](){ - onReferenceModelChanged(); - saveAppGroupInfo(); - }); - connect(m_referenceModel, &AMAppItemModel::rowsRemoved, this, [this](){ - onReferenceModelChanged(); - saveAppGroupInfo(); - }); - connect(m_dumpTimer, &QTimer::timeout, this, [this](){ - saveAppGroupInfo(); - }); - connect(this, &AppGroupManager::dataChanged, this, &AppGroupManager::saveAppGroupInfo); -} - -QVariant AppGroupManager::data(const QModelIndex &index, int role) const -{ - if (!index.isValid()) - return QVariant(); - - if (role == GroupIdRole) { - return index.row(); - } - - return QStandardItemModel::data(index, role); + // Source adapters can still be between begin/endRemoveRows while the + // source emits rowsRemoved. Mutate folders only after those transactions + // finish, and coalesce application metadata changes into one refresh. + m_referenceSyncTimer.setSingleShot(true); + m_referenceSyncTimer.setInterval(0); + connect(&m_referenceSyncTimer, &QTimer::timeout, this, &AppGroupManager::onReferenceModelChanged); + if (!m_referenceModel) return; + connect(m_referenceModel, &QAbstractItemModel::rowsInserted, this, &AppGroupManager::scheduleReferenceSync); + connect(m_referenceModel, &QAbstractItemModel::rowsRemoved, this, &AppGroupManager::scheduleReferenceSync); + connect(m_referenceModel, &QAbstractItemModel::modelReset, this, &AppGroupManager::scheduleReferenceSync); + if (m_referenceModel->metaObject()->indexOfSignal("readyChanged(bool)") >= 0) + connect(m_referenceModel, SIGNAL(readyChanged(bool)), this, SLOT(scheduleReferenceSync())); + connect(m_referenceModel, &QAbstractItemModel::dataChanged, this, &AppGroupManager::scheduleReferenceSync); + onReferenceModelChanged(); } // Find the item's location. If folderId is -1, search all folders. @@ -72,15 +69,14 @@ ItemPosition AppGroupManager::findItem(const QString &appId, int folderId) void AppGroupManager::appendItemToGroup(const QString &appId, int groupId) { auto folder = group(groupId); - Q_CHECK_PTR(folder); + if (!folder) return; folder->itemsPage()->appendItem(appId); } bool AppGroupManager::removeItemFromGroup(const QString &appId, int groupId) { auto folder = group(groupId); - Q_CHECK_PTR(folder); - return folder->itemsPage()->removeItem(appId); + return folder && folder->itemsPage()->removeItem(appId); } QModelIndex AppGroupManager::groupIndexById(int groupId) @@ -121,12 +117,12 @@ ItemsPage * AppGroupManager::groupPages(int groupId) return folder->itemsPage(); } -void AppGroupManager::bringToFromt(const QString & id) +void AppGroupManager::bringToFront(const QString & id) { - const ItemPosition origPos = findItem(id); + const ItemPosition origPos = findItem(id.endsWith(".desktop") ? id.chopped(8) : id); // can only bring top-level item to front - Q_ASSERT(origPos.group() != TOPLEVEL_FOLDERID); + if (origPos.group() != TOPLEVEL_FOLDERID) { return; } @@ -141,7 +137,7 @@ void AppGroupManager::bringToFromt(const QString & id) saveAppGroupInfo(); - // TODO: emit signal to refresh the view + notifyArrangementChanged(); } void AppGroupManager::commitRearrangeOperation(const QString & dragId, const QString & dropId, DndOperation operation, int pageHint) @@ -151,27 +147,45 @@ void AppGroupManager::commitRearrangeOperation(const QString & dragId, const QSt const ItemPosition dragOrigPos = findItem(dragId); const ItemPosition dropOrigPos = findItem(dropId); - Q_ASSERT(dragOrigPos.group() != -1); + if (dragOrigPos.group() == -1) { qWarning() << "Cannot found" << dragId << "in current item arrangement."; return; } + if (operation < DndPrepend || operation > DndAppend) return; + if (dropId != AppGroup::groupIdFromNumber(0) && dropOrigPos.group() < 0) return; + if (operation != DndJoin && dropOrigPos.group() < 0) return; + // A folder may only be reordered at top level. + if (operation != DndJoin && AppGroup::idIsFolder(dragId) && dropOrigPos.group() != 0) return; + if (operation != DndOperation::DndJoin) { // move to dropId's front or back if (dragOrigPos.group() == dropOrigPos.group()) { // same folder item re-arrangement ItemsPage * folder = groupPages(dropOrigPos.group()); - folder->moveItemPosition(dragOrigPos.page(), dragOrigPos.pos(), dropOrigPos.page(), dropOrigPos.pos(), operation == DndOperation::DndAppend); + int destination = dropOrigPos.pos() + (operation == DndAppend ? 1 : 0); + if (dragOrigPos.page() == dropOrigPos.page() && dragOrigPos.pos() < destination) --destination; + folder->moveItemPosition(dragOrigPos.page(), dragOrigPos.pos(), dropOrigPos.page(), destination, false); } else { // different folder item arrangement ItemsPage * srcFolder = groupPages(dragOrigPos.group()); ItemsPage * dstFolder = groupPages(dropOrigPos.group()); srcFolder->removeItem(dragId); - if (srcFolder->pageCount() == 0 && srcFolder != dstFolder) { + const bool replaceSourceFolder = srcFolder->itemCount() == 0 + && dropId == AppGroup::groupIdFromNumber(dragOrigPos.group()); + if (replaceSourceFolder) { + // The last child takes its folder's slot for either edge. Insert + // before deleting the folder so its position and page stay valid. + dstFolder->insertItem(dragId, dropOrigPos.page(), dropOrigPos.pos()); + } + if (srcFolder->pageCount() == 0 && srcFolder != dstFolder && dragOrigPos.group() != 0) { removeGroup(dragOrigPos.group()); } - dstFolder->insertItem(dropId, dropOrigPos.page(), dropOrigPos.pos()); + if (!replaceSourceFolder) { + const auto target = findItem(dropId); + dstFolder->insertItem(dragId, target.page(), target.pos() + (operation == DndAppend ? 1 : 0)); + } } } else { if (AppGroup::idIsFolder(dragId) && dropId != "internal/folders/0") return; // cannot drag folder onto something @@ -184,6 +198,7 @@ void AppGroupManager::commitRearrangeOperation(const QString & dragId, const QSt // drop into existing folder ItemsPage * dstFolder = groupPages(AppGroup::parseGroupId(dropId)); const int fromPage = dragOrigPos.page(); + if (!dstFolder || pageHint < -1 || pageHint > dstFolder->pageCount()) return; const int toPage = pageHint; if (srcFolder == dstFolder) { if (srcFolder->pageCount() == 1 || @@ -194,45 +209,39 @@ void AppGroupManager::commitRearrangeOperation(const QString & dragId, const QSt // hold the empty page avoid access out of page range! srcFolder->removeItem(dragId, false); - if (srcFolder->itemCount() == 0 && srcFolder != dstFolder) { - removeGroup(dragOrigPos.group()); - } dstFolder->insertItemToPage(dragId, toPage); - - // clear empty page srcFolder->removeEmptyPages(); + if (srcFolder != dstFolder && srcFolder->itemCount() == 0 && dragOrigPos.group() != 0) + removeGroup(dragOrigPos.group()); } else { srcFolder->removeItem(dragId); // we need the app info for the new folder name... - AMAppItem * item = m_referenceModel->appItem(dropId); - AppItemModel::DDECategories dropCategories = item ? item->ddeCategories() : AppItemModel::Others; + const auto app = m_referenceModel + ? m_referenceModel->match(m_referenceModel->index(0, 0), AppItemModel::DesktopIdRole, dropId, 1, Qt::MatchExactly).value(0) + : QModelIndex(); + const int dropCategories = app.isValid() ? app.data(AppItemModel::DDECategoryRole).toInt() : AppItemModel::Others; // make a new folder, move two items into the folder QString dstFolderId = assignGroupId(); - appendGroup(dstFolderId, "internal/category/" + QString::number(dropCategories), {{dragId, dropId}}); + appendGroup(dstFolderId, "internal/category/" + QString::number(dropCategories), {{dropId, dragId}}); if (srcFolder->pageCount() == 0 && dragOrigPos.group() != 0) { removeGroup(dragOrigPos.group()); } auto * topLevel = groupPages(0); - topLevel->insertItem(dstFolderId, dropOrigPos.page(), dropOrigPos.pos()); + const auto target = findItem(dropId); + topLevel->insertItem(dstFolderId, target.page(), target.pos()); topLevel->removeItem(dropId); } } saveAppGroupInfo(); - // TODO: emit signal to refresh the view + notifyArrangementChanged(); } QVariantList AppGroupManager::fromListOfStringList(const QList & list) { QVariantList data; - std::transform(list.begin(), list.end(), std::back_inserter(data), [](const QStringList &items) { - QVariantList tmp; - std::transform(items.begin(), items.end(), std::back_inserter(tmp), [](const auto &item) { - return QVariant::fromValue(item); - }); - return items; - }); + for (const auto &items : list) data.append(QVariant::fromValue(items)); return data; } @@ -246,7 +255,7 @@ void AppGroupManager::onReferenceModelChanged() } // Avoid remove all existing records when first time (AM model is not ready). - if (m_referenceModel->rowCount() == 0) { + if (m_referenceModel->property("ready").isValid() && !m_referenceModel->property("ready").toBool()) { qDebug() << "referenceModel not ready, wait for next time"; return; } @@ -267,6 +276,11 @@ void AppGroupManager::onReferenceModelChanged() } } + // Top level also contains folder IDs, not just desktop IDs. + for (int i = 0; i < rowCount(); ++i) { + auto folder = group(index(i, 0)); + if (folder->folderId() > 0) appSet.insert(folder->appId()); + } // remove all non-existing ones for (int i = rowCount() - 1; i >= 0; i--) { auto folder = group(index(i, 0)); @@ -280,19 +294,49 @@ void AppGroupManager::onReferenceModelChanged() } } - // TODO: emit datachanged signal? - // TODO: save item arrangement to user data? + saveAppGroupInfo(); + notifyArrangementChanged(); } void AppGroupManager::launchpadArrangementConfigMigration() { + const auto saved = m_config->value("Groups").toList(); + // The old applet used row numbers as folder IDs. Its separate migration + // flag cannot certify these data; keep the version with the root record. + for (const auto &value : saved) { + const auto record = value.toMap(); + if (AppGroup::parseGroupId(record.value("groupId").toString()) == TOPLEVEL_FOLDERID + && record.value("arrangementVersion").toInt() == 1) return; + } + const auto importGroups = [this](QVariantList groups) { + bool hasRoot = false; + for (auto &value : groups) { + auto record = value.toMap(); + if (AppGroup::parseGroupId(record.value("groupId").toString()) != TOPLEVEL_FOLDERID) continue; + record.insert("arrangementVersion", 1); + value = record; + hasRoot = true; + } + if (!hasRoot) groups.append(QVariantMap{{"groupId", AppGroup::groupIdFromNumber(TOPLEVEL_FOLDERID)}, + {"name", "Top Level"}, {"arrangementVersion", 1}}); + m_config->setValue("Groups", groups); + m_config->setValue("ArrangementMigrated", true); + }; const QString arrangementSettingBasePath(QStandardPaths::writableLocation(QStandardPaths::GenericConfigLocation)); const QString arrangementSettingPath(QDir(arrangementSettingBasePath).absoluteFilePath("deepin/dde-launchpad/item-arrangement.ini")); - if (!QFile::exists(arrangementSettingPath)) return; + if (!QFile::exists(arrangementSettingPath)) { + importGroups(saved); + return; + } QSettings itemArrangementSettings(arrangementSettingPath, QSettings::NativeFormat); itemArrangementSettings.beginGroup("fullscreen"); const QStringList folderGroups(itemArrangementSettings.childGroups()); + if (!folderGroups.contains("toplevel")) { + // A partial INI must not replace a complete saved layout. + importGroups(saved); + return; + } QVariantList list; @@ -303,7 +347,12 @@ void AppGroupManager::launchpadArrangementConfigMigration() QVariantMap valueMap; valueMap.insert("name", folderName); - valueMap.insert("groupId", groupName.toInt()); + const int groupId = groupName == "toplevel" ? 0 : AppGroup::parseGroupId(groupName); + if (groupId < 0) { + itemArrangementSettings.endGroup(); + continue; + } + valueMap.insert("groupId", groupId); QList pages; for (int i = 0; i < pageCount; i++) { @@ -311,7 +360,7 @@ void AppGroupManager::launchpadArrangementConfigMigration() // check if items have ".desktop" suffix, and remove the suffix std::transform(items.begin(), items.end(), items.begin(), [](QString &item) { if (item.endsWith(".desktop")) { - item.remove(".desktop"); + item.chop(8); } return item; }); @@ -324,37 +373,51 @@ void AppGroupManager::launchpadArrangementConfigMigration() itemArrangementSettings.endGroup(); } - m_config->setValue("Groups", list); - - // TODO: delete the old configuration file + // Keep the original INI untouched; subsequent starts use Groups. + importGroups(list); } void AppGroupManager::loadAppGroupInfo() { - auto groups = m_config->value("Groups").toList(); - - for (int i = 0; i < groups.length(); i++) { - auto group = groups[i].toMap(); - auto groupId = group.value("groupId", "").toString(); - auto name = group.value("name", "").toString(); - auto pages = group.value("appItems", QVariantList()).toList(); - QList items; - - for (int j = 0; j < pages.length(); j++) { - auto page = pages[j].toStringList(); - items << page; - } - - if (groupId.isEmpty()) { - groupId = assignGroupId(); + const auto saved = m_config->value("Groups").toList(); + QHash groups; + QList order; + for (const auto &value : saved) { + const auto record = value.toMap(); + const int id = AppGroup::parseGroupId(record.value("groupId").toString()); + if (id < 0 || groups.contains(id)) continue; + groups.insert(id, record); + if (id != 0) order.append(id); + } + // Children take precedence over stale top-level copies from older stores. + // Each application/folder must occur once, and folders cannot be nested. + order.append(0); + QSet placed; + for (const int groupId : order) { + const auto record = groups.value(groupId); + QList pages; + for (const auto &value : record.value("appItems").toList()) { + QStringList page; + for (auto id : value.toStringList()) { + if (id.startsWith("internal/")) { + const int folderId = AppGroup::parseGroupId(id); + if (groupId != 0 || folderId <= 0 || !groups.contains(folderId)) continue; + id = AppGroup::groupIdFromNumber(folderId); + } else if (id.endsWith(".desktop")) { + id.chop(8); + } + if (id.isEmpty() || placed.contains(id)) continue; + placed.insert(id); + page.append(id); + } + if (!page.isEmpty()) pages.append(page); } - appendGroup(groupId, name, items); + appendGroup(groupId, record.value("name", groupId == 0 ? "Top Level" : "").toString(), pages); } - - // always ensure top-level group exists - if (rowCount() == 0) { - auto p = appendGroup(assignGroupId(), "Top Level", {}); - Q_ASSERT(p->folderId() == TOPLEVEL_FOLDERID); + // Recover folders saved without a top-level reference by the old manager. + for (const int groupId : order) { + const auto id = AppGroup::groupIdFromNumber(groupId); + if (groupId > 0 && !placed.contains(id)) appendItemToGroup(id, 0); } } @@ -366,6 +429,7 @@ void AppGroupManager::saveAppGroupInfo() QVariantMap valueMap; valueMap.insert("name", folder->data(AppItemModel::NameRole)); valueMap.insert("groupId", folder->appId()); + if (folder->folderId() == TOPLEVEL_FOLDERID) valueMap.insert("arrangementVersion", 1); valueMap.insert("appItems", fromListOfStringList(folder->pages())); list << valueMap; } @@ -375,18 +439,12 @@ void AppGroupManager::saveAppGroupInfo() QString AppGroupManager::assignGroupId() const { - QStringList knownGroupIds; - for (int i = 0; i < rowCount(); i++) { - auto group = index(i, 0); - knownGroupIds.append(group.data(AppItemModel::DesktopIdRole).toString()); - } - - int idNumber = 0; - while (knownGroupIds.contains(QString("internal/group/%1").arg(idNumber))) { - idNumber++; - } - - return QString("internal/group/%1").arg(idNumber); + QSet ids; + for (int i = 0; i < rowCount(); ++i) + ids.insert(static_cast(item(i))->folderId()); + int number = 1; + while (ids.contains(number)) ++number; + return AppGroup::groupIdFromNumber(number); } AppGroup * AppGroupManager::appendGroup(int groupId, QString groupName, const QList &appItemIDs) @@ -404,9 +462,83 @@ AppGroup * AppGroupManager::appendGroup(QString groupId, QString groupName, cons void AppGroupManager::removeGroup(int groupId) { - auto groupIndex = groupIndexById(groupId); - Q_ASSERT(groupIndex.isValid()); - removeRow(groupIndex.row()); + if (groupId <= 0) return; + auto idx = groupIndexById(groupId); + if (!idx.isValid()) return; + groupPages(0)->removeItem(AppGroup::groupIdFromNumber(groupId)); + removeRow(idx.row()); +} + +QHash AppGroupManager::roleNames() const +{ + return {{GroupIdRole, "groupId"}, {AppItemModel::DesktopIdRole, "desktopId"}, + {AppItemModel::NameRole, "name"}}; +} + +QVariantMap AppGroupManager::itemPosition(const QString &id) +{ + const auto position = findItem(id.endsWith(".desktop") ? id.chopped(8) : id); + return {{"group", position.group()}, {"page", position.page()}, {"pos", position.pos()}}; +} + +QVariantMap AppGroupManager::folderInfo(const QString &id) +{ + auto folder = group(AppGroup::parseGroupId(id)); + if (!folder) return {}; + QVariantList contents; + for (const auto &appId : folder->itemsPage()->allArrangedItems()) { + if (!m_referenceModel) break; // Missing application metadata is not folder deletion. + const auto app = m_referenceModel->match(m_referenceModel->index(0, 0), AppItemModel::DesktopIdRole, appId, 1, Qt::MatchExactly).value(0); + if (!app.isValid()) continue; + contents.append(QVariantMap{{"id", appId}, {"name", app.data(AppItemModel::NameRole)}, + {"iconName", app.data(AppItemModel::IconNameRole)}}); + } + return {{"id", folder->appId()}, {"name", folder->itemsPage()->name()}, + {"contents", contents}, {"pageCount", folder->itemsPage()->pageCount()}}; +} + +int AppGroupManager::pageCount(int folderId) +{ + const auto pages = groupPages(folderId); + return pages ? pages->pageCount() : 0; +} + +void AppGroupManager::updateFolderName(int folderId, const QString &name) +{ + auto folder = group(folderId); + if (!folder || folderId == 0) return; + folder->itemsPage()->setName(name); + folder->setData(name, AppItemModel::NameRole); + saveAppGroupInfo(); + notifyArrangementChanged(); +} + +int AppGroupManager::createEmptyPage(int folderId) +{ + auto pages = groupPages(folderId); + if (!pages) return -1; + pages->appendEmptyPage(); + notifyArrangementChanged(); + return pages->pageCount() - 1; +} + +void AppGroupManager::removeEmptyPages() +{ + for (int i = 0; i < rowCount(); ++i) group(index(i, 0))->itemsPage()->removeEmptyPages(); + saveAppGroupInfo(); + notifyArrangementChanged(); +} + +void AppGroupManager::rearrange(const QString &dragId, const QString &dropId, int operation, int pageHint) +{ + const auto normalize = [](const QString &id) { return id.endsWith(".desktop") ? id.chopped(8) : id; }; + commitRearrangeOperation(normalize(dragId), normalize(dropId), static_cast(operation), pageHint); +} + +void AppGroupManager::notifyArrangementChanged() +{ + if (rowCount()) emit dataChanged(index(0, 0), index(rowCount() - 1, 0)); + emit arrangementChanged(); } } diff --git a/applets/dde-apps/appgroupmanager.h b/applets/dde-apps/appgroupmanager.h index 5e96f1ee4..f584f567c 100644 --- a/applets/dde-apps/appgroupmanager.h +++ b/applets/dde-apps/appgroupmanager.h @@ -34,7 +34,6 @@ struct ItemPosition int pos() const { return m_positionInPage; }; }; -class AMAppItemModel; class AppGroup; /*! \brief AppGroupManager is a interface to manager all groups. * @@ -55,9 +54,18 @@ class AppGroupManager : public QStandardItemModel DndAppend = 1 }; Q_ENUM(DndOperation) - explicit AppGroupManager(AMAppItemModel * referenceModel, QObject* parent = nullptr); + explicit AppGroupManager(QAbstractItemModel *referenceModel, QObject *parent = nullptr); + AppGroupManager(QAbstractItemModel *referenceModel, Dtk::Core::DConfig *config, QObject *parent); - QVariant data(const QModelIndex &index, int role = GroupIdRole) const override; + QHash roleNames() const override; + Q_INVOKABLE QVariantMap itemPosition(const QString &id); + Q_INVOKABLE QVariantMap folderInfo(const QString &id); + Q_INVOKABLE int pageCount(int folderId = 0); + Q_INVOKABLE void updateFolderName(int folderId, const QString &name); + Q_INVOKABLE int createEmptyPage(int folderId = 0); + Q_INVOKABLE void removeEmptyPages(); + Q_INVOKABLE void rearrange(const QString &dragId, const QString &dropId, int operation, int pageHint = -1); + Q_INVOKABLE void bringToFront(const QString &id); Q_INVOKABLE ItemPosition findItem(const QString &appId, int folderId = -1); Q_INVOKABLE void appendItemToGroup(const QString &appId, int groupId); @@ -66,13 +74,19 @@ class AppGroupManager : public QStandardItemModel AppGroup * group(int groupId); AppGroup * group(QModelIndex idx); Q_INVOKABLE ItemsPage * groupPages(int groupId); - Q_INVOKABLE void bringToFromt(const QString & id); Q_INVOKABLE void commitRearrangeOperation(const QString & dragId, const QString & dropId, DndOperation operation, int pageHint = -1); static QVariantList fromListOfStringList(const QList & list); -private: +signals: + void arrangementChanged(); + +private slots: void onReferenceModelChanged(); + void scheduleReferenceSync() { m_referenceSyncTimer.start(); } + +private: + void notifyArrangementChanged(); void launchpadArrangementConfigMigration(); void loadAppGroupInfo(); @@ -84,9 +98,8 @@ class AppGroupManager : public QStandardItemModel void removeGroup(int groupId); private: - bool m_appGroupInitialized; - QPointer m_referenceModel; - QTimer* m_dumpTimer; + QPointer m_referenceModel; Dtk::Core::DConfig *m_config; + QTimer m_referenceSyncTimer; }; } diff --git a/applets/dde-apps/configs/org.deepin.ds.dde-apps.json b/applets/dde-apps/configs/org.deepin.ds.dde-apps.json index 624cee898..b9b511ea4 100644 --- a/applets/dde-apps/configs/org.deepin.ds.dde-apps.json +++ b/applets/dde-apps/configs/org.deepin.ds.dde-apps.json @@ -11,6 +11,15 @@ "description": "用于记录运行过程中产生的程序组信息", "permissions": "readwrite", "visibility": "private" + }, + "ArrangementMigrated": { + "value": false, + "serial": 0, + "flags": [], + "name": "Launcher arrangement migration completed", + "description": "Import the legacy launcher INI once, keeping it as a backup", + "permissions": "readwrite", + "visibility": "private" } } } diff --git a/applets/dde-apps/itemspage.cpp b/applets/dde-apps/itemspage.cpp index 4c068cb16..d72d7cd8a 100644 --- a/applets/dde-apps/itemspage.cpp +++ b/applets/dde-apps/itemspage.cpp @@ -1,4 +1,4 @@ -// SPDX-FileCopyrightText: 2023 UnionTech Software Technology Co., Ltd. +// SPDX-FileCopyrightText: 2023-2026 UnionTech Software Technology Co., Ltd. // // SPDX-License-Identifier: GPL-3.0-or-later @@ -49,12 +49,13 @@ int ItemsPage::pageCount() const QStringList ItemsPage::items(int page) const { - return m_pages.at(page); + return m_pages.value(page); } QStringList ItemsPage::firstNItems(int count) { QStringList result; + if (count <= 0) return result; for (const QStringList & pageItems : std::as_const(m_pages)) { for (const QString & item : pageItems) { @@ -128,10 +129,9 @@ void ItemsPage::appendItem(const QString id, int page) void ItemsPage::insertItem(const QString id, int page, int pos) { - Q_ASSERT(page >= 0 && page < m_pages.size()); - - if (m_pages[page].count() < pos) - pos = m_pages[page].count(); + if (page < 0 || page > m_pages.size()) return; + if (page == m_pages.size()) appendEmptyPage(); + pos = qBound(0, pos, int(m_pages[page].size())); m_pages[page].insert(pos, id); if (m_pages[page].size() > m_maxItemCountPerPage) { @@ -146,15 +146,16 @@ void ItemsPage::insertItem(const QString id, int page, int pos) void ItemsPage::insertItemToPage(const QString &id, int toPage) { - Q_ASSERT(toPage < m_pages.count()); - - if (toPage < 0) toPage = qMax(m_pages.count() - 1, 0); - - insertItem(id, toPage, m_pages[toPage].count()); + if (toPage < 0) toPage = qMax(int(m_pages.count()) - 1, 0); + insertItem(id, toPage, m_pages.value(toPage).count()); } void ItemsPage::moveItemPosition(int fromPage, int fromIndex, int toPage, int toIndex, bool appendToIndexItem) { + if (fromPage < 0 || fromPage >= m_pages.size() || toPage < 0 || toPage >= m_pages.size() + || fromIndex < 0 || fromIndex >= m_pages[fromPage].size() || toIndex < 0 + || toIndex > m_pages[toPage].size()) return; + if (fromPage == toPage && toIndex == m_pages[toPage].size()) --toIndex; if (fromPage == toPage && fromIndex > toIndex && appendToIndexItem) { if (fromIndex == (toIndex+1)) { // same page, append operate, do nothing diff --git a/tests/applets/CMakeLists.txt b/tests/applets/CMakeLists.txt index 4a39bec06..2f2182333 100644 --- a/tests/applets/CMakeLists.txt +++ b/tests/applets/CMakeLists.txt @@ -87,13 +87,16 @@ applets_add_test(categoryutils_tests categoryutilstests.cpp) # itemspage: pure-logic pagination manager (append/insert/move/remove/find). applets_add_test(itemspage_tests itemspagetests.cpp) -# NOTE: appgroup static helpers (idIsFolder/groupIdFromNumber/parseGroupId) are -# deferred: appgroup.h → appitem.h → am.h → QDBusObjectPath (Qt6::DBus) + -# appgroupmanager.h → DConfig (Dtk6::Core) + QQmlEngine (Qt6::Qml) + yaml-cpp. -# The dependency chain is too heavy for 3 trivial one-liner static methods. -# Defer to a future batch with DConfig mock or after src refactoring. -# Also: parseGroupId has an off-by-one bug (mid(len+1) should be mid(len)) — -# recorded as a src defect, not fixed here. +# Shared launcher/dock folder mutations, using an in-memory source and configuration backend. +add_executable(appgroupmanager_tests appgroupmanagertests.cpp) +target_include_directories(appgroupmanager_tests PRIVATE ${CMAKE_SOURCE_DIR}/applets/dde-apps) +target_link_libraries(appgroupmanager_tests PRIVATE dde-apps Qt${QT_VERSION_MAJOR}::Test + Qt${QT_VERSION_MAJOR}::Qml Qt${QT_VERSION_MAJOR}::Gui Qt${QT_VERSION_MAJOR}::DBus Dtk${DTK_VERSION_MAJOR}::Core) +add_test(NAME applets_appgroupmanager COMMAND appgroupmanager_tests) +# Debian builds disable build RPATH; use the plugin and frame from this build. +set_tests_properties(applets_appgroupmanager PROPERTIES ENVIRONMENT + "LD_LIBRARY_PATH=$:$") +list(APPEND APPLETS_TEST_TARGETS appgroupmanager_tests) # ---- Coverage instrumentation + report generation --------------------------- if(APPLETS_BUILD_COVERAGE) diff --git a/tests/applets/appgroupmanagertests.cpp b/tests/applets/appgroupmanagertests.cpp new file mode 100644 index 000000000..d72a8f1bd --- /dev/null +++ b/tests/applets/appgroupmanagertests.cpp @@ -0,0 +1,361 @@ +// SPDX-FileCopyrightText: 2026 UnionTech Software Technology Co., Ltd. +// SPDX-License-Identifier: GPL-3.0-or-later +#include "appgroupmanager.h" +#include "appitemmodel.h" +#include "itemspage.h" +#include +#include +#include +#include +#include +#include +#include +#include + +class MemoryConfig : public Dtk::Core::DConfigBackend +{ +public: + QVariantMap values{{"ArrangementMigrated", true}, {"Groups", QVariantList()}}; + bool isValid() const override { return true; } + bool load(const QString &) override { return true; } + QStringList keyList() const override { return values.keys(); } + QVariant value(const QString &key, const QVariant &fallback) const override { return values.value(key, fallback); } + void setValue(const QString &key, const QVariant &value) override { values.insert(key, value); } +}; + +class ReadyApplications : public QStandardItemModel +{ + Q_OBJECT + Q_PROPERTY(bool ready READ ready NOTIFY readyChanged) +public: + bool ready() const { return m_ready; } + void setReady(bool ready) { m_ready = ready; emit readyChanged(ready); } +signals: + void readyChanged(bool ready); +private: + bool m_ready = true; +}; + +class AppGroupManagerTest : public QObject +{ + Q_OBJECT + ReadyApplications applications; + std::unique_ptr configuration; + std::unique_ptr folders; + QTemporaryDir configurationHome; + QByteArray originalConfigHome; + + QString createFolder(const QString &first = "a", const QString &second = "b") + { + folders->rearrange(first, second, 0); + return QStringLiteral("internal/folders/%1").arg(folders->itemPosition(first).value("group").toInt()); + } + +private slots: + void initTestCase() + { + QVERIFY(configurationHome.isValid()); + originalConfigHome = qgetenv("XDG_CONFIG_HOME"); + qputenv("XDG_CONFIG_HOME", configurationHome.path().toUtf8()); + } + void cleanupTestCase() + { + if (originalConfigHome.isNull()) qunsetenv("XDG_CONFIG_HOME"); + else qputenv("XDG_CONFIG_HOME", originalConfigHome); + } + void init() + { + applications.setReady(true); + for (const auto &id : {"a", "b", "c", "d"}) { + auto item = new QStandardItem; + item->setData(QString(id), apps::AppItemModel::DesktopIdRole); + item->setData(QString(id).toUpper(), apps::AppItemModel::NameRole); + item->setData(QString(id) + "-icon", apps::AppItemModel::IconNameRole); + applications.appendRow(item); + } + // No DConfig or application-manager service: all mutations are in memory. + configuration.reset(Dtk::Core::DConfig::create(new MemoryConfig, "folder-tests", "folders")); + folders = std::make_unique(&applications, configuration.get(), nullptr); + } + void cleanup() { folders.reset(); configuration.reset(); applications.clear(); } + + void createAndRename() + { + QAbstractItemModelTester tester(folders.get(), QAbstractItemModelTester::FailureReportingMode::QtTest); + const auto id = createFolder(); + QCOMPARE(id, "internal/folders/1"); + const auto contents = folders->folderInfo(id).value("contents").toList(); + QCOMPARE(contents.size(), 2); + QCOMPARE(contents[0].toMap().value("id").toString(), "b"); + QCOMPARE(contents[1].toMap().value("id").toString(), "a"); + QSignalSpy changed(folders.get(), &apps::AppGroupManager::arrangementChanged); + folders->updateFolderName(1, "My apps"); + QCOMPARE(folders->folderInfo(id).value("name").toString(), "My apps"); + QCOMPARE(changed.count(), 1); + } + void moveLastChildOut() + { + const auto id = createFolder(); + folders->rearrange("b", "c", -1); + folders->rearrange("a", "c", -1); + QVERIFY(folders->folderInfo(id).isEmpty()); + QCOMPARE(folders->groupPages(0)->allArrangedItems(), QStringList({"b", "a", "c", "d"})); + } + void moveLastChildOntoOwnFolder_data() + { + QTest::addColumn("operation"); + QTest::addColumn("layout"); + for (int operation : {-1, 1}) { + const QByteArray side = operation < 0 ? "prepend" : "append"; + QTest::newRow((side + "-first-page").constData()) << operation << 0; + QTest::newRow((side + "-only-item-on-last-page").constData()) << operation << 1; + QTest::newRow((side + "-end-of-full-page").constData()) << operation << 2; + } + } + void moveLastChildOntoOwnFolder() + { + QFETCH(int, operation); + QFETCH(int, layout); + const auto id = createFolder(); + folders->rearrange("b", "c", -1); + if (layout == 1) { + const int page = folders->createEmptyPage(0); + folders->rearrange(id, "internal/folders/0", 0, page); + } else if (layout == 2) { + for (int i = 0; i < 28; ++i) { + auto item = new QStandardItem; + item->setData(QStringLiteral("extra-%1").arg(i), apps::AppItemModel::DesktopIdRole); + applications.appendRow(item); + } + QTRY_COMPARE(folders->groupPages(0)->itemCount(), 32); + folders->rearrange(id, "extra-27", 1); + QCOMPARE(folders->findItem(id).pos(), 31); + } + const auto position = folders->findItem(id); + auto expected = folders->groupPages(0)->allArrangedItems(); + expected.replace(expected.indexOf(id), "a"); + + // Hovering at the source folder's edge can remove it before release. + folders->rearrange("a.desktop", id, operation); + QVERIFY(folders->folderInfo(id).isEmpty()); + QCOMPARE(folders->findItem("a").group(), 0); + QCOMPARE(folders->findItem("a").page(), position.page()); + QCOMPARE(folders->findItem("a").pos(), position.pos()); + QCOMPARE(folders->groupPages(0)->allArrangedItems(), expected); + + // A release still referring to the deleted folder must not lose the app. + folders->rearrange("a.desktop", id, 0); + QCOMPARE(folders->groupPages(0)->allArrangedItems(), expected); + folders.reset(); + folders = std::make_unique(&applications, configuration.get(), nullptr); + QCOMPARE(folders->groupPages(0)->allArrangedItems(), expected); + QCOMPARE(folders->findItem("a").group(), 0); + } + void joinIntoOtherFolderAndDeleteSource() + { + const auto first = createFolder(); + const auto second = createFolder("c", "d"); + folders->rearrange("a", second, 0); + folders->rearrange("b", second, 0); + QVERIFY(folders->folderInfo(first).isEmpty()); + QCOMPARE(folders->folderInfo(second).value("contents").toList().size(), 4); + // Group identity is not its shifting row number. + QCOMPARE(folders->groupIndexById(2).data(apps::AppGroupManager::GroupIdRole).toInt(), 2); + folders->updateFolderName(2, "Remaining folder"); + QCOMPARE(folders->folderInfo(second).value("name").toString(), "Remaining folder"); + } + void rejectNestedAndStaleDrops() + { + const auto first = createFolder(); + const auto second = createFolder("c", "d"); + const auto before = folders->groupPages(0)->allArrangedItems(); + folders->rearrange(first, second, 0); + folders->rearrange(first, "c", -1); + folders->rearrange("missing", first, 0); + folders->rearrange("a", "missing", -1); + QCOMPARE(folders->groupPages(0)->allArrangedItems(), before); + QCOMPARE(folders->folderInfo(first).value("contents").toList().size(), 2); + } + void uninstallRemovesEmptyFolder() + { + const auto id = createFolder(); + applications.removeRows(0, 2); + QTRY_VERIFY(folders->folderInfo(id).isEmpty()); + QVERIFY(!folders->groupPages(0)->contains(id)); + applications.removeRows(0, applications.rowCount()); + QTRY_COMPARE(folders->rowCount(), 1); // Top level survives an empty app set. + } + void unreadySourceDoesNotPruneFolders() + { + const auto id = createFolder(); + applications.setReady(false); + applications.removeRows(0, applications.rowCount()); + QCoreApplication::processEvents(); + QCOMPARE(folders->folderInfo(id).value("id").toString(), id); + QCOMPARE(folders->groupPages(1)->itemCount(), 2); + // Only the ready notification makes an empty source authoritative. + applications.setReady(true); + QTRY_VERIFY(folders->folderInfo(id).isEmpty()); + QCOMPARE(folders->rowCount(), 1); + } + void missingSourceKeepsFolderMetadata() + { + auto source = std::make_unique(); + for (const auto &id : {"a", "b"}) { + auto item = new QStandardItem; + item->setData(QString(id), apps::AppItemModel::DesktopIdRole); + source->appendRow(item); + } + apps::AppGroupManager manager(source.get(), configuration.get(), nullptr); + manager.rearrange("a", "b", 0); + manager.updateFolderName(1, "Still pinned"); + source.reset(); + QCoreApplication::processEvents(); + const auto info = manager.folderInfo("internal/folders/1"); + QCOMPARE(info.value("id").toString(), "internal/folders/1"); + QCOMPARE(info.value("name").toString(), "Still pinned"); + QVERIFY(info.value("contents").toList().isEmpty()); + QCOMPARE(manager.groupPages(1)->itemCount(), 2); + } + void bringToFrontAcceptsDesktopSuffix() + { + folders->bringToFront("c.desktop"); + QCOMPARE(folders->groupPages(0)->allArrangedItems(), QStringList({"c", "a", "b", "d"})); + folders->bringToFront("missing.desktop"); + QCOMPARE(folders->groupPages(0)->allArrangedItems(), QStringList({"c", "a", "b", "d"})); + const auto id = createFolder(); + folders->bringToFront(id); + QCOMPARE(folders->groupPages(0)->allArrangedItems().first(), id); + } + void reorderBeforeAndAfter() + { + folders->rearrange("a", "c", -1); + QCOMPARE(folders->groupPages(0)->allArrangedItems(), QStringList({"b", "a", "c", "d"})); + folders->rearrange("a", "c", 1); + QCOMPARE(folders->groupPages(0)->allArrangedItems(), QStringList({"b", "c", "a", "d"})); + folders->rearrange("d", "b", 1); + QCOMPARE(folders->groupPages(0)->allArrangedItems(), QStringList({"b", "d", "c", "a"})); + } + void persistenceAndLegacyIds() + { + auto backend = new MemoryConfig; + std::unique_ptr config(Dtk::Core::DConfig::create(backend, "folder-tests", "folders")); + backend->values["Groups"] = QVariantList{ + QVariantMap{{"groupId", "internal/group/12"}, {"name", "Imported"}, + {"appItems", apps::AppGroupManager::fromListOfStringList({{"b.desktop", "a.desktop", "a.desktop"}})}}, + QVariantMap{{"groupId", 0}, {"name", "Top"}, + {"appItems", apps::AppGroupManager::fromListOfStringList({{"a", "internal/folder/12", "c", "internal/folders/99", "d"}})}} + }; + folders = std::make_unique(&applications, config.get(), nullptr); + QCOMPARE(folders->groupPages(0)->allArrangedItems(), QStringList({"internal/folders/12", "c", "d"})); + QCOMPARE(folders->groupPages(12)->allArrangedItems(), QStringList({"b", "a"})); + folders->updateFolderName(12, "Persistent folder"); + folders->rearrange("a", "b", -1); + folders.reset(); + folders = std::make_unique(&applications, config.get(), nullptr); + QCOMPARE(folders->folderInfo("internal/folders/12").value("name").toString(), "Persistent folder"); + QCOMPARE(folders->groupPages(12)->allArrangedItems(), QStringList({"a", "b"})); + // Destroy manager before its externally owned config. + folders.reset(); + } + void importIniOnlyOnce_data() + { + QTest::addColumn("legacyMigrated"); + QTest::newRow("first-upgrade") << false; + QTest::newRow("legacy-migration-already-marked") << true; + } + void importIniOnlyOnce() + { + QFETCH(bool, legacyMigrated); + QTemporaryDir temporary; + QVERIFY(temporary.isValid()); + const auto originalConfigHome = qgetenv("XDG_CONFIG_HOME"); + qputenv("XDG_CONFIG_HOME", temporary.path().toUtf8()); + const auto path = temporary.filePath("deepin/dde-launchpad/item-arrangement.ini"); + QDir().mkpath(QFileInfo(path).absolutePath()); + QSettings ini(path, QSettings::NativeFormat); + ini.setValue("fullscreen/toplevel/pageCount", 2); + ini.setValue("fullscreen/toplevel/pageItems/0", QStringList({"c.desktop", "internal/folders/5"})); + ini.setValue("fullscreen/toplevel/pageItems/1", QStringList({"d.desktop", "internal/folders/1"})); + ini.setValue("fullscreen/1/name", "internal/category/9"); + ini.setValue("fullscreen/1/pageCount", 1); + ini.setValue("fullscreen/1/pageItems/0", QStringList({"b.desktop"})); + ini.setValue("fullscreen/5/name", "Legacy folder"); + ini.setValue("fullscreen/5/pageCount", 1); + ini.setValue("fullscreen/5/pageItems/0", QStringList({"a.desktop"})); + ini.sync(); + QFile originalIni(path); + QVERIFY(originalIni.open(QIODevice::ReadOnly)); + const auto iniContents = originalIni.readAll(); + originalIni.close(); + auto backend = new MemoryConfig; + backend->values["ArrangementMigrated"] = legacyMigrated; + // Upgrade from an old applet that has already populated the shared key + // incorrectly. Its migration flag cannot validate the launcher layout. + backend->values["Groups"] = QVariantList{ + QVariantMap{{"groupId", 1}, {"name", "internal/category/9"}, + {"appItems", apps::AppGroupManager::fromListOfStringList({{"a", "b", "c", "d"}})}}}; + std::unique_ptr config(Dtk::Core::DConfig::create(backend, "folder-tests", "folders")); + folders = std::make_unique(&applications, config.get(), nullptr); + // Restore even if any of the checks below fail. + if (originalConfigHome.isNull()) qunsetenv("XDG_CONFIG_HOME"); + else qputenv("XDG_CONFIG_HOME", originalConfigHome); + QCOMPARE(folders->folderInfo("internal/folders/5").value("name").toString(), "Legacy folder"); + // The first INI child is the System folder, not the fullscreen root. + // Keep root apps, folder entrances, page boundaries and order intact. + const QList topPages{{"c", "internal/folders/5"}, {"d", "internal/folders/1"}}; + QCOMPARE(folders->rowCount(), 3); + QCOMPARE(folders->groupPages(0)->allPagedItems(), topPages); + QCOMPARE(folders->groupPages(0)->maxItemCountPerPage(), 32); + QCOMPARE(folders->groupPages(1)->allArrangedItems(), QStringList({"b"})); + QCOMPARE(folders->groupPages(5)->allArrangedItems(), QStringList({"a"})); + QCOMPARE(folders->findItem("c").group(), 0); + QCOMPARE(folders->findItem("d").group(), 0); + QVERIFY(!folders->groupPages(0)->contains("internal/folders/0")); + QVERIFY(QFileInfo::exists(path)); + QVERIFY(originalIni.open(QIODevice::ReadOnly)); + QCOMPARE(originalIni.readAll(), iniContents); + originalIni.close(); + bool versionedRoot = false; + for (const auto &value : config->value("Groups").toList()) { + const auto record = value.toMap(); + if (record.value("groupId").toString() == "internal/folders/0") + versionedRoot = record.value("arrangementVersion").toInt() == 1; + } + QVERIFY(versionedRoot); + folders->updateFolderName(5, "Renamed after migration"); + folders->rearrange("d", "c", -1); + const auto updatedPages = folders->groupPages(0)->allPagedItems(); + folders.reset(); + qputenv("XDG_CONFIG_HOME", temporary.path().toUtf8()); + folders = std::make_unique(&applications, config.get(), nullptr); + if (originalConfigHome.isNull()) qunsetenv("XDG_CONFIG_HOME"); + else qputenv("XDG_CONFIG_HOME", originalConfigHome); + QCOMPARE(folders->folderInfo("internal/folders/5").value("name").toString(), "Renamed after migration"); + QCOMPARE(folders->groupPages(0)->allPagedItems(), updatedPages); + QCOMPARE(folders->groupPages(1)->allArrangedItems(), QStringList({"b"})); + folders.reset(); + } + void folderCanMoveToNewTopLevelPage() + { + const auto id = createFolder(); + QCOMPARE(folders->createEmptyPage(0), 1); + folders->rearrange(id, "internal/folders/0", 0, 1); + QCOMPARE(folders->findItem(id).group(), 0); + QCOMPARE(folders->findItem(id).page(), 1); + QCOMPARE(folders->folderInfo(id).value("contents").toList().size(), 2); + } + void emptyPageAndDesktopSuffix() + { + const auto id = createFolder("a.desktop", "b.desktop"); + QCOMPARE(folders->createEmptyPage(1), 1); + folders->rearrange("a.desktop", id, 0, 1); + QCOMPARE(folders->findItem("a").page(), 1); + folders->rearrange("a", "internal/folders/0", 0); + QCOMPARE(folders->findItem("a").group(), 0); + folders->removeEmptyPages(); + QCOMPARE(folders->pageCount(1), 1); + } +}; +QTEST_GUILESS_MAIN(AppGroupManagerTest) +#include "appgroupmanagertests.moc" diff --git a/tests/applets/itemspagetests.cpp b/tests/applets/itemspagetests.cpp index 9c35269c3..1e1dd95e2 100644 --- a/tests/applets/itemspagetests.cpp +++ b/tests/applets/itemspagetests.cpp @@ -385,8 +385,8 @@ TEST(ItemsPage, FirstNItems) EXPECT_EQ(page.firstNItems(2), (QStringList{QStringLiteral("a"), QStringLiteral("b")})); EXPECT_EQ(page.firstNItems(3), (QStringList{QStringLiteral("a"), QStringLiteral("b"), QStringLiteral("c")})); EXPECT_EQ(page.firstNItems(10), (QStringList{QStringLiteral("a"), QStringLiteral("b"), QStringLiteral("c"), QStringLiteral("d")})); - // src defect: firstNItems(0) appends first item then checks count >= 0, returns [a] - EXPECT_EQ(page.firstNItems(0), (QStringList{QStringLiteral("a")})); + EXPECT_TRUE(page.firstNItems(0).isEmpty()); + EXPECT_TRUE(page.firstNItems(-1).isEmpty()); } // --- Additional edge-case coverage (per review) --- @@ -423,3 +423,20 @@ TEST(ItemsPage, MoveItemSamePageForwardNonAdjacentAppend) // "c" moved from index 2 to index 1 → [a, c, b] EXPECT_EQ(page.items(0), (QStringList{QStringLiteral("a"), QStringLiteral("c"), QStringLiteral("b")})); } + +TEST(ItemsPage, InsertIntoInitiallyEmptyDestination) +{ + ItemsPage page(4); + page.insertItemToPage("a", -1); + EXPECT_EQ(page.allArrangedItems(), QStringList({"a"})); +} + +TEST(ItemsPage, StaleDragCoordinatesAreIgnored) +{ + ItemsPage page(4); + page.appendPage({"a", "b"}); + page.moveItemPosition(-1, 0, 0, 1, false); + page.moveItemPosition(0, 5, 0, 1, false); + page.insertItemToPage("c", 99); + EXPECT_EQ(page.allArrangedItems(), QStringList({"a", "b"})); +}