[feat]: Add question mark validation to question title form (ONEARMY#…

…3758) * feat: add question mark validation to input and change composeValidators to return multiple errors * test: add value validator to noSpecialCharacters * test: increase research routes custom timeouts
goratt12 · Jul 12, 2024 · 497d10b · 497d10b
1 parent 8f75c79
commit 497d10b
Show file tree

Hide file tree

Showing 4 changed files with 169 additions and 54 deletions.
diff --git a/src/pages/Question/Content/Common/FormFields/QuestionTitle.field.tsx b/src/pages/Question/Content/Common/FormFields/QuestionTitle.field.tsx
@@ -9,6 +9,7 @@ import {
 import { fields } from 'src/pages/Question/labels'
 import {
   composeValidators,
+  endsWithQuestionMark,
   minValue,
   required,
   validateTitle,
@@ -38,6 +39,7 @@ export const QuestionTitleField = (props: IProps) => {
         validate={composeValidators(
           required,
           minValue(QUESTION_MIN_TITLE_LENGTH),
+          endsWithQuestionMark(),
           validateTitle(parentType, questionId, questionStore),
         )}
         component={FieldInput}

diff --git a/src/pages/Research/research.routes.test.tsx b/src/pages/Research/research.routes.test.tsx
@@ -145,7 +145,7 @@ describe('research.routes', () => {
             wrapper.getByText(/Help out with Research & Development/),
           ).toBeInTheDocument(),
         {
-          timeout: 2000,
+          timeout: 10000,
         },
       )
     })
@@ -165,7 +165,7 @@ describe('research.routes', () => {
           )
         },
         {
-          timeout: 5000,
+          timeout: 10000,
         },
       )
     })
@@ -179,11 +179,16 @@ describe('research.routes', () => {
         wrapper = renderFn('/research/create')
       })
 
-      await waitFor(() => {
-        expect(
-          wrapper.getByText(/role required to access this page/),
-        ).toBeInTheDocument()
-      })
+      await waitFor(
+        () => {
+          expect(
+            wrapper.getByText(/role required to access this page/),
+          ).toBeInTheDocument()
+        },
+        {
+          timeout: 10000,
+        },
+      )
     })
 
     it('rejects a logged in user missing required role', async () => {
@@ -192,11 +197,16 @@ describe('research.routes', () => {
         wrapper = renderFn('/research/create')
       })
 
-      await waitFor(() => {
-        expect(
-          wrapper.getByText(/role required to access this page/),
-        ).toBeInTheDocument()
-      })
+      await waitFor(
+        () => {
+          expect(
+            wrapper.getByText(/role required to access this page/),
+          ).toBeInTheDocument()
+        },
+        {
+          timeout: 10000,
+        },
+      )
     })
 
     it('accepts a logged in user with required role [research_creator]', async () => {
@@ -212,7 +222,7 @@ describe('research.routes', () => {
           expect(wrapper.getByText(/start your research/i)).toBeInTheDocument()
         },
         {
-          timeout: 2000,
+          timeout: 10000,
         },
       )
     })
@@ -229,7 +239,7 @@ describe('research.routes', () => {
           expect(wrapper.getByText(/start your research/i)).toBeInTheDocument()
         },
         {
-          timeout: 2000,
+          timeout: 10000,
         },
       )
     })
@@ -244,11 +254,16 @@ describe('research.routes', () => {
         wrapper = renderFn('/research/an-example/edit')
       })
 
