Use eachById
in mailbox:clean
#137
Open
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I added
mailbox:clean
to an application that has been using this package for several years, and noticed that older emails weren't being cleared out unless I ran the command multiple times.Reading the command, I noticed it used
chunk
when deleting records. The problem withchunk
in this context is that it usesoffset/limit
pagination under the hood. As rows are deleted in each chunk, the remaining rows shift, causing the next chunk to skip some records. This results in incomplete deletions unless the command is repeatedly executed.To verify the issue, I modified
CleanEmailsTest
to create 200 records instead of 60, which caused the test to fail due to records being skipped.To fix this, the command now uses
eachById
, which paginates based on primary key rather than offset. This avoids skipping rows during deletion, since it always continues from the last seen ID, not a shifting offset.