Skip to content
Open
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
40 changes: 36 additions & 4 deletions src/components/Firestore/CollectionFilter/CollectionFilter.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,23 @@
isSingleValueCollectionFilter,
isSortableCollectionFilter,
} from '../models';
import { isBoolean, isNumber } from '../utils';
import { useCollectionFilter, useDispatch } from '../store';
import styles from './CollectionFilter.module.scss';
import { ConditionEntries } from './ConditionEntries';
import { ConditionEntry } from './ConditionEntry';
import { SortRadioGroup } from './SortRadioGroup';

function getConditionEntryType(value: any) {
if (isBoolean(value)) {
return 'boolean';
}
if (isNumber(value)) {
return 'number';
}
return 'string';
}

export const CollectionFilter: React.FC<
React.PropsWithChildren<{
className?: string;
Expand All @@ -54,7 +65,20 @@

const cf = formMethods.watch();

const [fieldType, setFieldType] = useState(
getConditionEntryType(
isSingleValueCollectionFilter(collectionFilter) && collectionFilter.value

Check failure on line 70 in src/components/Firestore/CollectionFilter/CollectionFilter.tsx

View workflow job for this annotation

GitHub Actions / build (22.x)

Argument of type 'CollectionFilter | undefined' is not assignable to parameter of type 'CollectionFilter'.

Check failure on line 70 in src/components/Firestore/CollectionFilter/CollectionFilter.tsx

View workflow job for this annotation

GitHub Actions / build (20.x)

Argument of type 'CollectionFilter | undefined' is not assignable to parameter of type 'CollectionFilter'.

Check failure on line 70 in src/components/Firestore/CollectionFilter/CollectionFilter.tsx

View workflow job for this annotation

GitHub Actions / build (24.x)

Argument of type 'CollectionFilter | undefined' is not assignable to parameter of type 'CollectionFilter'.
)
);

const onSubmit = (data: CollectionFilterType) => {
if (isSingleValueCollectionFilter(data)) {
if (fieldType === 'number') {
data.value = Number(data.value);
} else if (fieldType === 'boolean') {
data.value = data.value === 'true';
}
Comment on lines +78 to +80

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

This logic for handling boolean values is incorrect. The BooleanCondition component ensures that the form value is already a boolean when this onSubmit handler is called. This code will incorrectly convert a true boolean value to false (because true === 'true' is false), and a false boolean value will also become false (false === 'true' is false). Since BooleanCondition resets the value to a boolean when the type is switched, there's no scenario where data.value would be a string like 'true' when fieldType is 'boolean'. This else if block should be removed to fix boolean filtering.

}
dispatch(
actions.addCollectionFilter({
path,
Expand Down Expand Up @@ -96,14 +120,19 @@
preview={<ConditionPreview cf={cf} />}
defaultOpen
>
<ConditionSelect>
<ConditionSelect
fieldType={fieldType}
setFieldType={setFieldType}
>
{cf && isSingleValueCollectionFilter(cf) && (
<ConditionEntry
name="value"
error={
(formMethods.formState.touchedFields as any)['value'] &&
(formMethods.formState.errors as any)['value']?.message
}
fieldType={fieldType}
setFieldType={setFieldType}
/>
)}

Expand Down Expand Up @@ -207,9 +236,12 @@
);
};

const ConditionSelect: React.FC<React.PropsWithChildren<unknown>> = ({
children,
}) => {
const ConditionSelect: React.FC<
React.PropsWithChildren<{
fieldType: string;
setFieldType: (type: string) => void;
}>
> = ({ children, fieldType, setFieldType }) => {
const options: Array<{
label: string;
value: WhereFilterOp;
Expand Down
18 changes: 4 additions & 14 deletions src/components/Firestore/CollectionFilter/ConditionEntry.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,36 +14,28 @@
* limitations under the License.
*/

import React, { useEffect, useState } from 'react';

Check failure on line 17 in src/components/Firestore/CollectionFilter/ConditionEntry.tsx

View workflow job for this annotation

GitHub Actions / build (22.x)

'useState' is defined but never used

Check failure on line 17 in src/components/Firestore/CollectionFilter/ConditionEntry.tsx

View workflow job for this annotation

GitHub Actions / build (20.x)

'useState' is defined but never used

Check failure on line 17 in src/components/Firestore/CollectionFilter/ConditionEntry.tsx

View workflow job for this annotation

GitHub Actions / build (24.x)

'useState' is defined but never used
import { Controller, useFormContext } from 'react-hook-form';

import { Field, SelectField } from '../../../components/common/Field';
import { NUMBER_REGEX, isBoolean, isNumber } from '../utils';

Check failure on line 21 in src/components/Firestore/CollectionFilter/ConditionEntry.tsx

View workflow job for this annotation

GitHub Actions / build (22.x)

'isNumber' is defined but never used

Check failure on line 21 in src/components/Firestore/CollectionFilter/ConditionEntry.tsx

View workflow job for this annotation

GitHub Actions / build (22.x)

'isBoolean' is defined but never used

Check failure on line 21 in src/components/Firestore/CollectionFilter/ConditionEntry.tsx

View workflow job for this annotation

GitHub Actions / build (20.x)

'isNumber' is defined but never used

Check failure on line 21 in src/components/Firestore/CollectionFilter/ConditionEntry.tsx

View workflow job for this annotation

GitHub Actions / build (20.x)

'isBoolean' is defined but never used

Check failure on line 21 in src/components/Firestore/CollectionFilter/ConditionEntry.tsx

View workflow job for this annotation

GitHub Actions / build (24.x)

'isNumber' is defined but never used

Check failure on line 21 in src/components/Firestore/CollectionFilter/ConditionEntry.tsx

View workflow job for this annotation

GitHub Actions / build (24.x)

'isBoolean' is defined but never used
import styles from './CollectionFilter.module.scss';

function getConditionEntryType(value: any) {
if (isBoolean(value)) {
return 'boolean';
}
if (isNumber(value)) {
return 'number';
}
return 'string';
}

interface ConditionEntryProps {
name: string;
// Error may be undefined; require consumers to still pass or we might end up
// in a state where we validate without showing the error message.
error: string | undefined;
fieldType: string;
setFieldType: (type: string) => void;
}

export const ConditionEntry: React.FC<
React.PropsWithChildren<ConditionEntryProps>
> = React.memo(({ name, error }) => {
> = React.memo(({ name, error, fieldType, setFieldType }) => {
const { setValue, watch } = useFormContext();
const value = watch(name);
const [fieldType, setFieldType] = useState(getConditionEntryType(value));

useEffect(() => {
// Essentially setting the defaultValue of this form-field,
Expand All @@ -58,9 +50,7 @@
<SelectField
options={['string', 'number', 'boolean']}
value={fieldType}
onChange={(evt) => {
setFieldType(evt.currentTarget.value);
}}
onChange={(evt) => setFieldType(evt.currentTarget.value)}
fieldClassName={styles.conditionEntryType}
/>

Expand Down
Loading