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 public name property for preference center #8238

Merged
merged 7 commits into from Nov 21, 2020

Conversation

patrykgruszka
Copy link
Member

Please be sure you are submitting this against the staging branch.

Q A
Bug fix? N
New feature? N
Automated tests included? N
Related user documentation PR URL N
Related developer documentation PR URL N
Issues addressed (#s or URLs) N
BC breaks? N
Deprecations? N

Description:

This PR added new segment property - public name which is visible in preference center. Includes migration that sets Public name with same value as Name. Public name value is not required and

image

Steps to test this PR:

  1. Load up this PR
  2. Run php app/console doctrine:migrations:migrate
  3. Create segment with different Name and Public name
  4. Activate segment Available in Preference Center option
  5. Activate preferences center, send mail with unsubscribe link
  6. Open contact preferences center (click unsubscribe link)
  7. In preferences center should appear public segment name

@npracht npracht added enhancement Any improvement to an existing feature or functionality ready-to-test PR's that are ready to test labels Dec 30, 2019
@npracht npracht added Triage M2/M3 and removed ready-to-test PR's that are ready to test labels Apr 4, 2020
@npracht
Copy link
Member

npracht commented Apr 4, 2020

Hi there!
It has been decided to not create any extra Mautic 2 minor versions (which means no more features in M2) knowing that Mautic 3 is coming very soon.

We now want to integrate your contribution in the Mautic 3 roadmap as 3.1.0 candidate.

How to do?

  1. Check if your feature or enhancement is still missing and relevant in Mautic 3
  2. If Yes: rebase your PR against the 3.x branch
  3. If No: please consider closing your PR if you are absolutely sure that the feature has made it into Mautic 3 already

Please report results by commenting on your PR to make us administration easier.


You can more information on how to do all of that on this blog post "Getting you PR ready for Mautic 3".

@TravisBuddy
Copy link

Travis tests have failed

Hey @patrykgruszka,
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.

2nd Build

View build log

if [ ${TRAVIS_PHP_VERSION:0:3} == "7.3" ]; then composer test -- --coverage-clover=coverage.xml && bash <(curl -s https://codecov.io/bash); 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.

................FERROR 1364 (HY000) at line 42: Field 'public_name' doesn't have a default value
FERROR 1364 (HY000) at line 42: Field 'public_name' doesn't have a default value
FERROR 1364 (HY000) at line 42: Field 'public_name' doesn't have a default value
FERROR 1364 (HY000) at line 42: Field 'public_name' doesn't have a default value
FERROR 1364 (HY000) at line 42: Field 'public_name' doesn't have a default value
FERROR 1364 (HY000) at line 42: Field 'public_name' doesn't have a default value
FERROR 1364 (HY000) at line 42: Field 'public_name' doesn't have a default value
......................................   61 / 1739 (  3%)
.............................................................  122 / 1739 (  7%)
.............................................................  183 / 1739 ( 10%)
.............................................................  244 / 1739 ( 14%)
...........................................................SS  305 / 1739 ( 17%)
.............................................................  366 / 1739 ( 21%)
.............................................................  427 / 1739 ( 24%)
..SS.........................................................  488 / 1739 ( 28%)
.............................................................  549 / 1739 ( 31%)
.............................................................  610 / 1739 ( 35%)
.............................................................  671 / 1739 ( 38%)
.............................................................  732 / 1739 ( 42%)
.............................................................  793 / 1739 ( 45%)
............S................................................  854 / 1739 ( 49%)
.............................................................  915 / 1739 ( 52%)
.............................................................  976 / 1739 ( 56%)
............................................................. 1037 / 1739 ( 59%)
............................................................. 1098 / 1739 ( 63%)
............................................................. 1159 / 1739 ( 66%)
............................................................. 1220 / 1739 ( 70%)
............................................................. 1281 / 1739 ( 73%)
............................................................. 1342 / 1739 ( 77%)
............................................................. 1403 / 1739 ( 80%)
............................................................. 1464 / 1739 ( 84%)
............................................................. 1525 / 1739 ( 87%)
............................................................. 1586 / 1739 ( 91%)
............................................................. 1647 / 1739 ( 94%)
............................................................. 1708 / 1739 ( 98%)
...............................                               1739 / 1739 (100%)

Time: 15.46 minutes, Memory: 1.04 GB

There were 7 failures:

1) Mautic\CampaignBundle\Tests\Command\ExecuteEventCommandTest::testEventsAreExecutedForInactiveEventWithSingleContact
Failed asserting that actual size 0 matches expected size 3.

/home/travis/build/mautic/mautic/app/bundles/CampaignBundle/Tests/Command/ExecuteEventCommandTest.php:24

2) Mautic\CampaignBundle\Tests\Command\TriggerCampaignCommandTest::testCampaignExecutionForAll
Failed asserting that actual size 0 matches expected size 50.

