How to structure a console command handle() method?

thought about the error a little bit:

public function handle()
    {
        $biddedJobs = SlugJob::where('deadline', '<=', Carbon::now())
            ->where('bidded', 0)
            ->get();

        foreach ($biddedJobs as $biddedJob):

            DB::transaction(function () use ($biddedJob) {
                DB::table('jobs')
                    ->update(['bidded' => 1]);
            });

            event(new DeadlineExpired($biddedJob));

        endforeach;
    }

Is this acceptable for your projects? I just haven't seen enough real websites to know if it's proper. This does send out emails in test I just tried...

I've tried to refactor this a bit more but am still unclear on the events and collections. I watched some of the videos related to this one on Laravel 6 series: https://laracasts.com/series/laravel-6-from-scratch/episodes/46

If handle() has a collection of objects, in this case $biddedJobs, is a foreach() the right way to process the events? I've heard foreach() with many posts can cause performance issues.

However, I found the listener __construct() did not like the collection and gave this error:

Argument 1 passed to App\Events\DeadlineExpired::__construct() must be an instance of App\SlugJob, instance of Illuminate\Database\Eloquent\Collection given,

https://laravel.com/docs/7.x/notifications#using-the-notification-facade This facade link above says that's the way to handle a collection - but I've read a post on laracasts stating having an event is good as you can always hook in more listeners.

Having a hard time knowing what's the best way forward ~

    /**
     * Execute the console command.
     *
     * @return mixed
     */
    public function handle()
    {
        $biddedJobs = SlugJob::where([
                ['deadline', '<=', Carbon::now('America/Los_Angeles')->toDateTimeString()],
                ['bidded', '=', 0]
            ])
            ->get();

        // foreach ($biddedJobs as $biddedJob):
            event(new DeadlineExpired($biddedJobs));
        //endforeach;

        DB::transaction(function () {
            DB::table('jobs')->where([
                ['deadline', '<=', Carbon::now('America/Los_Angeles')->toDateTimeString()],
                ['bidded', '=', 0]
            ])
                ->update(['bidded' => 1]);
        });

    }

What I personally do is have the handle method call my external services and pass them data directly. When you have a collection you can either use foreach or the collection method ->each(). For your constructor error, I would need to see your DeadlineExpired class to better understand, but it seems that class expects a single SlugJob and not the collection $biddedJobs you are sending it. I can link you my repo for examples of how I handle commands if you want.

Hi @tippin Thank you so much. If you can share a sample I'd love that. This is the DeadlineExpired. I'm really in the weeds on this one!

class DeadlineExpired
{
    use Dispatchable, InteractsWithSockets, SerializesModels;

    /**
     * The job instance.
     *
     * @var SlugJob $job
     */
    public $job;

    /**
     * Create a new event instance.
     *
     * @param SlugJob $job
     */
    public function __construct(SlugJob $job)
    {
            $this->job = $job;
    }

    /**
     * Get the channels the event should broadcast on.
     *
     * @return \Illuminate\Broadcasting\Channel|array
     */
    public function broadcastOn()
    {
        return new PrivateChannel('channel-name');
    }
}

First off, if your first example above works, you can choose to stick with that for now. I just prefer separating logic out myself. After your refactor, the error seems because you are passing the whole collection from the ->get() query instead of the individual slugJob which the event needs. Here is a commands directory in a project of mine you can look over. https://github.com/RTippin/messenger-demo/tree/master/app/Console/Commands

On a side note, it seems you are running a DB::transaction for every individual action, when that is more meant to run outside of your foreach so that if any one failed, you can roll the entire updates back on all that had already passed.

@tippin ok, very cool app you're working on. I gave the various tools a shot on your demo site `

Came back basically to the last refactor. I'm wondering if an event is the best tool for the job? The SlugJobs can be numerous, but each job also has many users who all need notification emails.

Notification::send($users, new SendJobBiddedLosingBiddersNotified($biddedJobs));

Would you scrap the event and swap the above line out in the command? Or use the event and fire this notification from the listener handle()?

Is it even possible to send a batch of users and SlugJobs to a notification facade? Or does that need a loop?

Just to get a better picture of what's happening: a deadline ends for bids, and I'd like to notify via a cron job the bidders and buyer who won, who lost, and the final price.

Kind of complicated...really appreciate your thoughts.

Thanks! I personally have not used Notifications in a batch before in your sense, however there is a difference between Notification/Event/Broadcast. If you are using notifications, I assume you use the notifications database table to store them. You may be confusing event listeners to broadcast events. Your DeadlineExpired is a broadcast event for a private channel, which means that itself is not the notification channel for any individual user. Using broadcast events would only be realtime events but would not be shown to anyone offline, however broadcast events can take in a batch of user private channels (up to 100 at a time) to push simple data out. If you are storing the notification, I would just loop through and notify each user one at a time, and if it is a huge data set, try chunking it into multiple jobs.

If there is no way any one of your situations gets called twice for a user ( bidders and buyer who won, who lost, and the final price ), then get 3 collections to begin with using those constraints, and send them through their own logic services to loop through them, say 3 separate jobs.

@tippin - you are extremely helpful. Spent the entire day working this out - not saying it's the best way but it works.

After getting too many emails/second type error on the first loops I watched the entire queue series at laracasts and got that issue solved..

Here's a sample email it spit out!!!

Your job has bidded: Phone number test

The low bid was 25,782.01 //dollars

The following bids were placed:

- 27,173.22 //dollars


- 25,782.01 //dollars


- 30,374.64 //dollars


- 89,640.66 //dollars

Pay for your order now //button

Thank you,
BidBird

Back to the original post and the handle()—is this too much logic to have here or is this kind of what you envisioned??

   /**
     * Execute the console command.
     *
     * @return mixed
     */
    public function handle()
    {
        $biddedJobs = SlugJob::where([
                ['deadline', '<=', Carbon::now('America/Los_Angeles')->toDateTimeString()],
                ['bidded', '=', 0]
            ])
            ->get();

        foreach ($biddedJobs as $biddedJob):

            $bids = $biddedJob->bids;
            $sortedHighBids = $bids->sortByDesc('bid');
            $sortedHighBids->pop();
//            dd($sortedHighBids);

            foreach ($sortedHighBids as $sortedHighBid):
                $highBidUser = $sortedHighBid->user;
//                dd($sortedHighBid);
                $highBidUser->notify(new SendJobBiddedLosingBiddersNotified($sortedHighBid));
//                dd('hi');
            endforeach;

        endforeach;

        foreach ($biddedJobs as $biddedJob):

            $bids = $biddedJob->bids;
            $lowBid = $bids->sortBy('bid');
            $lowBid->splice(1);
//            dd($lowBid);
//            dd($lowBid->first()->user);
            $winningBidder = $lowBid->first()->user;
//            dd($winningBidder);

            $winningBidder->notify(new SendJobBiddedWinningBidderNotified($biddedJob));

        endforeach;

        foreach ($biddedJobs as $biddedJob):

//            dd($biddedJob);

            $buyer = $biddedJob->first()->user;
            $buyer->notify(new SendJobBiddedBuyerNotified($biddedJob));

        endforeach;

        DB::transaction(function () {
            DB::table('jobs')->where([
                ['deadline', '<=', Carbon::now('America/Los_Angeles')->toDateTimeString()],
                ['bidded', '=', 0]
            ])
                ->update(['bidded' => 1]);
        });

    }

Personally I try to avoid too many foreach's or nesting them within one main method. Getting it working is the first step though! Now I have no way of testing this, but I believe you can at least condense your collections down and reuse them instead of grabbing and looping multiple times. Here is how I would condense it along with eager loading the bids :

    public function handle()
    {
        $biddedJobs = SlugJob::where([
            ['deadline', '<=', Carbon::now('America/Los_Angeles')->toDateTimeString()],
            ['bidded', '=', 0]
        ])
        ->with(['bids'])
        ->get();

        foreach ($biddedJobs as $biddedJob):
            $bids = $biddedJob->bids;
            $winning = $bids->sortBy('bid')->first();
            $loosing = $bids->sortByDesc('bid')->pop();

            foreach ($loosing as $lost):
                $lost->user->notify(new SendJobBiddedLosingBiddersNotified($lost));
            endforeach;
            
            $winning->user->notify(new SendJobBiddedWinningBidderNotified($biddedJob));
            
            $biddedJob->user->notify(new SendJobBiddedBuyerNotified($biddedJob));
        endforeach;
        
        $biddedJobs->update(['bidded' => 1]);
        
    }

Hi @tippin - I can definitely see I need to work on refactoring! I picked up the course Refactoring to Collections - just haven't put in the time yet.

The only issue that came up is the update() is not Method Illuminate\Database\Eloquent\Collection::update does not exist.

So I just turned on the transaction again.

Big thank you!!