Eloquent using foreach inside each function

Question

I want to exchange image order(Data retrieved from angularjs/ui-sortable, so the $newImgOrder array is representing the new order.)

(array) $newImgOrder = ['test.jpeg', 'another.jpeg'];

Images::select('url')->where('project_id', '=', $args['id'])->get()
   ->each(function($img) use (&$newImgOrder) {
       foreach ($newImgOrder as $item) {
           $img->url = $item;
           $img->save(); 
       }
   });

* UPDATED = RIGHT WAY FOR THE PERFORMING THIS ACTION *

Thanks @Devon

Removed foreach + select()

(array) $newImgOrder = ['test.jpeg', 'another.jpeg'];

Images::where('project_id', '=', $args['id'])->get()
   ->each(function($img) use (&$newImgOrder) {
           $img->url = array_shift($item);
           $img->save(); 
       }
   });

Here is a quick demonstration for what i want to do:

Table state before the action

+---------+-------------+--------------+
| id      | project_id  | url          |  
+---------+-------------+--------------+
| 1       | 15          | another.jpeg |
+---------+-------------+--------------+
| 2       | 15          | test.jpeg    |
+---------+-------------+--------------+

Expected results =

+---------+-------------+--------------+
| id      | project_id  | url          |  
+---------+-------------+--------------+
| 1       | 15          | test.jpeg    |
+---------+-------------+--------------+
| 2       | 15          | another.jpeg |
+---------+-------------+--------------+

Actual results =

+---------+-------------+--------------+
| id      | project_id  | url          |  
+---------+-------------+--------------+
| 1       | 15          | another.jpeg |
+---------+-------------+--------------+
| 2       | 15          | another.jpeg |
+---------+-------------+--------------+

What's the problem with that iteration. I tried with double foreach but i got the same results... Am i something missing out? Any help would be very appreciated. Thanks.

Answer 1

For each image, your code is looping through $newImageOrder and saving the url for that image. Hence, you're performing two saves for every image, which is clearly not what you want.

There is no reason for the inner foreach loop. If you're certain the number of elements in $newImageOrder will match the number of rows from your query and since you're passing $newImageOrder as a reference to the closure, you could make use of shift:

each(function($img) use (&$newImgOrder) {
   $img->url = array_shift($newImgOrder);
   $img->save();
});

This will shift the first element off of the array and return it, meaning you'll be removing and using the first element of $newImgOrder for each iteration.

Keep in mind, this is a mutable change. $newImgOrder is mutated by the closure. If this is not what you want, you may want to use a running count for the offset instead.