Repository: api-platform/admin
Pull request #217 (2019-09-18 13:50:08)

Fix edit and delete buttons
| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #214
| License       | MIT
| Doc PR        | n/a

Setting `undoable=false` fixes the issue... but disables the nice "undo" feature.

The problem seems to be a race condition between the `LIST` and the `UPDATE` operations. Whe `undoable` is enabled:

1. the `LIST` request is sent
2. The `UPDATE` request is sent with a delay
3. The result of `LIST` doesn't dispatch the function wrapped in `<Query>` as it should, and the loading screen never disappears.

@fzaninotto do you have an idea of how we can fix this?

Thanks to @mauchede for finding the underlying issue.


Patch:
diff --git a/package.json b/package.json
index 69236cc..10405fb 100644
--- a/package.json
+++ b/package.json
@@ -15,7 +15,7 @@
   "author": "Kévin Dunglas",
   "license": "MIT",
   "devDependencies": {
-    "@api-platform/api-doc-parser": "^0.8.1",
+    "@api-platform/api-doc-parser": "^0.7.2",
     "@babel/cli": "^7.2.3",
     "@babel/core": "^7.2.2",
     "@babel/plugin-proposal-class-properties": "^7.2.3",
@@ -40,14 +40,14 @@
     "prettier": "^1.15.3",
     "prop-types": "^15.6.2",
     "react": "~16.9.0",
-    "react-admin": "^2.9.0",
+    "react-admin": "^2.7.0",
     "react-dom": "~16.3.1",
     "react-redux": "~5.0.7",
     "react-test-renderer": "^16.8.6"
   },
   "peerDependencies": {
-    "react": "~16.0.0 || ~16.1.0 || ~16.2.0 || ~16.3.0 || ~16.4.0",
-    "react-dom": "~16.0.0 || ~16.1.0 || ~16.2.0 || ~16.3.0 || ~16.4.0"
+    "react": "~16.0.0 || ~16.1.0 || ~16.2.0 || ~16.3.0",
+    "react-dom": "~16.0.0 || ~16.1.0 || ~16.2.0 || ~16.3.0"
   },
   "scripts": {
     "build": "babel src -d lib --ignore '*.test.js'",
diff --git a/src/AdminGuesser.js b/src/AdminGuesser.js
index 6a31740..66644d3 100644
--- a/src/AdminGuesser.js
+++ b/src/AdminGuesser.js
@@ -1,4 +1,4 @@
-import React from 'react';
+import React, {Fragment} from 'react';
 import PropTypes from 'prop-types';
 import {Admin, Loading, Query, TranslationProvider} from 'react-admin';
 import {createMuiTheme} from '@material-ui/core/styles';
@@ -24,21 +24,21 @@ export const render = (
     return <div>Error while reading the API schema</div>;
   }
 
-  const childrenAsArray = React.Children.toArray(children);
-  data.resources
-    .filter(
-      resource =>
-        !resource.deprecated &&
-        !blacklist.includes(resource.name) &&
-        existsAsChild(children)(resource.name),
-    )
-    .forEach(resource =>
-      childrenAsArray.push(
-        <ResourceGuesser name={resource.name} key={resource.name} />,
-      ),
-    );
-
-  return <Admin {...props}>{childrenAsArray}</Admin>;
+  return (
+    <Admin {...props}>
+      {children || <Fragment />}
+      {data.resources
+        .filter(
+          resource =>
+            !resource.deprecated &&
+            !blacklist.includes(resource.name) &&
+            existsAsChild(children)(resource.name),
+        )
+        .map(resource => (
+          <ResourceGuesser name={resource.name} key={resource.name} />
+        ))}
+    </Admin>
+  );
 };
 
 const AdminGuesser = props => (
diff --git a/src/EditGuesser.js b/src/EditGuesser.js
index 9f559ec..4b496d4 100644
--- a/src/EditGuesser.js
+++ b/src/EditGuesser.js
@@ -6,10 +6,10 @@ import WithReactAdminQuery from './withReactAdminQuery';
 
 // useful for testing (we don't need Query)
 const EditGuesserComponent = props => {
-  const {children, fields} = props;
+  const {children, fields, resourceSchema, ...filteredProps} = props;
 
   return (
-    <Edit {...props}>
+    <Edit {...filteredProps} undoable={false}>
       <SimpleForm>
         {children}
         {fields.map(field => (
diff --git a/src/FilterGuesser.js b/src/FilterGuesser.js
index b0d87a4..6310b01 100644
--- a/src/FilterGuesser.js
+++ b/src/FilterGuesser.js
@@ -1,10 +1,10 @@
 import React from 'react';
-import {Filter, Loading, Query} from 'react-admin';
-import {getResource} from './docsUtils';
+import {Filter} from 'react-admin';
 import InputGuesser from './InputGuesser';
+import WithReactAdminQuery from './withReactAdminQuery';
 import {getFiltersParametersFromResourceSchema} from './docsUtils';
 
-class Filters extends React.Component {
+class FilterGuesserComponent extends React.Component {
   constructor(props) {
     super(props);
     this.state = {
@@ -28,18 +28,11 @@ class Filters extends React.Component {
   }
 
   render() {
-    const {
-      resourceSchema: resource,
-      fields: allowedFieldNames,
-      hasEdit,
-      hasShow,
-      ...props
-    } = this.props;
-
     if (!this.state.filtersParameters.length) {
       return null;
     }
 
+    const {resourceSchema, fields, hasShow, hasEdit, ...props} = this.props;
     return (
       <Filter {...props}>
         {this.state.filtersParameters.map(filter => (
@@ -54,37 +47,8 @@ class Filters extends React.Component {
   }
 }
 
-const FilterGuesser = props => {
-  const {resource: resourceName} = props;
-  return (
-    <Query type="INTROSPECT">
-      {({data: api, loading, error}) => {
-        if (loading) {
-          return <Loading />;
-        }
-
-        if (error) {
-          console.error(error);
-          return <div>Error while reading the API schema</div>;
-        }
-
-        const resourceSchema = getResource(api.resources, resourceName);
-
-        if (!resourceSchema || !resourceSchema.fields) {
-          console.error(
-            `Resource ${resourceName} not present inside api description`,
-          );
-          return (
-            <div>
-              Resource ${resourceName} not present inside api description
-            </div>
-          );
-        }
-
-        return <Filters resourceSchema={resourceSchema} {...props} />;
-      }}
-    </Query>
-  );
-};
+const FilterGuesser = props => (
+  <WithReactAdminQuery component={FilterGuesserComponent} {...props} />
+);
 
 export default FilterGuesser;
diff --git a/src/ListGuesser.js b/src/ListGuesser.js
index a2a4416..4da7102 100644
--- a/src/ListGuesser.js
+++ b/src/ListGuesser.js
@@ -1,26 +1,15 @@
 import React, {Children} from 'react';
 import PropTypes from 'prop-types';
-import {
-  Datagrid,
-  List,
-  Query,
-  EditButton,
-  ShowButton,
-  Loading,
-} from 'react-admin';
-import {existsAsChild, getResource} from './docsUtils';
+import {Datagrid, List, EditButton, ShowButton} from 'react-admin';
+import {getOrderParametersFromResourceSchema} from './docsUtils';
 import FieldGuesser from './FieldGuesser';
 import FilterGuesser from './FilterGuesser';
-import {getOrderParametersFromResourceSchema} from './docsUtils';
-
-const getFields = (
-  {fields},
-  allowedFieldNames = fields.map(defaultField => defaultField.name),
-) => fields.filter(({name}) => allowedFieldNames.includes(name));
+import WithReactAdminQuery from './withReactAdminQuery';
 
-class ResourcesList extends React.Component {
+class ListGuesserComponent extends React.Component {
   constructor(props) {
     super(props);
+
     this.state = {
       orderParameters: this.props.resourceSchema
         ? getOrderParametersFromResourceSchema(this.props.resourceSchema)
@@ -29,31 +18,22 @@ class ResourcesList extends React.Component {
   }
 
   componentDidMount() {
-    if (!this.state.orderParameters.length) {
-      this.props.resourceSchema.getParameters().then(parameters => {
-        this.setState({
-          orderParameters: getOrderParametersFromResourceSchema({
-            ...this.props.resourceSchema,
-            parameters,
-          }),
-        });
-      });
+    if (this.state.orderParameters.length) {
+      return;
     }
+
+    this.props.resourceSchema.getParameters().then(() => {
+      this.setState({
+        orderParameters: getOrderParametersFromResourceSchema(
+          this.props.resourceSchema,
+        ),
+      });
+    });
   }
 
   render() {
-    const {
-      resourceSchema: resource,
-      fields: allowedFieldNames,
-      ...props
-    } = this.props;
-
+    const {resourceSchema, fields, hasShow, hasEdit, ...props} = this.props;
     const children = Children.toArray(props.children);
-
-    const fields = getFields(resource, allowedFieldNames).filter(
-      existsAsChild(children),
-    );
-
     return (
       <List {...props}>
         <Datagrid>
@@ -65,47 +45,17 @@ class ResourcesList extends React.Component {
               sortable={this.state.orderParameters.includes(field.name)}
             />
           ))}
-          {props.hasShow && <ShowButton />}
-          {props.hasEdit && <EditButton />}
+          {hasShow && <ShowButton />}
+          {hasEdit && <EditButton />}
         </Datagrid>
       </List>
     );
   }
 }
 
-const ListGuesser = props => {
-  const {resource: resourceName} = props;
-
-  return (
-    <Query type="INTROSPECT">
-      {({data: api, loading, error}) => {
-        if (loading) {
-          return <Loading />;
-        }
-
-        if (error) {
-          console.error(error);
-          return <div>Error while reading the API schema</div>;
-        }
-
-        const resourceSchema = getResource(api.resources, resourceName);
-
-        if (!resourceSchema || !resourceSchema.fields) {
-          console.error(
-            `Resource ${resourceName} not present inside api description`,
-          );
-          return (
-            <div>
-              Resource ${resourceName} not present inside api description
-            </div>
-          );
-        }
-
-        return <ResourcesList resourceSchema={resourceSchema} {...props} />;
-      }}
-    </Query>
-  );
-};
+const ListGuesser = props => (
+  <WithReactAdminQuery component={ListGuesserComponent} {...props} />
+);
 
 export default ListGuesser;
 
diff --git a/src/hydra/hydraClient.js b/src/hydra/hydraClient.js
index 4a75637..d9e8710 100644
--- a/src/hydra/hydraClient.js
+++ b/src/hydra/hydraClient.js
@@ -165,11 +165,9 @@ export default (entrypoint, httpClient = fetchHydra) => {
       return Promise.resolve(data);
     }
 
-    return convertReactAdminDataToHydraData(resource, data).then(data => {
-      return undefined === resource.encodeData
-        ? JSON.stringify(data)
-        : resource.encodeData(data);
-    });
+    return convertReactAdminDataToHydraData(resource, data).then(data =>
+      JSON.stringify(data),
+    );
   };
 
   /**
diff --git a/src/withReactAdminQuery.js b/src/withReactAdminQuery.js
index fbe58a5..f2bf424 100644
--- a/src/withReactAdminQuery.js
+++ b/src/withReactAdminQuery.js
@@ -5,10 +5,6 @@ import {existsAsChild} from './docsUtils';
 const withReactAdminQuery = ({component: Component, ...props}) => (
   <Query type="INTROSPECT" resource={props.ressource}>
     {({data, loading, error}) => {
-      if (data == null) {
-        return null;
-      }
-
       if (loading) {
         return <Loading />;
       }
@@ -18,6 +14,10 @@ const withReactAdminQuery = ({component: Component, ...props}) => (
         return <div>Error while reading the API schema</div>;
       }
 
+      if (data == null) {
+        return null;
+      }
+
       const resourceSchema = data.resources.find(
         r => r.name === props.resource,
       );
@@ -41,6 +41,7 @@ const withReactAdminQuery = ({component: Component, ...props}) => (
         <Component
           data={data}
           resource={props.ressource}
+          resourceSchema={resourceSchema}
           fields={fields}
           {...props}
         />
diff --git a/yarn.lock b/yarn.lock
index c777a3a..bc899a4 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -2,12 +2,12 @@
 # yarn lockfile v1
 
 
-"@api-platform/api-doc-parser@^0.8.1":
-  version "0.8.1"
-  resolved "https://registry.yarnpkg.com/@api-platform/api-doc-parser/-/api-doc-parser-0.8.1.tgz#256cd7719d85b1f54440841a77679c19c9b97058"
-  integrity sha512-AfyeuQL58zjayiiB5fysY88twkDfGTWCYr7R/mtWxGJVXJHp0nJwaNWHQD00VzFYLoDXcDq1l/dZjOZA79YUAg==
+"@api-platform/api-doc-parser@^0.7.2":
+  version "0.7.2"
+  resolved "https://registry.yarnpkg.com/@api-platform/api-doc-parser/-/api-doc-parser-0.7.2.tgz#c5ca5913fa6ee319016a635df5225e0db5ecc9ab"
+  integrity sha512-FHzAldq1/ZZZhRyI8sgfxIrf3NtKLNgaW6P8Z12cjK3qWDuYCjsQZ1H2Z4WGZk0tGhDxA3lVJXcw4HHD5kjTrQ==
   dependencies:
-    "@babel/runtime" "^7.0.0"
+    babel-runtime "^6.23.0"
     jsonld "^1.5.0"
     lodash.get "^4.4.2"
     lodash.uniq "^4.5.0"
@@ -747,7 +747,7 @@
   dependencies:
     regenerator-runtime "^0.12.0"
 
-"@babel/runtime@^7.0.0", "@babel/runtime@^7.1.2", "@babel/runtime@^7.2.0", "@babel/runtime@^7.4.5":
+"@babel/runtime@^7.1.2", "@babel/runtime@^7.2.0", "@babel/runtime@^7.4.5":
   version "7.6.0"
   resolved "https://registry.yarnpkg.com/@babel/runtime/-/runtime-7.6.0.tgz#4fc1d642a9fd0299754e8b5de62c631cf5568205"
   integrity sha512-89eSBLJsxNxOERC0Op4vd+0Bqm6wRMqMbFtV3i0/fbaWw/mJ8Q3eBvgX0G4SyrOOLCtbu98HspF8o09MRT+KzQ==
@@ -1490,7 +1490,7 @@ babel-register@^6.26.0:
     mkdirp "^0.5.1"
     source-map-support "^0.4.15"
 
-babel-runtime@^6.22.0, babel-runtime@^6.26.0:
+babel-runtime@^6.22.0, babel-runtime@^6.23.0, babel-runtime@^6.26.0:
   version "6.26.0"
   resolved "https://registry.yarnpkg.com/babel-runtime/-/babel-runtime-6.26.0.tgz#965c7058668e82b55d7bfe04ff2337bc8b5647fe"
   integrity sha1-llxwWGaOgrVde/4E/yM3vItWR/4=
@@ -5169,7 +5169,7 @@ rdf-canonize@^1.0.2:
     node-forge "^0.8.1"
     semver "^5.6.0"
 
-react-admin@^2.9.0:
+react-admin@^2.7.0:
   version "2.9.6"
   resolved "https://registry.yarnpkg.com/react-admin/-/react-admin-2.9.6.tgz#ee5d4198a03d9cbc157a10ecaa901464b54ae3b6"
   integrity sha512-AhhsbJxabP9SaA1hBAY12fEVL0GxlBPld+vcVAlThtb0zy8HVlUxdD1pqKcNnWClFmKsgP0LFEalcZP+vcgkzQ==


Test patch:
diff --git a/src/__snapshots__/AdminGuesser.test.js.snap b/src/__snapshots__/AdminGuesser.test.js.snap
index c06f2cf..7468e21 100644
--- a/src/__snapshots__/AdminGuesser.test.js.snap
+++ b/src/__snapshots__/AdminGuesser.test.js.snap
@@ -20,6 +20,7 @@ exports[`<AdminGuesser /> renders without custom resources 1`] = `
   loginPage={[Function]}
   logoutButton={[Function]}
 >
+  <React.Fragment />
   <ResourceGuesser
     name="books"
   />