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

RFC: Decouple data actions from views #2952

Closed
fzaninotto opened this issue Mar 4, 2019 · 1 comment
Closed

RFC: Decouple data actions from views #2952

fzaninotto opened this issue Mar 4, 2019 · 1 comment

Comments

@fzaninotto
Copy link
Member

Problem

The page components (List, Edit, etc.). dispatch crud actions on mount to recuperate the data from the data provider. These actions are data oriented, i.e. the crudGetList asks for a list of resources, the crudGetOne asks for a single record, etc. But we've added side effects to these actions (e.g. redirect to the list when the crudGetOne returns a 404) that are view oriented, i.e. they are only valid for a given view.

export const crudGetOne = (
    resource: string,
    id: Identifier,
    basePath: string,
    refresh: RefreshSideEffect = true
): CrudGetOneAction => ({
    type: CRUD_GET_ONE,
    payload: { id },
    meta: {
        resource,
        fetch: GET_ONE,
        basePath,
        onFailure: {
            notification: {
                body: 'ra.notification.item_doesnt_exist',
                level: 'warning',
            },
            redirectTo: 'list',
            refresh,
        },
    },
});

The problem occurs when trying to use the crud- actions for a custom component. When calling a crudGetOne to grab e.g. the basket of an order, the user doesn't want to be redirected to the list in case of 404 on the basket resource.

Solution

The side effects of the crud actions should be configurable, and by default, there should be none. Something like:

export const crudGetOne = (
    resource: string,
    id: Identifier,
    basePath: string,
    onSuccess: {},
    onFailure: {}
): CrudGetOneAction => ({
    type: CRUD_GET_ONE,
    payload: { id },
    meta: {
        resource,
        fetch: GET_ONE,
        basePath,
        onSuccess,
        onFailure,
    },
});

It would be up to the caller (the controller) to specify the side effects

// in EditController.js
    updateData(resource = this.props.resource, id = this.props.id) {
-       this.props.crudGetOne(resource, id, this.props.basePath);
+      this.props.crudGetOne(resource, id, this.props.basePath, {}, {
+           notification: {
+               body: 'ra.notification.item_doesnt_exist',
+               level: 'warning',
+           },
+           redirectTo: 'list',
+           refresh: true,
+       });
    }

This is a great improvement in decoupling the data layer and the vue layer, and to promote the reusability of our action creators. It's also a breaking change.

Thoughts?

@fzaninotto
Copy link
Member Author

As of v3-alpha.4, this is done. We no longer use the action creators (which include side effects) directly, but we use hooks and the controllers decide which side effects to call.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant