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

Referenced items won't be selected when IDs type differ in <ReferenceArrayInput> and <AutoCompleteInput> #8500

Merged
merged 23 commits into from
Dec 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
50a5684
bug fix and stories
antoinefricker Dec 13, 2022
1e4ffa5
WIP test
antoinefricker Dec 13, 2022
0353a7b
Fix tests
antoinefricker Dec 13, 2022
3670f60
Working on tests / Story to test nullish values
antoinefricker Dec 14, 2022
6abfec5
Work on nullish tests
antoinefricker Dec 14, 2022
3c34031
Fix nullish tests
antoinefricker Dec 14, 2022
c3334e9
Error in imports
antoinefricker Dec 14, 2022
d7acf97
Fix Datagrid
antoinefricker Dec 14, 2022
28ec43c
Integrates review comments
antoinefricker Dec 14, 2022
c457f14
Update packages/ra-ui-materialui/src/input/AutocompleteInput.stories.tsx
antoinefricker Dec 14, 2022
ccc7af7
Update packages/ra-ui-materialui/src/input/AutocompleteInput.stories.tsx
antoinefricker Dec 14, 2022
5c6964f
Update packages/ra-ui-materialui/src/input/AutocompleteInput.tsx
antoinefricker Dec 14, 2022
4d0c56b
Update packages/ra-ui-materialui/src/input/ReferenceArrayInput.spec.tsx
antoinefricker Dec 14, 2022
c283d65
Simplify NullishValuesSupport / fix renaming issues
antoinefricker Dec 14, 2022
567f4c3
Update packages/ra-ui-materialui/src/input/ReferenceArrayInput.spec.tsx
antoinefricker Dec 14, 2022
90e90dd
Renaming story component
antoinefricker Dec 15, 2022
07013c8
Corrections from review
antoinefricker Dec 15, 2022
c42b44f
Leave create item code alone
antoinefricker Dec 15, 2022
3c2495d
Add comment for unexpected code
antoinefricker Dec 15, 2022
a008e1d
Update packages/ra-ui-materialui/src/field/ReferenceArrayField.spec.tsx
antoinefricker Dec 15, 2022
c427d96
Let the errors flood
antoinefricker Dec 15, 2022
16dd719
Merge branch 'reference-inconsistant-types-ids' of github.com:marmela…
antoinefricker Dec 15, 2022
3c51a69
More verbose comment for unclear code section on option deselection
antoinefricker Dec 16, 2022
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
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ export const useReferenceArrayFieldController = (

const ids = useMemo(() => {
if (Array.isArray(value)) return value;
console.warn(`Value of field '${source}' is not an array.`);
console.warn(`Value of field '${source}' is not an array.`, value);
return emptyArray;
}, [value, source]);

Expand Down
15 changes: 12 additions & 3 deletions packages/ra-ui-materialui/src/field/ReferenceArrayField.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
import { TextField } from './TextField';
import { SingleFieldList } from '../list';
import { AdminContext } from '../AdminContext';
import { DifferentIdTypes } from './ReferenceArrayField.stories';

const theme = createTheme({});

Expand Down Expand Up @@ -78,7 +79,7 @@ describe('<ReferenceArrayField />', () => {
</MemoryRouter>
);
expect(queryAllByRole('progressbar')).toHaveLength(0);
expect(container.firstChild.textContent).not.toBeUndefined();
expect(container.firstChild?.textContent).not.toBeUndefined();
expect(getByText('hello')).not.toBeNull();
expect(getByText('world')).not.toBeNull();
});
Expand Down Expand Up @@ -106,7 +107,7 @@ describe('<ReferenceArrayField />', () => {
</ThemeProvider>
);
expect(queryAllByRole('progressbar')).toHaveLength(0);
expect(container.firstChild.textContent).toBe('');
expect(container.firstChild?.textContent).toBe('');
});

it('should support record with string identifier', () => {
Expand Down Expand Up @@ -138,7 +139,7 @@ describe('<ReferenceArrayField />', () => {
</MemoryRouter>
);
expect(queryAllByRole('progressbar')).toHaveLength(0);
expect(container.firstChild.textContent).not.toBeUndefined();
expect(container.firstChild?.textContent).not.toBeUndefined();
expect(getByText('hello')).not.toBeNull();
expect(getByText('world')).not.toBeNull();
});
Expand Down Expand Up @@ -301,4 +302,12 @@ describe('<ReferenceArrayField />', () => {
await screen.findByText('tag:management');
await screen.findByText('tag:design');
});

it('should handle IDs of different types', async () => {
render(<DifferentIdTypes />);

expect(await screen.findByText('artist_1')).not.toBeNull();
expect(await screen.findByText('artist_2')).not.toBeNull();
expect(await screen.findByText('artist_3')).not.toBeNull();
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import * as React from 'react';
import fakeRestProvider from 'ra-data-fakerest';

import { AdminContext } from '../AdminContext';
import { Datagrid } from '../list';
import { ReferenceArrayField } from './ReferenceArrayField';
import { TextField } from './TextField';
import { Show } from '../detail';
import { CardContent } from '@mui/material';

const fakeData = {
bands: [{ id: 1, name: 'band_1', members: [1, '2', '3'] }],
artists: [
{ id: 1, name: 'artist_1' },
{ id: 2, name: 'artist_2' },
{ id: 3, name: 'artist_3' },
{ id: 4, name: 'artist_4' },
],
};

export default { title: 'ra-ui-materialui/fields/ReferenceArrayField' };

export const DifferentIdTypes = () => {
return (
<AdminContext dataProvider={fakeRestProvider(fakeData, false)}>
<CardContent>
<Show resource="bands" id={1} sx={{ width: 600 }}>
<TextField source="name" fullWidth />
<ReferenceArrayField
fullWidth
source="members"
reference="artists"
>
<Datagrid bulkActionButtons={false}>
<TextField source="id" />
<TextField source="name" />
</Datagrid>
</ReferenceArrayField>
</Show>
</CardContent>
</AdminContext>
);
};
21 changes: 21 additions & 0 deletions packages/ra-ui-materialui/src/input/AutocompleteInput.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
InsideReferenceInput,
InsideReferenceInputDefaultValue,
Nullable,
NullishValuesSupport,
VeryLargeOptionsNumber,
} from './AutocompleteInput.stories';
import { act } from '@testing-library/react-hooks';
Expand Down Expand Up @@ -1477,4 +1478,24 @@ describe('<AutocompleteInput />', () => {
expect(input.value).toEqual('');
});
});

it('should handle nullish values', async () => {
render(<NullishValuesSupport />);

const checkInputValue = async (label: string, expected: any) => {
const input = (await screen.findByLabelText(
label
)) as HTMLInputElement;
await waitFor(() => {
expect(input.value).toStrictEqual(expected);
});
};

await checkInputValue('prefers_empty-string', '');
await checkInputValue('prefers_null', '');
await checkInputValue('prefers_undefined', '');
await checkInputValue('prefers_zero-string', '0');
await checkInputValue('prefers_zero-number', '0');
await checkInputValue('prefers_valid-value', '1');
});
});
88 changes: 86 additions & 2 deletions packages/ra-ui-materialui/src/input/AutocompleteInput.stories.tsx
Original file line number Diff line number Diff line change
@@ -1,16 +1,26 @@
import * as React from 'react';
import { Admin, AdminContext } from 'react-admin';
import { Resource, required, useCreate, useRecordContext } from 'ra-core';
import {
Resource,
required,
useCreate,
useRecordContext,
ListBase,
useListContext,
RecordContextProvider,
} from 'ra-core';
import { createMemoryHistory } from 'history';
import {
Dialog,
DialogContent,
TextField,
DialogActions,
Button,
Stack,
TextField,
Typography,
Box,
} from '@mui/material';
import fakeRestProvider from 'ra-data-fakerest';

import { Edit } from '../detail';
import { SimpleForm } from '../form';
Expand Down Expand Up @@ -791,3 +801,77 @@ export const EmptyText = () => (
<Resource name="books" edit={BookEditWithEmptyText} />
</Admin>
);

const nullishValuesFakeData = {
fans: [
{ id: 'null', name: 'null', prefers: null },
{ id: 'undefined', name: 'undefined', prefers: undefined },
{ id: 'empty-string', name: 'empty string', prefers: '' },
{ id: 'zero-string', name: '0', prefers: 0 },
{ id: 'zero-number', name: '0', prefers: '0' },
{ id: 'valid-value', name: '1', prefers: 1 },
],
artists: [{ id: 0 }, { id: 1 }],
};

