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

[RFR] Use hooks in controllers #3213

Merged
merged 6 commits into from
May 13, 2019
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
3 changes: 3 additions & 0 deletions cypress/integration/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ describe('Edit Page', () => {
it('should allow to update elements', () => {
EditPostPage.setInputValue('title', 'Lorem Ipsum');
EditPostPage.submit();
// Ensure react-admin has handled the update as it will redirect to the list page
// once done
cy.url().should(url => expect(url).to.match(/.*\/posts$/))
EditPostPage.navigate();
cy.get(EditPostPage.elements.input('title')).should(el =>
expect(el).to.have.value('Lorem Ipsum')
Expand Down
6 changes: 4 additions & 2 deletions packages/ra-core/src/actions/listActions.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { Identifier } from '../types';

export const CRUD_CHANGE_LIST_PARAMS = 'RA/CRUD_CHANGE_LIST_PARAMS';

export interface ListParams {
Expand Down Expand Up @@ -27,12 +29,12 @@ export const SET_LIST_SELECTED_IDS = 'RA/SET_LIST_SELECTED_IDS';

export interface SetListSelectedIdsAction {
readonly type: typeof SET_LIST_SELECTED_IDS;
readonly payload: [];
readonly payload: Identifier[];
readonly meta: { resource: string };
}
export const setListSelectedIds = (
resource: string,
ids: []
ids: Identifier[]
): SetListSelectedIdsAction => ({
type: SET_LIST_SELECTED_IDS,
payload: ids,
Expand Down
114 changes: 36 additions & 78 deletions packages/ra-core/src/controller/CreateController.spec.tsx
Original file line number Diff line number Diff line change
@@ -1,95 +1,53 @@
import React from 'react';
import { shallow } from 'enzyme';

import { UnconnectedCreateController as CreateController } from './CreateController';
import { getRecord } from './CreateController';

describe('CreateController', () => {
describe('Presetting the record from the location', () => {
const defaultProps = {
basePath: '',
crudCreate: jest.fn(),
hasCreate: true,
hasEdit: true,
hasList: true,
hasShow: true,
isLoading: false,
location: {
pathname: '/foo',
search: undefined,
state: undefined,
hash: undefined,
},
match: { isExact: true, path: '/foo', params: undefined, url: '' },
resource: 'foo',
title: 'Foo',
translate: x => x,
describe('getRecord', () => {
const location = {
pathname: '/foo',
search: undefined,
state: undefined,
hash: undefined,
};

it('should return an empty record by default', () => {
const childrenMock = jest.fn();
const props = {
...defaultProps,
children: childrenMock,
};

shallow(<CreateController {...props} />);
expect(childrenMock).toHaveBeenCalledWith(
expect.objectContaining({ record: {} })
);
expect(getRecord(location, undefined)).toEqual({});
});

it('should return location state record when set', () => {
const childrenMock = jest.fn();
const props = {
...defaultProps,
children: childrenMock,
location: {
...defaultProps.location,
state: { record: { foo: 'bar' } },
},
};

shallow(<CreateController {...props} />);
expect(childrenMock).toHaveBeenCalledWith(
expect.objectContaining({ record: { foo: 'bar' } })
);
expect(
getRecord(
{
...location,
state: { record: { foo: 'bar' } },
},
undefined
)
).toEqual({ foo: 'bar' });
});

it('should return location search when set', () => {
const childrenMock = jest.fn();
const props = {
...defaultProps,
children: childrenMock,
location: {
...defaultProps.location,
search: '?foo=baz&array[]=1&array[]=2',
},
};

shallow(<CreateController {...props} />);
expect(childrenMock).toHaveBeenCalledWith(
expect.objectContaining({
record: { foo: 'baz', array: ['1', '2'] },
})
);
expect(
getRecord(
{
...location,
search: '?foo=baz&array[]=1&array[]=2',
},
undefined
)
).toEqual({ foo: 'baz', array: ['1', '2'] });
});

it('should return location state record when both state and search are set', () => {
const childrenMock = jest.fn();
const props = {
...defaultProps,
children: childrenMock,
location: {
...defaultProps.location,
state: { record: { foo: 'bar' } },
search: '?foo=baz',
},
};

shallow(<CreateController {...props} />);
expect(childrenMock).toHaveBeenCalledWith(
expect.objectContaining({ record: { foo: 'bar' } })
);
expect(
getRecord(
{
...location,
state: { record: { foo: 'bar' } },
search: '?foo=baz',
},
undefined
)
).toEqual({ foo: 'bar' });
});
});
});
165 changes: 69 additions & 96 deletions packages/ra-core/src/controller/CreateController.tsx
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
import { Component, ReactNode, ComponentType } from 'react';
import { connect } from 'react-redux';
import compose from 'recompose/compose';
import { ReactNode, useCallback } from 'react';
// @ts-ignore
import { useDispatch, useSelector } from 'react-redux';
import inflection from 'inflection';
import { parse } from 'query-string';

import withTranslate from '../i18n/translate';
import { crudCreate as crudCreateAction } from '../actions';
import checkMinimumRequiredProps from './checkMinimumRequiredProps';
import { crudCreate } from '../actions';
import { useCheckMinimumRequiredProps } from './checkMinimumRequiredProps';
import { Location } from 'history';
import { match as Match } from 'react-router';
import { Record, Translate, Dispatch } from '../types';
import { Record, Translate, ReduxState } from '../types';
import { RedirectionSideEffect } from '../sideEffect';
import { useTranslate } from '../i18n';

interface ChildrenFuncParams {
isLoading: boolean;
Expand All @@ -36,12 +36,6 @@ interface Props {
resource: string;
}

interface EnhancedProps {
crudCreate: Dispatch<typeof crudCreateAction>;
isLoading: boolean;
translate: Translate;
}

/**
* Page component for the Create view
*
Expand Down Expand Up @@ -83,95 +77,74 @@ interface EnhancedProps {
* );
* export default App;
*/
export class UnconnectedCreateController extends Component<
Props & EnhancedProps
> {
public static defaultProps: Partial<Props> = {
record: {},
};
const CreateController = (props: Props) => {
useCheckMinimumRequiredProps(
'Create',
['basePath', 'location', 'resource'],
props
);
const {
basePath,
children,
resource,
location,
record = {},
hasShow,
hasEdit,
} = props;

private record: Partial<Record>;
const translate = useTranslate();
const dispatch = useDispatch();
const recordToUse = getRecord(location, record);
const isLoading = useSelector(
(state: ReduxState) => state.admin.loading > 0
);

constructor(props) {
super(props);
const {
location: { state, search },
record,
} = this.props;
this.record =
Copy link
Member

Choose a reason for hiding this comment

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

I think you've broken the clone feature by not porting this

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The e2e tests disagree with you ;)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

And btw I ported this. This is the getRecord function you commented earlier

state && state.record
? state.record
: search
? parse(search, { arrayFormat: 'bracket' })
: record;
}
const save = useCallback(
(data: Partial<Record>, redirect: RedirectionSideEffect) => {
dispatch(crudCreate(resource, data, basePath, redirect));
},
[resource, basePath]
);

defaultRedirectRoute() {
const { hasShow, hasEdit } = this.props;
if (hasEdit) {
return 'edit';
}
if (hasShow) {
return 'show';
}
return 'list';
if (!children) {
return null;
}

save = (record: Partial<Record>, redirect: RedirectionSideEffect) => {
this.props.crudCreate(
this.props.resource,
record,
this.props.basePath,
redirect
);
};
const resourceName = translate(`resources.${resource}.name`, {
smart_count: 1,
_: inflection.humanize(inflection.singularize(resource)),
});
const defaultTitle = translate('ra.page.create', {
name: `${resourceName}`,
});
return children({
isLoading,
defaultTitle,
save,
resource,
basePath,
record: recordToUse,
redirect: getDefaultRedirectRoute(hasShow, hasEdit),
translate,
});
};

render() {
const {
basePath,
children,
isLoading,
resource,
translate,
} = this.props;
export default CreateController;

if (!children) {
return null;
}
export const getRecord = ({ state, search }, record: any = {}) =>
state && state.record
? state.record
: search
? parse(search, { arrayFormat: 'bracket' })
: record;

const resourceName = translate(`resources.${resource}.name`, {
smart_count: 1,
_: inflection.humanize(inflection.singularize(resource)),
});
const defaultTitle = translate('ra.page.create', {
name: `${resourceName}`,
});
return children({
isLoading,
defaultTitle,
save: this.save,
resource,
basePath,
record: this.record,
redirect: this.defaultRedirectRoute(),
translate,
});
const getDefaultRedirectRoute = (hasShow, hasEdit) => {
if (hasEdit) {
return 'edit';
}
}

function mapStateToProps(state) {
return {
isLoading: state.admin.loading > 0,
};
}

const CreateController = compose(
checkMinimumRequiredProps('Create', ['basePath', 'location', 'resource']),
connect(
mapStateToProps,
{ crudCreate: crudCreateAction }
),
withTranslate
)(UnconnectedCreateController);

export default CreateController as ComponentType<Props>;
if (hasShow) {
return 'show';
}
return 'list';
};
Loading