spatie/laravel-settings

v3 Upgrade Guide Missing Steps

ashleyshenton opened this issue · 5 comments

ashleyshenton commented

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.

image

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.

image

image

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 commented

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?

rubenvanassche commented

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

ashleyshenton commented

Haha no worries. Thanks for accepting my PR.

rubenvanassche commented

No problem, time for vacation for me 🏖️

  • 🎉1