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

[11.x] Add pending attributes to child models #53677

Open
wants to merge 9 commits into
base: 11.x
Choose a base branch
from

Conversation

tontonsb
Copy link
Contributor

Problem

Currently a relationship like this:

public function children(): HasMany
{
    return $this->hasMany(ChildModel::class);
}

Allows you to query $parent->children()->get() as well as create related models $parent->children()->create() that can be queried by the former $parent->children()->get().

Meanwhile a scoped relationship like this:

public function children(): HasMany
{
    return $this->hasMany(ChildModel::class)->where('active', true);
}

Allows you to query for active records $parent->children()->get(), but doing $parent->children()->create() will insert an inactive (with the default value of active to be precise) child and $parent->children()->get() will not return what you just inserted.

Solution

I propose adding "pending attributes" on relationships to allow specifying additional attributes that should be applied when creating new models. In theory this would allow setting any attributes on the newly created related models, but in particular this would allow solving the above issue like this:

public function children(): HasMany
{
    return $this->hasMany(ChildModel::class)
        ->where('active', true)
        ->withAttributes([
            'active' => true,
        ]);
}

And with such setup $parent->children()->create() will create an active ChildModel.

The pending attributes serve as defaults that can be overriden:

public function children(): HasMany
{
    
    return $this->hasMany(ChildModel::class)
        ->withAttributes(['a' => 'A'])
        ->withAttributes(['b' => 'B'])
        ->withAttributes(['a' => 'AA']); // overriding the attribute by another withAttributes()
}

$parent->children()->create(); // ['a' => 'AA', 'b' => 'B']

// overriding during creation
$parent->children()->create(['b' => 'BB']); // ['a' => 'AA', 'b' => 'BB']

@tontonsb tontonsb changed the title Add pending attributes to child models [11.x] Add pending attributes to child models Nov 27, 2024
@taylorotwell
Copy link
Member

@tontonsb thanks for the PR. One potential inconsistency to surface... We have this method:

public function withPivotValue($column, $value = null);

It works very similar to this method you have proposed; however, it also sets a "where" clause on the pivot table in addition to putting the specified value on any newly created pivot models.

I'm wondering if this method you're proposing should also set where clauses for the given attributes as well to keep things consistent?

@tontonsb
Copy link
Contributor Author

@taylorotwell that would make this even more convenient for my usecases. I was going to use it with a corresponding ->where() call.

Should I also change the method to support an equivalent API? I.e.

    /**
     * @param  string|\Illuminate\Contracts\Database\Query\Expression|array<string, string>  $column
     * @param  mixed  $value
     */
    public function withAttribute($column, $value = null);

I'm now wondering whether I should investigate if this can be done on the Builder itself. I don't need it at the moment, but being able to not only Post::hidden()->count(), but also Post::hidden()->create() seems kinda elegant.

@taylorotwell
Copy link
Member

@tontonsb yeah a similar API might be nice, in addition to withAttributes (plural)?

@tontonsb
Copy link
Contributor Author

@taylorotwell I added there where setting and changed the API. And added test cases for all of that.

By trying it out in an actual project, I noticed that relations created using ->one() do not inherit the pending attributes which felt like a bug. To solve this I made the pendingAttributes public and set its value on the new relationship in the one() method. I didn't use ->withAttributes() as it would double the wheres which are already inherited. Let me know if this solution is too naive.

@tontonsb
Copy link
Contributor Author

See #53720 for a simpler implementation (without manual handling in one()), but it changes the Builder itself instead of only the has/morphs relationships.

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.

2 participants