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

Fix behaviour of hydrateRelationships with dangling references #6040

Merged
merged 11 commits into from
Jul 2, 2021
Merged

Fix behaviour of hydrateRelationships with dangling references #6040

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
926783a
Fix behaviour of hydrateRelationships with dangling references
timleslie Jul 1, 2021
887ca6e
Update comment
timleslie Jul 1, 2021
f43866e
Update return value
timleslie Jul 1, 2021
918aa2e
Fix dep order
timleslie Jul 1, 2021
92a0da9
Add tests
timleslie Jul 1, 2021
e19a1f2
Add changeset
timleslie Jul 1, 2021
0776b08
Update types
timleslie Jul 1, 2021
46a4edc
WIP
timleslie Jul 1, 2021
44760e2
WIP
timleslie Jul 2, 2021
946e82c
Error handling
timleslie Jul 2, 2021
fb29c33
Clean up
timleslie Jul 2, 2021
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
Prev Previous commit
Next Next commit
WIP
  • Loading branch information
@timleslie
timleslie committed Jul 2, 2021
commit 44760e2ba446ad2555d667a25ea09b02159e0538
2 changes: 1 addition & 1 deletion packages-next/document-renderer/src/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ export const defaultRenderers: Renderers = {
superscript: 'sup',
underline: 'u',
relationship: ({ data }) => {
return <span>{data?.label}</span>;
return <span>{data?.label || data?.id}</span>;
},
},
block: {
Expand Down
2 changes: 1 addition & 1 deletion packages-next/fields-document/src/relationship-data.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ export function addRelationshipData(
const r = JSON.stringify(relationship);
console.error(`Unable to fetch relationship data: relationship: ${r}, id: ${id} `);
console.error(err);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm undecided as to whether these console error messages are helpful, or just noise 🤔

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, i don't think they're helpful though could you look at what it looks like to only do this on access control/does not exist? (the easiest way might be to do a many query and just get the first one from the array and handle if the array is empty?)

return { id, label: id, data: null };
return { id, data: null };
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return { id, data: null };
return { id };

}
return {
id,
Expand Down
8 changes: 4 additions & 4 deletions tests/api-tests/fields/types/document.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ describe('Document field type', () => {
};
content[1].children[1].data = { id: bob.id, label: 'Bob', data: { id: bob.id, name: 'Bob' } };
// Access denied on charlie;
content[2].children[1].data = { id: charlie.id, label: charlie.id, data: null };
content[2].children[1].data = { id: charlie.id, data: null };
expect(_post.content.document).toEqual(content);
})
);
Expand All @@ -203,9 +203,9 @@ describe('Document field type', () => {
data: { id: alice.id, name: 'Alice' },
};
// We expect the `data` field of the relationship to be null
content[1].children[1].data = { id: bob.id, label: bob.id, data: null };
content[1].children[1].data = { id: bob.id, data: null };
// Access denied on charlie;
content[2].children[1].data = { id: charlie.id, label: charlie.id, data: null };
content[2].children[1].data = { id: charlie.id, data: null };
expect(_post.content.document).toEqual(content);
})
);
Expand All @@ -223,7 +223,7 @@ describe('Document field type', () => {
// With no selection, we expect data to be an empty object
bio[0].children[1].data = { id: alice.id, label: 'Alice', data: {} };
// But still, and access-denied user will return data: null
bio[1].children[1].data = { id: charlie.id, label: charlie.id, data: null };
bio[1].children[1].data = { id: charlie.id, data: null };

expect(_dave.bio.document).toEqual(bio);
})
Expand Down