For non-campaign emails, we use scheduled tasks to display processed stats.
The invalid tasks are no longer processed, and they don't block the queue
and are not counted in the stats.
For campaign emails (bulk emails), we just mark them as sent to zero recipients.
[MAILPOET-5881]
The batch will be continued anyway because of the next condition
if (!$queue->getCountToProcess()) {
continue;
}
The newsletter entity will be marked as sent (plus other stuff related to finished sending)
after the loop in $this->endSending.
When a post notification history is deleted in
MailPoet\Cron\Workers\SendingQueue\Tasks\Newsletter::preProcessNewsletter
it is also detached from EntityManager.
Any further attempt to manipulate the entity via EntityManager (remove or flush) causes
errors like "ERROR: A new entity was found through the relationship 'MailPoet\Entities\ScheduledTaskEntity#sendingQueue'"
In this commit we prevent doing such changes.
[MAILPOET-5880]
The goal of this commit is to change the preProcessNewsletter method
to return false only in case when it deleted the newsletter and
all associated entities. So that we know for sure that false means all was deleted.
[MAILPOET-5880]
I opted to remove testItDeletesQueueWhenNewsletterIsNotFound() as I
wasn't sure how to recreate this test using Doctrine and I found out
that it was actually not working. The call to
$this->newslettersRepository->bulkDelete() was deleting the sending
queue entry and not SendingQueue::deleteTaskIfNewsletterDoesNotExist().
[MAILPOET-5737]
This test was checking that the status of a welcome email is set to
`sent` after it is processed. Turns out this is not what should happen
and also not what is actually happen. The code only changes the status
of emails of the types standard and notification history:
7d43059f15/mailpoet/lib/Cron/Workers/SendingQueue/Tasks/Newsletter.php (L285-L286)
The test changes the type of the email to welcome
using Paris and never persists it to the database. The code mentioned
above that changes the status of the email uses Doctrine, so it was
still acting as if this was an standard email and not a welcome email.
To fix this problem, I updated the test to check if the status of the
welcome email is `active` and also made sure to persist the change to
the database and refresh the Doctrine entity.
[MAILPOET-5737]
In 3394568, SendingQueue::stopProgress() was refactored to use Doctrine
but a typo introduced a bug causing this method to set
ScheduledTaskEntity::inProgress to true instead of false:
3394568792 (diff-3a26b2d8faf9cc01efd5aef47b058c088c0de01b8074c3be7cefd9adb77fbaaaR551)
This luckly was caught by the
EditorCouponCest.php:seeNoticeWhenCouponCantGenerateAndResumeSending
acceptance test.
This commit fixes the problem and also adds two integration tests to
protect against similar regressions in the future.
[MAILPOET-5682]
While working on the previous commit, I noticed that we had a test to
check removing subscribers that are not part of a segment associated
with a newsletter when sending
(testItRemovesSubscribersFromProcessingListWhenNewsletterHasSegmentAndSubscriberIsNotPartOfIt()).
But SendingQueue::processSending() also handles removing subscribers
when sending newsletters without segments. And there was not test to
cover this part of the code.
[MAILPOET-5682]
This commit also removes Sending::getSendingQueueEntity() as the last
calls to this method were removed when refactoring SendingQueueTest.
[MAILPOET-5682]
testItLogsErrorWhenExistingRenderedNewsletterBodyIsInvalid() was removed
because since 0e13cd02c31baef706f852664570e3ddb014f56d,
preProcessNewsletter() doesn't throw an exception anymore for an invalid
newsletter body:
0e13cd02c3 (diff-449c06813489ce1e60006fdd40b4f23eb3a0cf290ff84b703dcee44b04435eadL142)
Presumably, this is because Doctrine won't allow the code to save an
invalid body.
testItLogsErrorWhenNewlyRenderedNewsletterBodyIsInvalid() was removed
because it doesn't seem to test what it was supposed to test and it is
not trivial to make it work. Even on trunk this test was passing because
`self::fail('Sending error exception was not thrown.')` throwns an
exception and not because preProcessNewsletter() was throwing an
exception.
[MAILPOET-5682]
We don't need to use the value from `processedAt` here since we can rightfully use the current datetime value
Also, I found out `processedAt` is usually null here for standard newsletters which cause new Carbon to use the server datetime settings
MAILPOET-5795
As far as I could test, calling validate() directly is not necessary.
The call to $sendingTask->save() right above calls validate() internaly
and then any errors will be returned by $sendingTask->getErrors().
I added a integration test that seems to confirm this interpretation.
[MAILPOET-4693]
It was necessary to update a test to be more lenient when comparing
dates and another one to persist a change in Paris to the database.
[MAILPOET-4368]
The second parameter of expect()->equals() could be a integer and in
this case it would be used as a delta when comparing the values.
verify()->equals() does not accept that and verify()->equalsWithDelta()
should be used.
[MAILPOET-5664]
codeception/verify 2.1 removed support for expect()->stringNotContainsString() so we need
to replace it with verify()->stringNotContainsString().
[MAILPOET-5664]
codeception/verify 2.1 removed support for expect()->stringContainsString() so we need
to replace it with verify()->stringContainsString().
[MAILPOET-5664]
