Skip to content
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

add option to import documents only as drafts #95

Merged
merged 31 commits into from
Nov 4, 2024
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
### Adds

* Adds AI-generated missing translations.
* Adds a checkbox to the import modal that, when selected, imports published versions of documents from an export file **as drafts**. If a document doesn’t have a published version, it will still be imported. Setting the `importDraftsOnlyDefault: true` option selects this checkbox by default.

## 2.4.1 (2024-10-03)

Expand Down
8 changes: 8 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,14 @@ If multiple locales are set up, the user will be prompted to choose between canc

![Screenshot highlighting the confirm modal letting the user choose between aborting on continuing the import when the docs locale is different from the site one.](https://static.apostrophecms.com/apostrophecms/import-export/images/different-locale-modal.png)

## Importing as drafts only

The import modal includes a checkbox labeled "_Import all documents as drafts_" that, when selected, imports the published version of documents in draft status only. This feature allows users to review and make modifications to imported documents before they are published.

By default, this checkbox can be set to selected by enabling the `importDraftsOnlyDefault: true` option in this module.

If a document in the import file has no published version, it will still be imported. Content types that do not have separate drafts will be imported normally.

## Updating existing pieces using CSV format

You can also update existing pieces via this module.
Expand Down
2 changes: 2 additions & 0 deletions i18n/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
"importCleanFailed": "The cleaning of the imported file on the server failed.",
"importWithCurrentLocaleHeading": "Different locale detected",
"importWithCurrentLocaleDescription": "This file was exported from the \"{{ docsLocale }}\" locale. Are you sure you want to import it into the \"{{ currentLocale }}\" locale?",
"importDraftsOnly": "Import all documents as drafts",
"importDraftsOnlyTooltip": "Content types that do not have separate drafts will be imported normally.",
"importDuplicateContinue": "Continue Import",
"importDuplicateDetected": "Duplicates Detected.",
"importDuplicateMessage": "Check the items you'd like to overwrite during import.",
Expand Down
3 changes: 2 additions & 1 deletion index.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ module.exports = {
ns: 'aposImportExport',
browser: true
},
preventUpdateAssets: false
preventUpdateAssets: false,
importDraftsOnlyDefault: false
},
init(self) {
self.formats = {
Expand Down
114 changes: 94 additions & 20 deletions lib/methods/import.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ module.exports = self => {
}

const { file } = req.files || {};
const importDraftsOnly = self.apos.launder.boolean(req.body.importDraftsOnly);
const overrideLocale = self.apos.launder.boolean(req.body.overrideLocale);
const formatLabel = self.apos.launder.string(req.body.formatLabel);
let exportPath = await self.getExportPathById(self.apos.launder.string(req.body.exportPathId));
Expand Down Expand Up @@ -74,6 +75,7 @@ module.exports = self => {
moduleName,
exportPathId: await self.getExportPathId(exportPath),
formatLabel: format.label,
importDraftsOnly,
content: {
heading: req.t('aposImportExport:importWithCurrentLocaleHeading'),
description: req.t('aposImportExport:importWithCurrentLocaleDescription', {
Expand All @@ -96,6 +98,10 @@ module.exports = self => {
self.rewriteDocsWithCurrentLocale(req, docs);
}

if (importDraftsOnly) {
docs = self.getPublishedDocsAsDraft(docs);
}

const total = docs.length + attachmentsInfo.length;

const {
Expand Down Expand Up @@ -129,7 +135,8 @@ module.exports = self => {
reporting,
duplicatedIds,
duplicatedDocs,
failedIds
failedIds,
importDraftsOnly
});

if (!duplicatedDocs.length) {
Expand Down Expand Up @@ -174,6 +181,7 @@ module.exports = self => {
await self.apos.attachment.recomputeAllDocReferences();

const results = {
importDraftsOnly,
overrideLocale,
duplicatedDocs,
importedAttachments,
Expand Down Expand Up @@ -248,6 +256,37 @@ module.exports = self => {
}
},

// Get only the published docs and convert them into draft.
// If a doc is a draft and has no published version, we keep it.
// If a doc has no aposMode, we keep it.
getPublishedDocsAsDraft(docs) {
return docs
.filter(isPublishedOrDraftAlone)
.map(convertToDraft);

function isPublishedOrDraftAlone(doc) {
// i.e "is draft without a published version"
const isDraftAlone =
doc.aposMode === 'draft' &&
!docs.find(item => item.aposDocId === doc.aposDocId && item.aposMode === 'published');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could use some here that return a boolean instead of a document like find.


return !doc.aposMode || doc.aposMode === 'published' || isDraftAlone;
}

function convertToDraft(doc) {
if (doc.aposMode === 'published') {
return {
...doc,
_id: doc._id?.replace('published', 'draft'),
aposLocale: doc.aposLocale?.replace('published', 'draft'),
aposMode: 'draft'
};
}

return doc;
}
},

isLocaleDifferent(req, doc) {
return doc.aposLocale && self.extractLocale(doc.aposLocale) !== req.locale;
},
Expand Down Expand Up @@ -385,7 +424,7 @@ module.exports = self => {
},

async insertDocs(req, {
docs, reporting, duplicatedIds, duplicatedDocs, failedIds
docs, reporting, duplicatedIds, duplicatedDocs, failedIds, importDraftsOnly
}) {
for (const doc of docs) {
if (duplicatedIds.has(doc.aposDocId) || failedIds.includes(doc.aposDocId)) {
Expand All @@ -402,7 +441,8 @@ module.exports = self => {
doc,
updateKey,
updateField,
duplicatedDocs
duplicatedDocs,
importDraftsOnly
});
reporting.success();
} catch (error) {
Expand All @@ -420,7 +460,8 @@ module.exports = self => {
const inserted = await self.insertOrUpdateDoc(req, {
doc,
failedIds,
duplicatedDocs
duplicatedDocs,
importDraftsOnly
});
if (inserted) {
reporting.success();
Expand All @@ -437,7 +478,8 @@ module.exports = self => {
doc,
updateKey,
updateField,
duplicatedDocs = []
duplicatedDocs = [],
importDraftsOnly
}) {
const manager = self.apos.doc.getManager(doc.type);
if (!self.canImport(req, doc.type)) {
Expand Down Expand Up @@ -481,7 +523,7 @@ module.exports = self => {
async function insert() {
// Insert as "published" to insert
// in both draft and published versions:
const _req = req.clone({ mode: 'published' });
const _req = req.clone({ mode: importDraftsOnly ? 'draft' : 'published' });

const type = doc.type;
const docToInsert = {};
Expand All @@ -491,6 +533,10 @@ module.exports = self => {
fetchRelationships: false
});

if (importDraftsOnly) {
delete docToInsert.lastPublishedAt;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this could be done in the function getPublishedDocsAsDraft to avoid spreading the logic?

      function convertToDraft(doc) {
        if (doc.aposMode === 'published') {
          doc._id = doc._id?.replace('published', 'draft');
          doc.aposLocale = doc.aposLocale?.replace('published', 'draft');
          doc.aposMode = 'draft';
        }
        delete doc.lastPublishedAt;

        return doc;
      }

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want to stay "functional":

    function convertToDraft({ lastPublishedAt, ...doc }) {
        if (doc.aposMode === 'published') {
          return {
            ...doc,
            _id: doc._id?.replace(':published', ':draft'),
            aposLocale: doc.aposLocale?.replace(':published', ':draft'),
            aposMode: 'draft'
          };
        }

        return doc;
      }

I think these lines should exist in the function convertToDraft.


if (self.isPage(manager)) {
// TODO: check if this is still true
// `convert` sets the type to `@apostrophecms/home-page`,
Expand Down Expand Up @@ -528,7 +574,16 @@ module.exports = self => {
.toObject();
}

const docsToUpdate = [ matchingDraft, matchingPublished ].filter(Boolean);
const docsToUpdate = [ matchingDraft, matchingPublished ].filter(doc => {
if (!doc) {
return false;
}
// If `importDraftsOnly` is true, we only update the existing draft version.
if (importDraftsOnly && doc.aposMode === 'published' && matchingDraft) {
return false;
}
return true;
});

for (const docToUpdate of docsToUpdate) {
const _req = req.clone({ mode: docToUpdate.aposMode });
Expand All @@ -537,6 +592,7 @@ module.exports = self => {
presentFieldsOnly: true,
fetchRelationships: false
});

self.isPage(manager)
? await importPage.update({
manager: self.apos.page,
Expand All @@ -547,7 +603,13 @@ module.exports = self => {
: await manager.update(_req, docToUpdate);
}

await self.setDocAsNotModified(matchingDraft);
if (matchingDraft) {
await self.editDocMongoFields(matchingDraft, {
$set: {
modified: Boolean(importDraftsOnly)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would have put the $set part into the function. Or just have renamed setDocModified to take a boolean.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the function, as it was not really necessary.
Using the mongo connector directly is way clearer I think.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree

});
}
}
},

Expand Down Expand Up @@ -599,7 +661,7 @@ module.exports = self => {
},

async insertOrUpdateDoc(req, {
doc, method = 'insert', failedIds = [], duplicatedDocs = [], existingAposDocId
doc, method = 'insert', failedIds = [], duplicatedDocs = [], existingAposDocId, importDraftsOnly
}) {
const manager = self.apos.doc.getManager(doc.type);
if (existingAposDocId) {
Expand Down Expand Up @@ -637,6 +699,7 @@ module.exports = self => {

async function insert() {
const _req = req.clone({ mode: doc.aposMode });

self.apos.schema.simulateRelationshipsFromStorage(req, doc, manager.schema);

const type = doc.type;
Expand All @@ -647,6 +710,10 @@ module.exports = self => {
fetchRelationships: false
});

if (importDraftsOnly) {
delete docToInsert.lastPublishedAt;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here too, could be done inside getPublishedDocsAsDraft.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

? See my comment above to make it functional inside convertToDraft


if (self.isPage(manager)) {
// TODO: check if this is still true
// `convert` sets the type to `@apostrophecms/home-page`,
Expand All @@ -668,6 +735,9 @@ module.exports = self => {
const _req = req.clone({ mode: doc.aposMode });
self.apos.schema.simulateRelationshipsFromStorage(req, doc, manager.schema);
const docToUpdate = await self.apos.doc.db.findOne({ _id: doc._id });
if (!docToUpdate) {
return;
}
await manager.convert(_req, doc, docToUpdate, {
presentFieldsOnly: true,
fetchRelationships: false
Expand All @@ -683,7 +753,11 @@ module.exports = self => {
await manager.update(_req, docToUpdate);
}
if (doc.aposMode === 'draft') {
await self.setDocAsNotModified(docToUpdate);
await self.editDocMongoFields(docToUpdate, {
$set: {
modified: Boolean(importDraftsOnly)
}
});
}
}
},
Expand All @@ -699,14 +773,8 @@ module.exports = self => {
self.apos.instanceOf(manager, '@apostrophecms/any-page-type');
},

// Manually set `modified: false` because `setModified`
// option is not taken into account in the `update` method.
setDocAsNotModified(doc) {
return self.apos.doc.db.updateOne({ _id: doc._id }, {
$set: {
modified: false
}
});
editDocMongoFields(doc, criteria) {
return self.apos.doc.db.updateOne({ _id: doc._id }, criteria);
},

async insertOrUpdateAttachment(req, {
Expand Down Expand Up @@ -760,6 +828,7 @@ module.exports = self => {
},

async overrideDuplicates(req) {
const importDraftsOnly = self.apos.launder.boolean(req.body.importDraftsOnly);
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);
Expand Down Expand Up @@ -792,7 +861,7 @@ module.exports = self => {

const { docs, attachmentsInfo = [] } = await format.input(exportPath);

const filterDocs = docs.filter(doc => docIds.includes(doc.aposDocId));
let filterDocs = docs.filter(doc => docIds.includes(doc.aposDocId));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could avoid reassigning:

      const filterDocs = importDraftsOnly
        ? self.getPublishedDocsAsDraft(
          docs.filter(doc => docIds.includes(doc.aposDocId))
        )
        : docs.filter(doc => docIds.includes(doc.aposDocId));

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done, and filtering now happens before locale re-writing


const differentDocsLocale = self.getFirstDifferentLocale(req, filterDocs);
const siteHasMultipleLocales = Object.keys(self.apos.i18n.locales).length > 1;
Expand All @@ -804,6 +873,10 @@ module.exports = self => {
self.rewriteDocsWithCurrentLocale(req, filterDocs);
}

if (importDraftsOnly) {
filterDocs = self.getPublishedDocsAsDraft(filterDocs);
}

for (const doc of filterDocs) {
try {
if (attachmentsInfo.length) {
Expand Down Expand Up @@ -840,7 +913,8 @@ module.exports = self => {
method: 'update',
failedIds,
existingAposDocId,
duplicatedDocs
duplicatedDocs,
importDraftsOnly
});

if (existingAposDocId) {
Expand Down
3 changes: 2 additions & 1 deletion lib/methods/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ module.exports = self => {
name: key,
label: value.label,
allowedExtension: value.allowedExtension
}))
})),
importDraftsOnlyDefault: self.options.importDraftsOnlyDefault
};
},
// Filter our docs that have their module with the import or export option set to false
Expand Down
Loading