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

M3: Plugin bundle upgrade #8128

Merged
merged 58 commits into from Dec 11, 2019
Merged

M3: Plugin bundle upgrade #8128

merged 58 commits into from Dec 11, 2019

Conversation

hluchas
Copy link
Contributor

@hluchas hluchas commented Nov 14, 2019

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

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

Description:

Upgrade plugin bundle to support S3 and remove deprecations.

Steps to test this PR:

  1. Test forms with SF3

List backwards compatibility breaks:

  1. Removed deprecated method Mautic\PluginBundle\Controller\AjaxController::getIntegrationLeadFieldsAction()
  2. Removed deprecated method Mautic\PluginBundle\Controller\AjaxController::getIntegrationCompanyFieldsAction()
  3. Removed deprecated method Mautic\PluginBundle\Integration\AbstractIntegration::init()
  4. Removed deprecated method Mautic\PluginBundle\Integration\AbstractIntegration::getUserId()
  5. Changed constructor args in \Mautic\PluginBundle\Integration\AbstractIntegration and removed setter methods used by IntegrationPass
  6. Removed \CoreBundle\DependencyInjection\Compiler\IntegrationPass
  7. Removed \PluginBundle\Controller\AjaxController::getIntegrationLeadFieldsAction()
  8. Removed \PluginBundle\Controller\AjaxController::getIntegrationCompanyFieldsAction()
  9. Removed constructor from \Mautic\PluginBundle\Form\Type\FieldsType
  10. Removed constructor from \Mautic\PluginBundle\Form\Type\CompanyFieldsType

Leaved as it is:

  • \Mautic\PluginBundle\Helper\EventHelper::pushLead()
    It is called from \Mautic\PointBundle\Model\TriggerModel::triggerEvents which is part of PluginBundle.
  • \Mautic\PluginBundle\Bundle\PluginBundleBase
    This needs to be separated Issue/PR I think.

Testing steps:

  • Test plugin page. You have to see same plugin count.
  • Test plugin installation/update/reload via command.
  • Test Onesignal integration config
  • Test Outlook integration config
  • Test Facebook integration config
  • Test Foursquare integration config
  • Test Googleplus integration config
  • Test Instagram integration config
  • Test Linkedin integration config
  • Test Twitter integration config
  • Test configuration of linkedin plugin

@hluchas hluchas changed the title WIP: Plugin bundle upgrade Plugin bundle upgrade Nov 15, 2019
@hluchas hluchas changed the title Plugin bundle upgrade M3: Plugin bundle upgrade Nov 15, 2019
@hluchas hluchas added the code-review-needed PR's that require a code review before merging label Nov 15, 2019
@escopecz
Copy link
Sponsor Member

Failing tests in Travis. Please check.

@escopecz escopecz added the pending-feedback PR's and issues that are awaiting feedback from the author label Nov 15, 2019
@anton-vlasenko anton-vlasenko added the pending-feedback PR's and issues that are awaiting feedback from the author label Dec 9, 2019
@anton-vlasenko anton-vlasenko added this to the 3.0.0 milestone Dec 9, 2019
@hluchas hluchas removed the pending-feedback PR's and issues that are awaiting feedback from the author label Dec 9, 2019
Copy link
Contributor

@anton-vlasenko anton-vlasenko left a comment

Choose a reason for hiding this comment

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

I've clicked through all plugins and I got the following error in the app/logs.
Could you please have a look?

[2019-12-09 17:32:39] mautic.CRITICAL: Uncaught PHP Exception Symfony\Component\Debug\Exception\FatalThrowableError: "Type error: Argument 7 passed to MauticPlugin\MauticSocialBundle\Integration\SocialIntegration::__construct() must be an instance of Symfony\Component\Translation\DataCollectorTranslator, instance of Mautic\CoreBundle\Translation\Translator given, called in /Users/anton.vlasenko/html/community.test/app/cache/prod/appProdProjectContainer.php on line 11673" at /Users/anton.vlasenko/html/community.test/plugins/MauticSocialBundle/Integration/SocialIntegration.php line 60 {"exception":"[object] (Symfony\\Component\\Debug\\Exception\\FatalThrowableError(code: 0): Type error: Argument 7 passed to MauticPlugin\\MauticSocialBundle\\Integration\\SocialIntegration::__construct() must be an instance of Symfony\\Component\\Translation\\DataCollectorTranslator, instance of Mautic\\CoreBundle\\Translation\\Translator given, called in /Users/anton.vlasenko/html/community.test/app/cache/prod/appProdProjectContainer.php on line 11673 at /Users/anton.vlasenko/html/community.test/plugins/MauticSocialBundle/Integration/SocialIntegration.php:60)"} []

@anton-vlasenko anton-vlasenko added the pending-feedback PR's and issues that are awaiting feedback from the author label Dec 9, 2019
@hluchas
Copy link
Contributor Author

hluchas commented Dec 10, 2019

I've clicked through all plugins and I got the following error in the app/logs.
Could you please have a look?

[2019-12-09 17:32:39] mautic.CRITICAL: Uncaught PHP Exception Symfony\Component\Debug\Exception\FatalThrowableError: "Type error: Argument 7 passed to MauticPlugin\MauticSocialBundle\Integration\SocialIntegration::__construct() must be an instance of Symfony\Component\Translation\DataCollectorTranslator, instance of Mautic\CoreBundle\Translation\Translator given, called in /Users/anton.vlasenko/html/community.test/app/cache/prod/appProdProjectContainer.php on line 11673" at /Users/anton.vlasenko/html/community.test/plugins/MauticSocialBundle/Integration/SocialIntegration.php line 60 {"exception":"[object] (Symfony\\Component\\Debug\\Exception\\FatalThrowableError(code: 0): Type error: Argument 7 passed to MauticPlugin\\MauticSocialBundle\\Integration\\SocialIntegration::__construct() must be an instance of Symfony\\Component\\Translation\\DataCollectorTranslator, instance of Mautic\\CoreBundle\\Translation\\Translator given, called in /Users/anton.vlasenko/html/community.test/app/cache/prod/appProdProjectContainer.php on line 11673 at /Users/anton.vlasenko/html/community.test/plugins/MauticSocialBundle/Integration/SocialIntegration.php:60)"} []

I can't reproduce this. Did you upade your cache and use app/console mautic:plugin:update?
Or which one is producing this error?

@hluchas hluchas added pending-feedback PR's and issues that are awaiting feedback from the author and removed pending-feedback PR's and issues that are awaiting feedback from the author labels Dec 10, 2019
Copy link
Contributor

@anton-vlasenko anton-vlasenko left a comment

Choose a reason for hiding this comment

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

