czim / laravel-repository

Repository pattern implementation for Laravel
MIT License
51 stars 19 forks source link

Adding `fill` method #5

Closed ghost closed 7 years ago

ghost commented 7 years ago

Example use case:

When having guarded properties, like foreign keys. Let's assume we have a User and a Profile. On the Profile model we have a foreign key to User model: user_id, but it's guarded.

$user = $this->usersRepository->find($user);
$profile = $this->profilesRepository->fill($request->all());
$this->profilesRepository->associate($profile,  'user',  $user);
$this->profilesRepository->save($profile);

Calling update straight away will throw a \Illuminate\Database\Eloquent\MassAssignmentException

This is the cleanest way I found to handle this.

What do you think?

czim commented 7 years ago

Thanks for the PR, added test is appreciated.

Hmm. I'm not sure about this. The name fill is confusing: I'd expect to use that on an existing model, so something like an update without saving: fill(array $data, $id, $attribute = 'id').

I'd rather name it make(), in line with the method naming that Laravel's factory() uses (ie. create = make + save).

ghost commented 7 years ago

@czim Good point. What do you think of the implementation now?

czim commented 7 years ago

Much better. I made some further tweaks, only one of which should affect you:
The fill signature is now the same as that of update:

fill(array $data, $id, $attribute = null)

This means that you'll have to swap the id & attribute data parameters.

Note that I'm not overly fond of this order of parameters. Since this project was created with the aim of keeping the same interface as Bosnadev's version, I'd just rather not make any breaking changes to that.