strapi / strapi

🚀 Strapi is the leading open-source headless CMS. It’s 100% JavaScript/TypeScript, fully customizable, and developer-first.
https://strapi.io
Other
64.01k stars 8.16k forks source link

core::5.0.0-discard-drafts migration fails when not using i18n #20225

Closed laurenskling closed 3 months ago

laurenskling commented 7 months ago

Bug report

Required System information

Environment: development OS: darwin-arm64 Strapi Version: 5.0.0-beta.5 Node/Yarn Version: yarn/1.22.19 npm/? node/v18.20.2 darwin arm64 Edition: Community Database: sqlite

Describe the bug

Starting beta.5 runs migration core::5.0.0-discard-drafts thats crashes on a missing locale field

Steps to reproduce the behavior

  1. have a strapi 4 without i18n
  2. install beta 5
  3. start
  4. crash

Expected behavior

No crash

n-alonso commented 7 months ago

Hi @laurenskling ,

I am not able to reproduce the in 5.0.0-beta.5. Im also in a mac, using yarn 1.22, node 18, SQLite and upgraded from v4@latest without i18. yarn develop still works just fine and I am able to create and delete content.

Also your reproduction steps are not very descriptive, please in the future try to be as explicit as possible.

I will mark this issue as 'unable to reproduce' until you provide further steps.

EDIT: Also, in addition, i18 is a required package in v5 as entities are now grouped as 'documents' which include also locales.

github-actions[bot] commented 7 months ago

This is a templated message

Hello @laurenskling,

Thank you for reporting this bug, however we are unable to reproduce the issue you described given the information we have on hand. Can you please create a fresh project that you are able to reproduce the issue in, provide clear steps to reproduce this issue, and either upload this fresh project to a new GitHub repo or compress it into a .zip and upload it on this issue?

We would greatly appreciate your assistance with this, by working in a fresh project it will cut out any possible variables that might be unrelated. Please note that issues labeled with status: can not reproduce will be closed in 14 days if there is no activity.

Thank you!

laurenskling commented 7 months ago

Hi Nicolas, thanks for taking the time to debug this. Sorry for my limited bug report. Somewhere I was sure this migration just forgot about v4 working without having i18n turned on and it just failed to work with that use case. But you mention that you are able to migration from a v4 without i18n, that changes the situation. My error:

â ™ Loading Strapi{ event: 'migrating', name: 'core::5.0.0-discard-drafts' }
[ERROR]  There seems to be an unexpected error, try again with --debug for more information 

MigrationError: Migration core::5.0.0-discard-drafts (up) failed: Original error: select `t0`.`id`, `t0`.`document_id`, `t0`.`locale` from                  
`documents_document` as `t0` where (`t0`.`published_at` is not null) order by `t0`.`id` asc limit 1000 - no such column: t0.locale                          
at /Users/laurenskling/sites/strapi5/node_modules/umzug/lib/umzug.js:151:27                                                                                
at async Umzug.runCommand (/Users/laurenskling/sites/strapi5/node_modules/umzug/lib/umzug.js:107:20)                                                       
at async Object.up (/Users/laurenskling/sites/strapi5/node_modules/@strapi/database/dist/index.js:6239:7)                                                  
at async Object.up (/Users/laurenskling/sites/strapi5/node_modules/@strapi/database/dist/index.js:6263:11)                                                 
at async Object.sync (/Users/laurenskling/sites/strapi5/node_modules/@strapi/database/dist/index.js:2211:9)                                                
at async Strapi.bootstrap (/Users/laurenskling/sites/strapi5/node_modules/@strapi/core/dist/Strapi.js:328:5)                                                
at async Strapi.load (/Users/laurenskling/sites/strapi5/node_modules/@strapi/core/dist/Strapi.js:294:5)                                                     
at async Module.develop (/Users/laurenskling/sites/strapi5/node_modules/@strapi/strapi/dist/node/develop.js:177:28)                                         
at async action (/Users/laurenskling/sites/strapi5/node_modules/@strapi/strapi/dist/cli/commands/develop.js:18:5)                                           
at async Command.parseAsync (/Users/laurenskling/sites/strapi5/node_modules/commander/lib/command.js:923:5)                                                 

My situation:

Edit: I got confused by the from documents_document.. This is not Strapi Documents. I have a content type called Document in my Documents plugin 🤷 If i disable that plugin, it just errors on the next content with draftAndPublish enabled

laurenskling commented 7 months ago

By the way, I am talking about the migration. If I start without a database, yes it boots and you can work. But thats not the point. The point is the migration called core::5.0.0-discard-drafts when I have content.

derrickmehaffy commented 7 months ago

By the way, I am talking about the migration. If I start without a database, yes it boots and you can work. But thats not the point. The point is the migration called core::5.0.0-discard-drafts when I have content.

@laurenskling we aren't to the point of being ready for testing migrations from v4 yet. We don't expect to be able to do that until probably around May 22nd or so.

laurenskling commented 7 months ago

Thanks @derrickmehaffy , I guess we can keep this open until testing of migration starts then :)

Convly commented 6 months ago

Hey @laurenskling, sorry I took so long to come back to you.

I've taken a shot at fixing your issue in https://github.com/strapi/strapi/pull/20422

If you could try this experimental version (0.0.0-experimental.4ddd402c7194d1a969a797313bf007c93148d59a) and tell me if it fixes the issue on your side, it would be perfect.

Have a nice afternoon!

laurenskling commented 6 months ago

Thanks for your work on this @Convly

I installed version 0.0.0-experimental.4ddd402c7194d1a969a797313bf007c93148d59a for all strapi packages Still crashes:

â ™ Loading Strapi{
  event: 'migrating',
  name: '5.0.0-rename-identifiers-longer-than-max-length'
}
{
  event: 'migrated',
  name: '5.0.0-rename-identifiers-longer-than-max-length',
  durationSeconds: 1.337
}
{ event: 'migrating', name: '5.0.0-02-created-document-id' }
{
  event: 'migrated',
  name: '5.0.0-02-created-document-id',
  durationSeconds: 1.784
}
{ event: 'migrating', name: '5.0.0-03-created-locale' }
{
  event: 'migrated',
  name: '5.0.0-03-created-locale',
  durationSeconds: 0.18
}
{ event: 'migrating', name: 'core::5.0.0-discard-drafts' }
[ERROR]  There seems to be an unexpected error, try again with --debug for more information 

MigrationError: Migration core::5.0.0-discard-drafts (up) failed: Original error: select `t0`.`id`, `t0`.`document_id`, `t0`.`locale` from `documents_document` as `t0` where (`t0`.`published_at` is not null) order by `t0`.`id` asc limit 1000 - no such column: t0.locale                                                                                                                                   
at /Users/laurenskling/sites/strapi5/node_modules/umzug/lib/umzug.js:151:27                                                                                                                               
at async Umzug.runCommand (/Users/laurenskling/sites/strapi5/node_modules/umzug/lib/umzug.js:107:20)                                                                                                      
at async Object.up (/Users/laurenskling/sites/strapi5/node_modules/@strapi/database/dist/index.js:6284:7)                                                                                                 
at async Object.up (/Users/laurenskling/sites/strapi5/node_modules/@strapi/database/dist/index.js:6308:11)                                                                                                
at async Object.sync (/Users/laurenskling/sites/strapi5/node_modules/@strapi/database/dist/index.js:2220:9)                                                                                               
at async Strapi.bootstrap (/Users/laurenskling/sites/strapi5/node_modules/@strapi/core/dist/Strapi.js:328:5)                                                                                              
at async Strapi.load (/Users/laurenskling/sites/strapi5/node_modules/@strapi/core/dist/Strapi.js:294:5)                                                                                                   
at async Module.develop (/Users/laurenskling/sites/strapi5/node_modules/@strapi/strapi/dist/node/develop.js:177:28)                                                                                       
at async action (/Users/laurenskling/sites/strapi5/node_modules/@strapi/strapi/dist/cli/commands/develop.js:18:5)                                                                                         
at async Command.parseAsync (/Users/laurenskling/sites/strapi5/node_modules/commander/lib/command.js:923:5)                                                                                              

✨  Done in 7.14s.

did I miss any installation steps?

laurenskling commented 6 months ago

I did some digging, now I realized where this migration code actually is.

It's crashing on this line: https://github.com/strapi/strapi/blob/4ddd402c7194d1a969a797313bf007c93148d59a/packages/core/core/src/migrations/database/5.0.0-discard-drafts.ts#L35 there locale is hard selected.

Convly commented 5 months ago

Hey, thanks for testing the experimental version. It seems I didn't have the right setup to reproduce the issue. The problem comes from the migration itself (assuming locale is here) and the discardDraft method that assumes locale will always exist and add the param even if you didn't add it.

I'll try to dig more in the coming days and keep you up-to-date.

ildfreelancer commented 5 months ago

@Convly sorry to mention you. However, I see you made a PR here: https://github.com/strapi/strapi/pull/20422 Do you know when the PR is merged? Any issues we can do to make it merged?

Convly commented 5 months ago

Hey there, my PR allegedly fixes the issue, but as pointed out in this comment (https://github.com/strapi/strapi/pull/20422#discussion_r1644458596) another change might fix it in a simpler manner. I would like to wait for this to be merged, see if it solves the issue, and potentially close my PR if it's not needed anymore.

shamglam commented 5 months ago

seems like the migration works but something else fails when trying to start strapi

Unknown column 't0.locale' in 'field list'
    at Packet.asError (/home/user/my-pro/node_modules/mysql2/lib/packets/packet.js:728:17)
    at Query.execute (/home/user/my-pro/node_modules/mysql2/lib/commands/command.js:29:26)
    at Connection.handlePacket (/home/user/my-pro/node_modules/mysql2/lib/connection.js:481:34)
    at PacketParser.onPacket (/home/user/my-pro/node_modules/mysql2/lib/connection.js:97:12)
    at PacketParser.executeStart (/home/user/my-pro/node_modules/mysql2/lib/packet_parser.js:75:16)
    at Socket.<anonymous> (/home/user/my-pro/node_modules/mysql2/lib/connection.js:104:25)
    at Socket.emit (node:events:518:28)
    at Socket.emit (node:domain:488:12)
    at addChunk (node:internal/streams/readable:559:12)
    at readableAddChunkPushByteMode (node:internal/streams/readable:510:3) {
  code: 'ER_BAD_FIELD_ERROR',
  errno: 1054,
  sqlState: '42S22',
  sqlMessage: "Unknown column 't0.locale' in 'field list'",
alexandrebodin commented 5 months ago

@shamglam can you test this with the RC version we release 2 days ago ?

shamglam commented 4 months ago

@alexandrebodin I actually got it working in rc0 after seeing that the reason for the error was that the migration wasn't completed properly. I deleted the migration entry from the database and then everything worked as expected.

Marc-Roig commented 3 months ago

this issue should be fixed in the latest rc versions! closing it