From 14fd624faa8f0c61a446d5459478eb19b737a7b5 Mon Sep 17 00:00:00 2001
From: Thomas Heartman <thomas@getunleash.io>
Date: Wed, 22 May 2024 08:44:39 +0200
Subject: [PATCH] fix: don't send change request info unless using the new form
 (#7102)

I realized that, in an oversight, the form now shows and sends project
CR config, even if the new form isn't active. The API just ignores it if
it doesn't understand it, so it's not very harmful, but it's better if
we don't send it at all. This PR does that.

It does not actually test that change request info isn't included (but
it does test ID inclusion). This is because:
- change request info is only included if we're enterprise. The rendered
version of the hook isn't by default.
- Setting up module mocking and making it work seems like a lot of work
for a small gain, considering we're probably going to be removing the
old form anyway.
- I've tested it locally.

Also adds some testing for the hook related to name validation and
payload creation
---
 .../CreateProjectDialog.tsx                   | 12 +++++----
 .../Project/hooks/useProjectForm.test.ts      | 26 +++++++++++++++++++
 .../project/Project/hooks/useProjectForm.ts   |  9 +++++--
 3 files changed, 40 insertions(+), 7 deletions(-)

diff --git a/frontend/src/component/project/Project/CreateProject/CreateProjectDialog/CreateProjectDialog.tsx b/frontend/src/component/project/Project/CreateProject/CreateProjectDialog/CreateProjectDialog.tsx
index b2a9d91117..89078a1678 100644
--- a/frontend/src/component/project/Project/CreateProject/CreateProjectDialog/CreateProjectDialog.tsx
+++ b/frontend/src/component/project/Project/CreateProject/CreateProjectDialog/CreateProjectDialog.tsx
@@ -75,11 +75,16 @@ export const CreateProjectDialogue = ({
     const clearDocumentationOverride = () =>
         setDocumentation(generalDocumentation);
 
+    const projectPayload = getCreateProjectPayload({
+        omitId: true,
+        includeChangeRequestConfig: true,
+    });
+
     const formatApiCode = () => {
         return `curl --location --request POST '${uiConfig.unleashUrl}/api/admin/projects' \\
 --header 'Authorization: INSERT_API_KEY' \\
 --header 'Content-Type: application/json' \\
---data-raw '${JSON.stringify(getCreateProjectPayload(), undefined, 2)}'`;
+--data-raw '${JSON.stringify(projectPayload, undefined, 2)}'`;
     };
 
     const handleSubmit = async (e: Event) => {
@@ -88,11 +93,8 @@ export const CreateProjectDialogue = ({
         const validName = validateName();
 
         if (validName) {
-            const payload = getCreateProjectPayload({
-                omitId: true,
-            });
             try {
-                const createdProject = await createProject(payload);
+                const createdProject = await createProject(projectPayload);
                 refetchUser();
                 navigate(`/projects/${createdProject.id}`, { replace: true });
                 setToastData({
diff --git a/frontend/src/component/project/Project/hooks/useProjectForm.test.ts b/frontend/src/component/project/Project/hooks/useProjectForm.test.ts
index 88ec54b706..fd0a0cf1b8 100644
--- a/frontend/src/component/project/Project/hooks/useProjectForm.test.ts
+++ b/frontend/src/component/project/Project/hooks/useProjectForm.test.ts
@@ -35,3 +35,29 @@ test(`adding a change request config for an env not in the project envs doesn't
         'dev' in result.current.projectChangeRequestConfiguration,
     ).toBeFalsy();
 });
+
+describe('payload generation', () => {
+    test(`id is omitted only when explicitly asked to be`, () => {
+        const { result } = renderHook(() => useProjectForm());
+
+        const payloadWithId = result.current.getCreateProjectPayload();
+        expect('id' in payloadWithId).toBeTruthy();
+
+        const payloadWithoutId = result.current.getCreateProjectPayload({
+            omitId: true,
+        });
+        expect('id' in payloadWithoutId).toBeFalsy();
+    });
+});
+
+describe('name validation', () => {
+    test.each([
+        ['An empty string', ''],
+        ['Just whitespace', '     '],
+    ])(`%s is not valid`, (_, value) => {
+        const { result } = renderHook(() => useProjectForm());
+
+        result.current.setProjectName(value);
+        expect(result.current.validateName()).toBeFalsy();
+    });
+});
diff --git a/frontend/src/component/project/Project/hooks/useProjectForm.ts b/frontend/src/component/project/Project/hooks/useProjectForm.ts
index be42a7549e..03943bf601 100644
--- a/frontend/src/component/project/Project/hooks/useProjectForm.ts
+++ b/frontend/src/component/project/Project/hooks/useProjectForm.ts
@@ -94,7 +94,10 @@ const useProjectForm = (
         setProjectMode(initialProjectMode);
     }, [initialProjectMode]);
 
-    const getCreateProjectPayload = (options?: { omitId?: boolean }) => {
+    const getCreateProjectPayload = (options?: {
+        omitId?: boolean;
+        includeChangeRequestConfig?: boolean;
+    }) => {
         const environmentsPayload =
             projectEnvironments.size > 0
                 ? { environments: [...projectEnvironments] }
@@ -119,7 +122,9 @@ const useProjectForm = (
             ? {
                   ...ossPayload,
                   mode: projectMode,
-                  changeRequestEnvironments,
+                  ...(options?.includeChangeRequestConfig
+                      ? { changeRequestEnvironments }
+                      : {}),
               }
             : ossPayload;
     };