Skip to content

Commit

Permalink
Merge pull request #74 from apostrophecms/pro-6418-parked-singletons
Browse files Browse the repository at this point in the history
PRO-6418: parked pages & singletons
  • Loading branch information
myovchev authored Aug 22, 2024
2 parents 8e7bc39 + 3e8e26b commit beb3abc
Show file tree
Hide file tree
Showing 7 changed files with 229 additions and 107 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
156 changes: 127 additions & 29 deletions lib/methods/import.js
Original file line number Diff line number Diff line change
Expand Up @@ -279,9 +279,13 @@ module.exports = self => {
reporting, docs, failedIds
}) {
const docIds = [];
const replaceItems = [];
const aposLocale = `${req.locale}:draft`;
const alreadyFailed = [ ...failedIds ];

for (const { type, aposDocId } of docs) {
for (const {
type, aposDocId, parkedId, title, updatedAt, aposMode, aposLocale
} of docs) {
if (alreadyFailed.includes(aposDocId)) {
continue;
}
Expand All @@ -290,12 +294,38 @@ module.exports = self => {
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 the parked/singleton doc is not found, it should be processed as a
// regular doc.
if (isSingleton || parkedId) {
if (aposMode !== 'draft' || !aposDocId) {
continue;
}
const singletonAposDocId = await self.findExistingSingletonId({
type,
aposLocale
});
if (singletonAposDocId) {
replaceItems.push({
aposDocId,
title,
type,
updatedAt,
replaceId: singletonAposDocId
});
continue;
}
}
if (aposDocId) {
docIds.push(aposDocId);
}
}

const aposLocale = `${req.locale}:draft`;
const criteria = {
$and: [
{ aposDocId: { $in: docIds } },
Expand All @@ -322,6 +352,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
Expand Down Expand Up @@ -368,31 +405,9 @@ module.exports = self => {
reporting.success();
}
} catch (error) {
if (!self.apos.doc.isUniqueError(error)) {
reporting.failure();
failedIds.push(cloned.aposDocId);
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);
}
}
reporting.failure();
failedIds.push(cloned.aposDocId);
self.apos.util.error(error);
}
}
},
Expand Down Expand Up @@ -546,9 +561,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)) {
Expand All @@ -571,6 +589,7 @@ module.exports = self => {
}
}

await self.replaceDocId(doc, existingAposDocId);
self.handleRichTextFields(manager, doc);

if (method === 'insert') {
Expand Down Expand Up @@ -686,10 +705,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 });
Expand Down Expand Up @@ -743,12 +765,22 @@ module.exports = self => {
}
}

const [ , replaceAposDocId ] = replaceAposDocIds
.find(([ importId ]) => importId === doc.aposDocId) ?? [];
await self.insertOrUpdateDoc(req, {
doc,
method: 'update',
failedIds
failedIds,
existingAposDocId: replaceAposDocId
});

if (replaceAposDocId) {
replaceDocIdPairs.push([
doc._id,
`${replaceAposDocId}:${doc.aposLocale}`
]);
}

jobManager.success(job);
} catch (err) {
jobManager.failure(job);
Expand All @@ -758,6 +790,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: {
Expand Down Expand Up @@ -804,6 +840,68 @@ module.exports = self => {
}

await self.remove(exportPath);
},

// This methods expects a singleton type and it doesn't
// perform any related checks. It's up to the caller to
// make sure the singleton exists.
// Singletons are parked pages (having parkedId property) and
// pieces having option `singleton: true`.
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;
},

// Replace ID fields of imported document with a new ID,
// usually the ID of an existing document in the database.
// This methods shouldn't be "perfect" as `apos.doc.changeDocIds`
// will be called at the end of the import process to replace
// all the IDs and update relationships.
async replaceDocId(doc, replaceId) {
if (!replaceId) {
return doc;
}
doc._id = `${replaceId}:${doc.aposLocale}`;
doc.aposDocId = replaceId;

const manager = self.apos.doc.getManager(doc.type);
if (!manager || !self.isPage(manager)) {
return doc;
}

const existing = await self.apos.doc.db.findOne({
_id: doc._id
}, {
projection: {
path: 1,
rank: 1,
level: 1
}
});
if (existing) {
doc.path = existing.path;
doc.rank = existing.rank;
doc.level = existing.level;
}

return doc;
}
};
};
7 changes: 4 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
},
Expand All @@ -26,8 +27,8 @@
"eslint-plugin-promise": "^6.1.1",
"eslint-plugin-vue": "^9.19.2",
"mocha": "^10.2.0",
"stylelint": "^15.9.0",
"stylelint-config-apostrophe": "^3.0.0",
"stylelint": "^16.0.0",
"stylelint-config-apostrophe": "^4.1.0",
"vue-eslint-parser": "^9.3.2"
},
"dependencies": {
Expand Down
26 changes: 21 additions & 5 deletions test/overrideDuplicates.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'
}
]
}
}
})
});
Expand Down Expand Up @@ -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,
Expand All @@ -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);
Expand Down
Loading

0 comments on commit beb3abc

Please sign in to comment.