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

Merged
merged 12 commits into from
Jan 6, 2025
Merged

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 10 commits
Commits
Show all changes
12 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
b8ab23f
do not use it.only
boutell Dec 17, 2024
2bad787
skip inappropriate page type
boutell Jan 2, 2025
dc2e632
fix tests
boutell Jan 2, 2025
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
175 changes: 147 additions & 28 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,80 @@ module.exports = self => {
});
}
}

function relatedTypesIncludes(name) {
if ([ '@apostrophecms/any-page-type', '@apostrophecms/page' ].includes(name)) {
return relatedTypes.some(type => {
const module = self.apos.modules[type];
return self.apos.instanceOf(module, '@apostrophecms/page-type');
});
}
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
}) {
let linkedDocs = await self.apos.doc.db.find({
aposDocId: {
$in: doc.permalinkIds
}
}).project({
type: 1,
aposDocId: 1,
slug: 1
}).toArray();
// We're likely going to fetch them all with an @apostrophecms/any-page-type query, so we
// need to do our real related types check early or we'll allow all page types
// whenever we allow even one
linkedDocs = linkedDocs.filter(doc => relatedTypes.includes(doc.type));
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not checking types direclty in the find request above?

Copy link
Member Author

@boutell boutell Jan 2, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This logic is sound and I would prefer not to rework it at this point, I'd rather keep the simplicity. Only document ids that were actually linked are going to be queried for, so it's not critical to turn this into a mongo filter as long as we do filter it here.

const linkedIdsByType = new Map();
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(docType, 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 @@ -476,36 +558,73 @@ module.exports = self => {
timeoutId
};
},

// The entry point. Modifies `related`, also returns `related` because code elsewhere
// expects that
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);
self.findSchemaRelatedTypes(req, schema, related, 0);
return related;
},
// Called recursively for you. Modifies `related`, has no useful return value
findSchemaRelatedTypes(req, schema, related, recursions) {
recursions++;
if (recursions >= MAX_RECURSION) {
return;
}
for (const field of schema) {
if (
field.type === 'relationship' &&
self.canExport(req, field.withType) &&
!related.includes(field.withType)
) {
self.pushRelatedType(req, related, field.withType, recursions);
} else if ([ 'array', 'object' ].includes(field.type)) {
self.findSchemaRelatedTypes(req, field.schema, related, recursions);
} else if (field.type === 'area') {
const widgets = self.apos.area.getWidgets(field.options);
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, recursions);
}
self.findSchemaRelatedTypes(req, schema, related, recursions);
}
}
}
},
pushRelatedType(req, related, type, recursions) {
if ((type === '@apostrophecms/page') || (type === '@apostrophecms/any-page-type')) {
const pageTypes = Object.entries(self.apos.doc.managers).filter(([ name, module ]) => self.apos.instanceOf(module, '@apostrophecms/page-type')).map(([ name, module ]) => name);
Copy link
Contributor

Choose a reason for hiding this comment

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

This line is 184 chars 😮 ahah
Please indentate

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.

for (const type of pageTypes) {
self.pushRelatedType(req, related, type, recursions);
}
return;
}
if (!related.includes(type)) {
related.push(type);
const relatedManager = self.apos.doc.getManager(type);
self.findSchemaRelatedTypes(req, relatedManager.schema, related, recursions);
}
},
// 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, recursions) {
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')
) {
self.pushRelatedType(req, related, '@apostrophecms/image', recursions);
}
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)) {
self.pushRelatedType(req, related, name, recursions);
}
}

return related;
}
}
};
Expand Down
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