-      await waitFor(() => {
-        expect(
-          wrapper.getByText(/role required to access this page/),
-        ).toBeInTheDocument()
-      })
+      await waitFor(
+        () => {
+          expect(
+            wrapper.getByText(/role required to access this page/),
+          ).toBeInTheDocument()
+        },
+        {
+          timeout: 10000,
+        },
+      )
     })
 
     it('accepts a logged in user with required role', async () => {
@@ -285,9 +300,16 @@ describe('research.routes', () => {
         renderFn('/research/an-example/edit')
       })
 
-      await waitFor(() => {
-        expect(mockedUsedNavigate).toHaveBeenCalledWith('/research/an-example')
-      })
+      await waitFor(
+        () => {
+          expect(mockedUsedNavigate).toHaveBeenCalledWith(
+            '/research/an-example',
+          )
+        },
+        {
+          timeout: 10000,
+        },
+      )
     })
 
     it('blocks a valid editor when document is locked by another user', async () => {
@@ -310,13 +332,18 @@ describe('research.routes', () => {
         wrapper = renderFn('/research/an-example/edit')
       })
 
-      await waitFor(() => {
-        expect(
-          wrapper.getByText(
-            'The research description is currently being edited by another editor.',
-          ),
-        ).toBeInTheDocument()
-      })
+      await waitFor(
+        () => {
+          expect(
+            wrapper.getByText(
+              'The research description is currently being edited by another editor.',
+            ),
+          ).toBeInTheDocument()
+        },
+        {
+          timeout: 10000,
+        },
+      )
     })
 
     it('accepts a user when document is mark locked by them', async () => {
@@ -339,9 +366,14 @@ describe('research.routes', () => {
         wrapper = renderFn('/research/an-example/edit')
       })
 
-      await waitFor(() => {
-        expect(wrapper.getByText('Edit your Research')).toBeInTheDocument()
-      })
+      await waitFor(
+        () => {
+          expect(wrapper.getByText('Edit your Research')).toBeInTheDocument()
+        },
+        {
+          timeout: 10000,
+        },
+      )
     })
 
     it('accepts a user with required role and contributor acccess', async () => {
@@ -378,11 +410,16 @@ describe('research.routes', () => {
         wrapper = renderFn('/research/an-example/new-update')
       })
 
-      await waitFor(() => {
-        expect(
-          wrapper.getByText(/role required to access this page/),
-        ).toBeInTheDocument()
-      })
+      await waitFor(
+        () => {
+          expect(
+            wrapper.getByText(/role required to access this page/),
+          ).toBeInTheDocument()
+        },
+        {
+          timeout: 10000,
+        },
+      )
     })
 
     it('accepts a logged in user with required role', async () => {
@@ -392,9 +429,14 @@ describe('research.routes', () => {
         wrapper = renderFn('/research/an-example/new-update')
       })
 
-      await waitFor(() => {
-        expect(wrapper.getByTestId('EditResearchUpdate')).toBeInTheDocument()
-      })
+      await waitFor(
+        () => {
+          expect(wrapper.getByTestId('EditResearchUpdate')).toBeInTheDocument()
+        },
+        {
+          timeout: 10000,
+        },
+      )
     })
   })
 
@@ -437,9 +479,14 @@ describe('research.routes', () => {
         )
       })
 
-      await waitFor(() => {
-        expect(wrapper.getByTestId(/EditResearchUpdate/i)).toBeInTheDocument()
-      })
+      await waitFor(
+        () => {
+          expect(wrapper.getByTestId(/EditResearchUpdate/i)).toBeInTheDocument()
+        },
+        {
+          timeout: 10000,
+        },
+      )
     })
 
     it('blocks valid author when document is locked', async () => {
@@ -507,9 +554,14 @@ describe('research.routes', () => {
         )
       })
 
-      await waitFor(() => {
-        expect(wrapper.getByText('Edit your update')).toBeInTheDocument()
-      })
+      await waitFor(
+        () => {
+          expect(wrapper.getByText('Edit your update')).toBeInTheDocument()
+        },
+        {
+          timeout: 10000,
+        },
+      )
     })
 
     it('rejects logged in user who is not author', async () => {
@@ -554,9 +606,14 @@ describe('research.routes', () => {
         )
       })
 
-      await waitFor(() => {
-        expect(wrapper.getByTestId(/EditResearchUpdate/i)).toBeInTheDocument()
-      })
+      await waitFor(
+        () => {
+          expect(wrapper.getByTestId(/EditResearchUpdate/i)).toBeInTheDocument()
+        },
+        {
+          timeout: 10000,
+        },
+      )
     })
   })
 })

diff --git a/src/utils/validators.test.ts b/src/utils/validators.test.ts
@@ -1,14 +1,19 @@
 import { afterEach, describe, expect, it, vi } from 'vitest'
 
 import {
+  composeValidators,
   draftValidationWrapper,
+  endsWithQuestionMark,
+  minValue,
   noSpecialCharacters,
   validateTitle,
 } from './validators'
 
 // Mock out module store to limit impact of circular dependency
 vi.mock('src/stores/common/module.store')
 
+import { QUESTION_MIN_TITLE_LENGTH } from 'src/pages/Question/constants'
+
 import { ResearchStore } from '../stores/Research/research.store'
 
 import type { IRootStore } from 'src/stores/RootStore'
@@ -84,3 +89,40 @@ describe('noSpecialCharacters', () => {
     expect(result).toContain('Only letters and numbers are allowed')
   })
 })
+
+describe('endsWithQuestionMark', () => {
+  const errorMessage = 'Needs to end with a question mark'
+  it('returns proper message when there are more than one question mark at the end', () => {
+    const result = endsWithQuestionMark()
+    expect(result('this is my question??')).toContain(errorMessage)
+  })
+
+  it('returns proper message when there are more than one question mark', () => {
+    const result = endsWithQuestionMark()
+    expect(result('this is my qu?estion?')).toContain(errorMessage)
+  })
+
+  it('returns proper message when there is no question mark', () => {
+    const result = endsWithQuestionMark()
+    expect(result('this is my question')).toContain(errorMessage)
+  })
+
+  it('returns proper message when there the question mark is not at the end', () => {
+    const result = endsWithQuestionMark()
+    expect(result('this is my? question')).toContain(errorMessage)
+  })
+})
+
+describe('composeValidators', () => {
+  it('should join multiple error messages', async () => {
+    const composedValidatorsFunc = composeValidators(
+      minValue(QUESTION_MIN_TITLE_LENGTH),
+      endsWithQuestionMark(),
+    )
+    const messages = await composedValidatorsFunc('this is')
+    expect(messages).toContain('Needs to end with a question mark')
+    expect(messages).toContain(
+      `Should be more than ${QUESTION_MIN_TITLE_LENGTH} characters`,
+    )
+  })
+})
diff --git a/src/utils/validators.ts b/src/utils/validators.ts
@@ -18,7 +18,7 @@ const required = (value: any) =>
   value ? undefined : 'Make sure this field is filled correctly'
 
 const noSpecialCharacters = (value: string) => {
-  const specialCharacters = getSpecialCharacters(value)
+  const specialCharacters = value ? getSpecialCharacters(value) : ''
   return specialCharacters.length > 0
     ? 'Only letters and numbers are allowed'
     : undefined
@@ -40,14 +40,27 @@ const minValue = (min: number) => (value) => {
     : undefined
 }
 
+const endsWithQuestionMark = () => (value) => {
+  const index = value.indexOf('?')
+  return index < 0 || index < value.length - 1
+    ? 'Needs to end with a question mark'
+    : undefined
+}
+
 const composeValidators =
   (...validators) =>
-  (value) =>
-    validators.reduce(
-      (error, validator) => error || validator(value),
-      undefined,
+  async (value) => {
+    const allResponse = await Promise.all(
+      validators.map((validator) => validator(value)),
     )
 
+    return allResponse.reduce(
+      (message, value) =>
+        typeof value === 'string' ? (message += value + '. ') : message,
+      '',
+    )
+  }
+
 const validateUrl = (value: any) => {
   if (value) {
     return isUrl(value) ? undefined : 'Invalid url'
@@ -137,4 +150,5 @@ export {
   composeValidators,
   validateTitle,
   noSpecialCharacters,
+  endsWithQuestionMark,
 }