Add batch tag editing for multiple selected files

Select several notes in a bundled notebook, choose Tags, and add or remove
tags on all of them through the existing ViewTagsDialog2 surface.

TagViewer2 becomes multi-node with three per-tag states: All (every target
has it), Partial (some do, shown italic with an "N of M" tooltip) and None.
Clicking cycles Partial/None to All and All to None, and the dialog yields
addedTags()/removedTags(). Only that delta is written, via the incremental
TagService::tagFile/untagFile primitives, so a tag left Partial and any tag
outside the list are never touched and no file's tag array is overwritten.

Persistence goes through the controller per the multi-target batch pattern:
manageTagsRequested carries a QList, NotebookExplorer2 shows ONE dialog, and
the per-id handleTagDeltaResult is looped in the view. Targets are the
current selection filtered to valid, non-folder, non-root nodes of the
clicked notebook; the same predicate greys out the action. TagPopup2 keeps
the legacy single-node save() path unchanged.

vxcore's tag primitives are not idempotent, so applyTagDelta pre-reads each
file and drops the ops that would be no-ops on it; every failure that then
comes back is genuine. Unreadable targets are excluded from the tri-state
rather than counted as untagged, since getFileInfo returns an empty object
on failure. TagModel coalesces its reload so a batch resets the tag tree
once instead of once per tag per file.

Author: tamlok

src/controllers/CMakeLists.txt

@@ -5,6 +5,7 @@ target_sources(vnote PRIVATE
     notebooknodecontroller.cpp notebooknodecontroller.h
     notebooknodecontroller_reorder.cpp
     notebooknodecontroller_shareseam.cpp
+    notebooknodecontroller_tagseam.cpp
     newnotebookcontroller.cpp newnotebookcontroller.h
     newnotecontroller.cpp newnotecontroller.h
     newfoldercontroller.cpp newfoldercontroller.h

src/controllers/notebooknodecontroller.cpp

@@ -21,6 +21,7 @@
 #include <core/servicelocator.h>
 #include <core/services/bufferservice.h>
 #include <core/services/notebookcoreservice.h>
+#include <core/services/tagservice.h>
 #include <core/services/workspacecoreservice.h>
 #include <core/sessionconfig.h>
 #include <core/widgetconfig.h>
@@ -192,6 +193,15 @@ bool NotebookNodeController::isSingleEffectiveSelection(const NodeIdentifier &p_
   return resolveSelection(p_clickedId).size() == 1;
 }
 
+QList<NodeIdentifier>
+NotebookNodeController::resolveTagTargets(const NodeIdentifier &p_clickedId) const {
+  if (!p_clickedId.isValid()) {
+    return {};
+  }
+  return filterTagTargets(resolveSelection(p_clickedId), p_clickedId,
+                          [this](const NodeIdentifier &p_id) { return getNodeInfo(p_id); });
+}
+
 QMenu *NotebookNodeController::createContextMenu(const NodeIdentifier &p_nodeId,
                                                  QWidget *p_parent) {
   // Get node info to determine if it's a folder or external
@@ -497,7 +507,7 @@ void NotebookNodeController::addInfoActions(QMenu *p_menu, const NodeIdentifier
       auto *tagAction = p_menu->addAction(tr("&Tags"));
       connect(tagAction, &QAction::triggered, this,
               [this, p_nodeId]() { manageNodeTags(p_nodeId); });
-      tagAction->setEnabled(isSingleEffectiveSelection(p_nodeId) && !p_readOnly);
+      tagAction->setEnabled(!p_readOnly && !resolveTagTargets(p_nodeId).isEmpty());
     }
   }
 }
