Handling of field properties

Looks like those are given by FieldItemInterface::propertyDefinitions.

I'm looking at FieldDefinitionInterface, since that what we get from entityTypeManager::getFieldDefinitions(), and I don't see how to traverse to FieldItemInterface to get at those properties.

I'm closing up for the day, but will look a bit more deeply tomorrow for the correct path.

OK, after digging through docs I figured out how to get at the property definitions.

For some ungodly reason, BaseFieldDefiniton and FieldConfig are not analogous, so we have to distinguish between the two.
Otherwise, it's relatively straightforward:

      if ($property instanceof BaseFieldDefinition) {
        $field_properties = $property->getPropertyDefinitions();
        $main_property = $property->getMainPropertyName();
      }
      elseif ($property instanceof FieldConfig) {
        $field_properties = $property->getFieldStorageDefinition()->getPropertyDefinitions();
        $main_property = $property->getFieldStorageDefinition()->getMainPropertyName();
      }

In terms of implementation, 9/10 times we'll just be using the main property.
So, I think the UI should make that easy and even encourage selecting the main property.
And, let's take the opportunity to move away from colon-separated array keys we've been stuck with since D6.
The "property" selection should be structured data.

As much as I'm loathe to add dependencies, typed_data probably makes a lot of sense, especially in that it could help us integrate with rules.

I think it probably makes sense to preserve most of the existing Properties plugin, and store the mapping fields as typed_data property paths.
This would also open the door to a property-path plugin (similar to Token plugin) which would provide a ton of power to mapping admin.

I can't actually figure out in the meantime how to alter the value for a pull. The only way I could think of was to build on the referenced constructor value, but I still needed to add another reference when it gets saved as a property, as well as a setter function which seems to be in line with the rest of the event structures.

I have a small patch, unless maybe there is some work done on this that I can use - or something I'm missing entirely.

Crap, sorry, wrong patch. Fail.

