Desktop: Fixes #13844: OneNote importer: Fix wrong page version imported #13850
Conversation
…mporter/fix-page-versions-bug
| // TODO: It would make more sense to use the **last** revision, rather than | ||
| // the first to get the content root. However, doing so seems to return | ||
| // version history information, rather than the true content root. | ||
| // In the future, if there are issues related to importing the wrong versions | ||
| // of pages, look into this. | ||
| // .rev() |
There was a problem hiding this comment.
If this call to .rev() is left uncommented, the page-parsing logic fails with an "unexpected object type" error, where the object was expected to be of type PageMetadata, but was VersionHistoryMetadata.
The "TODO" comment has been added because, logically, it seems as though the content root should be determined by the last revision, which should have the most up-to-date data.
| let mut last_index = iterator.get_index(); | ||
| while let Some(current) = iterator.peek() { | ||
| if let FileNodeData::RevisionManifestEndFND = current { | ||
| iterator.next(); |
There was a problem hiding this comment.
Without this iterator.next(), the RevisionManifestList parser sees the RevisionManifestEndFND and only parses one revision. As a result, the imported revision lists only contained one revision and second/third/fourth/etc. revisions were ignored.
For context, the RevisionManifestList parser runs the Revision parser to build a list of revisions.
2 participants
Summary
This pull fixes an issue where Joplin imported old versions of notes exported from the OneNote desktop app. Previously, when an export file contained multiple revisions, Joplin would import content from the older revision.
May fix #13844.
Testing
This pull request adds a new automated test file that includes a note with two revisions. Prior to this pull request, the first (old) revision was imported. With this change, the second (new) revision is imported.