Skip to content
This repository has been archived by the owner on Oct 7, 2024. It is now read-only.

[Solution found, needs review from Strapi] Multi-word Collection Types with many-to-many Relationships are skipped #87

Open
MO-Lewis opened this issue Jan 25, 2023 · 4 comments
Assignees
Labels
issue: bug Issue reporting a bug severity: medium If it breaks the basic use of the product but can be worked around source: v3-SQL to v4-SQL status: pending reproduction Waiting for free time to reproduce the issue, or more information

Comments

@MO-Lewis
Copy link

Bug report

Required System information

  • Node.js version: v16.17.0
  • NPM version: v8.15.0
  • Source Strapi version: v3.5.0
  • Target Strapi version: v4.5.5
  • Source Database: MySQL
  • Target Database: MySQL
  • Operating system: Windows 10 64-bit
  • Which script are you running: v3-sql-v4-sql (using the MySQL .env files)

Describe the bug (TL;DR Version)

Please see here: https://github.com/strapi/migration-scripts/blob/main/v3-sql-v4-sql/migrate/helpers/relationHelpers.js#L49-L50

ManyToMany relationships in CollectionTypes with hyphens (e.g. Care-roles) are not migrated properly (screenshots in the full description below).

The following lines need to have the value of modelF and attributeF wrapped in snakeCase(), like so:

addRelation(
        {
          uid,
          model: collectionName,
          attribute: key,
          type: 'manyToMany',
          modelF: snakeCase(value.collection),
          attributeF: snakeCase(value.attribute),
        },
        relations
      );

Through testing on my side, wrapping just modelF seems to work. However, no better / worse behavior was observed when adding it to attribtueF, so ideally I'd need a second pair of eyes from the Strapi team to check for any consequences when adding it to the attributeF field.

Describe the bug (Full Version)

I have two CollectionTypes that interact with one another, articles and care-roles. They hold a many-to-many relationship, where many articles can hold many care-roles.

When I run the migration scripts, I seem to lose all relationships relating to the care-roles collection. I would expect to see something like this in the migration script console output, when run:

Migrating X items from articles_care_roles__care_roles_articles to articles_care_roles_links

However, no message appears. This means the migration script has skipped over it for some reason. I spent a few hours debugging the migration script in VSCode line-by-line, and found the cause of it.

image

As shown in this screenshot, this will mean that the migrations script is looking for a table called articles_care-roles__care-roles_articles (note the hyphens), which doesn't exist, and so is skipped over.

Whilst looking for potential fixes to this, I could see that the one-to-many addRelation() function uses snakeCase(). This sounded like something I needed for my many-to-many relationship. I added it like so and re-ran the script:

image

This seemed to fix the issue and all of my relationships migrated properly upon re-run! I ran this fix twice, once with just the modelF value snakecased, and once with both modelF and attributeF snakecased.

There appears to be no difference in behavior; both fixed the issue properly, which makes me wonder if I'm potentially causing any other issues under-the-hood with this.

It would be much appreciated if I could have this reviewed by someone on the Strapi team, as I've found the solution and would like this to be reviewed and a fix merged into the repo, but I've been trying to contact the Strapi team for ~3 weeks to raise this issue, to no avail.

Steps to reproduce the behavior

  1. Create a Strapi V3 App.
  2. Create two Collection Types called "Article" and "Care-role".
  3. Form a many-to-many relationship between them.
  4. Add some test data
  5. Run CodeMods to upgrade it to V4.
  6. Run the migration scripts on the DB to upgrade it to V4.
  7. All relationships involving the Care-roles table should be skipped over, due to the reasons stated above.

Expected behavior

The Care-roles relationships should be migrated properly, outputting the following message in the console:

Migrating X items from articles_care_roles__care_roles_articles to articles_care_roles_links

Screenshots

Relevant screenshots included above, please see my messages in the Strapi discord for more screenshots:

Code snippets

Code samples AND pending solution posted above.

Additional context

This is pretty-much solved and I just need this solution reviewed and merged into the main repo by a member of the Strapi team. The only reason I haven't made this a PR is because I'm unsure whether attributeF requires the snake-case fix, so I'd like someone from the Strapi team to review the change to ensure no further issues happen with a result.

I'll be running this eventually on a real client's database, so having this fix officially merged in would be a real confidence-booster in running the script against their real database in a few weeks time.

@MO-Lewis MO-Lewis changed the title [Solution found, needs review] Multi-word Collection Types with many-to-many Relationships are skipped [Solution found, needs review from Strapi] Multi-word Collection Types with many-to-many Relationships are skipped Jan 25, 2023
@cpaczek
Copy link

cpaczek commented Jan 30, 2023

@derrickmehaffy this user in discord asked if someone could give a sanity check on this issue to make sure they are not missing anything. I looked over it and it looks logical but I don't know enough about the v3 db layer.

@MO-Lewis
Copy link
Author

Hi Cpaczek, thanks for chasing this up! The main question I have for Derrick (or any of the team) is:

Would the fix need to be:

modelF: snakeCase(value.collection),
attributeF: snakeCase(value.attribute),

or

modelF: snakeCase(value.collection),
attributeF: value.attribute,

Again, thanks for your time today! I hope this fix can go to main to help future users 😄

@geeky-biz
Copy link

geeky-biz commented Feb 23, 2023

I'm not from Strapi team but just letting know - for us, just changing the modelF: snakeCase(value.collection), resolved the issue of many-to-many relationships not getting migrated for collections with hyphen.

@derrickmehaffy derrickmehaffy self-assigned this Feb 23, 2023
@derrickmehaffy derrickmehaffy added issue: bug Issue reporting a bug severity: medium If it breaks the basic use of the product but can be worked around source: v3-SQL to v4-SQL status: pending reproduction Waiting for free time to reproduce the issue, or more information labels Feb 23, 2023
@valentyn-dasburo
Copy link

I have the same issue, but for me, both suggested fixes break the script with the following error:

Migrating 100 items from foo_bars_test_entries__test_entries_foo_bars to test_entries_foo_bars_links
foo_bars_test_entries__test_entries_foo_bars batch #1
WARNING - items of test_entries_foo_bars_links was filtered ["inv_test_entry_id","inv_foo_bar_id"]
WARNING - items of test_entries_foo_bars_links was filtered ["inv_test_entry_id","inv_foo_bar_id"]
WARNING - items of test_entries_foo_bars_links was filtered ["inv_test_entry_id","inv_foo_bar_id"]
(node:20292) UnhandledPromiseRejectionWarning: Error: The query is empty
    at Client_PG._query (v3-sql-v4-sql/node_modules/knex/lib/dialects/postgres/index.js:214:25)
    at executeQuery (v3-sql-v4-sql/node_modules/knex/lib/execution/internal/query-executioner.js:37:17)
    at Client_PG.query (v3-sql-v4-sql/node_modules/knex/lib/client.js:146:12)
    at Runner.query (v3-sql-v4-sql/node_modules/knex/lib/execution/runner.js:130:36)
    at ensureConnectionCallback (v3-sql-v4-sql/node_modules/knex/lib/execution/internal/ensure-connection-callback.js:13:17)
    at Runner.ensureConnection (v3-sql-v4-sql/node_modules/knex/lib/execution/runner.js:307:20)
    at runMicrotasks (<anonymous>)
    at processTicksAndRejections (internal/process/task_queues.js:95:5)
    at async Runner.run (v3-sql-v4-sql/node_modules/knex/lib/execution/runner.js:30:19)
    at async migrate (v3-sql-v4-sql/migrate/helpers/migrate.js:146:7)
(Use `node --trace-warnings ...` to show where the warning was created)
(node:20292) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 2)
(node:20292) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

I guess, it gets broken for a specific table/field name

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
issue: bug Issue reporting a bug severity: medium If it breaks the basic use of the product but can be worked around source: v3-SQL to v4-SQL status: pending reproduction Waiting for free time to reproduce the issue, or more information
Projects
Status: To be reviewed
Development

No branches or pull requests

5 participants