Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dispatch jobs without relating to models #33

Merged
merged 38 commits into from
Jan 30, 2024

Conversation

mateusjunges
Copy link
Owner

@mateusjunges mateusjunges commented Jun 30, 2022

This PR changes how relating a tracked job to a model works.

In v1, you must add a Illuminate\Database\Eloquent\Model instance as the first parameter of your's job constructor, which will be used to track the job and relate it to the given model. In this PR, I changed this behavior to have no related models by default.

You can define the new trackableKey and trackableType methods in your job class if you need to relate your job to any given model.

For example, if you define these two methods:

public function trackableKey(): ?string
{
    return (string) $this->podcast->id;
}

public function trackableType(): ?string
{
    return $this->podcast->gerMorphClass();
}

the TrackedJob instance will be created using the podcast id as trackable_id and the podcast model FQCN (or any relation morph map you have defined) as the trackable_type.

@mateusjunges mateusjunges marked this pull request as draft June 30, 2022 01:45
@mateusjunges mateusjunges force-pushed the dispatch-jobs-without-relating-to-models branch from fe4240f to 1f5ee27 Compare July 2, 2022 04:21
@mateusjunges mateusjunges marked this pull request as ready for review July 2, 2022 04:45
@mateusjunges mateusjunges changed the title Dispatch jobs without relating to models [v2.x] Dispatch jobs without relating to models Jul 2, 2022
@mateusjunges mateusjunges self-assigned this Jul 2, 2022
@mateusjunges mateusjunges added the enhancement New feature or request label Jul 2, 2022
mateusjunges and others added 5 commits February 1, 2023 23:05
…ut-relating-to-models

# Conflicts:
#	composer.json
#	src/Concerns/Trackable.php
#	src/Models/TrackedJob.php
@DimaVIII
Copy link
Contributor

DimaVIII commented Aug 9, 2023

Hey @mateusjunges
Are you planning to publish this as V2?
I'm working on implementing few new features like tracking the Job ID and adding a attempts count to TrackedJob.

There is also a bug with markAsRetrying() and markAsStarted().

Function markAsStarted() is triggered on each attempt and started_at is reseted to the current time.
markAsRetrying() should do something similar to avoid confusion. We need some logic here.

Thats the quick fix for markAsRetrying() .
This occurs when Job has retryUntil which prevents the release of the Job.

class NewTrackedJobMiddleware
{
    public function handle(object $job, Closure $next): void
    {
//        echo 'Attempts # ' . $job->job->attempts() . PHP_EOL;
//        echo 'Job ID # ' . $job->job->getJobId() . PHP_EOL;
        if ($job->job->attempts() > 1)
        {
            if ($job->trackedJob->status !== TrackedJob::STATUS_RETRYING) // Avoid an unnecessary DB writes.
            {
                $job->trackedJob->markAsRetrying();
            }
        } else
        {
            $job->trackedJob->markAsStarted();
        }

        $response = $next($job);
...

@mateusjunges
Copy link
Owner Author

Hey @DimaVIII 👋 I'm quite out of time to work on it right now, that's why it is paused. I plan to ship it on v2, as soon as I have the time to go back to it 🙂

@DimaVIII
Copy link
Contributor

DimaVIII commented Aug 14, 2023

Hey @mateusjunges !

Based on this PR I added all the mentioned features and fixed all bug, I also added tests for all new features that I implemented.
https://github.com/DimaVIII/trackable-jobs-for-laravel/tree/features-v2

I'm still testing and a bit more clean up is missing, so haven't pushed the the PR yet.

Things to consider

  • This Commit 31cbbb0 is kind of controversial. In my opinion it's not a good idea to publish the migration files, it should stay in the package. I would suggest to revert this in v2. I added a possible solution for this which hopefully should avoid breaking changes.

A small preview on how it looks like implemented into a frontend.
Dashboard

@mateusjunges mateusjunges changed the title [v2.x] Dispatch jobs without relating to models Dispatch jobs without relating to models Jan 30, 2024
@mateusjunges mateusjunges merged commit d6f6943 into master Jan 30, 2024
5 checks passed
@mateusjunges mateusjunges deleted the dispatch-jobs-without-relating-to-models branch January 30, 2024 17:15
@bokanechase-digistorm
Copy link

bokanechase-digistorm commented Apr 24, 2024

@mateusjunges is it possible to get a new release that includes these changes? 🙏
The latest release v1.7.0 doesn't appear to include them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants