-
Notifications
You must be signed in to change notification settings - Fork 58
Conversation
Could you add a test for this? |
src/HasBinaryUuid.php
Outdated
@@ -90,4 +90,9 @@ public function newQueryForRestoration($id) | |||
{ | |||
return $this->newQueryWithoutScopes()->whereKey(base64_decode($id)); | |||
} | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove trailing whitespace: https://styleci.io/analyses/zYabxo
tests/Feature/HasBinaryUuidTest.php
Outdated
app('router')->get('uuid-test/{model}')->name('uuid-test'); | ||
|
||
$expected = "http://localhost/uuid-test/$uuid"; | ||
$actual = route('uuid-test', $model); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add an extra line break here, see the style of other tests.
tests/Feature/HasBinaryUuidTest.php
Outdated
|
||
app('router')->get('uuid-test/{model}')->name('uuid-test'); | ||
|
||
$expected = "http://localhost/uuid-test/$uuid"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is hard-coding localhost the best idea? Maybe it would be better to assert like so?
$this->assertContains("/uuid-test/{$uuid}", $actual);
tests/Feature/HasBinaryUuidTest.php
Outdated
public function it_generates_valid_routes() | ||
{ | ||
$uuid = Uuid::uuid1(); | ||
$model = $this->createModel($uuid); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In some tests, these two lines are put together because there's a whole list of them. But in this case, I'd add a line break here.
Thanks! @freekmurze care to take a final look? |
Very nice! Thanks! |
Currently passing models to the
route
method to generate a url will result in a malformed url as the binary uuid will be used.