-
Notifications
You must be signed in to change notification settings - Fork 206
Add Reflect JSON importer #509
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
base: master
Are you sure you want to change the base?
Changes from 7 commits
910f956
1a720f3
13173d2
1ef9689
f87c50e
164ffed
fbd5e8c
88de74a
2df23f0
24ac0d8
36c2510
65a7193
10faf6d
a18e871
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,347 @@ | ||||||
| import { moment, normalizePath, Notice, requestUrl, Setting, TFile } from 'obsidian'; | ||||||
| import { parseFilePath } from '../filesystem'; | ||||||
| import { FormatImporter } from '../format-importer'; | ||||||
| import { ImportContext } from '../main'; | ||||||
| import { sanitizeFileName, serializeFrontMatter } from '../util'; | ||||||
| import { sanitizeTag } from './keep/util'; | ||||||
| import { ReflectExport, ReflectNote } from './reflect/models'; | ||||||
| import { convertDocument, ConvertOptions } from './reflect/convert'; | ||||||
|
|
||||||
| const MAX_FILENAME_LENGTH = 200; | ||||||
|
|
||||||
| function truncateTitle(title: string): string { | ||||||
|
||||||
| if (title.length <= MAX_FILENAME_LENGTH) return title; | ||||||
| return title.substring(0, MAX_FILENAME_LENGTH).trim(); | ||||||
| } | ||||||
|
|
||||||
| export class ReflectImporter extends FormatImporter { | ||||||
| downloadAttachments: boolean; | ||||||
| tagsFrontmatter: boolean; | ||||||
| dateFrontmatter: boolean; | ||||||
| titleFrontmatter: boolean; | ||||||
|
|
||||||
| init() { | ||||||
| // Initialize defaults in init() because FormatImporter calls init() from its constructor. | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This contradicts another change in this PR.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be consistent again now that the extra |
||||||
| this.downloadAttachments = false; | ||||||
| this.tagsFrontmatter = true; | ||||||
| this.dateFrontmatter = false; | ||||||
| this.titleFrontmatter = false; | ||||||
|
|
||||||
| this.addFileChooserSetting('Reflect (.json)', ['json']); | ||||||
| this.addOutputLocationSetting('Reflect'); | ||||||
|
|
||||||
| new Setting(this.modal.contentEl) | ||||||
| .setName('Import settings') | ||||||
| .setHeading(); | ||||||
|
|
||||||
| new Setting(this.modal.contentEl) | ||||||
| .setName('Download all attachments') | ||||||
| .setDesc('If enabled, all attachments uploaded to Reflect will be downloaded to your attachments folder.') | ||||||
| .addToggle(toggle => { | ||||||
| toggle.setValue(this.downloadAttachments); | ||||||
| toggle.onChange(async (value) => { | ||||||
| this.downloadAttachments = value === true; | ||||||
| }); | ||||||
| }); | ||||||
|
|
||||||
| new Setting(this.modal.contentEl) | ||||||
| .setName('Add YAML tags') | ||||||
| .setDesc('If enabled, notes will have tags from Reflect added as properties.') | ||||||
| .addToggle(toggle => { | ||||||
| toggle.setValue(this.tagsFrontmatter); | ||||||
| toggle.onChange(async (value) => { | ||||||
| this.tagsFrontmatter = value; | ||||||
| }); | ||||||
| }); | ||||||
|
|
||||||
| new Setting(this.modal.contentEl) | ||||||
| .setName('Add YAML created/updated date') | ||||||
| .setDesc('If enabled, notes will have the created and updated timestamps from Reflect added as properties.') | ||||||
| .addToggle(toggle => { | ||||||
| toggle.setValue(this.dateFrontmatter); | ||||||
| toggle.onChange(async (value) => { | ||||||
| this.dateFrontmatter = value; | ||||||
| }); | ||||||
| }); | ||||||
|
|
||||||
| new Setting(this.modal.contentEl) | ||||||
| .setName('Add YAML title') | ||||||
| .setDesc('If enabled, notes will have the full title added as a property (regardless of illegal file name characters).') | ||||||
| .addToggle(toggle => { | ||||||
| toggle.setValue(this.titleFrontmatter); | ||||||
| toggle.onChange(async (value) => { | ||||||
| this.titleFrontmatter = value; | ||||||
| }); | ||||||
| }); | ||||||
| } | ||||||
|
|
||||||
| private getUserDNPFormat(): string { | ||||||
| // @ts-expect-error : Internal Method | ||||||
| const plugin = this.app.internalPlugins.getPluginById('daily-notes'); | ||||||
| if (!plugin?.instance) { | ||||||
| return 'YYYY-MM-DD'; | ||||||
| } | ||||||
| return plugin.instance.options?.format || 'YYYY-MM-DD'; | ||||||
| } | ||||||
|
|
||||||
| private getNoteTitle(note: ReflectNote, userDNPFormat: string): string { | ||||||
| if (note.daily_at) { | ||||||
| return moment(note.daily_at).format(userDNPFormat); | ||||||
| } | ||||||
| return truncateTitle(note.subject); | ||||||
|
||||||
| return truncateTitle(note.subject); | |
| return truncateTitle(note.subject).trim() || 'Untitled'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added the Untitled fallback here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where are images generally downloaded from? Is it a centralized location which would require rate-limiting or backoffs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a good question. Reflect assets are stored with a link that looks something like this
https://reflect-assets.app/v1/users/tfgFpw.../999abc79-e45...?key=2227f9…
I doubt they have strict rate limits, but it's probably not a bad idea to put it in there as a safety measure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea, I actually didn't have enough images to even know if there were limits. My tests on my own export only had around 190 images and they all downloaded fine. I'll add some sort of rate limiting
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added retries with backoff for attachment downloads and respect Retry-After when the server sends it. Downloads are still sequential.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The downloadAttachments toggle is snapshotted at import start (here) with a comment about preventing mid-import drift, but tagsFrontmatter, dateFrontmatter, and titleFrontmatter are read directly from this throughout the loop (lines 265, 282, 285, 289).
Either all four settings should be snapshotted for consistency, or none of them need to be — the import modal presumably isn't interactive while the import is running.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, all four settings are snapshotted at import start now
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be in a try/catch and include validation that the imported data matches the expected format.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a dedicated parse/validate step here, so malformed exports fail early with a readable error instead of blowing up in the middle of the import.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider putting the note.id in the frontmatter like is done in other importer format. That would allow incremental imports in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added id to the frontmatter
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| const frontMatter: Record<string, any> = {}; | |
| const frontMatter: FrontmatterCache = {}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed to FrontMatterCache
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please do not add these entries here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI if you need to exclude files locally without modifying .gitignore, you can modify your
.git/info/excludefile.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks, thats on me 🤦
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed these