-
Notifications
You must be signed in to change notification settings - Fork 2
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
export docs and related docs #7
Conversation
PRO-4261 As an editor, I can download a ZIP file of my exported documents
When clicking on the Two new routes This method should take the IDs of the documents the user want to export, as well as the related types that he wants to get. Only the types, not the IDs, since we decided that it would be too heavy for the export modal to get the amount of related docs when we batch a large amount of documents. Generate filesThis method will need to use the Since we need to store documents in a flat EJSON array, it might be a good idea to add an option to this method in order to get a flat array of related documents instead of filling requested documents with their related docs, see how it could potentially be used with query builders. We want one EJSON file for each db collection, for example We want, for each document to export the draft and live versions (only draft if there is no live). The draft version must appear first to avoid situation where a live exists without draft which is forbidden. Same thing for related documents, they must be written first in the EJSON file, otherwise the doc manager will yell when we'll import it. We will create an Generate ZipTo generate the ZIP file we would prefer using the native node library. Progress bar notificationThis progress bar notification is triggered by wrapping your export function with the
The Finally the ZIP file will be sent back to the client and the download triggered from the user's browser. PermissionsFor exporting documents, a user that has at least one per-type permission should be able to export anything. He won’t be able to see For Reference https://github.com/apostrophecms/tech-designs/blob/main/3.x/share-documents-across-sites/design.md Acceptance Criteria
|
997ef19
to
48bb490
Compare
48bb490
to
2e848a7
Compare
2e848a7
to
ef7a790
Compare
} | ||
if (self.options.import !== false) { | ||
self.apos.asset.iconMap['apos-import-export-upload-icon'] = 'Upload'; | ||
} |
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.
not related to this PR, but improves this code
aec7a5d
to
41413a1
Compare
lib/methods/export.js
Outdated
const data = { | ||
'aposDocs.json': docsData, | ||
'aposAttachments.json': attachmentsData | ||
// attachments: 'attachments/' // TODO: add attachment into an "/attachments" folder |
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.
next step
lib/methods/export.js
Outdated
|
||
const data = { | ||
'aposDocs.json': docsData, | ||
'aposAttachments.json': attachmentsData |
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.
aposAttachment.json has mocked data, for now (see TODOs)
lib/methods/export.js
Outdated
setTimeout(() => { | ||
self.apos.attachment.uploadfs.remove(downloadPath, error => { | ||
if (error) { | ||
self.apos.util.error(error); | ||
} | ||
}); | ||
}, expiration || 1000 * 60 * 60); |
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.
setTimeout is bad... if the process gets killed, that won't be clearing the downloads
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.
So we should find a better way to clean files.. is this cleaning files every hours?
Couldn't we clean the file when the client downloaded it by performing a new request with the file ID? 🤔
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.
Every hour yes.
The client is requesting the file directly, from the public
folder, and I don't think apostrophe handles that.
So I fear we cannot know if the file has been downloaded.
I may be wrong though...
It might be possible, I'm quickly looking for a possible solution, but I don't think we can.
Besides, we might want the archive to reside in public to be available for some time in the case the first download fails, IDK...
And if we're implementing a solution that's piping the whole thing (without writing the archvive on the server but directly streaming it to the client), we won't have this problem
lib/methods/export.js
Outdated
return { | ||
url: downloadUrl | ||
}; |
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.
return url, we're not forced to do this because the url is given and opened by the export-download
event, but let's return something in case of...
lib/methods/index.js
Outdated
// TODO: remove: | ||
const attachmentsMock = [ { foo: 'bar' } ]; |
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 TODO
lib/methods/index.js
Outdated
const relatedTypes = self.apos.launder.strings(req.body.relatedTypes); | ||
const extension = self.apos.launder.string(req.body.extension, 'zip'); | ||
|
||
// TODO: add batchSize? |
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.
??
lib/methods/index.js
Outdated
if (!relatedTypes.length) { | ||
const docs = await self.fetchActualDocs(req, allIds, reporting); | ||
|
||
// TODO: change undefined to attachments |
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.
and this TODO
lib/methods/index.js
Outdated
|
||
const docs = await self.fetchActualDocs(req, [ ...allIds, ...allRelatedIds ], reporting); | ||
|
||
// TODO: change undefined to attachments |
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.
and that TODO
lib/methods/index.js
Outdated
.map(relatedDoc => relatedDoc._id); | ||
}, | ||
|
||
// TODO: factorize with the one from AposI18nLocalize.vue |
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.
@ValJed this TODO can be removed when we'll merge our work
lib/methods/index.js
Outdated
}, | ||
|
||
// TODO: factorize with the one from AposI18nLocalize.vue | ||
// TODO: limit recursion to 10 as we do when retrieving related types? |
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.
??
lib/methods/index.js
Outdated
return related.filter(doc => self.apos.modules[doc.type].relatedDocument !== false); | ||
}, | ||
|
||
// TODO: use actual attachments when calling this method: |
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.
and that TODO
// FIXME: the progress notification is not always dismissed. | ||
// Probably a fix that needs to be done in job core module. |
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.
yeah...
window.apos.bus && window.apos.bus.$on('export-download', event => { | ||
if (!event.url) { | ||
return; | ||
} | ||
window.open(event.url, '_blank'); | ||
}); |
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 event I was referring to above
// TODO: do not include types that have the `relatedDocument = false` | ||
// option in their module configuration |
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.
we should do that, let's keep the TODO
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.
Should be done in my PR
lib/methods/index.js
Outdated
// FIXME: seems like reporting is not working very well, | ||
// when calling failure on purpose, we have the progress bar | ||
// at 200%... 🤷♂️ |
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.
to verify when making final tests
self.exportFormats = { | ||
zip, | ||
gzip, | ||
...(self.options.exportFormats || {}) |
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.
projects can add custom formats
@@ -36,7 +36,10 @@ module.exports = { | |||
export: { | |||
label: 'aposImportExport:export', | |||
messages: { | |||
progress: 'aposImportExport:exporting' | |||
progress: 'aposImportExport:exporting', | |||
completed: 'aposImportExport:exported', |
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 "completed" message makes the job handle the display of the final notification (eg. "Exported 2 Articles"
progress: 'aposImportExport:exporting' | ||
progress: 'aposImportExport:exporting', | ||
completed: 'aposImportExport:exported', | ||
icon: 'database-export-icon', |
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.
already by default in the core, which is weird and too close to the piece-type-exporter
module.
still set it here because it's this module responsibility to set that database export icon
progress: 'aposImportExport:exporting', | ||
completed: 'aposImportExport:exported', | ||
icon: 'database-export-icon', | ||
resultsEventName: 'export-download' |
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.
that is necessary in order for the job to include the results into a 'export-download'
which will trigger the url opening in the frontend (see ui/apos/apps/index.js
).
the core has been changed so that resultsEventName
is taken into account (see apostrophecms/apostrophe#4270)
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.
We will need to document properly all these changes before to release.
<AposSelect | ||
:choices="extensions" | ||
:selected="extension" | ||
@change="onExtensionChange" |
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.
yeah, we said only ZIP...
but since gzip was added for the sake of testing another library, let's keep these 2 formats
const result = await window.apos.http.post(`${action}/${this.action}`, { | ||
busy: true, | ||
body: { | ||
_ids: docsId, | ||
relatedTypes, | ||
messages: this.messages, | ||
extension: this.extension | ||
} | ||
}); |
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 actual API call to export docs
archive.append(content, { name: filename }); | ||
} | ||
|
||
archive.finalize(); |
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 triggers the event finish
?
Summary
Big choucroute to batch-export documents, export a single document and page.
Export their actual data that is in the database (we don't use the manager that is populating fields and adding data).
We export the docs as they are in the DB.
We only use the docs manager to get the related docs.
Then we compress the docs in a
aposDocs.json
that's inside a ZIP or GZIP archive, and we return the URL to the client that's opened by the browser for download.What are the specific steps to test this change?
single doc (you can also test with a page):
multiple docs:
What kind of change does this PR introduce?
(Check at least one)
Make sure the PR fulfills these requirements:
If adding a new feature without an already open issue, it's best to open a feature request issue first and wait for approval before working on it.
Other information: