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

Fix the contact merge form on Drupal 8 #11992

Merged
merged 1 commit into from
Apr 19, 2018

Conversation

dsnopek
Copy link
Contributor

@dsnopek dsnopek commented Apr 18, 2018

Overview

Attempting to merge contacts which are associated with Drupal users will result in a fatal error.

Before

The steps to reproduce are basically:

  1. Create two new contacts with the same name
  2. Go to Search -> Find contacts and search for that name
  3. Select both and click "Merge contacts"
  4. Get a recoverable fatal error!

Here's the stack trace we get:

Recoverable fatal error: Object of class Drupal\Core\Field\FieldItemList could not be converted to string in include() (line 222 of /srv/bindings/4e16373a1b93425ab737c843ea127734/files/private/civicrm/templates_c/en_US/%%8B/8B3/8B3565BB%%Merge.tpl.php)
#0 /srv/bindings/4e16373a1b93425ab737c843ea127734/code/web/core/includes/bootstrap.inc(566): _drupal_error_handler_real(4096, 'Object of class...', '/srv/bindings/4...', 222, Array)
#1 /srv/bindings/4e16373a1b93425ab737c843ea127734/files/private/civicrm/templates_c/en_US/%%8B/8B3/8B3565BB%%Merge.tpl.php(222): _drupal_error_handler(4096, 'Object of class...', '/srv/bindings/4...', 222, Array)
#2 /srv/bindings/4e16373a1b93425ab737c843ea127734/code/vendor/civicrm/civicrm-core/packages/Smarty/Smarty.class.php(1911): include('/srv/bindings/4...')
#3 /srv/bindings/4e16373a1b93425ab737c843ea127734/files/private/civicrm/templates_c/en_US/%%0C/0CB/0CBEC124%%default.tpl.php(19): Smarty->_smarty_include(Array)
#4 /srv/bindings/4e16373a1b93425ab737c843ea127734/code/vendor/civicrm/civicrm-core/packages/Smarty/Smarty.class.php(1911): include('/srv/bindings/4...')
#5 /srv/bindings/4e16373a1b93425ab737c843ea127734/files/private/civicrm/templates_c/en_US/%%2B/2BD/2BD99720%%drupal8.tpl.php(56): Smarty->_smarty_include(Array)
#6 /srv/bindings/4e16373a1b93425ab737c843ea127734/code/vendor/civicrm/civicrm-core/packages/Smarty/Smarty.class.php(1270): include('/srv/bindings/4...')
#7 /srv/bindings/4e16373a1b93425ab737c843ea127734/code/vendor/civicrm/civicrm-core/CRM/Core/Smarty.php(197): Smarty->fetch('CRM/common/drup...', NULL, NULL, false)
#8 /srv/bindings/4e16373a1b93425ab737c843ea127734/code/vendor/civicrm/civicrm-core/CRM/Core/QuickForm/Action/Display.php(133): CRM_Core_Smarty->fetch('CRM/common/drup...')
#9 /srv/bindings/4e16373a1b93425ab737c843ea127734/code/vendor/civicrm/civicrm-core/CRM/Core/QuickForm/Action/Display.php(99): CRM_Core_QuickForm_Action_Display->renderForm(Object(CRM_Contact_Form_Merge))
#10 /srv/bindings/4e16373a1b93425ab737c843ea127734/code/vendor/civicrm/civicrm-core/packages/HTML/QuickForm/Controller.php(203): CRM_Core_QuickForm_Action_Display->perform(Object(CRM_Contact_Form_Merge), 'display')
#11 /srv/bindings/4e16373a1b93425ab737c843ea127734/code/vendor/civicrm/civicrm-core/packages/HTML/QuickForm/Page.php(103): HTML_QuickForm_Controller->handle(Object(CRM_Contact_Form_Merge), 'display')
#12 /srv/bindings/4e16373a1b93425ab737c843ea127734/code/vendor/civicrm/civicrm-core/CRM/Core/Controller.php(351): HTML_QuickForm_Page->handle('display')
#13 /srv/bindings/4e16373a1b93425ab737c843ea127734/code/vendor/civicrm/civicrm-core/CRM/Utils/Wrapper.php(113): CRM_Core_Controller->run()
#14 /srv/bindings/4e16373a1b93425ab737c843ea127734/code/vendor/civicrm/civicrm-core/CRM/Core/Invoke.php(282): CRM_Utils_Wrapper->run('CRM_Contact_For...', 'Merge Contact', Array)
#15 /srv/bindings/4e16373a1b93425ab737c843ea127734/code/vendor/civicrm/civicrm-core/CRM/Core/Invoke.php(84): CRM_Core_Invoke::runItem(Array)
#16 /srv/bindings/4e16373a1b93425ab737c843ea127734/code/vendor/civicrm/civicrm-core/CRM/Core/Invoke.php(52): CRM_Core_Invoke::_invoke(Array)
#17 /srv/bindings/4e16373a1b93425ab737c843ea127734/code/web/modules/contrib/civicrm/src/Civicrm.php(68): CRM_Core_Invoke::invoke(Array)
#18 /srv/bindings/4e16373a1b93425ab737c843ea127734/code/web/modules/contrib/civicrm/src/Controller/CivicrmController.php(47): Drupal\civicrm\Civicrm->invoke(Array)
#19 [internal function]: Drupal\civicrm\Controller\CivicrmController->main(Array, '')
#20 /srv/bindings/4e16373a1b93425ab737c843ea127734/code/web/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(123): call_user_func_array(Array, Array)
#21 /srv/bindings/4e16373a1b93425ab737c843ea127734/code/web/core/lib/Drupal/Core/Render/Renderer.php(582): Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber{closure}()
#22 /srv/bindings/4e16373a1b93425ab737c843ea127734/code/web/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(124): Drupal\Core\Render\Renderer->executeInRenderContext(Object(Drupal\Core\Render\RenderContext), Object(Closure))
#23 /srv/bindings/4e16373a1b93425ab737c843ea127734/code/web/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(97): Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array)
#24 [internal function]: Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber{closure}()
#25 /srv/bindings/4e16373a1b93425ab737c843ea127734/code/vendor/symfony/http-kernel/HttpKernel.php(153): call_user_func_array(Object(Closure), Array)
#26 /srv/bindings/4e16373a1b93425ab737c843ea127734/code/vendor/symfony/http-kernel/HttpKernel.php(68): Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object(Symfony\Component\HttpFoundation\Request), 1)
#27 /srv/bindings/4e16373a1b93425ab737c843ea127734/code/web/core/lib/Drupal/Core/StackMiddleware/Session.php(57): Symfony\Component\HttpKernel\HttpKernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#28 /srv/bindings/4e16373a1b93425ab737c843ea127734/code/web/core/lib/Drupal/Core/StackMiddleware/KernelPreHandle.php(47): Drupal\Core\StackMiddleware\Session->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#29 /srv/bindings/4e16373a1b93425ab737c843ea127734/code/web/core/modules/page_cache/src/StackMiddleware/PageCache.php(99): Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#30 /srv/bindings/4e16373a1b93425ab737c843ea127734/code/web/core/modules/page_cache/src/StackMiddleware/PageCache.php(78): Drupal\page_cache\StackMiddleware\PageCache->pass(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#31 /srv/bindings/4e16373a1b93425ab737c843ea127734/code/web/core/lib/Drupal/Core/StackMiddleware/ReverseProxyMiddleware.php(47): Drupal\page_cache\StackMiddleware\PageCache->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#32 /srv/bindings/4e16373a1b93425ab737c843ea127734/code/web/core/lib/Drupal/Core/StackMiddleware/NegotiationMiddleware.php(50): Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#33 /srv/bindings/4e16373a1b93425ab737c843ea127734/code/vendor/stack/builder/src/Stack/StackedHttpKernel.php(23): Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#34 /srv/bindings/4e16373a1b93425ab737c843ea127734/code/web/core/lib/Drupal/Core/DrupalKernel.php(664): Stack\StackedHttpKernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#35 /srv/bindings/4e16373a1b93425ab737c843ea127734/code/web/index.php(19): Drupal\Core\DrupalKernel->handle(Object(Symfony\Component\HttpFoundation\Request))
#36 {main}.

After

The form works as expected!

Technical Details

The problematic code is basically two instances of this:

       if ($config->userSystem->is_drupal == '1') {
         $mainUser = user_load($mainUfId);
       }
       elseif ($config->userFramework == 'Joomla') {
         $mainUser = JFactory::getUser($mainUfId);
       }
       $this->assign('mainUfName', $mainUser ? $mainUser->name : NULL);

While user_load() will actually work in Drupal 8, the name field is accessed like $mainUser->name->value (because in Drupal 8 all entity properties are fields).

Instead of directly using Drupal-specific code, this PR changes to using a pre-existing method: $config->userSystem->getUser()

However, it updates the Drupal 8 implementation to actually work, and also adds a Joomla one to make sure the name is set the same way as the original code. I don't know anything about Joomla and didn't test that, so I'll need some advice on if that was the right thing to do!

But the Drupal 8 is good, and tested :-)

@eileenmcnaughton
Copy link
Contributor

This fix is poetry - totally agree it's a code improvement - wonder if someone has a joomla instance to test it on - will ping in chat channel

@colemanw
Copy link
Member

Perhaps @lcdservices could verify it from a Joomla perspective?

@monishdeb
Copy link
Member

monishdeb commented Apr 19, 2018

I tested the patch in my local on Joomla and it works fine.

@monishdeb
Copy link
Member

(CiviCRM Review Template WORD-1.1)

@monishdeb
Copy link
Member

Test failure unrelated.

@monishdeb monishdeb merged commit edae2a2 into civicrm:master Apr 19, 2018
@dsnopek
Copy link
Contributor Author

dsnopek commented Apr 19, 2018

Woohoo, thanks, Everyone! :-)

@eileenmcnaughton
Copy link
Contributor

thanks @dsnopek @monishdeb

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