fix(APP-989): Allow empty strings in action ABI string inputs (#719)

Author: harryburger

.changeset/allow-empty-strings-abi-input.md

@@ -0,0 +1,5 @@
+---
+"@aragon/gov-ui-kit": patch
+---
+
+Allow empty strings on `ProposalActionsDecoder` string parameters (empty strings are valid Solidity values, including elements inside `string[]` inputs), skip validation on hidden tuple registration fields to avoid invisible submit-blocking errors, and keep the array index label and remove button aligned to the input when a validation alert is displayed.

src/modules/components/proposal/proposalActions/proposalActionsDecoder/proposalActionsDecoder.stories.tsx

@@ -214,6 +214,31 @@ export const ArrayType: Story = {
     },
 };
 
+/**
+ * Usage example of the Decoded view with a string array type, empty strings are valid values for string parameters.
+ */
+export const StringArrayType: Story = {
+    render: defaultRender,
+    args: {
+        view: ProposalActionsDecoderView.DECODED,
+        mode: ProposalActionsDecoderMode.EDIT,
+        action: generateProposalAction({
+            inputData: {
+                function: 'schedule',
+                contract: 'Timelock',
+                parameters: [
+                    {
+                        name: 'functionSignatures',
+                        type: 'string[]',
+                        value: ['transfer(address,uint256)', ''],
+                        notice: 'Function signatures - optional: if empty string, datas[i] is already starting with the function selector.',
+                    },
+                ],
+            },
+        }),
+    },
+};
+
 /**
  * Usage example of the Decoded view from the ProposalActionsItem component with an array type inside a tuple.
  */

src/modules/components/proposal/proposalActions/proposalActionsDecoder/proposalActionsDecoderField/proposalActionsDecoderField.test.tsx

@@ -92,7 +92,8 @@ describe('<ProposalActionsDecoderField /> component', () => {
         // eslint-disable-next-line testing-library/no-node-access
         expect(screen.getByText('[0]:').parentElement).toContainElement(textInputs[0]);
         // eslint-disable-next-line testing-library/no-node-access
-        expect(screen.getByText('[0]:').parentElement).toHaveClass('items-center');
+        expect(screen.getByText('[0]:').parentElement).toHaveClass('items-start');
+        expect(screen.getByText('[0]:')).toHaveClass('items-center');
         textInputs.forEach((input, index) => expect(input).toHaveDisplayValue(parameter.value[index]));
     });
 

src/modules/components/proposal/proposalActions/proposalActionsDecoder/proposalActionsDecoderField/proposalActionsDecoderField.tsx

@@ -131,17 +131,18 @@ export const ProposalActionsDecoderField: React.FC<IProposalActionsDecoderFieldP
                             const isNestedParameter =
                                 proposalActionsDecoderUtils.isTupleType(parameter.type) ||
                                 proposalActionsDecoderUtils.isArrayType(parameter.type);
+                            const isBooleanEditParameter =
+                                parameter.type === 'bool' && mode === ProposalActionsDecoderMode.EDIT;
+                            const alignToInput = !isNestedParameter && !isBooleanEditParameter;
 
                             return (
-                                <div
-                                    className={classNames('flex flex-row gap-2', {
-                                        'items-start': isNestedParameter,
-                                        'items-center': !isNestedParameter,
-                                    })}
-                                    key={nestedFieldPath}
-                                >
+                                <div className="flex flex-row items-start gap-2" key={nestedFieldPath}>
                                     {isArray && (
-                                        <p className={classNames(nestedParameterHeaderClassName, 'shrink-0')}>
+                                        <p
+                                            className={classNames(nestedParameterHeaderClassName, 'shrink-0', {
+                                                'flex h-12 items-center': alignToInput,
+                                            })}
+                                        >
                                             [{index.toString()}]:
                                         </p>
                                     )}
@@ -154,7 +155,7 @@ export const ProposalActionsDecoderField: React.FC<IProposalActionsDecoderFieldP
                                             parameter={parameter}
                                         />
                                     </div>
-                                    {renderDeleteButton(removeArrayItem, 'sm')}
+                                    {renderDeleteButton(removeArrayItem, 'sm', alignToInput ? 'mt-2' : undefined)}
                                 </div>
                             );
                         })}

src/modules/components/proposal/proposalActions/proposalActionsDecoder/proposalActionsDecoderTextField/proposalActionsDecoderTextFieldEdit.test.tsx

@@ -1,6 +1,7 @@
 import { render, screen } from '@testing-library/react';
 import userEvent from '@testing-library/user-event';
 import * as ReactHookForm from 'react-hook-form';
+import { modulesCopy } from '../../../../../assets';
 import * as useFormContext from '../../../../../hooks';
 import {
     type IProposalActionsDecoderTextFieldEditProps,
@@ -90,8 +91,8 @@ describe('<ProposalActionsDecoderTextFieldEdit /> component', () => {
         });
     });
 