/home/travis/build/mautic/mautic/app/bundles/CampaignBundle/Tests/Command/TriggerCampaignCommandTest.php:43

3) Mautic\CampaignBundle\Tests\Command\TriggerCampaignCommandTest::testCampaignExecutionForOne
Failed asserting that actual size 0 matches expected size 1.

/home/travis/build/mautic/mautic/app/bundles/CampaignBundle/Tests/Command/TriggerCampaignCommandTest.php:208

4) Mautic\CampaignBundle\Tests\Command\TriggerCampaignCommandTest::testCampaignExecutionForSome
Failed asserting that actual size 0 matches expected size 5.

/home/travis/build/mautic/mautic/app/bundles/CampaignBundle/Tests/Command/TriggerCampaignCommandTest.php:367

5) Mautic\CampaignBundle\Tests\Command\ValidateEventCommandTest::testEventsAreExecutedForInactiveEventWithSingleContact
Failed asserting that actual size 0 matches expected size 1.

/home/travis/build/mautic/mautic/app/bundles/CampaignBundle/Tests/Command/ValidateEventCommandTest.php:35

6) Mautic\CampaignBundle\Tests\Command\ValidateEventCommandTest::testEventsAreExecutedForInactiveEventWithMultipleContact
Failed asserting that actual size 0 matches expected size 3.

/home/travis/build/mautic/mautic/app/bundles/CampaignBundle/Tests/Command/ValidateEventCommandTest.php:59

7) Mautic\CampaignBundle\Tests\Command\ValidateEventCommandTest::testContactsRemovedFromTheCampaignAreNotExecutedForInactiveEvents
Failed asserting that actual size 0 matches expected size 2.

/home/travis/build/mautic/mautic/app/bundles/CampaignBundle/Tests/Command/ValidateEventCommandTest.php:90

FAILURES!
Tests: 1739, Assertions: 4984, Failures: 7, 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 1
TravisBuddy Request Identifier: b799d960-c1c6-11ea-8e31-7fd6e6dac888

@TravisBuddy
Copy link

Travis tests have failed

Hey @patrykgruszka,
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.

2nd Build

View build log

if [ ${TRAVIS_PHP_VERSION:0:3} == "7.3" ]; then composer test -- --coverage-clover=coverage.xml && bash <(curl -s https://codecov.io/bash); 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.

................FERROR 1364 (HY000) at line 42: Field 'public_name' doesn't have a default value
FERROR 1364 (HY000) at line 42: Field 'public_name' doesn't have a default value
FERROR 1364 (HY000) at line 42: Field 'public_name' doesn't have a default value
FERROR 1364 (HY000) at line 42: Field 'public_name' doesn't have a default value
FERROR 1364 (HY000) at line 42: Field 'public_name' doesn't have a default value
FERROR 1364 (HY000) at line 42: Field 'public_name' doesn't have a default value
FERROR 1364 (HY000) at line 42: Field 'public_name' doesn't have a default value
......................................   61 / 1739 (  3%)
.............................................................  122 / 1739 (  7%)
.............................................................  183 / 1739 ( 10%)
.............................................................  244 / 1739 ( 14%)
...........................................................SS  305 / 1739 ( 17%)
.............................................................  366 / 1739 ( 21%)
.............................................................  427 / 1739 ( 24%)
..SS.........................................................  488 / 1739 ( 28%)
.............................................................  549 / 1739 ( 31%)
.............................................................  610 / 1739 ( 35%)
.............................................................  671 / 1739 ( 38%)
.............................................................  732 / 1739 ( 42%)
.............................................................  793 / 1739 ( 45%)
............S................................................  854 / 1739 ( 49%)
.............................................................  915 / 1739 ( 52%)
.............................................................  976 / 1739 ( 56%)
............................................................. 1037 / 1739 ( 59%)
............................................................. 1098 / 1739 ( 63%)
............................................................. 1159 / 1739 ( 66%)
............................................................. 1220 / 1739 ( 70%)
............................................................. 1281 / 1739 ( 73%)
............................................................. 1342 / 1739 ( 77%)
............................................................. 1403 / 1739 ( 80%)
............................................................. 1464 / 1739 ( 84%)
............................................................. 1525 / 1739 ( 87%)
............................................................. 1586 / 1739 ( 91%)
............................................................. 1647 / 1739 ( 94%)
............................................................. 1708 / 1739 ( 98%)
...............................                               1739 / 1739 (100%)

Time: 15.04 minutes, Memory: 1.04 GB

There were 7 failures:

1) Mautic\CampaignBundle\Tests\Command\ExecuteEventCommandTest::testEventsAreExecutedForInactiveEventWithSingleContact
Failed asserting that actual size 0 matches expected size 3.

