v3 Upgrade Guide Missing Steps

[ashleyshenton]

I upgraded to the v3 release today but had issues with the database repository. The issue in particular was a missing default value for the locked column.

I had a look at the diff between 3.0.0 and 2.8.3 and noticed that the migration has changed. The changes add a default value of false to the locked column. There is another change involving adding a unique index. This will also cause issues for those updating as the v3 release has changed to use upserts which depend on a unique constraint. This needs adding to the upgrade guide to make anyone upgrading aware.

I've used this in my project.

<?php

use Illuminate\Database\Migrations\Migration;
use Illuminate\Database\Schema\Blueprint;
use Illuminate\Support\Facades\Schema;

return new class extends Migration
{
    /**
     * Run the migrations.
     */
    public function up(): void
    {
        Schema::table('settings', function (Blueprint $table): void {
            $table->boolean('locked')->default(false)->change();

            $table->unique(['group', 'name']);

//            $table->dropIndex(['group']);
        });
    }

    /**
     * Reverse the migrations.
     */
    public function down(): void
    {
        Schema::table('settings', function (Blueprint $table): void {
            $table->boolean('locked')->default(null)->change();

            $table->dropUnique(['group', 'name']);

//            $table->index('group');
        });
    }
};

I would also advise that the original index is dropped, as the new unique one covers all queries and is used over the old one. I know this thanks to @aarondfrancis "left to right, no skipping". This isn't completely necessary though so is commented out in the above snippet.

Here are some examples from a project where I use this package.

Notice how the composite unique index is used even when querying on group alone. I've looked through the database repository and all queries are written in a way that this index will be used.

[rubenvanassche]

Hi @ashleyshenton,

Thanks for the heads up, I've released v3 too soon, woops 😬

I'll merge your PR in an instant

As for the group index, I don't think we ever had such an index in the default migrations so I'll move it out of your PR, I guess you've added it yourself?

[ashleyshenton]

Hi @rubenvanassche thank you for replying.

No it came as part of the package when I installed. It's still there in the latest v2 release and the current v3.

https://github.com/spatie/laravel-settings/blob/2.8.3/database/migrations/create_settings_table.php.stub#L14

https://github.com/spatie/laravel-settings/blob/3.0.0/database/migrations/create_settings_table.php.stub#LL14C18-L14C18

[rubenvanassche]

Oh good point, damn completely looked over that function. I'll fix the guide

[ashleyshenton]

Haha no worries. Thanks for accepting my PR.

[rubenvanassche]

No problem, time for vacation for me 🏖️