Problem/Motivation

For fields with multiple property values, in the 7.x version of the module we could select them right in the mapping UI. This was particularly convenient for Address fields, which require a lot of clicking to map anyway. In #2899460, we created a workaround, the "Properties, Extended" field mapping type. That is more powerful still, allowing you to drill down into related objects, but the UX for it is confusing and awkward, especially when editing existing mappings.

Proposed resolution

While working on the Mailchimp module in D8, I noticed a nice field property selector. I realized I could pull that code over to this module to recreate the D7 experience. Doing this produced some need for more sophisticated value code, but I found that the 'Properties, Extended" mapping field type already did all the work I needed.

So, I pulled a great deal of the Properties Extended field plugin into the Properties plugin, and made the Properties, Extended plugin an extension of the Properties plugin (it reads well!).

The result is a better "Properties" plugin.

User interface changes

You now see properties of complex-data fields like address fields in the mapping UI for "Properties" mappings.

API changes

Effectively none.

Data model changes

None.

Comments

gcb created an issue. See original summary.

gcb’s picture

aaronbauman’s picture

Status: Needs review » Needs work

I'm getting an error when saving a new mapping after adding fields:

Error: Cannot create references to/from string offsets in core/lib/Drupal/Component/Utility/NestedArray.php on line 155 #0 core/lib/Drupal/Core/Form/FormBuilder.php(1247): Drupal\\Component\\Utility\
estedArray::setValue(Array, Array, NULL)
#1 core/lib/Drupal/Core/Form/FormBuilder.php(992): Drupal\\Core\\Form\\FormBuilder->handleInputElement('salesforce_mapp...', Array, Object(Drupal\\Core\\Form\\FormState))
#2 core/lib/Drupal/Core/Form/FormBuilder.php(1062): Drupal\\Core\\Form\\FormBuilder->doBuildForm('salesforce_mapp...', Array, Object(Drupal\\Core\\Form\\FormState))
#3 core/lib/Drupal/Core/Form/FormBuilder.php(1062): Drupal\\Core\\Form\\FormBuilder->doBuildForm('salesforce_mapp...', Array, Object(Drupal\\Core\\Form\\FormState))
#4 core/lib/Drupal/Core/Form/FormBuilder.php(1062): Drupal\\Core\\Form\\FormBuilder->doBuildForm('salesforce_mapp...', Array, Object(Drupal\\Core\\Form\\FormState))
#5 core/lib/Drupal/Core/Form/FormBuilder.php(1062): Drupal\\Core\\Form\\FormBuilder->doBuildForm('salesforce_mapp...', Array, Object(Drupal\\Core\\Form\\FormState))
#6 core/lib/Drupal/Core/Form/FormBuilder.php(1062): Drupal\\Core\\Form\\FormBuilder->doBuildForm('salesforce_mapp...', Array, Object(Drupal\\Core\\Form\\FormState))
#7 core/lib/Drupal/Core/Form/FormBuilder.php(1062): Drupal\\Core\\Form\\FormBuilder->doBuildForm('salesforce_mapp...', Array, Object(Drupal\\Core\\Form\\FormState))
#8 core/lib/Drupal/Core/Form/FormBuilder.php(563): Drupal\\Core\\Form\\FormBuilder->doBuildForm('salesforce_mapp...', Array, Object(Drupal\\Core\\Form\\FormState))
#9 core/lib/Drupal/Core/Form/FormBuilder.php(320): Drupal\\Core\\Form\\FormBuilder->processForm('salesforce_mapp...', Array, Object(Drupal\\Core\\Form\\FormState))
#10 core/lib/Drupal/Core/Controller/FormController.php(91): Drupal\\Core\\Form\\FormBuilder->buildForm(Object(Drupal\\salesforce_mapping_ui\\Form\\SalesforceMappingFieldsForm), Object(Drupal\\Core\\Form\\FormState))
#11 [internal function]: Drupal\\Core\\Controller\\FormController->getContentResult(Object(Symfony\\Component\\HttpFoundation\\Request), Object(Drupal\\Core\\Routing\\RouteMatch))
#12 core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(123): call_user_func_array(Array, Array)
#13 core/lib/Drupal/Core/Render/Renderer.php(573): Drupal\\Core\\EventSubscriber\\EarlyRenderingControllerWrapperSubscriber->Drupal\\Core\\EventSubscriber\\{closure}()
#14 core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(124): Drupal\\Core\\Render\\Renderer->executeInRenderContext(Object(Drupal\\Core\\Render\\RenderContext), Object(Closure))
#15 core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(97): Drupal\\Core\\EventSubscriber\\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array)
#16 vendor/symfony/http-kernel/HttpKernel.php(151): Drupal\\Core\\EventSubscriber\\EarlyRenderingControllerWrapperSubscriber->Drupal\\Core\\EventSubscriber\\{closure}()
#17 vendor/symfony/http-kernel/HttpKernel.php(68): Symfony\\Component\\HttpKernel\\HttpKernel->handleRaw(Object(Symfony\\Component\\HttpFoundation\\Request), 1)
#18 core/lib/Drupal/Core/StackMiddleware/Session.php(57): Symfony\\Component\\HttpKernel\\HttpKernel->handle(Object(Symfony\\Component\\HttpFoundation\\Request), 1, true)
#19 core/lib/Drupal/Core/StackMiddleware/KernelPreHandle.php(47): Drupal\\Core\\StackMiddleware\\Session->handle(Object(Symfony\\Component\\HttpFoundation\\Request), 1, true)
#20 core/modules/page_cache/src/StackMiddleware/PageCache.php(106): Drupal\\Core\\StackMiddleware\\KernelPreHandle->handle(Object(Symfony\\Component\\HttpFoundation\\Request), 1, true)
#21 core/modules/page_cache/src/StackMiddleware/PageCache.php(85): Drupal\\page_cache\\StackMiddleware\\PageCache->pass(Object(Symfony\\Component\\HttpFoundation\\Request), 1, true)
#22 core/lib/Drupal/Core/StackMiddleware/ReverseProxyMiddleware.php(47): Drupal\\page_cache\\StackMiddleware\\PageCache->handle(Object(Symfony\\Component\\HttpFoundation\\Request), 1, true)
#23 core/lib/Drupal/Core/StackMiddleware/NegotiationMiddleware.php(52): Drupal\\Core\\StackMiddleware\\ReverseProxyMiddleware->handle(Object(Symfony\\Component\\HttpFoundation\\Request), 1, true)
#24 vendor/stack/builder/src/Stack/StackedHttpKernel.php(23): Drupal\\Core\\StackMiddleware\
egotiationMiddleware->handle(Object(Symfony\\Component\\HttpFoundation\\Request), 1, true)
#25 core/lib/Drupal/Core/DrupalKernel.php(708): Stack\\StackedHttpKernel->handle(Object(Symfony\\Component\\HttpFoundation\\Request), 1, true)
#26 index.php(19): Drupal\\Core\\DrupalKernel->handle(Object(Symfony\\Component\\HttpFoundation\\Request))

gcb’s picture

Assigned: aaronbauman » gcb
gcb’s picture

Assigned: gcb » aaronbauman
Status: Needs work » Needs review
StatusFileSize
new11.11 KB

Well that's ugly. I see what happened there: my attempt to be cute and turn PropertiesExtended into an extension of Properties, for purposes of DRYness, was probably ill-advised.

Here's a simpler patch that just adds the functionality to Properties field plugin.

I might advocate for deprecating PropertiesExtended if this works: I'm not sure what data structure you'd want to use PropertiesExtended for if you can get to things from here.

aaronbauman’s picture

Status: Needs review » Reviewed & tested by the community

Cool, seems to be working now.
Properties Extended allows to traverse arbitrarily deep into entity relationships, so I don't think it can be replaced yet.
I just can't picture a way to make that interface work in a static select-option widget without having an overwhelmingly long list.

Really both the Properties plugins could be replaced by Token...
but I think it's fine that there's some overlap though, so long as the maintenance doesn't get too onerous.
To that end, eventually (after this patch is committed) I'd like to figure some refactoring here because the constructors are getting super ugly.

Not necessary for this patch, but would be good to eventually get some test coverage for actually submitting the mapping form to save the mapping. I think the coverage we have now basically just loads the form and clicks the "add field" button.

gcb’s picture

Agreed: It seems like the "Properties Extended" was built to solve the problem of selecting sub-properties, but accidentally provided a ton more functionality (if I'm reading the ticket history accurately). Certainly a drop-down is impractical -- too easy to get a circular reference!

The tokens are a little developer-y for some of the site admins we work with, who actually do make mapping changes when they add new fields. Scary but surprisingly viable.

It does seem like a lot of overlapping code could be united in the two of them, though. Tests would definitely be good.

Thanks for looking at this!

  • AaronBauman committed 8c6855a on 8.x-4.x authored by gcb
    Issue #3168053 by gcb: Field Property Access in mapping ui
    
aaronbauman’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.