Thanks for the patch
I understand the addition of a setter, but not the double-reference:

   public function __construct(&$value, SalesforceMappingFieldPluginInterface $field_plugin, MappedObjectInterface $mapped_object) {
-    $this->entity_value = $value;
+    $this->entity_value = &$value;

Does the initial pass-by-reference not allow setting $entity_value in subscribers?
Why do we need both the reference and the setter?

Yeah I actually think when you set the property, if you're not setting by reference, then it loses the reference. I tried it out originally and couldn't get it to work without setting the property as a reference.

I tried out a dumb little PHP script just to isolate it.

class ReferenceTester {
  protected $var;
  public function __construct(&$var) {
    $this->var = $var;
  }

  public function changeVar($var) {
    $this->var = $var;
  }
}

$originalVar = 'originalVar';

$tester = new ReferenceTester($originalVar);

$tester->changeVar('newVar');
var_dump($originalVar);

Following up here, via #2947393: Address field support
Any progress on this issue?

Currently working on this,

I rebased the branch on top of the most recent 8.x-3.x branch.
Should I post as one single patch? I'm not sure how I can push to the salesforce repo to make a pull request.

Couple things to address

• SalesforceMappingFieldPluginBase: 222 There is a @TODO for referencing this issue. Do we want this field properties extension on just the Properties Plugin or do we want to implement this on the PluginBase? (Currently just on the Properties Plugin
• Properties: 104 > Should we validate User selection of the mappings or just trust the user to map correct fields?

I haven't ran all of the tests locally but pushing up some work for additional input.

Hopefully this fixes some coding conventions and module dependency issues.

Thanks you for the patch.
Uploading here for review is the right approach.

• SalesforceMappingFieldPluginBase: 222 There is a @TODO for referencing this issue. Do we want this field properties extension on just the Properties Plugin or do we want to implement this on the PluginBase? (Currently just on the Properties Plugin

  Rather then rewrite pullValue in Properties plugin, we should have a way for the plugin to return its raw value to the base in order to be munged. For example, SalesforceMappingFieldPluginBase::pullValue() could call a new method, Properties::getRawValue(), instead of inspecting the mapping definition directly.

• Properties: 104 > Should we validate User selection of the mappings or just trust the user to map correct fields?

  Doesn't the autocomplete widget provide some kind of validation?
  Hopefully we don't need anything beyond what typed_data provides.
  How does rules validate typed_data input?

• +++ b/modules/salesforce_mapping/js/autocomplete.js
  @@ -0,0 +1,171 @@
  +/**
  + * @file
  + * Forked from core's autocomplete.

  Why do we need to fork autocomplete from core?
  If possible, I'd like to not maintain any more javascript than the absolute minimum.

• After selecting a field type and adding a field, clicking the "switch to data selection" / "switch to direct entry" button adds an additional property.
• typed_data dependency needs to be on dev version.
  The current release version is missing the dependencies used by Properties.
  Fixing this may also resolve the test failure(s).
• Applying this patch causes a WSOD unless typed_data is already available and enabled.
  Need to figure out how to get this new dependency into a release without WSOD'ing all sites using this module.

I must have done my #16 diff wrong. Reuploading..

The only thing Properties->pullValue() is really overriding is the way we retrieve the field settings.
We could add a function in the Base class for getting $drupal_field_type and $drupal_field_settings and override that.

I haven't really looked into the JS file, but I agree that we should just utilize the autocomplete library that ships with core.

I do like the services typed_data gives us. can we just require this module to be enabled in a hook_install?

We could add a function in the Base class for getting $drupal_field_type and $drupal_field_settings and override that.

This is a great idea, let's do it.

hook_install is probably good enough, given the relatively low adoption of the 8.x version so far.
I'll probably want to push this into a 3.1 release though, which will hopefully help encourage people to read the release notes.
If I can throw something together quickly using plugin alter to provide alternative plugin versions based on typed_data status, then i'll do that.

Added a overridable function that retrieves the data definition for current field in the Base Class.
Added a fairly simple hook_install check.
I also added a composer.json, maybe thats why the dependency isint getting installed during the tests.

Still have to do some additional tests.

Some phpcs fixes.
Also moved composer dependencies to the root project. This seems to be the way the community handles it for now according to This Issue.

Trying to apply this patch to 3.0 is almost successful. Through an IDE it fails to add the new typed_data dependency, but ignoring that means the new functionality works... Of course however this means that composer will not apply this patch at all.

Trying to apply this patch with the latest dev version fails for multiple reasons, can you re-roll this against the latest dev?

Composer output from patch
https://pastebin.com/q9JtMiJb

Thanks for the heads up.
I re-rolled the patch against the latest 8.x-3.x version.
I also added the encrypt module as a composer dependency. In theory this should allow for all test to pass.

Lets try it!

@aaronbauman

The Autocomplete JS file is actually a copy of the Rules module's autocomplete JS file.
I actually works great and I think we can just keep it for now.

I find this field properties feature very useful. I would like to see this extended into the Related Entity Properties as well.

What do you think?

Small cleanups
* Correctly filter out entity-reference fields
* Fix input with on form.

Todos:
* Validate Input

This comment is confusing then, if we're forking from Rules:
+ * Forked from core's autocomplete.
Any reason we can't use core's autocomplete? What's the delta here?

This logic doesn't belong in MappedObject::pull :

+        if (!$value) {
+          continue;
+        }
+
+        $target = $drupal_entity;
+        $sub_paths = explode('.', $drupal_field);
+        $sub_paths = array_reverse($sub_paths);
+
+        while (\count($sub_paths) > 1) {
+          $sub_path = array_pop($sub_paths);
+          $target = $target->$sub_path;
+        }
+        $sub_path = array_pop($sub_paths);
+        $target->$sub_path = $value;
+
+        // @Todo: Should we validate the $value based on the $data_definition of the field?

This should probably be in Properties, or at least broken into a protected MappedObject method

Cursory review looks good otherwise.
I'll test locally and get back to you with any more feedback.
Thanks for the patch!

Changes,
I added a isEmpty() insanity check before grabing nested values as well as some comments.

@aaronbaumon,
That is a solid point about MappedObject::pull().
Perhaps SalesforceMappingFieldPluginInterface should have a targetEntity() function that returns what should be updated.
Base could just use the entity while Properties could override by grabbing the subfields, or even use a NestedDataTrait.

The new patch applies, thanks for that. However I now have a problem regarding the added typed_data dependency that drupal is, as ever, fighting me at every possible avenue.

Even despite having typed_data dev version installed, I routinely get errors in requirements.

Salesforce Mapping requires this module and version. Currently using Typed Data version  (Currently using Unresolved dependency Typed Data (Version 1.x-dev required))     [error]
Salesforce Push requires this module and version. Currently using Typed Data version  (Currently using Unresolved dependency Typed Data (Version 1.x-dev required))        [error]
Foo Bar requires this module and version. Currently using Typed Data version  (Currently using Unresolved dependency Typed Data (Version    [error]
1.x-dev required))

I can't uninstall the module because it's a requirement in salesforce, and it would delete all my salesforce data. If I try deleting the mappings, I still have mapped entities in the content and it's not really feasible (or desirable) to delete those. If I remove the requirement temporarily in salesforce .info I then can't re-enable it because I get constant "PHP Fatal error: Trait 'Drupal\typed_data\DataFetcherTrait' not found" Because obviously Drupal won't just enable the module first. I tried adding the version number of 1.x-dev to typed_data as a quick hack and I just got

Salesforce Mapping requires this module and version. Currently using Typed Data version 1.x-dev (Currently using Unresolved dependency Typed Data (Version 1.x-dev         [error]
required))
Salesforce Push requires this module and version. Currently using Typed Data version 1.x-dev (Currently using Unresolved dependency Typed Data (Version 1.x-dev required)) [error]
Foo Bar requires this module and version. Currently using Typed Data version 1.x-dev (Currently using Unresolved dependency Typed Data      [error]
(Version 1.x-dev required))

That error message really is quintessentially drupally.

Ok so in case anyone else finds themselves in the same situation, I fixed the above by:

Commenting out the requirement for typed_data in salesforce
Commenting out all use statements for typed_data classes in the salesforce module
Uninstalling typed_data
Reinstalling typed_data and reversing the above changes.

I'd be really interested to know what that actually achieved because I truncated all the cache tables, restarted php + apache. So not sure what was being cached incorrectly for it to worry so much about the version number.

Sorry for the tangent!

+  /**
+   * {@inheritdoc}
+   *
+   * @return array
+   *   Field config upon which this mapping depends
+   */
+  public function getDependencies(SalesforceMappingInterface $mapping) {
+    $field_config = FieldConfig::loadByName($mapping->get('drupal_entity_type'), $mapping->get('drupal_bundle'), $this->config('drupal_field_value'));
+    if (empty($field_config)) {
+      return [];
+    }
+    return [
+      'config' => [$field_config->getConfigDependencyName()],
+    ];
+  }

A mechanism like this was in place previously, but is not tenable.
Unless dependency API has changed in the past couple releases, this will delete (not just disable) an entire mapping if/when any of its mapped fields get deleted.
This is not desirable behavior.

Speaking of tangents, this update is getting close to forcing the issue of breaking out a separate "ui" module for salesforce_mapping.
Although it doesn't have to get shoe-horned into this issue, mappings have no inherent need for this front-end heavy typed_data dependency.
Splitting out a salesforce_mapping_ui module would make salesforce_mapping much more compact.

Removed Field Dependencies and rerolled #28 against the latest dev.

@aaron
Without the Field Dependencies, perhaps fields that are deleted should just not get synced.

As for the UI Module, I think having a separate UI module is not a bad idea.
I'm not sure if this typed_data logic should all go into the UI since it also include processing logic.

Changes: Updated module dependencies to the latest alpha branches.

For anyone using this patch, Composer does not read composer.json updates from patches. Therefore, to use this module, you need to have the following in your projects composer.json.

        "drupal/dynamic_entity_reference": "^2.0@alpha",
        "drupal/typed_data": "^1.0@alpha",

You may have to run composer update drupal/salesforce, drupal/typed_data, drupal/dynamic_entity_reference --with-dependencies if you already had either of these modules installed.

OK, picking this up again finally with a few changes.
Most notably, rather than replace the existing Properties plugin, I'm moving those changes into PropertiesExtended plugin.
If users want to rely on the simpler select widget for basic properties, they can do so.

A few more details:

@@ -561,7 +563,22 @@ class MappedObject extends RevisionableContentEntityBase implements MappedObject
       $drupal_field = $field->get('drupal_field_value');
 
       try {
-        $drupal_entity->set($drupal_field, $value);
+        if (!$value) {
+          continue;
+        }

This is a significant change of the current behavior. Skipping empty values means that we can never pull NULL, FALSE, or 0 (zero) values from Salesforce. I'm getting rid of this bit. After that:

+        $target = $drupal_entity;
+        $sub_paths = explode('.', $drupal_field);
+        $sub_paths = array_reverse($sub_paths);
+
+        while (\count($sub_paths) > 1) {
+          $sub_path = array_pop($sub_paths);
+          $target = $target->$sub_path;
+        }
+        $sub_path = array_pop($sub_paths);
+        $target->$sub_path = $value;
+
+        // @Todo: Should we validate the $value based on the $data_definition of the field?

Yes, we should validate the field.
Additionally, we should use typed_data.data_fetcher service, rather than re-implementing it here.
Attached patch rolls in those changes.

PS. #34 no longer applies, and interdiff is failing.

Hi,
Patch Fails for me .

When executing the Patch received error like :

$ git apply -v salesforce-field_properties-2899460-35.patch
Skipped patch 'composer.json'.
Skipped patch 'css/salesforce.css'.
Skipped patch 'modules/salesforce_mapping/composer.json'.
Skipped patch 'modules/salesforce_mapping/js/autocomplete.js'.
Skipped patch 'modules/salesforce_mapping/salesforce_mapping.info.yml'.
Skipped patch 'modules/salesforce_mapping/salesforce_mapping.install'.
Skipped patch 'modules/salesforce_mapping/salesforce_mapping.libraries.yml'.
Skipped patch 'modules/salesforce_mapping/salesforce_mapping.routing.yml'.
Skipped patch 'modules/salesforce_mapping/src/Controller/AutocompleteController.php'.
Skipped patch 'modules/salesforce_mapping/src/Entity/MappedObject.php'.
Skipped patch 'modules/salesforce_mapping/src/Plugin/SalesforceMappingField/PropertiesExtended.php'.
Skipped patch 'modules/salesforce_mapping/src/SalesforceMappingFieldPluginBase.php'.
Skipped patch 'modules/salesforce_mapping/src/SalesforceMappingFieldPluginInterface.php'.

WHY?

Perhaps you're patching against 8.x-3.0?
You need to patch against 8.x-3.x-dev

If git is skipping all the patches, you may wish to revert your repository completely and start fresh

If you're running a composer install, you can use composer-patches to manage all this for you.

Hi,

Previously I have installed 8.x-3.0 version. But As per your request, I just installed 8.x-3.x-dev version and manually applied your recent patch salesforce-field_properties-2899460-35.patch

Activated the module in Backend and While adding the Mapping fields, I can find the Properties, Extended option under Drupal Field Type.

But I Can't able to Map Geolocation field in that. Why?

Received below errors :

Drupal\typed_data\Exception\InvalidArgumentException: The parent property is not a list or a complex structure at 'field_geo_pairs.lng'. in Drupal\typed_data\DataFetcher->fetchDataBySubPaths() (line 79 of /docroot/modules/contrib/typed_data/src/DataFetcher.php).

Drupal\typed_data\Exception\InvalidArgumentException: The parent property is not a list or a complex structure at 'field_geo_pairs.lat'. in Drupal\typed_data\DataFetcher->fetchDataBySubPaths() (line 79 of /docroot/modules/contrib/typed_data/src/DataFetcher.php).

Can you please let me know How to Map Salesforce Field with Geolocation Field in that recent Patch?

Thanks.

Yes, patch #35 works to map lat/lng values for a geolocation field, (or lat_cos, lat_sin, lng_rad values).
If you can attach a yml export of your salesforce mapping, and the entity you're trying to map, i can try to help troubleshoot further.

Thanks for the reply. As per your image above, I just need to Map field_geo.lat for latitude and field_geo.lng for longitude. Have separate fields for Latitude & Longitude in Salesforce and its also have proper values.

Mapping :

I just tried to Map but its showing an Exception like below for two fields:

Exception during pull for Account.Shipping_Longitude__c 001a000001XNhXTAA1 to node.field_geo.lng with value 

Drupal\typed_data\Exception\InvalidArgumentException: The parent property is not a list or a complex structure at 'field_geo.lng'. in Drupal\typed_data\DataFetcher->fetchDataBySubPaths() (line 79 of /docroot/modules/contrib/typed_data/src/DataFetcher.php).


Exception during pull for Account.ShippingLatitude__c 001a000001XNhXTAA1 to node.field_geo.lat with value 

Drupal\typed_data\Exception\InvalidArgumentException: The parent property is not a list or a complex structure at 'field_geo.lat'. in Drupal\typed_data\DataFetcher->fetchDataBySubPaths() (line 79 of /docroot/modules/contrib/typed_data/src/DataFetcher.php).

Can you please let me know what's an issue?

Thanks for that info, but I'm still unable to recreate this issue on my end.
If you can please provide the yml configurations for your content type and salesforce mapping, that may provide some better insight.

Thanks for reply. How I can get yml configurations for Salesforce mapping & that content type? Please explain. OR Please send your email id and I will send you the admin credentials

https://www.drupal.org/docs/8/configuration-management/managing-your-sites-configuration

Thanks for the info. Here is the details :

Salesforce Mapping :

uuid: f30741eb-7f26-4b90-bda9-c782911c31ef
langcode: en
status: true
dependencies:
  module:
    - salesforce_pull
id: salesforce_with_drupal
label: 'Salesforce with Drupal'
weight: 0
type: salesforce_mapping
key: ''
async: false
push_standalone: false
pull_trigger_date: LastModifiedDate
pull_where_clause: ''
sync_triggers:
  push_create: false
  push_update: false
  push_delete: false
  pull_create: true
  pull_update: true
  pull_delete: true
salesforce_object_type: Account
drupal_entity_type: node
drupal_bundle: salesforce_mapping
field_mappings:
  -
    drupal_field_type: properties_extended
    drupal_field_value: title
    salesforce_field: Id
    direction: sync
  -
    drupal_field_type: properties_extended
    drupal_field_value: field_dealer_locator_name
    salesforce_field: Dealer_Locator_Name__c
    direction: sync
  -
    drupal_field_type: properties_extended
    drupal_field_value: field_geo.lat
    salesforce_field: ShippingLatitude__c
    direction: sync
  -
    drupal_field_type: properties_extended
    drupal_field_value: field_geo.lng
    salesforce_field: Shipping_Longitude__c
    direction: sync
push_limit: 0
push_retries: 3
push_frequency: 0
pull_frequency: 0
always_upsert: false

Content Type :

uuid: 990e2ca9-5fdd-4b1d-ad43-e3f152a2c950
langcode: en
status: true
dependencies:
  module:
    - menu_ui
third_party_settings:
  menu_ui:
    available_menus: {  }
    parent: ''
name: 'Salesforce Mapping'
type: salesforce_mapping
description: ''
help: ''
new_revision: true
preview_mode: 1
display_submitted: true

For your kind information I can able to Map other Drupal fields except Lat/Lan Values.

Kindly let me know How to Map Geolocation lat/lan values?

Hi, kindly reply on this? Why I can't able to Map Geolocation field lat/lng?

Selva, I've reproduced your issue and verified that the current patch doesn't work to pull data from Salesforce into an uninitialized field.
I'm looking into a patch but don't have anything to post just yet.

Here's an updated patch.

The biggest change here is that, when pulling from SF, we need to initialize the field item before trying to set it.
This is, unfortunately, pretty ridiculously complicated and not supported by any core or typed_data components that I could fine.

I've also removed the hard dependency on typed_data.
If typed_data is not enabled, the new PropertiesExtended plugin will not be available.

I've tested briefly to confirm that basic push and pull are working.
More eyes on this would be lovely.

Reroll against latest dev, and some code standards cleanup.

I've created a PR in github for this patch https://github.com/messageagency/sfd8/pull/3 in case it's easier to collaborate there

Thanks for the work!
I have not ran into any problems with the latest patch (#49).
The only bug I am seeing is when I press the "Switch to data selection" or "Switch to direct input mode" button, a new Properties Extended form field gets added along with the switch. Other than this, my data is syncing as expected.

Jonathan

I noticed this as well.
I think the "add field" dropdown needs to be reset after submitting the form.
Not sure the best way to achieve this - probably either straight javascript, or an ajax command.

Removing hard dependencies on typed_data, as this was causing installs / upgrades to fail.

Reposting @cwcorrigan's updates from https://github.com/messageagency/sfd8/pull/3
I think this is just about ready to go - assuming this passes tests, @jonnyeom can you re-test once more before this gets pushed?

I'd like to get this into a 3.2 release in the next few weeks.

Sounds good!
Looking to test this soon.

Will post an update this week.

Jonathan

So far so good here. Everything is syncing as expected.
I am going to let some syncs run over the weekend and see if we run into anything.

Jonathan.

Testing is successful again.
Any questions I have, I will follow up on the PR.

Jonathan

Committed.
Thanks for everyone's help on this one

Hello all,

I came to this thread while looking for a way to map address field sub fields (City, State, etc.) to specific (individual) fields in Salesforce.
After reading through the code changes introduced by this thread I saw that the "typed_data" module is required for this new "Properties, Extended" feature. I was pleasantly surprised to see this field type appear after installing this module.

I see "typed_data (optional but recommended for 3.x, required for 4.x)" is mentioned on the main project page, but I don't see any documentation around this Properties, Extended functionality. There is some really nice work here that deserves some documentation. Am I simply overlooking it? Do we need to write some? It'd be nice to have some documentation explaining what "Properties, Extended" is, and how to use the autocomplete / direct input mode features related to it. I'm happy to help if this hasn't been done yet.

Would love to get some more documentation!
There isn't much specifically about creating mappings, the various field options available, or the plugin system.

I've put some examples in salesforce_example module, but the docs on drupal.org and in README are pretty lacking.
If you can please open a new issue and link or post whatever you can put together, that would be awesome.
Thanks!

Sounds great. I've commandeered this issue: #2799411: Salesforce Suite D8 needs better documentation

Thanks @agileadam! #61 worked for me!

[EDIT] For anyone experiencing this problem specifically with Address fields: in my case, the issue turned out not to be related to "initializing", or anything with Salesforce specifically. The problem was that the Address field requires a country_code value or it will not validate, leaving the entire field blank - which feels a lot like the "initialize" problem.

If you update the field manually, the Address module sets the default country for you, allowing Salesforce pulls to work thereafter. The solution for me was to add a listener on the Salesforce pullpreSave event, in which I added the country code manually. This allowed the field to validate with the values from salesforce in the other address fields.

$node = $event->getEntity();
$node->field_address->country_code = 'US';

@AaronBauman - Using Salesforce Suite 4.0 on Drupal 8.8.5, is the issue described comment #48 still expected? I'm running into what appears to be this exact problem with "initializing the field".

I have several separate fields for an address in Salesforce, mapped via Extended Properties to an Address field in Drupal. When a node is created via pull, the address fields are not filled in. Update pulls also do not populate the fields. Until I manually enter some data into the field. After I've entered data manually, then it all starts working - subsequent pulls will update the address fields with data from Salesforce.

Any thoughts?