Skip to content

Conversation

@stein-j
Copy link

@stein-j stein-j commented Jul 3, 2022

Hello,

I've just found out your package and it seems really cool ! I'm glad it was just updated to Laravel 9.

I installed the package and I have two issues:

  1. The migration fails, and I'm not quite sure why :
SQLSTATE[HY000]: General error: 1215 Cannot add foreign key constraint (SQL: alter table `achievement_progress` add constraint `achievement_progress_achievement_id_foreign` foreign key (`achievement_id`) references `achievement_det
ails` (`id`))

I'm guessing it's related to my setup, because I have multiple database connections... ¯_(ツ)_/¯

  1. In my project I have multi-tenants, the tenants migrations are stored in a sub-folder. The package loads the migration in the /Migrations folder but I need it to be in the /Migrations/tenant folder.

So this PR adds a config parameter to enable or disable the migration loader. This is similar to what Laravel Cashier does but with the config instead of a static property. This allows me to write my own migration and adapt to my project.

Also, I changed so that all the publishables load only when running in the console.

@mstralka
Copy link

The database migration file should also be published with the current timestamp in the filename instead of 0000_00_000000.

@stein-j
Copy link
Author

stein-j commented Jul 25, 2022

Hello @mstralka,
the name of the file is already in the repo so I cannot change the name of the file in this PR, you would need to see with the maintainers of this package.
But sill, because the migration has already been pushed it cannot be changed or it'll become a breaking change. If you would like to rename the file, you need to publish the file and then you're free to rename it.

Also, to answer add on to your question. The filename should be static otherwise if you don't publish the migration, the package will generate a different file name every time you run your migrations and Laravel will keep migrating it.

@assada
Copy link
Owner

assada commented Sep 6, 2022

Looks great! Thank you.
If you update your PR by master I can merge it!

@stein-j
Copy link
Author

stein-j commented Sep 6, 2022

I'm don't follow @assada, it's already on master. Also you should have access to change my MR, so feel free to do so and comply to your package and merge to whichever branch you need.

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.

3 participants