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

Delete failed messages on subscriber deletion #110

Merged
merged 5 commits into from
Mar 8, 2021

Conversation

willselby
Copy link
Contributor

Issue

mettle/sendportal#116

Description

Delete failed messages on subscriber deletion.

@willselby willselby requested a review from JonoB February 26, 2021 11:30
@joshuafranks joshuafranks self-requested a review March 5, 2021 10:39
Joshua Franks added 3 commits March 5, 2021 11:06
This adds a test for the deletion of a subscriber's messages and any associated message failures. In adding this test, I realised that - because we were performing a mass deletion - eloquent events were not being fired. This meant that the failures were not being deleted and the operation would bomb out due to a constraint error. I'm in two minds as to whether the loop and delete per-message is better than a mass deletion with a loop that just handles the messages in our subscriber class. I think the deletion of a message's failures should be done at the Message level to ensure consistent behaviour and in practice, this should be a relatively inexpensive operation.
Although it adds some duplication, the performance benefits are simply more valuable.
@joshuafranks joshuafranks requested a review from mauricius March 5, 2021 11:18
@JonoB JonoB merged commit dd6c29d into master Mar 8, 2021
@JonoB JonoB deleted the issue-116/delete-subscriber-failed-messages branch March 8, 2021 10:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants