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

Segment email send to email address just once #9781

Open
wants to merge 14 commits into
base: 4.x
Choose a base branch
from

Conversation

kuzmany
Copy link
Member

@kuzmany kuzmany commented Mar 12, 2021

Q A
Branch? "features" for all features, enhancements and bug fixes (until 3.3.0 is released)
Bug fix? yes/no
New feature? yes/no
Deprecations? yes/no
BC breaks? yes/no
Automated tests included? yes/no
Related user documentation PR URL mautic/mautic-documentation#...
Related developer documentation PR URL mautic/developer-documentation#...
Issue(s) addressed Fixes #...

Description:

Our client want use contacts with same email address, but avoid send more times to that email address, even If contact's are in segments. For example If more employees use same email address, or family use one email address for parents and children.
This PR added new configuration parameter to enable avoid send segment email to more like one unique email address.

Steps to test this PR:

  1. Load up this PR

  2. Create segment

  3. Create 3 contacts with same email@domain.tld and add them to segment

  4. Create segment email

  5. Go to configuration and enable new options - Send to duplicated just once
    image

  6. Send segment email

  7. See just one email you receive

@kuzmany kuzmany added WIP PR's that are not ready for review and are currently in progress enhancement Any improvement to an existing feature or functionality labels Mar 12, 2021
@kuzmany kuzmany added this to the 3.3.3 milestone Mar 12, 2021
@cla-bot cla-bot bot added the cla-signed The PR contributors have signed the contributors agreement label Mar 12, 2021
@RCheesley RCheesley modified the milestones: 3.3.3, Mautic 4.0 Mar 16, 2021
@codecov
Copy link

codecov bot commented Mar 25, 2021

Codecov Report

Merging #9781 (380197e) into 4.x (f0e0292) will decrease coverage by 0.34%.
The diff coverage is 96.15%.

❗ Current head 380197e differs from pull request most recent head ead249f. Consider uploading reports for the commit ead249f to get more accurate results

Impacted file tree graph

@@             Coverage Diff              @@
##                4.x    #9781      +/-   ##
============================================
- Coverage     48.21%   47.86%   -0.35%     
+ Complexity    35275    35206      -69     
============================================
  Files          2125     2120       -5     
  Lines        105200   118329   +13129     
============================================
+ Hits          50719    56642    +5923     
- Misses        54481    61687    +7206     
Impacted Files Coverage Δ
app/bundles/EmailBundle/Entity/EmailRepository.php 55.94% <94.73%> (+6.13%) ⬆️
app/bundles/EmailBundle/Form/Type/ConfigType.php 100.00% <100.00%> (ø)
app/bundles/EmailBundle/Model/EmailModel.php 45.68% <100.00%> (+4.02%) ⬆️
...nt/Query/Filter/DoNotContactFilterQueryBuilder.php 10.00% <0.00%> (-90.00%) ⬇️
...amicContentBundle/Form/Type/DynamicContentType.php 0.00% <0.00%> (-76.77%) ⬇️
...mBundle/Controller/Api/SubmissionApiController.php 0.00% <0.00%> (-67.65%) ⬇️
...undles/LeadBundle/Entity/ExpressionHelperTrait.php 0.00% <0.00%> (-64.29%) ⬇️
app/bundles/ReportBundle/Helper/ReportHelper.php 0.00% <0.00%> (-63.05%) ⬇️
...t/IntegrationCampaign/IntegrationCampaignParts.php 33.33% <0.00%> (-57.58%) ⬇️
...p/bundles/CoreBundle/Entity/FiltersEntityTrait.php 50.00% <0.00%> (-50.00%) ⬇️
... and 2104 more

@RCheesley RCheesley modified the milestones: 4.0-alpha, 4.0-beta Mar 29, 2021
@npracht npracht modified the milestones: 4.0-beta, 4.0-rc May 14, 2021
@kuzmany kuzmany changed the base branch from 3.3 to features June 24, 2021 11:52
@RCheesley RCheesley modified the milestones: 4.0-rc, 4.0-ga Jun 28, 2021
@npracht npracht added the email Anything related to email label Jul 9, 2021
@RCheesley RCheesley removed this from the 4.0-ga milestone Jul 15, 2021
@RCheesley RCheesley added the has-conflicts Pull requests that cannot be merged until conflicts have been resolved label Jul 15, 2021
@Damzoneuh Damzoneuh force-pushed the segment-email-send-to-email-address-once branch from a4c0d6d to dd2d383 Compare July 26, 2021 06:59
@Damzoneuh Damzoneuh force-pushed the segment-email-send-to-email-address-once branch from dd2d383 to 643f44a Compare July 26, 2021 07:07
@kuzmany kuzmany added ready-to-test PR's that are ready to test and removed WIP PR's that are not ready for review and are currently in progress has-conflicts Pull requests that cannot be merged until conflicts have been resolved labels Jul 26, 2021
@stale
Copy link

