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 custom populate field for export data #7

Merged
merged 11 commits into from Jun 27, 2022

Conversation

florianmarical
Copy link
Contributor

Hello,

First I want to say thank you for your work. I did a fork for add a custom populate field. indeed the default populate value (*) not work for me because I use a repeatable component with relations.

It the result with the default populate value : *
result1

It the result with the custom value : IngredientToRecipe, IngredientToRecipe.ingredient, IngredientToRecipe.unit
result2

I don't know if this is interesting and if it can be useful to other people

The export Modal with the new field
exportModal

you are did a great plugin

@Baboo7
Copy link
Owner

Baboo7 commented Jun 12, 2022

PR is clean ✨

I wonder though if this shouldn't be the default behavior when exporting data. For example, I noticed that translations are not properly exported when part of a relation but I think this can and should be done automatically.

I appreciate your work, and I'll dig into automatic export of relations' relations. I want this tool to be used by non tech people as well, so knowing the underlying data structure shouldn't be a requirement to use it.

@florianmarical
Copy link
Contributor Author

florianmarical commented Jun 12, 2022

Thanks you for the response.

I'm agree with you about this tool should be used by non tech people. I find this post during my searchs :
https://forum.strapi.io/t/strapi-v4-populate-media-and-dynamiczones-from-components/12670/3

this helper use the schema.json and construct a dynamic populate but that not work in my case (maybe I did some errors during my tests). but it's seem interesting.

@florianmarical
Copy link
Contributor Author

Hello

I tried again tonight the method seen on this post : https://forum.strapi.io/t/strapi-v4-populate-media-and-dynamiczones-from-components/12670/3
It works well and creates a populate dynamically based on the schema.json.

In my project it seems to work well

@Baboo7
Copy link
Owner

Baboo7 commented Jun 15, 2022

I just tested and it doesn't work for me. Dynamic zones and components are well exported but relations don't appear anymore.
It seems promising though, and this logic should be applied to relations as well to export their dynamic zones and components.

@florianmarical
Copy link
Contributor Author

hello,

Sorry for that, I didn't saw. I did try to fix that, it's better but not perfect.

admin/src/api/exportProxy/ExportProxy.js Outdated Show resolved Hide resolved
admin/src/api/exportProxy/ExportProxy.js Outdated Show resolved Hide resolved
server/controllers/export.js Outdated Show resolved Hide resolved
server/controllers/export.js Outdated Show resolved Hide resolved
server/controllers/export.js Outdated Show resolved Hide resolved
server/controllers/export.js Outdated Show resolved Hide resolved
server/controllers/export.js Outdated Show resolved Hide resolved
server/controllers/export.js Outdated Show resolved Hide resolved
server/controllers/export.js Outdated Show resolved Hide resolved
server/controllers/export.js Outdated Show resolved Hide resolved
…erty, simplified function getPopulateFromSchema by processing, clarify variable name
@florianmarical
Copy link
Contributor Author

Thank you for your time and your analysis. I have made the changes. It does make the code much clearer and simpler.

Thank you again for your help.

Copy link
Owner

@Baboo7 Baboo7 left a comment

Choose a reason for hiding this comment

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

Some more changes too be made ;) It's getting better and better

I see some regression in the PR (see my comment line 96). For better code quality, you should setup a small strapi project and test your plugin code to be sure that it works has expected before requesting a review

server/controllers/export.js Outdated Show resolved Hide resolved
server/controllers/export.js Outdated Show resolved Hide resolved
@florianmarical
Copy link
Contributor Author

Hi
I followed your recommendations and installed a small strapi locally with the plugin. It seems to work fine following your latest recommendations.

But I wanted to resolve the conflicts and commit but I get the message : Merging is blocked
I hope I didn't do anything wrong (this is my first pull request, I'm not used to it, sorry).

@Baboo7
Copy link
Owner

Baboo7 commented Jun 27, 2022

It seems it works as expected. It's a good thing you tested on a local repo before pushing 👍

The merging was blocked because protections are set on the branch to prevent unintended merge. You did nothing wrong ;) PR is merged

@Baboo7 Baboo7 merged commit 8a8f3df into Baboo7:master Jun 27, 2022
@florianmarical
Copy link
Contributor Author

think you for your advice and help :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants