Improve drag & drop behavior

- Preserve the LastModificationTime for a group/entry cloned with the CloneResetTimeInfo flag
- Retain UUID's and timeInfo when moving group(s)/entry(s) between db's
- Prevent the Recycle Bin group from being moved to another db
- Cross-db moves that might require merge logic are not (yet) supported and show an error message instead

Fixes #6202
Fixes #8170

src/core/Entry.cpp

@@ -947,9 +947,9 @@ Entry* Entry::clone(CloneFlags flags) const
     if (flags & CloneResetTimeInfo) {
         QDateTime now = Clock::currentDateTimeUtc();
         entry->m_data.timeInfo.setCreationTime(now);
-        entry->m_data.timeInfo.setLastModificationTime(now);
         entry->m_data.timeInfo.setLastAccessTime(now);
         entry->m_data.timeInfo.setLocationChanged(now);
+        // preserve LastModificationTime
     }
 
     if (flags & CloneRenameTitle) {
@@ -1267,11 +1267,7 @@ void Entry::setGroup(Group* group, bool trackPrevious)
             m_group->database()->addDeletedObject(m_uuid);
 
             // copy custom icon to the new database
-            if (!iconUuid().isNull() && group->database() && m_group->database()->metadata()->hasCustomIcon(iconUuid())
-                && !group->database()->metadata()->hasCustomIcon(iconUuid())) {
-                group->database()->metadata()->addCustomIcon(iconUuid(),
-                                                             m_group->database()->metadata()->customIcon(iconUuid()));
-            }
+            group->database()->metadata()->copyCustomIcon(iconUuid(), m_group->database()->metadata());
         } else if (trackPrevious && m_group->database() && group != m_group) {
             setPreviousParentGroup(m_group);
         }

src/core/Entry.h

@@ -175,13 +175,15 @@ public:
     {
         CloneNoFlags = 0,
         CloneNewUuid = 1, // generate a random uuid for the clone
-        CloneResetTimeInfo = 2, // set all TimeInfo attributes to the current time
+        CloneResetTimeInfo = 2, // set all TimeInfo attributes except LastModificationTime to the current time
         CloneIncludeHistory = 4, // clone the history items
-        CloneDefault = CloneNewUuid | CloneResetTimeInfo,
-        CloneCopy = CloneNewUuid | CloneResetTimeInfo | CloneIncludeHistory,
         CloneRenameTitle = 8, // add "-Clone" after the original title
         CloneUserAsRef = 16, // Add the user as a reference to the original entry
         ClonePassAsRef = 32, // Add the password as a reference to the original entry
+
+        CloneCopy = CloneNewUuid | CloneResetTimeInfo | CloneIncludeHistory,
+        CloneExactCopy = CloneIncludeHistory,
+        CloneDefault = CloneNewUuid | CloneResetTimeInfo,
     };
     Q_DECLARE_FLAGS(CloneFlags, CloneFlag)
 

src/core/Group.cpp

@@ -469,10 +469,7 @@ void Group::setParent(Group* parent, int index, bool trackPrevious)
             recCreateDelObjects();
 
             // copy custom icon to the new database
-            if (!iconUuid().isNull() && parent->m_db && m_db->metadata()->hasCustomIcon(iconUuid())
-                && !parent->m_db->metadata()->hasCustomIcon(iconUuid())) {
-                parent->m_db->metadata()->addCustomIcon(iconUuid(), m_db->metadata()->customIcon(iconUuid()));
-            }
+            parent->m_db->metadata()->copyCustomIcon(iconUuid(), m_db->metadata());
         }
         if (m_db != parent->m_db) {
             connectDatabaseSignalsRecursive(parent->m_db);
@@ -949,9 +946,9 @@ Group* Group::clone(Entry::CloneFlags entryFlags, Group::CloneFlags groupFlags)
 
         QDateTime now = Clock::currentDateTimeUtc();
         clonedGroup->m_data.timeInfo.setCreationTime(now);
-        clonedGroup->m_data.timeInfo.setLastModificationTime(now);
         clonedGroup->m_data.timeInfo.setLastAccessTime(now);
         clonedGroup->m_data.timeInfo.setLocationChanged(now);
+        // preserve LastModificationTime
     }
 
     if (groupFlags & Group::CloneRenameTitle) {

src/core/Group.h

@@ -47,10 +47,13 @@ public:
     {
         CloneNoFlags = 0,
         CloneNewUuid = 1, // generate a random uuid for the clone
-        CloneResetTimeInfo = 2, // set all TimeInfo attributes to the current time
+        CloneResetTimeInfo = 2, // set all TimeInfo attributes except LastModificationTime to the current time
         CloneIncludeEntries = 4, // clone the group entries
-        CloneDefault = CloneNewUuid | CloneResetTimeInfo | CloneIncludeEntries,
         CloneRenameTitle = 8, // add "- Clone" after the original title
+
+        CloneCopy = CloneNewUuid | CloneResetTimeInfo | CloneIncludeEntries,
+        CloneExactCopy = CloneIncludeEntries,
+        CloneDefault = CloneCopy,
     };
     Q_DECLARE_FLAGS(CloneFlags, CloneFlag)
 

src/core/Metadata.cpp

@@ -419,14 +419,21 @@ QUuid Metadata::findCustomIcon(const QByteArray& candidate)
     return m_customIconsHashes.value(hash, QUuid());
 }
 
+void Metadata::copyCustomIcon(const QUuid& iconUuid, const Metadata* otherMetadata)
+{
+    if (iconUuid.isNull()) { 
+        return;
+    }
+    Q_ASSERT(otherMetadata->hasCustomIcon(iconUuid));
+    if (!hasCustomIcon(iconUuid) && otherMetadata->hasCustomIcon(iconUuid)) {
+        addCustomIcon(iconUuid, otherMetadata->customIcon(iconUuid));
+    }
+}
+
 void Metadata::copyCustomIcons(const QSet<QUuid>& iconList, const Metadata* otherMetadata)
 {
     for (const QUuid& uuid : iconList) {
-        Q_ASSERT(otherMetadata->hasCustomIcon(uuid));
-
-        if (!hasCustomIcon(uuid) && otherMetadata->hasCustomIcon(uuid)) {
-            addCustomIcon(uuid, otherMetadata->customIcon(uuid));
-        }
+        copyCustomIcon(uuid, otherMetadata);
     }
 }
 

src/core/Metadata.h

@@ -138,6 +138,7 @@ public:
                        const QString& name = {},
                        const QDateTime& lastModified = {});
     void removeCustomIcon(const QUuid& uuid);
+    void copyCustomIcon(const QUuid& iconUuid, const Metadata* otherMetadata);
     void copyCustomIcons(const QSet<QUuid>& iconList, const Metadata* otherMetadata);
     QUuid findCustomIcon(const QByteArray& candidate);
     void setRecycleBinEnabled(bool value);

src/gui/MainWindow.cpp

@@ -708,6 +708,10 @@ QList<DatabaseWidget*> MainWindow::getOpenDatabases()
     return dbWidgets;
 }
 