Could you please also change DataCollectorTranslator interface to TranslatorInterface for the following classes (they all extend \Mautic\PluginBundle\Integration\AbstractIntegration's constructor):
\MauticPlugin\MauticCrmBundle\Integration\SugarcrmIntegration
\MauticPlugin\MauticCrmBundle\Integration\PipedriveIntegration
\MauticPlugin\MauticCrmBundle\Integration\HubspotIntegration

@hluchas
Copy link
Contributor Author

hluchas commented Dec 10, 2019

Could you please also change DataCollectorTranslator interface to TranslatorInterface for the following classes (they all extend \Mautic\PluginBundle\Integration\AbstractIntegration's constructor):
\MauticPlugin\MauticCrmBundle\Integration\SugarcrmIntegration
\MauticPlugin\MauticCrmBundle\Integration\PipedriveIntegration
\MauticPlugin\MauticCrmBundle\Integration\HubspotIntegration

Found it. Thanx.

@hluchas
Copy link
Contributor Author

hluchas commented Dec 10, 2019

Test is failing with notice. Do not merge it before fix. @anton-vlasenko @escopecz

@hluchas
Copy link
Contributor Author

hluchas commented Dec 10, 2019

\MauticPlugin\MauticCrmBundle\Tests\Pipedrive\Export\LeadExportTest is producing notice after m2-to-m3 merge because these variables are not available in CLI.

PHP Notice:  Undefined index: SERVER_PROTOCOL in /Users/lukas.drahy/dev/community-fork/app/bundles/CoreBundle/Helper/UrlHelper.php on line 93
Undefined index: SERVER_PORT in /Users/lukas.drahy/dev/community-fork/app/bundles/CoreBundle/Helper/UrlHelper.php on line 95
PHP Notice:  Undefined index: SERVER_NAME in /Users/lukas.drahy/dev/community-fork/app/bundles/CoreBundle/Helper/UrlHelper.php on line 98
PHP Notice:  Undefined index: REQUEST_URI in /Users/lukas.drahy/dev/community-fork/app/bundles/CoreBundle/Helper/UrlHelper.php on line 99

I'm not sure if there was workaround in tests for this cases, but I don't know where and why it is not working anymore.

Stacktrace:

PHP   1. {main}() /Users/lukas.drahy/dev/community-fork/vendor/phpunit/phpunit/phpunit:0
PHP   2. PHPUnit_TextUI_Command::main() /Users/lukas.drahy/dev/community-fork/vendor/phpunit/phpunit/phpunit:52
PHP   3. PHPUnit_TextUI_Command->run() /Users/lukas.drahy/dev/community-fork/vendor/phpunit/phpunit/src/TextUI/Command.php:116
PHP   4. PHPUnit_TextUI_TestRunner->doRun() /Users/lukas.drahy/dev/community-fork/vendor/phpunit/phpunit/src/TextUI/Command.php:186
PHP   5. PHPUnit_Framework_TestSuite->run() /Users/lukas.drahy/dev/community-fork/vendor/phpunit/phpunit/src/TextUI/TestRunner.php:517
PHP   6. PHPUnit_Framework_TestSuite->run() /Users/lukas.drahy/dev/community-fork/vendor/phpunit/phpunit/src/Framework/TestSuite.php:733
PHP   7. PHPUnit_Framework_TestSuite->run() /Users/lukas.drahy/dev/community-fork/vendor/phpunit/phpunit/src/Framework/TestSuite.php:733
PHP   8. MauticPlugin\MauticCrmBundle\Tests\Pipedrive\Export\LeadExportTest->run() /Users/lukas.drahy/dev/community-fork/vendor/phpunit/phpunit/src/Framework/TestSuite.php:733
PHP   9. PHPUnit_Framework_TestResult->run() /Users/lukas.drahy/dev/community-fork/vendor/phpunit/phpunit/src/Framework/TestCase.php:868
PHP  10. MauticPlugin\MauticCrmBundle\Tests\Pipedrive\Export\LeadExportTest->runBare() /Users/lukas.drahy/dev/community-fork/vendor/phpunit/phpunit/src/Framework/TestResult.php:686
PHP  11. MauticPlugin\MauticCrmBundle\Tests\Pipedrive\Export\LeadExportTest->runTest() /Users/lukas.drahy/dev/community-fork/vendor/phpunit/phpunit/src/Framework/TestCase.php:913
PHP  12. ReflectionMethod->invokeArgs() /Users/lukas.drahy/dev/community-fork/vendor/phpunit/phpunit/src/Framework/TestCase.php:1062
PHP  13. MauticPlugin\MauticCrmBundle\Tests\Pipedrive\Export\LeadExportTest->testUpdatePersonWithOwner() /Users/lukas.drahy/dev/community-fork/vendor/phpunit/phpunit/src/Framework/TestCase.php:1062
PHP  14. Symfony\Bundle\FrameworkBundle\Client->request() /Users/lukas.drahy/dev/community-fork/plugins/MauticCrmBundle/Tests/Pipedrive/Export/LeadExportTest.php:192
PHP  15. Symfony\Bundle\FrameworkBundle\Client->doRequest() /Users/lukas.drahy/dev/community-fork/vendor/symfony/browser-kit/Client.php:311
PHP  16. Symfony\Bundle\FrameworkBundle\Client->doRequest() /Users/lukas.drahy/dev/community-fork/vendor/symfony/framework-bundle/Client.php:131
PHP  17. AppTestKernel->handle() /Users/lukas.drahy/dev/community-fork/vendor/symfony/http-kernel/Client.php:58
PHP  18. AppTestKernel->handle() /Users/lukas.drahy/dev/community-fork/app/AppKernel.php:141
PHP  19. Symfony\Component\HttpKernel\DependencyInjection\ContainerAwareHttpKernel->handle() /Users/lukas.drahy/dev/community-fork/vendor/symfony/http-kernel/Kernel.php:183
PHP  20. Symfony\Component\HttpKernel\DependencyInjection\ContainerAwareHttpKernel->handle() /Users/lukas.drahy/dev/community-fork/vendor/symfony/http-kernel/DependencyInjection/ContainerAwareHttpKernel.php:67
PHP  21. Symfony\Component\HttpKernel\DependencyInjection\ContainerAwareHttpKernel->handleRaw() /Users/lukas.drahy/dev/community-fork/vendor/symfony/http-kernel/HttpKernel.php:57
PHP  22. Mautic\LeadBundle\Controller\LeadController->executeAction() /Users/lukas.drahy/dev/community-fork/vendor/symfony/http-kernel/HttpKernel.php:135
PHP  23. Mautic\LeadBundle\Controller\LeadController->editAction() /Users/lukas.drahy/dev/community-fork/app/bundles/CoreBundle/Controller/CommonController.php:474
PHP  24. Mautic\LeadBundle\Controller\LeadController->delegateView() /Users/lukas.drahy/dev/community-fork/app/bundles/LeadBundle/Controller/LeadController.php:722
PHP  25. Mautic\LeadBundle\Controller\LeadController->ajaxAction() /Users/lukas.drahy/dev/community-fork/app/bundles/CoreBundle/Controller/CommonController.php:224
PHP  26. Mautic\LeadBundle\Controller\LeadController->renderView() /Users/lukas.drahy/dev/community-fork/app/bundles/CoreBundle/Controller/CommonController.php:393
PHP  27. Symfony\Bundle\FrameworkBundle\Templating\DelegatingEngine->render() /Users/lukas.drahy/dev/community-fork/vendor/symfony/framework-bundle/Controller/Controller.php:163
PHP  28. Mautic\CoreBundle\Templating\Engine\PhpEngine->render() /Users/lukas.drahy/dev/community-fork/vendor/symfony/templating/DelegatingEngine.php:41
PHP  29. Mautic\CoreBundle\Templating\Engine\PhpEngine->render() /Users/lukas.drahy/dev/community-fork/app/bundles/CoreBundle/Templating/Engine/PhpEngine.php:144
PHP  30. Mautic\CoreBundle\Templating\Engine\PhpEngine->evaluate() /Users/lukas.drahy/dev/community-fork/vendor/symfony/templating/PhpEngine.php:78
PHP  31. require() /Users/lukas.drahy/dev/community-fork/app/bundles/CoreBundle/Templating/Engine/PhpEngine.php:175
PHP  32. Mautic\LeadBundle\Templating\Helper\AvatarHelper->getAvatar() /Users/lukas.drahy/dev/community-fork/app/bundles/LeadBundle/Views/Lead/form.html.php:22
PHP  33. Mautic\LeadBundle\Templating\Helper\AvatarHelper->getDefaultAvatar() /Users/lukas.drahy/dev/community-fork/app/bundles/LeadBundle/Templating/Helper/AvatarHelper.php:71
PHP  34. Mautic\CoreBundle\Helper\UrlHelper::rel2abs() /Users/lukas.drahy/dev/community-fork/app/bundles/LeadBundle/Templating/Helper/AvatarHelper.php:100

@alanhartless Do you have an idea?

$scheme = strtolower($_SERVER['SERVER_PROTOCOL']);

@hluchas hluchas mentioned this pull request Dec 11, 2019
@hluchas
Copy link
Contributor Author

hluchas commented Dec 11, 2019

This was introduced in #8137 - travis log here https://travis-ci.org/mautic/mautic/builds/623066898?utm_source=github_status&utm_medium=notification. This has nothing to do with plugin bundle and it is already merged in m2-to-m3 branch.

I think we can merge this and I'll create ticket for bug described above #8128 (comment) as I'm not able to fix it in reasonable time now. What do you think @escopecz

@anton-vlasenko
Copy link
Contributor

@hluchas I'd like to test it again before we merge. Just to be sure that everything is OK, because it's a big PR.

Copy link
Contributor

@anton-vlasenko anton-vlasenko left a comment

Choose a reason for hiding this comment

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

I've code reviewed and tested this PR and at the moment I can't find any errors using the test steps provided by the author.
Therefore I approve it.

@escopecz
Copy link
Sponsor Member

Thanks guys! You are right @hluchas. I missed it in the LeadBundle PR. Please create the issue for it.

@escopecz escopecz merged commit 2f3d8c8 into mautic:m2-to-m3 Dec 11, 2019
@escopecz escopecz removed code-review-needed PR's that require a code review before merging pending-feedback PR's and issues that are awaiting feedback from the author labels Dec 11, 2019
@hluchas
Copy link
Contributor Author

hluchas commented Dec 11, 2019

New issue here #8217

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants