[CPT-1332] Separate validation from QueryBuilder implementation (#4566)

* Externalize validation from query builder impl

* Change hook name and update test

* Replace customValueEditor with validator

* Update changeset

.changeset/breezy-ads-shave.md

@@ -0,0 +1,6 @@
+---
+'@toptal/picasso-query-builder': major
+---
+
+- query validation separated from QueryBuilder component implementation
+- customValueEditor prop renamed as valueEditor

packages/picasso-query-builder/src/QueryBuilder/QueryBuilder.tsx

@@ -32,7 +32,6 @@ import { controlClassnames, useQueryBuilderValidator } from '../utils'
 import styles from './styles'
 import { useOnQueryChange } from './hooks/useOnQueryChange'
 import { ValidationErrors } from '../ValidationErrors'
-import type { ValidatorResult } from '../utils/use-query-builder-validator'
 
 type ValueEditorComponentProps = ComponentType<DefaultValueEditorProps>
 
@@ -54,7 +53,7 @@ type Props = {
   /** Defines a function that is called when the user submits a query constructed in the QB. This function takes a single argument - constructed query. */
   onSubmit?: (query: RuleGroupTypeAny) => void
   /** Defines a component that allows possibility to customize value editor that is used in QB. By default, QB provides default set of editors (text inputs, dropdowns, etc.). */
-  customValueEditor?: ValueEditorComponentProps
+  valueEditor?: ValueEditorComponentProps
   /** Defines the loading state. */
   loading?: boolean
   /** Defines padded layout. */
@@ -87,7 +86,7 @@ const QueryBuilder = ({
   maxGroupDepth = 3,
   loading = false,
   onSubmit,
-  customValueEditor = ValueEditor,
+  valueEditor = ValueEditor,
   footer,
   hideControls,
   header,
@@ -101,10 +100,6 @@ const QueryBuilder = ({
   const classes = useStyles()
 
   const [submitButtonClicked, setSubmitButtonClicked] = useState(false)
-  const [queryBuilderValid, setIsQueryBuilderValid] = useState<
-    boolean | undefined
-  >()
-  const [validationErrors, setValidationErrors] = useState<ValidatorResult>({})
 
   const { showError } = useNotifications()
 
@@ -113,11 +108,10 @@ const QueryBuilder = ({
     callback: onQueryChange,
   })
 
-  const { validator } = useQueryBuilderValidator({
-    fields,
-    onValidChange: setIsQueryBuilderValid,
-    onValidationResultChange: setValidationErrors,
-  })
+  const { validator, validationErrors, queryBuilderValid } =
+    useQueryBuilderValidator({
+      fields,
+    })
 
   const resetQuery = useCallback(() => {
     if (onQueryReset) {
@@ -222,7 +216,7 @@ const QueryBuilder = ({
               } as QueryBuilderContext
             }
             controlElements={{
-              valueEditor: customValueEditor,
+              valueEditor,
             }}
             enableDragAndDrop={enableDragAndDrop}
           />

packages/picasso-query-builder/src/QueryBuilder/story/CustomValueEditor.example.tsx

@@ -1,9 +1,9 @@
 import React, { useState } from 'react'
-import type { ValueEditorProps } from '@toptal/picasso-query-builder'
-import {
-  QueryBuilder,
-  type RuleGroupTypeAny,
+import type {
+  RuleGroupTypeAny,
+  ValueEditorProps,
 } from '@toptal/picasso-query-builder'
+import { QueryBuilder } from '@toptal/picasso-query-builder'
 import { Input, Select } from '@toptal/picasso'
 
 const initialQuery = {
@@ -67,7 +67,7 @@ const Example = () => {
       query={query}
       onQueryChange={handleQueryChange}
       fields={fields}
-      customValueEditor={CustomValueEditor}
+      valueEditor={CustomValueEditor}
     />
   )
 }

packages/picasso-query-builder/src/utils/index.ts

@@ -2,5 +2,5 @@ export { controlClassnames } from './config'
 export { generateSelectOptions } from './generate-select-options'
 export { getQueryDepth } from './get-query-depth'
 export { default as useHandleTouched } from './use-handle-touched'
-export { default as useQueryBuilderValidator } from './use-query-builder-validator'
+export { default as useQueryBuilderValidator } from './use-query-builder-validation'
 export { default as validateValueEditor } from './validate-value-editor'

packages/picasso-query-builder/src/utils/use-query-builder-validator.test.ts → packages/picasso-query-builder/src/utils/use-query-builder-validation.test.ts

@@ -1,8 +1,8 @@
-import { renderHook } from '@testing-library/react-hooks'
+import { act, renderHook } from '@testing-library/react-hooks'
 import type { RuleGroupTypeAny, RuleType } from 'react-querybuilder'
 
 import type { Field } from '../types/query-builder'
-import useQueryBuilderValidator from './use-query-builder-validator'
+import useQueryBuilderValidation from './use-query-builder-validation'
 
 const validateMock1 = (rule: RuleType) => {
   if (!rule.value) {
@@ -97,27 +97,26 @@ const groupedQuery = (field3value = 'should be this'): RuleGroupTypeAny => ({
   ],
 })
 
-const onValidChangeMock = jest.fn()
-const onValidationResultChangeMock = jest.fn()
-
-describe('useQueryBuilderValidator', () => {
+describe('useQueryBuilderValidation', () => {
   describe('when query has only a single group of rule', () => {
     describe('when query does not contain an invalid rule', () => {
       it('returns true', () => {
         const { result } = renderHook(() =>
-          useQueryBuilderValidator({
+          useQueryBuilderValidation({
             fields,
-            onValidChange: onValidChangeMock,
-            onValidationResultChange: onValidationResultChangeMock,
           })
         )
 
         const { validator } = result.current
 
-        expect(validator(query())).toBe(true)
+        act(() => {
+          expect(validator(query())).toBe(true)
+        })
+
+        const { validationErrors, queryBuilderValid } = result.current
 
-        expect(onValidChangeMock).toHaveBeenCalledWith(true)
-        expect(onValidationResultChangeMock).toHaveBeenCalledWith({
+        expect(queryBuilderValid).toBe(true)
+        expect(validationErrors).toEqual({
           rule1: true,
           rule2: true,
           rule3: true,
@@ -128,21 +127,21 @@ describe('useQueryBuilderValidator', () => {
     describe('when query contains an invalid rule', () => {
       it('returns false', () => {
         const { result } = renderHook(() =>
-          useQueryBuilderValidator({
+          useQueryBuilderValidation({
             fields,
-            onValidChange: onValidChangeMock,
-            onValidationResultChange: onValidationResultChangeMock,
           })
         )
 
         const { validator } = result.current
 
-        const isValid = validator(query('invalid value for field2'))
+        act(() => {
+          expect(validator(query('invalid value for field2'))).toBe(false)
+        })
 
-        expect(isValid).toBe(false)
+        const { validationErrors, queryBuilderValid } = result.current
 
-        expect(onValidChangeMock).toHaveBeenCalledWith(false)
-        expect(onValidationResultChangeMock).toHaveBeenCalledWith({
+        expect(queryBuilderValid).toBe(false)
+        expect(validationErrors).toEqual({
           rule1: true,
           rule2: true,
           rule3: {
@@ -158,18 +157,21 @@ describe('useQueryBuilderValidator', () => {
     describe('when query does not contain invalid rule', () => {
       it('returns true', () => {
         const { result } = renderHook(() =>
-          useQueryBuilderValidator({
+          useQueryBuilderValidation({
             fields,
-            onValidChange: onValidChangeMock,
-            onValidationResultChange: onValidationResultChangeMock,
           })
         )
 
         const { validator } = result.current
 
-        expect(validator(groupedQuery())).toBe(true)
-        expect(onValidChangeMock).toHaveBeenCalledWith(true)
-        expect(onValidationResultChangeMock).toHaveBeenCalledWith({
+        act(() => {
+          expect(validator(groupedQuery())).toBe(true)
+        })
+
+        const { validationErrors, queryBuilderValid } = result.current
+
+        expect(queryBuilderValid).toBe(true)
+        expect(validationErrors).toEqual({
           group1: true,
           group2: true,
           rule1: true,
@@ -182,18 +184,21 @@ describe('useQueryBuilderValidator', () => {
     describe('when query contains invalid rule', () => {
       it('returns false', () => {
         const { result } = renderHook(() =>
-          useQueryBuilderValidator({
+          useQueryBuilderValidation({
             fields,
-            onValidChange: onValidChangeMock,
-            onValidationResultChange: onValidationResultChangeMock,
           })
         )
 
         const { validator } = result.current
 
-        expect(validator(groupedQuery('invalid rule'))).toBe(false)
-        expect(onValidChangeMock).toHaveBeenCalledWith(false)
-        expect(onValidationResultChangeMock).toHaveBeenCalledWith({
+        act(() => {
+          expect(validator(groupedQuery('invalid rule'))).toBe(false)
+        })
+
+        const { validationErrors, queryBuilderValid } = result.current
+
+        expect(queryBuilderValid).toBe(false)
+        expect(validationErrors).toEqual({
           group1: true,
           group2: true,
           rule1: true,

packages/picasso-query-builder/src/utils/use-query-builder-validator.ts → packages/picasso-query-builder/src/utils/use-query-builder-validation.ts

@@ -1,4 +1,4 @@
-import { useCallback, useMemo } from 'react'
+import { useCallback, useMemo, useState } from 'react'
 import type {
   RuleGroupTypeAny,
   RuleType,
@@ -14,8 +14,6 @@ export type ValidatorResult = Record<string, ValidationResult | boolean>
 
 type Props = {
   fields: Field[]
-  onValidChange?: (isValid: boolean) => void
-  onValidationResultChange?: (validationResult: ValidatorResult) => void
 }
 
 const validateRule = (rule: RuleType, fieldValidatorMap: ValidatorMap) => {
@@ -72,11 +70,11 @@ const validateQuery = (
   return validateRule(query as RuleType, fieldValidatorMap)
 }
 
-const useQueryBuilderValidator = ({
-  fields,
-  onValidChange,
-  onValidationResultChange,
-}: Props) => {
+const useQueryBuilderValidation = ({ fields }: Props) => {
+  const [validationErrors, setValidationErrors] = useState<ValidatorResult>({})
+  const [queryBuilderValid, setIsQueryBuilderValid] = useState<
+    boolean | undefined
+  >()
   const fieldValidatorMap: ValidatorMap = useMemo(() => {
     return fields.reduce(
       (acc, field) => ({
@@ -97,15 +95,15 @@ const useQueryBuilderValidator = ({
 
       const isValid = !Object.values(valResult).some(result => result !== true)
 
-      onValidChange?.(isValid)
-      onValidationResultChange?.(valResult)
+      setIsQueryBuilderValid?.(isValid)
+      setValidationErrors?.(valResult)
 
       return isValid
     },
-    [fieldValidatorMap, onValidChange, onValidationResultChange]
+    [fieldValidatorMap]
   )
 
-  return { validator }
+  return { validator, validationErrors, queryBuilderValid }
 }
 
-export default useQueryBuilderValidator
+export default useQueryBuilderValidation