Skip to content
This repository has been archived by the owner on Dec 10, 2018. It is now read-only.

Package breaks Laravel Notifications because of overwrites #19

Closed
faustbrian opened this issue Nov 30, 2017 · 3 comments
Closed

Package breaks Laravel Notifications because of overwrites #19

faustbrian opened this issue Nov 30, 2017 · 3 comments

Comments

@faustbrian
Copy link
Contributor

Laravels database notifications use $table->uuid('id')->primary(); by default so using this package breaks them which results in SQLSTATE[22001]: String data, right truncated: 1406 Data too long for column 'id' at row 1.

$table->char('id', 36)->primary(); solves the issue but having to change the default migrations is kinda off-putting.

<?php

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

class CreateNotificationsTable extends Migration
{
    /**
     * Run the migrations.
     *
     * @return void
     */
    public function up()
    {
        Schema::create('notifications', function (Blueprint $table) {
            $table->uuid('id')->primary();
            $table->string('type');
            $table->morphs('notifiable');
            $table->text('data');
            $table->timestamp('read_at')->nullable();
            $table->timestamps();
        });
    }

    /**
     * Reverse the migrations.
     *
     * @return void
     */
    public function down()
    {
        Schema::dropIfExists('notifications');
    }
}
@brendt
Copy link
Contributor

brendt commented Dec 1, 2017

Any thoughts besides what already discussed in #14 how we could fix this problem?

@freekmurze do you have any input?

@freekmurze
Copy link
Member

The easiest solution would be to add a little note in the readme with instructions on how to edit the default migration.

Let's do that first, maybe that is good enough for most.

@brendt
Copy link
Contributor

brendt commented Dec 8, 2017

I've added a little more explanation in the README: https://github.com/spatie/laravel-binary-uuid#a-note-on-the-uuid-blueprint-method

Sadly I don't see a better workaround for this, unless some changes are made in the core. These changes are in discussion over here: laravel/ideas#645

@brendt brendt closed this as completed Dec 8, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants