Edit variable button on variable list page (apache#45251)

* initial structure setup

* add editVariable

* initial edit setup

* decouple add variable with form

* edit modal

* fix checks

* rename folder

* bug fix

* reset error

* rename

* rename add

* not edit key

airflow/ui/src/pages/Variables/AddVariableModal.tsx → airflow/ui/src/pages/Variables/ManageVariable/AddVariableButton.tsx

@@ -20,19 +20,35 @@ import { Heading, useDisclosure } from "@chakra-ui/react";
 import { FiPlusCircle } from "react-icons/fi";
 
 import { Button, Dialog, Toaster } from "src/components/ui";
+import { useAddVariable } from "src/queries/useAddVariable";
 
-import AddVariableForm from "./AddVariableForm";
+import VariableForm, { type VariableBody } from "./VariableForm";
 
-const AddVariableModal: React.FC = () => {
+const AddVariableButton = () => {
   const { onClose, onOpen, open } = useDisclosure();
+  const { addVariable, error, isPending, setError } = useAddVariable({
+    onSuccessConfirm: onClose,
+  });
+
+  const initialVariableValue: VariableBody = {
+    description: "",
+    key: "",
+    value: "",
+  };
+
+  const handleClose = () => {
+    setError(undefined);
+    onClose();
+  };
 
   return (
     <>
       <Toaster />
       <Button colorPalette="blue" onClick={onOpen}>
         <FiPlusCircle /> Add Variable
       </Button>
-      <Dialog.Root onOpenChange={onClose} open={open} size="xl">
+
+      <Dialog.Root onOpenChange={handleClose} open={open} size="xl">
         <Dialog.Content backdrop>
           <Dialog.Header>
             <Heading size="xl">Add Variable</Heading>
@@ -41,12 +57,18 @@ const AddVariableModal: React.FC = () => {
           <Dialog.CloseTrigger />
 
           <Dialog.Body>
-            <AddVariableForm onClose={onClose} />
+            <VariableForm
+              error={error}
+              initialVariable={initialVariableValue}
+              isPending={isPending}
+              manageMutate={addVariable}
+              setError={setError}
+            />
           </Dialog.Body>
         </Dialog.Content>
       </Dialog.Root>
     </>
   );
 };
 
-export default AddVariableModal;
+export default AddVariableButton;

airflow/ui/src/pages/Variables/ManageVariable/EditVariableButton.tsx

@@ -0,0 +1,86 @@
+/*!
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { Heading, useDisclosure } from "@chakra-ui/react";
+import { FiEdit } from "react-icons/fi";
+
+import type { VariableResponse } from "openapi/requests/types.gen";
+import { Dialog } from "src/components/ui";
+import ActionButton from "src/components/ui/ActionButton";
+import { useEditVariable } from "src/queries/useEditVariable";
+
+import type { VariableBody } from "./VariableForm";
+import VariableForm from "./VariableForm";
+
+type Props = {
+  readonly variable: VariableResponse;
+};
+
+const EditVariableButton = ({ variable }: Props) => {
+  const { onClose, onOpen, open } = useDisclosure();
+  const initialVariableValue: VariableBody = {
+    description: variable.description ?? "",
+    key: variable.key,
+    value: variable.value ?? "",
+  };
+  const { editVariable, error, isPending, setError } = useEditVariable(
+    initialVariableValue,
+    { onSuccessConfirm: onClose },
+  );
+
+  const handleClose = () => {
+    setError(undefined);
+    onClose();
+  };
+
+  return (
+    <>
+      <ActionButton
+        actionName="Edit Variable"
+        icon={<FiEdit />}
+        onClick={() => {
+          onOpen();
+        }}
+        text="Edit Variable"
+        withText={false}
+      />
+
+      <Dialog.Root onOpenChange={handleClose} open={open} size="xl">
+        <Dialog.Content backdrop>
+          <Dialog.Header>
+            <Heading size="xl">Edit Variable</Heading>
+          </Dialog.Header>
+
+          <Dialog.CloseTrigger />
+
+          <Dialog.Body>
+            <VariableForm
+              error={error}
+              initialVariable={initialVariableValue}
+              isPending={isPending}
+              manageMutate={editVariable}
+              setError={setError}
+            />
+          </Dialog.Body>
+        </Dialog.Content>
+      </Dialog.Root>
+    </>
+  );
+};
+
+export default EditVariableButton;

airflow/ui/src/pages/Variables/AddVariableForm.tsx → airflow/ui/src/pages/Variables/ManageVariable/VariableForm.tsx

@@ -17,51 +17,50 @@
  * under the License.
  */
 import { Box, Field, HStack, Input, Spacer, Textarea } from "@chakra-ui/react";
-import { useEffect } from "react";
 import { Controller, useForm } from "react-hook-form";
 import { FiSave } from "react-icons/fi";
 
 import { ErrorAlert } from "src/components/ErrorAlert";
 import { Button } from "src/components/ui";
-import { useAddVariable } from "src/queries/useAddVariable";
 
-export type AddVariableBody = {
+export type VariableBody = {
   description: string | undefined;
   key: string;
   value: string;
 };
 
-type AddVariableFormProps = {
-  readonly onClose: () => void;
+type VariableFormProps = {
+  readonly error: unknown;
+  readonly initialVariable: VariableBody;
+  readonly isPending: boolean;
+  readonly manageMutate: (variableRequestBody: VariableBody) => void;
+  readonly setError: (error: unknown) => void;
 };
 
-const AddVariableForm = ({ onClose }: AddVariableFormProps) => {
-  const { addVariable, error, isPending } = useAddVariable(onClose);
-
+const VariableForm = ({
+  error,
+  initialVariable,
+  isPending,
+  manageMutate,
+  setError,
+}: VariableFormProps) => {
   const {
     control,
     formState: { isDirty, isValid },
     handleSubmit,
     reset,
-  } = useForm<AddVariableBody>({
-    defaultValues: {
-      description: "",
-      key: "",
-      value: "",
-    },
+  } = useForm<VariableBody>({
+    defaultValues: initialVariable,
     mode: "onChange",
   });
 
-  useEffect(() => {
-    reset({
-      description: "",
-      key: "",
-      value: "",
-    });
-  }, [reset]);
+  const onSubmit = (data: VariableBody) => {
+    manageMutate(data);
+  };
 
-  const onSubmit = (data: AddVariableBody) => {
-    addVariable(data);
+  const handleReset = () => {
+    setError(undefined);
+    reset();
   };
 
   return (
@@ -74,7 +73,12 @@ const AddVariableForm = ({ onClose }: AddVariableFormProps) => {
             <Field.Label fontSize="md">
               Key <Field.RequiredIndicator />
             </Field.Label>
-            <Input {...field} required size="sm" />
+            <Input
+              {...field}
+              disabled={Boolean(initialVariable.key)}
+              required
+              size="sm"
+            />
             {fieldState.error ? (
               <Field.ErrorText>{fieldState.error.message}</Field.ErrorText>
             ) : undefined}
@@ -123,7 +127,7 @@ const AddVariableForm = ({ onClose }: AddVariableFormProps) => {
       <Box as="footer" display="flex" justifyContent="flex-end" mt={8}>
         <HStack w="full">
           {isDirty ? (
-            <Button onClick={() => reset()} variant="outline">
+            <Button onClick={handleReset} variant="outline">
               Reset
             </Button>
           ) : undefined}
@@ -141,4 +145,4 @@ const AddVariableForm = ({ onClose }: AddVariableFormProps) => {
   );
 };
 
-export default AddVariableForm;
+export default VariableForm;

airflow/ui/src/pages/Variables/Variables.tsx

@@ -32,9 +32,9 @@ import {
   type SearchParamsKeysType,
 } from "src/constants/searchParams";
 
-import AddVariableModal from "./AddVariableModal";
-import DeleteVariableButton from "./DeleteVariableButton";
-import EditVariableButton from "./EditVariableButton";
+import AddVariableButton from "./ManageVariable/AddVariableButton";
+import DeleteVariableButton from "./ManageVariable/DeleteVariableButton";
+import EditVariableButton from "./ManageVariable/EditVariableButton";
 
 const columns: Array<ColumnDef<VariableResponse>> = [
   {
@@ -57,7 +57,7 @@ const columns: Array<ColumnDef<VariableResponse>> = [
     accessorKey: "actions",
     cell: ({ row: { original } }) => (
       <Flex justifyContent="end">
-        <EditVariableButton editKey={original.key} />
+        <EditVariableButton variable={original} />
         <DeleteVariableButton deleteKey={original.key} />
       </Flex>
     ),
@@ -115,7 +115,7 @@ export const Variables = () => {
           placeHolder="Search Keys"
         />
         <HStack mt={4}>
-          <AddVariableModal />
+          <AddVariableButton />
         </HStack>
       </VStack>
       <Box>

airflow/ui/src/queries/useAddVariable.ts

@@ -24,9 +24,13 @@ import {
   useVariableServicePostVariable,
 } from "openapi/queries";
 import { toaster } from "src/components/ui";
-import type { AddVariableBody } from "src/pages/Variables/AddVariableForm";
+import type { VariableBody } from "src/pages/Variables/ManageVariable/VariableForm";
 
-export const useAddVariable = (onClose: () => void) => {
+export const useAddVariable = ({
+  onSuccessConfirm,
+}: {
+  onSuccessConfirm: () => void;
+}) => {
   const queryClient = useQueryClient();
   const [error, setError] = useState<unknown>(undefined);
 
@@ -41,7 +45,7 @@ export const useAddVariable = (onClose: () => void) => {
       type: "success",
     });
 
-    onClose();
+    onSuccessConfirm();
   };
 
   const onError = (_error: unknown) => {
@@ -53,20 +57,20 @@ export const useAddVariable = (onClose: () => void) => {
     onSuccess,
   });
 
-  const addVariable = (addVariableRequestBody: AddVariableBody) => {
+  const addVariable = (variableRequestBody: VariableBody) => {
     const parsedDescription =
-      addVariableRequestBody.description === ""
+      variableRequestBody.description === ""
         ? undefined
-        : addVariableRequestBody.description;
+        : variableRequestBody.description;
 
     mutate({
       requestBody: {
         description: parsedDescription,
-        key: addVariableRequestBody.key,
-        value: addVariableRequestBody.value,
+        key: variableRequestBody.key,
+        value: variableRequestBody.value,
       },
     });
   };
 
-  return { addVariable, error, isPending };
+  return { addVariable, error, isPending, setError };
 };