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.

Sign up for GitHub

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

PRO-6561: export documents which are related via rich text widget "internal pages" links and inline images #98

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

PRO-6561: export documents which are related via rich text widget "internal pages" links and inline images #98

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
dd0fe7c
wip
boutell Nov 20, 2024
8b0d4ab
working
boutell Nov 20, 2024
4a274b9
refactored
boutell Nov 20, 2024
62822a8
wip
boutell Nov 25, 2024
c331738
tests passing
boutell Dec 2, 2024
524b5fe
test cleanup, lint
boutell Dec 2, 2024
6973fb1
cleanup
boutell Dec 9, 2024
7c43d5a
a lot of changes around page types to satisfy tests & ensure function…
boutell Dec 12, 2024
6b9794f
allow both valid aliases for "any page type"
boutell Dec 12, 2024
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
7 changes: 7 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
# Changelog

## UNRELEASED

### Adds

* Pages linked a page via the "Internal Page" option in the rich text editor are now candidates to be exported as related documents.
* Images embedded inline in rich text widgets via the `insert: [ 'image' ]` option are now candidates to be exported as related documents.

## 2.5.0 (2024-11-08)

### Adds
Expand Down
133 changes: 123 additions & 10 deletions lib/methods/export.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ module.exports = self => {

const docs = (await self.getDocs(req, ids, hasRelatedTypes, manager, reporting))
.map((doc) => self.apos.util.clonePermanent(doc));

if (!hasRelatedTypes) {
return self.exportFile(
req,
Expand Down Expand Up @@ -210,14 +209,23 @@ module.exports = self => {
mode = doc.aposMode || req.mode
}) {
recursion++;
if ((doc.type === '@apostrophecms/rich-text') && (type === 'relationship')) {
await self.getRelatedDocsFromRichTextWidget(req, {
doc,
relatedTypes,
storedData,
recursion,
mode
});
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that in that case schema will always be an empty array?
Maybe we can just return this function getRelatedDocsFromRichTextWidget here since there nothing else to do with it?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm leaving this in place because at least in A2 we eventually created situations where the rich text widget also had a conventional schema. It's at least possible so this is a little more future proof.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok makes sense then

}
for (const field of schema) {
const fieldValue = doc[field.name];
const shouldRecurse = recursion <= MAX_RECURSION;

if (!field.withType && !fieldValue) {
continue;
}
if (field.withType && relatedTypes && !relatedTypes.includes(field.withType)) {
if (field.withType && relatedTypes && !relatedTypesIncludes(field.withType)) {
continue;
}
if (field.withType && !self.canExport(req, field.withType)) {
Expand Down Expand Up @@ -282,6 +290,70 @@ module.exports = self => {
});
}
}

function relatedTypesIncludes(name) {
name = normalizeName(name);
return relatedTypes.includes(name);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I know it's not a big deal here, but if we can take the habit to avoid reassigning stuff like params it would be nice.
Also I know we discussed the fact to introduce a new eslint rule for that, so let's save future refacto time.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

},

async getRelatedDocsFromRichTextWidget(req, {
doc,
relatedTypes,
storedData,
recursion,
mode
}) {
const linkedDocs = await self.apos.doc.db.find({
aposDocId: {
$in: doc.permalinkIds
}
}).project({
type: 1,
aposDocId: 1,
slug: 1
}).toArray();
const linkedIdsByType = new Map();
for (const linkedDoc of linkedDocs) {
// Normalization is a little different here because these
// are individual pages or pieces
const name = linkedDoc.slug.startsWith('/') ? '@apostrophecms/any-page-type' : linkedDoc.type;
const linkedIds = linkedIdsByType.get(name) || new Set();
linkedIds.add(linkedDoc.aposDocId);
if (linkedIds.size === 1) {
linkedIdsByType.set(name, linkedIds);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be clearer to check if key exist in map using map has method.:

      for (const linkedDoc of linkedDocs) {
        // Normalization is a little different here because these
        // are individual pages or pieces
        const docType = linkedDoc.slug.startsWith('/')
          ? '@apostrophecms/any-page-type'
          : linkedDoc.type;
        const isTypeStored = linkedIdsByType.has(docType);
        const linkedIds = isTypeStored ? linkedIdsByType.get(docType) : new Set();
        linkedIds.add(linkedDoc.aposDocId);
        if (!isTypeStored) {
          linkedIdsByType.set(name, linkedIds);
        }
      }

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

}
if (doc.imageIds?.length > 0) {
linkedIdsByType.set('@apostrophecms/image', new Set(doc.imageIds));
}
const virtualDoc = {
type: '@apostrophecms/rich-text_related'
};
const virtualSchema = [];
for (const [ linkedType, linkedIds ] of linkedIdsByType.entries()) {
const baseName = self.apos.util.slugify(linkedType);
const fieldName = `_${baseName}`;
const idsStorage = `${baseName}Ids`;
virtualSchema.push({
name: fieldName,
type: 'relationship',
withType: linkedType,
idsStorage
});
const ids = [ ...linkedIds.values() ];
virtualDoc[idsStorage] = ids;
virtualDoc[fieldName] = ids.map(id => ({ aposDocId: id }));
}
await self.getRelatedDocsFromSchema(req, {
doc: virtualDoc,
schema: virtualSchema,
relatedTypes,
storedData,
type: 'relationship',
recursion,
mode
});
},

async handleRelatedField(req, {
Expand Down Expand Up @@ -478,35 +550,76 @@ module.exports = self => {
},

getRelatedTypes(req, schema = [], related = []) {
return findSchemaRelatedTypes(schema, related);

findSchemaRelatedTypes(schema, related);
return related;
function findSchemaRelatedTypes(schema, related, recursions = 0) {
recursions++;
if (recursions >= MAX_RECURSION) {
return related;
return;
}
for (const field of schema) {
if (
field.type === 'relationship' &&
self.canExport(req, field.withType) &&
!related.includes(field.withType)
) {
related.push(field.withType);
pushRelated(related, 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`];
for (const [ widget, options ] of Object.entries(widgets)) {
const schema = self.apos.area.getWidgetManager(widget).schema;
if (widget === '@apostrophecms/rich-text') {
self.getRelatedTypesFromRichTextWidget(req, {
options,
related
});
}
findSchemaRelatedTypes(schema, related, recursions);
}
}
}

return related;
}
},
// Does not currently utilize req, but it could be relevant in overrides and is
// always the first argument by convention, so it is included in the signature
getRelatedTypesFromRichTextWidget(req, {
options,
related
}) {
const manager = self.apos.modules['@apostrophecms/rich-text-widget'];
const rteOptions = {
...manager.options.defaultOptions,
...options
};
if ((rteOptions.toolbar?.includes('image') || rteOptions.insert?.includes('image')) && !related.includes('@apostrophecms/image')) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please indentation:

      if (
        (rteOptions.toolbar?.includes('image') || rteOptions.insert?.includes('image')) &&
        !related.includes('@apostrophecms/image')
      ) {

Copy link
Member Author

Choose a reason for hiding this comment

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

done

pushRelated(related, '@apostrophecms/image');
}
if (rteOptions.toolbar?.includes('link')) {
for (const name of manager.linkFields.linkTo.choices.map(choice => choice.value)) {
if (self.apos.doc.getManager(name) && !related.includes(name)) {
pushRelated(related, name);
}
}
}
}
};
};

function normalizeName(name) {
if (name === '@apostrophecms/page') {
return '@apostrophecms/any-page-type';
} else {
return name;
}
}

function pushRelated(related, name) {
name = normalizeName(name);
if (!related.includes(name)) {
related.push(name);
}
}
1 change: 0 additions & 1 deletion lib/methods/import.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -705,7 +705,6 @@ module.exports = self => {

const type = doc.type;
const docToInsert = doc;

await manager.convert(_req, doc, docToInsert, {
presentFieldsOnly: true,
fetchRelationships: false
Expand Down
Loading
Loading