Correct refactor issues with entry selection and search (#2518)

* Align entryview selection change signals with groupview
* Eliminate redundent and confusing signals/slots
* Correct group selection canceling search

src/gui/DatabaseWidget.cpp

@@ -157,16 +157,14 @@ DatabaseWidget::DatabaseWidget(QSharedPointer<Database> db, QWidget* parent)
 
     connect(m_mainSplitter, SIGNAL(splitterMoved(int,int)), SIGNAL(mainSplitterSizesChanged()));
     connect(m_previewSplitter, SIGNAL(splitterMoved(int,int)), SIGNAL(previewSplitterSizesChanged()));
-    connect(this, SIGNAL(pressedEntry(Entry*)), m_previewView, SLOT(setEntry(Entry*)));
-    connect(this, SIGNAL(pressedGroup(Group*)), m_previewView, SLOT(setGroup(Group*)));
     connect(this, SIGNAL(currentModeChanged(DatabaseWidget::Mode)), m_previewView, SLOT(setDatabaseMode(DatabaseWidget::Mode)));
     connect(m_previewView, SIGNAL(errorOccurred(QString)), this, SLOT(showErrorMessage(QString)));
     connect(m_entryView, SIGNAL(viewStateChanged()), SIGNAL(entryViewStateChanged()));
-    connect(m_groupView, SIGNAL(groupChanged(Group*)), this, SLOT(onGroupChanged(Group*)));
+    connect(m_groupView, SIGNAL(groupSelectionChanged(Group*)), SLOT(onGroupChanged(Group*)));
-    connect(m_groupView, SIGNAL(groupChanged(Group*)), SIGNAL(groupChanged()));
+    connect(m_groupView, SIGNAL(groupSelectionChanged(Group*)), SIGNAL(groupChanged()));
     connect(m_entryView, SIGNAL(entryActivated(Entry*,EntryModel::ModelColumn)),
         SLOT(entryActivationSignalReceived(Entry*,EntryModel::ModelColumn)));
-    connect(m_entryView, SIGNAL(entrySelectionChanged()), SIGNAL(entrySelectionChanged()));
+    connect(m_entryView, SIGNAL(entrySelectionChanged(Entry*)), SLOT(onEntryChanged(Entry*)));
     connect(m_editEntryWidget, SIGNAL(editFinished(bool)), SLOT(switchToMainView(bool)));
     connect(m_editEntryWidget, SIGNAL(historyEntryActivated(Entry*)), SLOT(switchToHistoryView(Entry*)));
     connect(m_historyEditEntryWidget, SIGNAL(editFinished(bool)), SLOT(switchBackToEntryEdit()));
@@ -180,10 +178,6 @@ DatabaseWidget::DatabaseWidget(QSharedPointer<Database> db, QWidget* parent)
     connect(&m_fileWatchUnblockTimer, SIGNAL(timeout()), this, SLOT(unblockAutoReload()));
     connect(this, SIGNAL(currentChanged(int)), this, SLOT(emitCurrentModeChanged()));
 
-    connect(m_groupView, SIGNAL(groupPressed(Group*)), SLOT(emitPressedGroup(Group*)));
-    connect(m_groupView, SIGNAL(groupChanged(Group*)), SLOT(emitPressedGroup(Group*)));
-    connect(m_editEntryWidget, SIGNAL(editFinished(bool)), SLOT(emitEntrySelectionChanged()));
-
     connectDatabaseSignals();
 
     m_fileWatchTimer.setSingleShot(true);
@@ -700,6 +694,12 @@ void DatabaseWidget::switchToMainView(bool previousDialogAccepted)
     }
 
     setCurrentWidget(m_mainWidget);
+
+    if (sender() == m_entryView) {
+        onEntryChanged(m_entryView->currentEntry());
+    } else if (sender() == m_groupView) {
+        onGroupChanged(m_groupView->currentGroup());
+    }
 }
 
 void DatabaseWidget::switchToHistoryView(Entry* entry)
@@ -879,7 +879,6 @@ void DatabaseWidget::entryActivationSignalReceived(Entry* entry, EntryModel::Mod
         // Call this first to clear out of search mode, otherwise
         // the desired entry is not properly selected
         endSearch();
-        emit clearSearch();
         m_groupView->setCurrentGroup(entry->group());
         m_entryView->setCurrentEntry(entry);
         break;
@@ -1030,14 +1029,15 @@ void DatabaseWidget::setSearchLimitGroup(bool state)
 void DatabaseWidget::onGroupChanged(Group* group)
 {
     // Intercept group changes if in search mode
-    if (isSearchActive()) {
+    if (isSearchActive() && m_searchLimitGroup) {
         search(m_lastSearchText);
     } else if (isSearchActive()) {
-        // Otherwise cancel search
+        endSearch();
-        emit clearSearch();
     } else {
         m_entryView->displayGroup(group);
     }
+
+    m_previewView->setGroup(group);
 }
 
 QString DatabaseWidget::getCurrentSearch()
@@ -1060,6 +1060,9 @@ void DatabaseWidget::endSearch()
     m_searchingLabel->setText(tr("Searching..."));
 
     m_lastSearchText.clear();
+
+    // Tell the search widget to clear
+    emit clearSearch();
 }
 
 void DatabaseWidget::emitGroupContextMenuRequested(const QPoint& pos)
@@ -1072,26 +1075,15 @@ void DatabaseWidget::emitEntryContextMenuRequested(const QPoint& pos)
     emit entryContextMenuRequested(m_entryView->viewport()->mapToGlobal(pos));
 }
 
-void DatabaseWidget::emitEntrySelectionChanged()
+void DatabaseWidget::onEntryChanged(Entry* entry)
 {
-    Entry* currentEntry = m_entryView->currentEntry();
+    if (entry) {
-    if (currentEntry) {
+        m_previewView->setEntry(entry);
-        m_previewView->setEntry(currentEntry);
     }
 
     emit entrySelectionChanged();
 }
 
-void DatabaseWidget::emitPressedGroup(Group* currentGroup)
-{
-    if (!currentGroup) {
-        // if no group is pressed, leave in details the last group
-        return;
-    }
-
-    emit pressedGroup(currentGroup);
-}
-
 bool DatabaseWidget::canDeleteCurrentGroup() const
 {
     bool isRootGroup = m_db->rootGroup() == m_groupView->currentGroup();

src/gui/DatabaseWidget.h

@@ -132,8 +132,6 @@ signals:
     void databaseMerged(QSharedPointer<Database> mergedDb);
     void groupContextMenuRequested(const QPoint& globalPos);
     void entryContextMenuRequested(const QPoint& globalPos);
-    void pressedEntry(Entry* selectedEntry);
-    void pressedGroup(Group* selectedGroup);
     void listModeAboutToActivate();
     void listModeActivated();
     void searchModeAboutToActivate();
@@ -168,7 +166,6 @@ public slots:
     void openUrlForEntry(Entry* entry);
     void createGroup();
     void deleteGroup();
-    void onGroupChanged(Group* group);
     void switchToMainView(bool previousDialogAccepted = false);
     void switchToEntryEdit();
     void switchToGroupEdit();
@@ -210,8 +207,8 @@ private slots:
     void switchToGroupEdit(Group* entry, bool create);
     void emitGroupContextMenuRequested(const QPoint& pos);
     void emitEntryContextMenuRequested(const QPoint& pos);
-    void emitPressedGroup(Group* currentGroup);
+    void onEntryChanged(Entry* entry);
-    void emitEntrySelectionChanged();
+    void onGroupChanged(Group* group);
     void connectDatabaseSignals();
     void loadDatabase(bool accepted);
     void unlockDatabase(bool accepted);

src/gui/entry/EntryView.cpp

@@ -50,7 +50,7 @@ EntryView::EntryView(QWidget* parent)
     setDefaultDropAction(Qt::MoveAction);
 
     connect(this, SIGNAL(doubleClicked(QModelIndex)), SLOT(emitEntryActivated(QModelIndex)));
-    connect(selectionModel(), SIGNAL(selectionChanged(QItemSelection,QItemSelection)), SIGNAL(entrySelectionChanged()));
+    connect(selectionModel(), SIGNAL(selectionChanged(QItemSelection,QItemSelection)), SLOT(emitEntrySelectionChanged()));
     connect(m_model, SIGNAL(usernamesHiddenChanged()), SIGNAL(viewStateChanged()));
     connect(m_model, SIGNAL(passwordsHiddenChanged()), SIGNAL(viewStateChanged()));
 
@@ -144,13 +144,13 @@ void EntryView::keyPressEvent(QKeyEvent* event)
 
 void EntryView::focusInEvent(QFocusEvent* event)
 {
-    emit entrySelectionChanged();
+    emit entrySelectionChanged(currentEntry());
     QTreeView::focusInEvent(event);
 }
 
 void EntryView::focusOutEvent(QFocusEvent* event)
 {
-    emit entrySelectionChanged();
+    emit entrySelectionChanged(nullptr);
     QTreeView::focusOutEvent(event);
 }
 
@@ -181,7 +181,7 @@ void EntryView::setFirstEntryActive()
         QModelIndex index = m_sortModel->mapToSource(m_sortModel->index(0, 0));
         setCurrentEntry(m_model->entryFromIndex(index));
     } else {
-        emit entrySelectionChanged();
+        emit entrySelectionChanged(currentEntry());
     }
 }
 
@@ -196,6 +196,11 @@ void EntryView::emitEntryActivated(const QModelIndex& index)
     emit entryActivated(entry, static_cast<EntryModel::ModelColumn>(m_sortModel->mapToSource(index).column()));
 }
 