+DatabaseWidget* MainWindow::currentDatabaseWidget() {
+    return m_ui->tabWidget->currentDatabaseWidget();
+}
+
 void MainWindow::showErrorMessage(const QString& message)
 {
     m_ui->globalMessageWidget->showMessage(message, MessageWidget::Error);

src/gui/MainWindow.h

@@ -52,6 +52,7 @@ public:
     ~MainWindow() override;
 
     QList<DatabaseWidget*> getOpenDatabases();
+    DatabaseWidget* currentDatabaseWidget();
     void restoreConfigState();
     void setAllowScreenCapture(bool state);
 

src/gui/group/GroupModel.cpp

@@ -25,6 +25,7 @@
 #include "core/Tools.h"
 #include "gui/DatabaseIcons.h"
 #include "gui/Icons.h"
+#include "gui/MainWindow.h"
 #include "keeshare/KeeShare.h"
 
 GroupModel::GroupModel(Database* db, QObject* parent)
@@ -229,6 +230,13 @@ bool GroupModel::dropMimeData(const QMimeData* data,
 
     Group* parentGroup = groupFromIndex(parent);
 
+    auto showErrorMessage = [](const QString& errorMessage){
+        auto dbWidget = getMainWindow()->currentDatabaseWidget();
+        if(dbWidget) {
+            dbWidget->showErrorMessage(errorMessage);
+        }
+    };
+
     if (isGroup) {
         QUuid dbUuid;
         QUuid groupUuid;
@@ -258,17 +266,49 @@ bool GroupModel::dropMimeData(const QMimeData* data,
         Group* group = dragGroup;
 
         if (sourceDb != targetDb) {
-            QSet<QUuid> customIcons = group->customIconsRecursive();
-            targetDb->metadata()->copyCustomIcons(customIcons, sourceDb->metadata());
-
-            // Always clone the group across db's to reset UUIDs
-            group = dragGroup->clone(Entry::CloneDefault | Entry::CloneIncludeHistory);
             if (action == Qt::MoveAction) {
-                // Remove the original group from the sourceDb
-                delete dragGroup;
+                Group* binGroup = sourceDb->metadata()->recycleBin();
+                if(binGroup && binGroup->uuid() == dragGroup->uuid()) {
+                    showErrorMessage(tr("Move Error: %1 group cannot be moved").arg(tr("Recycle bin")));
+                    return true;
+                }
+
+                // the move is complex when any group or entry is known in the other db
+                bool complexMove = false;
+                for (const Group* g: group->groupsRecursive(true)) {
+                    if (targetDb->containsDeletedObject(g->uuid()) || targetDb->rootGroup()->findGroupByUuid(g->uuid())) {
+                        complexMove = true;
+                        break;
+                    }
+                }
+                for (const Entry* e: group->entriesRecursive(false)) {
+                    if (targetDb->containsDeletedObject(e->uuid()) || targetDb->rootGroup()->findEntryByUuid(e->uuid())) {
+                        complexMove = true;
+                        break;
+                    }
+                }
+                // TODO: unable to handle complex moves until the Merger interface supports single group/entry merging.
+                if (complexMove) {
+                    showErrorMessage(tr("Move Error: (sub-)group(s) and/or entry(s) already present in the other database"));
+                    return true;
+                }
+
+                group = dragGroup->clone(Entry::CloneExactCopy, Group::CloneExactCopy);
             }
-        } else if (action == Qt::CopyAction) {
-            group = dragGroup->clone(Entry::CloneCopy);
+
+            targetDb->metadata()->copyCustomIcons(dragGroup->customIconsRecursive(), sourceDb->metadata());
+        }
+
+        if (action == Qt::MoveAction) {
+            // remove the original group if it moved a clone
+            if (group != dragGroup) {
+                QList<DeletedObject> delObjects(sourceDb->deletedObjects());
+                delete dragGroup;
+                // prevent group, sub-group(s) & entry(s) from ending up on the deleted object list
+                sourceDb->setDeletedObjects(delObjects);
+            }
+        } else { // Action == Qt::CopyAction
+            group = dragGroup->clone(Entry::CloneCopy, Group::CloneCopy); 
         }
 
         group->setParent(parentGroup, row);
@@ -277,11 +317,13 @@ bool GroupModel::dropMimeData(const QMimeData* data,
             return false;
         }
 
+        int numEntries = 0, numEntriesNotMoved = 0;
         while (!stream.atEnd()) {
             QUuid dbUuid;
             QUuid entryUuid;
             stream >> dbUuid >> entryUuid;
-
+            ++numEntries;
+            
             Database* db = Database::databaseByUuid(dbUuid);
             if (!db) {
                 continue;
@@ -298,22 +340,39 @@ bool GroupModel::dropMimeData(const QMimeData* data,
             Entry* entry = dragEntry;
 
             if (sourceDb != targetDb) {
-                QUuid customIcon = entry->iconUuid();
-                if (!customIcon.isNull() && !targetDb->metadata()->hasCustomIcon(customIcon)) {
-                    targetDb->metadata()->addCustomIcon(customIcon, sourceDb->metadata()->customIcon(customIcon).data);
+                if (action == Qt::MoveAction) {
+                    // the move is complex when the entry is known in the other db
+                    bool complexMove = targetDb->containsDeletedObject(entry->uuid()) 
+                        || targetDb->rootGroup()->findEntryByUuid(entry->uuid());
+                    // TODO: unable to handle complex moves until the Merger interface supports single group/entry merging.
+                    if (complexMove) {
+                        ++numEntriesNotMoved;
+                        continue;
+                    }
+                    entry = dragEntry->clone(Entry::CloneExactCopy);
                 }
 
-                // Reset the UUID when moving across db boundary
-                entry = dragEntry->clone(Entry::CloneDefault | Entry::CloneIncludeHistory);
-                if (action == Qt::MoveAction) {
+                targetDb->metadata()->copyCustomIcon(entry->iconUuid(), sourceDb->metadata());
+            }
+
+            if (action == Qt::MoveAction) {
+                // remove the original entry if it moved a clone
+                if (entry != dragEntry) {
+                    QList<DeletedObject> delObjects(sourceDb->deletedObjects());
                     delete dragEntry;
+                    // prevent entry from ending up on the deleted object list
+                    sourceDb->setDeletedObjects(delObjects);
                 }
-            } else if (action == Qt::CopyAction) {
+            } else { // Action == Qt::CopyAction
                 entry = dragEntry->clone(Entry::CloneCopy);
             }
 
             entry->setGroup(parentGroup);
         }
+
+        if (numEntriesNotMoved) {            
+            showErrorMessage(tr("Move Error: %1 of %2 entry(s) already present in the other database").arg(numEntriesNotMoved).arg(numEntries));
+        }
     }
 
     return true;