[GEN-1806]: fix "test-connection" (button style, notification notes, …

…and dirty form) (#1883) This pull request focuses on enhancing the form validation and connection testing functionalities in the `DestinationFormBody` component. The main changes include adding a form validation function, improving the connection status handling, and cleaning up unused imports. ### Form Validation and Connection Testing Improvements: * [`frontend/webapp/containers/main/destinations/destination-drawer/index.tsx`](diffhunk://#diff-392a25e5740454eca88d8df2f33e965ed7abe29b0312e9ce5f82b8a8ae036badL124-R126): Added the `validateForm` prop to the `DestinationFormBody` component to enable form validation before testing the connection. * `frontend/webapp/containers/main/destinations/destination-form-body/index.tsx`: - Introduced the `validateForm` prop in the `Props` interface and updated the component to use this function. - Created a `dirtyForm` function to manage form dirty state and connection status. [[1]](diffhunk://#diff-dfeb6d04b2e764197b17375e8347405ed7c67a7d706a34b6a2eaf09582e3b52cL26-R42) [[2]](diffhunk://#diff-dfeb6d04b2e764197b17375e8347405ed7c67a7d706a34b6a2eaf09582e3b52cL61-R67) [[3]](diffhunk://#diff-dfeb6d04b2e764197b17375e8347405ed7c67a7d706a34b6a2eaf09582e3b52cL118-R129) [[4]](diffhunk://#diff-dfeb6d04b2e764197b17375e8347405ed7c67a7d706a34b6a2eaf09582e3b52cL128-R139) - Replaced the `showConnectionError` state with `connectionStatus` to handle connection success and error states more effectively. * `frontend/webapp/containers/main/destinations/destination-form-body/test-connection/index.tsx`: - Updated the `TestConnection` component to use the `validateForm` function before initiating the connection test. - Refactored the `ActionButton` styles to reflect different connection statuses. ### Code Cleanup: * [`frontend/webapp/containers/main/destinations/destination-form-body/index.tsx`](diffhunk://#diff-dfeb6d04b2e764197b17375e8347405ed7c67a7d706a34b6a2eaf09582e3b52cL1-R1): Removed unused `useEffect` import. * [`frontend/webapp/containers/main/destinations/destination-modal/index.tsx`](diffhunk://#diff-abafe53761371d28f6f0a3ad30eaa63cf5232f7fd16ac6a6bf3c266e77439c99L137-R139): Added the `validateForm` prop to the `DestinationFormBody` component.
odigos-io · Nov 28, 2024 · a2a81e3 · a2a81e3
1 parent 92ea40f
commit a2a81e3
Show file tree

Hide file tree

Showing 4 changed files with 76 additions and 49 deletions.
diff --git a/frontend/webapp/containers/main/destinations/destination-drawer/index.tsx b/frontend/webapp/containers/main/destinations/destination-drawer/index.tsx
@@ -121,8 +121,9 @@ export const DestinationDrawer: React.FC<Props> = () => {
           <DestinationFormBody
             isUpdate
             destination={thisDestination}
-            formErrors={formErrors}
             formData={formData}
+            formErrors={formErrors}
+            validateForm={validateForm}
             handleFormChange={(...params) => {
               setIsFormDirty(true);
               handleFormChange(...params);

diff --git a/frontend/webapp/containers/main/destinations/destination-form-body/index.tsx b/frontend/webapp/containers/main/destinations/destination-form-body/index.tsx
@@ -1,4 +1,4 @@
-import React, { Dispatch, SetStateAction, useEffect, useMemo, useState } from 'react';
+import React, { Dispatch, SetStateAction, useMemo, useState } from 'react';
 import styled from 'styled-components';
 import { SignalUppercase } from '@/utils';
 import { TestConnection } from './test-connection';
@@ -11,6 +11,7 @@ interface Props {
   destination?: DestinationTypeItem;
   formData: DestinationInput;
   formErrors: Record<string, string>;
+  validateForm: () => boolean;
   handleFormChange: (key: keyof DestinationInput | string, val: any) => void;
   dynamicFields: DynamicField[];
   setDynamicFields: Dispatch<SetStateAction<DynamicField[]>>;
@@ -23,17 +24,22 @@ const Container = styled.div`
   padding: 0 4px;
 `;
 
-export function DestinationFormBody({ isUpdate, destination, formData, formErrors, handleFormChange, dynamicFields, setDynamicFields }: Props) {
+const NotesWrapper = styled.div`
+  display: flex;
+  flex-direction: column;
+  gap: 12px;
+`;
+
+export function DestinationFormBody({ isUpdate, destination, formData, formErrors, validateForm, handleFormChange, dynamicFields, setDynamicFields }: Props) {
   const { supportedSignals, testConnectionSupported, displayName } = destination || {};
-  const isFormOk = useMemo(() => !Object.keys(formErrors).length, [formErrors]);
 
   const [isFormDirty, setIsFormDirty] = useState(false);
-  const [showConnectionError, setShowConnectionError] = useState(false);
+  const [connectionStatus, setConnectionStatus] = useState<'success' | 'error'>();
 
-  // this is to allow test connection when there are default values loaded
-  useEffect(() => {
-    if (isFormOk) setIsFormDirty(true);
-  }, [isFormOk]);
+  const dirtyForm = () => {
+    setIsFormDirty(true);
+    setConnectionStatus(undefined);
+  };
 
   const supportedMonitors = useMemo(() => {
     const { logs, metrics, traces } = supportedSignals || {};
@@ -58,7 +64,7 @@ export function DestinationFormBody({ isUpdate, destination, formData, formError
   }, [formData['exportedSignals']]);
 
   const handleSelectedSignals = (signals: SignalUppercase[]) => {
-    setIsFormDirty(true);
+    dirtyForm();
     handleFormChange('exportedSignals', {
       logs: signals.includes('LOGS'),
       metrics: signals.includes('METRICS'),
@@ -72,30 +78,35 @@ export function DestinationFormBody({ isUpdate, destination, formData, formError
         <>
           <SectionTitle
             title='Create connection'
-            description={`Connect ${displayName} destination with Odigos.`}
+            description={`Connect ${displayName} with Odigos.`}
             actionButton={
               testConnectionSupported && (
                 <TestConnection
                   destination={formData}
-                  disabled={!isFormOk || !isFormDirty}
-                  clearStatus={() => {
+                  disabled={!isFormDirty}
+                  status={connectionStatus}
+                  onError={() => {
                     setIsFormDirty(false);
-                    setShowConnectionError(false);
+                    setConnectionStatus('error');
                   }}
-                  onError={() => {
+                  onSuccess={() => {
                     setIsFormDirty(false);
-                    setShowConnectionError(true);
+                    setConnectionStatus('success');
                   }}
+                  validateForm={validateForm}
                 />
               )
             }
           />
 
-          {testConnectionSupported && showConnectionError ? (
-            <NotificationNote type='error' message='Connection failed. Please check your input and try again.' />
-          ) : testConnectionSupported && !showConnectionError && !!displayName ? (
-            <NotificationNote type='default' message={`Odigos autocompleted ${displayName} connection details.`} />
-          ) : null}
+          {testConnectionSupported && (
+            <NotesWrapper>
+              {connectionStatus === 'error' && <NotificationNote type='error' message='Connection failed. Please check your input and try again.' />}
+              {connectionStatus === 'success' && <NotificationNote type='success' message='Connection succeeded.' />}
+              {!connectionStatus && <NotificationNote type='default' message={`Odigos autocompleted ${displayName} connection details.`} />}
+            </NotesWrapper>
+          )}
+
           <Divider />
         </>
       )}
@@ -115,7 +126,7 @@ export function DestinationFormBody({ isUpdate, destination, formData, formError
           placeholder='Enter destination name'
           value={formData['name']}
           onChange={(e) => {
-            setIsFormDirty(true);
+            dirtyForm();
             handleFormChange('name', e.target.value);
           }}
           errorMessage={formErrors['name']}
@@ -125,7 +136,7 @@ export function DestinationFormBody({ isUpdate, destination, formData, formError
       <DestinationDynamicFields
         fields={dynamicFields}
         onChange={(name: string, value: any) => {
-          setIsFormDirty(true);
+          dirtyForm();
           setDynamicFields((prev) => {
             const payload = [...prev];
             const foundIndex = payload.findIndex((field) => field.name === name);

diff --git a/frontend/webapp/containers/main/destinations/destination-form-body/test-connection/index.tsx b/frontend/webapp/containers/main/destinations/destination-form-body/test-connection/index.tsx
@@ -1,53 +1,67 @@
-import React, { useEffect, useMemo } from 'react';
+import React, { useEffect } from 'react';
 import Image from 'next/image';
-import styled from 'styled-components';
+import theme from '@/styles/theme';
 import { getStatusIcon } from '@/utils';
 import { useTestConnection } from '@/hooks';
 import type { DestinationInput } from '@/types';
+import styled, { css } from 'styled-components';
 import { Button, FadeLoader, Text } from '@/reuseable-components';
 
-interface TestConnectionProps {
+type Status = 'success' | 'error';
+
+interface Props {
   destination: DestinationInput;
   disabled: boolean;
-  clearStatus: () => void;
+  status?: Status;
   onError: () => void;
+  onSuccess: () => void;
+  validateForm: () => boolean;
 }
 
-const ActionButton = styled(Button)<{ $success?: boolean }>`
+const ActionButton = styled(Button)<{ $status?: Status }>`
   display: flex;
   align-items: center;
   gap: 8px;
-  background-color: ${({ $success }) => ($success ? 'rgba(129, 175, 101, 0.16)' : 'transparent')};
-`;
 
-const ActionButtonText = styled(Text)<{ $success?: boolean }>`
-  font-family: ${({ theme }) => theme.font_family.secondary};
-  font-weight: 500;
-  text-decoration: underline;
-  text-transform: uppercase;
-  font-size: 14px;
-  line-height: 157.143%;
-  color: ${({ theme, $success }) => ($success ? theme.text.success : theme.colors.white)};
+  ${({ $status, theme }) =>
+    $status === 'success'
+      ? css`
+          border-color: transparent;
+          background-color: ${theme.colors.success};
+        `
+      : $status === 'error'
+      ? css`
+          border-color: transparent;
+          background-color: ${theme.colors.error};
+        `
+      : css`
+          background-color: transparent;
+        `}
 `;
 
-export const TestConnection: React.FC<TestConnectionProps> = ({ destination, disabled, clearStatus, onError }) => {
+export const TestConnection: React.FC<Props> = ({ destination, disabled, status, onError, onSuccess, validateForm }) => {
   const { testConnection, loading, data } = useTestConnection();
-  const success = useMemo(() => data?.testConnectionForDestination.succeeded || false, [data]);
 
   useEffect(() => {
     if (data) {
-      clearStatus();
-      if (!success) onError && onError();
+      const { succeeded } = data.testConnectionForDestination;
+
+      if (succeeded) onSuccess();
+      else onError();
     }
-  }, [data, success]);
+  }, [data]);
+
+  const onClick = async () => {
+    if (validateForm()) await testConnection(destination);
+  };
 
   return (
-    <ActionButton variant='secondary' disabled={disabled} onClick={() => testConnection(destination)} $success={success}>
-      {loading ? <FadeLoader /> : success ? <Image alt='checkmark' src={getStatusIcon('success')} width={16} height={16} /> : null}
+    <ActionButton $status={status} variant='secondary' disabled={disabled} onClick={onClick}>
+      {loading ? <FadeLoader /> : !!status ? <Image src={getStatusIcon(status)} alt='status' width={16} height={16} /> : null}
 
-      <ActionButtonText size={14} $success={success}>
-        {loading ? 'Checking' : success ? 'Connection OK' : 'Test Connection'}
-      </ActionButtonText>
+      <Text family='secondary' decoration='underline' size={14} color={!!status ? theme.text[status] : undefined}>
+        {loading ? 'Checking' : status === 'success' ? 'Connection OK' : status === 'error' ? 'Connection Failed' : 'Test Connection'}
+      </Text>
     </ActionButton>
   );
 };
diff --git a/frontend/webapp/containers/main/destinations/destination-modal/index.tsx b/frontend/webapp/containers/main/destinations/destination-modal/index.tsx
@@ -134,8 +134,9 @@ export const DestinationModal: React.FC<AddDestinationModalProps> = ({ isOnboard
           {!!selectedItem && (
             <DestinationFormBody
               destination={selectedItem}
-              formErrors={formErrors}
               formData={formData}
+              formErrors={formErrors}
+              validateForm={validateForm}
               handleFormChange={handleFormChange}
               dynamicFields={dynamicFields}
               setDynamicFields={setDynamicFields}