troelskn / laravel-fillable-relations

Provides HasFillableRelations trait to Eloquent models
64 stars 23 forks source link

Is there any way to clear attached relations? #18

Closed lexuzieel closed 4 years ago

lexuzieel commented 5 years ago

Problem

I use request validation and my relation schema is a simple array of ids:

'permissions' => [
  [ 'id' => 1 ],
  [ 'id' => 2 ],
]

Problem arises when relation array is empty:

'permissions' => []

In this case relations aren't cleared and stay the same. The only way I found to clear them is to add additional check after filling:

$data = $request->validated();

$role->fill($data)->save();

if (empty(array_get($data, 'permissions', []))) {
    $role->permissions()->sync([]);
}

However I find this cumbersome because then I would need to write such check for every relation. After inspecting mixin code I found that this is caused by the check inside extractFillableRelations method:

if($val) {

This statement returns false on empty array and therefore doesn't allow to populate $relationsAttributes

Possible solution

I cloned your repository and altered this code to use new parameter $allowEmpty which is false by default which allows it to act same as before.

Using this parameter can clean up the code:

$role->fill($request->validated(), true)->save();

I will create a pull request in order for you to check out the code.

troelskn commented 5 years ago

Good find. Instead of making it optional, I suppose it would be better to change the if-statement to test for null explicitly. If one doesn't want to change the relation, one shouldn't pass any value at all then.

lexuzieel commented 5 years ago

Indeed that would make it less cumbersome and an even easier fix, however such change might break projects that might be relying on this.

I have updated my pull request.

troelskn commented 4 years ago

Merged