From c3df6777af07ff2dd9d729178c3130de8d0e1f1c Mon Sep 17 00:00:00 2001 From: Miro Yovchev <2827783+myovchev@users.noreply.github.com> Date: Mon, 19 Aug 2024 13:41:18 +0300 Subject: [PATCH 01/14] Collect parked and singleton pages as duplicates, record _replaceId --- lib/methods/import.js | 56 ++++++++++++++++++++++++++++++++------ test/overrideDuplicates.js | 26 ++++++++++++++---- 2 files changed, 69 insertions(+), 13 deletions(-) diff --git a/lib/methods/import.js b/lib/methods/import.js index c51fa476..b3e0d0e9 100644 --- a/lib/methods/import.js +++ b/lib/methods/import.js @@ -277,28 +277,61 @@ module.exports = self => { reporting, docs, failedIds }) { const docIds = []; + const replaceItems = []; + const aposLocale = `${req.locale}:draft`; + const localeCriteria = { + $or: [ + { aposLocale: { $exists: false } }, + { aposLocale } + ] + }; - for (const { type, aposDocId } of docs) { + for (const { + type, aposDocId, parkedId, title, updatedAt, aposLocale + } of docs) { if (!self.canImport(req, type)) { failedIds.push(aposDocId); reporting.failure(); continue; } + + const isSingleton = self.apos.modules[type] && + self.apos.modules[type].options.singleton === true; + + // Mark singleton and parked docs as "replace" items. + // Be sure to always continue the loop to avoid duplicates. + if (isSingleton || parkedId) { + if (!aposLocale.endsWith(':draft') || !aposDocId) { + continue; + } + const parkedDoc = await self.apos.doc.db.findOne({ + $and: [ + { type }, + { ...localeCriteria } + ] + }, { + projection: { aposDocId: 1 } + }); + if (parkedDoc && parkedDoc.aposDocId) { + replaceItems.push({ + aposDocId, + title, + type, + updatedAt, + _replaceId: parkedDoc.aposDocId + }); + } + continue; + } if (aposDocId) { docIds.push(aposDocId); } } - const aposLocale = `${req.locale}:draft`; const criteria = { $and: [ { aposDocId: { $in: docIds } }, - { - $or: [ - { aposLocale: { $exists: false } }, - { aposLocale } - ] - } + { ...localeCriteria } ] }; @@ -316,6 +349,13 @@ module.exports = self => { updatedAt })); + // Append the replace items to the duplicate lists, so that we can + // get confirmation from the user to replace them + for (const item of replaceItems) { + duplicatedIds.add(item.aposDocId); + duplicatedDocs.push(item); + } + return { duplicatedIds, duplicatedDocs diff --git a/test/overrideDuplicates.js b/test/overrideDuplicates.js index 7e7c14f5..ec29a3c7 100644 --- a/test/overrideDuplicates.js +++ b/test/overrideDuplicates.js @@ -147,6 +147,18 @@ describe('#overrideDuplicates - overriding locales integration tests', function( } } } + }, + '@apostrophecms/page': { + options: { + park: [ + { + parkedId: 'search-parked', + slug: '/search', + title: 'Search', + type: '@apostrophecms/search' + } + ] + } } }) }); @@ -196,10 +208,12 @@ describe('#overrideDuplicates - overriding locales integration tests', function( }); const [ nonLocalized ] = await apos.doc.db.find({ title: 'nonLocalized1' }).toArray(); const enArticles = await apos.article.find(req).toArray(); + const parkedPages = await apos.page.find(req, { parkedId: { $exists: true } }).toArray(); + const singleton = await apos.global.findGlobal(req); const failedIds = []; const reporting = { failure: () => {} }; - const enDocs = enArticles.concat([ nonLocalized ]); + const enDocs = enArticles.concat([ nonLocalized, singleton ]).concat(parkedPages); const enDuplicates = await importExportManager.checkDuplicates(req, { reporting, docs: enDocs, @@ -222,17 +236,19 @@ describe('#overrideDuplicates - overriding locales integration tests', function( const actual = { enDuplicates: [ enDocs.every((doc) => enDuplicates.duplicatedIds.has(doc.aposDocId)), - enDuplicates.duplicatedDocs.length + enDuplicates.duplicatedDocs.length, + enDuplicates.duplicatedDocs.filter(doc => !!doc._replaceId).length ], frDuplicates: [ frDocs.every((doc) => frDuplicates.duplicatedIds.has(doc.aposDocId)), frDuplicates.duplicatedIds.has(nonLocalized.aposDocId), - frDuplicates.duplicatedDocs.length + frDuplicates.duplicatedDocs.length, + enDuplicates.duplicatedDocs.filter(doc => !!doc._replaceId).length ] }; const expected = { - enDuplicates: [ true, 3 ], - frDuplicates: [ false, true, 1 ] + enDuplicates: [ true, 6, 3 ], + frDuplicates: [ false, true, 4, 3 ] }; assert.deepStrictEqual(actual, expected); From f4544009d6b955f48f95d1937efb2f29ff253c0a Mon Sep 17 00:00:00 2001 From: Miro Yovchev <2827783+myovchev@users.noreply.github.com> Date: Mon, 19 Aug 2024 18:58:52 +0300 Subject: [PATCH 02/14] Update existing singletons, while keeping their ID's in tact --- .stylelintrc.json | 12 ++- CHANGELOG.md | 1 + lib/methods/import.js | 99 +++++++++++++------ package.json | 5 +- test/overrideDuplicates.js | 4 +- .../components/AposDuplicateImportModal.vue | 66 ++++++++----- ui/apos/components/AposExportModal.vue | 49 ++++----- ui/apos/components/AposImportModal.vue | 31 +++--- 8 files changed, 163 insertions(+), 104 deletions(-) diff --git a/.stylelintrc.json b/.stylelintrc.json index 827def6a..68449fcd 100644 --- a/.stylelintrc.json +++ b/.stylelintrc.json @@ -1,3 +1,13 @@ { - "extends": "stylelint-config-apostrophe" + "extends": "stylelint-config-apostrophe", + "overrides": [ + { + "files": ["*.vue", "**/*.vue"], + "customSyntax": "postcss-html" + }, + { + "files": ["*.s{c,a}ss", "**/*.s{c,a}ss"], + "customSyntax": "postcss-scss" + } + ] } diff --git a/CHANGELOG.md b/CHANGELOG.md index cd3bfb62..9a474abd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ * Uses the new method `simulateRelationshipsFromStorage` from core to simulate relationships on data from DB to be able to pass the convert, also uses the new option `fetchRelationships: false` on the convert to avoid fetching relationships from the DB. It prevents issues when a relationship has not been inserted yet. +* Requires a duplicate confirmation for existing singleton documents (including parked pages), keeping their original ID's while importing (if the user chooses to do so). ## 2.2.0 (2024-07-12) diff --git a/lib/methods/import.js b/lib/methods/import.js index b3e0d0e9..259e2291 100644 --- a/lib/methods/import.js +++ b/lib/methods/import.js @@ -300,28 +300,26 @@ module.exports = self => { // Mark singleton and parked docs as "replace" items. // Be sure to always continue the loop to avoid duplicates. + // If the parked/singleton doc is not found, it should be processed as a + // regular doc. if (isSingleton || parkedId) { if (!aposLocale.endsWith(':draft') || !aposDocId) { continue; } - const parkedDoc = await self.apos.doc.db.findOne({ - $and: [ - { type }, - { ...localeCriteria } - ] - }, { - projection: { aposDocId: 1 } + const singletonAposDocId = await self.findExistingSingletonId({ + type, + aposLocale }); - if (parkedDoc && parkedDoc.aposDocId) { + if (singletonAposDocId) { replaceItems.push({ aposDocId, title, type, updatedAt, - _replaceId: parkedDoc.aposDocId + replaceId: singletonAposDocId }); + continue; } - continue; } if (aposDocId) { docIds.push(aposDocId); @@ -408,25 +406,6 @@ module.exports = self => { self.apos.util.error(error); continue; } - - // TODO: We should ask user too if he want to override singleton docs - // This logic might be useless since we pre check duplicates - const isSingleton = self.apos.modules[cloned.type] && - self.apos.modules[cloned.type].options.singleton === true; - - if (isSingleton) { - try { - await self.insertOrUpdateDoc(req, { - doc: cloned, - method: 'update', - failedIds - }); - reporting.success(); - } catch (err) { - reporting.failure(); - failedIds.push(cloned.aposDocId); - } - } } } }, @@ -580,9 +559,12 @@ module.exports = self => { }, async insertOrUpdateDoc(req, { - doc, method = 'insert', failedIds = [] + doc, method = 'insert', failedIds = [], existingAposDocId }) { const manager = self.apos.doc.getManager(doc.type); + if (existingAposDocId) { + method = 'update'; + } // Import can be disable at the page-type level if (!self.canImport(req, doc.type)) { @@ -605,6 +587,7 @@ module.exports = self => { } } + self.replaceDocId(doc, existingAposDocId); self.handleRichTextFields(manager, doc); if (method === 'insert') { @@ -720,10 +703,13 @@ module.exports = self => { const overrideLocale = self.apos.launder.boolean(req.body.overrideLocale); const exportPath = await self.getExportPathById(self.apos.launder.string(req.body.exportPathId)); const docIds = self.apos.launder.strings(req.body.docIds); + // Import aposDocId/Existing aposDocId pairs + const replaceAposDocIds = req.body.replaceDocIds?.map(arr => self.apos.launder.strings(arr)) ?? []; const jobId = self.apos.launder.string(req.body.jobId); const importedAttachments = self.apos.launder.strings(req.body.importedAttachments); const formatLabel = self.apos.launder.string(req.body.formatLabel); const failedIds = []; + const replaceDocIdPairs = []; const jobManager = self.apos.modules['@apostrophecms/job']; const job = await jobManager.db.findOne({ _id: jobId }); @@ -777,12 +763,21 @@ module.exports = self => { } } + const replaceItem = replaceAposDocIds.find(([ importId ]) => importId === doc.aposDocId); await self.insertOrUpdateDoc(req, { doc, method: 'update', - failedIds + failedIds, + existingAposDocId: replaceItem?.[1] }); + if (replaceItem) { + replaceDocIdPairs.push([ + doc._id, + `${replaceItem[1]}:${doc.aposLocale}` + ]); + } + jobManager.success(job); } catch (err) { jobManager.failure(job); @@ -792,6 +787,10 @@ module.exports = self => { } } + if (replaceDocIdPairs.length) { + await self.apos.doc.changeDocIds(replaceDocIdPairs, { skipReplace: true }); + } + if (failedIds.length) { await self.apos.notify(req, 'aposImportExport:importFailedForSome', { interpolate: { @@ -838,6 +837,44 @@ module.exports = self => { } await self.remove(exportPath); + }, + + async findExistingSingletonId({ type, aposLocale }) { + const singleton = await self.apos.doc.db.findOne({ + $and: [ + { type }, + { + $or: [ + { aposLocale: { $exists: false } }, + { aposLocale } + ] + } + ] + }, { + projection: { aposDocId: 1 } + }); + + if (singleton && singleton.aposDocId) { + return singleton.aposDocId; + } + + return null; + }, + + replaceDocId(doc, replaceId) { + if (!replaceId) { + return doc; + } + const oldAposDocId = doc.aposDocId; + doc._id = `${replaceId}:${doc.aposLocale}`; + doc.aposDocId = replaceId; + + const manager = self.apos.doc.getManager(doc.type); + if (manager && self.isPage(manager)) { + doc.path = doc.path.replace(oldAposDocId, replaceId); + } + + return doc; } }; }; diff --git a/package.json b/package.json index 12327a59..30ccebae 100644 --- a/package.json +++ b/package.json @@ -5,7 +5,8 @@ "main": "index.js", "scripts": { "eslint": "eslint --ext .js,.vue .", - "lint": "npm run eslint", + "stylelint": "stylelint ui/**/*.{scss,vue}", + "lint": "npm run eslint && npm run stylelint", "mocha": "mocha", "test": "npm run lint && npm run mocha" }, @@ -17,7 +18,7 @@ "author": "Apostrophe Technologies", "license": "UNLICENSED", "devDependencies": { - "apostrophe": "github:apostrophecms/apostrophe", + "apostrophe": "github:apostrophecms/apostrophe#pro-6418-parked-singletons", "eslint": "^8.44.0", "eslint-config-apostrophe": "^4.2.0", "eslint-config-standard": "^17.1.0", diff --git a/test/overrideDuplicates.js b/test/overrideDuplicates.js index ec29a3c7..1880bc32 100644 --- a/test/overrideDuplicates.js +++ b/test/overrideDuplicates.js @@ -237,13 +237,13 @@ describe('#overrideDuplicates - overriding locales integration tests', function( enDuplicates: [ enDocs.every((doc) => enDuplicates.duplicatedIds.has(doc.aposDocId)), enDuplicates.duplicatedDocs.length, - enDuplicates.duplicatedDocs.filter(doc => !!doc._replaceId).length + enDuplicates.duplicatedDocs.filter(doc => !!doc.replaceId).length ], frDuplicates: [ frDocs.every((doc) => frDuplicates.duplicatedIds.has(doc.aposDocId)), frDuplicates.duplicatedIds.has(nonLocalized.aposDocId), frDuplicates.duplicatedDocs.length, - enDuplicates.duplicatedDocs.filter(doc => !!doc._replaceId).length + enDuplicates.duplicatedDocs.filter(doc => !!doc.replaceId).length ] }; const expected = { diff --git a/ui/apos/components/AposDuplicateImportModal.vue b/ui/apos/components/AposDuplicateImportModal.vue index ddab1b03..e564c425 100644 --- a/ui/apos/components/AposDuplicateImportModal.vue +++ b/ui/apos/components/AposDuplicateImportModal.vue @@ -179,6 +179,13 @@ export default { return this.checked.length ? 'var(--a-white)' : 'transparent'; + }, + replaceDocIds() { + return this.checked.map( + aposDocId => this.duplicatedDocs.find(doc => doc.replaceId && doc.aposDocId === aposDocId) + ) + .filter(Boolean) + .map(({ aposDocId, replaceId }) => [ aposDocId, replaceId ]); } }, @@ -219,6 +226,7 @@ export default { apos.http.post('/api/v1/@apostrophecms/import-export/override-duplicates', { body: { docIds: this.checked, + replaceDocIds: this.replaceDocIds, importedAttachments: this.importedAttachments, exportPathId: this.exportPathId, jobId: this.jobId, @@ -259,29 +267,25 @@ export default {