From ab9a165bcaadf56c5d9922fc200a2ba3dea7255e Mon Sep 17 00:00:00 2001 From: Jed Date: Tue, 27 Aug 2024 14:30:58 +0200 Subject: [PATCH] Pro 6416 missing fields (#76) * updates getRelatedDocsFromSchema to fetch relationships with findForEditing * reworks related routes to get related types of related types * fixes recursion not working properly for relationships * moves route related logic in method to be testable * test related route logic --- CHANGELOG.md | 7 + lib/apiRoutes.js | 41 +---- lib/methods/export.js | 223 +++++++++++++++++++------ lib/methods/import.js | 16 +- test/index.js | 113 +++++++++---- test/util/index.js | 19 ++- ui/apos/components/AposExportModal.vue | 2 +- 7 files changed, 290 insertions(+), 131 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0dd890e2..a3942630 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,12 @@ # Changelog +## UNRELEASED + +### Fixes + +* Exported related documents now contain the entire document and not only the projected fields. +* The `related` route also returns the related types of the exported documents related documents. + ## 2.3.0 (2024-08-08) ### Adds diff --git a/lib/apiRoutes.js b/lib/apiRoutes.js index e3ffb328..c22f03e5 100644 --- a/lib/apiRoutes.js +++ b/lib/apiRoutes.js @@ -10,41 +10,18 @@ module.exports = self => { throw self.apos.error('forbidden'); } - const type = self.apos.launder.string(req.query.type); - if (!type) { - throw self.apos.error('invalid'); - } - - const { schema = [] } = self.apos.modules[type]; - - // Limit recursions in order to avoid the "Maximum call stack size exceeded" error - // if widgets or pieces are related to themselves. - const maxRecursions = 10; - let recursions = 0; - - const relatedTypes = schema - .flatMap(searchRelationships) - .filter(Boolean); + const types = self.apos.launder.strings(req.query.types); - return [ ...new Set(relatedTypes) ]; + const related = types.reduce((acc, type) => { + const manager = self.apos.doc.getManager(type); + if (!manager) { + throw self.apos.error('invalid'); + } - function searchRelationships(obj) { - const shouldRecurse = recursions <= maxRecursions; + return self.getRelatedTypes(req, manager.schema, acc); + }, []); - if (obj.type === 'relationship') { - return self.canExport(req, obj.withType) && obj.withType; - } else if (obj.type === 'array' || obj.type === 'object') { - recursions++; - return shouldRecurse && obj.schema.flatMap(searchRelationships); - } else if (obj.type === 'area') { - recursions++; - const widgets = self.apos.area.getWidgets(obj.options); - return Object.keys(widgets).flatMap(widget => { - const { schema = [] } = self.apos.modules[`${widget}-widget`]; - return shouldRecurse && schema.flatMap(searchRelationships); - }); - } - } + return related; } }, post: { diff --git a/lib/methods/export.js b/lib/methods/export.js index 1264073d..a70522ac 100644 --- a/lib/methods/export.js +++ b/lib/methods/export.js @@ -32,47 +32,49 @@ module.exports = self => { } const hasRelatedTypes = !!relatedTypes.length; - const docs = await self.getDocs(req, ids, hasRelatedTypes, manager, reporting); - const cleanDocs = self.clean(docs); + + const docs = (await self.getDocs(req, ids, hasRelatedTypes, manager, reporting)) + .map((doc) => self.apos.util.clonePermanent(doc)); if (!hasRelatedTypes) { return self.exportFile( req, reporting, format, - { docs: cleanDocs }, + { docs }, expiration ); } - const relatedDocs = docs.flatMap(doc => - self.getRelatedDocsFromSchema(req, { + const allDocs = [ ...docs ]; + for (const doc of docs) { + await self.getRelatedDocsFromSchema(req, { doc, schema: self.apos.modules[doc.type].schema, - relatedTypes - }) - ); - const allCleanDocs = [ ...self.clean(relatedDocs), ...cleanDocs ]; + relatedTypes, + storedData: allDocs + }); + } if (!format.includeAttachments) { return self.exportFile( req, reporting, format, - { docs: allCleanDocs }, + { docs: allDocs }, expiration ); } - const attachmentsIds = uniqBy([ ...docs, ...relatedDocs ], doc => doc._id) - .flatMap(doc => - self.getRelatedDocsFromSchema(req, { - doc, - schema: self.apos.modules[doc.type].schema, - type: 'attachment' - }) - ) - .map(attachment => attachment._id); + const attachmentsIds = []; + for (const doc of allDocs) { + await self.getRelatedDocsFromSchema(req, { + doc, + schema: self.apos.modules[doc.type].schema, + type: 'attachment', + storedData: attachmentsIds + }); + } const attachments = await self.getAttachments(attachmentsIds); const cleanAttachments = self.clean(attachments); @@ -92,7 +94,7 @@ module.exports = self => { reporting, format, { - docs: allCleanDocs, + docs: allDocs, attachments: cleanAttachments, attachmentUrls }, @@ -198,66 +200,147 @@ module.exports = self => { return self.canImportOrExport(req, docType, 'export'); }, - getRelatedDocsFromSchema( - req, - { - doc, - schema, - type = 'relationship', - relatedTypes, - recursion = 0 - } - ) { - return schema.flatMap(field => { + async getRelatedDocsFromSchema(req, { + doc, + schema, + relatedTypes, + storedData, + type = 'relationship', + recursion = 0, + mode = doc.aposMode || req.mode + }) { + recursion++; + for (const field of schema) { const fieldValue = doc[field.name]; const shouldRecurse = recursion <= MAX_RECURSION; - if (!fieldValue) { - return []; + if (!field.withType && !fieldValue) { + continue; } if (field.withType && relatedTypes && !relatedTypes.includes(field.withType)) { - return []; + continue; } if (field.withType && !self.canExport(req, field.withType)) { - return []; + continue; } if (shouldRecurse && field.type === 'array') { - return fieldValue.flatMap((subField) => self.getRelatedDocsFromSchema(req, { - doc: subField, - schema: field.schema, - type, - relatedTypes, - recursion: recursion + 1 - })); + for (const subField of fieldValue) { + await self.getRelatedDocsFromSchema(req, { + doc: subField, + schema: field.schema, + type, + relatedTypes, + recursion, + storedData, + mode + }); + } + continue; } if (shouldRecurse && field.type === 'object') { - return self.getRelatedDocsFromSchema(req, { + await self.getRelatedDocsFromSchema(req, { doc: fieldValue, schema: field.schema, type, relatedTypes, - recursion: recursion + 1 + recursion, + storedData, + mode }); + continue; } if (shouldRecurse && field.type === 'area') { - return (fieldValue.items || []).flatMap((widget) => self.getRelatedDocsFromSchema(req, { - doc: widget, - schema: self.apos.modules[`${widget?.type}-widget`]?.schema || [], - type, - relatedTypes, - recursion: recursion + 1 - })); + for (const widget of (fieldValue.items || [])) { + const schema = self.apos.modules[`${widget?.type}-widget`]?.schema || []; + await self.getRelatedDocsFromSchema(req, { + doc: widget, + schema, + type, + relatedTypes, + recursion, + storedData, + mode + }); + } + continue; } if (field.type === type) { - return Array.isArray(fieldValue) ? fieldValue : [ fieldValue ]; + await self.handleRelatedField(req, { + doc, + field, + fieldValue, + relatedTypes, + type, + storedData, + shouldRecurse, + recursion, + mode + }); } + } + }, - return []; - }); + async handleRelatedField(req, { + doc, + field, + fieldValue, + relatedTypes, + type, + storedData, + shouldRecurse, + recursion, + mode + }) { + if (type === 'attachment') { + if (fieldValue && !storedData.includes(fieldValue._id)) { + storedData.push(fieldValue._id); + } + return; + } + + const manager = self.apos.doc.getManager(field.withType); + const relatedIds = doc[field.idsStorage]; + if (!relatedIds?.length) { + return; + } + const criteria = { + aposDocId: { $in: relatedIds }, + aposLocale: doc.aposLocale || `${req.locale}:${mode}` + }; + + const clonedReq = mode ? req.clone({ mode }) : req; + const foundRelated = await manager + .findForEditing(clonedReq, criteria) + .permission('view') + .relationships(false) + .areas(false) + .toArray(); + + for (const related of foundRelated) { + const alreadyAdded = storedData.some(({ _id }) => _id === related._id); + if (alreadyAdded) { + continue; + } + + storedData.push(self.apos.util.clonePermanent(related)); + if (!shouldRecurse) { + continue; + } + + const relatedManager = self.apos.doc.getManager(related.type); + await self.getRelatedDocsFromSchema(req, { + doc: related, + schema: relatedManager.schema, + type, + relatedTypes, + recursion, + storedData + }); + } }, async exportFile(req, reporting, format, data, expiration) { @@ -379,6 +462,38 @@ module.exports = self => { } }); }, ms); + }, + + getRelatedTypes(req, schema = [], related = []) { + return findSchemaRelatedTypes(schema, related); + + function findSchemaRelatedTypes(schema, related, recursions = 0) { + recursions++; + if (recursions >= MAX_RECURSION) { + return related; + } + for (const field of schema) { + if ( + field.type === 'relationship' && + self.canExport(req, field.withType) && + !related.includes(field.withType) + ) { + related.push(field.withType); + const relatedManager = self.apos.doc.getManager(field.withType); + findSchemaRelatedTypes(relatedManager.schema, related, recursions); + } else if ([ 'array', 'object' ].includes(field.type)) { + findSchemaRelatedTypes(field.schema, related, recursions); + } else if (field.type === 'area') { + const widgets = self.apos.area.getWidgets(field.options); + for (const widget of Object.keys(widgets)) { + const { schema = [] } = self.apos.modules[`${widget}-widget`]; + findSchemaRelatedTypes(schema, related, recursions); + } + } + } + + return related; + } } }; }; diff --git a/lib/methods/import.js b/lib/methods/import.js index 4752ded1..0bdf2eec 100644 --- a/lib/methods/import.js +++ b/lib/methods/import.js @@ -321,7 +321,7 @@ module.exports = self => { continue; } } - if (aposDocId) { + if (aposDocId && !docIds.includes(aposDocId)) { docIds.push(aposDocId); } } @@ -742,23 +742,25 @@ module.exports = self => { for (const doc of filterDocs) { try { if (attachmentsInfo.length) { - const attachmentsToOverride = self.getRelatedDocsFromSchema(req, { + const attachmentsIds = []; + await self.getRelatedDocsFromSchema(req, { doc, schema: self.apos.modules[doc.type].schema, - type: 'attachment' + type: 'attachment', + storedData: attachmentsIds }); - for (const { _id } of attachmentsToOverride) { - if (importedAttachments.includes(_id)) { + for (const id of attachmentsIds) { + if (importedAttachments.includes(id)) { continue; } const attachmentInfo = attachmentsInfo - .find(({ attachment }) => attachment._id === _id); + .find(({ attachment }) => attachment._id === id); try { await self.insertOrUpdateAttachment(req, { attachmentInfo }); jobManager.success(job); - importedAttachments.push(_id); + importedAttachments.push(id); } catch (err) { jobManager.failure(job); } diff --git a/test/index.js b/test/index.js index 8111a902..e7d6eafe 100644 --- a/test/index.js +++ b/test/index.js @@ -115,52 +115,70 @@ describe('@apostrophecms/import-export', function () { docs, attachments, attachmentFiles } = await getExtractedFiles(exportPath); + const docsNames = docs.map(({ title, aposMode }) => ({ + title, + aposMode + })); + + const topicsContainNonProjectedFields = docs + .filter(({ type }) => type === 'topic') + .every(({ + createdAt, titleSortified, aposLocale + }) => createdAt && titleSortified && aposLocale); + const actual = { - docsNames: docs.map(({ title, aposMode }) => ({ - title, - aposMode - })), + docsNames, attachmentsLength: attachments.length, - attachmentFiles + attachmentFiles, + topicsContainNonProjectedFields }; + const expected = { docsNames: [ { - aposMode: 'draft', - title: 'topic1' + title: 'article2', + aposMode: 'draft' }, { - aposMode: 'draft', - title: 'topic2' + title: 'article1', + aposMode: 'draft' }, { - aposMode: 'published', - title: 'topic1' + title: 'article2', + aposMode: 'published' }, { - aposMode: 'published', - title: 'topic2' + title: 'article1', + aposMode: 'published' }, { - aposMode: 'draft', - title: 'article2' + title: 'topic1', + aposMode: 'draft' }, { - aposMode: 'draft', - title: 'article1' + title: 'topic3', + aposMode: 'draft' }, { - aposMode: 'published', - title: 'article2' + title: 'topic2', + aposMode: 'draft' }, - { - aposMode: 'published', - title: 'article1' + title: 'topic1', + aposMode: 'published' + }, + { + title: 'topic3', + aposMode: 'published' + }, + { + title: 'topic2', + aposMode: 'published' } ], attachmentsLength: 1, - attachmentFiles: [ `${attachmentId}-test-image.jpg` ] + attachmentFiles: [ `${attachmentId}-test-image.jpg` ], + topicsContainNonProjectedFields: true }; assert.deepEqual(actual, expected); @@ -201,27 +219,27 @@ describe('@apostrophecms/import-export', function () { docsNames: [ { aposMode: 'draft', - title: 'image1' + title: 'page1' }, { - aposMode: 'draft', - title: 'article2' + aposMode: 'published', + title: 'page1' }, { - aposMode: 'published', + aposMode: 'draft', title: 'image1' }, { - aposMode: 'published', + aposMode: 'draft', title: 'article2' }, { - aposMode: 'draft', - title: 'page1' + aposMode: 'published', + title: 'image1' }, { aposMode: 'published', - title: 'page1' + title: 'article2' } ], attachmentsLength: 1, @@ -302,12 +320,12 @@ describe('@apostrophecms/import-export', function () { name: 'test-image', type: 'attachment' } ], - docsLength: 8, + docsLength: 10, docsTitles: [ 'article2', 'article1', 'article2', 'article1', - 'topic1', 'topic2', - 'topic1', 'topic2' + 'topic1', 'topic3', 'topic2', + 'topic1', 'topic3', 'topic2' ], attachmentsNames: [ 'test-image' ], attachmentFileNames: new Array(apos.attachment.imageSizes.length + 1) @@ -365,8 +383,13 @@ describe('@apostrophecms/import-export', function () { } delete req.files; + + // Overrides all docs excepted topic3 + const docIds = duplicatedDocs + .filter((doc) => doc.title !== 'topic3') + .map(({ aposDocId }) => aposDocId); req.body = { - docIds: duplicatedDocs.map(({ aposDocId }) => aposDocId), + docIds, importedAttachments, exportPathId, jobId, @@ -416,7 +439,7 @@ describe('@apostrophecms/import-export', function () { .fill('test-image'), job: { good: 9, - total: 9 + total: 11 } }; @@ -764,6 +787,24 @@ describe('@apostrophecms/import-export', function () { }), true, `expected imported docs 'lastPublishedAt' value to be of '${lastPublishedAt}'`); }); + it('should get related types of a given doc type', async function() { + const req = apos.task.getReq(); + const relatedTypesArticles = importExportManager.getRelatedTypes(req, apos.article.schema); + const relatedTypesTopics = importExportManager.getRelatedTypes(req, apos.topic.schema); + + const actual = { + relatedTypesArticles, + relatedTypesTopics + }; + const expected = { + relatedTypesArticles: [ 'topic', '@apostrophecms/image', '@apostrophecms/image-tag' ], + relatedTypesTopics: [ 'topic' ] + }; + + assert.deepEqual(actual, expected); + + }); + describe('#getFirstDifferentLocale', function() { it('should find among the docs the first locale that is different from the req one', async function() { const req = { diff --git a/test/util/index.js b/test/util/index.js index c9146c7b..d7211e91 100644 --- a/test/util/index.js +++ b/test/util/index.js @@ -73,7 +73,24 @@ function getAppConfig(modules = {}) { _topics: { label: 'Topics', type: 'relationship', - withType: 'topic' + withType: 'topic', + builders: { + project: { + title: 1, + image: 1, + main: 1, + aposMode: 1 + } + } + }, + main: { + label: '', + type: 'area', + options: { + widgets: { + '@apostrophecms/image': {} + } + } } } } diff --git a/ui/apos/components/AposExportModal.vue b/ui/apos/components/AposExportModal.vue index a7a6f88f..79be1687 100644 --- a/ui/apos/components/AposExportModal.vue +++ b/ui/apos/components/AposExportModal.vue @@ -229,7 +229,7 @@ export default { this.relatedTypes = await apos.http.get('/api/v1/@apostrophecms/import-export/related', { busy: true, qs: { - type: this.type + types: [ this.type ] } }); this.checkedRelatedTypes = this.relatedTypes;