From 024f66dd25b04d55ec073045d44e2c747002a12a Mon Sep 17 00:00:00 2001 From: Brais Couce <9771896+braiscouce@users.noreply.github.com> Date: Sat, 29 Aug 2026 19:50:22 +0200 Subject: [PATCH 1/6] feat: Add support to rename tags for all entries (#9142) --- share/translations/keepassxc_en.ts | 28 +++++++++ src/core/Database.cpp | 62 +++++++++++++++++++ src/core/Database.h | 2 + src/gui/tag/TagView.cpp | 37 ++++++++--- tests/CMakeLists.txt | 3 + tests/TestTags.cpp | 98 ++++++++++++++++++++++++++++++ tests/TestTags.h | 35 +++++++++++ 7 files changed, 257 insertions(+), 8 deletions(-) create mode 100644 tests/TestTags.cpp create mode 100644 tests/TestTags.h diff --git a/share/translations/keepassxc_en.ts b/share/translations/keepassxc_en.ts index 4ca387959d..8ac9847816 100644 --- a/share/translations/keepassxc_en.ts +++ b/share/translations/keepassxc_en.ts @@ -1659,6 +1659,22 @@ Backup database located at %2 Key not transformed. This is a bug, please report it to the developers. + + The tag name cannot be empty. + + + + Both tag names are the same. + + + + The tag "%1" already exists. + + + + The tag "%1" was not found. + + Recycle Bin @@ -10473,10 +10489,22 @@ This option is deprecated, use --set-key-file instead. Remove Search + + Rename Tag + + Remove Tag + + New tag name for "%1": + + + + Error + + Confirm Remove Tag diff --git a/src/core/Database.cpp b/src/core/Database.cpp index 87a03d30f2..59ce17054b 100644 --- a/src/core/Database.cpp +++ b/src/core/Database.cpp @@ -878,6 +878,68 @@ void Database::removeTag(const QString& tag) } } +bool Database::hasTag(const QString& tag) +{ + // First update the tag list because the new test renameExistingTag fails. The existing tag is not detected + // because the list is outdated although from manual tests it works fine + updateTagList(); + const auto tags = tagList(); + for (const auto& t : tags) { + if (t.compare(tag, Qt::CaseInsensitive) == 0) { + return true; + } + } + return false; +} + +bool Database::renameTag(const QString& oldTag, const QString& newTag, QString* error) +{ + const QString cleanOldTag = oldTag.trimmed(); + const QString cleanNewTag = newTag.trimmed(); + + if (cleanOldTag.isEmpty() || cleanNewTag.isEmpty()) { + if (error) { + *error = tr("The tag name cannot be empty."); + } + return false; + } + + if (cleanOldTag.compare(cleanNewTag, Qt::CaseInsensitive) == 0) { + if (error) { + *error = tr("Both tag names are the same."); + } + return false; + } + + if (hasTag(cleanNewTag)) { + if (error) { + *error = tr("The tag \"%1\" already exists.").arg(cleanNewTag); + } + return false; + } + + bool modified = false; + for (auto entry : m_rootGroup->entriesRecursive()) { + auto tags = entry->tagList(); + for (int i = 0; i < tags.size(); ++i) { + if (tags[i].compare(cleanOldTag, Qt::CaseInsensitive) == 0) { + tags[i] = cleanNewTag; + entry->setTags(tags.join(",")); + modified = true; + break; + } + } + } + + if (!modified) { + if (error) { + *error = tr("The tag \"%1\" was not found.").arg(cleanOldTag); + } + } + + return modified; +} + const QUuid& Database::cipher() const { return m_data.cipher; diff --git a/src/core/Database.h b/src/core/Database.h index ebecbbe6b3..8e65649cc1 100644 --- a/src/core/Database.h +++ b/src/core/Database.h @@ -146,6 +146,8 @@ class Database : public ModifiableObject const QStringList& customAttributeKeys() const; const QStringList& tagList() const; void removeTag(const QString& tag); + bool hasTag(const QString& tag); + bool renameTag(const QString& oldTag, const QString& newTag, QString* error); QSharedPointer key() const; bool setKey(const QSharedPointer& key, diff --git a/src/gui/tag/TagView.cpp b/src/gui/tag/TagView.cpp index 0330612920..61e7a9c504 100644 --- a/src/gui/tag/TagView.cpp +++ b/src/gui/tag/TagView.cpp @@ -24,6 +24,7 @@ #include "gui/Icons.h" #include "gui/MessageBox.h" +#include #include #include #include @@ -86,17 +87,37 @@ void TagView::contextMenuRequested(const QPoint& pos) m_db->metadata()->deleteSavedSearch(index.data(Qt::DisplayRole).toString()); } } else if (type == TagModel::TAG) { - // Allow removing tags from all entries in a database + // Allow removing and renaming tags from all entries in a database QMenu menu; - auto action = menu.exec({new QAction(icons()->icon("trash"), tr("Remove Tag"), nullptr)}, mapToGlobal(pos)); + auto renameAction = menu.addAction(icons()->icon("entry-edit"), tr("Rename Tag")); + auto removeAction = menu.addAction(icons()->icon("trash"), tr("Remove Tag")); + + auto action = menu.exec(mapToGlobal(pos)); if (action) { auto tag = index.data(Qt::DisplayRole).toString(); - auto ans = MessageBox::question(this, - tr("Confirm Remove Tag"), - tr("Remove tag \"%1\" from all entries in this database?").arg(tag), - MessageBox::Remove | MessageBox::Cancel); - if (ans == MessageBox::Remove) { - m_db->removeTag(tag); + if (action == renameAction) { + bool ok = false; + QString newTag = QInputDialog::getText(this, + tr("Rename Tag"), + tr("New tag name for \"%1\":").arg(tag), + QLineEdit::Normal, + tag, + &ok).trimmed(); + + if (ok && !newTag.isEmpty() && newTag != tag) { + QString error; + if (!m_db->renameTag(tag, newTag, &error)) { + MessageBox::warning(this, tr("Error"), error); + } + } + } else if (action == removeAction) { + auto ans = MessageBox::question(this, + tr("Confirm Remove Tag"), + tr("Remove tag \"%1\" from all entries in this database?").arg(tag), + MessageBox::Remove | MessageBox::Cancel); + if (ans == MessageBox::Remove) { + m_db->removeTag(tag); + } } } } diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 44d4cabeea..6e926c37ac 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -154,6 +154,9 @@ add_unit_test(NAME testsharing SOURCES TestSharing.cpp add_unit_test(NAME testdatabase SOURCES TestDatabase.cpp LIBS testsupport ${TEST_LIBRARIES}) +add_unit_test(NAME testtags SOURCES TestTags.cpp + LIBS ${TEST_LIBRARIES}) + add_unit_test(NAME testtools SOURCES TestTools.cpp LIBS testsupport ${TEST_LIBRARIES}) diff --git a/tests/TestTags.cpp b/tests/TestTags.cpp new file mode 100644 index 0000000000..c4852c95ad --- /dev/null +++ b/tests/TestTags.cpp @@ -0,0 +1,98 @@ +/* + * Copyright (C) 2026 Brais Couce + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 2 or (at your option) + * version 3 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#include "TestTags.h" + +#include "QTest" + +#include "core/Database.h" +#include "core/Entry.h" +#include "core/Group.h" +#include "crypto/Crypto.h" + +QTEST_GUILESS_MAIN(TestTags) + +void TestTags::initTestCase() +{ + QVERIFY(Crypto::init()); + QLocale::setDefault(QLocale::c()); +} + +void TestTags::testRenameTag() +{ + QScopedPointer db(new Database()); + QVERIFY(db); + + auto* entry1 = new Entry(); + db->rootGroup()->addEntry(entry1); + entry1->setTags("tag1, tag 2"); + + auto* entry2 = new Entry(); + db->rootGroup()->addEntry(entry2); + entry2->setTags("TaG 2, tag3"); + + QString error; + QVERIFY(db->renameTag("tag 2", "tag2_ren", &error)); + QVERIFY(error.isEmpty()); + + QCOMPARE(entry1->tagList(), QStringList({"tag1", "tag2_ren"})); + QCOMPARE(entry2->tagList(), QStringList({"tag2_ren", "tag3"})); +} + +void TestTags::renameEmptyTag() +{ + QScopedPointer db(new Database()); + QVERIFY(db); + + QString error; + + QVERIFY(!db->renameTag("tag1", " ", &error)); + QCOMPARE(error, QObject::tr("The tag name cannot be empty.")); + error.clear(); + + QVERIFY(!db->renameTag(" ", "tag1_ren", &error)); + QCOMPARE(error, QObject::tr("The tag name cannot be empty.")); + error.clear(); +} + +void TestTags::renameExistingTag() +{ + QScopedPointer db(new Database()); + QVERIFY(db); + + auto* entry = new Entry(); + db->rootGroup()->addEntry(entry); + entry->setTags("tag1, tag2"); + + QString error; + QVERIFY(!db->renameTag("tag1", "tag2", &error)); + QCOMPARE(error, QObject::tr("The tag \"%1\" already exists.").arg("tag2")); +} + +void TestTags::testRenameNotExistingTag() +{ + QScopedPointer db(new Database()); + QVERIFY(db); + + auto* entry = new Entry(); + db->rootGroup()->addEntry(entry); + entry->setTags("tag1, tag2"); + + QString error; + QVERIFY(!db->renameTag("tag3", "tag3_ren", &error)); + QCOMPARE(error, QObject::tr("The tag \"%1\" was not found.").arg("tag3")); +} diff --git a/tests/TestTags.h b/tests/TestTags.h new file mode 100644 index 0000000000..b65f437755 --- /dev/null +++ b/tests/TestTags.h @@ -0,0 +1,35 @@ +/* + * Copyright (C) 2026 Brais Couce + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 2 or (at your option) + * version 3 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#ifndef KEEPASSXC_TESTTAGS_H +#define KEEPASSXC_TESTTAGS_H + +#include + +class TestTags : public QObject +{ + Q_OBJECT + +private slots: + void initTestCase(); + void testRenameTag(); + void renameEmptyTag(); + void renameExistingTag(); + void testRenameNotExistingTag(); +}; + +#endif // KEEPASSXC_TESTTAGS_H From d1b3f38e29c958c4077cfb13b3c18c3a6ee89d0e Mon Sep 17 00:00:00 2001 From: Brais Couce <9771896+braiscouce@users.noreply.github.com> Date: Sat, 29 Aug 2026 22:14:35 +0200 Subject: [PATCH 2/6] Import QTest in TestTags via angle brackets Detected by Copilot in the code review --- tests/TestTags.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/TestTags.cpp b/tests/TestTags.cpp index c4852c95ad..817f9d2962 100644 --- a/tests/TestTags.cpp +++ b/tests/TestTags.cpp @@ -17,7 +17,7 @@ #include "TestTags.h" -#include "QTest" +#include #include "core/Database.h" #include "core/Entry.h" From 17ac00515bc0a8525f0aceed47f87bea8f4c23a8 Mon Sep 17 00:00:00 2001 From: Brais Couce <9771896+braiscouce@users.noreply.github.com> Date: Sat, 29 Aug 2026 22:21:51 +0200 Subject: [PATCH 3/6] Show an error when you confirm the renaming to an empty tag Detected by Copilot in the code review --- src/gui/tag/TagView.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gui/tag/TagView.cpp b/src/gui/tag/TagView.cpp index 61e7a9c504..a755a8d116 100644 --- a/src/gui/tag/TagView.cpp +++ b/src/gui/tag/TagView.cpp @@ -104,7 +104,7 @@ void TagView::contextMenuRequested(const QPoint& pos) tag, &ok).trimmed(); - if (ok && !newTag.isEmpty() && newTag != tag) { + if (ok && newTag != tag) { QString error; if (!m_db->renameTag(tag, newTag, &error)) { MessageBox::warning(this, tr("Error"), error); From 396394dffc7c926789ebcc20f9a996e631673943 Mon Sep 17 00:00:00 2001 From: Brais Couce <9771896+braiscouce@users.noreply.github.com> Date: Sat, 29 Aug 2026 23:00:27 +0200 Subject: [PATCH 4/6] Change implementation of Database::hasTag to improve performance Detected by Copilot in the code review: Database::hasTag() currently forces updateTagList() (full database scan + sort + tagListUpdated() emission) on every call, and the comment references a unit-test failure workaround. This makes a cheap query unexpectedly expensive and can trigger extra UI model resets. Prefer checking tags directly on entries without touching the cached tag list. --- src/core/Database.cpp | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/core/Database.cpp b/src/core/Database.cpp index 59ce17054b..26a4be332a 100644 --- a/src/core/Database.cpp +++ b/src/core/Database.cpp @@ -880,13 +880,15 @@ void Database::removeTag(const QString& tag) bool Database::hasTag(const QString& tag) { - // First update the tag list because the new test renameExistingTag fails. The existing tag is not detected - // because the list is outdated although from manual tests it works fine - updateTagList(); - const auto tags = tagList(); - for (const auto& t : tags) { - if (t.compare(tag, Qt::CaseInsensitive) == 0) { - return true; + if (!m_rootGroup) { + return false; + } + + for (auto entry : m_rootGroup->entriesRecursive()) { + for (const auto& t : entry->tagList()) { + if (t.compare(tag, Qt::CaseInsensitive) == 0) { + return true; + } } } return false; From 2131ff1d9a3c54583e2cf469360e9dc2e6ce05c1 Mon Sep 17 00:00:00 2001 From: Brais Couce <9771896+braiscouce@users.noreply.github.com> Date: Sun, 30 Aug 2026 00:13:27 +0200 Subject: [PATCH 5/6] Changes to avoid issues when the tag name contains the delimiters (,;\t) --- src/core/Database.cpp | 22 ++++++---------------- src/core/Entry.cpp | 38 ++++++++++++++++++++++++++++++++++++++ src/core/Entry.h | 2 ++ tests/TestTags.cpp | 16 ++++++++++++++++ tests/TestTags.h | 1 + 5 files changed, 63 insertions(+), 16 deletions(-) diff --git a/src/core/Database.cpp b/src/core/Database.cpp index 26a4be332a..5aeb260b01 100644 --- a/src/core/Database.cpp +++ b/src/core/Database.cpp @@ -885,10 +885,8 @@ bool Database::hasTag(const QString& tag) } for (auto entry : m_rootGroup->entriesRecursive()) { - for (const auto& t : entry->tagList()) { - if (t.compare(tag, Qt::CaseInsensitive) == 0) { - return true; - } + if (entry->hasTag(tag)) { + return true; } } return false; @@ -920,26 +918,18 @@ bool Database::renameTag(const QString& oldTag, const QString& newTag, QString* return false; } - bool modified = false; + bool renamed = false; for (auto entry : m_rootGroup->entriesRecursive()) { - auto tags = entry->tagList(); - for (int i = 0; i < tags.size(); ++i) { - if (tags[i].compare(cleanOldTag, Qt::CaseInsensitive) == 0) { - tags[i] = cleanNewTag; - entry->setTags(tags.join(",")); - modified = true; - break; - } - } + renamed |= entry->renameTag(oldTag, newTag); } - if (!modified) { + if (!renamed) { if (error) { *error = tr("The tag \"%1\" was not found.").arg(cleanOldTag); } } - return modified; + return renamed; } const QUuid& Database::cipher() const diff --git a/src/core/Entry.cpp b/src/core/Entry.cpp index 121e4d02bc..1e7e4f3dbb 100644 --- a/src/core/Entry.cpp +++ b/src/core/Entry.cpp @@ -752,6 +752,44 @@ void Entry::removeTag(const QString& tag) } } +bool Entry::hasTag(const QString& tag) +{ + auto cleanTag = tag.trimmed(); + cleanTag.remove(TagDelimiterRegex); + + auto tagList = m_data.tags; + for (const auto& t : tagList) { + if (t.compare(cleanTag, Qt::CaseInsensitive) == 0) { + return true; + } + } + return false; +} + +bool Entry::renameTag(const QString& oldTag, const QString& newTag) +{ + auto cleanOldTag = oldTag.trimmed(); + cleanOldTag.remove(TagDelimiterRegex); + + auto cleanNewTag = newTag.trimmed(); + cleanNewTag.remove(TagDelimiterRegex); + + auto tagList = m_data.tags; + bool renamed = false; + for (int i = 0; i < tagList.size(); i++) { + if (tagList[i].compare(cleanOldTag, Qt::CaseInsensitive) == 0) { + tagList[i] = cleanNewTag; + renamed = true; + break; + } + } + if (renamed) { + tagList.sort(); + set(m_data.tags, tagList); + } + return renamed; +} + void Entry::setTimeInfo(const TimeInfo& timeInfo) { m_data.timeInfo = timeInfo; diff --git a/src/core/Entry.h b/src/core/Entry.h index be140e91f0..061a171b4a 100644 --- a/src/core/Entry.h +++ b/src/core/Entry.h @@ -169,6 +169,8 @@ class Entry : public ModifiableObject void addTag(const QString& tag); void removeTag(const QString& tag); + bool hasTag(const QString& tag); + bool renameTag(const QString& oldTag, const QString& newTag); QList historyItems(); const QList& historyItems() const; diff --git a/tests/TestTags.cpp b/tests/TestTags.cpp index 817f9d2962..dd5660075d 100644 --- a/tests/TestTags.cpp +++ b/tests/TestTags.cpp @@ -96,3 +96,19 @@ void TestTags::testRenameNotExistingTag() QVERIFY(!db->renameTag("tag3", "tag3_ren", &error)); QCOMPARE(error, QObject::tr("The tag \"%1\" was not found.").arg("tag3")); } + +void TestTags::testRenameTagWithDelimiter() +{ + QScopedPointer db(new Database()); + QVERIFY(db); + + auto* entry = new Entry(); + db->rootGroup()->addEntry(entry); + entry->setTags("tag1"); + + QString error; + QVERIFY(db->renameTag("tag,;1", "tag,;1_ren", &error)); + QVERIFY(error.isEmpty()); + + QCOMPARE(entry->tagList(), QStringList({"tag1_ren"})); +} diff --git a/tests/TestTags.h b/tests/TestTags.h index b65f437755..dad357240b 100644 --- a/tests/TestTags.h +++ b/tests/TestTags.h @@ -30,6 +30,7 @@ private slots: void renameEmptyTag(); void renameExistingTag(); void testRenameNotExistingTag(); + void testRenameTagWithDelimiter(); }; #endif // KEEPASSXC_TESTTAGS_H From 05fdf0378b35416931936a4ddda5d1a320244392 Mon Sep 17 00:00:00 2001 From: Brais Couce <9771896+braiscouce@users.noreply.github.com> Date: Fri, 4 Sep 2026 21:11:57 +0200 Subject: [PATCH 6/6] fix: renaming a tag should create a new history entry --- src/core/Entry.cpp | 2 ++ tests/TestTags.cpp | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/src/core/Entry.cpp b/src/core/Entry.cpp index 1e7e4f3dbb..624e2af769 100644 --- a/src/core/Entry.cpp +++ b/src/core/Entry.cpp @@ -768,6 +768,7 @@ bool Entry::hasTag(const QString& tag) bool Entry::renameTag(const QString& oldTag, const QString& newTag) { + beginUpdate(); auto cleanOldTag = oldTag.trimmed(); cleanOldTag.remove(TagDelimiterRegex); @@ -787,6 +788,7 @@ bool Entry::renameTag(const QString& oldTag, const QString& newTag) tagList.sort(); set(m_data.tags, tagList); } + endUpdate(); return renamed; } diff --git a/tests/TestTags.cpp b/tests/TestTags.cpp index dd5660075d..6cb7d4b1e9 100644 --- a/tests/TestTags.cpp +++ b/tests/TestTags.cpp @@ -40,17 +40,21 @@ void TestTags::testRenameTag() auto* entry1 = new Entry(); db->rootGroup()->addEntry(entry1); entry1->setTags("tag1, tag 2"); + QCOMPARE(entry1->historyItems().size(), 0); auto* entry2 = new Entry(); db->rootGroup()->addEntry(entry2); entry2->setTags("TaG 2, tag3"); + QCOMPARE(entry2->historyItems().size(), 0); QString error; QVERIFY(db->renameTag("tag 2", "tag2_ren", &error)); QVERIFY(error.isEmpty()); QCOMPARE(entry1->tagList(), QStringList({"tag1", "tag2_ren"})); + QCOMPARE(entry1->historyItems().size(), 1); QCOMPARE(entry2->tagList(), QStringList({"tag2_ren", "tag3"})); + QCOMPARE(entry2->historyItems().size(), 1); } void TestTags::renameEmptyTag()