From a6d582844ed269408a28cc7b092bc6a955752116 Mon Sep 17 00:00:00 2001 From: toptalwadiibasmi <108020813+toptalwadiibasmi@users.noreply.github.com> Date: Wed, 25 Sep 2024 14:29:04 +0100 Subject: [PATCH] [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 | 6 ++ .../src/QueryBuilder/QueryBuilder.tsx | 20 ++---- .../story/CustomValueEditor.example.tsx | 10 +-- .../picasso-query-builder/src/utils/index.ts | 2 +- ...s => use-query-builder-validation.test.ts} | 67 ++++++++++--------- ...tor.ts => use-query-builder-validation.ts} | 24 +++---- 6 files changed, 66 insertions(+), 63 deletions(-) create mode 100644 .changeset/breezy-ads-shave.md rename packages/picasso-query-builder/src/utils/{use-query-builder-validator.test.ts => use-query-builder-validation.test.ts} (67%) rename packages/picasso-query-builder/src/utils/{use-query-builder-validator.ts => use-query-builder-validation.ts} (81%) diff --git a/.changeset/breezy-ads-shave.md b/.changeset/breezy-ads-shave.md new file mode 100644 index 0000000000..7e425a6551 --- /dev/null +++ b/.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 diff --git a/packages/picasso-query-builder/src/QueryBuilder/QueryBuilder.tsx b/packages/picasso-query-builder/src/QueryBuilder/QueryBuilder.tsx index d7e22ce3c4..99f4d370c9 100644 --- a/packages/picasso-query-builder/src/QueryBuilder/QueryBuilder.tsx +++ b/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 @@ -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({}) 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} /> diff --git a/packages/picasso-query-builder/src/QueryBuilder/story/CustomValueEditor.example.tsx b/packages/picasso-query-builder/src/QueryBuilder/story/CustomValueEditor.example.tsx index 508ee2771a..b56e487eb8 100644 --- a/packages/picasso-query-builder/src/QueryBuilder/story/CustomValueEditor.example.tsx +++ b/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} /> ) } diff --git a/packages/picasso-query-builder/src/utils/index.ts b/packages/picasso-query-builder/src/utils/index.ts index 3909ab347b..f7153edda4 100644 --- a/packages/picasso-query-builder/src/utils/index.ts +++ b/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' diff --git a/packages/picasso-query-builder/src/utils/use-query-builder-validator.test.ts b/packages/picasso-query-builder/src/utils/use-query-builder-validation.test.ts similarity index 67% rename from packages/picasso-query-builder/src/utils/use-query-builder-validator.test.ts rename to packages/picasso-query-builder/src/utils/use-query-builder-validation.test.ts index 148a8a48cf..50f2920332 100644 --- a/packages/picasso-query-builder/src/utils/use-query-builder-validator.test.ts +++ b/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, diff --git a/packages/picasso-query-builder/src/utils/use-query-builder-validator.ts b/packages/picasso-query-builder/src/utils/use-query-builder-validation.ts similarity index 81% rename from packages/picasso-query-builder/src/utils/use-query-builder-validator.ts rename to packages/picasso-query-builder/src/utils/use-query-builder-validation.ts index 3807bd5c74..dcaead7440 100644 --- a/packages/picasso-query-builder/src/utils/use-query-builder-validator.ts +++ b/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 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({}) + 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