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

[8.x] Extract attribute getter for performInsert #36963

Merged
merged 1 commit into from
Apr 13, 2021
Merged

[8.x] Extract attribute getter for performInsert #36963

merged 1 commit into from
Apr 13, 2021

Conversation

donnysim
Copy link
Contributor

@donnysim donnysim commented Apr 13, 2021

With models we disabled $guarded or $fillable and instead opted for a $columns concept where we define all database columns on the model e.g.

protected static array $columns = [
    'id',
    'email',
    'password',
    'name',
    'last_name',
    'remember_token',
    'created_at',
    'updated_at',
    'deleted_at',
    'email_verified_at',
];

This allows us to have a column reference list without having to dig into the database, and also we use this list to filter attributes before saving to avoid some unexpected missing column exceptions when paired with custom properties (after joins etc.). For this to fully work we need to override performInsert where instead of $this->getAttributes() we do

$columns = static::columns();
$attributes = empty($columns) ? $this->getAttributes() : Arr::only($this->getAttributes(), $columns);

While we have no problems overriding this, there still is a possibility of a breaking change to occur without us knowing and it'd be safer to manage if the attribute extraction is separated. This also helps us with computed columns where it exists in database as a column but is readonly, in that case we mark it as a comment in the column list.

@taylorotwell taylorotwell merged commit 9a9f59f into laravel:8.x Apr 13, 2021
@GrahamCampbell GrahamCampbell changed the title [8.x] extract attribute getter for performInsert [8.x] Extract attribute getter for performInsert Apr 13, 2021
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