Posted by Bevan on July 31, 2009 at 5:41am
| Project: | Salesforce Suite |
| Version: | 6.x-2.x-dev |
| Component: | Code |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
Issue Summary
An admin might want to specify that a drupal object type be synced on create, but not update, both, or not at all. Change the sync automatically checkbox to a set of three radios to handle these cases, and add/update sf_node_nodeapi() and sf_user_user() respectively.
Comments
#1
#2
I think two checkboxes might be more flexible over the long run than three radio buttons. Is there a reason to use radio buttons over multiple checkboxes?
#3
Two checkboxes provides 4 possible combinations, one of which is invalid/undesirable. 3 radio buttons provides less options and is easier to read/understand.
#4
Patch with radio buttons attached.
This builds upon "fix.automatic.patch" from #507638: Node exported on node-add even if automatic export is disabled, and "salesforce_api-fieldmap-checkbox.diff" from #536292: Automatic syncing can not be changed after fieldmap-creation.
I'm a little wary of doing an extra database call on node-update, for performance / efficiency. Since it's followed by a salesforce-synchronization, the db-check is probably not too significant. However, it appeared to be the most clean solution.
#5
Does that mean it requires those patches to be applied first? Includes those patches? Something else?
#6
It requires those patches to be applied first.
If it is desired, I could create the patch where it integrates those other two, and just have one bigger patch. That might take a little time, though. Just let me know what is preferred.
#7
Bevan:
Why is "sync on update only" invalid / undesirable?
I can imagine a situation in which a site should sync only nodes which have been imported manually.
I can also imagine a situation in which a site should sync only on insert.
I agree with bibeksahu that this should be 2 checkboxes or at least 4 radio buttons.
Just to clarify, here are the 4 possible states:
#8
> I can imagine a situation in which a site should sync only nodes which have been imported manually.
This seems very much an edge case to me. Most of the time they will need a manual update-sync callback if they have a manual insert-sync function. And if not, since the user will need to write custom code anyway, they can just disable sync on create in code.
If there are valid AND common reasons to have all four settings this is fine, but I can't see any yet.
#9
OK, point taken - it would make more sense if there were a manual insert function, e.g. on node/%/salesforce or user/%/salesforce. I have a patch on my work machine that I'll upload tomorrow.
#10
I believe that feature already exists, doesn't it? If not, it's a usability bug, and the patch would be welcome.
#11
Oh right, it does already exist -- I maintain that all 4 options should be available.
I don't see how "synch on update only" is any more an edge case than "synch on insert only".
Consider these highly plausible hypotheticals for "synch on update only":
-- A site membership-based site offers limited access to free accounts and extra privileges for approved accounts. Administrators wish only to track member accounts in Salesforce. After administrators manually approve a user's membership, only then should the record be automatically synched.
-- An admin for a highly trafficked community site with open registration wishes to track a select group of users in salesforce -- say "high value users". When the site admin makes a subjective assessment that the user is of high value, the admin exports the user manually. Subsequent updates are synched to salesforce.
Further, even if "synch on update only" were an edge case, the software should not artificially inhibit functionality because the developers didn't think it would be useful, or thought it would be too confusing. Any administrator savvy enough to configure a salesforce synchronization should also be savvy enough to distinguish between 4 synch options rather than 3.
Finally, given a large enough user base for salesforce module, an "edge case" may apply to hundreds or thousands of users (wishful thinking perhaps, but still...)
To this end, this patch builds on bibeksahu's efforts by offering all 4 options with informative #description properties for each.
This patch also switches the values for synch mode to constants, rather than inline integers.
Requires the following patches:
#507638: Node exported on node-add even if automatic export is disabled
#536292: Automatic syncing can not be changed after fieldmap-creation
#12
It's also possible that an administrator might want one fieldmap for create, but another fieldmap for update. E.g., if I want some information to be added upon user creation, but only a subset of that information to be automatically updated.
#13
From #11;
A "synch on update only" setting would break in both these cases because when user accounts are edited that are 1) not approved, or 2) not in the special group, they will get exported to Salesforce – which would leave a data set in Salesforce that is inconsistent with the models from your examples. To create and maintain these data models consistently custom code is required in both cases.
I still don't agree that the fourth setting makes enough sense to go into the UI. Having it would be a (minor) usability bug in my opinion. Perhaps it makes sense for there to be a hidden setting or configuration for this that is not available through the UI but is available for custom modules and drupal code developers?
Either way this isn't a significant enough issue to stop this patch from going in. Patch still needs review.
#12; What would be a use-case for wanting only to synchronise a subset of the data?
#14
#13: Anything using a "Master-Detail" relationship on Salesforce would need to synchronize only a subset of the data after the initial setting.
The "Master-Detail" field is required when a Salesforce object (the detail object) is initially created, but read-only afterwards. Even setting it to its existing value causes an error.
One way or another, we need a way to set it upon creation, and not try to set it later on. Perhaps if "_export" handlers could return FALSE (or "SALESFORCE_API_DO_NOT_SET_VALUE", or something) so that the value is not set into the SF object; that could work, too. Either way, something is needed.
#15
2 things:
1. This also needs to be implemented for sf_user
2. These 2 lines, around line 85:
$salesforce = salesforce_api_id_load('node', $node->nid);if ($salesforce['fieldmap'] && $salesforce['sfid']) {
prevent existing objects from being exported if they have not already been exported.
If "export on update" or "export on update and insert" is set to true,
then the drupal object should always be exported on
update.Otherwise, the site admin will have to manually export all pre-existing objects.
#16
Does this patch handle the following scenario:
1. A sync is setup between an object in Drupal which stores company names -> Accounts in Salesforce.
2. A sync is setup between a user in Drupal and a Contact in Salesforce. The Contact in Salesforce has a lookup to the Account in Salesforce already - this is standard relationship.
3. A new user is added in Drupal, and this user is assigned to a new company in Drupal.
4. Now, the new user and new company need to be synced to Salesforce, but since the Contact has a lookup field in Salesforce to the Account, it is not possible to upsert the Contact first, as the Account will not yet exist.
If it was possible to sync only when update (as suggested above) then the user could be created first, followed by the company, and then an update made to the user to reference the company they work for. The update to the user could then cause the sync to occur because the sync will be done on update. My assumption is that the user being synced would cause the account to be synced also, since there is a relationship between them.
Also, perahsp that needs to be a way to configure the sync and the order of the sync events, so that Account updates are always done before Users/Contacts. The same feature would be useful for other objects which have parent/child relationships.
#17
Tim:
Step 3: how does this work? userreference? nodereference?
Either way, the company will have to exist in Drupal before any user can be assigned to it.
#18
I am not familiar with userreference or nodereference, but I am very familiar with how Salesforce works.
My thinking is that if objects are being synced between Salesforce and Drupal, then the relationship between these objects cannot be ignored and the sync module needs to be configurable to sync when objects are not being updated/created in the right order. I think the changes made by this patch will help, but perhaps more is required to make it possible to sync objects where a relationship is involved with another object. I used the example of Account/Contact earlier, but the same would apply for a more complex relationship, e.g. A Case has Case Comments associated with it, and a Case is associated with a Contact, and a Contact is associated with an Account. The sync feature in this module would therefore need to be configurable so that it does not assume that the Account/Contact/Case exist in Salesforce when a Case Comment is added or updated in Drupal. it would have to upsert the Account first, then the Contact, then the Case, and lastly the Comment woud be upserted. I feel that this requirement is going to be common, so needs to be considered. Some users of the module might be happy to assume that the Salesforce Account and Contact and Case exist when a Case Comment is upserted, but others might not - the module needs to be able to satisfy both use cases.
#19
Off the shelf, the only way to trigger the race condition you describe would be to create a two fieldmaps that use the same Drupal node type but map to 2 parent-child Salesforce objects.
For example, a Drupal user fieldmap that relates to Contact and another that relates to Account.
This is not a feasible use case.
#20
Yes, multiple fieldmaps would be required. The bit that is missing is a way to configure the order in which the fieldmaps are processed by the module. Let me give you a better example... If a Drupal user is created and then then later associated with a Drupal company via a user update, the sync should not be done when the User is created, but should be done when the user is updated. In the same Drupal environment, somebody might add a user to a Company and expect this user to be synced with Salesforce, but it won't becuase the configuration is made to only sync on update of user for the reason given in my first example.
So, how do you propose the above use case is satisfied ?
#21
In the Salesforce setups I've used, if a Contact is created without being associated to an Account, they're either dumped in a default bucket (a special account called "Individual") or they're associated with a special placeholder / self-referential Account that simply links the Contact back to itself. This would match the Drupal workflow where a Drupal user is created without being associated with a Drupal company.
But, in fact, it doesn't matter how your Salesforce instance is set up, because it doesn't matter whether the initial upsert operation succeeds. If, later, the Drupal user gets assigned an AccountId of a legitimate company, the user will get upserted at that time. If the upsert failed previously because AccountId was blank, it will now succeed.
Again, Drupal is completely ignorant of Salesforce schema, including validation rules. If your Salesforce instance will not allow you to create a Contact without an AccountId, then you'll have to tap info hook_nodeapi or hook_user and use "sf_node_skip_export" or "sf_user_skip_export" to prevent an upsert.
#22
This patch builds on #11 and:
Requires the following patches:
#507638: Node exported on node-add even if automatic export is disabled
#536292: Automatic syncing can not be changed after fieldmap-creation
#23
#24
Yes, it is true that in a standard salesforce setup the Contact can be created without an account asociation, but I thought this module was supposed to be for any drupal object and any salesforce object (standard and custom). I am merely suggesting that there are cases where more than a simple 1:1 sync is required, and I can see cases where the relationship between objects would need to be considered. I will do more design work around this area and from this, it will be clearer whether I can meet my needs with the standard salesforce module as it is today, or if I need to write add-on module, or propose changes to standard module. Thankyou for your help so far. The patch described in this issue is a useful enhancement.
#25
Hello,
I'm sorry if this is already a known issue or was already noted... I just wanted to point out that it appears that the 3 patches listed in #22 and #23 need to be applied only after the SF modules are installed and enabled. If patching the code before install/enable, I receive the following warning (upon install/enable):
user warning: Invalid default value for 'automatic' query: CREATE TABLE salesforce_field_map ( `fieldmap` INT unsigned NOT NULL auto_increment, `drupal` VARCHAR(32) NOT NULL DEFAULT '', `salesforce` VARCHAR(32) NOT NULL DEFAULT '', `action` VARCHAR(32) NOT NULL DEFAULT '', `automatic` TINYINT unsigned NOT NULL DEFAULT 'SALESFORCE_AUTO_SYNC_OFF', `fields` TEXT NOT NULL, PRIMARY KEY (fieldmap) ) /*!40100 DEFAULT CHARACTER SET UTF8 */ in /home/lumity_dev/dev1.lumitycms.org/includes/database.inc on line 529.
If I install/enable the un-patched SF modules first and then apply the patches, all seems to work correctly.
Also, one other comment. We are hoping to automatically track a user's "last login" timestamp within salesforce (in order to sort out unverified users, etc.). Through there is a field mapping for this, it is only automatically synced when a user or admin manually updates a user's details. Would it make sense for a sync to also be triggered each time a user logs in (assuming sync on update is selected)... as the login process can change one or more mappable fields?
Cheers,
Ryan
#26
Ryan:
First, I guess I never realized that the .module file is not included when hook_schema gets called. Seems bass ackwards to me, but this patch addresses the error you report by require_once'ing the salesforce_api.module.
Second, this patch fixes the issue that user synch does not happen automatically ever. The issue you raise about "last login" is a valid one and should be submitted separately.
#27
Thanks for the info Aaron!
Did you mean to attach a file (new patch version) on your last message? Sorry, I'm not sure if that was your intent?
Then regarding the "sync/update on login" idea, it is accurate to say that the sync/update triggers (hooks?) that would call for an update (ie, sf_user_export()) should not be defined as part of this patch? It sounds to me like sf_user_export() needs to be called as part of the login process (via the right hook) if "sync on update" is selected. I guess I'm just curious as to what context that would it best be addressed? I'm happy to open a separate issue for this, I'm just not sure how to "package" it, or if I should bring it up as part of another existing issue.
As I'm sure you can see, my php knowledge and understanding of the Drupal module system is a little "nascent", I just hope I can help out by providing good feedback.
Cheers,
Ryan
#28
It looks like all of this, and more, could be handled by making a fairly generic "salesforce_fieldmap_export_action", that ties in with Drupal's built-in Actions system (which is still in use in Drupal 7, from what I see), and uses the Trigger module (part of Drupal core) to provide a UI to handle it.
Is there any interest in this?
#29
See issue #707254: Salesforce Actions (proposed module) for another way to handle this. The Actions modules does not require any patches to salesforce_api, and could more easily be extended to support import as well.
#30
Ryan,
Attached is the patch I mentioned in #26... having trouble keeping everything straight.
#31
Aaron - thanks for the attachment! I'll give it a try by applying it before installing the module (I'm going to do a fresh install test shortly).
Bibek - sounds like a great solution in terms of implementing the sync via various Drupal triggers/actions. I'm only just learning the action/trigger system, but I think it's potentially an ideal solution as it means that people can selectively choose when they want auto syncs to run beyond the defaults, and those that are happy with the defaults won't have the extra overhead of more syncs taking place. Good stuff. I'll refer any comments on that topic over to the thread that you started.
Thanks!
Ryan
#32
This is in
http://drupal.org/cvs?commit=372724
#33
Automatically closed -- issue fixed for 2 weeks with no activity.