hipsterjazzbo / Landlord

A simple, single database multi-tenancy solution for Laravel 5.2+
MIT License
614 stars 138 forks source link

Saving observer #21

Closed Jmrich closed 8 years ago

Jmrich commented 8 years ago

Changed creating observer to saving so it can watch for update/new models

hipsterjazzbo commented 8 years ago

Upon further reflection. I don't think this is right. I don't think you ALWAYS want to update the tenants to the current ones every time you update a model. Thoughts?

Jmrich commented 8 years ago

I agree that with updates it is not necessary since it gets it by the models id, but in a case like this:

$phone_number = new PhoneNumber(['some attributes']);

$client = Client::find(1);

// tenant column will not be present on $phone_number unless explicitly stated
$client->phoneNumbers()->save($phone_number);

Is where it would be helpful to have that tenant column automatically inserted. So not really sure what the best approach would be in this case

hipsterjazzbo commented 8 years ago

In that example, Model::creating() will still fire — if $phone_number doesn't yet exist in the DB, then save() simply initiates a regular create().

I can't think of an example when using save() would not currently be scoped correctly (though there may be one that I'm not thinking of).

Jmrich commented 8 years ago

I had ran into some problems before with this not working properly (foreign key constraint failures), but just tested and everything works fine. Not sure what was going on before.

hipsterjazzbo commented 8 years ago

Alright. I'm going to close this for now, but if you do run across an instance where it fails to add the constraint, feel free to re-open another issue and we'll track it down. Thanks!