stale bot commented Dec 6, 2021

This issue or PR has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you would like to keep it open please let us know by replying and confirming that this is still relevant to the latest version of Mautic and we will try to get to it as soon as we can. Thank you for your contributions.

@stale stale bot added the stale Issues which have not received an update within 90 days label Dec 6, 2021
@stale
Copy link

stale bot commented Dec 21, 2021

This issue or PR has been automatically closed because it has not had recent activity. In the case of issues, if it persists in the latest version of Mautic, please create a new issue and link back to this one for reference. With PRs if you wish to pick up the PR and update it so that it can be considered for a future release, please comment and we will re-open it. Thank you for your contributions.

@stale stale bot closed this Dec 21, 2021
@kuzmany kuzmany reopened this Jan 14, 2022
@stale stale bot removed the stale Issues which have not received an update within 90 days label Jan 14, 2022
Copy link
Contributor

@volha-pivavarchyk volha-pivavarchyk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works well as expected

@kuzmany kuzmany 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 Feb 18, 2022
@kuzmany kuzmany added this to the 4.3 milestone Feb 18, 2022
…o-email-address-once

# Conflicts:
#	app/bundles/EmailBundle/Translations/en_US/messages.ini
@jos0405
Copy link
Contributor

jos0405 commented May 4, 2022

@kuzmany could you plz update this? I'll test immediately and it could be merged.

# Conflicts:
#	app/bundles/EmailBundle/Tests/Model/EmailModelFunctionalTest.php
@kuzmany
Copy link
Member Author

kuzmany commented May 6, 2022

@jos0405 done

jos0405
jos0405 previously approved these changes May 9, 2022
Copy link
Contributor

@jos0405 jos0405 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested, works fine!

@jos0405 jos0405 added ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged and removed pending-test-confirmation PR's that require one test before they can be merged labels May 10, 2022
Copy link
Contributor

@volha-pivavarchyk volha-pivavarchyk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works well

Copy link
Sponsor Member

@escopecz escopecz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a failing test:

There was 1 error:

1) Mautic\EmailBundle\Tests\Model\EmailModelFunctionalTest::testSendEmailToTwoSameEmailAddressWithOptionSegmentEmailOnceToEmailAddress
LogicException: You cannot re-create the client when a transaction rollback for cleanup is enabled. Turn it off using $useCleanupRollback property or avoid re-creating a client.

/home/runner/work/mautic/mautic/app/bundles/CoreBundle/Test/MauticMysqlTestCase.php:85
/home/runner/work/mautic/mautic/app/bundles/EmailBundle/Tests/Model/EmailModelFunctionalTest.php:106
phpvfscomposer:///home/runner/work/mautic/mautic/vendor/phpunit/phpunit/phpunit:97

app/bundles/EmailBundle/Entity/EmailRepository.php Outdated Show resolved Hide resolved

self::assertEquals(2, $sentCount, $email->getCustomHtml());

$this->connection->executeQuery("SET GLOBAL sql_mode=(SELECT REPLACE(@@sql_mode,'ONLY_FULL_GROUP_BY',''))");
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this here? I think that's why the transaction rollback fails.

@escopecz escopecz added the pending-feedback PR's and issues that are awaiting feedback from the author label May 16, 2022
@escopecz escopecz removed this from the 4.3 milestone May 17, 2022
Co-authored-by: John Linhart <jan@linhart.email>
@kuzmany kuzmany dismissed stale reviews from volha-pivavarchyk and jos0405 via ead249f June 20, 2022 10:48
@RCheesley RCheesley added the needs-rebase PR's that need to be rebased label Aug 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed The PR contributors have signed the contributors agreement email Anything related to email enhancement Any improvement to an existing feature or functionality needs-rebase PR's that need to be rebased pending-feedback PR's and issues that are awaiting feedback from the author ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants