Add batch export folder/storage in context menu by Komediruzecki · Pull Request #734 · BoostIO/BoostNote-App

Komediruzecki · 2020-12-21T19:12:19Z

Initial batch export for PDF (#491)

Add prepare export functions in exports.ts
Add handling of menus for folder exports
Add support for non-recursive/recursive exports
Add export progress bar
Add support for storage/workspace export
Refactor functions for exports
Add component for batch export handling (state, procedure)
Refactor folder and storage export to use the component
Fix cancelation state logic
Add dim background on export procedure

General functionality:

Users can export folders via the context menu.
By right-clicking on the desired folder the context menu with export options comes up:

A similar context menu dialog is available for workspace/storage navigation item (for exporting whole storage)

Once the exported is selected, the file explorer comes up and offers the user to choose the save location.
When the location is confirmed export process starts and users can see export progress:

First directories are created in the destination (depending on recursive/non-recursive and selected folder/storage)

Then notes are being exported to their appropriate locations:

During export, for now, users can only cancel the export.
This stops the export, and the message of the export state is shown as a push message:

During export, export errors generated from export functions are shown in push messages as well.
In the end, the summary is also generated, as well as any export errors.

Summary for successfully finish - push message:

The whole procedure while exporting can be seen here:

Error handling:

Test:

In electron Linux App (dev)
In electron Linux App production version (appImage)

Rokt33r

We should do refactoring first. It is too difficult to review this pr. So, please try this.

Reduce arguments
Provide proper names for variables and functions(The names should explain what their variables and functions actually do

Rokt33r · 2020-12-27T21:21:21Z

+  const { t } = useTranslation()
+  const { storageMap } = useDb()
+
+  const isExportCanceledRef = useRef<boolean>(false)


canceledRef

Rokt33r · 2020-12-27T21:21:37Z

+  folderName: string
+  folderPathname: string
+  exportType: string
+  isRecursive: boolean


recursive

Rokt33r · 2020-12-27T21:21:53Z

+  folderPathname: string
+  exportType: string
+  isRecursive: boolean
+  isStorageExport: boolean


exportingStorage

Rokt33r · 2020-12-27T21:22:47Z

+    []
+  )
+
+  const exportCanceled = useCallback(() => {


isExportingCanceled

Rokt33r · 2020-12-27T21:24:39Z

  })
 }

+export function getValidNoteTitle(note: NoteDoc): string {


note._id is not a title. Please return Untitled

I want to return untitled, but if we do that, two notes within the same folder with no title would be named "Untitled.ext", which would overwrite the note contents.

So the solution is some kind of ID for untitled notes, either note._id or some index we could introduce when exporting notes.

Hmm, how do you think appending note id after the title?

Untitled-XXXXXXXX.md
Untitled-XXXXXXXX.md

That's great, if note id is something we can let users know of, I think that should be the easiest, safe solution.

Rokt33r · 2020-12-27T21:25:47Z

+  return Math.floor((value / maxValue) * exportProgressMaxValue)
+}
+
+async function writeNoteData(


I don't see any benefits to have this method. It is just writeFile

Refactored!

Rokt33r · 2020-12-27T21:26:19Z

+async function writeNoteData(
+  noteData: Buffer | string,
+  location: string,
+  exportErrors: string[]


Don't provide an array like this. This design makes keeping status difficult

Method removed!

Rokt33r · 2020-12-27T21:35:22Z

+  }
+}
+
+export async function batchExport(


Too many arguments

Will try to refactor it in smaller functions, but a lot of arguments are needed because it calls each export type, for each note, creates directories etc.

I think we could design communication between component and exports.ts in a more clear way, and this could reduce the number of arguments

Rokt33r · 2020-12-27T21:36:52Z

+  return Math.floor((value / maxValue) * exportProgressMaxValue)
+}
+
+async function writeNoteData(


This is just writeFile with some extra. I don't think we need this funciton

Rokt33r · 2020-12-27T21:38:36Z

+  }
+}
+
+async function getAttachmentData(storage: NoteStorage, src: string) {


Please throw error rather than returning reject

Rename the function. Its name should explain what it does.

Reconsider that we really need this function.

Refactored/removed.

Rokt33r · 2021-01-01T00:03:10Z

+      )
+      let exportingNoteCount = calculateNoteCountCondition ? 0 : noteCount
+      if (exportSettings.exportingStorage) {
+        exportingNoteCount = values(noteMap).length


Avoid nested conditional

if (exportSettings.exportingStorage) { return values(noteMap).length } ...

Refactored this altogether, so no longer here!

Rokt33r · 2021-01-01T00:07:04Z

+            if (
+              noteDoc.trashed ||
+              (exportSettings.recursive &&
+                !noteDoc.folderPathname.startsWith(


I think we cannot use startsWith in this case. For example, when exporting /my-folder, /my-folder2 probably considered as a child folder

You are right, I added additional "/" to differentiate between startsWith and subfolder which starts with a pathname.
I tested it and it seems to work fine, would that be error prone for all cases?

Rokt33r · 2021-01-01T00:08:35Z

+          for (const [, noteDoc] of entries(noteMap)) {
+            if (
+              noteDoc.trashed ||
+              (exportSettings.recursive &&


Separate conditions for better readability

if (noteDoc.trashed) { continue } if (exportSettings.recursive && !noteDoc.folderPathname.startsWith) { continue }

Refactored!

Rokt33r · 2021-01-01T00:09:23Z

+
+  const [showExportProgress, setShowExportProgress] = useState(false)
+  const [exportProgressMaxValue] = useState(100)
+  const [exportErrors, setExportErrors] = useState([] as string[])


Prefer generic useState<string[]>([])

Oh, didn't know about that, adjusted!

Rokt33r · 2021-01-01T00:10:43Z

+  const { storageMap } = useDb()
+
+  const [showExportProgress, setShowExportProgress] = useState(false)
+  const [exportProgressMaxValue] = useState(100)


This is a const, not a state. Discard this hook and define the value as a const outside of this function component

Rokt33r · 2021-01-01T00:42:51Z

+      const totalNotesToExport = calculateNumberOfExportingNotes(noteMap)
+
+      let exportingNoteIndex = 0
+      for (const [, noteDoc] of entries(noteMap)) {


We've already known what to export while executing calculateNumberOfExportingNotes. I think we should refactor calculateNumberOfExportingNotes so exporting doesn't have to check whole notes twice.

Refactored this, now looks better, no repetition!

Rokt33r · 2021-01-01T00:45:37Z

+  ])
+
+  useEffectOnce(() => {
+    exportDocuments().then(onFinish).catch(onFinish)


If we run onFinish either results, please use .finally method. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/finally

Refactored to finally!

Rokt33r · 2021-01-01T00:46:19Z

+    ]
+  )
+
+  const exportDocuments = useCallback(async () => {


startExportingNotes

Rokt33r · 2021-01-01T00:51:33Z

    [storage.id, note, updateNote]
  )
-
-  const getAttachmentData = useCallback(


Please revoke this change. All attachments must be retrieved from this method.

If we really need to access the map, then we can introduce a new method to retrieve it. But please avoid introducing the method if possible.

Umm, I am confused by this comments.

So in essence, we need to have attachments data, this means attachmentMap is an variable wee need access to.
This can be done either via sending it in a method and using it directly, or (as previously coded) using the getAttachmentData method, which was retriving attachment data for a particular src value.

From first comment, you said we should not need this method maybe, so I removed it in this refactor, and used attachmentMap directly.

How would new method to retrieve it be used like? do you mean a method to retrieve attachmentMap, or attachment data? or something else?

I cannot really think of a way to get needed attachments without using attachmentData or some method to retrtieve it for particular found src in content.

Maybe we could somehow preprocess content and fetch attachments needed (similar to how it is done in export now)
and send attachments needed to export? Or in the preprocess step embedd needed changes, thus not needing to send the attachments at all - and send modified content?

Rokt33r · 2021-01-01T00:55:38Z

  })
 }

+export function getValidNoteTitle(note: NoteDoc): string {


Hmm, how do you think appending note id after the title?

Untitled-XXXXXXXX.md
Untitled-XXXXXXXX.md

Add prepare export functions in exports.ts Add handling of menus for folder exports Add support for non-recursive/recursive exports Add export progress bar Add support for storage export Add component for batch export handling (state, procedure) Refactor folder and storage export to use the component Fix cancelation state logic Add dim background on export procedure Add support for note statistics Add current note exporting vs total number of notes to export beside export note operation Remove getAttachment data helper function Set note title to untitled if note has no title Refactor writeNoteData function Fix PDF export push message to include export type (HTML when set) Update naming of few variables in export Remove unnecessary try/catch in PDF conversion function Refactor attachment linking Add better handling of injecting attachments Removed old code for pdf conversion header/footer Fix attachment rendering in PDF export from HTML tag Add support for attachment rendering from HTML tags for PDF export Add better support for attachment regex: single and double qoutes Add untitled-note_id for notes without title Fix startsWith condition by adding additional '/' to find sub-folders Update code after rebase v0.14.1 Fix progress bar (use new one) Refactor NoteContextMenu Update batch export after rebase v0.14.3 Update batch export to use new exports API Update exports to validate untitled note names

Komediruzecki requested a review from Rokt33r December 21, 2020 19:12

Komediruzecki added the awaiting review ❇️ Pull request is awaiting a review. label Dec 21, 2020

Komediruzecki changed the title ~~Feature/add batch export folder in context menu~~ Add batch export folder/storage in context menu Dec 21, 2020

Komediruzecki force-pushed the feature/add-batch-export-folder-in-context-menu branch from 9edb7de to 30b0ad1 Compare December 24, 2020 12:14

Rokt33r requested changes Dec 27, 2020

View reviewed changes

Komediruzecki requested a review from Rokt33r December 28, 2020 11:32

Komediruzecki force-pushed the feature/add-batch-export-folder-in-context-menu branch from 6e68773 to dab5eff Compare December 29, 2020 15:24

Rokt33r requested changes Jan 1, 2021

View reviewed changes

Rokt33r added awaiting changes 🖊️ Pull request has been reviewed, but contributor needs to make changes. and removed awaiting review ❇️ Pull request is awaiting a review. labels Jan 1, 2021

Komediruzecki requested a review from Rokt33r January 1, 2021 11:06

Komediruzecki added awaiting review ❇️ Pull request is awaiting a review. and removed awaiting changes 🖊️ Pull request has been reviewed, but contributor needs to make changes. labels Jan 1, 2021

Komediruzecki force-pushed the feature/add-batch-export-folder-in-context-menu branch from 624955b to 5096b9a Compare January 7, 2021 07:45

Komediruzecki mentioned this pull request Mar 12, 2021

Create/support spaces with notes in separate .md files divided by folders #811

Closed

Komediruzecki force-pushed the feature/add-batch-export-folder-in-context-menu branch from 5096b9a to 14f3272 Compare March 16, 2021 21:25

Komediruzecki force-pushed the feature/add-batch-export-folder-in-context-menu branch from 14f3272 to 10cbed4 Compare March 30, 2021 20:09

Komediruzecki force-pushed the feature/add-batch-export-folder-in-context-menu branch from 10cbed4 to 59f54f1 Compare May 27, 2021 17:53

Komediruzecki force-pushed the feature/add-batch-export-folder-in-context-menu branch from 59f54f1 to 05745fc Compare May 27, 2021 18:29

Rokt33r merged commit 343d0b1 into BoostIO:master May 28, 2021

Conversation

Komediruzecki commented Dec 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Rokt33r left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Komediruzecki Dec 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Komediruzecki commented Dec 21, 2020 •

edited

Loading

Komediruzecki Dec 28, 2020 •

edited

Loading