Importer: Add core Simplenote format support #922

roundhill · 2018-10-16T19:26:10Z

Here's the first step towards getting an importer going. This module expects to receive a JSON object that contains note data in the same format as when you export your notes from the app (the notes.json file).

I'm slightly concerned about performance here with large imports, perhaps we could use a web worker? The exporter is crazy fast with thousands of notes, however, so maybe we don't need to worry about it.

To Test

Verify npm run test passes (I added some very basic tests)
Export some notes from the app, and grab the notes.json file and put it in the source code in the same folder as app.jsx.
Add import exportedJSON from './notes.json' and import importNotes from './utils/import'; to the top of app.jsx.
In render() of app.jsx (or anywhere you'd like) run importNotes(exportedJSON, this.props.noteBucket);
The notes should import to the app and sync!

Refs: #916

cc @dmsnell!

mirka

Thank you Dan! 🙂

Two things I noticed that are missing:

Is there a reason why the markdown systemTag was not included in the original export json? Seems like something that could/should be included like the pinned prop.
We also need to add the tags to the tagBucket.

mirka · 2018-10-17T20:41:17Z

lib/utils/import/index.js

+    throw new Error('No notes to import.');
+  }
+
+  if (!get(notes, 'activeNotes') || !get(notes, 'trashedNotes')) {


Suggested change

if (!get(notes, 'activeNotes') || !get(notes, 'trashedNotes')) {

if (!notes.activeNotes || !notes.trashedNotes) {

I’m wondering if this would be more readable and functionally equivalent in this case.

Also this should be an && not an || 😬. Will fix!

mirka · 2018-10-17T20:42:43Z

lib/utils/import/index.js

+  };
+
+  get(notes, 'activeNotes', []).map(note => importNote(note));
+  get(notes, 'trashedNotes', []).map(note => importNote(note));


I think .map(importNote) would be equivalent here.

mirka · 2018-10-17T20:44:00Z

lib/utils/import/index.js

+    importedNote.shareURL = '';
+    importedNote.systemTags = [];
+
+    importedNote.deleted = get(importedNote, 'deleted', false);


The deleted prop isn’t set on trashed notes in the export json, so nothing is being set to true here. We will have to explicitly set this prop to true with each note in trashedNotes.

Should we add deleted prop to the export?

The deleted property is required, or else Simperium will reject it.

I'm guessing that the deleted prop was not in the export because having that and also keeping a trashedNotes array would be redundant. We could flatten the export structure, but that would be disruptive for users who have export files with the old structure.

Explicitly setting back the deleted prop in the importer side should be ok, I think.

mirka · 2018-10-17T20:44:28Z

lib/utils/import/README.MD

+
+This is the core importer for Simplenote. It expects a JSON object of notes in the same format as the `import` module. It is aggressive about only allowing certain properties, and converts dates into timestamps where appropriate. Importing specific export files (Such as Evernote .enex) should use a separate conversion script and then pass the resulting JSON object to `importNotes()`.
+
+Example JSON:


Maybe the “Example JSON” should be “Example JSON with allowed properties”, and then remove the id prop?

mirka · 2018-10-17T20:46:32Z

lib/utils/import/test.js

+
+  it('should throw when invalid json is passed', () => {
+    const bogusNotes = { actveNotes: [] };
+    expect(importNotes.bind(importNotes, bogusNotes)).toThrow(


Out of curiosity, are these bind statements intended to do anything more than a simple expect(() => importNotes(null))? It had me wondering why a bind was necessary here.

I think it was executing the function during the test instead of evaluating it for the result. Hope that makes sense :)

Ok, if it's just to avoid the immediate call, then simply wrapping the call in a function (e.g. expect(() => importNotes(null)) ) should be sufficient.

Nice! Thanks.

dmsnell · 2018-10-17T21:32:22Z

How about we add some tag to indicate which import the notes came from? even if we leave it as a user-facing tag the customer could remove them if they wanted.

could make mess-ups significantly easier to deal with, like when dealing with duplicate notes or partial imports.

roundhill · 2018-10-17T22:46:57Z

We also need to add the tags to the tagBucket.

Good call!

roundhill · 2018-10-17T22:52:46Z

How about we add some tag to indicate which import the notes came from? even if we leave it as a user-facing tag the customer could remove them if they wanted.

Hmm. How about we add a systemTag for that? Then it wouldn't be user facing.

dmsnell · 2018-10-17T22:59:52Z

How about we add a systemTag for that? Then it wouldn't be user facing.

Seems just as good. I bet some people want to see it, others don't. System tag seems fine. importedFrom: [ 'some-export-filename.json', '2018-10-17T05:13:31-07:00Z' ] or something like that

Cleaner (and fixed!) check for no activeNotes or trashedNotes props.

… be marked deleted. Probably a better way to do this?

…r the note came from.

…it. Fixed bug where creationDate could be `NaN`.

roundhill · 2018-10-19T18:10:42Z

ToDo:

Via Add Markdown Field to the export json #938, we should import the markdown property to the systemTags if present.

Fixed date parsing to get correct timestamp.

…stead of throwing.

…ments.

mirka · 2018-10-27T22:14:34Z

Sorry I didn't catch this on the first pass, but this CoreImporter is actually not dealing with JSON at all (it just takes JavaScript objects as input), so we should probably remove mentions of JSON in the readme.

(So why is the import exportedJSON from './notes.json' working? TIL: webpack >= 2.0.0 has a json-loader enabled by default that parses the JSON string from a file into a JS object automatically 😮)

To allow users to import their JSON files, we're going to need a separate SimplenoteImporter to read from Files and pass the JSON.parsed result to the CoreImporter. (If we want to get the Evernote importer out the door, I guess we can add it later though? Doesn't seem like a must-have feature for the first iteration, and it will spread the surface area for testing.)

roundhill · 2018-10-29T15:42:01Z

CoreImporter is actually not dealing with JSON at all

Oh yeah, lol! I think I can change importNotes of CoreImporter to handle a File, in the same way the other importers do it. It'd take the file and load the file in as a string to JSON.parse(). Could be some memory issues with that, but Simplenote exports should be relatively small files for most users.

mirka · 2018-11-01T18:24:44Z

@roundhill I added some fixes to the tests and readme so this can be merged. Anything left on your side?

roundhill · 2018-11-01T18:34:10Z

I'm good!

roundhill added 2 commits October 16, 2018 12:12

Adding core importer.

b398d59

Adding trashedNotes import.

238ddd2

roundhill requested a review from mirka October 16, 2018 19:26

roundhill added the enhancement Improve existing functionality label Oct 16, 2018

mirka reviewed Oct 17, 2018

View reviewed changes

roundhill added 6 commits October 18, 2018 14:17

Pull in the systemTags if they're available (for markdown)

be383d1

Cleaner (and fixed!) check for no activeNotes or trashedNotes props.

Also import the tags to the tag bucket, if there's any present.

266ebf3

Added a boolean argument to determine whether an imported note should…

80fc441

… be marked deleted. Probably a better way to do this?

README cleanup!

96bed8a

Use fat arrow instead of bind in tests.

21d8a7e

Adding an optional argument to importNotes to signify which importe…

632ec9f

…r the note came from.

roundhill mentioned this pull request Oct 18, 2018

Add Markdown Field to the export json #938

Merged

Removing importedFrom property because the note schema doesn't allow …

12fa3aa

…it. Fixed bug where creationDate could be `NaN`.

roundhill mentioned this pull request Oct 19, 2018

Add Evernote Import Parser #940

Merged

1 task

roundhill added 3 commits October 22, 2018 10:46

Adding pinned and markdown props to the systemTags if present.

d9ef006

Fixed date parsing to get correct timestamp.

Wrapping the core importer in a class, which also now emits errors in…

7d1370a

…stead of throwing.

Add a default empty tags array if none was present in the imported note.

ec9aebb

roundhill mentioned this pull request Oct 24, 2018

Add Text File Importer #952

Merged

roundhill added this to the 1.2.2 milestone Oct 24, 2018

mirka mentioned this pull request Oct 26, 2018

Evernote importer #916

Closed

5 tasks

roundhill added 3 commits October 26, 2018 14:28

Constructor now takes an object so we don't muck up the bucket assign…

5921940

…ments.

Add a markdown override option to the core importer.

9ae9dbf

Add fallback object to fix exception if no options are passed.

d31ecbe

mirka mentioned this pull request Oct 27, 2018

Import dialog #957

Merged

3 tasks

mirka mentioned this pull request Oct 30, 2018

Add SimplenoteImporter #975

Merged

Fix tests

442a601

mirka added enhancement Improve existing functionality and removed enhancement Improve existing functionality labels Nov 1, 2018

Update readme

bd0fce5

mirka approved these changes Nov 1, 2018

View reviewed changes

roundhill merged commit 00c0cc0 into master Nov 1, 2018

mirka deleted the feature/import-parser branch December 26, 2018 17:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Importer: Add core Simplenote format support #922

Importer: Add core Simplenote format support #922

roundhill commented Oct 16, 2018 •

edited

Loading

mirka left a comment

mirka Oct 17, 2018

roundhill Oct 18, 2018 •

edited

Loading

mirka Oct 17, 2018

mirka Oct 17, 2018

roundhill Oct 17, 2018

roundhill Oct 17, 2018

mirka Oct 18, 2018

mirka Oct 17, 2018

mirka Oct 17, 2018

roundhill Oct 17, 2018

mirka Oct 18, 2018

roundhill Oct 18, 2018

dmsnell commented Oct 17, 2018

roundhill commented Oct 17, 2018

roundhill commented Oct 17, 2018

dmsnell commented Oct 17, 2018

roundhill commented Oct 19, 2018 •

edited

Loading

mirka commented Oct 27, 2018

roundhill commented Oct 29, 2018

mirka commented Nov 1, 2018

roundhill commented Nov 1, 2018

	if (!get(notes, 'activeNotes') \|\| !get(notes, 'trashedNotes')) {
	if (!notes.activeNotes \|\| !notes.trashedNotes) {


		This is the core importer for Simplenote. It expects a JSON object of notes in the same format as the `import` module. It is aggressive about only allowing certain properties, and converts dates into timestamps where appropriate. Importing specific export files (Such as Evernote .enex) should use a separate conversion script and then pass the resulting JSON object to `importNotes()`.

		Example JSON:

Importer: Add core Simplenote format support #922

Importer: Add core Simplenote format support #922

Conversation

roundhill commented Oct 16, 2018 • edited Loading

mirka left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

roundhill Oct 18, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dmsnell commented Oct 17, 2018

roundhill commented Oct 17, 2018

roundhill commented Oct 17, 2018

dmsnell commented Oct 17, 2018

roundhill commented Oct 19, 2018 • edited Loading

mirka commented Oct 27, 2018

roundhill commented Oct 29, 2018

mirka commented Nov 1, 2018

roundhill commented Nov 1, 2018

roundhill commented Oct 16, 2018 •

edited

Loading

roundhill Oct 18, 2018 •

edited

Loading

roundhill commented Oct 19, 2018 •

edited

Loading