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

Support non-auto-incrementing models #9

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

EspadaV8
Copy link
Contributor

This is a pull request for #8

There's a new model 'Foo' that uses UUIDs instead of an auto-incrementing integer column as its primary key. The few changes that need to be made a to make sure that the Laravel model events that a model has set up get fired when being created.

The addition of orchestra/testbench is to help with that (without it there's no dispatcher for the models). In the NonIncrementingVersionedTest file, the events need to be reset every for every test due to what seems like a bug in laravel/framework#1181.

The final change in Versioned is due to the testing and needing to be able to set a fixed model_id when creating a model that uses UUIDs. If one has been passed in when creating then it doesn't attempt to get a new model_id.

Andrew Smith added 6 commits June 10, 2015 10:11
There are a number of issues related to Laravel not firing model events
when using PHPUnit

laravel/framework#1181
laravel/framework#4975

This package seems to help with that

https://github.com/orchestral/testbench
Without firing the model events (or using a raw insert query) models
that don't have an auto-incrementing column won't get a new ID generated
and fail at inserting.
Since the default 'getNextModelId' assumes that it's going to be
incrementing a number, if a model isn't auto-incrementing (e.g UUID)
then they need to override it in a way that works for them.
Ideally this wouldn't be mostly a copy, but a few things need to behave
a bit differently (like needing to pass the model_id from the data
provider).
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

Successfully merging this pull request may close these issues.

1 participant