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

Add getters for aggregates job #177

Merged
merged 1 commit into from
Oct 2, 2020
Merged

Conversation

ujackson
Copy link
Contributor

@ujackson ujackson commented Oct 2, 2020

I'm using Laravel Job events to determine when IncrementReactionAggregatesJob or DecrementReactionAggregatesJob is processed.

Queue::after(function (JobProcessed $event) {
  dd(unserialize($event->job->payload()['data']['command']));
});

This PR allows easily accessing the private properties in the job class.

@antonkomarev
Copy link
Member

antonkomarev commented Oct 2, 2020

Thank you for the contribution @ujackson! Could you provide a use case for it?

@ujackson
Copy link
Contributor Author

ujackson commented Oct 2, 2020

Thank you for the contribution @ujackson! Could you provide a use case for it?

Hello @antonkomarev

I needed a way to update total counts for certain reactant models.
Instead of manually loading the service providers as stated here: #160 (comment), I could easily tap into the job events which gives a more accurate record than overriding the IncrementAggregates and DecrementAggregates event listeners

  Queue::after(function (JobProcessed $event) {
            $className = class_basename($event->job->resolveName());
            $command = unserialize($event->job->payload()['data']['command']);

            if ($className === 'DecrementReactionAggregatesJob' || $className === 'IncrementReactionAggregatesJob') {
                $this->syncReactionTotals($command->getReactant());
            }
        });

@antonkomarev
Copy link
Member

To be honest by this way application logic become less transparent. As for me hooking into the JobProcessed event is required to send metrics to monitoring or log debug information, but not run synchronization tasks.

@antonkomarev antonkomarev merged commit 105f15b into cybercog:master Oct 2, 2020
@antonkomarev
Copy link
Member

antonkomarev commented Oct 2, 2020

I've merged it because it might be useful for the debugging reasons, but I suggest you to think about another way to do it.
Release will be pushed soon.

If you'll stick to your current solution you could use FQCN comparison to simplify class usage search:

$className = $event->job->resolveName();
$className === \Cog\Laravel\Love\Reactant\Jobs\DecrementReactionAggregatesJob::class

@ujackson
Copy link
Contributor Author

ujackson commented Oct 2, 2020

I've merged it because it might be useful for the debugging reasons, but I suggest you to think about another way to do it.
Release will be pushed soon.

If you'll stick to your current solution you could use FQCN comparison to simplify class usage search:

$className = $event->job->resolveName();
$className === \Cog\Laravel\Love\Reactant\Jobs\DecrementReactionAggregatesJob::class

Thank you @antonkomarev

I figured the job events weren't too transparent as you mentioned.
I created accessors & mutators for the reactant model attributes and I could return computed values with this method.

public function getReactionsCount(string $reactionType): int
    {
        return $this
            ->viaLoveReactant()
            ->getReactionCounterOfType($reactionType)
            ->getCount();
    }

Thanks for your help. 🙏

@antonkomarev
Copy link
Member

Laravel Love v8.6.0 has been released.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants