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

raffelustig's avatar

Extra invoices created with zero content

I have problem with creating invoices. Every invoice is followed by an extra nulled invoice like this:

https://imgur.com/qifOM2e

LOOK FURTHER DOWN, FOUND OUT THE FAULT ORIGIN

It’s in our pistol competition system in Laravel 9 and where a club arrange a competition and every other club is responsible to create invoices for their members that are signed up to the competition. An extra invoice is created every time but it have no data i it.

First the InvoiceRepository:

/**
     * Collect all the clubs which to generate a invoice to.
     * The club must have one or more competitions that has signups where invoices_id is null.
     * Or the club must have one or more competitions that has teams where invoices_id is null.
     */
    public function createInvoicesForClub($club, $signupIds = null, $teamIds = null)
    {
        /**
         * Instanciate $invoices as new collection.
         */
        $invoices = new \Illuminate\Database\Eloquent\Collection;

        $query = Competition::where(function($query) use ($club, $signupIds, $teamIds){
            $query->where(function($query) use ($club, $signupIds, $teamIds){
                $query->whereHas('Signups', function($query) use ($club, $signupIds){
                    $query->where(function($query) use ($club) {
                        $query->whereNull('invoices_id');
                        $query->where('clubs_id', $club->id);
                    });

                    if($signupIds){
                        $query->whereIn('competitions_signups.id', $signupIds);
                    }
                });
                $query->orWhereHas('Teams', function($query) use ($club){
                    $query->where(function($query) use ($club) {
                        $query->whereNull('invoices_id');
                        $query->where('clubs_id', $club->id);
                    });
                });
            });
        });
        $competitions = $query->get();

        $invoicesRecipientTypes = $competitions->groupBy('invoices_recipient_type');
        $invoicesRecipientTypes->each(function($invoiceRecipientType, $type) use($club, $invoices, $teamIds){
            $signupsGroupedBySender = $invoiceRecipientType->groupBy('invoices_recipient_id');
            $signupsGroupedBySender->each(function($signups, $index) use($club, $invoices, $type, $teamIds) {
                $signupsGroupedByCompetition = $signups->groupBy('competitions_id');
                $signupsGroupedByCompetition->each(function($item) use ($index, $club, $invoices, $type, $teamIds){
                    $sender = ($type == 'App\Models\District') ? District::find($index) : Club::find($index);

                    /**
                     * Collect the signups to attach to the invoice.
                     */
                    $query = \App\Models\Signup::with('User','Competition', 'Weaponclass');
                    $query->orderBy('competitions_id') ->  orderBy('users_id');
                    $query->where(function($query) use ($club, $sender, $type){
                        $query->whereNull('invoices_id');
                        $query->where('clubs_id', $club->id);
                        $query->whereHas('Competition', function($query) use ($sender, $type){
                            $query->where('invoices_recipient_type', $type);
                            $query->where('invoices_recipient_id', $sender->id);
                        });
                    });
                    $signups = $query->get();
                    if($signups):
                        
                        $invoice = new \App\Models\Invoice;
                        $invoice->created_by = \Auth::id();
                        $invoice->recipient_id = $club->id;
                        $invoice->recipient_type = 'App\Models\Club';
                        $invoice->recipient_name = $club->name;
                        $invoice->recipient_address_street = $club->address_street;
                        $invoice->recipient_address_street_2 = $club->address_street_2;
                        $invoice->recipient_address_zipcode = $club->address_zipcode;
                        $invoice->recipient_address_city = $club->address_city;

                        $invoice->sender_id = $sender->id;
                        $invoice->sender_type = $type;
                        $invoice->sender_name = $sender->name;

COMMENT Getting invoice number here from app/Models/Invoice.php:
    $invoice_nr = \App\Models\Invoice::GenerateInvoiceNumber($type, $sender->id);
 
                        $invoice->invoice_nr = $invoice_nr;
                        $invoice->invoice_reference = $sender->id.date('Y').$invoice_nr;
                        $invoice->sender_address_street = $sender->address_street;
                        $invoice->sender_address_street_2 = $sender->address_street_2;
                        $invoice->sender_address_zipcode = $sender->address_zipcode;
                        $invoice->sender_address_city = $sender->address_city;
                        $invoice->sender_bankgiro = $sender->bankgiro;
                        $invoice->sender_postgiro = $sender->postgiro;
                        $invoice->sender_swish = $sender->swish;
                        dump($invoice);
                        $invoice->save();

                        $sortorder = 0;

                        foreach($signups as $signup):
                            $sortorder++;
                            $invoice->expiration_date = (!$invoice->expiration_date || $invoice->expiration_date > $signup->Competition->signups_closing_date) ?  $signup->Competition->signups_closing_date : $invoice->expiration_date;
                            $invoice->InvoiceRows()->create([
                                'description' => $signup->User->full_name.' '.$signup->Competition->name.' '.$signup->Competition->date.' ('.(($signup->Competition->championships_id) ? $signup->Weaponclass->classname_general : $signup->Weaponclass->classname).')',
                                'quantity' => 1,
                                'unit' => _('st'),
                                'net_unit_amount' => $signup->registration_fee,
                                'vat_percent' => 0,
                                'vat_amount' => 0,
                                'row_net_amount' => 1 * $signup->registration_fee,
                                'row_vat_amount' => 0,
                                'row_sum_amount' => 1 * $signup->registration_fee,
                                'sortorder' => $sortorder
                            ]);
                            $invoice->Signups()->save($signup);
                        endforeach;
                    endif;

                    $amount = $invoice->InvoiceRows()->sum('row_sum_amount');
                    $invoice->amount = $amount;

                    /**
                     * Expiration date is set during the team and signups loop.
                     * But change this to current date if the compitions signups_closing_date is in the past.
                     */
                    $invoice->expiration_date = (!$invoice->expiration_date || $invoice->expiration_date < date('Y-m-d')) ? date('Y-m-d') : $invoice->expiration_date;
                    dump($invoice);
                    $invoice->save();
                   // $invoices->push($invoice);

Then the part in app/Models/Invoice.php where the invoice number is set.

    static public function GenerateInvoiceNumber($recipient_type, $recipient_id)
    {
        $latestInvoiceNr = Invoice::where('sender_type', $recipient_type)->where('sender_id', $recipient_id)->orderBy('invoice_nr','desc')->first();

        if($latestInvoiceNr == NULL):
            return 1500; 
        else:
            return $latestInvoiceNr->invoice_nr+1; 
            
        endif;
    }

Last the part in ClubInvoicesController:

    public function store(Request $request)
    {
        $user = \Auth::user();
        $club = $user->Clubs->first();

        $signupIds = array();
        $signupsSelected = ($request->exists('signup_ids'));
        if ($request->has('signup_ids')) {
            $signupIds = explode(',', $request->get('signup_ids'));
        }

        $teamIds = array();
        $teamsSelected = ($request->exists('team_ids'));
        if ($request->has('team_ids')) {
            $teamIds = explode(',', $request->get('team_ids'));
        }

        if($teamIds == ""){
            $teamIds ="false" ;
        }  
        // If all options are unchecked, display error message.
        if(($signupsSelected && (in_array("", $signupIds))) && (in_array("", $teamIds))){
            return response()->json(['message'=>_(’Y’ou must choose at least one row)], 422);
        }

        $this->invoices->createInvoicesForClub($club, $signupIds, $teamIds);

        return response()->json(['message'=>_(’Y’our invoices are created)]);
    }

I can’t understand why an extra zeroed invoice every time.

0 likes
22 replies
raffelustig's avatar

I just found out that this part is responsible. It is after the code in InvoiceRepository above. It's for the making of an invoice if there is a shooting team with a few shooters. But I have not created any team so we shouldn't be here at all, but apparently we are. I think checking for if a team is present could be the solution. If I comment out all this code ordinary invoices are created correctly without any extra empty ones.

				/**
                     * Create a separate invoice for the teams instead of attaching 
                     * them to the same invoice.
                     */
                   
                    if($teamIds){
                        $query = \App\Models\Team::with('Competition', 'Weapongroup');
                        $query->whereIn('teams.id', $teamIds);
                        
                        $query->orderBy('competitions_id') ->  orderBy('name');
                        $query->where(function($query) use ($club, $sender, $type){
                            $query->whereNull('invoices_id');
                            $query->where('clubs_id', $club->id);
                            $query->whereHas('Competition', function($query) use ($sender, $type){
                                $query->where('invoices_recipient_type', $type);
                                $query->where('invoices_recipient_id', $sender->id);
                            });
                        });
                        $teams = $query->get();
                    } else {
                        $teams = false;
                    }
                    dump($invoice_nr);
                    
                    if($teams):

                        // Create the invoice.
                        $invoice = new \App\Models\Invoice;
                        $invoice->created_by = \Auth::id();
                        $invoice->recipient_id = $club->id;
                        $invoice->recipient_type = 'App\Models\Club';
                        $invoice->recipient_name = $club->name;
                        $invoice->recipient_address_street = $club->address_street;
                        $invoice->recipient_address_street_2 = $club->address_street_2;
                        $invoice->recipient_address_zipcode = $club->address_zipcode;
                        $invoice->recipient_address_city = $club->address_city;

                        $invoice->sender_id = $sender->id;
                        $invoice->sender_type = $type;
                        $invoice->sender_name = $sender->name;

                        $invoice_nr = \App\Models\Invoice::GenerateInvoiceNumber($type, $sender->id);

                        $invoice->invoice_nr = $invoice_nr;
                        $invoice->invoice_reference = $sender->id.date('Y').$invoice_nr;
                        $invoice->sender_address_street = $sender->address_street;
                        $invoice->sender_address_street_2 = $sender->address_street_2;
                        $invoice->sender_address_zipcode = $sender->address_zipcode;
                        $invoice->sender_address_city = $sender->address_city;
                        $invoice->sender_bankgiro = $sender->bankgiro;
                        $invoice->sender_postgiro = $sender->postgiro;
                        $invoice->sender_swish = $sender->swish;
                        $invoice->save();

                        foreach($teams as $team):
                            $sortorder++;
                            $invoice->expiration_date = (!$invoice->expiration_date || $invoice->expiration_date > $team->Competition->signups_closing_date) ?  $team->Competition->signups_closing_date : $invoice->expiration_date;
                            $invoice->InvoiceRows()->create([
                                'description' => _('Lag').': '.$team->name.' '.$team->Competition->name.' '.$team->Competition->date.' ('.$team->Weapongroup->name.')',
                                'quantity' => 1,
                                'unit' => _('st'),
                                'net_unit_amount' => $team->registration_fee,
                                'vat_percent' => 0,
                                'vat_amount' => 0,
                                'row_net_amount' => 1 * $team->registration_fee,
                                'row_vat_amount' => 0,
                                'row_sum_amount' => 1 * $team->registration_fee,
                                'sortorder' => $sortorder
                            ]);
                            $invoice->Teams()->save($team);
                        endforeach;

                        $amount = $invoice->InvoiceRows()->sum('row_sum_amount');
                        $invoice->amount = $amount;                        
                        $invoice->expiration_date = (!$invoice->expiration_date || $invoice->expiration_date < date('Y-m-d')) ? date('Y-m-d') : $invoice->expiration_date;

>>>>>>>>>COMMENT Here I can see the extra invoice number being dumped:<<<<<<<<<<<<
                        dump($invoice_nr);
                        $invoice->save();

                        $invoices->push($invoice);
                    endif;
martinbean's avatar

@raffelustig I think you really need to re-factor and start breaking that code down and writing some tests. It just line after line after line after line, which makes debugging difficult.

Extract sub-routines to methods. Write tests for those methods. With more granular code, you’ll be able to pinpoint easier where bugs and undesirable side effects are occurring, rather than going, “Here’s a ~200 line method. Where’s the error?”

1 like
raffelustig's avatar

@martinbean Ok. I have setup a dump-server so I can easily reproduce many times. This code:

                            $invoice->save();

are in two places in the teams part. If I comment they both out no extra invoice is created when I make invoice for ordinary shooters. You can see those in the closest code above with all the code for teams invoice. The dump output from either $teamIds or $team when no team is created are:

array:1 [
  0 => ""
]

and I must in a way check that teams are not present and prevent zero invoices to create. For some reason both if($teamIds){ and if($teams){ are satisfied despite "array:1 [0 => ""] and the fact that both

 $invoice->save();

are within the code. It seems that if($teamIds){ and if($teams) isn't enough to prevent enter the code. So what to do?

raffelustig's avatar

As before, this code works fine in Laravel 5.3 but not here in Laravel 9.

automica's avatar

@raffelustig

So what to do?

what @martinbean said:

@raffelustig I think you really need to re-factor and start breaking that code down and writing some tests. It just line after line after line after line, which makes debugging difficult.

if you create some test cases, then you can mock your initial data which will allow you to test every step of your process individually. Then you can work out which bit is causing issue with confidence (as well as making it clearer what you are doing).

raffelustig's avatar

@automica Well, as I'm not so experienced I really don't know how to do that.

As I change these:

if($teamIds){ TO if(!$teamIds == ""){

and

if($teams): TO if(!$teams == true):

I'm not getting any empty invoices anymore. But then I can't get teams to create invoices expect empty ones.

martinbean's avatar

@raffelustig The problem is, you have a bug in your code, you’ve dumped a massive method here, and now asking us to debug it without really knowing much about the system or the codebase.

You need to step through the code, test variables are being set to the values you expect them to be set, and records are created (or not created) where you expect them to be. When you get to a point where something you don’t expect is happening, and fix that.

1 like
automica's avatar

@raffelustig also you should validate your team_ids before executing any of the logic in your store method. if you dont want an empty team_ids then enforce it using laravel's validation methods.

Laravel will throw a 422 response without you needing to do

        if(($signupsSelected && (in_array("", $signupIds))) && (in_array("", $teamIds))){
            return response()->json(['message'=>_(’Y’ou must choose at least one row)], 422);
        }

see https://laravel.com/docs/9.x/validation#quick-writing-the-validation-logic

1 like
raffelustig's avatar

@automica Well, i'm testing a lot with the help of Laravel Var Dump Server which is great. And I have come a long way already. Now I don't gen any zero invoices anymore except when making a team-invoice. So I'm on the track. But I'll checkout the testing system for 9.x

raffelustig's avatar

There is even a larger fault that also in 5.3 don't work. If you choose only two of four posts, all four are invoices created for. But that's a later question.

automica's avatar

@raffelustig if you are on 5.3 then look at https://laravel.com/docs/5.3/testing

With your current approach of testing, its reliant on you manually setting data and then hitting your browser and manually checking dump server.

if you set up as a feature test, then you can define all data in your test which means irrespective of the state of your db then the test scenario will always produce the same result.

jlrdw's avatar

@raffelustig also look for ways to have a shorter method, Maybe have a few shorter methods that returns portions of needed data.

One of Jeffrey's testing videos is just for that, looking for ways to refactor and shorten long methods.

Randy_Johnson's avatar

Mate, are you using Eloquent Relations, I cannot really tell since looking at the code gives me a head ache. If you're not I would recommend it because its rocket fuel!

1 like
raffelustig's avatar
raffelustig
OP
4 years ago
Best Answer
Level 2

I have solved the problem. First the

if($signups):

needed to be changed to

if(!$signups->isEmpty()):

because it's a collective version of php array.

Also the

if($teamsId) {

needed to be

if(!$teamIds[0] == '') {

But I got an error Undefined variable $sortorder because in the $teamIds there were a $sortorder = 0; missing. Here in Laravel 9 it was necessary obviously.

So for now, I'm glad and I want to thank you all for your help and effort.

Please or to participate in this conversation.