Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/breezy-ads-shave.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@toptal/picasso-query-builder': major
---

- query validation separated from QueryBuilder component implementation
- customValueEditor prop renamed as valueEditor
Original file line number Diff line number Diff line change
Expand Up @@ -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>

Expand All @@ -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
toptalwadiibasmi marked this conversation as resolved.
Show resolved Hide resolved
/** Defines the loading state. */
loading?: boolean
/** Defines padded layout. */
Expand Down Expand Up @@ -87,7 +86,7 @@ const QueryBuilder = ({
maxGroupDepth = 3,
loading = false,
onSubmit,
customValueEditor = ValueEditor,
valueEditor = ValueEditor,
footer,
hideControls,
header,
Expand All @@ -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()

Expand All @@ -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) {
Expand Down Expand Up @@ -222,7 +216,7 @@ const QueryBuilder = ({
} as QueryBuilderContext
}
controlElements={{
valueEditor: customValueEditor,
valueEditor,
}}
enableDragAndDrop={enableDragAndDrop}
/>
Expand Down
Original file line number Diff line number Diff line change
@@ -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 = {
Expand Down Expand Up @@ -67,7 +67,7 @@ const Example = () => {
query={query}
onQueryChange={handleQueryChange}
fields={fields}
customValueEditor={CustomValueEditor}
valueEditor={CustomValueEditor}
/>
)
}
Expand Down
2 changes: 1 addition & 1 deletion packages/picasso-query-builder/src/utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
toptalwadiibasmi marked this conversation as resolved.
Show resolved Hide resolved
export { default as validateValueEditor } from './validate-value-editor'
Original file line number Diff line number Diff line change
@@ -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) {
Expand Down Expand Up @@ -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,
Expand All @@ -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: {
Expand All @@ -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,
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useCallback, useMemo } from 'react'
import { useCallback, useMemo, useState } from 'react'
import type {
RuleGroupTypeAny,
RuleType,
Expand All @@ -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) => {
Expand Down Expand Up @@ -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) => ({
Expand All @@ -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
Loading