laravel-mailbox icon indicating copy to clipboard operation
laravel-mailbox copied to clipboard

Queue getQueuableId issue

Open davidrushton opened this issue 5 years ago • 5 comments

Thanks for this package.

I was running into this exception

BeyondCode\Mailbox\InboundEmail::id must return a relationship instance.
[stacktrace]
#0 vendor/laravel/framework/src/Illuminate/Database/Eloquent/Concerns/HasAttributes.php(397): Illuminate\\Database\\Eloquent\\Model->getRelationshipFromMethod('id')
#1 vendor/laravel/framework/src/Illuminate/Database/Eloquent/Concerns/HasAttributes.php(327): Illuminate\\Database\\Eloquent\\Model->getRelationValue('id')
#2 vendor/laravel/framework/src/Illuminate/Database/Eloquent/Model.php(1388): Illuminate\\Database\\Eloquent\\Model->getAttribute('id')
#3 vendor/laravel/framework/src/Illuminate/Database/Eloquent/Model.php(1398): Illuminate\\Database\\Eloquent\\Model->getKey()
#4 vendor/laravel/framework/src/Illuminate/Queue/SerializesAndRestoresModelIdentifiers.php(32): Illuminate\\Database\\Eloquent\\Model->getQueueableId()
#5 vendor/laravel/framework/src/Illuminate/Queue/SerializesModels.php(23): App\\Jobs\\ProcessInboundEmail->getSerializedPropertyValue(Object(BeyondCode\\Mailbox\\InboundEmail))

When running

Mailbox::catchAll(function (InboundEmail $email) {
    dispatch(new ProcessInboundEmail($email));
});

I see this is because the email model is only saved at the end of any Mailbox routes, so there is no id to serialize for queued jobs.

I can fix this by updating the queued job to the following, but wanted to check if you'd accept a PR to match Mailbox routes first then call storeEmail before running any routes?

Mailbox::catchAll(function (InboundEmail $email) {
    $email->save();
    dispatch(new ProcessInboundEmail($email));
});

Thanks :)

davidrushton avatar Jan 27 '19 15:01 davidrushton

@davidrushton how does adding the $email->save() line before the dispatching works? If I do this, it will be inserted into the database, but not by the queue/job, but just normally.

oliverbj avatar Mar 12 '19 14:03 oliverbj

@oliverbj The default behaviour of the package is to run all routes before saving the email - https://github.com/beyondcode/laravel-mailbox/blob/master/src/Routing/Router.php#L97.

Because Laravel serializes all Eloquent models and then re-fetches from the database when running queue jobs, this causes an exception - the e-mail wasn't saved so there's no id to serialize/fetch.

davidrushton avatar Mar 12 '19 14:03 davidrushton

Ah OK, I see. It would make sense that we should be able to overwrite the save method - or at least call it ourself, based on conditions.

There might be situations where the incoming email is being redirected to the correct mailbox/class, but should not be saved if some other user specific conditions are met.

oliverbj avatar Mar 13 '19 12:03 oliverbj

@davidrushton @oliverbj I also have the same issue how did you manage to solve this? Thanks

https://github.com/beyondcode/laravel-mailbox/issues/102

jhayg12 avatar Feb 28 '22 03:02 jhayg12

@jhayg12

From memory, I think I just solved this in the project by adding in the $email->save() line:

Mailbox::catchAll(function (InboundEmail $email) {
    $email->save();
    dispatch(new ProcessInboundEmail($email));
});

Hope that helps!

davidrushton avatar Feb 28 '22 09:02 davidrushton