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.

Sign up for GitHub

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

Jump to bottom

Correctly load Content field views when field access set to update: false #2151

Merged
merged 2 commits into from
Jan 14, 2020
Merged

Correctly load Content field views when field access set to update: false #2151

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1b9a5df
Correctly load Content field views when field access set to update: f…
jesstelford Dec 23, 2019
241c18f
Merge branch 'master' into fix-content-rendering-item-view
jesstelford Jan 14, 2020
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
5 changes: 5 additions & 0 deletions .changeset/swift-gorillas-switch.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@keystonejs/app-admin-ui': patch
---

Correctly load Content field views when field access set to update: false
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this the description for the other PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope, that's correct. Without this fix, the Content field will not load if you set update: false - it'll throw an error about deserialisation being called before it was loaded.

The other issue is #2150 which is how fields with update: false on them will just disapear completely from the Admin UI

138 changes: 70 additions & 68 deletions packages/app-admin-ui/client/pages/Item/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,17 +53,21 @@ 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) {
super(props);
// 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'
)
);
Expand Down Expand Up @@ -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}
Expand Down Expand Up @@ -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()),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reason it's important to do this here is later on line 452 we call deserializeItem which internally calls deserialize, which relies on the fields views having been loaded.

// 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.
Expand Down