Furze Furze - 5 months ago 6
SQL Question

Optimising code used for inserting, updating or deleting relationship

I'm writing logic to insert, update or delete a table row, based on the provided input and existing data.


  • If there is no input, and the row exists, the row should be deleted.

  • If there is input, but the row does not exist, it should be created.

  • And lastly, if there is input and the row exists, the row should be updated.



This project is test code for an
attributes
table, which has a
name
,
value
and
media_id
(which references the piece of media that has been given the attributes). The
name
and
media_id
are unique together (one media cannot have two attributes with the same name). It is important to the project that the insert, update and delete are used, as opposed to leaving them blank if empty.

My question is whether there is an easier way to carry out this logic. It would have been okay to me to use it for just one input, but when there is two (
name
and
description
) and I might potentially add more later, it seems like a massive piece of code to use. Is there a more efficient way?




My database migration:

Schema::create('attributes', function (Blueprint $table) {
$table->increments('id')->unsigned();
$table->integer('media_id')->unsigned();
$table->string('name');
$table->text('value')->nullable();
$table->timestamps();
$table->unique(['media_id', 'name']);
$table->foreign('media_id')->references('id')->on('media')->onDelete('cascade');
});





My view:

<form action="{{ Request::path() }}" method="post">
<input type="hidden" name="_token" value="{{ csrf_token() }}">
<label style="display:block" for="name">Name</label>
<input type="text" name="name" id="name" value="{{ $attributes->get('name') ? $attributes->get('name')->value : '' }}">
<label style="display:block" for="description">Description</label>
<textarea name="description" id="description">{{ $attributes->get('description') ? $attributes->get('description')->value : '' }}</textarea>
<button type="submit">Save</button>
</form>





My makeshift controller:

Route::post('/', function(Request $request) {
$media = Media::find(1);

$attributes = $media->attributes->keyBy('name');

if($request->input('name')) {
if($attributes->get('name')) {
$attributes->get('name')->value = $request->input('name');
if(!$attributes->get('name')->save()) {
return "error updating name";
}
} else {
$attribute = new Attribute;
$attribute->name = 'name';
$attribute->value = $request->input('name');
if(!$media->attributes()->save($attribute)) {
return "error inserting name";
}
}
} else {
if($attributes->get('name')) {
if(!$attributes->get('name')->delete()) {
return "error deleting name";
}
}
}
if($request->input('description')) {
if($attributes->get('description')) {
$attributes->get('description')->value = $request->input('description');
if(!$attributes->get('description')->save()) {
return "error updating description";
}
} else {
$attribute = new Attribute;
$attribute->name = 'description';
$attribute->value = $request->input('description');
if(!$media->attributes()->save($attribute)) {
return "error inserting description";
}
}
} else {
if($attributes->get('description')) {
if(!$attributes->get('description')->delete()) {
return "error deleting description";
}
}
}
});
return redirect('/');


(There is a controller for GET as well, which displays the form and the database output. I didn't bother including it as I felt it wasn't necessary.)




My dead simple model:

class Attribute extends Model
{
public function media()
{
return $this->belongsTo('App\Media');
}
}

Answer

Okay, here is what I got:

Routes File:

use Illuminate\Http\Request;
use App\Models\Media;
use App\Models\Attribute;

Route::get('/', function(Request $request) {
    $media = Media::all();
    $media->attributes;
    return response()->json($media);
});
Route::get('/media/{id}', function($id, Request $request) {
    $media = Media::find($id);
    $media->attributes;
    return response()->json($media);
});
Route::post('/media/{id}', function($id, Request $request) {
    if($request->has('name')) {
        $attr = Attribute::firstOrNew(['name' => 'name', "media_id" => $id]);
        $attr->value = $request->input('name');
        $attr->save();
    } else {
        Attribute::where('name', 'name')->where("media_id",$id)->first()->delete();
    }
    if($request->has('description')) {
        $attr = Attribute::firstOrNew(['name' => 'description', "media_id" => $id]);
        $attr->value = $request->input('description');
        $attr->save();
    } else {
        Attribute::where('name', 'description')->where("media_id",$id)->first()->delete();
    }
});
return redirect('/');

Media Model:

namespace App\Models;

use Illuminate\Database\Eloquent\Model;

class Media extends Model
{
    protected $table = 'media';

    protected $appends = [];

    public function attributes() {
        return $this->hasMany(Attribute::class);
    }
}

Attribute Model

namespace App\Models;

use Illuminate\Database\Eloquent\Model;

class Attribute extends Model
{
    protected $table = 'attributes';

    protected $fillable = ['name', 'media_id'];

    public function media() {
        return $this->belongsTo(Media::class);
    }
}

Basically, by making Attributes a model, I was able to do firstOrNew() on the attributes table, and looking for ['name' => 'name', "media_id" => $id] on the firstOrNew() makes it so it either finds the matching row, or creates a new one. You can then update the value column based on such.

Much shorter, much more simplified, and it allows your attributes to be a model!

(My opinion, almost everything should be a model :P)

EDIT: I modified it to my most recent code. Changed it from belongsTo("Media") to belongsTo(Media::class), and removed the dd() from the routes file.