Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot assign id property if its not in the fillable array #10

Closed
baopham opened this issue Feb 29, 2016 · 11 comments
Closed

Cannot assign id property if its not in the fillable array #10

baopham opened this issue Feb 29, 2016 · 11 comments

Comments

@baopham
Copy link
Owner

baopham commented Feb 29, 2016

It was working previously

@baopham
Copy link
Owner Author

baopham commented Feb 29, 2016

Due to Model.php class changes for L5.2:
image

@zoul0813
Copy link
Contributor

Any values being set outside of the model class should be in the $fillable array - not sure this is a bug, or something the package needs to handle.

Ref: https://laracasts.com/discuss/channels/general-discussion/manually-set-id-attribute?page=1

@baopham
Copy link
Owner Author

baopham commented Jun 23, 2017

No it's not a bug but because we don't support auto increment id, often that user will need to do $user->id = 'uuid'. Having to put id in $fillable array might be risky if user is not too careful and let id be mass assigned. I'm just putting this here as a reminder.

@zoul0813
Copy link
Contributor

In my project, I set the ID using the event handler mentioned in the laracast.

My Model:

<?php namespace App;

use Illuminate\Database\Eloquent\Model;
use Illuminate\Notifications\Notifiable;
use BaoPham\DynamoDb\DynamoDbModel;
use App\Listeners;

class Listing extends DynamoDbModel
{
    use Notifiable;

    protected $table = "Listings";

    protected $dynamoDbIndexKeys = [
      'listing_index' => [
        'hash' => 'id'
      ],
    ];

    protected $primaryKey = 'id';

    protected $fillable = [
        // fillable fields
    ];
    protected $guarded = ['id'];
}

My Observer:

<?php

namespace App\Listeners;

use App\Listing;
use Webpatser\Uuid\Uuid;
use ElasticSearch;


class ListingObserver {
  public function saved(Listing $listing) {
    // TODO: Index item in ElasticSearch
  }

  public function creating(Listing $listing) {
    $listing->id = Uuid::generate()->string;
  }
}

This works as expected, and is probably the recommended way to do it.

Perhaps put a note in README.md about setting id's explicitly?

@baopham
Copy link
Owner Author

baopham commented Jun 23, 2017

Great idea 👍

@zoul0813
Copy link
Contributor

Should this issue be closed, or do you want to update the README and reference the issue?

@baopham
Copy link
Owner Author

baopham commented Jun 24, 2017

We can add a FAQ section and reference this issue. I'll do that later.

@bubba-h57
Copy link

More elegantly, you can simply add

protected static function boot()
    {
        parent::boot();

        static::creating(function (User $user) {
            $user->identity = Uuid::uuid4()->toString();
        });
    }

to your User class, making it handle it's own event.

@zoul0813
Copy link
Contributor

Unfortunately though, you can’t mock that and generate consistent ID values for your test environment.

@bubba-h57
Copy link

I suppose we should point out that you could also just put a generator object in the IOC and pull it out of there to use it as well.

@bubba-h57
Copy link

Oh ... well pft. Apparently you do not really have to do all of that and you can mock the Uuid::uuid4() method directly.

<?php
use PHPUnit\Framework\TestCase;
use Ramsey\Uuid\Uuid;
use Ramsey\Uuid\UuidFactory;

class UuidMockTest extends TestCase
{
    public function testKnownUuidByMockingFactory()
    {
        // Create a Uuid object from a known UUID string
        $stringUuid = '253e0f90-8842-4731-91dd-0191816e6a28';
        $uuid = Uuid::fromString($stringUuid);

        // Partial mock of the factory;
        // returns $uuid object when uuid4() is called.
        $factoryMock = \Mockery::mock(UuidFactory::class . '[uuid4]', [
            'uuid4' => $uuid,
        ]);

        // Replace the default factory with our mock
        Uuid::setFactory($factoryMock);

        // Uuid::uuid4() is a proxy to the $factoryMock->uuid4() method, so
        // when Uuid::uuid4() is called, it calls the mocked method on the
        // factory and returns a valid Uuid object that we have defined.
        $this->assertSame($uuid, Uuid::uuid4());
        $this->assertEquals($stringUuid, Uuid::uuid4()->toString());
    }

    /**
     * @runInSeparateProcess
     * @preserveGlobalState disabled
     */
    public function testMockStaticUuidMethodToReturnKnownString()
    {
        // We replace the Ramsey\Uuid\Uuid class with one created by Mockery
        // (using the "alias:" prefix) so that we can mock the static uuid4()
        // method. For this to work without affecting the Ramsey\Uuid\Uuid
        // class used by other tests, we must run this in a separate process
        // with preserveGlobalState disabled (see method annotations).
        \Mockery::mock('alias:' . Uuid::class, [
            'uuid4' => 'uuid-baz',
        ]);

        // We've replaced Ramsey\Uuid\Uuid with our own class that defines the
        // method uuid4(). This method returns the string "uuid-baz."
        $this->assertEquals('uuid-baz', Uuid::uuid4());
    }
}

ramsey/uuid#147 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants