PRO-6561: export documents which are related via rich text widget "internal pages" links and inline images #98
Conversation
| linkedIds.add(linkedDoc.aposDocId); | ||
| if (linkedIds.size === 1) { | ||
| linkedIdsByType.set(name, linkedIds); | ||
| } |
There was a problem hiding this comment.
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);
}
}| function relatedTypesIncludes(name) { | ||
| name = normalizeName(name); | ||
| return relatedTypes.includes(name); | ||
| } |
There was a problem hiding this comment.
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.
| storedData, | ||
| recursion, | ||
| mode | ||
| }); |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
lib/methods/export.js
Outdated
| ...manager.options.defaultOptions, | ||
| ...options | ||
| }; | ||
| if ((rteOptions.toolbar?.includes('image') || rteOptions.insert?.includes('image')) && !related.includes('@apostrophecms/image')) { |
There was a problem hiding this comment.
Please indentation:
if (
(rteOptions.toolbar?.includes('image') || rteOptions.insert?.includes('image')) &&
!related.includes('@apostrophecms/image')
) {
| // 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)); |
There was a problem hiding this comment.
Why not checking types direclty in the find request above?
| }, | ||
| 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); |
There was a problem hiding this comment.
This line is 184 chars 😮 ahah
Please indentate
Summary
See changelog and title.
What are the specific steps to test this change?
insert: [ 'image' ]to rich text widget config, add an inline image, add an "Internal Page" link to another page — do not paste a URL, that does not count)What kind of change does this PR introduce?
(Check at least one)
Make sure the PR fulfills these requirements:
If adding a new feature without an already open issue, it's best to open a feature request issue first and wait for approval before working on it.
Other information: