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

UI-7417 - Handle nodes with missing content #25

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

rgosse
Copy link
Contributor

@rgosse rgosse commented Jun 2, 2022

If it happens that bookmarks are missing the content attribute in their ContentEntry, the migration tool now reports it gracefully and does not go down these node's structure. Doing so would trigger an error when resolving the path of these children and assuming that their parent are folders.

With no content we cannot determine the type of the node and are basically helpless.

@@ -180,6 +180,10 @@ const accumulateStructure = ({
folders,
path: [...path, id],
});
} else {
console.error(
`The 'structure' folder contains an object of id '${id}' that could not be identified (missing or corrupted entry in 'content'). Children of that object will be ignored.`
Copy link
Collaborator

@antoinetissier antoinetissier Jun 3, 2022

Choose a reason for hiding this comment

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

I get that we can warn about the missing entry in content here.
But if the entry is here, and just has a missing content, this is handled outside, isn't it ?
If so, I would remove the part about the corrupt content here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What I observed was that you would still attempt to resolve the structure of children of a corrupt node. Then, when trying to create the parent of that child in the new structure, you are not able to find the information about the parent in the legacy structure.

So you need this.

src/migrateUIFolder.ts Outdated Show resolved Hide resolved
Co-authored-by: Antoine Tissier <[email protected]>
@rgosse rgosse assigned antoinetissier and unassigned rgosse Jun 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants