Update specific model attributes from a request

Question

Background

This is for an API and I'd like to use this Upsert action to take data from a request and only update the requested fields. I.e. These are partial updates and I will not update every field from a model in every request. A previous version from examples I was working through took the incoming data and updated all fields which resulted in a lot of nulls which overwrote the original models.

A rough old example was along the lines of:

$model->attribute1 = $modelData->attribute1;
$model->attribute2 = $modelData->attribute2;
$model->save();

Please don't review that code, I know it's garbage.

I was thinking about doing something hideous and long-winded like checking for the presence of each one like this:

if ($modelData->attribtue1 !== null) {
 $model->attribute1 = $modelData->attribute1;
}

I was also thinking perhaps I could iterate over the keys of the incoming model data and only update those but that feels like it could be exploited.

My solution

My solution was to pull in the fillable attributes from the model and iterate over those instead. My current solution is as follows:

<?php
namespace App\Actions;
use App\DataTransferObjects\MyModelData;
use App\Models\MyModel;
class UpsertMyModelAction
{
 public function execute(MyModel $myModel, MyModelData $myModelData): MyModel
 {
 $attributes = app(MyModel::class)->getFillable();
 
 foreach ($attributes as $attribute) {
 /* if ($myModelData->{$attribute} !== null) { */
 /* 
 * Edit - this won't let me set a value to null 
 * if I want to. Altered to check specifically
 * for the property instead.
 */
 if (property_exists($myModelData, $attribute)) {
 $myModel->{$attribute} = $myModelData->{$attribute};
 }
 }
 
 $myModel->save();
 return $myModel;
 }
}

My thinking being that the fillable attributes are defined and controlled by me, this looks a lot cleaner than writing an if statement per attribute and it should be re-usable for different requests.

This MUST come up all the time, I just haven't found any good examples online. How does this look? Is there a cleaner way?

Answer 1

How does this look?

It looks okay. It appears to mostly adhere to PSR-12 and is easy to read. While there is only one method with a single loop it isn't very complicated. The variable names are appropriate.

Is there a cleaner way?

I’ve used Laravel for more than 8 years and grown to primarily use controller methods to update models. I've read about using the repository pattern to have thin controllers methods. I haven't seen the manual mention actions though I do see the laravel-actions package - is that what is being utilized here? 🤔

If you look at the source of the update() method within src/Illuminate/Database/Eloquent /Model.php notice that the last line of the method calls the fill() method before calling the save() method.

/**
 * Update the model in the database.
 *
 * @param array $attributes
 * @param array $options
 * @return bool
 */
public function update(array $attributes = [], array $options = [])
{
 if (! $this->exists) {
 return false;
 }
 return $this->fill($attributes)->save($options);
}

The fill() method calls $this->fillableFromArray() passing along the $attributes parameter.

/**
 * Fill the model with an array of attributes.
 *
 * @param array $attributes
 * @return $this
 *
 * @throws \Illuminate\Database\Eloquent\MassAssignmentException
 */
public function fill(array $attributes)
{
 $totallyGuarded = $this->totallyGuarded();
 $fillable = $this->fillableFromArray($attributes);

And that method calls getFillable() to determine which attributes can be filled based on the $attributes parameter.

/**
 * Get the fillable attributes of a given array.
 *
 * @param array $attributes
 * @return array
 */
protected function fillableFromArray(array $attributes)
{
 if (count($this->getFillable()) > 0 && ! static::$unguarded) {
 return array_intersect_key($attributes, array_flip($this->getFillable()));
 }
 return $attributes;
}

Consequently one can simply call the update() method passing in the attributes to be set on the model. The foreach loop can be removed and the save() call replaced with a call to update() - using a method to get the attributes from MyModelData or with a reflection class and its getProperties() method.

• Taintedmedialtd: Thanks for the detailed answer. I'm not using laravel-actions. I've been trying to follow SOLID principles and pulled the Create and Update methods in to this single Upsert action.