troelskn / laravel-fillable-relations

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

Clear attached belongsTo relation #26

Closed carlobeltrame closed 3 years ago

carlobeltrame commented 4 years ago

In #19 and #18, support was added for detaching relations by passing an empty array. This works fine for most relation types. However, in the case of belongsTo relations, this doesn't work because fillBelongsToRelation reacts to an array by searching a related model in the database. An empty array isn't naturally used to represent the absence of a belongsTo relation.

$foo = new Foo([
    'creator' => [ 'name' => 'Tom' ],
]);
$foo->fill([
    'creator' => [], // doesn't work for belongsTo, it results in a search for any creator in the database (empty criteria array)
]);

I'd like to propose to also interpret null as "detach all attached related records":

$foo->fill([
    'creator' => null,
]);

The developer could then still to leave a relation untouched by leaving or filtering it out of the attributes array:

$foo->fill([
    'something' => 'else', // creator will be untouched
]);

The advantage is that this way, Laravel FormRequests can be used directly (as is the purpose of this library I think):

<select name="creator">
  <option value="">None</option> <!-- when the user selects this option and submits the form -->
  <option value="3" selected="selected">Tom</option>
</select>
$foo->fill($request->validated()); // then this will work

I have created a PR #27 for this using Arr::has instead of a null check. I am aware this is a small breaking change. If requested, I could also do an alternate implementation that only accepts this type of detaching for belongsTo relations. But that might involve some refactoring in the fill method in order to reduce code duplication.

carlobeltrame commented 3 years ago

@troelskn Any feedback on this? I actually currently don't need it anymore because my belongsTo relation has changed to belongsToMany, but the issue might return in the future or for other users of this package.

carlobeltrame commented 3 years ago

I guess this package is not maintained anymore... too bad.