mateusjunges / trackable-jobs-for-laravel

This package allows you to easily track your laravel jobs!
https://mateusjunges.com
MIT License
280 stars 16 forks source link

Attempt to fix should be unique bug #47

Closed mateusjunges closed 5 hours ago

mateusjunges commented 9 months ago

This attempts to fix #45

sandermanneke commented 9 months ago

@mateusjunges

Perhaps it is an option to look at the side effects of the job not being created when unique?

I just checked and the second trackedJob that is created for the job that has ShouldBeUnique has null for job_id - which makes sense as it does not exist. Why not validate for the presence of a job_id - if null the do not create a tracked job?

image
mateusjunges commented 9 months ago

@sandermanneke that's a good idea. I'll try to implement it this week and see if this fixes the issue. Thanks!

DimaVIII commented 2 months ago

@mateusjunges Hey, I have no time currently to work on this but take a look at my "unfinished" approach: https://github.com/DimaVIII/trackable-jobs-for-laravel/tree/features-v2 (It was a while ago and now it's out of sync with your recent work.)

To recall: The original issue was that jobs been marked as queued without being queued. QueueTrackedJob will fix the should be unique issue as this is the most reliable way to mark jobs as queued.

Well, yes, jobs will be still created with no job_id which are not queued but that's a feature and not a bug! This package tracks your jobs, it also include jobs which are not queued, it's very good information which should not be discard as it may shows you bugs in your application. In the frontend you can filter your jobs and display only the queued jobs. In the backend you can delete not queued job; It's a package for developer!

class QueueTrackedJob
{
    public function handle(JobQueued $event): void
    {
        if (isset($event->job->trackedJob))
        {
            $event->job->trackedJob->markAsQueued($event->id);
        }
    }
}

I was going through the Laravel source code of Queue and Horizon and came up with the solution to use event listen, similar to how Horizon handles "should be unique"; It's possible the only way to fix this issue and the best approach to work with the queue system as it opens the possibility to add queue management to this package.

It's a breaking change to the main branch as the database structure was altered to fit the logic, that's why I was originally thinking to release this under v2.

I will add a pull request in case you want implement and extend the logic. EDIT: I see you implemented most of the logic already. 👍

mateusjunges commented 4 hours ago

Closed this. After giving this some thought, the job being tracked even when not queued should not be considered as a bug, as the intent of this package is to track all your jobs. You can decide how to handle this in your own frontend or even just delete the entry in the database. Thanks for your input on this @DimaVIII!