const FanList = props => {
const { data } = useListContext();
return data ? (
<>
{data.map(fan => (
<RecordContextProvider value={fan}>
<Stack
direction="row"
alignItems="center"
sx={{ m: 1, width: '90%' }}
>
<Box sx={{ width: '320px' }}>
<Typography variant="body1">
<b>Fan #{fan.id}</b>
<br />
<code>{`${
fan.name
} [${typeof fan.prefers}]`}</code>
</Typography>
</Box>
<Box sx={{ flex: '1 1 100%' }}>
<SimpleForm toolbar={<></>}>
<AutocompleteInput
id={`prefers_${fan.id}`}
label={`prefers_${fan.id}`}
fullWidth
source="prefers"
optionText={option => option.id}
choices={nullishValuesFakeData.artists}
helperText={false}
/>
</SimpleForm>
</Box>
</Stack>
</RecordContextProvider>
))}
</>
) : (
<>Loading</>
);
};

export const NullishValuesSupport = () => {
return (
<AdminContext
dataProvider={fakeRestProvider(nullishValuesFakeData, false)}
>
<Typography variant="h6" gutterBottom>
Test nullish values
</Typography>
<Typography variant="body1">
Story demonstrating nullish values support: each fan specify a
preferred artist. The <code>prefer</code> value is evaluated
against artist IDs.
</Typography>
<ListBase resource="fans">
<FanList />
</ListBase>
</AdminContext>
);
};
12 changes: 9 additions & 3 deletions packages/ra-ui-materialui/src/input/AutocompleteInput.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -525,7 +525,7 @@ If you provided a React element for the optionText prop, you must also provide t
]);

const isOptionEqualToValue = (option, value) => {
return getChoiceValue(option) === getChoiceValue(value);
return String(getChoiceValue(option)) === String(getChoiceValue(value));
};

return (
Expand Down Expand Up @@ -740,11 +740,17 @@ const getSelectedItems = (
if (multiple) {
return (value || [])
.map(item =>
choices.find(choice => item === get(choice, optionValue))
choices.find(
choice => String(item) === String(get(choice, optionValue))
)
)
.filter(item => !!item);
}
return choices.find(choice => get(choice, optionValue) === value) || '';
return (
choices.find(
choice => String(get(choice, optionValue)) === String(value)
) || ''
);
};

const DefaultFilterToQuery = searchText => ({ q: searchText });
46 changes: 45 additions & 1 deletion packages/ra-ui-materialui/src/input/ReferenceArrayInput.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { DatagridInput } from './DatagridInput';
import { TextField } from '../field';
import { ReferenceArrayInput } from './ReferenceArrayInput';
import { SelectArrayInput } from './SelectArrayInput';
import { DifferentIdTypes } from './ReferenceArrayInput.stories';

describe('<ReferenceArrayInput />', () => {
const defaultProps = {
Expand Down Expand Up @@ -85,7 +86,10 @@ describe('<ReferenceArrayInput />', () => {
const dataProvider = testDataProvider({
getList: () =>
// @ts-ignore
Promise.resolve({ data: [{ id: 1 }, { id: 2 }], total: 2 }),
Promise.resolve({
data: [{ id: 1 }, { id: 2 }],
total: 2,
}),
});
render(
<AdminContext dataProvider={dataProvider}>
Expand Down Expand Up @@ -244,4 +248,44 @@ describe('<ReferenceArrayInput />', () => {
});
});
});

it('should support different types of ids', async () => {
render(<DifferentIdTypes />);
await screen.findByText('#1', {
selector: 'div.MuiChip-root .MuiChip-label',
});
expect(
screen.queryByText('#2', {
selector: 'div.MuiChip-root .MuiChip-label',
})
).not.toBeNull();
expect(
screen.queryByText('#3', { selector: 'div.MuiChip-root' })
).toBeNull();
});

it('should unselect a value when types of ids are different', async () => {
render(<DifferentIdTypes />);

const chip1 = await screen.findByText('#1', {
selector: '.MuiChip-label',
});
const chip2 = await screen.findByText('#2', {
selector: '.MuiChip-label',
});

if (chip2.nextSibling) fireEvent.click(chip2.nextSibling);
expect(
screen.queryByText('#2', {
selector: '.MuiChip-label',
})
).toBeNull();

if (chip1.nextSibling) fireEvent.click(chip1.nextSibling);
expect(
screen.queryByText('#1', {
selector: '.MuiChip-label',
})
).toBeNull();
});
});
Loading