fix(site): show error on duplicate template rename attempt (#15348)

ethanndickson · web-flow · commit 3c60dc3bb518 · 2024-11-06T01:18:38.000+11:00
Fixes #15311.
diff --git a/coderd/templates.go b/coderd/templates.go
@@ -841,7 +841,17 @@ func (api *API) patchTemplateMeta(rw http.ResponseWriter, r *http.Request) {
 		return nil
 	}, nil)
 	if err != nil {
-		httpapi.InternalServerError(rw, err)
+		if database.IsUniqueViolation(err, database.UniqueTemplatesOrganizationIDNameIndex) {
+			httpapi.Write(ctx, rw, http.StatusConflict, codersdk.Response{
+				Message: fmt.Sprintf("Template with name %q already exists.", req.Name),
+				Validations: []codersdk.ValidationError{{
+					Field:  "name",
+					Detail: "This value is already in use and should be unique.",
+				}},
+			})
+		} else {
+			httpapi.InternalServerError(rw, err)
+		}
 		return
 	}
 
diff --git a/coderd/templates_test.go b/coderd/templates_test.go
@@ -18,6 +18,7 @@ import (
 	"github.com/coder/coder/v2/coderd/coderdtest"
 	"github.com/coder/coder/v2/coderd/database"
 	"github.com/coder/coder/v2/coderd/database/dbauthz"
+	"github.com/coder/coder/v2/coderd/database/dbtestutil"
 	"github.com/coder/coder/v2/coderd/database/dbtime"
 	"github.com/coder/coder/v2/coderd/notifications"
 	"github.com/coder/coder/v2/coderd/rbac"
@@ -612,6 +613,32 @@ func TestPatchTemplateMeta(t *testing.T) {
 		assert.Equal(t, database.AuditActionWrite, auditor.AuditLogs()[4].Action)
 	})
 
+	t.Run("AlreadyExists", func(t *testing.T) {
+		t.Parallel()
+
+		if !dbtestutil.WillUsePostgres() {
+			t.Skip("This test requires Postgres constraints")
+		}
+
+		ownerClient := coderdtest.New(t, nil)
+		owner := coderdtest.CreateFirstUser(t, ownerClient)
+		client, _ := coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID, rbac.ScopedRoleOrgTemplateAdmin(owner.OrganizationID))
+
+		version := coderdtest.CreateTemplateVersion(t, client, owner.OrganizationID, nil)
+		version2 := coderdtest.CreateTemplateVersion(t, client, owner.OrganizationID, nil)
+		template := coderdtest.CreateTemplate(t, client, owner.OrganizationID, version.ID)
+		template2 := coderdtest.CreateTemplate(t, client, owner.OrganizationID, version2.ID)
+
+		ctx := testutil.Context(t, testutil.WaitLong)
+
+		_, err := client.UpdateTemplateMeta(ctx, template.ID, codersdk.UpdateTemplateMeta{
+			Name: template2.Name,
+		})
+		var apiErr *codersdk.Error
+		require.ErrorAs(t, err, &apiErr)
+		require.Equal(t, http.StatusConflict, apiErr.StatusCode())
+	})
+
 	t.Run("AGPL_Deprecated", func(t *testing.T) {
 		t.Parallel()
 
diff --git a/site/src/pages/TemplateSettingsPage/TemplateGeneralSettingsPage/TemplateSettingsPage.test.tsx b/site/src/pages/TemplateSettingsPage/TemplateGeneralSettingsPage/TemplateSettingsPage.test.tsx
@@ -4,7 +4,11 @@ import { API, withDefaultFeatures } from "api/api";
 import type { Template, UpdateTemplateMeta } from "api/typesGenerated";
 import { Language as FooterFormLanguage } from "components/FormFooter/FormFooter";
 import { http, HttpResponse } from "msw";
-import { MockEntitlements, MockTemplate } from "testHelpers/entities";
+import {
+	MockEntitlements,
+	MockTemplate,
+	mockApiError,
+} from "testHelpers/entities";
 import {
 	renderWithTemplateSettingsLayout,
 	waitForLoaderToBeRemoved,
@@ -112,6 +116,28 @@ describe("TemplateSettingsPage", () => {
 		await waitFor(() => expect(API.updateTemplateMeta).toBeCalledTimes(1));
 	});
 
+	it("displays an error if the name is taken", async () => {
+		await renderTemplateSettingsPage();
+		jest.spyOn(API, "updateTemplateMeta").mockRejectedValueOnce(
+			mockApiError({
+				message: `Template with name "test-template" already exists`,
+				validations: [
+					{
+						field: "name",
+						detail: "This value is already in use and should be unique.",
+					},
+				],
+			}),
+		);
+		await fillAndSubmitForm(validFormValues);
+		await waitFor(() => expect(API.updateTemplateMeta).toBeCalledTimes(1));
+		expect(
+			await screen.findByText(
+				"This value is already in use and should be unique.",
+			),
+		).toBeInTheDocument();
+	});
+
 	it("allows a description of 128 chars", () => {
 		const values: UpdateTemplateMeta = {
 			...validFormValues,
diff --git a/site/src/pages/TemplateSettingsPage/TemplateGeneralSettingsPage/TemplateSettingsPage.tsx b/site/src/pages/TemplateSettingsPage/TemplateGeneralSettingsPage/TemplateSettingsPage.tsx
@@ -1,7 +1,8 @@
 import { API } from "api/api";
+import { getErrorMessage } from "api/errors";
 import { templateByNameKey } from "api/queries/templates";
 import type { UpdateTemplateMeta } from "api/typesGenerated";
-import { displaySuccess } from "components/GlobalSnackbar/utils";
+import { displayError, displaySuccess } from "components/GlobalSnackbar/utils";
 import { useDashboard } from "modules/dashboard/useDashboard";
 import { linkToTemplate, useLinks } from "modules/navigation";
 import type { FC } from "react";
@@ -51,6 +52,9 @@ export const TemplateSettingsPage: FC = () => {
 				displaySuccess("Template updated successfully");
 				navigate(getLink(linkToTemplate(data.organization_name, data.name)));
 			},
+			onError: (error) => {
+				displayError(getErrorMessage(error, "Failed to update template"));
+			},
 		},
 	);
 
