From f84e6cbbc6c3e7af596794831a1ff77a2733ef6f Mon Sep 17 00:00:00 2001 From: Gildas Garcia Date: Mon, 16 Sep 2019 18:11:03 +0200 Subject: [PATCH 1/2] Introduce useGetMatching and useReferenceArrayInputController --- .../ReferenceArrayInputController.spec.tsx | 417 ++++++++---------- .../input/ReferenceArrayInputController.tsx | 306 +++---------- .../controller/input/referenceDataStatus.ts | 11 +- .../input/useReferenceArrayInputController.ts | 145 ++++++ packages/ra-core/src/dataProvider/index.ts | 2 + .../src/dataProvider/useGetMatching.ts | 113 +++++ 6 files changed, 513 insertions(+), 481 deletions(-) create mode 100644 packages/ra-core/src/controller/input/useReferenceArrayInputController.ts create mode 100644 packages/ra-core/src/dataProvider/useGetMatching.ts diff --git a/packages/ra-core/src/controller/input/ReferenceArrayInputController.spec.tsx b/packages/ra-core/src/controller/input/ReferenceArrayInputController.spec.tsx index 3cff107b576..3b80b982570 100644 --- a/packages/ra-core/src/controller/input/ReferenceArrayInputController.spec.tsx +++ b/packages/ra-core/src/controller/input/ReferenceArrayInputController.spec.tsx @@ -1,97 +1,107 @@ import React from 'react'; -import assert from 'assert'; -import { shallow } from 'enzyme'; -import { UnconnectedReferenceArrayInputController as ReferenceArrayInputController } from './ReferenceArrayInputController'; +import expect from 'expect'; +import { cleanup, wait } from '@testing-library/react'; +import ReferenceArrayInputController from './ReferenceArrayInputController'; +import { renderWithRedux } from '../../util'; +import { DataProviderContext } from '../../../lib'; describe('', () => { const defaultProps = { children: jest.fn(), - crudGetMatching: () => true, - crudGetMany: () => true, input: { value: undefined }, - matchingReferences: [], - meta: {}, record: undefined, basePath: '/tags', reference: 'tags', resource: 'posts', source: 'tag_ids', - translate: x => `*${x}*`, }; + afterEach(cleanup); + it('should set loading to true as long as there are no references fetched and no selected references', () => { - const children = jest.fn(); - shallow( + const children = jest.fn(() =>
); + renderWithRedux( {children} ); - assert.equal(children.mock.calls[0][0].loading, true); + expect(children.mock.calls[0][0].loading).toEqual(true); }); it('should set loading to true as long as there are no references fetched and there are no data found for the references already selected', () => { - const children = jest.fn(); - shallow( + const children = jest.fn(() =>
); + renderWithRedux( {children} ); - assert.equal(children.mock.calls[0][0].loading, true); + expect(children.mock.calls[0][0].loading).toEqual(true); }); it('should set loading to false if the references are being searched but data from at least one selected reference was found', () => { - const children = jest.fn(); - shallow( + const children = jest.fn(() =>
); + renderWithRedux( {children} - + , + { + admin: { + resources: { + tags: { + data: { + 1: { + id: 1, + }, + }, + list: { + total: 42, + }, + }, + }, + }, + } ); - assert.equal(children.mock.calls[0][0].loading, false); - assert.deepEqual(children.mock.calls[0][0].choices, [{ id: 1 }]); + expect(children.mock.calls[0][0].loading).toEqual(false); + expect(children.mock.calls[0][0].choices).toEqual([{ id: 1 }]); }); - it('should set error in case of references fetch error and there are no selected reference in the input value', () => { - const children = jest.fn(); - shallow( - { + const children = jest.fn(({ error }) =>
{error}
); + const { queryByText } = renderWithRedux( + { + console.log(args); + return Promise.reject(); + })} > - {children} - + + {children} + + ); - assert.equal( - children.mock.calls[0][0].error, - '*ra.input.references.all_missing*' + + await wait(() => + expect( + queryByText('ra.input.references.all_missing') + ).not.toBeNull() ); }); it('should set error in case of references fetch error and there are no data found for the references already selected', () => { - const children = jest.fn(); - shallow( + const children = jest.fn(() =>
); + renderWithRedux( ', () => { {children} ); - assert.equal( - children.mock.calls[0][0].error, + expect(children.mock.calls[0][0].error).toEqual( '*ra.input.references.all_missing*' ); }); it('should not display an error in case of references fetch error but data from at least one selected reference was found', () => { - const children = jest.fn(); - shallow( + const children = jest.fn(() =>
); + renderWithRedux( ', () => { {children} ); - assert.equal(children.mock.calls[0][0].error, undefined); - assert.deepEqual(children.mock.calls[0][0].choices, [{ id: 2 }]); + expect(children.mock.calls[0][0].error).toEqual(undefined); + expect(children.mock.calls[0][0].choices).toEqual([{ id: 2 }]); }); it('should set warning if references fetch fails but selected references are not empty', () => { - const children = jest.fn(); - shallow( + const children = jest.fn(() =>
); + renderWithRedux( ', () => { {children} ); - assert.equal(children.mock.calls[0][0].warning, '*fetch error*'); + expect(children.mock.calls[0][0].warning).toEqual('*fetch error*'); }); it('should set warning if references were found but selected references are not complete', () => { - const children = jest.fn(); - shallow( + const children = jest.fn(() =>
); + renderWithRedux( ', () => { {children} ); - assert.equal( - children.mock.calls[0][0].warning, + expect(children.mock.calls[0][0].warning).toEqual( '*ra.input.references.many_missing*' ); }); it('should set warning if references were found but selected references are empty', () => { - const children = jest.fn(); - shallow( + const children = jest.fn(() =>
); + renderWithRedux( ', () => { {children} ); - assert.equal( - children.mock.calls[0][0].warning, + expect(children.mock.calls[0][0].warning).toEqual( '*ra.input.references.many_missing*' ); }); it('should not set warning if all references were found', () => { - const children = jest.fn(); - shallow( + const children = jest.fn(() =>
); + renderWithRedux( ', () => { {children} ); - assert.equal(children.mock.calls[0][0].warning, undefined); - }); - - it('should pass onChange down to child component', () => { - const onChange = jest.fn(); - const children = jest.fn(); - shallow( - - {children} - - ); - assert.equal(children.mock.calls[0][0].onChange, onChange); + expect(children.mock.calls[0][0].warning).toEqual(undefined); }); - it('should call crudGetMatching on mount with default fetch values', () => { + it.skip('should call crudGetMatching on mount with default fetch values', () => { const crudGetMatching = jest.fn(); - shallow( - + renderWithRedux( + ); - assert.deepEqual(crudGetMatching.mock.calls[0], [ + expect(crudGetMatching.mock.calls[0]).toEqual([ 'tags', 'posts@tag_ids', { @@ -240,19 +228,18 @@ describe('', () => { ]); }); - it('should allow to customize crudGetMatching arguments with perPage, sort, and filter props', () => { + it.skip('should allow to customize crudGetMatching arguments with perPage, sort, and filter props', () => { const crudGetMatching = jest.fn(); - shallow( + renderWithRedux( ); - assert.deepEqual(crudGetMatching.mock.calls[0], [ + expect(crudGetMatching.mock.calls[0]).toEqual([ 'tags', 'posts@tag_ids', { @@ -269,207 +256,181 @@ describe('', () => { ]); }); - it('should call crudGetMatching when setFilter is called', () => { - const crudGetMatching = jest.fn(); - const wrapper = shallow( - + it.skip('should call crudGetMatching when setFilter is called', () => { + const wrapper = renderWithRedux( + ); - wrapper.instance().setFilter('bar'); - assert.deepEqual(crudGetMatching.mock.calls[1], [ - 'tags', - 'posts@tag_ids', - { - page: 1, - perPage: 25, - }, - { - field: 'id', - order: 'DESC', - }, - { - q: 'bar', - }, - ]); + // wrapper.instance().setFilter('bar'); + // exp(crudGetMatching.mock.calls[1], [ + // 'tags', + // 'posts@tag_ids', + // { + // page: 1, + // perPage: 25, + // }, + // { + // field: 'id', + // order: 'DESC', + // }, + // { + // q: 'bar', + // }, + // ]); }); - it('should use custom filterToQuery function prop', () => { - const crudGetMatching = jest.fn(); - const wrapper = shallow( + it.skip('should use custom filterToQuery function prop', () => { + const wrapper = renderWithRedux( ({ foo: searchText })} /> ); - wrapper.instance().setFilter('bar'); - assert.deepEqual(crudGetMatching.mock.calls[1], [ - 'tags', - 'posts@tag_ids', - { - page: 1, - perPage: 25, - }, - { - field: 'id', - order: 'DESC', - }, - { - foo: 'bar', - }, - ]); + // wrapper.instance().setFilter('bar'); + // assert.deepEqual(crudGetMatching.mock.calls[1], [ + // 'tags', + // 'posts@tag_ids', + // { + // page: 1, + // perPage: 25, + // }, + // { + // field: 'id', + // order: 'DESC', + // }, + // { + // foo: 'bar', + // }, + // ]); }); - it('should call crudGetMany on mount if value is set', () => { - const crudGetMany = jest.fn(); - shallow( + it.skip('should call crudGetMany on mount if value is set', () => { + // const crudGetMany = jest.fn(); + renderWithRedux( ); - assert.deepEqual(crudGetMany.mock.calls[0], ['tags', [5, 6]]); + // expect(crudGetMany.mock.calls[0]).toEqual(['tags', [5, 6]]); }); - it('should only call crudGetMatching when calling setFilter', () => { - const crudGetMatching = jest.fn(); - const crudGetMany = jest.fn(); - const wrapper = shallow( + it.skip('should only call crudGetMatching when calling setFilter', () => { + const wrapper = renderWithRedux( ); - assert.equal(crudGetMatching.mock.calls.length, 1); - assert.equal(crudGetMany.mock.calls.length, 1); + // assert.equal(crudGetMatching.mock.calls.length, 1); + // assert.equal(crudGetMany.mock.calls.length, 1); - wrapper.instance().setFilter('bar'); - assert.equal(crudGetMatching.mock.calls.length, 2); - assert.equal(crudGetMany.mock.calls.length, 1); + // wrapper.instance().setFilter('bar'); + // assert.equal(crudGetMatching.mock.calls.length, 2); + // assert.equal(crudGetMany.mock.calls.length, 1); }); - it('should only call crudGetMatching when props are changed from outside', () => { - const crudGetMatching = jest.fn(); - const crudGetMany = jest.fn(); - const wrapper = shallow( + it.skip('should only call crudGetMatching when props are changed from outside', () => { + const wrapper = renderWithRedux( ); - assert.equal(crudGetMatching.mock.calls.length, 1); - assert.equal(crudGetMany.mock.calls.length, 1); + // assert.equal(crudGetMatching.mock.calls.length, 1); + // assert.equal(crudGetMany.mock.calls.length, 1); - wrapper.setProps({ filter: { foo: 'bar' } }); - assert.deepEqual(crudGetMatching.mock.calls[1], [ - 'tags', - 'posts@tag_ids', - { page: 1, perPage: 25 }, - { field: 'id', order: 'DESC' }, - { foo: 'bar' }, - ]); - assert.equal(crudGetMany.mock.calls.length, 1); + // wrapper.setProps({ filter: { foo: 'bar' } }); + // assert.deepEqual(crudGetMatching.mock.calls[1], [ + // 'tags', + // 'posts@tag_ids', + // { page: 1, perPage: 25 }, + // { field: 'id', order: 'DESC' }, + // { foo: 'bar' }, + // ]); + // assert.equal(crudGetMany.mock.calls.length, 1); - wrapper.setProps({ sort: { field: 'foo', order: 'ASC' } }); - assert.deepEqual(crudGetMatching.mock.calls[2], [ - 'tags', - 'posts@tag_ids', - { page: 1, perPage: 25 }, - { field: 'foo', order: 'ASC' }, - { foo: 'bar' }, - ]); - assert.equal(crudGetMany.mock.calls.length, 1); + // wrapper.setProps({ sort: { field: 'foo', order: 'ASC' } }); + // assert.deepEqual(crudGetMatching.mock.calls[2], [ + // 'tags', + // 'posts@tag_ids', + // { page: 1, perPage: 25 }, + // { field: 'foo', order: 'ASC' }, + // { foo: 'bar' }, + // ]); + // assert.equal(crudGetMany.mock.calls.length, 1); - wrapper.setProps({ perPage: 42 }); - assert.deepEqual(crudGetMatching.mock.calls[3], [ - 'tags', - 'posts@tag_ids', - { page: 1, perPage: 42 }, - { field: 'foo', order: 'ASC' }, - { foo: 'bar' }, - ]); - assert.equal(crudGetMany.mock.calls.length, 1); + // wrapper.setProps({ perPage: 42 }); + // assert.deepEqual(crudGetMatching.mock.calls[3], [ + // 'tags', + // 'posts@tag_ids', + // { page: 1, perPage: 42 }, + // { field: 'foo', order: 'ASC' }, + // { foo: 'bar' }, + // ]); + // assert.equal(crudGetMany.mock.calls.length, 1); }); - it('should call crudGetMany when input value changes', () => { - const crudGetMany = jest.fn(); - const wrapper = shallow( + it.skip('should call crudGetMany when input value changes', () => { + const wrapper = renderWithRedux( ); - assert.equal(crudGetMany.mock.calls.length, 1); - wrapper.setProps({ input: { value: [6] } }); - assert.equal(crudGetMany.mock.calls.length, 2); + // assert.equal(crudGetMany.mock.calls.length, 1); + // wrapper.setProps({ input: { value: [6] } }); + // assert.equal(crudGetMany.mock.calls.length, 2); }); - it('should call crudGetMany when input value changes only with the additional input values', () => { - const crudGetMany = jest.fn(); - const wrapper = shallow( + it.skip('should call crudGetMany when input value changes only with the additional input values', () => { + const wrapper = renderWithRedux( ); - expect( - crudGetMany.mock.calls[crudGetMany.mock.calls.length - 1] - ).toEqual([defaultProps.reference, [5]]); - wrapper.setProps({ input: { value: [5, 6] } }); - expect( - crudGetMany.mock.calls[crudGetMany.mock.calls.length - 1] - ).toEqual([defaultProps.reference, [6]]); + // expect( + // crudGetMany.mock.calls[crudGetMany.mock.calls.length - 1] + // ).toEqual([defaultProps.reference, [5]]); + // wrapper.setProps({ input: { value: [5, 6] } }); + // expect( + // crudGetMany.mock.calls[crudGetMany.mock.calls.length - 1] + // ).toEqual([defaultProps.reference, [6]]); }); - it('should not call crudGetMany when already fetched input value changes', () => { - const crudGetMany = jest.fn(); - const wrapper = shallow( + it.skip('should not call crudGetMany when already fetched input value changes', () => { + const wrapper = renderWithRedux( ); - expect(crudGetMany.mock.calls[0]).toEqual([ - defaultProps.reference, - [5, 6], - ]); - wrapper.setProps({ input: { value: [6] } }); - expect(crudGetMany.mock.calls.length).toEqual(1); + // expect(crudGetMany.mock.calls[0]).toEqual([ + // defaultProps.reference, + // [5, 6], + // ]); + // wrapper.setProps({ input: { value: [6] } }); + // expect(crudGetMany.mock.calls.length).toEqual(1); }); - it('should only call crudGetOne and not crudGetMatching when only the record changes', () => { - const crudGetMany = jest.fn(); - const crudGetMatching = jest.fn(); - const wrapper = shallow( + it.skip('should only call crudGetOne and not crudGetMatching when only the record changes', () => { + const wrapper = renderWithRedux( ); - assert.equal(crudGetMatching.mock.calls.length, 1); - assert.equal(crudGetMany.mock.calls.length, 1); - wrapper.setProps({ record: { id: 1 } }); - assert.equal(crudGetMatching.mock.calls.length, 2); - assert.equal(crudGetMany.mock.calls.length, 1); + // assert.equal(crudGetMatching.mock.calls.length, 1); + // assert.equal(crudGetMany.mock.calls.length, 1); + // wrapper.setProps({ record: { id: 1 } }); + // assert.equal(crudGetMatching.mock.calls.length, 2); + // assert.equal(crudGetMany.mock.calls.length, 1); }); }); diff --git a/packages/ra-core/src/controller/input/ReferenceArrayInputController.tsx b/packages/ra-core/src/controller/input/ReferenceArrayInputController.tsx index fb692721b1f..506134c86a5 100644 --- a/packages/ra-core/src/controller/input/ReferenceArrayInputController.tsx +++ b/packages/ra-core/src/controller/input/ReferenceArrayInputController.tsx @@ -1,34 +1,19 @@ -import { Component, ReactNode, ComponentType } from 'react'; -import { connect } from 'react-redux'; -import debounce from 'lodash/debounce'; -import compose from 'recompose/compose'; -import { createSelector } from 'reselect'; -import get from 'lodash/get'; -import isEqual from 'lodash/isEqual'; -import difference from 'lodash/difference'; - import { - crudGetMany as crudGetManyAction, - crudGetMatching as crudGetMatchingAction, -} from '../../actions/dataActions'; -import { - getPossibleReferences, - getPossibleReferenceValues, - getReferenceResource, -} from '../../reducer'; -import { getStatusForArrayInput as getDataStatus } from './referenceDataStatus'; -import withTranslate from '../../i18n/translate'; -import { Record, Sort, Translate, Pagination, Dispatch } from '../../types'; -import { MatchingReferencesError } from './types'; + ComponentType, + FunctionComponent, + ReactElement, + useCallback, +} from 'react'; +import debounce from 'lodash/debounce'; -const defaultReferenceSource = (resource: string, source: string) => - `${resource}@${source}`; +import { Record, Sort, Pagination } from '../../types'; +import useReferenceArrayInputController from './useReferenceArrayInputController'; interface ChildrenFuncParams { choices: Record[]; error?: string; + loaded: boolean; loading: boolean; - onChange: (value: any) => void; setFilter: (filter: any) => void; setPagination: (pagination: Pagination) => void; setSort: (sort: Sort) => void; @@ -38,29 +23,19 @@ interface ChildrenFuncParams { interface Props { allowEmpty?: boolean; basePath: string; - children: (params: ChildrenFuncParams) => ReactNode; + children: (params: ChildrenFuncParams) => ReactElement; filter?: object; - filterToQuery: (filter: {}) => any; + filterToQuery?: (filter: {}) => any; input?: any; meta?: object; perPage?: number; record?: Record; reference: string; - referenceSource: typeof defaultReferenceSource; resource: string; sort?: Sort; source: string; } -interface EnhancedProps { - crudGetMatching: Dispatch; - crudGetMany: Dispatch; - matchingReferences?: Record[] | MatchingReferencesError; - onChange?: () => void; - referenceRecords?: Record[]; - translate: Translate; -} - /** * An Input component for fields containing a list of references to another resource. * Useful for 'hasMany' relationship. @@ -139,218 +114,53 @@ interface EnhancedProps { * * */ -export class UnconnectedReferenceArrayInputController extends Component< - Props & EnhancedProps -> { - public static defaultProps = { - allowEmpty: false, - filter: {}, - filterToQuery: searchText => ({ q: searchText }), - matchingReferences: null, - perPage: 25, - sort: { field: 'id', order: 'DESC' }, - referenceRecords: [], - referenceSource: defaultReferenceSource, // used in unit tests - }; - - private params; - private debouncedSetFilter; - - constructor(props: Props & EnhancedProps) { - super(props); - const { perPage, sort, filter } = props; - // stored as a property rather than state because we don't want redraw of async updates - this.params = { pagination: { page: 1, perPage }, sort, filter }; - this.debouncedSetFilter = debounce(this.setFilter.bind(this), 500); - } - - componentDidMount() { - this.fetchReferencesAndOptions(this.props, {} as Props & EnhancedProps); - } - - componentWillReceiveProps(nextProps: Props & EnhancedProps) { - let shouldFetchOptions = false; - - if ( - (this.props.record || { id: undefined }).id !== - (nextProps.record || { id: undefined }).id - ) { - this.fetchReferencesAndOptions(nextProps); - } else if (this.props.input.value !== nextProps.input.value) { - this.fetchReferences(nextProps); - } else { - if (!isEqual(nextProps.filter, this.props.filter)) { - this.params = { ...this.params, filter: nextProps.filter }; - shouldFetchOptions = true; - } - if (!isEqual(nextProps.sort, this.props.sort)) { - this.params = { ...this.params, sort: nextProps.sort }; - shouldFetchOptions = true; - } - if (nextProps.perPage !== this.props.perPage) { - this.params = { - ...this.params, - pagination: { - ...this.params.pagination, - perPage: nextProps.perPage, - }, - }; - shouldFetchOptions = true; - } - } - if (shouldFetchOptions) { - this.fetchOptions(); - } - } - - setFilter = (filter: any) => { - if (filter !== this.params.filter) { - this.params.filter = this.props.filterToQuery(filter); - this.fetchOptions(); - } - }; - - setPagination = (pagination: Pagination) => { - if (pagination !== this.params.pagination) { - this.params.pagination = pagination; - this.fetchOptions(); - } - }; - - setSort = (sort: Sort) => { - if (sort !== this.params.sort) { - this.params.sort = sort; - this.fetchOptions(); - } - }; - - fetchReferences = (nextProps, currentProps = this.props) => { - const { crudGetMany, input, reference } = nextProps; - const ids = input.value; - if (ids) { - if (!Array.isArray(ids)) { - throw Error( - 'The value of ReferenceArrayInput should be an array' - ); - } - const idsToFetch = difference( - ids, - get(currentProps, 'input.value', []) - ); - if (idsToFetch.length) crudGetMany(reference, idsToFetch); - } - }; - - fetchOptions = (props = this.props) => { - const { - crudGetMatching, - reference, - source, - resource, - referenceSource, - filter: defaultFilter, - } = props; - const { pagination, sort, filter } = this.params; - crudGetMatching( - reference, - referenceSource(resource, source), - pagination, - sort, - { - ...filter, - ...defaultFilter, - } - ); - }; - - fetchReferencesAndOptions(nextProps, currentProps = this.props) { - this.fetchReferences(nextProps, currentProps); - this.fetchOptions(nextProps); - } - - render() { - const { - input, - referenceRecords, - matchingReferences, - onChange, - children, - translate, - } = this.props; - - const dataStatus = getDataStatus({ - input, - matchingReferences, - referenceRecords, - translate, - }); - - return children({ - choices: dataStatus.choices, - error: dataStatus.error, - loading: dataStatus.waiting, - onChange, - setFilter: this.debouncedSetFilter, - setPagination: this.setPagination, - setSort: this.setSort, - warning: dataStatus.warning, - }); - } -} - -const makeMapStateToProps = () => - createSelector( - [ - getReferenceResource, - getPossibleReferenceValues, - (_, { resource, input }) => { - const { value: referenceIds } = input; - - if (!referenceIds) { - return []; - } - - if (Array.isArray(referenceIds)) { - return referenceIds; - } - - throw new Error( - ` expects value to be an array, but the value passed as '${resource}.${ - input.name - }' is type '${typeof referenceIds}': ${referenceIds}` - ); - }, - ], - (referenceState, possibleValues, inputIds) => ({ - matchingReferences: getPossibleReferences( - referenceState, - possibleValues, - inputIds - ), - referenceRecords: - referenceState && - inputIds.reduce((references, referenceId) => { - if (referenceState.data[referenceId]) { - references.push(referenceState.data[referenceId]); - } - return references; - }, []), - }) - ); - -const ReferenceArrayInputController = compose( - withTranslate, - connect( - makeMapStateToProps(), - { - crudGetMany: crudGetManyAction, - crudGetMatching: crudGetMatchingAction, - } - ) -)(UnconnectedReferenceArrayInputController); - -ReferenceArrayInputController.defaultProps = { - referenceSource: defaultReferenceSource, // used in makeMapStateToProps +const ReferenceArrayInputController: FunctionComponent = ({ + basePath, + children, + filter = {}, + input, + filterToQuery = searchText => ({ q: searchText }), + perPage = 25, + reference, + resource, + sort = { field: 'id', order: 'DESC' }, + source, +}) => { + const { + choices, + error, + loaded, + loading, + setFilter, + setPagination, + setSort, + warning, + } = useReferenceArrayInputController({ + basePath, + filter, + filterToQuery, + input, + perPage, + sort, + reference, + resource, + source, + }); + + const debouncedSetFilter = useCallback(debounce(setFilter, 500), [ + setFilter, + ]); + + return children({ + choices, + error, + loaded, + loading, + setFilter: debouncedSetFilter, + setPagination, + setSort, + warning, + }); }; export default ReferenceArrayInputController as ComponentType; diff --git a/packages/ra-core/src/controller/input/referenceDataStatus.ts b/packages/ra-core/src/controller/input/referenceDataStatus.ts index b58431bf310..2677b323acb 100644 --- a/packages/ra-core/src/controller/input/referenceDataStatus.ts +++ b/packages/ra-core/src/controller/input/referenceDataStatus.ts @@ -86,10 +86,10 @@ export const getStatusForArrayInput = ({ referenceRecords, translate = x => x, }: GetStatusForArrayInputParams) => { - // selectedReferencesData can be "empty" (no data was found for references from input.value) + // selectedReferencesDataStatus can be "empty" (no data was found for references from input.value) // or "incomplete" (Not all of the reference data was found) // or "ready" (all references data was found or there is no references from input.value) - const selectedReferencesData = getSelectedReferencesStatus( + const selectedReferencesDataStatus = getSelectedReferencesStatus( input, referenceRecords ); @@ -106,20 +106,21 @@ export const getStatusForArrayInput = ({ waiting: (!matchingReferences && input.value && - selectedReferencesData === REFERENCES_STATUS_EMPTY) || + selectedReferencesDataStatus === REFERENCES_STATUS_EMPTY) || (!matchingReferences && !input.value), error: matchingReferencesError && (!input.value || (input.value && - selectedReferencesData === REFERENCES_STATUS_EMPTY)) + selectedReferencesDataStatus === REFERENCES_STATUS_EMPTY)) ? translate('ra.input.references.all_missing', { _: 'ra.input.references.all_missing', }) : null, warning: matchingReferencesError || - (input.value && selectedReferencesData !== REFERENCES_STATUS_READY) + (input.value && + selectedReferencesDataStatus !== REFERENCES_STATUS_READY) ? matchingReferencesError || translate('ra.input.references.many_missing', { _: 'ra.input.references.many_missing', diff --git a/packages/ra-core/src/controller/input/useReferenceArrayInputController.ts b/packages/ra-core/src/controller/input/useReferenceArrayInputController.ts new file mode 100644 index 00000000000..0797d4afa1c --- /dev/null +++ b/packages/ra-core/src/controller/input/useReferenceArrayInputController.ts @@ -0,0 +1,145 @@ +import { useMemo, useState } from 'react'; + +import { Record, Pagination, Sort } from '../../types'; +import { useGetMany } from '../../dataProvider'; +import { FieldInputProps } from 'react-final-form'; +import useGetMatching from '../../dataProvider/useGetMatching'; +import { useTranslate } from '../../i18n'; +import { getStatusForArrayInput as getDataStatus } from './referenceDataStatus'; + +/** + * @typedef ReferenceArrayProps + * @type {Object} + * @property {Array} ids the list of ids. + * @property {Object} data Object holding the reference data by their ids + * @property {Object} error the error returned by the dataProvider + * @property {boolean} loading is the reference currently loading + * @property {boolean} loaded has the reference already been loaded + * @property {string} referenceBasePath basePath of the reference + */ +interface ReferenceArrayInputProps { + choices: Record[]; + error?: any; + warning?: any; + loading: boolean; + loaded: boolean; + referenceBasePath: string; + setFilter: (filter: any) => void; + setPagination: (pagination: Pagination) => void; + setSort: (sort: Sort) => void; +} + +interface Option { + basePath: string; + filter?: any; + filterToQuery?: (filter: any) => any; + input: FieldInputProps; + options?: any; + perPage?: number; + record?: Record; + reference: string; + resource: string; + sort?: Sort; + source: string; +} + +/** + * Prepare data for the ReferenceArrayInput components + * + * @example + * + * const { choices, error, loaded, loading, referenceBasePath } = useReferenceArrayInputController({ + * basePath: 'resource'; + * record: { referenceIds: ['id1', 'id2']}; + * reference: 'reference'; + * resource: 'resource'; + * source: 'referenceIds'; + * }); + * + * @param {Object} option + * @param {boolean} option.allowEmpty do we allow for no referenced record (default to false) + * @param {string} option.basePath basepath to current resource + * @param {string | false} option.linkType The type of the link toward the referenced record. edit, show of false for no link (default to edit) + * @param {Object} option.record The The current resource record + * @param {string} option.reference The linked resource name + * @param {string} option.resource The current resource name + * @param {string} option.source The key of the linked resource identifier + * + * @return {Object} controllerProps Fetched data and callbacks for the ReferenceArrayInput components + */ +const useReferenceArrayInputController = ({ + basePath, + filter: defaultFilter, + filterToQuery = searchText => ({ q: searchText }), + input, + perPage = 25, + sort: defaultSort = { field: 'id', order: 'DESC' }, + options, + reference, + resource, + source, +}: Option): ReferenceArrayInputProps => { + const translate = useTranslate(); + + const [pagination, setPagination] = useState({ page: 1, perPage }); + const [sort, setSort] = useState(defaultSort); + const [filter, setFilter] = useState(''); + + const { data: referenceRecords, loaded } = useGetMany( + reference, + input.value || [] + ); + + const finalFilter = useMemo( + () => ({ + ...defaultFilter, + ...filterToQuery(filter), + }), + [defaultFilter, filter, filterToQuery] + ); + + const { data: matchingReferences } = useGetMatching( + reference, + pagination, + sort, + finalFilter, + source, + resource, + options + ); + + const finalReferenceRecords = referenceRecords.filter( + item => item != undefined + ); + + const finalMatchingReferences = + matchingReferences && matchingReferences.length > 0 + ? (matchingReferences || []).concat(finalReferenceRecords) + : finalReferenceRecords.length > 0 + ? finalReferenceRecords + : undefined; + + const dataStatus = getDataStatus({ + input, + // We merge the currently selected records with the matching ones, otherwise + // the component displaying the currently selected records may fail + matchingReferences: finalMatchingReferences, + referenceRecords: finalReferenceRecords, + translate, + }); + + const referenceBasePath = basePath.replace(resource, reference); // FIXME obviously very weak + return { + choices: dataStatus.choices, + error: dataStatus.error, + loaded, + loading: dataStatus.waiting, + referenceBasePath, + setFilter, + setPagination, + setSort, + warning: dataStatus.warning, + }; +}; + +export default useReferenceArrayInputController; diff --git a/packages/ra-core/src/dataProvider/index.ts b/packages/ra-core/src/dataProvider/index.ts index 2f2df64dc2f..ebf7f661155 100644 --- a/packages/ra-core/src/dataProvider/index.ts +++ b/packages/ra-core/src/dataProvider/index.ts @@ -13,6 +13,7 @@ import useGetOne from './useGetOne'; import useGetList from './useGetList'; import useGetMany from './useGetMany'; import useGetManyReference from './useGetManyReference'; +import useGetMatching from './useGetMatching'; import useUpdate from './useUpdate'; import useUpdateMany from './useUpdateMany'; import useCreate from './useCreate'; @@ -33,6 +34,7 @@ export { useGetList, useGetMany, useGetManyReference, + useGetMatching, useUpdate, useUpdateMany, useCreate, diff --git a/packages/ra-core/src/dataProvider/useGetMatching.ts b/packages/ra-core/src/dataProvider/useGetMatching.ts new file mode 100644 index 00000000000..e788fa8a0f2 --- /dev/null +++ b/packages/ra-core/src/dataProvider/useGetMatching.ts @@ -0,0 +1,113 @@ +import { useSelector } from 'react-redux'; +import { CRUD_GET_MATCHING } from '../actions/dataActions/crudGetMatching'; +import { GET_LIST } from '../dataFetchActions'; +import { Pagination, Sort, ReduxState } from '../types'; +import useQueryWithStore from './useQueryWithStore'; +import { + getReferenceResource, + getPossibleReferenceValues, + getPossibleReferences, +} from '../reducer'; + +const referenceSource = (resource, source) => `${resource}@${source}`; + +/** + * Call the dataProvider with a GET_LIST verb and return the result as well as the loading state. + * + * The return value updates according to the request state: + * + * - start: { loading: true, loaded: false } + * - success: { data: [data from store], ids: [ids from response], total: [total from response], loading: false, loaded: true } + * - error: { error: [error from response], loading: false, loaded: true } + * + * This hook will return the cached result when called a second time + * with the same parameters, until the response arrives. + * + * @param {string} resource The referenced resource name, e.g. 'tags' + * @param {Object} pagination The request pagination { page, perPage }, e.g. { page: 1, perPage: 10 } + * @param {Object} sort The request sort { field, order }, e.g. { field: 'id', order: 'DESC' } + * @param {Object} filters The request filters, e.g. { title: 'hello, world' } + * @param {string} source The field in resource containing the ids of the referenced records, e.g. 'tag_ids' + * @param {string} referencingResource The resource name, e.g. 'posts'. Used to build a cache key + * @param {Object} options Options object to pass to the dataProvider. May include side effects to be executed upon success of failure, e.g. { onSuccess: { refresh: true } } + * + * @returns The current request state. Destructure as { data, total, ids, error, loading, loaded }. + * + * @example + * + * import { useGetMatching } from 'react-admin'; + * + * const LatestNews = () => { + * const { data, ids, loading, error } = useGetList( + * 'posts', + * { page: 1, perPage: 10 }, + * { field: 'published_at', order: 'DESC' } + * ); + * if (loading) { return ; } + * if (error) { return

ERROR

; } + * return
    {ids.map(id => + *
  • {data[id].title}
    • + * )}
; + * }; + */ +const useGetMatching = ( + resource: string, + pagination: Pagination, + sort: Sort, + filter: object, + source: string, + referencingResource: string, + options?: any +) => { + const { + data: possibleValues, + total, + error, + loading, + loaded, + } = useQueryWithStore( + { + type: GET_LIST, + resource, + payload: { pagination, sort, filter }, + }, + { + ...options, + relatedTo: referenceSource(referencingResource, source), + action: CRUD_GET_MATCHING, + }, + (state: ReduxState) => + getPossibleReferenceValues(state, { + referenceSource, + resource: referencingResource, + source, + }), + (state: ReduxState) => + state.admin.resources[resource] + ? state.admin.resources[resource].list.total + : null + ); + + const referenceState = useSelector(state => + getReferenceResource(state, { + reference: resource, + }) + ); + + const possibleReferences = getPossibleReferences( + referenceState, + possibleValues, + [] + ); + + return { + data: possibleReferences, + ids: possibleValues, + total, + error, + loading, + loaded, + }; +}; + +export default useGetMatching; From 9ef7ae7207355ae669e0186fb4ff9dc541a5c924 Mon Sep 17 00:00:00 2001 From: Gildas Garcia Date: Tue, 17 Sep 2019 11:53:56 +0200 Subject: [PATCH 2/2] Review & tests --- .../ReferenceArrayInputController.spec.tsx | 721 +++++++++++------- .../input/useReferenceArrayInputController.ts | 137 ++-- .../src/dataProvider/useGetMatching.ts | 18 +- 3 files changed, 557 insertions(+), 319 deletions(-) diff --git a/packages/ra-core/src/controller/input/ReferenceArrayInputController.spec.tsx b/packages/ra-core/src/controller/input/ReferenceArrayInputController.spec.tsx index 3b80b982570..6440ac76a51 100644 --- a/packages/ra-core/src/controller/input/ReferenceArrayInputController.spec.tsx +++ b/packages/ra-core/src/controller/input/ReferenceArrayInputController.spec.tsx @@ -1,13 +1,12 @@ import React from 'react'; import expect from 'expect'; -import { cleanup, wait } from '@testing-library/react'; +import { cleanup, wait, fireEvent } from '@testing-library/react'; import ReferenceArrayInputController from './ReferenceArrayInputController'; import { renderWithRedux } from '../../util'; -import { DataProviderContext } from '../../../lib'; +import { CRUD_GET_MATCHING, CRUD_GET_MANY } from '../../../lib'; describe('', () => { const defaultProps = { - children: jest.fn(), input: { value: undefined }, record: undefined, basePath: '/tags', @@ -19,8 +18,10 @@ describe('', () => { afterEach(cleanup); it('should set loading to true as long as there are no references fetched and no selected references', () => { - const children = jest.fn(() =>
); - renderWithRedux( + const children = jest.fn(({ loading }) => ( +
{loading.toString()}
+ )); + const { queryByText } = renderWithRedux( ', () => { ); - expect(children.mock.calls[0][0].loading).toEqual(true); + expect(queryByText('true')).not.toBeNull(); }); it('should set loading to true as long as there are no references fetched and there are no data found for the references already selected', () => { - const children = jest.fn(() =>
); - renderWithRedux( + const children = jest.fn(({ loading }) => ( +
{loading.toString()}
+ )); + const { queryByText } = renderWithRedux( ', () => { {children} ); - expect(children.mock.calls[0][0].loading).toEqual(true); + expect(queryByText('true')).not.toBeNull(); }); it('should set loading to false if the references are being searched but data from at least one selected reference was found', () => { - const children = jest.fn(() =>
); - renderWithRedux( + const children = jest.fn(({ loading }) => ( +
{loading.toString()}
+ )); + const { queryByText } = renderWithRedux( {children} , @@ -73,364 +76,558 @@ describe('', () => { }, } ); - expect(children.mock.calls[0][0].loading).toEqual(false); - expect(children.mock.calls[0][0].choices).toEqual([{ id: 1 }]); + + expect(queryByText('false')).not.toBeNull(); }); it('should set error in case of references fetch error and there are no selected reference in the input value', async () => { const children = jest.fn(({ error }) =>
{error}
); + const { queryByText } = renderWithRedux( - { - console.log(args); - return Promise.reject(); - })} - > - - {children} - - + + {children} + , + { + admin: { + references: { + possibleValues: { + 'posts@tag_ids': { error: 'boom' }, + }, + }, + }, + } ); - await wait(() => - expect( - queryByText('ra.input.references.all_missing') - ).not.toBeNull() - ); + expect(queryByText('ra.input.references.all_missing')).not.toBeNull(); }); it('should set error in case of references fetch error and there are no data found for the references already selected', () => { - const children = jest.fn(() =>
); - renderWithRedux( + const children = jest.fn(({ error }) =>
{error}
); + const { queryByText } = renderWithRedux( {children} - - ); - expect(children.mock.calls[0][0].error).toEqual( - '*ra.input.references.all_missing*' + , + { + admin: { + references: { + possibleValues: { + 'posts@tag_ids': { error: 'boom' }, + }, + }, + }, + } ); + expect(queryByText('ra.input.references.all_missing')).not.toBeNull(); }); it('should not display an error in case of references fetch error but data from at least one selected reference was found', () => { - const children = jest.fn(() =>
); - renderWithRedux( + const children = jest.fn(({ error }) =>
{error}
); + const { queryByText } = renderWithRedux( {children} - + , + { + admin: { + resources: { + tags: { + data: { + 1: { + id: 1, + }, + }, + list: { + total: 42, + }, + }, + }, + references: { + possibleValues: { + 'posts@tag_ids': { error: 'boom' }, + }, + }, + }, + } ); - expect(children.mock.calls[0][0].error).toEqual(undefined); - expect(children.mock.calls[0][0].choices).toEqual([{ id: 2 }]); + expect(queryByText('ra.input.references.all_missing')).toBeNull(); }); it('should set warning if references fetch fails but selected references are not empty', () => { - const children = jest.fn(() =>
); - renderWithRedux( + const children = jest.fn(({ warning }) =>
{warning}
); + const { queryByText } = renderWithRedux( {children} - + , + { + admin: { + resources: { + tags: { + data: { + 1: { + id: 1, + }, + }, + list: { + total: 42, + }, + }, + }, + references: { + possibleValues: { + 'posts@tag_ids': { error: 'boom' }, + }, + }, + }, + } ); - expect(children.mock.calls[0][0].warning).toEqual('*fetch error*'); + expect(queryByText('ra.input.references.many_missing')).not.toBeNull(); }); it('should set warning if references were found but selected references are not complete', () => { - const children = jest.fn(() =>
); - renderWithRedux( + const children = jest.fn(({ warning }) =>
{warning}
); + const { queryByText } = renderWithRedux( {children} - - ); - expect(children.mock.calls[0][0].warning).toEqual( - '*ra.input.references.many_missing*' + , + { + admin: { + resources: { + tags: { + data: { + 1: { + id: 1, + }, + }, + list: { + total: 42, + }, + }, + }, + references: { + possibleValues: { + 'posts@tag_ids': [], + }, + }, + }, + } ); + expect(queryByText('ra.input.references.many_missing')).not.toBeNull(); }); it('should set warning if references were found but selected references are empty', () => { - const children = jest.fn(() =>
); - renderWithRedux( + const children = jest.fn(({ warning }) =>
{warning}
); + const { queryByText } = renderWithRedux( {children} - - ); - expect(children.mock.calls[0][0].warning).toEqual( - '*ra.input.references.many_missing*' + , + { + admin: { + references: { + possibleValues: { + 'posts@tag_ids': [], + }, + }, + }, + } ); + expect(queryByText('ra.input.references.many_missing')).not.toBeNull(); }); it('should not set warning if all references were found', () => { - const children = jest.fn(() =>
); - renderWithRedux( + const children = jest.fn(({ warning }) =>
{warning}
); + const { queryByText } = renderWithRedux( {children} - + , + { + admin: { + resources: { + tags: { + data: { + 1: { + id: 1, + }, + 2: { + id: 2, + }, + }, + list: { + total: 42, + }, + }, + }, + references: { + possibleValues: { + 'posts@tag_ids': [], + }, + }, + }, + } ); - expect(children.mock.calls[0][0].warning).toEqual(undefined); + expect(queryByText('ra.input.references.many_missing')).toBeNull(); }); - it.skip('should call crudGetMatching on mount with default fetch values', () => { - const crudGetMatching = jest.fn(); - renderWithRedux( - + it('should call crudGetMatching on mount with default fetch values', () => { + const children = jest.fn(() =>
); + + const { dispatch } = renderWithRedux( + + {children} + ); - expect(crudGetMatching.mock.calls[0]).toEqual([ - 'tags', - 'posts@tag_ids', - { - page: 1, - perPage: 25, + expect(dispatch.mock.calls[0][0]).toEqual({ + type: CRUD_GET_MATCHING, + meta: { + relatedTo: 'posts@tag_ids', + resource: 'tags', }, - { - field: 'id', - order: 'DESC', + payload: { + pagination: { + page: 1, + perPage: 25, + }, + sort: { + field: 'id', + order: 'DESC', + }, + filter: { q: '' }, }, - {}, - ]); + }); }); - it.skip('should allow to customize crudGetMatching arguments with perPage, sort, and filter props', () => { - const crudGetMatching = jest.fn(); - renderWithRedux( + it('should allow to customize crudGetMatching arguments with perPage, sort, and filter props', () => { + const children = jest.fn(() =>
); + + const { dispatch } = renderWithRedux( + filter={{ permanentFilter: 'foo' }} + > + {children} + ); - expect(crudGetMatching.mock.calls[0]).toEqual([ - 'tags', - 'posts@tag_ids', - { - page: 1, - perPage: 5, - }, - { - field: 'foo', - order: 'ASC', + expect(dispatch.mock.calls[0][0]).toEqual({ + type: CRUD_GET_MATCHING, + meta: { + relatedTo: 'posts@tag_ids', + resource: 'tags', }, - { - q: 'foo', + payload: { + pagination: { + page: 1, + perPage: 5, + }, + sort: { + field: 'foo', + order: 'ASC', + }, + filter: { permanentFilter: 'foo', q: '' }, }, - ]); + }); }); - it.skip('should call crudGetMatching when setFilter is called', () => { - const wrapper = renderWithRedux( - + it('should call crudGetMatching when setFilter is called', async () => { + const children = jest.fn(({ setFilter }) => ( +