Connor Bishop Connor Bishop - 3 months ago 11
PHP Question

updating several records with one request in laravel

Some quick context:
I have a sql table and a eloquent model for JobCards and each JobCard has several Operations belonging to it. I have a table and model for Operations. The users of my application browse and edit JobCards, but when I say editing a Jobcard this can include editing Operations associated with a JobCard. I have a page where a user can edit the Operations for a certain JobCard, I submit the the data as an array of Operations.

I want a clean way to update the data for the Operations of a JobCard. There are 3 different actions I may or may not need to do:


  1. Update an existing Operation with new data

  2. Create a new Operation

  3. Delete an Operatation



I tried dealing with the first 2 and things are getting messy already. I still need a way of deleting an Operation if it is not present in the array sent in the request.

Heres my code:

public function SaveOps(Request $a)
{
$JobCardNum = $a -> get('JobCardNum');
$Ops = $a -> get('Ops');

foreach ($Ops as $Op) {

$ExistingOp = JobCardOp::GetOp($JobCardNum, $Op['OpNum'])->first();
if(count($ExistingOp)==0) {
$NewOp = new JobCardOp;
$NewOp -> JobCardNum = $JobCardNum;
$NewOp -> fill($Op);
$NewOp -> save();
$this->UpdateNextOpStatus($JobCardNum, $NewOp);
}
else {
$ExistingOp -> fill($Op);
$ExistingOp -> save();

}
}


Can anyone help with the deletion part and/or help make my code tidier.

Answer

This is how your method should look like. Please note that, I added a new method getCache($JobCardNum) this method will get an array of operations per job card (assuming that your model is designed to be related this way) this method will go to the DB only once, to get all the Operations that you need for this method call instead of getting them one-by-one (in the foreach loop), this way you make sure that the expensive call to the DB is done only once, on the other hand you got your JobCard's operations in the form of an array ready to compare with the new ones (coming in the request), the return of this method will be in the form of (key=>value with the key being the operation number and the value being the operation object it self).

/**
 * This function will get you an array of current operations in the given job card
 * @param $JobCardNum
 * @return array
 */
public function getCache($JobCardNum)
{
    /**
     * asuming that the relation in your model is built that way. if not you should then
     * use JobCardOp::all(); (Not recommended)
     */
    $ExistingOps = JobCardOp::where('job_card_id', '=', $JobCardNum);

    $opCache = array();
    foreach ($ExistingOps as $Op) {
        $opCache[(string)$Op->OpNum] = $Op;
    }
    return $opCache;
}

public function SaveOps(Request $a)
{
    $strOpNum = (string)$Op['OpNum'];
    $JobCardNum = $a->get('JobCardNum');
    $Ops = $a->get('Ops');
    $opCache = $this->getCache($JobCardNum);
    foreach ($Ops as $Op) {
        if (!isset($opCache[$strOpNum])) {
            $NewOp = new JobCardOp;
            $NewOp->JobCardNum = $JobCardNum;
            $NewOp->fill($Op);
            $NewOp->save();
            $this->UpdateNextOpStatus($JobCardNum, $NewOp);
        } else {
            $ExistingOp = $opCache[$strOpNum];
            $ExistingOp->fill($Op);
            $ExistingOp->save();
        }
        unset($opCache[$strOpNum]);
    }

    /*
     * at this point any item in the $opCache array must be deleted because it was not 
     * matched in the previous for loop that looped through the requested operations :)
     */
    foreach ($opCache as $op) {
        $op->delete();
    }
}
Comments