From c477992467f80bd7fd81119fad9a44be2c1712a0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= <sybren@blender.org>
Date: Tue, 15 Aug 2023 10:29:44 +0200
Subject: [PATCH] Manager: tag update without description now keeps the
 description

Updating a tag without `description` field in the request body will keep
the tag's description as-is. Previously this caused it to become empty,
which is now still possible by using an explicit `description: ""`.
---
 internal/manager/api_impl/worker_mgt.go      |  5 ++-
 internal/manager/api_impl/worker_mgt_test.go | 36 ++++++++++++++++++++
 2 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/internal/manager/api_impl/worker_mgt.go b/internal/manager/api_impl/worker_mgt.go
index 8a2307fe..7dfa6ce2 100644
--- a/internal/manager/api_impl/worker_mgt.go
+++ b/internal/manager/api_impl/worker_mgt.go
@@ -307,11 +307,10 @@ func (f *Flamenco) UpdateWorkerTag(e echo.Context, tagUUID string) error {
 		return sendAPIError(e, http.StatusInternalServerError, "error fetching worker tag: %v", err)
 	}
 
+
 	// Update the tag.
 	dbTag.Name = update.Name
-	if update.Description == nil {
-		dbTag.Description = ""
-	} else {
+	if update.Description != nil && dbTag.Description != *update.Description {
 		dbTag.Description = *update.Description
 	}
 
diff --git a/internal/manager/api_impl/worker_mgt_test.go b/internal/manager/api_impl/worker_mgt_test.go
index 11b3e5a8..ec4b6011 100644
--- a/internal/manager/api_impl/worker_mgt_test.go
+++ b/internal/manager/api_impl/worker_mgt_test.go
@@ -299,6 +299,23 @@ func TestWorkerTagCRUDHappyFlow(t *testing.T) {
 		Name: "updated name",
 	}
 	expectNewDBTag := persistence.WorkerTag{
+		UUID:        UUID,
+		Name:        newAPITag.Name,
+		Description: *apiTag.Description, // Not mentioning new description should keep old one.
+	}
+	// TODO: expect SocketIO broadcast of the tag update.
+	mf.persistence.EXPECT().FetchWorkerTag(gomock.Any(), UUID).Return(&expectDBTag, nil)
+	mf.persistence.EXPECT().SaveWorkerTag(gomock.Any(), &expectNewDBTag)
+	echo = mf.prepareMockedJSONRequest(newAPITag)
+	require.NoError(t, mf.flamenco.UpdateWorkerTag(echo, UUID))
+	assertResponseNoContent(t, echo)
+
+	// Update both description + name & save.
+	newAPITag = api.WorkerTag{
+		Name:        "updated name",
+		Description: ptr(""),
+	}
+	expectNewDBTag = persistence.WorkerTag{
 		UUID:        UUID,
 		Name:        newAPITag.Name,
 		Description: "",
@@ -310,6 +327,23 @@ func TestWorkerTagCRUDHappyFlow(t *testing.T) {
 	require.NoError(t, mf.flamenco.UpdateWorkerTag(echo, UUID))
 	assertResponseNoContent(t, echo)
 
+	// Update both description + name & save.
+	newAPITag = api.WorkerTag{
+		Name:        "updated name",
+		Description: ptr("New Description"),
+	}
+	expectNewDBTag = persistence.WorkerTag{
+		UUID:        UUID,
+		Name:        newAPITag.Name,
+		Description: *newAPITag.Description,
+	}
+	// TODO: expect SocketIO broadcast of the tag update.
+	mf.persistence.EXPECT().FetchWorkerTag(gomock.Any(), UUID).Return(&expectDBTag, nil)
+	mf.persistence.EXPECT().SaveWorkerTag(gomock.Any(), &expectNewDBTag)
+	echo = mf.prepareMockedJSONRequest(newAPITag)
+	require.NoError(t, mf.flamenco.UpdateWorkerTag(echo, UUID))
+	assertResponseNoContent(t, echo)
+
 	// Delete.
 	mf.persistence.EXPECT().DeleteWorkerTag(gomock.Any(), UUID)
 	// TODO: expect SocketIO broadcast of the tag deletion.
@@ -317,3 +351,5 @@ func TestWorkerTagCRUDHappyFlow(t *testing.T) {
 	require.NoError(t, mf.flamenco.DeleteWorkerTag(echo, UUID))
 	assertResponseNoContent(t, echo)
 }
+
+// TODO: add test for creation of already-existing tag.