fix: a workflow update now means what it says when a field is left out

PUT /workflows/{id} wrote a patch, and a patch is a DEEP merge server-side. So
a field removed from a nested object survived: clearing settings.certificateIds
by sending settings without it left the old value in place, and there was no
way to remove anything from settings at all.

It now builds the record from the stored one, overlays what the request sent,
and writes it whole. Starting from the stored record is what keeps that safe -
a plain replace with the request body would drop every field the client did not
send, ownerId and the failure counters among them, which is why a patch was
used in the first place. Overlaying gives both: a top-level field the body
omits keeps its stored value, and a nested object the body does send replaces
the stored one entirely.

Checked across everything that could regress, since this is the same call the
editor makes on every autosave: a key removed from settings is gone; a body
with no settings leaves them alone; ownerId, createdBy and nodes survive a
partial body; active is not switched off by a save that omits it; deleting a
node and a connection works, so arrays replace rather than merge; and a save
that changes nothing still writes nothing, which autosave relies on to keep the
version history free of entries nobody made.

Node suite: 92 passed, 0 failed.

src/webserver/api/workflow_controller.cpp

@@ -516,12 +516,13 @@ void WorkflowController::updateWorkflow(const httplib::Request& req, httplib::Re
         // behind by and gets the current record back, so it can show what
         // changed instead of just losing.
         //
-        // This is a read then a write, not one atomic compare-and-set: the
-        // database can do that, but only for a whole-record replace, and this
-        // endpoint writes a patch - a replace here would drop every field the
-        // client did not send, ownerId among them. The window is the few
-        // milliseconds between the two calls, and what it protects against is
-        // two people editing for minutes, so it catches what it is for.
+        // This is a read then a write, not one atomic compare-and-set. The
+        // database offers one for a whole-record replace, which is what this
+        // endpoint now writes - but the record it replaces with is built from
+        // the read below, so the two are already tied together by that read.
+        // The window is the few milliseconds between them, and what this
+        // protects against is two people editing for minutes, so it catches
+        // what it is for.
         if (expected_version >= 0 && before.ok()) {
             const int64_t current_version = before.value().value("_version", int64_t{0});
             if (current_version != expected_version) {
@@ -578,7 +579,32 @@ void WorkflowController::updateWorkflow(const httplib::Request& req, httplib::Re
             }
         }
 
-        auto result = storage_.update("workflows", id, body, 0, true);
+        // Built from the stored record, then overlaid with what was sent, and
+        // written as a whole record rather than as a patch.
+        //
+        // A patch is a DEEP merge server-side, so a field removed from a nested
+        // object survived: clearing settings.certificateIds by sending settings
+        // without it left the old value in place, and the caller had no way to
+        // remove anything at all. Sending the whole record makes an omitted key
+        // mean what it says.
+        //
+        // Starting from the stored record is what keeps that safe. A plain
+        // replace with the request body would drop every field the client did
+        // not send - ownerId, createdBy, the failure counters - which is the
+        // reason a patch was used in the first place. Overlaying gives both:
+        // top-level fields the body omits are kept, and a nested object the
+        // body does send replaces the stored one entirely.
+        nlohmann::json document = before.ok() ? before.value() : nlohmann::json::object();
+        for (const auto& [key, value] : body.items()) {
+            document[key] = value;
+        }
+        // Metadata belongs to the database. Writing _id or _version back is at
+        // best ignored and at worst confuses the version the record is on.
+        for (auto it = document.begin(); it != document.end();) {
+            it = (!it.key().empty() && it.key().front() == '_') ? document.erase(it) : std::next(it);
+        }
+
+        auto result = storage_.update("workflows", id, document, 0, false);
         if (result.failed()) {
             sendError(res, result.error().message(), 404);
             return;