Be part of JetBrains PHPverse 2026 on June 9 – a free online event bringing PHP devs worldwide together.

vincej's avatar
Level 15

Thanks for coming back. The orders array is the product of a $request:

 $id = $request->get('product_id');  // GRAB PRODUCT DATA FROM POST ARRAY
 $qty = $request->get('quantity');       // GRAB QUANTITY DATA FROM POST ARRAY

The only the thing that changed from last week was that I had different products and quantities on my form. I have inputed the same products and same quantities and did the dd() as requested:

array:3 [▼
  123 => "131"
  789 => "12"
  "" => null
]

Cronix's avatar

You'll need to figure out why you're getting empty inputs from your form (where you get $id and $qty from $request). That last entry shouldn't be in the $orders array with an empty key and a null value.

Maybe you need to do some additional checking in the code I gave as well before modifying the entry in $orders to add the new allocated number to it. I was only working off the data you supplied in your first post. Now it's changed and has an empty key and null value.

I don't promise this will work for you in all cases, but you could use array_filter($orders) to remove the empty entries from $orders before you process it. That's not the route I'd go, though. I'd make sure the input is what it is supposed to be and contains the data you need and find out why it's there to begin with.

$orders = array_filter(array_combine($id,$qty));

You'll also need to make sure you actually have a value for $key and $value before just updating the db in the 2nd loop you created.

vincej's avatar
Level 15

Good point. My form includes and empty row. I gave you the raw output of the dd($orders), however, in my testing over the weekend, I anticipated that this empty row could be a problem, and so have tried I commenting the $request out, and just hard coded the values into $orders as below, and sure enough it still wants to loop over twice:

 $orders = [123=>"131",789=>"12"];
 //$orders = array_combine($id,$qty);

As for array_filter I tried that as well, and for what ever reason, I could not for the life of me make that work either.

I am now thinking if the foreach can not be made to work, perhaps I could create a DB query which sums $orders and $get_allocation, as both values can be pulled from the DB.

Cronix's avatar

and sure enough it still wants to loop over twice:

What exactly is looping twice, and how do you know?

You said your array looks like

$orders = [
  123 => "131",
  789 => "12",
  "" => null
];

I'm not sure why this doesn't work for you.

dump(array_filter($orders));

output

[
     123 => "131",
     789 => "12",
]

It removed the empty array key/value

vincej's avatar
Level 15

I know it is looping multiple times because I use phpstorm and I place a whole collection of debug points into the code such that I can follow the order of execution. if I only have a single value in the orders array it executes the first time correctly updates the database and then it returns back to orders and executes the second time. But for example I have five values in the orders array it will execute five times.

I'm not sure the empty row in the array is of any real importance as if you hard-code values in the array it loops regardless.

Cronix's avatar

It sounds like something else is causing that. There isn't anything I posted that would cause a second loop.

Post the entire controller method being used? Are you using ajax or something and didn't prevent the default form submission on submit so it posts 2x? (once normally, once for ajax)?

jlrdw's avatar

I did not read everything:

Can't you sum the one with the objects and put into a variable.

Then sum the array, put into another variable.

Then sum the two variables. Just an idea.

vincej's avatar
Level 15

Sorry for the delay, I had to walk the dog :o),

The controller is very long, so a) I have abbreviated it, and b) yes, it needs refactoring. However, here it is:

// THIS TAKES A QUOTATION AND CONVERTS IT TO AN ORDER.  IT ALSO UPDATES THE INVENTORY DB. 

    public function convertquote($request)
    {
        $customer = $request->only('lastname1');
        $ref = $request->input('reference');
        $id = $request->get('product_id');  // GRAB PRODUCT DATA FROM POST ARRAY
        $qty = $request->get('quantity');       // GRAB QUANTITY DATA FROM POST ARRAY
        $line_cost = $request->get('line_cost_hidden');
        $discount_value_raw = $request->get('discountvalue_hidden');
        $discount_value = round($discount_value_raw, 2);
        $count_ids = count($id);


        for ($i = 0; $i < $count_ids; $i++) {
            if (empty($id[$i])) continue;
            $newOrder = new Order();
            $newOrder->reference = $ref;
            // Loads of values go here. 
        // etc etc etc
            $newOrder->save();
        }


        $get_allocation = $this->inventory->allocation($ref);  // This returns an array of "items" each of which is an object containing two values, "inv_product_id" and "allocated".

        $orders = [];
        $orders = array_combine($id,$qty);

        foreach ($get_allocation as $object) {
             if ((!empty($orders[$object->inv_product_id]) )) {
             $orders[$object->inv_product_id] += $object->allocated;
            }
        }


        //This updates the allocation column in the inventory table with the new order
        foreach($orders as $key=>$value){
            DB::table('inventory')->where('inv_product_id', '=',$key)->update(['allocated' => $value]);
        }


    // This updates the quotations table
     DB::table('quotations')->where('reference', '=',$ref)->update(['status' => 'order']);

        return redirect('open_quotes');

    }

Cronix's avatar

Ok, there's nothing in there that would cause it to loop over everything again that I can see, but I'm not totally understanding what you're meaning exactly. Is the whole controller looping multiple times, or specifically what is happening? You have 3 different loops going on in there. You could eliminate the one doing the updating by combining it when you're looping over orders initially, but don't want to get ahead of ourselves yet.

How is this request being sent to the server? Is it just a regular form, or an ajax request?

vincej's avatar
Level 15

My following comment placed in the code was incorrect:

    // NB: the code never gets to the the foreach below until it has finished looping.  Just to repeat myself for clarity, it will loop the same number of times as there are values in the $orders array. 


vincej's avatar
Level 15

I have discovered something. However, first to answer your questions: The code gets to the server by way of a straight forward form::open() and form::submit()

I made the assumption that the whole controller did not loop multiple times, only that portion of code from $orders[]. I tested that assumption by placing a debug point on the $customer line, and damn it, yes the whole controller is looping. But why ???

public function convertquote($request)
    {
        $customer = $request->only('lastname1'); // debug here. 
Cronix's avatar

I don't understand why you're not getting an error there. You don't typehint Request in the method declaration.

public function convertquote(Request $request)

Try with your debugger turned off. Add dump('loop'); as the very first thing in your method and check the output in the browser. Does it still display 'loop' multiple times?

Does your browser consoles server tab show multiple requests being sent?

vincej's avatar
Level 15

Ok, when I do a var dump and disable the redirect, I can see 2 loops reported in the browser, even though I have 3 products on the form.

Not sure what you mean by browser consoles server tab. There is no JS.

I've gotta go - it's 6.23 pm in Calgary and my wife is yapping at me - can we pick this up again tomorrow??

I'm indebted to you. Many, Many thanks !!

Cronix's avatar

I meant the network tab of the browsers dev tools (same place as console is located, but the network tab).

vincej's avatar
Level 15

back again: so to recap, I have two "loops" being presented with dump() and No, the browser console shows only 1 request being sent.

Cronix's avatar
public function convertquote($request) {
    dump('convertQuote hit');  // first thing in controller method


   // all of your other code as is
}

hit the route. How many times do you see the 'conertQuote' dump? If it's more than once, something is retriggering it and calling the method multiple times, which isn't normal.

Did you fix the Request $request that I mentioned in your method declaration?

vincej's avatar
Level 15

Thanks again - Yes, I fixed the Request $request. F*CK!!, There are two convertQuote hits.. This going to be interesting trying to trace this. But being positive, you helped me ID the likely problem. I'll keep you posted :o)

vincej's avatar
Level 15

I found the problem. In order to understand the problem, I have to first share with you a small image from my view: (https://drive.google.com/file/d/10VrG79gZODAHTtsP2OeXCkPEuPEwcv4w/view?usp=sharing)

As I was using convertquote it was going to the 3rd option. Where the redirect was wrong. I assume the others are wrong as well. When I changed the redirect to open_quotes, all was well.

**THIS CODE IS USED TO CHECK WHAT THE USER HAS CLICKED **

    public function save_quote_edited(Request $request)  //,$id
    {

        if($request->quote_type =='new_quote') {
            $this->newquote($request);
            return $this->newquote($request);
        }

       if($request->quote_type =='correctquote') {

         $this->correctquote($request);
          return $this->correctquote($request);
        }

        if($request->quote_type == 'convertquote') {
            $this->convertquote($request);
            return $this->convertquote($request);  //THIS SHOULD HAVE BEEN: redirect ('open_quotes');
        }

I am indebted to you. Not only did you solved the addition problem but the looping problem as well.

Thank you is not enough. All the best Vince

Cronix's avatar

Isn't that code in the various if blocks there being executing twice?

if($request->quote_type =='new_quote') {
    $this->newquote($request);
    return $this->newquote($request);
}

It seems all you'd need is the return?

if($request->quote_type =='new_quote') {
    return $this->newquote($request);
}

It could also be cleaned up a bit if you change the one form input name from new_quote to newquote. Then it would be consistent with the other method names that don't have an underscore, which would allow you to rewrite all of that to something like

public function save_quote_edited(Request $request)
{
    // get the quote type as a variable
    $quoteType = $request->quote_type;
  
    // is there a method in this class that has the same name as $quoteType?
    if (method_exists($this, $quoteType)) {
        // yes! execute it as a dynamic method name and return it
        return $this->$quoteType($request);
    }
}
vincej's avatar
Level 15

Isn't that code in the various if blocks there being executing twice?

Yes, correct. I made that point in my last post. "I assume the others are wrong as well." I will have to change them as well.

This is a brilliant idea! :

public function save_quote_edited(Request $request)
{
    // get the quote type as a variable
    $quoteType = $request->quote_type;
  
    // is there a method in this class that has the same name as $quoteType?
    if (method_exists($this, $quoteType)) {
        // yes! execute it as a dynamic method name and return it
        return $this->$quoteType($request);
    }
}

I was not aware of method exists. I will freely admit that I am not as good as I should be at refactoring my code. Especially all those validations I have that go on for 100 lines. Jeffrey does not have much in the way of refactoring. In fact, I probably still use Laravel like it was Codeigniter. I would love to write code like Taylor Otwell, with really skinny controllers.

You are a rock star, thank you.

Cronix's avatar

I was not aware of method exists.

I guess one advantage is I learned PHP before there were any frameworks, so you had to know a lot of basic built-in php functions, like method_exists(). There's a lot more to php than just what's in laravel.

vincej's avatar
Level 15

@CRONIX - Fair comment. About ten years ago I did learn raw PHP through Lynda.com and did some stuff with it in Wordpress and Virtuemart POS system. However, I quickly realised there was a better way with Codeigniter, and so left PHP behind. That is when I first came across you, in Codeigniter. You were a a master there too.

I still reference the PHP manual and the W3schools site regularly to find things not in Laravel, however, yes, I use it only tactically. There is just too much to learn, and time is my single most scarce resource. I spent 26 years working in ERP as a consultant for major international software corps. I have to accept that my application is functionally very rich but not as well crafted as what you or Taylor would deliver.

Previous

Please or to participate in this conversation.