-    it('does not set validation rules for array types', () => {
-        const parameter = { name: 'tupleArray', type: 'tuple[]', value: undefined };
+    it.each(['tuple[]', 'tuple', 'string'])('does not set validation rules for %s types', (type) => {
+        const parameter = { name: 'testParam', type, value: undefined };
         const fieldName = 'test';
         render(createTestComponent({ fieldName, parameter }));
         expect(useControllerSpy).toHaveBeenCalledWith({ name: fieldName, rules: { validate: undefined } });
@@ -163,6 +164,45 @@ describe('<ProposalActionsDecoderTextFieldEdit /> component', () => {
         );
     });
 
+    it('rejects empty values for uint types', () => {
+        const parameter = { name: 'uintParam', type: 'uint256', value: undefined };
+        render(createTestComponent({ parameter }));
+        const { validate } = useControllerSpy.mock.calls[0][0]!.rules!;
+        expect((validate as (value: unknown) => unknown)('')).toEqual(
+            modulesCopy.proposalActionsDecoder.validation.required(parameter.name),
+        );
+    });
+
+    it('initialises string types to empty strings when value is null', () => {
+        const onChange = jest.fn();
+        const parameter = { name: 'stringParam', type: 'string', value: undefined };
+        const controllerReturn = {
+            fieldState: { error: undefined },
+            field: { value: parameter.value, onChange },
+        } as unknown as ReactHookForm.UseControllerReturn;
+        useControllerSpy.mockReturnValue(controllerReturn);
+        useFormContextSpy.mockReturnValue({
+            watch: () => controllerReturn.field.value as unknown,
+        } as useFormContext.UseFormContextReturn);
+        render(createTestComponent({ parameter }));
+        expect(onChange).toHaveBeenCalledWith('');
+    });
+
+    it('does not change string values when parameter value is defined', () => {
+        const onChange = jest.fn();
+        const parameter = { name: 'stringParam', type: 'string', value: 'test-value' };
+        const controllerReturn = {
+            fieldState: { error: undefined },
+            field: { value: parameter.value, onChange },
+        } as unknown as ReactHookForm.UseControllerReturn;
+        useControllerSpy.mockReturnValue(controllerReturn);
+        useFormContextSpy.mockReturnValue({
+            watch: () => controllerReturn.field.value as unknown,
+        } as useFormContext.UseFormContextReturn);
+        render(createTestComponent({ parameter }));
+        expect(onChange).not.toHaveBeenCalled();
+    });
+
     it('initialises arrays to empty-arrays when value is null', () => {
         const onChange = jest.fn();
         const parameter = { name: 'uintArray', type: 'uint[]', value: undefined };

src/modules/components/proposal/proposalActions/proposalActionsDecoder/proposalActionsDecoderTextField/proposalActionsDecoderTextFieldEdit.tsx

@@ -13,6 +13,7 @@ export const ProposalActionsDecoderTextFieldEdit: React.FC<IProposalActionsDecod
 
     const { name, type } = parameter;
     const isArrayType = proposalActionsDecoderUtils.isArrayType(type);
+    const isStringType = proposalActionsDecoderUtils.isStringType(type);
 
     const { copy } = useGukModulesContext();
     const { watch } = useFormContext<Record<string, ProposalActionsFieldValue>>(true);
@@ -21,24 +22,30 @@ export const ProposalActionsDecoderTextFieldEdit: React.FC<IProposalActionsDecod
     const validateFunction = (value: ProposalActionsFieldValue) =>
         proposalActionsDecoderUtils.validateValue(value, { label: name, type, required: true, errorMessages });
 
+    // Skip validation for string types (empty strings are valid Solidity values) and for array / tuple types, which
+    // are hidden registration fields whose nested fields validate themselves.
+    const skipValidation = isArrayType || isStringType || proposalActionsDecoderUtils.isTupleType(type);
+
     // Watch value for changes as useControlled does not return updated value for array types
     // Note: using watch instead of useWatch because of form values being out of sync otherwise
     const fieldValue = watch(fieldName);
 
     const { fieldState, field } = useController<Record<string, ProposalActionsFieldValue>>({
         name: fieldName,
-        rules: { validate: isArrayType ? undefined : validateFunction },
+        rules: { validate: skipValidation ? undefined : validateFunction },
     });
 
     const { error } = fieldState;
     const { value, onChange, ...fieldProps } = field;
 
     useEffect(() => {
-        // Initialise array types as empty arrays to properly decode transaction data
+        // Initialise array types as empty arrays and string types as empty strings to properly encode transaction data
         if (isArrayType && fieldValue == null) {
             onChange([]);
+        } else if (isStringType && fieldValue == null) {
+            onChange('');
         }
-    }, [fieldValue, isArrayType, onChange]);
+    }, [fieldValue, isArrayType, isStringType, onChange]);
 
     const handleFieldChange = (event: ChangeEvent<HTMLInputElement | HTMLTextAreaElement>) => {
         if (type === 'bool') {

src/modules/components/proposal/proposalActions/proposalActionsDecoder/proposalActionsDecoderUtils.test.ts

@@ -65,6 +65,11 @@ describe('ProposalActionsDecoder utils', () => {
             expect(proposalActionsDecoderUtils.validateValue('value', params)).toBeUndefined();
         });
 
+        it('returns undefined when value is an empty string and not required', () => {
+            const params = buildValidateValueParams({ type: 'string', required: false });
+            expect(proposalActionsDecoderUtils.validateValue('', params)).toBeUndefined();
+        });
+
         it('returns bool error message when value has bool type and is not valid', () => {
             validateBooleanSpy.mockReturnValue(false);
             const params = buildValidateValueParams({ type: 'bool' });

src/modules/components/proposal/proposalActions/proposalActionsDecoder/proposalActionsDecoderUtils.ts

@@ -79,6 +79,8 @@ class ProposalActionsDecoderUtils {
 
     isTupleType = (type: string) => type === 'tuple';
 
+    isStringType = (type: string) => type === 'string';
+
     isNumberType = (type: string) => this.isUnsignedNumberType(type) || this.isSignedNumberType(type);
 
     isUnsignedNumberType = (type: string) => type.startsWith('uint');