+void EntryView::emitEntrySelectionChanged()
+{
+    emit entrySelectionChanged(currentEntry());
+}
+
 void EntryView::setModel(QAbstractItemModel* model)
 {
     Q_UNUSED(model);

src/gui/entry/EntryView.h

@@ -52,7 +52,7 @@ public:
 
 signals:
     void entryActivated(Entry* entry, EntryModel::ModelColumn column);
-    void entrySelectionChanged();
+    void entrySelectionChanged(Entry* entry);
     void viewStateChanged();
 
 public slots:
@@ -66,6 +66,7 @@ protected:
 
 private slots:
     void emitEntryActivated(const QModelIndex& index);
+    void emitEntrySelectionChanged();
     void showHeaderMenu(const QPoint& position);
     void toggleColumnVisibility(QAction* action);
     void fitColumnsToWindow();

src/gui/group/GroupView.cpp

@@ -34,15 +34,12 @@ GroupView::GroupView(Database* db, QWidget* parent)
     setHeaderHidden(true);
     setUniformRowHeights(true);
 
-    connect(this, SIGNAL(expanded(QModelIndex)), this, SLOT(expandedChanged(QModelIndex)));
+    connect(this, SIGNAL(expanded(QModelIndex)), SLOT(expandedChanged(QModelIndex)));
-    connect(this, SIGNAL(collapsed(QModelIndex)), this, SLOT(expandedChanged(QModelIndex)));
+    connect(this, SIGNAL(collapsed(QModelIndex)), SLOT(expandedChanged(QModelIndex)));
     connect(m_model, SIGNAL(rowsInserted(QModelIndex,int,int)), SLOT(syncExpandedState(QModelIndex,int,int)));
     connect(m_model, SIGNAL(modelReset()), SLOT(modelReset()));
-
     connect(selectionModel(), SIGNAL(currentChanged(QModelIndex,QModelIndex)), SLOT(emitGroupChanged()));
 
-    connect(this, SIGNAL(clicked(QModelIndex)), SLOT(emitGroupPressed(QModelIndex)));
-
     modelReset();
 
     setDragEnabled(true);
@@ -110,11 +107,6 @@ void GroupView::expandGroup(Group* group, bool expand)
     setExpanded(index, expand);
 }
 
-void GroupView::emitGroupChanged(const QModelIndex& index)
-{
-    emit groupChanged(m_model->groupFromIndex(index));
-}
-
 void GroupView::setModel(QAbstractItemModel* model)
 {
     Q_UNUSED(model);
@@ -123,12 +115,7 @@ void GroupView::setModel(QAbstractItemModel* model)
 
 void GroupView::emitGroupChanged()
 {
-    emit groupChanged(currentGroup());
+    emit groupSelectionChanged(currentGroup());
-}
-
-void GroupView::emitGroupPressed(const QModelIndex& index)
-{
-    emit groupPressed(m_model->groupFromIndex(index));
 }
 
 void GroupView::syncExpandedState(const QModelIndex& parent, int start, int end)

src/gui/group/GroupView.h

@@ -37,14 +37,11 @@ public:
     void expandGroup(Group* group, bool expand = true);
 
 signals:
-    void groupChanged(Group* group);
+    void groupSelectionChanged(Group* group);
-    void groupPressed(Group* group);
 
 private slots:
     void expandedChanged(const QModelIndex& index);
-    void emitGroupChanged(const QModelIndex& index);
     void emitGroupChanged();
-    void emitGroupPressed(const QModelIndex& index);
     void syncExpandedState(const QModelIndex& parent, int start, int end);
     void modelReset();
 

tests/gui/TestGui.cpp

@@ -875,18 +875,28 @@ void TestGui::testSearch()
     QTRY_COMPARE(entryView->model()->rowCount(), 2);
 
     // Test group search
+    searchWidget->setLimitGroup(false);
     GroupView* groupView = m_dbWidget->findChild<GroupView*>("groupView");
     QCOMPARE(groupView->currentGroup(), m_db->rootGroup());
     QModelIndex rootGroupIndex = groupView->model()->index(0, 0);
     clickIndex(groupView->model()->index(0, 0, rootGroupIndex), groupView, Qt::LeftButton);
     QCOMPARE(groupView->currentGroup()->name(), QString("General"));
-
+    // Selecting a group should cancel search
-    searchWidget->setLimitGroup(false);
+    QTRY_COMPARE(entryView->model()->rowCount(), 0);
+    // Restore search
+    QTest::keyClick(m_mainWindow.data(), Qt::Key_F, Qt::ControlModifier);
+    QTest::keyClicks(searchTextEdit, "someTHING");
     QTRY_COMPARE(entryView->model()->rowCount(), 2);
+    // Enable group limiting
     searchWidget->setLimitGroup(true);
     QTRY_COMPARE(entryView->model()->rowCount(), 0);
+    // Selecting another group should NOT cancel search
+    clickIndex(rootGroupIndex, groupView, Qt::LeftButton);
+    QCOMPARE(groupView->currentGroup(), m_db->rootGroup());
+    QTRY_COMPARE(entryView->model()->rowCount(), 2);
 
     // reset
+    searchWidget->setLimitGroup(false);
     clickIndex(rootGroupIndex, groupView, Qt::LeftButton);
     QCOMPARE(groupView->currentGroup(), m_db->rootGroup());