nWidart / laravel-modules

Module Management In Laravel
https://docs.laravelmodules.com
MIT License
5.5k stars 954 forks source link

Migration priority ignored #1887

Closed JohnnyMaynne closed 3 weeks ago

JohnnyMaynne commented 3 months ago

Laravel: v11.1.2 Package: v11.0.11

I have such a list in a module:

image

If you look into it, it becomes clear that I have 4 modules that have a priority of 10, this is necessary because the tables of these modules have foreign keys to the users table in the User module, which has a priority of 0.

But I get an error because modules ignore priority and are executed not in priority order, but in name order

image
dcblogdev commented 3 months ago

@alissn would you be able to look into this?, if not I'll look at it as soon as I get a chance.

alissn commented 3 months ago

@dcblogdev i'll check it.

alissn commented 3 months ago

@JohnnyMaynne

Hi,

When calling the command with the --direction option, the modules are sorted by priority. You can view the order in the laravel Prompt Select Modules.

The order logic hasn't changed. First, the method allEnabled is called, and then the modules are sorted based on the specified direction. You can view the relevant code here.

In your case, the initial list looks like this:

[
'Auth',
'Author',
'Briefcase',
...
'Timer',
'User'
]

After sorting, it changes to this:

[
'Auth',
'Briefcase',
'Core'
...
'Author',
'Comment',
'Member',
'Timer'
]

Is it okay to run the migration for the Auth module first? Do you agree with this approach?

JohnnyMaynne commented 3 months ago

@alissn in fact, in Prompt Select Modules they are sorted in a priority order even without a flag, but for some reason migration doesn’t happen that way

image
JohnnyMaynne commented 3 months ago

Another strange thing is that it shows that migrations are performed in the Auth module, although they are not there

image
alissn commented 3 months ago

@alissn in fact, in Prompt Select Modules they are sorted in a priority order even without a flag, but for some reason migration doesn’t happen that way

it's correct, command option define in class, and logic here:

        $modules = $this->hasOption('direction')
            ? array_keys($this->laravel['modules']->getOrdered($input->hasOption('direction')))
            : array_keys($this->laravel['modules']->all());

and command module:migrate has dirction option, here:

    protected function getOptions()
    {
        return [
            ['direction', 'd', InputOption::VALUE_OPTIONAL, 'The direction of ordering.', 'asc'],
            ...
        ];
    }
alissn commented 3 months ago

Another strange thing is that it shows that migrations are performed in the Auth module, although they are not there.

When the migrations table does not exist, it is created first.

The package uses the migrate command from the Laravel framework to perform migrations. If you check the migrate command of the framework, you will see that it first checks whether the migrations table exists before running other migrations. You can find the relevant code here and here

JohnnyMaynne commented 3 months ago

Thank you for the detailed explanation, but unfortunately, even with the directive php artisan module:migrate --direction, the order is not being followed.

JohnnyMaynne commented 3 months ago

By the way, I tried these options

php artisan module:migrate --direction=asc php artisan module:migrate --direction=desc

alissn commented 3 months ago

@JohnnyMaynne Please change you module priority.

When priority is equal, sort by module name alphabetically.

JohnnyMaynne commented 3 months ago

@alissn As soon as I set the first module (in alphabetical order) priority 1 - everything worked.

I think I understand the point - the module that comes first in alphabetical order must have a priority greater than zero.

Thank you for helping me solve the problem, I really appreciate it 👍