@@ -1031,7 +1041,11 @@ void NotebookNodeController::manageNodeTags(const NodeIdentifier &p_nodeId) {
   if (isNotebookReadOnly(p_nodeId.notebookId)) {
     return;
   }
-  emit manageTagsRequested(p_nodeId);
+  const auto ids = resolveTagTargets(p_nodeId);
+  if (ids.isEmpty()) {
+    return;
+  }
+  emit manageTagsRequested(ids);
 }
 
 bool NotebookNodeController::canPaste() const { return !m_clipboard->nodes.isEmpty(); }
@@ -1192,6 +1206,56 @@ void NotebookNodeController::handleMarkResult(const NodeIdentifier &p_nodeId,
   }
 }
 
+bool NotebookNodeController::handleTagDeltaResult(const NodeIdentifier &p_nodeId,
+                                                  const QSet<QString> &p_added,
+                                                  const QSet<QString> &p_removed) {
+  if (p_added.isEmpty() && p_removed.isEmpty()) {
+    return true;
+  }
+  if (!p_nodeId.isValid() || isNotebookReadOnly(p_nodeId.notebookId)) {
+    return false;
+  }
+
+  const NodeInfo nodeInfo = getNodeInfo(p_nodeId);
+  if (!nodeInfo.isValid() || nodeInfo.isFolder || nodeInfo.isRoot()) {
+    return false;
+  }
+
+  auto *notebookService = m_services.get<NotebookCoreService>();
+  auto *tagService = m_services.get<TagService>();
+  if (!notebookService || !tagService) {
+    return false;
+  }
+
+  // Orchestration (pre-read, plan, issue, aggregate) lives in
+  // notebooknodecontroller_tagseam.cpp so it is covered by a real controller
+  // test; this method only binds it to the services.
+  TagDeltaIo io;
+  io.readTags = [notebookService, &p_nodeId](QSet<QString> &p_out) {
+    VxCoreError err = VXCORE_OK;
+    const auto fileInfo =
+        notebookService->getFileInfo(p_nodeId.notebookId, p_nodeId.relativePath, &err);
+    if (err != VXCORE_OK) {
+      // getFileInfo returns an empty object on failure, indistinguishable from
+      // "no tags".
+      return false;
+    }
+    const auto tagsArray = fileInfo.value(QLatin1String(vxcore::kJsonKeyTags)).toArray();
+    for (const auto &tagVal : tagsArray) {
+      p_out.insert(tagVal.toString());
+    }
+    return true;
+  };
+  io.tagFile = [tagService, &p_nodeId](const QString &p_tag) {
+    return tagService->tagFile(p_nodeId.notebookId, p_nodeId.relativePath, p_tag);
+  };
+  io.untagFile = [tagService, &p_nodeId](const QString &p_tag) {
+    return tagService->untagFile(p_nodeId.notebookId, p_nodeId.relativePath, p_tag);
+  };
+
+  return applyTagDelta(io, p_added, p_removed);
+}
+
 void NotebookNodeController::handleDeleteConfirmed(const QList<NodeIdentifier> &p_nodeIds,
                                                    bool p_permanent) {
   if (p_nodeIds.isEmpty()) {

src/controllers/notebooknodecontroller.h

@@ -162,6 +162,15 @@ public slots:
   void handleRenameResult(const NodeIdentifier &p_nodeId, const QString &p_newName);
   void handleMarkResult(const NodeIdentifier &p_nodeId, const QString &p_textColor,
                         const QString &p_bgColor);
+  // Per-id apply primitive for the batch Tags flow. Applies the DELTA produced
+  // by ViewTagsDialog2: tags in p_added are applied with TagService::tagFile,
+  // tags in p_removed are dropped with TagService::untagFile. Deliberately NOT
+  // a whole-array rewrite (updateFileTags) — a read-modify-write would erase
+  // concurrent/hook-driven tag changes and would silently succeed for a
+  // remove-only delta when getFileInfo fails. Re-checks read-only + eligibility.
+  // Returns false if any call fails.
+  bool handleTagDeltaResult(const NodeIdentifier &p_nodeId, const QSet<QString> &p_added,
+                            const QSet<QString> &p_removed);
   void handleDeleteConfirmed(const QList<NodeIdentifier> &p_nodeIds, bool p_permanent);
   void handleRemoveConfirmed(const QList<NodeIdentifier> &p_nodeIds);
   void handleImportFiles(const NodeIdentifier &p_targetFolderId, const QStringList &p_files);
@@ -216,7 +225,7 @@ public slots:
 
   void ignoreRequested(const NodeIdentifier &p_nodeId);
 
-  void manageTagsRequested(const NodeIdentifier &p_nodeId);
+  void manageTagsRequested(const QList<NodeIdentifier> &p_ids);
 
   // T7 (notebook-explorer-drag-reorder): emitted after the service confirms
   // a successful folder-children reorder. The View should reload the folder
@@ -253,6 +262,55 @@ public slots:
   static bool isFolderShareEligible(const NodeInfo &p_nodeInfo, bool p_notebookIsBundled,
                                     bool p_singleEffectiveSelection);
 
+  // Pure target-filter behind resolveTagTargets(), extracted into
+  // notebooknodecontroller_tagseam.cpp so controller tests can link the REAL
+  // production predicate without the controller's 18+ transitive service deps.
+  // p_infoLookup plays the role of getNodeInfo(); an unresolved id must yield a
+  // default-constructed NodeInfo, exactly as the controller sees it.
+  //
+  // Keeps only ids that resolve to a valid, non-folder, non-root node in the
+  // SAME notebook as the clicked node, de-duplicated and order-preserving.
+  using NodeInfoLookup = std::function<NodeInfo(const NodeIdentifier &)>;
+  static QList<NodeIdentifier> filterTagTargets(const QList<NodeIdentifier> &p_resolvedSelection,
+                                                const NodeIdentifier &p_clickedId,
+                                                const NodeInfoLookup &p_infoLookup);
+
+  // The ops handleTagDeltaResult actually issues for ONE file, after dropping
+  // the ones that would be no-ops on it. Sorted, so the order is deterministic.
+  struct TagDeltaOps {
+    QStringList toAdd;
+    QStringList toRemove;
+  };
+
+  // Pure delta planner behind handleTagDeltaResult, extracted into
+  // notebooknodecontroller_tagseam.cpp alongside filterTagTargets so controller
+  // tests can link the REAL production decision.
+  //
+  // vxcore's incremental primitives are NOT idempotent (TagFile returns
+  // VXCORE_ERR_ALREADY_EXISTS, UntagFile returns VXCORE_ERR_NOT_FOUND), and a
+  // delta derived from a Partial tag necessarily hits both across a batch. So a
+  // tag already present is dropped from toAdd and a tag already absent is dropped
+  // from toRemove, leaving every remaining failure a genuine one.
+  static TagDeltaOps planTagDelta(const QSet<QString> &p_currentTags, const QSet<QString> &p_added,
+                                  const QSet<QString> &p_removed);
+
+  // I/O seam for applyTagDelta, so the orchestration can be driven without the
+  // controller's services. readTags returns false when the file's current tags
+  // could NOT be determined; tagFile / untagFile return false on failure.
+  struct TagDeltaIo {
+    std::function<bool(QSet<QString> &)> readTags;
+    std::function<bool(const QString &)> tagFile;
+    std::function<bool(const QString &)> untagFile;
+  };
+
+  // Pre-read -> plan -> issue -> aggregate, for ONE file. Returns false when the
+  // pre-read fails (nothing is issued in that case) or when any issued op fails.
+  // Extracted alongside planTagDelta so controller tests cover the REAL
+  // orchestration; handleTagDeltaResult adds only its read-only / eligibility
+  // guards and binds this to the services.
+  static bool applyTagDelta(const TagDeltaIo &p_io, const QSet<QString> &p_added,
+                            const QSet<QString> &p_removed);
+
 private:
   // Resolve selection following Qt right-click convention:
   // - If clicked node is in current selection, return full selection
@@ -265,6 +323,12 @@ public slots:
   // Returns true if resolveSelection(p_clickedId).size() == 1.
   bool isSingleEffectiveSelection(const NodeIdentifier &p_clickedId) const;
 
+  // Eligible targets for the "Tags" action, derived from resolveSelection().
+  // Keeps only nodes that resolve to a valid, non-folder, non-root node in the
+  // SAME notebook as the clicked node. Drives both action enablement and
+  // execution so the menu never offers an action that would be a no-op.
+  QList<NodeIdentifier> resolveTagTargets(const NodeIdentifier &p_clickedId) const;
+
   // Add actions to context menu based on node type. p_readOnly greys out
   // mutating actions when the clicked notebook is read-only.
   void addNewActions(QMenu *p_menu, const NodeIdentifier &p_nodeId, bool p_isFolder,

src/controllers/notebooknodecontroller_tagseam.cpp

@@ -0,0 +1,117 @@
+// Pure helpers behind the batch "Tags" context-menu action: the target filter
+// (filterTagTargets) and the per-file delta planner (planTagDelta).
+//
+// Deliberately isolated in their own translation unit for the same reason as
+// notebooknodecontroller_shareseam.cpp: notebooknodecontroller.cpp pulls in ~18
+// transitive service dependencies, so a GUILESS controller test that only wants
+// these decisions would otherwise have to link the whole controller stack.
+// Compiling just this TU gives the tests REAL production coverage instead of a
+// hand-mirrored copy of the logic.
+
+#include "notebooknodecontroller.h"
+
+#include <algorithm>
+
+#include <core/nodeidentifier.h>
+#include <nodeinfo.h>
+
+using namespace vnotex;
+
+QList<NodeIdentifier>
+NotebookNodeController::filterTagTargets(const QList<NodeIdentifier> &p_resolvedSelection,
+                                         const NodeIdentifier &p_clickedId,
+                                         const NodeInfoLookup &p_infoLookup) {
+  QList<NodeIdentifier> targets;
+  if (!p_clickedId.isValid() || !p_infoLookup) {
+    return targets;
+  }
+
+  for (const auto &id : p_resolvedSelection) {
+    // A batch spans exactly one notebook: the dialog lists that notebook's tags.
+    if (id.notebookId != p_clickedId.notebookId) {
+      continue;
+    }
+
+    // NodeIdentifier::isValid() only checks a non-empty notebookId, and an
+    // unresolved node yields a DEFAULT NodeInfo whose isFolder is false — so
+    // NodeInfo::isValid() is the discriminator that rejects a stale id.
+    const NodeInfo info = p_infoLookup(id);
+    if (!info.isValid() || info.isFolder || info.isRoot()) {
+      continue;
+    }
+
+    if (!targets.contains(id)) {
+      targets.append(id);
+    }
+  }
+
+  return targets;
+}
+
+NotebookNodeController::TagDeltaOps
+NotebookNodeController::planTagDelta(const QSet<QString> &p_currentTags,
+                                     const QSet<QString> &p_added, const QSet<QString> &p_removed) {
+  TagDeltaOps ops;
+
+  for (const auto &tag : p_added) {
+    // Already on this file: issuing TagFile would return
+    // VXCORE_ERR_ALREADY_EXISTS, which is a routine no-op for a delta derived
+    // from a Partial tag, not a failure. Skip it so every failure that does come
+    // back from the service is genuine.
+    if (!p_currentTags.contains(tag)) {
+      ops.toAdd.append(tag);
+    }
+  }
+
+  for (const auto &tag : p_removed) {
+    // Not on this file: UntagFile would return VXCORE_ERR_NOT_FOUND. Same
+    // reasoning as above.
+    if (p_currentTags.contains(tag)) {
+      ops.toRemove.append(tag);
+    }
+  }
+
+  // QSet iteration order is unspecified; sort so the issued order (and any test
+  // assertion over it) is deterministic.
+  std::sort(ops.toAdd.begin(), ops.toAdd.end());
+  std::sort(ops.toRemove.begin(), ops.toRemove.end());
+  return ops;
+}
+bool NotebookNodeController::applyTagDelta(const TagDeltaIo &p_io, const QSet<QString> &p_added,
+                                           const QSet<QString> &p_removed) {
+  if (p_added.isEmpty() && p_removed.isEmpty()) {
+    return true;
+  }
+  if (!p_io.readTags || !p_io.tagFile || !p_io.untagFile) {
+    return false;
+  }
+
+  // Read this file's CURRENT tags so the ops that would be no-ops on it can be
+  // dropped. This is a pre-check that SKIPS work; it is NOT a read-modify-write
+  // of the tag array -- nothing is written back wholesale, so a tag added
+  // concurrently (or by a hook) survives. Because the redundant ops never reach
+  // the service, every failure that does come back is a GENUINE failure and is
+  // reported as one. Confirming state AFTER a failure would instead mask a
+  // persistence error, since vxcore mutates its cached FileRecord before saving.
+  QSet<QString> currentTags;
+  if (!p_io.readTags(currentTags)) {
+    // The current tags are unknown, and "unknown" is indistinguishable from
+    // "none" -- fail loud rather than guess, and issue nothing.
+    return false;
+  }
+
+  const TagDeltaOps ops = planTagDelta(currentTags, p_added, p_removed);
+
+  bool allOk = true;
+  for (const auto &tag : ops.toAdd) {
+    if (!p_io.tagFile(tag)) {
+      allOk = false;
+    }
+  }
+  for (const auto &tag : ops.toRemove) {
+    if (!p_io.untagFile(tag)) {
+      allOk = false;
+    }
+  }
+  return allOk;
+}

src/models/tagmodel.cpp

@@ -22,8 +22,7 @@ TagModel::TagModel(ServiceLocator &p_services, QObject *p_parent)
   }
 }
 
-TagModel::~TagModel() {
-}
+TagModel::~TagModel() {}
 
 QModelIndex TagModel::index(int p_row, int p_column, const QModelIndex &p_parent) const {
   if (m_notebookId.isEmpty() || p_row < 0 || p_column < 0 || p_column > 0) {
@@ -153,9 +152,7 @@ void TagModel::setNotebookId(const QString &p_notebookId) {
   emit notebookChanged();
 }
 
-QString TagModel::getNotebookId() const {
-  return m_notebookId;
-}
+QString TagModel::getNotebookId() const { return m_notebookId; }
 
 void TagModel::reload() {
   beginResetModel();
@@ -206,9 +203,26 @@ void TagModel::reload() {
 }
 
 void TagModel::onTagsChanged(const QString &p_notebookId) {
-  if (p_notebookId == m_notebookId) {
-    reload();
+  if (p_notebookId != m_notebookId) {
+    return;
   }
+
+  // Coalesce: a batch tag edit emits one tagsChanged per file per tag (all
+  // synchronous on the GUI thread), and each reload() is a full
+  // beginResetModel + listTags + cache rebuild. Collapse the whole burst into a
+  // single reload posted to the event loop, which still runs before the next
+  // repaint so the tag tree is never visibly stale.
+  if (m_reloadPending) {
+    return;
+  }
+  m_reloadPending = true;
+  QMetaObject::invokeMethod(
+      this,
+      [this]() {
+        m_reloadPending = false;
+        reload();
+      },
+      Qt::QueuedConnection);
 }
 
 void TagModel::reloadTag(const QString &p_tagName) {
@@ -368,4 +382,3 @@ QString TagModel::fullTagPath(const QString &p_tagName) const {
 
   return parts.join(QLatin1Char('/'));
 }
-