M3: Refactor builder token helper #8204

mtshaw3 · 2019-12-09T16:20:00Z

\Mautic\CoreBundle\Helper\BuilderTokenHelper is dependent on MauticFactory and thus needs to be refactored to a service instead in order to use dependency injection.

This PR replaces the broken PR that was rebased to the 3.x branch: #8160

GH issue: #8088

…okenBuilder. Refactored out use of MauticFactory from HelperTokenBuilder. Made HelperTokenBuilder a service.

…recated leadModel->getCurrentLead()

escopecz

Good idea with the factory. I noticed some issues.

app/bundles/AssetBundle/EventListener/BuilderSubscriber.php

app/bundles/CoreBundle/Helper/BuilderTokenHelperFactory.php

app/bundles/FormBundle/EventListener/PageSubscriber.php

app/bundles/LeadBundle/Config/config.php

app/bundles/LeadBundle/EventListener/EmailSubscriber.php

app/bundles/PageBundle/EventListener/BuilderSubscriber.php

escopecz

Thanks! I reviewed again and suggested small improvements.

app/bundles/CoreBundle/Helper/BuilderTokenHelper.php

app/bundles/CoreBundle/Helper/BuilderTokenHelperFactory.php

plugins/MauticFocusBundle/EventListener/PageSubscriber.php

Co-Authored-By: John Linhart <jan@linhart.email>

app/bundles/CoreBundle/Helper/BuilderTokenHelper.php

plugins/MauticFocusBundle/EventListener/PageSubscriber.php

Co-Authored-By: John Linhart <jan@linhart.email>

dongilbert · 2019-12-13T20:51:52Z

Something is causing this error when I'm walking through testing.

[2019-12-13 20:50:06] mautic.CRITICAL: Uncaught PHP Exception Doctrine\Common\Persistence\Mapping\MappingException: "Class 'asset' does not exist" at /Users/dgilbert/Dev/mautic/vendor/doctrine/common/lib/Doctrine/Common/Persistence/Mapping/MappingException.php line 96 {"exception":"[object] (Doctrine\\Common\\Persistence\\Mapping\\MappingException(code: 0): Class 'asset' does not exist at /Users/dgilbert/Dev/mautic/vendor/doctrine/common/lib/Doctrine/Common/Persistence/Mapping/MappingException.php:96)"} []

I wanted to create a new Page so that the I could test the builder tokens are being inserted into the builder correctly, but when clicking the new button on the page list view, it failed to load the form and instead dumped this error into the logs.

app/bundles/CoreBundle/Helper/BuilderTokenHelper.php

dongilbert · 2019-12-16T14:27:39Z

We need to update the BuilderTokenHelperFactory as well - https://github.com/mautic/mautic/pull/8204/files#diff-966ddd047b8bdaf985231305e091f415R525

escopecz

Looks good and works well. I tested page builder and email builder. Tokens are being replaced when sent to contacts. Thanks @mtshaw3 👍

mtshaw3 and others added 6 commits December 9, 2019 11:12

Initial commit for refactoring MauticFactory out of BuilderTokenHelper

3b18789

Created new HelperTokenBuilderFactory to provide instances of HelperT…

1fb4fce

…okenBuilder. Refactored out use of MauticFactory from HelperTokenBuilder. Made HelperTokenBuilder a service.

Clean up builder subscribers a bit. Use ContactTracker instead of dep…

1646ce6

…recated leadModel->getCurrentLead()

Revert the addTokensFromHelper to be consistent with previous code

7d17943

Import missing class name

80f3e1a

Merge branch 'm2-to-m3' into refactor-builder-token-helper

50a4cf6

mtshaw3 changed the title ~~Refactor builder token helper~~ M3: Refactor builder token helper Dec 9, 2019

mtshaw3 added code-review-needed PR's that require a code review before merging Mautic 3 labels Dec 9, 2019

mtshaw3 mentioned this pull request Dec 9, 2019

M3: Refactor BuilderTokenHelper away from MauticFactory #8088 #8160

Closed

mtshaw3 and others added 2 commits December 9, 2019 11:52

Merge branch 'm2-to-m3' into refactor-builder-token-helper

6bda906

Fixes for build problem from merge

6235518

mtshaw3 requested a review from dongilbert December 9, 2019 18:54

escopecz mentioned this pull request Dec 10, 2019

M3: Refactor BuilderTokenHelper away from MauticFactory #8088

Closed

escopecz added the ready-to-test PR's that are ready to test label Dec 10, 2019

escopecz added this to the 3.0.0 milestone Dec 10, 2019

escopecz requested changes Dec 10, 2019

View reviewed changes

escopecz added the pending-feedback PR's and issues that are awaiting feedback from the author label Dec 10, 2019

mtshaw3 and others added 3 commits December 10, 2019 13:17

changes from code review

7429549

Merge branch 'm2-to-m3' into refactor-builder-token-helper

bb4892c

fixing build error

514c1ce

mtshaw3 removed the pending-feedback PR's and issues that are awaiting feedback from the author label Dec 10, 2019

mtshaw3 requested a review from escopecz December 10, 2019 19:02

escopecz requested changes Dec 12, 2019

View reviewed changes

escopecz added the pending-feedback PR's and issues that are awaiting feedback from the author label Dec 12, 2019

mtshaw3 and others added 4 commits December 12, 2019 11:34

Update plugins/MauticFocusBundle/EventListener/PageSubscriber.php

da1cd8c

Co-Authored-By: John Linhart <jan@linhart.email>

changes for code review

c6f305d

fixing build error

fedfbed

fixed build error

7a122b9

escopecz reviewed Dec 13, 2019

View reviewed changes

app/bundles/CoreBundle/Helper/BuilderTokenHelper.php Outdated Show resolved Hide resolved

escopecz reviewed Dec 13, 2019

View reviewed changes

app/bundles/CoreBundle/Helper/BuilderTokenHelper.php Outdated Show resolved Hide resolved

escopecz reviewed Dec 13, 2019

View reviewed changes

app/bundles/CoreBundle/Helper/BuilderTokenHelper.php Outdated Show resolved Hide resolved

escopecz reviewed Dec 13, 2019

View reviewed changes

plugins/MauticFocusBundle/EventListener/PageSubscriber.php Outdated Show resolved Hide resolved

mtshaw3 and others added 2 commits December 13, 2019 10:57

Update app/bundles/CoreBundle/Helper/BuilderTokenHelper.php

912cef1

Co-Authored-By: John Linhart <jan@linhart.email>

changes for code review

49b4f4f

mtshaw3 changed the title ~~M3: Refactor builder token helper~~ M3: Refactor builder token helper Dec 13, 2019

removed useless doc blocks

3de4a18

dongilbert requested changes Dec 13, 2019

View reviewed changes

app/bundles/CoreBundle/Helper/BuilderTokenHelper.php Outdated Show resolved Hide resolved

fixed issue with resolving repository from model name

ddab0e7

updated dependencies for builderTokenHelperFactory

c1cba19

escopecz removed the pending-feedback PR's and issues that are awaiting feedback from the author label Dec 18, 2019

escopecz approved these changes Dec 19, 2019

View reviewed changes

escopecz merged commit 0806cbd into mautic:m2-to-m3 Dec 19, 2019

escopecz removed code-review-needed PR's that require a code review before merging ready-to-test PR's that are ready to test labels Dec 19, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

M3: Refactor builder token helper #8204

M3: Refactor builder token helper #8204

mtshaw3 commented Dec 9, 2019 •

edited by escopecz

escopecz left a comment

escopecz left a comment

dongilbert commented Dec 13, 2019

dongilbert commented Dec 16, 2019

escopecz left a comment

M3: Refactor builder token helper #8204

M3: Refactor builder token helper #8204

Conversation

mtshaw3 commented Dec 9, 2019 • edited by escopecz

escopecz left a comment

Choose a reason for hiding this comment

escopecz left a comment

Choose a reason for hiding this comment

dongilbert commented Dec 13, 2019

dongilbert commented Dec 16, 2019

escopecz left a comment

Choose a reason for hiding this comment

mtshaw3 commented Dec 9, 2019 •

edited by escopecz