Feat: Import/export for types with populated components #93

PhilippPaoli · 2023-05-09T08:31:11Z

What does it do?

Change Strapi Query Engine Api to Strapi entityService Api. Add parameter "populate" to plugin config.

Why is it needed?

Export and import types with populated components.

How to test it?

Use this example config:

export default ({ env }) => ({
  "config-sync": {
    enabled: true,
    config: {
      customTypes: [
        {
          configName: "accordion",
          queryString: "api::accordion.accordion",
          uid: "accordionUid",
          populate: [
            "populate.testA.firstLevel",
            "populate.testA.secondLevel",
            "populate.testB,firstLevel",
          ],
        },
      ],
      importOnBootstrap: false,
    },
  },
});

Related issue(s)/PR(s)

#73

Change Strapi Query Engine Api to Strapi entityService Api. Add parameter "populate" to plugin config. Use import and export with components logic.

boazpoolman · 2023-05-09T09:51:50Z

I'll try and test this asap

boazpoolman

Is there a reason you can't use the relations key to include your relations?

We allready do that for the roles/permissions. See:
https://github.com/boazpoolman/strapi-plugin-config-sync/blob/master/server/config/types.js#L31

PhilippPaoli · 2023-06-21T09:40:19Z

This PullRequest is not about a relation between two Single/Collection Types. I want to make sure that Single/CollectionTypes that have components are saved up to the specified level.

Example:

There is a component link:

This component is added in a SingleType to provide multiple links (repeatable):

The content of the SingleType looks like:

The API of the SingleType then looks like:

With the customization from this PullRequest, we can extend the backup to the lower levels (populate):

Without this adjustment you get only the first level:

The plugin config for populate components looks like:

boazpoolman

This PR is great.
Thanks @PhilippPaoli

I've tested your PR with the Footer/Links example and it worked like a charm.

I do have a couple of small comments,
it would be amazing if you could check those out.

On another note:
I've even tried to use the populate option instead of the relations option for the admin-role config type to populate the permissions relation and that worked as well.
I'm trying to figure out why I made this complicated relations option anyways and not go straight with a populate option.

I'll do some more testing, but if everything works out I think we can replace the relations option with populate completely 😮

server/config/type.js

boazpoolman · 2023-06-28T17:18:29Z

server/config/type.js

@@ -221,6 +236,25 @@ const ConfigType = class ConfigType {
        formattedConfig[relationName] = relations;
      }));

+      this.populate
+        .filter((populatedFields) => !populatedFields.includes("."))


Can you tell me why you explicitly exclude populate strings that contain a .?

You can also use populate for multiple levels, for example:

The levels are separated by ".". At this point, we do not need to clean up each levels, the main level is enough.

Ah that makes sense. I guess that's something we would have to write in the README documentation as well.

boazpoolman · 2023-06-28T17:26:27Z

server/config/type.js