/home/travis/build/mautic/mautic/app/bundles/CampaignBundle/Tests/Command/ExecuteEventCommandTest.php:24

2) Mautic\CampaignBundle\Tests\Command\TriggerCampaignCommandTest::testCampaignExecutionForAll
Failed asserting that actual size 0 matches expected size 50.

/home/travis/build/mautic/mautic/app/bundles/CampaignBundle/Tests/Command/TriggerCampaignCommandTest.php:43

3) Mautic\CampaignBundle\Tests\Command\TriggerCampaignCommandTest::testCampaignExecutionForOne
Failed asserting that actual size 0 matches expected size 1.

/home/travis/build/mautic/mautic/app/bundles/CampaignBundle/Tests/Command/TriggerCampaignCommandTest.php:208

4) Mautic\CampaignBundle\Tests\Command\TriggerCampaignCommandTest::testCampaignExecutionForSome
Failed asserting that actual size 0 matches expected size 5.

/home/travis/build/mautic/mautic/app/bundles/CampaignBundle/Tests/Command/TriggerCampaignCommandTest.php:367

5) Mautic\CampaignBundle\Tests\Command\ValidateEventCommandTest::testEventsAreExecutedForInactiveEventWithSingleContact
Failed asserting that actual size 0 matches expected size 1.

/home/travis/build/mautic/mautic/app/bundles/CampaignBundle/Tests/Command/ValidateEventCommandTest.php:35

6) Mautic\CampaignBundle\Tests\Command\ValidateEventCommandTest::testEventsAreExecutedForInactiveEventWithMultipleContact
Failed asserting that actual size 0 matches expected size 3.

/home/travis/build/mautic/mautic/app/bundles/CampaignBundle/Tests/Command/ValidateEventCommandTest.php:59

7) Mautic\CampaignBundle\Tests\Command\ValidateEventCommandTest::testContactsRemovedFromTheCampaignAreNotExecutedForInactiveEvents
Failed asserting that actual size 0 matches expected size 2.

/home/travis/build/mautic/mautic/app/bundles/CampaignBundle/Tests/Command/ValidateEventCommandTest.php:90

FAILURES!
Tests: 1739, Assertions: 4984, Failures: 7, 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 1
TravisBuddy Request Identifier: 19878b90-c1e0-11ea-8e31-7fd6e6dac888

@codecov
Copy link

codecov bot commented Jul 13, 2020

Codecov Report

Merging #8238 (64618c1) into staging (8d5c970) will increase coverage by 0.27%.
The diff coverage is 90.00%.

Impacted file tree graph

@@              Coverage Diff              @@
##             staging    #8238      +/-   ##
=============================================
+ Coverage      31.17%   31.44%   +0.27%     
- Complexity     33546    33558      +12     
=============================================
  Files           1943     1943              
  Lines         116002   115828     -174     
=============================================
+ Hits           36162    36422     +260     
+ Misses         79840    79406     -434     
Impacted Files Coverage Δ Complexity Δ
...p/bundles/LeadBundle/Entity/LeadListRepository.php 35.58% <0.00%> (ø) 64.00 <0.00> (ø)
app/bundles/LeadBundle/Form/Type/LeadListType.php 82.35% <66.66%> (-4.32%) 9.00 <0.00> (+2.00) ⬇️
app/bundles/LeadBundle/Entity/LeadList.php 96.87% <100.00%> (+0.32%) 24.00 <2.00> (+2.00)
app/bundles/LeadBundle/Form/Type/ListType.php 79.64% <100.00%> (+0.74%) 13.00 <0.00> (ø)
app/bundles/LeadBundle/Model/ListModel.php 63.64% <100.00%> (+0.15%) 173.00 <0.00> (+1.00)
app/bundles/PageBundle/Model/PageModel.php 32.23% <0.00%> (+0.42%) 162.00% <0.00%> (+1.00%)
...undle/Swiftmailer/Transport/SparkpostTransport.php 72.98% <0.00%> (+1.12%) 89.00% <0.00%> (+5.00%)
...le/Form/DataTransformer/ArrayStringTransformer.php 90.00% <0.00%> (+60.00%) 5.00% <0.00%> (ø%)
app/bundles/PageBundle/Form/Type/PageListType.php 81.81% <0.00%> (+81.81%) 6.00% <0.00%> (ø%)
...Form/DataTransformer/ArrayLinebreakTransformer.php 87.50% <0.00%> (+87.50%) 4.00% <0.00%> (ø%)
... and 1 more

@patrykgruszka
Copy link
Member Author

Hi @npracht, this PR is adapted to version 3 and ready to merge.

@npracht npracht added mautic-3 ready-to-test PR's that are ready to test and removed triage-mautic-3 labels Jul 13, 2020
@RCheesley
Copy link
Sponsor Member

@cla-bot check

@cla-bot
Copy link

cla-bot bot commented Sep 19, 2020

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 @patrykgruszka.

@cla-bot
Copy link

cla-bot bot commented Sep 19, 2020

The CLA Bot has been sent on a mission to check against the latest list and will be back shortly with its findings!

@RCheesley
Copy link
Sponsor Member

@patrykgruszka thanks for submitting the PR - what a useful feature!

In order for us to consider this for merging in 3.2 we will require you to sign the Contributors Agreement - please see the link above in the comments. This is a one-time thing that all contributors need to do, once it's signed we can accept all your PR's going forward.

We will then need to get it tested and should be able to get it merged with the next minor release at the end of November.

@RCheesley RCheesley added this to the 3.2 milestone Sep 19, 2020
@RCheesley
Copy link
Sponsor Member

@cla-bot check

@cla-bot cla-bot bot added the cla-signed The PR contributors have signed the contributors agreement label Oct 8, 2020
@cla-bot
Copy link

cla-bot bot commented Oct 8, 2020

The CLA Bot has been sent on a mission to check against the latest list and will be back shortly with its findings!

@npracht npracht added the segments Anything related to segments label Oct 9, 2020
kuzmany
kuzmany previously approved these changes Oct 25, 2020
Copy link
Member

@kuzmany kuzmany left a comment

Choose a reason for hiding this comment

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

Works good 👍
Code looks good as well
Migration works

@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 Oct 25, 2020
@RCheesley RCheesley closed this Nov 8, 2020
@RCheesley RCheesley reopened this Nov 8, 2020
@RCheesley RCheesley added the needs-rebase PR's that need to be rebased label Nov 20, 2020
RCheesley
RCheesley previously approved these changes Nov 20, 2020
Copy link
Sponsor Member

@RCheesley RCheesley left a comment

Choose a reason for hiding this comment

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

@patrykgruszka looks like we need to rebase against staging for this one - can you take a look? We will also need a PR to the documentation page on segmentation explaining how to use this feature - https://github.com/mautic/mautic-documentation/blob/master/pages/09.contacts/03.manage-segments/docs.en.md here is the file to edit.

I was able to test this up to checking it appearing on the landing page, as I can't edit the settings on Mautibox (settings which are system-set are getting cleared so you cannot save the settings).

Based on other +1's I'm good with getting this merged providing Codecov passes after a rebase and the documentation PR is made.

@RCheesley RCheesley removed the pending-test-confirmation PR's that require one test before they can be merged label Nov 20, 2020
@RCheesley RCheesley added the needs-documentation PR's that need documentation before they can be merged label Nov 20, 2020
@RCheesley RCheesley dismissed stale reviews from kuzmany and themself via 6d284cf November 21, 2020 16:23
RCheesley
RCheesley previously approved these changes Nov 21, 2020
Copy link
Sponsor Member

@RCheesley RCheesley left a comment

Choose a reason for hiding this comment

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

Great PR, really helpful for the preference centre!

Tested with the PR applied and confirm that the migration set the public name automatically to the segment's name, and that when I changed this it showed in the preference centre:

screenshot-local mautic3-2020 11 21-16_38_07

screenshot-local mautic3-2020 11 21-16_36_02

Nice feature!

RCheesley added a commit to RCheesley/mautic-documentation that referenced this pull request Nov 21, 2020
@RCheesley RCheesley removed needs-documentation PR's that need documentation before they can be merged needs-rebase PR's that need to be rebased labels Nov 21, 2020
@RCheesley RCheesley merged commit 60bdfc8 into mautic:staging Nov 21, 2020
@mautibot
Copy link

mautibot commented Dec 1, 2020

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

https://forum.mautic.org/t/mautic-3-2-growing-together/17244/1

RCheesley added a commit to mautic/mautic-documentation that referenced this pull request Dec 2, 2020
* Improve segment documentation and add documentation to support mautic/mautic#8238

* Clarify we're talking about the contact not the Mautic user

* Grammar fix 🤦

* Grammar again.... more coffee needed 🤦
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 enhancement Any improvement to an existing feature or functionality segments Anything related to segments
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants