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

Prevent decoding the uuid when the model does not exist #40

Merged
merged 2 commits into from
Mar 1, 2018
Merged

Prevent decoding the uuid when the model does not exist #40

merged 2 commits into from
Mar 1, 2018

Conversation

andrzejkupczyk
Copy link

This resolves issue #39.

return array_merge(parent::toArray(), [$this->getKeyName() => $this->uuid_text]);
}

public function getUuidTextAttribute(): string
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method should be type hinted as a nullable string:

public function getUuidTextAttribute(): ?string

Copy link
Author

@andrzejkupczyk andrzejkupczyk Feb 28, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nullable types are not available in PHP 7.0. Do you want me to change php requirement in the composer.json file?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I missed that, yes feel free to bump the PHP version!

Copy link
Author

@andrzejkupczyk andrzejkupczyk Mar 1, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done 👍 I've changed Travis configuration as well. Yolo, right?

{
if (! $this->exists) {
return;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

null should be returned here.

@@ -6,6 +6,9 @@
use Illuminate\Database\Eloquent\Model;
use Illuminate\Database\Eloquent\Builder;

/**
* @property string|null $uuid_text
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This docblock should be removed as $uuid_text isn't always the name of the textual property.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see what you mean 👍

@brendt brendt merged commit b0c23d6 into spatie:master Mar 1, 2018
@brendt
Copy link
Contributor

brendt commented Mar 1, 2018

🎊Thanks!

@brendt
Copy link
Contributor

brendt commented Mar 1, 2018

Released and tagged: https://github.com/spatie/laravel-binary-uuid/releases/tag/1.1.6

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

Successfully merging this pull request may close these issues.

2 participants