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

Deep copy does not work as expected when using component types #52

Closed
whitetown opened this issue Jun 15, 2023 · 14 comments · Fixed by #63
Closed

Deep copy does not work as expected when using component types #52

whitetown opened this issue Jun 15, 2023 · 14 comments · Fixed by #63
Labels
bug Something isn't working question Further information is requested

Comments

@whitetown
Copy link

whitetown commented Jun 15, 2023

Using v1.0.0-rc0, trying to deep-copy a content-type which has component types results in an undefined error.
See #52 (comment) for more information.


Original text:

What could be a reason that DeepCopyButton is not shown?
The plugin is installed (I can see it in the list of plugins),
config/plugins.js is updated,
but still nothing.
yarn build, yarn develop - no matter what I ran - no result.
latest strapi (4.9.2 ... 4.11.1)

@trijpstra-fourlights
Copy link
Contributor

Can you share the relevant part of your config/plugins.js

e.g.

{
  // ...
  'deep-copy': {
    enabled: true,
    config: {
      contentTypes: {
        'api::page.page': true,
      },
    },
  },
  // ...
}

contentTypes are opt-in, so if you don't have the the proper contentTypes defined then the button will not be shown.

@trijpstra-fourlights trijpstra-fourlights added the question Further information is requested label Jun 23, 2023
@whitetown
Copy link
Author

The config file is below:

`module.exports = ({ env }) => ({

'deep-copy': {
    enabled: true,
    config: {
        contentTypes: {
            'api::page.page': true,
            'api::block.block': true,
            'api::footer.footer': true,
            'api::web-app-image.web-app-image': true,
            'api::blog-article.blog-article': true,
        },
    },
},

})`

finally, I was able to show the button. we have a few configs in env/development and env/production.
so finally I just moved the config file to the root folder.

Screenshot 2023-06-23 at 09 23 03

hovewer it does not copy. it seems it cannot read a data here:
const { layout, initialData, isSingleType } = useCMEditViewDataManager()
and crashes:

error: Cannot read properties of undefined (reading 'data')
TypeError: Cannot read properties of undefined (reading 'data')
at ..../node_modules/@fourlights/strapi-plugin-deep-copy/dist/server/utils/prepareForCopy.js:42:45
...
at async Object.deepCopy (..../node_modules/@fourlights/strapi-plugin-deep-copy/dist/server/controllers/core.js:6:20)

@trijpstra-fourlights
Copy link
Contributor

Good to read that you managed to get the button to appear.

From the screenshot and the error message, I think you're hitting the current limits on the plugin.
Some fields / properties on the content-type are hardcoded. I've created #29 a while ago to track progress on improving that.

Would you be willing to create a minimal repro (either in GitHub, or in something like StackBlitz) so that I can investigate further? You could use the playground in this repository as a template.

If that's too much work, can you share the "offending" content-type definition?

@whitetown
Copy link
Author

It seems the issue if any collection uses "Component".
I can copy collections with internal types or relations to other collections. but as soon as I add a Component to a Collection-Type - it fails.

Screenshot 2023-06-24 at 11 33 17

@trijpstra-fourlights
Copy link
Contributor

Thanks! I'll take a look later today and implement a fix. Will keep you posted.

@trijpstra-fourlights
Copy link
Contributor

I've just merged my work on the configurability changes.
Please note that the config API has changed a bit. Take a look at the current README for more information.

Could you try the latest version directly from github? You can do this by updating your package.json to:

  "strapi-plugin-deepcopy": "github:Four-Lights-NL/strapi-plugin-deepcopy#main"

@whitetown
Copy link
Author

I have tried:

  • changed my config according to the readme
  • installed the package from 'main'
  • built it (it was missing /dist/*

so now I see in the console the following:
[2023-06-26 12:35:29.548] http: GET /content-manager/collection-types/api::page.page/10 (33 ms) 200
[2023-06-26 12:35:29.572] http: GET /deep-copy/contentTypes (2 ms) 404

    'deep-copy': {
        enabled: true,
        config: {
            contentTypes: {
                'api::page.page': {
                    enabled: true,
                    showButtonInAdmin: true,
                },
                'api::block.block': {
                    enabled: true,
                    showButtonInAdmin: true,
                },
etc

@trijpstra-fourlights
Copy link
Contributor

Thanks for getting back to me.

It's weird that you have to build it manually, as the prepublish command should have taken care of that automatically when pointing npm to the github repository.

It seems that the plugin wasn't built properly on your machine:

The fact that you're getting 404 on deep-copy/contentTypes shows that it's not using the latest version of the (admin) plugin.
The fact that is giving a 404 means it is using the latest version of the server plugin.

The url is deep-copy/content-types (see: https://github.com/Four-Lights-NL/strapi-plugin-deepcopy/blob/main/admin/components/DeepCopyButton/index.tsx#L69).

It really should be working directly from npm because of the prepublish command. Having dist in the repository is an anti-pattern.

Can you try the following:

  • Remove node modules (and your lock file) and do a clean npm install
  • Verify that the dist directory has been created automatically
  • If so, check if it's working in the admin.

If it's not building automatically:

  • make sure you build both the server and the admin part (see README for instructions)
  • start your admin with --watch-admin to make sure the admin sees any changes to plugins
  • check if it's working in the admin.

Thanks for bearing with me through this. Hope we can get it working.

@trijpstra-fourlights
Copy link
Contributor

trijpstra-fourlights commented Jun 26, 2023

Never mind the above, it seems I screwed up something in the build.
I'll see if I can fix it and publish as a pre-release package so that you can use it directly. Will keep you posted.

Okay, I've published the package as @fourlights/strapi-plugin-deepcopy@1.0.0-rc.0

Can you try that one? npm i @fourlights/strapi-plugin-deep-copy@1.0.0-rc.0

@whitetown
Copy link
Author

it's better. I see the button, the dialog. but getting this:
[2023-06-26 14:26:49.688] error: Cannot read properties of undefined (reading 'uniqueFields')
in node_modules/@fourlights/strapi-plugin-deep-copy/dist/server/utils/prepareForCopy.js:70:49

@trijpstra-fourlights
Copy link
Contributor

Hmm.. I'm having trouble making any sense of that error...

Line 70 in prepareForCopy.js should not be calling anything related to uniqueFields, see https://www.npmjs.com/package/@fourlights/strapi-plugin-deep-copy/v/1.0.0-rc.0?activeTab=code

The only part that's doing anything with uniqueFields is line 21 till 32.
reading properties of undefined would mean that the config object is undefined, but that should be impossible as any call to prepareForCopy (either directly or recursively) is guaranteed to always have a valid contentType (and thus, a config).

I'm assuming you've tried this with a fresh start of the admin and node modules? It's pretty weird that the line number is so out of sync with the packaged files.

It's disappointing that we can't seem to get it working.
I'm using the plugin in production and it's working correctly there, as it also is in the playground in the repository.

Maybe it has something to do with not having any unique fields in your content type definition. That should be pretty easy to fix on my side then.

Would you be willing to share your content type definitions + configs? It would make it easier for me to reproduce the error and fix it.

In any case, I'll continue my investigation. Let me know if you have any new info as well.

@whitetown
Copy link
Author

ok. one more time:

  • removed node_modules and yarn.lock
  • checked that package.json contains @fourlights/strapi-plugin-deep-copy@1.0.0-rc.0
  • yarn
  • checked that the folder ./dist exists
  • yarn develop
  • refreshed browser, tried to copy
  • got error: Cannot read properties of undefined (reading 'uniqueFields')

prepareForCopy, line 21: config.uniqueFields
prepareForCopy, line 7: const config = contentTypes[contentType];
however I do not have uniqueFields in my config file

even if I added this
defaultUniqueFieldValue: (strapi, src, name) => ${src[name]} (${new Date().toIsoFormat()}),
uniqueFields: {},
I got the same error.
so I went to the dist/.../prepareForCopy.js and added a console.log(...)
I discovered that config is undefined for the "Component" inside the collection-type

so ;-) conclusion - it still does not work for "Component"s

@trijpstra-fourlights
Copy link
Contributor

Thanks for your analysis.

It seems there is a regression for component types. I'll label this as a bug.

For now, you can (probably?) work around this regression by defining the component types in the plugin config as well e.g.:

  "component-name": {
    "enabled": true,
    "showButtonInAdmin": false
  }

However, as component types don't live as entities in Strapi, they should be implicitly enabled as deep-copyable in the plugin. So I'll create a PR for that to fix that behavior. I'll update this issue accordingly.

@trijpstra-fourlights trijpstra-fourlights added the bug Something isn't working label Jun 26, 2023
@trijpstra-fourlights trijpstra-fourlights changed the title DeepCopyButton does not appear Deep copy does not work as expected when using component types Jun 26, 2023
@trijpstra-fourlights
Copy link
Contributor

This should be fixed in https://github.com/Four-Lights-NL/strapi-plugin-deepcopy/tree/v1.0.0-rc.1

If you still encounter problems, please create a proper, shareable, reproduction of the problem using stackblitz or a similar tool.
Without the actual content-types being used, it's a guessing game on what is going wrong.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants