From 1b9a5dfe4e9dfb235c809112c1c109a756a216cd Mon Sep 17 00:00:00 2001
From: Jess Telford <jess+github@jes.st>
Date: Mon, 23 Dec 2019 16:47:37 +1100
Subject: [PATCH] Correctly load Content field views when field access set to
 update: false

---
 .changeset/swift-gorillas-switch.md           |   5 +
 .../app-admin-ui/client/pages/Item/index.js   | 138 +++++++++---------
 2 files changed, 75 insertions(+), 68 deletions(-)
 create mode 100644 .changeset/swift-gorillas-switch.md

diff --git a/.changeset/swift-gorillas-switch.md b/.changeset/swift-gorillas-switch.md
new file mode 100644
index 00000000000..5d970ad77db
--- /dev/null
+++ b/.changeset/swift-gorillas-switch.md
@@ -0,0 +1,5 @@
+---
+'@keystonejs/app-admin-ui': patch
+---
+
+Correctly load Content field views when field access set to update: false
diff --git a/packages/app-admin-ui/client/pages/Item/index.js b/packages/app-admin-ui/client/pages/Item/index.js
index 4a1d77cfc0a..0f0be0dc573 100644
--- a/packages/app-admin-ui/client/pages/Item/index.js
+++ b/packages/app-admin-ui/client/pages/Item/index.js
@@ -53,6 +53,12 @@ const getCurrentValues = memoizeOne(getValues);
 
 const deserializeItem = memoizeOne((list, data) => list.deserializeItemData(data));
 
+const getRenderableFields = memoizeOne(list =>
+  list.fields
+    .filter(({ isPrimaryKey }) => !isPrimaryKey)
+    .filter(({ maybeAccess, config }) => !!maybeAccess.update || !!config.isReadOnly)
+);
+
 const ItemDetails = withRouter(
   class ItemDetails extends Component {
     constructor(props) {
@@ -60,10 +66,8 @@ const ItemDetails = withRouter(
       // memoized function so we can call it multiple times _per component_
       this.getFieldsObject = memoizeOne(() =>
         arrayToObject(
-          props.list.fields
-            .filter(({ isPrimaryKey }) => !isPrimaryKey)
-            .filter(({ isReadOnly }) => !isReadOnly)
-            .filter(({ maybeAccess }) => !!maybeAccess.update),
+          // NOTE: We _exclude_ read only fields
+          getRenderableFields(props.list).filter(({ config }) => !config.isReadOnly),
           'path'
         )
       );
@@ -295,66 +299,61 @@ const ItemDetails = withRouter(
           <Card css={{ marginBottom: '3em', paddingBottom: 0 }}>
             <Form>
               <AutocompleteCaptor />
-              {list.fields
-                .filter(({ isPrimaryKey }) => !isPrimaryKey)
-                .filter(({ maybeAccess, config }) => {
-                  return !!maybeAccess.update || config.isReadOnly;
-                })
-                .map((field, i) => (
-                  <Render key={field.path}>
-                    {() => {
-                      const [Field] = field.adminMeta.readViews([field.views.Field]);
-
-                      let onChange = useCallback(
-                        value => {
-                          this.setState(({ item: itm }) => ({
-                            item: {
-                              ...itm,
-                              [field.path]: value,
-                            },
-                            validationErrors: {},
-                            validationWarnings: {},
-                            itemHasChanged: true,
-                          }));
-                        },
-                        [field]
-                      );
-
-                      return useMemo(
-                        () => (
-                          <Field
-                            autoFocus={!i}
-                            field={field}
-                            list={list}
-                            item={item}
-                            errors={[
-                              ...(itemErrors[field.path] ? [itemErrors[field.path]] : []),
-                              ...(validationErrors[field.path] || []),
-                            ]}
-                            warnings={validationWarnings[field.path] || []}
-                            value={item[field.path]}
-                            savedValue={savedData[field.path]}
-                            onChange={onChange}
-                            renderContext="page"
-                            CreateItemModal={CreateItemModal}
-                          />
-                        ),
-                        [
-                          i,
-                          field,
-                          list,
-                          itemErrors[field.path],
-                          item[field.path],
-                          item.id,
-                          validationErrors[field.path],
-                          validationWarnings[field.path],
-                          savedData[field.path],
-                          onChange,
-                        ]
-                      );
-                    }}
-                  </Render>
-                ))}
+              {getRenderableFields(list).map((field, i) => (
+                <Render key={field.path}>
+                  {() => {
+                    const [Field] = field.adminMeta.readViews([field.views.Field]);
+
+                    let onChange = useCallback(
+                      value => {
+                        this.setState(({ item: itm }) => ({
+                          item: {
+                            ...itm,
+                            [field.path]: value,
+                          },
+                          validationErrors: {},
+                          validationWarnings: {},
+                          itemHasChanged: true,
+                        }));
+                      },
+                      [field]
+                    );
+
+                    return useMemo(
+                      () => (
+                        <Field
+                          autoFocus={!i}
+                          field={field}
+                          list={list}
+                          item={item}
+                          errors={[
+                            ...(itemErrors[field.path] ? [itemErrors[field.path]] : []),
+                            ...(validationErrors[field.path] || []),
+                          ]}
+                          warnings={validationWarnings[field.path] || []}
+                          value={item[field.path]}
+                          savedValue={savedData[field.path]}
+                          onChange={onChange}
+                          renderContext="page"
+                          CreateItemModal={CreateItemModal}
+                        />
+                      ),
+                      [
+                        i,
+                        field,
+                        list,
+                        itemErrors[field.path],
+                        item[field.path],
+                        item.id,
+                        validationErrors[field.path],
+                        validationWarnings[field.path],
+                        savedData[field.path],
+                        onChange,
+                      ]
+                    );
+                  }}
+                </Render>
+              ))}
             </Form>
             <Footer
               onSave={this.onSave}
@@ -413,10 +412,13 @@ const ItemPage = ({ list, itemId, adminPath, getListByKey }) => {
   // try to initialise the fields. They are Suspense capable, so may
   // throw Promises which will be caught by the wrapping <Suspense>
   captureSuspensePromises([
-    ...list.fields
-      .filter(({ isPrimaryKey }) => !isPrimaryKey)
-      .filter(({ maybeAccess }) => !!maybeAccess.update)
-      .map(field => () => field.initFieldView()),
+    // NOTE: We should really filter out non-renderable fields here, but there's
+    // an edgecase where deserialising will include all fields (even
+    // non-renderable ones), so we have to ensure the field views are all loaded
+    // even if they may not be used. This is particularly salient for the
+    // Content field which needs its views loaded to correctly deserialize.
+    // Ideally we'd use getRenderableFields() here.
+    ...list.fields.map(field => () => field.initFieldView()),
     // Deserialising requires the field be loaded and also any of its
     // deserialisation dependencies (eg; the Content field relies on the Blocks
     // being loaded), so it too could suspend here.