+                sanitizeConfig(fields);
+                Object.keys(fields).map((key, index) => {
+                  if (fields[key] && typeof fields[key] === "object") {
+                    sanitizeObjects(fields[key]);


This recursive config sanitization is great!

Could you add this directly to the sanitizeConfig utility.
That way we benefit from this logic on all places where we're using the sanitizeConfig utility.

I have moved the logic according to your request ;)

Thanks. That's great!
There is just one eslint issue with this implementation.

Please see the pipeline output:
https://github.com/boazpoolman/strapi-plugin-config-sync/actions/runs/5819068278/job/15815463391?pr=93

boazpoolman

Thanks @PhilippPaoli for all the improvements you've made!

There are just a couple small issues to resolve and then we can merge this PR.
I'll list my comments here:

The ESLint warnings need to be fixed so that the pipeline passes
We need to write documentation about this in the README under the "Custom types" section
We should rename the config key from populate to components imo.

I'll go a bit more in to detail about that last point.
As I've mentioned in my previous review I would do some testing to see if we can replace the current relations config completely in favor of this population feature. Though sadly that is not possible. Relations still require us to manually create them with seperate DB queries, as apposed to components which can be created inline with the create query of it's parent content type.

Having said that I think it makes sense to rename this config to components. That way we have a clear distinction as to which config we should for the different use cases.

Fixed ESLint Warnings Add Documentation for Components under "Custom types" Change property populate to components

PhilippPaoli · 2023-09-04T11:20:13Z

Thanks @boazpoolman for the comment.

I took your suggestions and described the new property "components" under "Custom Types" a little bit. Probably it would be great to document "relations" as well.
Additionally the EsLint warning is fixed.

boazpoolman

Approved!

This looks great @PhilippPaoli.
Thanks for all your work on this feature.

I'll do a final round of testing soon and then I'll merge and release :)

…sync into comp

…y been sanitized because their parent was santized.

boazpoolman · 2023-09-06T15:09:55Z

Tested the PR and everything seems to work.
Made some small adjustments, but nothing game changing.

Sadly this PR seems to break the integration tests, but I can't figure out why.
Will have to do some further investigation.

The failed pipeline
https://github.com/boazpoolman/strapi-plugin-config-sync/actions/runs/6098423438/job/16548241406?pr=93

PhilippPaoli · 2023-10-09T15:49:08Z

@boazpoolman

Seems to be working. What’s blocking? Anything I can do to support or do to resolve a issue. Though looking at the log at … I do not see what is going wrong.

boazpoolman · 2023-10-09T19:22:34Z

Hi @PhilippPaoli,

Last time I checked I couldn't find out why the integration tests were failing.
I have confirmed that they do work on the master branch, and something in your PR seems to have broken them.
I will do some further investigation soon.

PhilippPaoli · 2023-10-10T09:11:00Z

Hi @boazpoolman,

Could it be that it is still due to version 1.1.2? I have tested the changes in the pull request with version 1.1.3, so far the integration test is completed successfully.

Maybe a rebase could be helpful

Change Strapi Query Engine Api to Strapi entityService Api. Add parameter "populate" to plugin config. Use import and export with components logic.

Fixed ESLint Warnings Add Documentation for Components under "Custom types" Change property populate to components

…y been sanitized because their parent was santized.

…-sync into comp

boazpoolman · 2023-10-10T09:43:02Z

I've rebased your PR, though sadly the integration tests are still failing.
See here the output https://github.com/boazpoolman/strapi-plugin-config-sync/actions/runs/6467296746/job/17557081644?pr=93

I've also made a commit on the master branch and that resulted in a successfull tests run.
See the successfull pipeline here https://github.com/boazpoolman/strapi-plugin-config-sync/actions/runs/6467436741

That means something in this PR is causing this issue.
We just have to do some debugging to see what that is.
Probably just one by one reverting all your changes and as soon as the tests start working again we will have pinpointed the exact changed that caused the issue.

Resolved Integration Test Error

PhilippPaoli · 2023-10-11T16:19:56Z

Hi @boazpoolman,

the integration test should now run successfully.

boazpoolman · 2023-10-13T14:23:59Z

server/config/type.js

@@ -69,7 +69,9 @@ const ConfigType = class ConfigType {
        });

        await Promise.all(relations.map(async (relation) => {
-          await strapi.entityService.delete(this.queryString, relation.id);
+          await strapi.query(queryString).delete({


Amazing, this indeed seems to fix the integration tests.
What do you reckon was the fix?

To me, looking at it, I think prior to this commit you used the wrong query string.
You changed this.queryString to queryString. I believe that was the fix.
Meaning you should still be able to get away with a entityService.delete instead of reverting back to using the strapi.query layer.

Update Query String

boazpoolman

Great!
Thanks for all your hard work @PhilippPaoli
I'll release this soon.

PhilippPaoli · 2023-10-15T15:01:45Z

Hi @boazpoolman,

thanks for merging :)

Modified Strapi Api calls for components

879e890

Change Strapi Query Engine Api to Strapi entityService Api. Add parameter "populate" to plugin config. Use import and export with components logic.

PhilippPaoli changed the title ~~Feature: Import/export for types with populated components~~ Feat: Import/export for types with populated components May 9, 2023

boazpoolman self-requested a review June 19, 2023 12:41

boazpoolman reviewed Jun 19, 2023

View reviewed changes

boazpoolman requested changes Jun 28, 2023

View reviewed changes

Fix small comments

510312d

boazpoolman requested changes Aug 15, 2023

View reviewed changes

Fixed ESLint warnings, change property naming

34426f5

Fixed ESLint Warnings Add Documentation for Components under "Custom types" Change property populate to components

boazpoolman approved these changes Sep 5, 2023

View reviewed changes

boazpoolman added 5 commits September 6, 2023 15:08

Merge branch 'master' of github.com:boazpoolman/strapi-plugin-config-…

0a10b19

…sync into comp

fix: No need to additionally sanitize the components. They've allread…

b952753

…y been sanitized because their parent was santized.

fix: Use strapi.log.warn instead of logging warnings with console.log

5199e7a

fix: Add missing env vars to playground

8e9cbdd

docs: Write docs about the relations feature

f667788

PhilippPaoli and others added 7 commits October 10, 2023 11:20

Modified Strapi Api calls for components

69597c7

Change Strapi Query Engine Api to Strapi entityService Api. Add parameter "populate" to plugin config. Use import and export with components logic.

Fix small comments

378cd7b

Fixed ESLint warnings, change property naming

c269b48

Fixed ESLint Warnings Add Documentation for Components under "Custom types" Change property populate to components

fix: No need to additionally sanitize the components. They've allread…

0c2086e

…y been sanitized because their parent was santized.

fix: Use strapi.log.warn instead of logging warnings with console.log

c0582bd

fix: Add missing env vars to playground

b35817e

Merge branch 'master' of github.com:PhilippPaoli/strapi-plugin-config…

ca41ec0

…-sync into comp

Update type.js

10ea389

Resolved Integration Test Error

boazpoolman reviewed Oct 13, 2023

View reviewed changes

Update type.js

d3bcb1c

Update Query String

boazpoolman approved these changes Oct 14, 2023

View reviewed changes

boazpoolman merged commit 4581bf1 into pluginpal:master Oct 14, 2023
6 checks passed

boazpoolman mentioned this pull request Oct 20, 2023

fix: Remove recursive config sanitisation #113

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat: Import/export for types with populated components #93

Feat: Import/export for types with populated components #93

PhilippPaoli commented May 9, 2023

boazpoolman commented May 9, 2023

boazpoolman left a comment

PhilippPaoli commented Jun 21, 2023

boazpoolman left a comment

boazpoolman Jun 28, 2023

PhilippPaoli Aug 10, 2023

boazpoolman Aug 15, 2023

boazpoolman Jun 28, 2023

PhilippPaoli Aug 10, 2023

boazpoolman Aug 15, 2023

boazpoolman left a comment

PhilippPaoli commented Sep 4, 2023

boazpoolman left a comment

boazpoolman commented Sep 6, 2023

PhilippPaoli commented Oct 9, 2023

boazpoolman commented Oct 9, 2023

PhilippPaoli commented Oct 10, 2023

boazpoolman commented Oct 10, 2023

PhilippPaoli commented Oct 11, 2023

boazpoolman Oct 13, 2023

boazpoolman left a comment

PhilippPaoli commented Oct 15, 2023

Feat: Import/export for types with populated components #93

Feat: Import/export for types with populated components #93

Conversation

PhilippPaoli commented May 9, 2023

What does it do?

Why is it needed?

How to test it?

Related issue(s)/PR(s)

boazpoolman commented May 9, 2023

boazpoolman left a comment

Choose a reason for hiding this comment

PhilippPaoli commented Jun 21, 2023

boazpoolman left a comment

Choose a reason for hiding this comment

boazpoolman Jun 28, 2023

Choose a reason for hiding this comment

PhilippPaoli Aug 10, 2023

Choose a reason for hiding this comment

boazpoolman Aug 15, 2023

Choose a reason for hiding this comment

boazpoolman Jun 28, 2023

Choose a reason for hiding this comment

PhilippPaoli Aug 10, 2023

Choose a reason for hiding this comment

boazpoolman Aug 15, 2023

Choose a reason for hiding this comment

boazpoolman left a comment

Choose a reason for hiding this comment

PhilippPaoli commented Sep 4, 2023

boazpoolman left a comment

Choose a reason for hiding this comment

boazpoolman commented Sep 6, 2023

PhilippPaoli commented Oct 9, 2023

boazpoolman commented Oct 9, 2023

PhilippPaoli commented Oct 10, 2023

boazpoolman commented Oct 10, 2023

PhilippPaoli commented Oct 11, 2023

boazpoolman Oct 13, 2023

Choose a reason for hiding this comment

boazpoolman left a comment

Choose a reason for hiding this comment

PhilippPaoli commented Oct 15, 2023