allow non-id columns to be nullable when using foreign

[pxpm]

Hello @jasonmccreary hope to find you well.

I think I found an issue with Blueprint when using foreigns.

So here is the scenario, given the following yaml file:

models:
  SomeTestsMigration:
    id
    user_id: id foreign:users.id nullable
    something: integer foreign:monsteras.id nullable
    timestamps

This would produce the following migration:

        Schema::disableForeignKeyConstraints();

        Schema::create('some_tests_migrations', function (Blueprint $table) {
            $table->id();
            $table->foreignId('user_id')->nullable()->constrained();
            $table->integer('something'); //notice here its missing the nullable modifier.
            $table->foreign('something')->references('id')->on('monsteras');
            $table->timestamps();
        });

        Schema::enableForeignKeyConstraints();

Blueprint does handle the nullable modifier well when the column dataType is id as you can see here:

blueprint/src/Generators/MigrationGenerator.php

Line 328 in e446a41

if ($this->isLaravel7orNewer() && $type === 'id') {

After that, it checks and removes nullable from the modifier list as it was already handled:

blueprint/src/Generators/MigrationGenerator.php

Line 210 in e446a41

$modifiers = collect($modifiers)->reject(

For non-id dataTypes (something in my example) it also removes the nullable modifier, and it shouldn't because those cases are not handled by Blueprint in prior code.

I've also added tests to prove this use case.

Let me know if I missed something here.

Best,
Pedro

[tabacitu]

Just tested this, worked great for me, thank you @pxpm .

@jasonmccreary is this something you will want to merge? The way I see it, it's a bug - previously even if we asked for a nullable relationship in the JSON, we ended up with non-nullable one. But you might think otherwise, of course.

Should we wait for the merge or overwrite this bit in our own projects?
Thanks, cheers!

[jasonmccreary]

@tabacitu, @pxpm, looks good to me. I'll merge it but wait to tag it for a few days. Feel free to depend upon dev-master and let me know everything works.