Repository: provectus/kafka-ui
Issue: Implement a view to set KSQL stream properties
Labels: type/enhancement
scope/frontend
status/accepted

string-string key-value instead, format as json

Issue comments:

Pull request review comments:
?
Can we use `useFieldArray`?
Timeout?
does userEvent not work here?
does userEvent not work here?
does userEvent not work here?
seems there is a problem related to fieldset tag, via userevent.paste we are getting error in tests but fireevent.paste is working without issues, if there is no mandatory we can let it be fireevent
fixed using waitFor
returned back validation but fields are not required for now, will add it when everything will work properly
done

Patch that resolved the issue:
diff --git a/kafka-ui-react-app/src/components/KsqlDb/Query/Query.tsx b/kafka-ui-react-app/src/components/KsqlDb/Query/Query.tsx
index 4982807ac24..c5757e2cae6 100644
--- a/kafka-ui-react-app/src/components/KsqlDb/Query/Query.tsx
+++ b/kafka-ui-react-app/src/components/KsqlDb/Query/Query.tsx
@@ -198,15 +198,23 @@ const Query: FC = () => {
 
   const submitHandler = useCallback(
     (values: FormValues) => {
+      const streamsProperties = values.streamsProperties.reduce(
+        (acc, current) => ({
+          ...acc,
+          [current.key as keyof string]: current.value,
+        }),
+        {} as { [key: string]: string }
+      );
       setFetching(true);
       dispatch(
         executeKsql({
           clusterName,
           ksqlCommandV2: {
             ...values,
-            streamsProperties: values.streamsProperties
-              ? JSON.parse(values.streamsProperties)
-              : undefined,
+            streamsProperties:
+              values.streamsProperties[0].key !== ''
+                ? JSON.parse(JSON.stringify(streamsProperties))
+                : undefined,
           },
         })
       );
diff --git a/kafka-ui-react-app/src/components/KsqlDb/Query/QueryForm/QueryForm.styled.ts b/kafka-ui-react-app/src/components/KsqlDb/Query/QueryForm/QueryForm.styled.ts
index 980fa0c2165..202e5862907 100644
--- a/kafka-ui-react-app/src/components/KsqlDb/Query/QueryForm/QueryForm.styled.ts
+++ b/kafka-ui-react-app/src/components/KsqlDb/Query/QueryForm/QueryForm.styled.ts
@@ -27,8 +27,47 @@ export const KSQLButtons = styled.div`
   gap: 16px;
 `;
 
+export const StreamPropertiesContainer = styled.label`
+  display: flex;
+  flex-direction: column;
+  gap: 10px;
+  width: 50%;
+`;
+
+export const InputsContainer = styled.div`
+  display: flex;
+  justify-content: center;
+  gap: 10px;
+`;
+
+export const StreamPropertiesInputWrapper = styled.div`
+  & > input {
+    height: 40px;
+    border: 1px solid grey;
+    border-radius: 4px;
+    min-width: 300px;
+    font-size: 16px;
+    padding-left: 15px;
+  }
+`;
+
+export const DeleteButtonWrapper = styled.div`
+  min-height: 32px;
+  display: flex;
+  flex-direction: column;
+  align-items: center;
+  justify-self: flex-start;
+  margin-top: 10px;
+`;
+
+export const LabelContainer = styled.div`
+  display: flex;
+  align-items: center;
+  justify-content: space-between;
+`;
+
 export const Fieldset = styled.fieldset`
-  width: 100%;
+  width: 50%;
 `;
 
 export const Editor = styled(BaseEditor)(
diff --git a/kafka-ui-react-app/src/components/KsqlDb/Query/QueryForm/QueryForm.tsx b/kafka-ui-react-app/src/components/KsqlDb/Query/QueryForm/QueryForm.tsx
index f824dd256a7..a4d2a275bea 100644
--- a/kafka-ui-react-app/src/components/KsqlDb/Query/QueryForm/QueryForm.tsx
+++ b/kafka-ui-react-app/src/components/KsqlDb/Query/QueryForm/QueryForm.tsx
@@ -1,11 +1,12 @@
 import React from 'react';
 import { FormError } from 'components/common/Input/Input.styled';
 import { ErrorMessage } from '@hookform/error-message';
+import { useForm, Controller, useFieldArray } from 'react-hook-form';
+import { Button } from 'components/common/Button/Button';
+import IconButtonWrapper from 'components/common/Icons/IconButtonWrapper';
+import CloseIcon from 'components/common/Icons/CloseIcon';
 import { yupResolver } from '@hookform/resolvers/yup';
 import yup from 'lib/yupExtended';
-import { useForm, Controller } from 'react-hook-form';
-import { Button } from 'components/common/Button/Button';
-import { SchemaType } from 'generated-sources';
 
 import * as S from './QueryForm.styled';
 
@@ -17,16 +18,22 @@ export interface Props {
   submitHandler: (values: FormValues) => void;
 }
 
+export type StreamsPropertiesType = {
+  key: string;
+  value: string;
+};
 export type FormValues = {
   ksql: string;
-  streamsProperties: string;
+  streamsProperties: StreamsPropertiesType[];
 };
 
+const streamsPropertiesSchema = yup.object().shape({
+  key: yup.string().trim(),
+  value: yup.string().trim(),
+});
 const validationSchema = yup.object({
   ksql: yup.string().trim().required(),
-  streamsProperties: yup.lazy((value) =>
-    value === '' ? yup.string().trim() : yup.string().trim().isJsonObject()
-  ),
+  streamsProperties: yup.array().of(streamsPropertiesSchema),
 });
 
 const QueryForm: React.FC<Props> = ({
@@ -46,9 +53,16 @@ const QueryForm: React.FC<Props> = ({
     resolver: yupResolver(validationSchema),
     defaultValues: {
       ksql: '',
-      streamsProperties: '',
+      streamsProperties: [{ key: '', value: '' }],
     },
   });
+  const { fields, append, remove } = useFieldArray<
+    FormValues,
+    'streamsProperties'
+  >({
+    control,
+    name: 'streamsProperties',
+  });
 
   return (
     <S.QueryWrapper>
@@ -93,48 +107,69 @@ const QueryForm: React.FC<Props> = ({
               <ErrorMessage errors={errors} name="ksql" />
             </FormError>
           </S.Fieldset>
-          <S.Fieldset aria-labelledby="streamsPropertiesLabel">
-            <S.KSQLInputHeader>
-              <label id="streamsPropertiesLabel">
-                Stream properties (JSON format)
-              </label>
-              <Button
-                onClick={() => setValue('streamsProperties', '')}
-                buttonType="primary"
-                buttonSize="S"
-                isInverted
-              >
-                Clear
-              </Button>
-            </S.KSQLInputHeader>
-            <Controller
-              control={control}
-              name="streamsProperties"
-              render={({ field }) => (
-                <S.Editor
-                  {...field}
-                  commands={[
-                    {
-                      // commands is array of key bindings.
-                      // name for the key binding.
-                      name: 'commandName',
-                      // key combination used for the command.
-                      bindKey: { win: 'Ctrl-Enter', mac: 'Command-Enter' },
-                      // function to execute when keys are pressed.
-                      exec: () => {
-                        handleSubmit(submitHandler)();
-                      },
-                    },
-                  ]}
-                  schemaType={SchemaType.JSON}
-                  readOnly={fetching}
-                />
-              )}
-            />
-            <FormError>
-              <ErrorMessage errors={errors} name="streamsProperties" />
-            </FormError>
-          </S.Fieldset>
+
+          <S.StreamPropertiesContainer>
+            Stream properties:
+            {fields.map((item, index) => (
+              <S.InputsContainer key={item.id}>
+                <S.StreamPropertiesInputWrapper>
+                  <Controller
+                    control={control}
+                    name={`streamsProperties.${index}.key`}
+                    render={({ field }) => (
+                      <input
+                        {...field}
+                        placeholder="Key"
+                        aria-label="key"
+                        type="text"
+                      />
+                    )}
+                  />
+                  <FormError>
+                    <ErrorMessage
+                      errors={errors}
+                      name={`streamsProperties.${index}.key`}
+                    />
+                  </FormError>
+                </S.StreamPropertiesInputWrapper>
+                <S.StreamPropertiesInputWrapper>
+                  <Controller
+                    control={control}
+                    name={`streamsProperties.${index}.value`}
+                    render={({ field }) => (
+                      <input
+                        {...field}
+                        placeholder="Value"
+                        aria-label="value"
+                        type="text"
+                      />
+                    )}
+                  />
+                  <FormError>
+                    <ErrorMessage
+                      errors={errors}
+                      name={`streamsProperties.${index}.value`}
+                    />
+                  </FormError>
+                </S.StreamPropertiesInputWrapper>
+
+                <S.DeleteButtonWrapper onClick={() => remove(index)}>
+                  <IconButtonWrapper aria-label="deleteProperty">
+                    <CloseIcon aria-hidden />
+                  </IconButtonWrapper>
+                </S.DeleteButtonWrapper>
+              </S.InputsContainer>
+            ))}
+            <Button
+              type="button"
+              buttonSize="M"
+              buttonType="secondary"
+              onClick={() => append({ key: '', value: '' })}
+            >
+              <i className="fas fa-plus" />
+              Add Stream Property
+            </Button>
+          </S.StreamPropertiesContainer>
         </S.KSQLInputsWrapper>
         <S.KSQLButtons>
           <Button
diff --git a/kafka-ui-react-app/src/components/KsqlDb/Query/QueryForm/__test__/QueryForm.spec.tsx b/kafka-ui-react-app/src/components/KsqlDb/Query/QueryForm/__test__/QueryForm.spec.tsx
index 09f61cccb4a..12a61a5fa5f 100644
--- a/kafka-ui-react-app/src/components/KsqlDb/Query/QueryForm/__test__/QueryForm.spec.tsx
+++ b/kafka-ui-react-app/src/components/KsqlDb/Query/QueryForm/__test__/QueryForm.spec.tsx
@@ -1,7 +1,7 @@
 import { render } from 'lib/testHelpers';
 import React from 'react';
 import QueryForm, { Props } from 'components/KsqlDb/Query/QueryForm/QueryForm';
-import { screen, within } from '@testing-library/dom';
+import { screen, waitFor, within } from '@testing-library/dom';
 import userEvent from '@testing-library/user-event';
 import { act } from '@testing-library/react';
 
@@ -26,20 +26,11 @@ describe('QueryForm', () => {
     // Represents SQL editor
     expect(within(KSQLBlock).getByRole('textbox')).toBeInTheDocument();
 
-    const streamPropertiesBlock = screen.getByLabelText(
-      'Stream properties (JSON format)'
-    );
+    const streamPropertiesBlock = screen.getByRole('textbox', { name: 'key' });
     expect(streamPropertiesBlock).toBeInTheDocument();
-    expect(
-      within(streamPropertiesBlock).getByText('Stream properties (JSON format)')
-    ).toBeInTheDocument();
-    expect(
-      within(streamPropertiesBlock).getByRole('button', { name: 'Clear' })
-    ).toBeInTheDocument();
-    // Represents JSON editor
-    expect(
-      within(streamPropertiesBlock).getByRole('textbox')
-    ).toBeInTheDocument();
+    expect(screen.getByText('Stream properties:')).toBeInTheDocument();
+    expect(screen.getByRole('button', { name: 'Clear' })).toBeInTheDocument();
+    expect(screen.queryAllByRole('textbox')[0]).toBeInTheDocument();
 
     // Form controls
     expect(screen.getByRole('button', { name: 'Execute' })).toBeInTheDocument();
@@ -69,58 +60,10 @@ describe('QueryForm', () => {
     await act(() =>
       userEvent.click(screen.getByRole('button', { name: 'Execute' }))
     );
-    expect(screen.getByText('ksql is a required field')).toBeInTheDocument();
-    expect(submitFn).not.toBeCalled();
-  });
-
-  it('renders error with non-JSON streamProperties', async () => {
-    renderComponent({
-      fetching: false,
-      hasResults: false,
-      handleClearResults: jest.fn(),
-      handleSSECancel: jest.fn(),
-      submitHandler: jest.fn(),
-    });
-
-    await act(() => {
-      // the use of `paste` is a hack that i found somewhere,
-      // `type` won't work
-      userEvent.paste(
-        within(
-          screen.getByLabelText('Stream properties (JSON format)')
-        ).getByRole('textbox'),
-        'not-a-JSON-string'
-      );
-
-      userEvent.click(screen.getByRole('button', { name: 'Execute' }));
+    waitFor(() => {
+      expect(screen.getByText('ksql is a required field')).toBeInTheDocument();
+      expect(submitFn).not.toBeCalled();
     });
-
-    expect(
-      screen.getByText('streamsProperties is not JSON object')
-    ).toBeInTheDocument();
-  });
-
-  it('renders without error with correct JSON', async () => {
-    renderComponent({
-      fetching: false,
-      hasResults: false,
-      handleClearResults: jest.fn(),
-      handleSSECancel: jest.fn(),
-      submitHandler: jest.fn(),
-    });
-
-    await act(() => {
-      userEvent.paste(
-        within(
-          screen.getByLabelText('Stream properties (JSON format)')
-        ).getByRole('textbox'),
-        '{"totallyJSON": "string"}'
-      );
-      userEvent.click(screen.getByRole('button', { name: 'Execute' }));
-    });
-    expect(
-      screen.queryByText('streamsProperties is not JSON object')
-    ).not.toBeInTheDocument();
   });
 
   it('submits with correct inputs', async () => {
@@ -134,18 +77,9 @@ describe('QueryForm', () => {
     });
 
     await act(() => {
-      userEvent.paste(
-        within(screen.getByLabelText('KSQL')).getByRole('textbox'),
-        'show tables;'
-      );
-
-      userEvent.paste(
-        within(
-          screen.getByLabelText('Stream properties (JSON format)')
-        ).getByRole('textbox'),
-        '{"totallyJSON": "string"}'
-      );
-
+      userEvent.paste(screen.getAllByRole('textbox')[0], 'show tables;');
+      userEvent.paste(screen.getByRole('textbox', { name: 'key' }), 'test');
+      userEvent.paste(screen.getByRole('textbox', { name: 'value' }), 'test');
       userEvent.click(screen.getByRole('button', { name: 'Execute' }));
     });
 
@@ -223,41 +157,7 @@ describe('QueryForm', () => {
     expect(submitFn.mock.calls.length).toBe(1);
   });
 
-  it('submits form with ctrl+enter on streamProperties editor', async () => {
-    const submitFn = jest.fn();
-    renderComponent({
-      fetching: false,
-      hasResults: false,
-      handleClearResults: jest.fn(),
-      handleSSECancel: jest.fn(),
-      submitHandler: submitFn,
-    });
-
-    await act(() => {
-      userEvent.paste(
-        within(screen.getByLabelText('KSQL')).getByRole('textbox'),
-        'show tables;'
-      );
-
-      userEvent.paste(
-        within(
-          screen.getByLabelText('Stream properties (JSON format)')
-        ).getByRole('textbox'),
-        '{"some":"json"}'
-      );
-
-      userEvent.type(
-        within(
-          screen.getByLabelText('Stream properties (JSON format)')
-        ).getByRole('textbox'),
-        '{ctrl}{enter}'
-      );
-    });
-
-    expect(submitFn.mock.calls.length).toBe(1);
-  });
-
-  it('clears KSQL with Clear button', async () => {
+  it('add new property', async () => {
     renderComponent({
       fetching: false,
       hasResults: false,
@@ -267,22 +167,15 @@ describe('QueryForm', () => {
     });
 
     await act(() => {
-      userEvent.paste(
-        within(screen.getByLabelText('KSQL')).getByRole('textbox'),
-        'show tables;'
-      );
       userEvent.click(
-        within(screen.getByLabelText('KSQL')).getByRole('button', {
-          name: 'Clear',
-        })
+        screen.getByRole('button', { name: 'Add Stream Property' })
       );
     });
-
-    expect(screen.queryByText('show tables;')).not.toBeInTheDocument();
+    expect(screen.getAllByRole('textbox', { name: 'key' }).length).toEqual(2);
   });
 
-  it('clears streamProperties with Clear button', async () => {
-    renderComponent({
+  it('delete stream property', async () => {
+    await renderComponent({
       fetching: false,
       hasResults: false,
       handleClearResults: jest.fn(),
@@ -291,20 +184,13 @@ describe('QueryForm', () => {
     });
 
     await act(() => {
-      userEvent.paste(
-        within(
-          screen.getByLabelText('Stream properties (JSON format)')
-        ).getByRole('textbox'),
-        '{"some":"json"}'
-      );
       userEvent.click(
-        within(
-          screen.getByLabelText('Stream properties (JSON format)')
-        ).getByRole('button', {
-          name: 'Clear',
-        })
+        screen.getByRole('button', { name: 'Add Stream Property' })
       );
     });
-    expect(screen.queryByText('{"some":"json"}')).not.toBeInTheDocument();
+    await act(() => {
+      userEvent.click(screen.getAllByLabelText('deleteProperty')[0]);
+    });
+    expect(screen.getAllByRole('textbox', { name: 'key' }).length).toEqual(1);
   });
 });
diff --git a/kafka-ui-react-app/src/components/KsqlDb/Query/__test__/Query.spec.tsx b/kafka-ui-react-app/src/components/KsqlDb/Query/__test__/Query.spec.tsx
index fd2cacfd7f1..985ebde5e5b 100644
--- a/kafka-ui-react-app/src/components/KsqlDb/Query/__test__/Query.spec.tsx
+++ b/kafka-ui-react-app/src/components/KsqlDb/Query/__test__/Query.spec.tsx
@@ -3,11 +3,11 @@ import React from 'react';
 import Query, {
   getFormattedErrorFromTableData,
 } from 'components/KsqlDb/Query/Query';
-import { screen, within } from '@testing-library/dom';
+import { screen } from '@testing-library/dom';
 import fetchMock from 'fetch-mock';
-import userEvent from '@testing-library/user-event';
 import { clusterKsqlDbQueryPath } from 'lib/paths';
 import { act } from '@testing-library/react';
+import userEvent from '@testing-library/user-event';
 
 const clusterName = 'testLocal';
 const renderComponent = () =>
@@ -25,9 +25,7 @@ describe('Query', () => {
     renderComponent();
 
     expect(screen.getByLabelText('KSQL')).toBeInTheDocument();
-    expect(
-      screen.getByLabelText('Stream properties (JSON format)')
-    ).toBeInTheDocument();
+    expect(screen.getByLabelText('Stream properties:')).toBeInTheDocument();
   });
 
   afterEach(() => fetchMock.reset());
@@ -41,12 +39,10 @@ describe('Query', () => {
     Object.defineProperty(window, 'EventSource', {
       value: EventSourceMock,
     });
-
+    const inputs = screen.getAllByRole('textbox');
+    const textAreaElement = inputs[0] as HTMLTextAreaElement;
     await act(() => {
-      userEvent.paste(
-        within(screen.getByLabelText('KSQL')).getByRole('textbox'),
-        'show tables;'
-      );
+      userEvent.paste(textAreaElement, 'show tables;');
       userEvent.click(screen.getByRole('button', { name: 'Execute' }));
     });
 
@@ -63,47 +59,19 @@ describe('Query', () => {
     Object.defineProperty(window, 'EventSource', {
       value: EventSourceMock,
     });
-
     await act(() => {
-      userEvent.paste(
-        within(screen.getByLabelText('KSQL')).getByRole('textbox'),
-        'show tables;'
-      );
-      userEvent.paste(
-        within(
-          screen.getByLabelText('Stream properties (JSON format)')
-        ).getByRole('textbox'),
-        '{"some":"json"}'
-      );
-      userEvent.click(screen.getByRole('button', { name: 'Execute' }));
+      const inputs = screen.getAllByRole('textbox');
+      const textAreaElement = inputs[0] as HTMLTextAreaElement;
+      userEvent.paste(textAreaElement, 'show tables;');
     });
-    expect(mock.calls().length).toBe(1);
-  });
-
-  it('fetch on execute with streamParams', async () => {
-    renderComponent();
-
-    const mock = fetchMock.postOnce(`/api/clusters/${clusterName}/ksql/v2`, {
-      pipeId: 'testPipeID',
-    });
-
-    Object.defineProperty(window, 'EventSource', {
-      value: EventSourceMock,
+    await act(() => {
+      userEvent.paste(screen.getByLabelText('key'), 'key');
+      userEvent.paste(screen.getByLabelText('value'), 'value');
     });
-
     await act(() => {
-      userEvent.paste(
-        within(screen.getByLabelText('KSQL')).getByRole('textbox'),
-        'show tables;'
-      );
-      userEvent.paste(
-        within(
-          screen.getByLabelText('Stream properties (JSON format)')
-        ).getByRole('textbox'),
-        '{"some":"json"}'
-      );
       userEvent.click(screen.getByRole('button', { name: 'Execute' }));
     });
+
     expect(mock.calls().length).toBe(1);
   });
 });


Test patch: