Skip to content
This repository has been archived by the owner on Dec 27, 2022. It is now read-only.

Add support for user content type and file content type #276

Merged
merged 2 commits into from
Feb 9, 2022

Conversation

soupette
Copy link
Contributor

@soupette soupette commented Feb 9, 2022

Signed-off-by: soupette [email protected]

Allow to query user content type and file content types.

@soupette soupette requested a review from remidej February 9, 2022 08:21
Copy link
Contributor

@remidej remidej left a comment

Choose a reason for hiding this comment

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

Very interesting PR! it don't see what it does about the user content type though 🤔


const nodeType = makeParentNodeName(ctx.schemas, uid);
if (uid === 'plugin::upload.file') {
for (let file of data[i]) {
Copy link
Contributor

@remidej remidej Feb 9, 2022

Choose a reason for hiding this comment

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

I'm curious if it would be possible to use parallelization for all these loops? With Promise.all

it could have some positive impact on build time

Copy link
Contributor Author

@soupette soupette Feb 9, 2022

Choose a reason for hiding this comment

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

We are already doing it for all entities of a content type.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For the file model it's a bit different I don't want to create children node but just to apply the image processing, I am not even sure we should support it at the moment, it does not really makes sense for query all the files in gatsby.

WDYT about removing it completely?

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 the previous plugin didn't have it, and I don't remember anyone in the community asking for it. If it requires special treatment (like no query params etc.) we might as well remove I think

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perfect!

It could always be added later if the community needs it anyway

src/helpers.js Outdated
kind,
uid,
endpoint: '/api/upload/files',
queryParams: {},
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it necessary to block user query params here? I don't know if it's possible today, but at some point we may have images with support for localization or draft & publish

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The API response is not the same for the files and the file model does not create any children node so for the moment I have jsut added the support and I'll let the expansion deal with what you think is best.

@@ -151,12 +151,13 @@ export const cleanAttributes = (attributes, currentSchema, schemas) => {
* @param {Object} ctx
* @returns {Object}
*/
export const cleanData = ({ id, attributes }, ctx) => {
export const cleanData = ({ id, attributes, ...rest }, ctx) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this because the /files endpoint doesn't have the v4 response format?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes and similarly for the user content type ....

Signed-off-by: soupette <[email protected]>
@soupette soupette merged commit e1e6116 into init-migration Feb 9, 2022
@soupette soupette deleted the fix-users-content-type branch February 9, 2022 11:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants