Don't fetch tags each time that we open a new message

src/tag/tagactionmanager.cpp

@@ -45,6 +45,7 @@ TagActionManager::TagActionManager(QObject *parent, KActionCollection *actionCol
     connect(tagMonitorManager, &TagMonitorManager::tagAdded, this, &TagActionManager::onTagAdded);
     connect(tagMonitorManager, &TagMonitorManager::tagRemoved, this, &TagActionManager::onTagRemoved);
     connect(tagMonitorManager, &TagMonitorManager::tagChanged, this, &TagActionManager::onTagChanged);
+    connect(tagMonitorManager, &TagMonitorManager::fetchTagDone, this, &TagActionManager::createActions);
 }
 
 TagActionManager::~TagActionManager()
@@ -122,34 +123,7 @@ void TagActionManager::createTagAction(const MailCommon::Tag::Ptr &tag, bool add
 
 void TagActionManager::createActions()
 {
-    if (mTagFetchInProgress) {
-        return;
-    }
-
-    if (mTags.isEmpty()) {
-        mTagFetchInProgress = true;
-        Akonadi::TagFetchJob *fetchJob = new Akonadi::TagFetchJob(this);
-        fetchJob->fetchScope().fetchAttribute<Akonadi::TagAttribute>();
-        connect(fetchJob, &Akonadi::TagFetchJob::result, this, &TagActionManager::finishedTagListing);
-    } else {
-        mTagFetchInProgress = false;
-        createTagActions(mTags);
-    }
-}
-
-void TagActionManager::finishedTagListing(KJob *job)
-{
-    if (job->error()) {
-        qCWarning(KMAIL_LOG) << job->errorString();
-    }
-    Akonadi::TagFetchJob *fetchJob = static_cast<Akonadi::TagFetchJob *>(job);
-    const Akonadi::Tag::List lstTags = fetchJob->tags();
-    for (const Akonadi::Tag &result : lstTags) {
-        mTags.append(MailCommon::Tag::fromAkonadi(result));
-    }
-    mTagFetchInProgress = false;
-    std::sort(mTags.begin(), mTags.end(), MailCommon::Tag::compare);
-    createTagActions(mTags);
+    createTagActions(TagMonitorManager::self()->tags());
 }
 
 void TagActionManager::onSignalMapped(const QString &tag)
@@ -231,7 +205,8 @@ void TagActionManager::updateActionStates(int numberOfSelectedMessages, const Ak
         for (; it != end; ++it) {
             //FIXME Not very performant tag label retrieval
             QString label(i18n("Tag not Found"));
-            for (const MailCommon::Tag::Ptr &tag : qAsConst(mTags)) {
+            const auto tags = TagMonitorManager::self()->tags();
+            for (const MailCommon::Tag::Ptr &tag : tags) {
                 if (tag->id() == it.key()) {
                     label = tag->name();
                     break;
@@ -241,7 +216,6 @@ void TagActionManager::updateActionStates(int numberOfSelectedMessages, const Ak
             it.value()->setEnabled(true);
             if (numberOfSelectedMessages == 1) {
                 const bool hasTag = selectedItem.hasTag(Akonadi::Tag(it.key()));
-                qDebug() << "hasTag  " << hasTag;
                 it.value()->setChecked(hasTag);
                 it.value()->setText(i18n("Message Tag: %1", label));
             } else {
@@ -256,39 +230,18 @@ void TagActionManager::updateActionStates(int numberOfSelectedMessages, const Ak
     }
 }
 
-void TagActionManager::onTagAdded(const Akonadi::Tag &akonadiTag)
+void TagActionManager::onTagAdded()
 {
-    const QList<qint64> checked = checkedTags();
-
-    clearActions();
-    mTags.append(MailCommon::Tag::fromAkonadi(akonadiTag));
-    std::sort(mTags.begin(), mTags.end(), MailCommon::Tag::compare);
-    createTagActions(mTags);
-
-    checkTags(checked);
+    fillTagList();
 }
 
-void TagActionManager::onTagRemoved(const Akonadi::Tag &akonadiTag)
+void TagActionManager::onTagRemoved()
 {
-    for (const MailCommon::Tag::Ptr &tag : qAsConst(mTags)) {
-        if (tag->id() == akonadiTag.id()) {
-            mTags.removeAll(tag);
-            break;
-        }
-    }
-
     fillTagList();
 }
 
-void TagActionManager::onTagChanged(const Akonadi::Tag &akonadiTag)
+void TagActionManager::onTagChanged()
 {
-    for (const MailCommon::Tag::Ptr &tag : qAsConst(mTags)) {
-        if (tag->id() == akonadiTag.id()) {
-            mTags.removeAll(tag);
-            break;
-        }
-    }
-    mTags.append(MailCommon::Tag::fromAkonadi(akonadiTag));
     fillTagList();
 }
 
@@ -297,8 +250,7 @@ void TagActionManager::fillTagList()
     const QList<qint64> checked = checkedTags();
 
     clearActions();
-    std::sort(mTags.begin(), mTags.end(), MailCommon::Tag::compare);
-    createTagActions(mTags);
+    createTagActions(TagMonitorManager::self()->tags());
 
     checkTags(checked);
 }
@@ -306,7 +258,7 @@ void TagActionManager::fillTagList()
 void TagActionManager::newTagActionClicked()
 {
     QPointer<MailCommon::AddTagDialog> dialog = new MailCommon::AddTagDialog(QList<KActionCollection *>() << mActionCollection, nullptr);
-    dialog->setTags(mTags);
+    dialog->setTags(TagMonitorManager::self()->tags());
     if (dialog->exec() == QDialog::Accepted) {
         mNewTagId = dialog->tag().id();
         // Assign tag to all selected items right away

src/tag/tagactionmanager.h

@@ -102,12 +102,11 @@ private:
     };
 
     Q_DISABLE_COPY(TagActionManager)
-    void finishedTagListing(KJob *job);
     void newTagActionClicked();
     void onSignalMapped(const QString &tag);
-    void onTagAdded(const Akonadi::Tag &);
-    void onTagRemoved(const Akonadi::Tag &);
-    void onTagChanged(const Akonadi::Tag &);
+    void onTagAdded();
+    void onTagRemoved();
+    void onTagChanged();
 
     void fillTagList();
     void createTagAction(const MailCommon::Tag::Ptr &tag, bool addToMenu);
@@ -130,9 +129,6 @@ private:
     // The actions of all tags that are in the toolbar
     QList<QAction *> mToolbarActions;
 
-    // A sorted list of all tags
-    QVector<MailCommon::Tag::Ptr> mTags;
-
     MessagesInfo mMessageInfo;
     // Uri of a newly created tag
     qint64 mNewTagId = -1;

src/tag/tagmonitormanager.cpp

@@ -5,6 +5,8 @@
 */
 
 #include "tagmonitormanager.h"
+#include "kmail_debug.h"
+
 #include <AkonadiCore/Monitor>
 #include <AkonadiCore/TagFetchJob>
 #include <AkonadiCore/TagFetchScope>
@@ -17,9 +19,10 @@ TagMonitorManager::TagMonitorManager(QObject *parent)
     mMonitor->setObjectName(QStringLiteral("TagActionManagerMonitor"));
     mMonitor->setTypeMonitored(Akonadi::Monitor::Tags);
     mMonitor->tagFetchScope().fetchAttribute<Akonadi::TagAttribute>();
-    connect(mMonitor, &Akonadi::Monitor::tagAdded, this, &TagMonitorManager::tagAdded);
-    connect(mMonitor, &Akonadi::Monitor::tagRemoved, this, &TagMonitorManager::tagRemoved);
-    connect(mMonitor, &Akonadi::Monitor::tagChanged, this, &TagMonitorManager::tagChanged);
+    connect(mMonitor, &Akonadi::Monitor::tagAdded, this, &TagMonitorManager::onTagAdded);
+    connect(mMonitor, &Akonadi::Monitor::tagRemoved, this, &TagMonitorManager::onTagRemoved);
+    connect(mMonitor, &Akonadi::Monitor::tagChanged, this, &TagMonitorManager::onTagChanged);
+    createActions();
 }
 
 TagMonitorManager::~TagMonitorManager()
@@ -32,3 +35,62 @@ TagMonitorManager *TagMonitorManager::self()
     static TagMonitorManager s_self;
     return &s_self;
 }
+
+void TagMonitorManager::createActions()
+{
+    if (mTags.isEmpty()) {
+        Akonadi::TagFetchJob *fetchJob = new Akonadi::TagFetchJob(this);
+        fetchJob->fetchScope().fetchAttribute<Akonadi::TagAttribute>();
+        connect(fetchJob, &Akonadi::TagFetchJob::result, this, &TagMonitorManager::finishedTagListing);
+    }
+}
+
+void TagMonitorManager::finishedTagListing(KJob *job)
+{
+    if (job->error()) {
+        qCWarning(KMAIL_LOG) << job->errorString();
+    }
+    Akonadi::TagFetchJob *fetchJob = static_cast<Akonadi::TagFetchJob *>(job);
+    const Akonadi::Tag::List lstTags = fetchJob->tags();
+    for (const Akonadi::Tag &result : lstTags) {
+        mTags.append(MailCommon::Tag::fromAkonadi(result));
+    }
+    std::sort(mTags.begin(), mTags.end(), MailCommon::Tag::compare);
+    Q_EMIT fetchTagDone();
+}
+
+QVector<MailCommon::Tag::Ptr> TagMonitorManager::tags() const
+{
+    return mTags;
+}
+
+void TagMonitorManager::onTagAdded(const Akonadi::Tag &akonadiTag)
+{
+    mTags.append(MailCommon::Tag::fromAkonadi(akonadiTag));
+    std::sort(mTags.begin(), mTags.end(), MailCommon::Tag::compare);
+    Q_EMIT tagAdded();
+}
+
+void TagMonitorManager::onTagRemoved(const Akonadi::Tag &akonadiTag)
+{
+    for (const MailCommon::Tag::Ptr &tag : qAsConst(mTags)) {
+        if (tag->id() == akonadiTag.id()) {
+            mTags.removeAll(tag);
+            break;
+        }
+    }
+    Q_EMIT tagRemoved();
+}
+
+void TagMonitorManager::onTagChanged(const Akonadi::Tag &akonadiTag)
+{
+    for (const MailCommon::Tag::Ptr &tag : qAsConst(mTags)) {
+        if (tag->id() == akonadiTag.id()) {
+            mTags.removeAll(tag);
+            break;
+        }
+    }
+    mTags.append(MailCommon::Tag::fromAkonadi(akonadiTag));
+    std::sort(mTags.begin(), mTags.end(), MailCommon::Tag::compare);
+    Q_EMIT tagChanged();
+}

src/tag/tagmonitormanager.h

@@ -9,9 +9,12 @@
 
 #include <QObject>
 #include <AkonadiCore/Tag>
+#include <mailcommon/tag.h>
 namespace Akonadi {
 class Monitor;
 }
+class KJob;
+
 class TagMonitorManager : public QObject
 {
     Q_OBJECT
@@ -21,12 +24,25 @@ public:
 
     static TagMonitorManager *self();
 
+
+    Q_REQUIRED_RESULT QVector<MailCommon::Tag::Ptr> tags() const;
+
 Q_SIGNALS:
-    void tagAdded(const Akonadi::Tag &tag);
-    void tagChanged(const Akonadi::Tag &tag);
-    void tagRemoved(const Akonadi::Tag &tag);
+    void tagAdded();
+    void tagChanged();
+    void tagRemoved();
+    void fetchTagDone();
 
 private:
+    void createActions();
+    void finishedTagListing(KJob *job);
+    void onTagAdded(const Akonadi::Tag &akonadiTag);
+    void onTagRemoved(const Akonadi::Tag &akonadiTag);
+    void onTagChanged(const Akonadi::Tag &akonadiTag);
+
+    // A sorted list of all tags
+    QVector<MailCommon::Tag::Ptr> mTags;
+
     Akonadi::Monitor *const mMonitor;
 };