Add lock name on email send command #9089

mabumusa1 · 2020-08-15T12:15:12Z

Q	A
Branch?	staging
Bug fix?	no
New feature?	yes
Deprecations?	no
BC breaks?	no
Automated tests included?	yes
Related user documentation PR URL	mautic/mautic-documentation#...
Related developer documentation PR URL	mautic/developer-documentation#...
Issue(s) addressed	Fixes #...

Description:

This PR is fixes the failed tests for PR #8617

Add lock-name option on 'mautic:emails:send' command. This option allow to have serval email send process running in same time with different lock name.

A swiftmailler bug is fixed in this PR swiftmailer/swiftmailer#1270 to avoid send the same email twice. This modification should come with next M3 composer update.

Steps to test this PR:

Load up this PR

codecov · 2020-08-15T12:38:34Z

Codecov Report

Merging #9089 (ebb2415) into features (a0bff58) will increase coverage by 0.00%.
The diff coverage is 84.21%.

@@             Coverage Diff             @@
##             features    #9089   +/-   ##
===========================================
  Coverage       31.84%   31.85%           
- Complexity      33595    33597    +2     
===========================================
  Files            1946     1946           
  Lines          115966   115973    +7     
===========================================
+ Hits            36933    36938    +5     
- Misses          79033    79035    +2

Impacted Files	Coverage Δ	Complexity Δ
...s/EmailBundle/Command/ProcessEmailQueueCommand.php	`76.92% <84.21%> (-0.46%)`	`30.00 <0.00> (+2.00)`	⬇️
.../bundles/CoreBundle/Helper/Chart/AbstractChart.php	`71.76% <0.00%> (ø)`	`34.00% <0.00%> (ø%)`

mabumusa1 · 2020-08-15T12:47:36Z

@Dcoutelle do you mind checking this PR, it is your PR with minor fixes

mabumusa1 · 2020-08-15T14:25:37Z

@RCheesley Please consider this as well

RCheesley · 2020-08-16T20:05:05Z

@mabumusa1 this will be considered for 3.2 if we can get good tests :) Thanks for picking it up and making the necessary updates, we just haven't had enough time to test.

mabumusa1 · 2020-10-17T21:06:06Z

@npracht will be great if you can take a quick look at this

cla-bot · 2020-10-18T03:25:52Z

Thank you for your contribution! We require all contributors to sign our Contributor License Agreement, and we do not have a record of your signature on file. In order for us to review and merge your code, please head over to https://www.mautic.org/contributor-agreement and complete the form. There may be a short delay while the team add you as a contributor - please be patient :). Any problems contact @RCheesley. CLA has not been signed by @drprofesq.

Dcoutelle · 2020-10-18T05:12:53Z

You removed the ability to add a name to email process from #8617 ( see https://github.com/Webmecanik/mautic/blob/5c9525dacb43c75986353bcbe13ee453535ebd00/app/bundles/EmailBundle/Command/ProcessEmailQueueCommand.php#L65 ) so the goal of this PR is different.

app/bundles/EmailBundle/Command/ProcessEmailQueueCommand.php

cla-bot · 2020-10-18T08:40:24Z

Thank you for your contribution! We require all contributors to sign our Contributor License Agreement, and we do not have a record of your signature on file. In order for us to review and merge your code, please head over to https://www.mautic.org/contributor-agreement and complete the form. There may be a short delay while the team add you as a contributor - please be patient :). Any problems contact @RCheesley. CLA has not been signed by @drprofesq.

cla-bot · 2020-10-18T08:42:07Z

Thank you for your contribution! We require all contributors to sign our Contributor License Agreement, and we do not have a record of your signature on file. In order for us to review and merge your code, please head over to https://www.mautic.org/contributor-agreement and complete the form. There may be a short delay while the team add you as a contributor - please be patient :). Any problems contact @RCheesley. CLA has not been signed by @drprofesq.

TravisBuddy · 2020-10-18T09:31:43Z

Travis tests have failed

Hey @mabumusa1,
Please read the following log in order to understand the failure reason.
It'll be awesome if you fix what's wrong and commit the changes.

4th Build

View build log

if [ ${TRAVIS_PHP_VERSION:0:3} == "7.3" ]; then composer test -- --coverage-clover=coverage.xml; else composer test; fi

> bin/phpunit --bootstrap vendor/autoload.php --configuration app/phpunit.xml.dist '--coverage-clover=coverage.xml'
PHPUnit 7.5.20 by Sebastian Bergmann and contributors.

.............................................................   61 / 1825 (  3%)
.............................................................  122 / 1825 (  6%)
.............................................................  183 / 1825 ( 10%)
.............................................................  244 / 1825 ( 13%)
.............................................................  305 / 1825 ( 16%)
............................SS...............................  366 / 1825 ( 20%)
.............................................................  427 / 1825 ( 23%)
..................................SS.........................  488 / 1825 ( 26%)
............................................................E  549 / 1825 ( 30%)
E............................................................  610 / 1825 ( 33%)
.............................................................  671 / 1825 ( 36%)
.............................................................  732 / 1825 ( 40%)
.............................................................  793 / 1825 ( 43%)
.................................................PHP Warning:  exif_imagetype(path/to/file/1/fieldId1/upload1): failed to open stream: No such file or directory in /home/travis/build/mautic/mautic/app/bundles/FormBundle/Helper/FormUploader.php on line 154
PHP Warning:  exif_imagetype(path/to/file/2/fieldId2/upload2): failed to open stream: No such file or directory in /home/travis/build/mautic/mautic/app/bundles/FormBundle/Helper/FormUploader.php on line 154
.PHP Warning:  exif_imagetype(path/to/file/1/fieldId1/upload1): failed to open stream: No such file or directory in /home/travis/build/mautic/mautic/app/bundles/FormBundle/Helper/FormUploader.php on line 154
...........  854 / 1825 ( 46%)
...........S.................................................  915 / 1825 ( 50%)
.............................................................  976 / 1825 ( 53%)
............................................................. 1037 / 1825 ( 56%)
............................................................. 1098 / 1825 ( 60%)
............................................................. 1159 / 1825 ( 63%)
............................................................. 1220 / 1825 ( 66%)
............................................................. 1281 / 1825 ( 70%)
............................................................. 1342 / 1825 ( 73%)
............................................................. 1403 / 1825 ( 76%)
............................................................. 1464 / 1825 ( 80%)
............................................................. 1525 / 1825 ( 83%)
............................................................. 1586 / 1825 ( 86%)
............................................................. 1647 / 1825 ( 90%)
............................................................. 1708 / 1825 ( 93%)
............................................................. 1769 / 1825 ( 96%)
........................................................      1825 / 1825 (100%)

Time: 17.47 minutes, Memory: 1.17 GB

There were 2 errors:

1) Mautic\EmailBundle\Tests\Command\ProcessEmailQueueCommandTest::testCommandWhenQueueIsDisabled
Symfony\Component\Console\Exception\InvalidArgumentException: The "lock-name" option does not exist.

/home/travis/build/mautic/mautic/vendor/symfony/console/Input/Input.php:150
/home/travis/build/mautic/mautic/app/bundles/EmailBundle/Command/ProcessEmailQueueCommand.php:64
/home/travis/build/mautic/mautic/vendor/symfony/console/Command/Command.php:255
/home/travis/build/mautic/mautic/app/bundles/EmailBundle/Tests/Command/ProcessEmailQueueCommandTest.php:79

2) Mautic\EmailBundle\Tests\Command\ProcessEmailQueueCommandTest::testCommandWhenQueueIsEnabled
Symfony\Component\Console\Exception\InvalidArgumentException: The "lock-name" option does not exist.

/home/travis/build/mautic/mautic/vendor/symfony/console/Input/Input.php:150
/home/travis/build/mautic/mautic/app/bundles/EmailBundle/Command/ProcessEmailQueueCommand.php:64
/home/travis/build/mautic/mautic/vendor/symfony/console/Command/Command.php:255
/home/travis/build/mautic/mautic/app/bundles/EmailBundle/Tests/Command/ProcessEmailQueueCommandTest.php:134

ERRORS!
Tests: 1825, Assertions: 5413, Errors: 2, Skipped: 5.

Generating code coverage report in Clover XML format ... done

THE ERROR HANDLER HAS CHANGED!
Script bin/phpunit --bootstrap vendor/autoload.php --configuration app/phpunit.xml.dist handling the test event returned with error code 2

TravisBuddy Request Identifier: ba0a3ca0-1124-11eb-bfd1-1f84b849c73d

mabumusa1 · 2020-10-18T11:49:34Z

@Dcoutelle please review again

Dcoutelle · 2020-10-18T13:32:37Z

It look like ok to me, but I don't see the difference with my original PR to male TU work ?

mabumusa1 · 2020-10-18T13:58:49Z

It look like ok to me, but I don't see the difference with my original PR to male TU work ?

This is your commit, it will be attributed to you. but because we did not get a reply on it #8617 (comment)

I duplicated this PR with your code to get the PR moving.

This PR should be attributed to you

Dcoutelle · 2020-10-18T14:03:03Z

I don't really care about PR attribution, I wanted to know why Unit Test are working now 😛

mabumusa1 · 2020-10-18T14:07:37Z

@npracht this one is ready

npracht · 2020-10-18T14:18:27Z

We need a second reviewer

RCheesley · 2020-11-20T20:48:45Z

@mabumusa1 @Dcoutelle happy to test but there are zero test instructions on either PR. Could somebody enlighten me please?

kuzmany · 2020-11-21T10:15:00Z

@RCheesley This option allow to have serval email send process running in same time with different lock name.

That means If you use queue email processing you can use multiple crons command in same time:

mautic:email:send
mautic:email:send --lock-name=process2
mautic:email:send --lock-name=process3

etc.
Also need fix in swiftmailer https://github.com/swiftmailer/swiftmailer/pull/1270/files

It's really hard to test it properly.
You can just set queue mode in emails, and try send emails.
If you receive then should works. Same like before.

We use it in production

The base branch was changed.

mautibot · 2023-10-13T08:39:02Z

This pull request has been mentioned on Mautic Forums. There might be relevant details there:

https://forum.mautic.org/t/script-to-replace-the-mauticsend-cronjob/29624/7

mabumusa1 mentioned this pull request Aug 15, 2020

Add lock name on email send command #8617

Closed

RCheesley added enhancement Any improvement to an existing feature or functionality T1 Low difficulty to fix (issue) or test (PR) ready-to-test PR's that are ready to test labels Aug 16, 2020

RCheesley added this to the 3.2 milestone Aug 16, 2020

mabumusa1 self-assigned this Aug 24, 2020

mabumusa1 requested a review from kuzmany August 24, 2020 10:32

RCheesley added the cla-signed The PR contributors have signed the contributors agreement label Aug 25, 2020

mabumusa1 requested a review from npracht October 16, 2020 19:28

cla-bot bot removed the cla-signed The PR contributors have signed the contributors agreement label Oct 18, 2020

mabumusa1 force-pushed the pr_8617 branch from d6cfaa3 to ed62196 Compare October 18, 2020 03:27

cla-bot bot added the cla-signed The PR contributors have signed the contributors agreement label Oct 18, 2020

mabumusa1 changed the base branch from staging to 3.1 October 18, 2020 03:27

mabumusa1 modified the milestones: 3.2, 3.1.2 Oct 18, 2020

Dcoutelle reviewed Oct 18, 2020

View reviewed changes

app/bundles/EmailBundle/Command/ProcessEmailQueueCommand.php Outdated Show resolved Hide resolved

cla-bot bot removed the cla-signed The PR contributors have signed the contributors agreement label Oct 18, 2020

mabumusa1 force-pushed the pr_8617 branch from 252e71d to da3ff2c Compare October 18, 2020 08:42

Fix queue command

73374ea

mabumusa1 force-pushed the pr_8617 branch from da3ff2c to 73374ea Compare October 18, 2020 08:47

cla-bot bot added the cla-signed The PR contributors have signed the contributors agreement label Oct 18, 2020

Fixe the Queue Command

7968682

Dcoutelle previously approved these changes Oct 18, 2020

View reviewed changes

npracht added pending-test-confirmation PR's that require one test before they can be merged and removed ready-to-test PR's that are ready to test labels Oct 18, 2020

npracht modified the milestones: 3.1.2, 3.2 Oct 19, 2020

npracht modified the milestones: 3.2.0, 3.2.1, 3.3 Nov 23, 2020

npracht changed the base branch from 3.1 to features December 1, 2020 07:54

Merge branch 'features' into pr_8617

ebb2415

npracht merged commit 9713933 into mautic:features Dec 1, 2020

npracht mentioned this pull request Jun 9, 2021

Email sender name not changing #9751

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add lock name on email send command #9089

Add lock name on email send command #9089

mabumusa1 commented Aug 15, 2020 •

edited by RCheesley

codecov bot commented Aug 15, 2020 •

edited

mabumusa1 commented Aug 15, 2020

mabumusa1 commented Aug 15, 2020

RCheesley commented Aug 16, 2020

mabumusa1 commented Oct 17, 2020

cla-bot bot commented Oct 18, 2020

Dcoutelle commented Oct 18, 2020

cla-bot bot commented Oct 18, 2020

cla-bot bot commented Oct 18, 2020

TravisBuddy commented Oct 18, 2020

mabumusa1 commented Oct 18, 2020

Dcoutelle commented Oct 18, 2020

mabumusa1 commented Oct 18, 2020

Dcoutelle commented Oct 18, 2020

mabumusa1 commented Oct 18, 2020

npracht commented Oct 18, 2020

RCheesley commented Nov 20, 2020

kuzmany commented Nov 21, 2020 •

edited

mautibot commented Oct 13, 2023

Add lock name on email send command #9089

Add lock name on email send command #9089

Conversation

mabumusa1 commented Aug 15, 2020 • edited by RCheesley

Description:

Steps to test this PR:

codecov bot commented Aug 15, 2020 • edited

Codecov Report

mabumusa1 commented Aug 15, 2020

mabumusa1 commented Aug 15, 2020

RCheesley commented Aug 16, 2020

mabumusa1 commented Oct 17, 2020

cla-bot bot commented Oct 18, 2020

Dcoutelle commented Oct 18, 2020

cla-bot bot commented Oct 18, 2020

cla-bot bot commented Oct 18, 2020

TravisBuddy commented Oct 18, 2020

Travis tests have failed

4th Build

TravisBuddy Request Identifier: ba0a3ca0-1124-11eb-bfd1-1f84b849c73d

mabumusa1 commented Oct 18, 2020

Dcoutelle commented Oct 18, 2020

mabumusa1 commented Oct 18, 2020

Dcoutelle commented Oct 18, 2020

mabumusa1 commented Oct 18, 2020

npracht commented Oct 18, 2020

RCheesley commented Nov 20, 2020

kuzmany commented Nov 21, 2020 • edited

mautibot commented Oct 13, 2023

mabumusa1 commented Aug 15, 2020 •

edited by RCheesley

codecov bot commented Aug 15, 2020 •

edited

kuzmany commented Nov 21, 2020 •

edited