fix: prevent empty poll options and questions in composition (#1541)

Author: MartinCupela

src/messageComposer/middleware/pollComposer/state.ts

@@ -11,6 +11,8 @@ export const VALID_MAX_VOTES_VALUE_REGEX = /^([2-9]|10)$/;
 
 export const MAX_POLL_OPTIONS = 100 as const;
 
+const textFieldIsEmpty = (text: string) => !text.trim();
+
 export type PollStateValidationOutput = Partial<
   Omit<Record<keyof PollComposerState['data'], string>, 'options'> & {
     options?: Record<string, string>;
@@ -74,9 +76,20 @@ export const defaultPollFieldBlurEventValidators: Partial<
     return { max_votes_allowed: undefined };
   },
   name: ({ value }) => {
-    if (!value) return { name: 'Name is required' };
+    if (textFieldIsEmpty(value)) return { name: 'Question is required' };
     return { name: undefined };
   },
+  options: (params) => {
+    const defaultResult = pollStateChangeValidators.options?.(params);
+    const errors = defaultResult?.options ?? {};
+    params.value.forEach((option: { id: string; text: string }, index: number) => {
+      const isTheLastOption = index === params.value.length - 1;
+      if (textFieldIsEmpty(option.text) && !isTheLastOption) {
+        errors[option.id] = 'Option is empty';
+      }
+    });
+    return Object.keys(errors).length > 0 ? { options: errors } : { options: undefined };
+  },
 };
 
 export type PollCompositionStateProcessorOutput = Partial<PollComposerState['data']>;
@@ -168,15 +181,19 @@ export const createPollComposerStateMiddleware = ({
   processors: customProcessors,
   validators: customValidators,
 }: PollComposerStateMiddlewareFactoryOptions = {}): PollComposerStateMiddleware => {
-  const universalHandler = (
-    state: PollComposerStateChangeMiddlewareValue,
+  const universalHandler = ({
+    state,
+    validators,
+    processors,
+  }: {
+    state: PollComposerStateChangeMiddlewareValue;
     validators: Partial<
       Record<keyof PollComposerState['data'], PollStateChangeValidator>
-    >,
+    >;
     processors?: Partial<
       Record<keyof PollComposerState['data'], PollCompositionStateProcessor>
-    >,
-  ) => {
+    >;
+  }) => {
     const { previousState, targetFields } = state;
 
     let newData: Partial<PollComposerState['data']>;
@@ -210,9 +227,9 @@ export const createPollComposerStateMiddleware = ({
       const validator = validators[key as keyof PollComposerState['data']];
       if (validator) {
         const error = validator({
+          currentError: previousState.errors[key as keyof PollComposerState['data']],
           data: previousState.data,
           value: newData[key as keyof PollComposerState['data']],
-          currentError: previousState.errors[key as keyof PollComposerState['data']],
         });
         acc = { ...acc, ...error };
       }
@@ -232,21 +249,19 @@ export const createPollComposerStateMiddleware = ({
       }: MiddlewareHandlerParams<PollComposerStateChangeMiddlewareValue>) => {
         if (!state.targetFields) return forward();
         const { previousState, injectedFieldErrors } = state;
-        const finalValidators = {
-          ...pollStateChangeValidators,
-          ...defaultPollFieldChangeEventValidators,
-          ...customValidators?.handleFieldChange,
-        };
-        const finalProcessors = {
-          ...pollCompositionStateProcessors,
-          ...customProcessors?.handleFieldChange,
-        };
-
-        const { newData, newErrors } = universalHandler(
+
+        const { newData, newErrors } = universalHandler({
+          processors: {
+            ...pollCompositionStateProcessors,
+            ...customProcessors?.handleFieldChange,
+          },
           state,
-          finalValidators,
-          finalProcessors,
-        );
+          validators: {
+            ...pollStateChangeValidators,
+            ...defaultPollFieldChangeEventValidators,
+            ...customValidators?.handleFieldChange,
+          },
+        });
 
         return next({
           ...state,
@@ -265,16 +280,15 @@ export const createPollComposerStateMiddleware = ({
         if (!state.targetFields) return forward();
 
         const { previousState } = state;
-        const finalValidators = {
-          ...pollStateChangeValidators,
-          ...defaultPollFieldBlurEventValidators,
-          ...customValidators?.handleFieldBlur,
-        };
-        const { newData, newErrors } = universalHandler(
+        const { newData, newErrors } = universalHandler({
+          processors: customProcessors?.handleFieldBlur,
           state,
-          finalValidators,
-          customProcessors?.handleFieldBlur,
-        );
+          validators: {
+            ...pollStateChangeValidators,
+            ...defaultPollFieldBlurEventValidators,
+            ...customValidators?.handleFieldBlur,
+          },
+        });
 
         return next({
           ...state,

src/messageComposer/pollComposer.ts

@@ -154,7 +154,9 @@ export class PollComposer {
           max_votes_allowed: data.max_votes_allowed
             ? parseInt(data.max_votes_allowed)
             : undefined,
-          options: data.options?.filter((o) => o.text).map((o) => ({ text: o.text })),
+          options: data.options
+            ?.filter((o) => o.text.trim())
+            .map((o) => ({ text: o.text })),
         },
         errors,
       },

test/unit/MessageComposer/middleware/pollComposer/state.test.ts

@@ -384,13 +384,43 @@ describe('PollComposerStateMiddleware', () => {
     });
 
     describe('options validation', () => {
-      it('should validate empty options on blur', async () => {
+      it('should not validate empty options on blur', async () => {
         const stateMiddleware = setup();
         const result = await stateMiddleware.handlers.handleFieldBlur(
           setupHandlerParams({
             nextState: { ...getInitialState() },
             previousState: { ...getInitialState() },
-            targetFields: { options: [{ id: 'option-id', text: '' }] },
+            targetFields: {
+              options: [
+                { id: 'option-id1', text: '' },
+                { id: 'option-id2', text: '' },
+              ],
+            },
+          }),
+        );
+
+        expect(result.state.nextState.errors.options).toBeDefined();
+        expect(Object.keys(result.state.nextState.errors.options!)).toHaveLength(2);
+        expect(result.state.nextState.errors.options!['option-id1']).toBe(
+          'Option is empty',
+        );
+        expect(result.state.nextState.errors.options!['option-id2']).toBe(
+          'Option already exists',
+        );
+      });
+
+      it('should validate last empty option on blur', async () => {
+        const stateMiddleware = setup();
+        const result = await stateMiddleware.handlers.handleFieldBlur(
+          setupHandlerParams({
+            nextState: { ...getInitialState() },
+            previousState: { ...getInitialState() },
+            targetFields: {
+              options: [
+                { id: 'option-id1', text: 'A' },
+                { id: 'option-id2', text: '' },
+              ],
+            },
           }),
         );