There should be configuration options to easily change your SFDC instance url eg: https://*.salesforce.com. Often times organizations have a few sandbox/development SFDC environments as well as their production environments.

These changes are in the patch.

Comments

epicflux’s picture

I also needed to connect a sandbox account. Different from the above patch I setup the field as a checkbox to flag if the account is a sandbox account. It might make more sense as a textfield if there are development URLs for salesforce other than https://test.salesforce.com.

rickyoh’s picture

I'm thinking this is more of an issue with the Salesforce apis and dev urls, but the returned urls you get from api calls from dev sandboxes don't always work, however the specified environment url would always work. Eg: the sfdc api returns https://cs*-api.salesforce.com, which fails on subsequent api calls while cs*.salesforce.com works. I think the only place this module uses that is in Salesforce::setIdentity

I haven't noticed any api URL wackiness on production environments when using https://login.salesforce.com,

haza’s picture

Status: Active » Needs review
StatusFileSize
new3.25 KB

@rickyoh : you have some coding standard fails in your patch.<
Plus :

+++ b/includes/salesforce.incundefined
@@ -233,7 +234,10 @@ class Salesforce {
-    $response = $this->httpRequest($id, NULL, $headers);
+   ¶
+    $data = parse_url($id);
+    $url = $this->login_url.$data['path'];

I'm not sure to understand that point.

@epicflux : Shouldn't we also change the salesforce_get_api() function ?

I'm also had a look on this issue and also make a patch for that. I'll drop it here. Let's discuss what's the better approach for that !

kostajh’s picture

Title: Add in ability to change the Salesforce instance url. » Support multiple Salesforce environments
Priority: Normal » Major
Status: Needs review » Needs work

Thanks for creating this issue and for the work on the patch.

A few thoughts on this:

  1. Documentation added to the README and hook_help() explaining this feature
  2. Support for multiple SF environments. Some organizations have multiple sandboxes. Probably the way to do it would be to create a Salesforce Environment entity, so that there could be be a CRUD interface for SF environments, along with an ability to set a default.
  3. Ensure that Salesforce Mapping Object entities are associated with a particular environment. For example, the link between user Foo and SFID Bar should be valid only for environment Baz. This way, when switching between environments, you won't run into invalid key reference errors with Salesforce. Each Salesforce Mapping Object entity would have to reference a valid Salesforce Environment entity.

I would like to see all of this done before this functionality is committed. The current scope of the patch, while helpful, will cause a lot of confusion and problems for many users who think that they will be able to easily switch back and forth between Sandbox and Production.

What do you all think?

levelos’s picture

@kostajh, I think that's an ambitious set of goals and it would add a lot complexity to both the code base and interface. In particular, I wonder about ramifications of having a single Drupal entity mapped to multiple SF instances, E.g., it could lead to inadvertent data corruption by sandbox data making it's way into production via the Drupal entity.

IMO, It seems reasonable to force a Drupal instance to a use a given Salesforce instance. Sandbox would be used when developing or testing sites, switching to production when a site's deployed.

kostajh’s picture

I agree; it would be challenging to to implement well so as to avoid the problems you're describing.

With the patch as it stands, here is the problem I see users running into:

  1. Developer clones production site to staging site to test new SF sync code
  2. Developer switches the staging site to use the sandbox using supplied patch
  3. Any test of Salesforce Pull / Salesforce Push will break as the Salesforce Mapping Object entities no longer contain valid SF key references.
  4. If the SF schema has changed in the sandbox (common scenario), then the Salesforce Mapping entities will be broken as well.
stella’s picture

@kostajh I'm not as familiar with SF as you are. Could you elaborate on points 3 and 4 in your last comment for me? Is this only an issue if MyCustomField__c on the production SF doesn't match a field in the sandbox SF? Or is it something else.

I had envisaged that the developer would do steps 1 and 2 above, but may also update the salesforce_instance_url and salesforce_refresh_token if necessary - either by re-authorizing it or having the already known values set in settings.php.

kostajh’s picture

Hi @stella,

Any test of Salesforce Pull / Salesforce Push will break as the Salesforce Mapping Object entities no longer contain valid SF key references.

When you create a link between a Drupal entity and a Salesforce object, that relationship is stored in a Salesforce Mapping Object entity. So in production, you might have a mapping between node/15 and Salesforce ID (SFID) a1X30000000HkonEAC. But once you use a Salesforce sandbox, the SFIDs change - the same Salesforce object might have an ID of b3y40000000HkonDVS. When you go to test out syncing with the Drupal staging and Salesforce sandbox environments, you'll get errors that the Salesforce record ID is invalid.

What's needed is a way to specify multiple Salesforce IDs for a given Salesforce Object Mapping and link these to different Salesforce environments (sandbox1, production, sandbox2, etc), or, as a simpler alternative, you could delete all of your Salesforce Mapping Object entities in the staging environment. But we'd need to expose that functionality to developers via the admin page or drush, because at the moment you have to run a SQL query to do that.

If the SF schema has changed in the sandbox (common scenario), then the Salesforce Mapping entities will be broken as well.

If in the Salesforce sandbox you've changed the type of a field, or adjusted the field name, or removed fields, then syncing between Drupal and Salesforce will fail.

Probably the best way to handle that situation would be, after switching Salesforce environments in the admin page, running an automated review of the fieldmaps and letting the admin know which fields in the field mapping have changed and need to be adjusted.

epicflux’s picture

I realize the thread has gone on to discuss higher level issues then my original patch addressed, but for folks that may need it I re-rolled that patch.

kostajh’s picture

Status: Needs work » Needs review
StatusFileSize
new2.54 KB

Ok, my concerns with this aside, it is a useful patch and could be committed, with follow-up tasks to deal with some of the other issues mentioned here. Thanks to all who have worked on it.

I reworked the patch to add a few words of warning for those who will be using this configuration setting.

tsphethean’s picture

Status: Needs review » Needs work

Doesn't this patch have the downside that we're still hardcoding (twice) the endpoint of the Salesforce environment? Not all sandboxes end up at test.salesforce.com, so rather than having a boolean "I'm using a test environment", why not a variable which simply defines the URL of the Salesforce environment to login to?

tsphethean’s picture

Status: Needs work » Needs review
StatusFileSize
new2.45 KB

Here is a tweak to the patch to reflect my comment in #11 - I had to do this to get the module to work with a cs18.salesforce.com sandbox.

In terms of a concern about switching from test to production, the dev would have to be cloning the sandbox to production in any case, in which case the links would be maintained, but the workflow I see more likely is both Drupal and Salesforce dev sites being built side by side with test data, using the endpoint configured to point to the test sandbox, and then when "finished", the configuration for both would be pushed to live which would then use a production Salesforce instance and have different objects in any case.

EvanDonovan’s picture

I think a simple patch that addresses the issue of the endpoint with a global variable - "test.salesforce.com" vs. "login.salesforce.com", etc. would meet the needs of most people without adding substantial complexity to the module. That is how I am reading #11 as well.

In light of that, I think that #12 is the direction to go, although I have not tested the patch yet.

nonsie’s picture

There's also the case of multiple production and corresponding dev instances but that would require a major rewrite to accomplish it:(

fabianx’s picture

Status: Needs review » Needs work

There is a bug in the #12 patch, providing a new patch now.

fabianx’s picture

Status: Needs work » Needs review
StatusFileSize
new2.44 KB

I would RTBC this as it works great for my purposes, but I had to fix #12, which had a typo in it.

So lets call this a:

RTBC + 1

Someone else will need to do the actual RTBC.

Interdiff:

diff --git a/sites/all/modules/contrib/salesforce/salesforce.module b/sites/all/modules/contrib/salesforce/salesforce.module
index cd0e6eb..406f5e3 100644
--- a/sites/all/modules/contrib/salesforce/salesforce.module
+++ b/sites/all/modules/contrib/salesforce/salesforce.module
@@ -230,7 +230,7 @@ function salesforce_oauth_form() {
 function salesforce_oauth_form_submit($form, &$form_state) {
   $consumer_key = $form_state['values']['salesforce_consumer_key'];
   $consumer_secret = $form_state['values']['salesforce_consumer_secret'];
-  $salesforce_endpoint = $form_state['values']['salesforce_sandbox_account'];
+  $salesforce_endpoint = $form_state['values']['salesforce_endpoint'];
   variable_set('salesforce_consumer_key', $consumer_key);
   variable_set('salesforce_consumer_secret', $consumer_secret);
   variable_set('salesforce_endpoint', $salesforce_endpoint);

The interdiff is trivial though. I am using this in combination with Pantheons environments and it works like a charm!

tsphethean’s picture

Status: Needs review » Reviewed & tested by the community

@Fabianx - Thanks for catching my typo

Not sure if I should really RTBC a patch to my own initial patch but your fix works on my sandbox so here goes...

kostajh’s picture

Status: Reviewed & tested by the community » Fixed

This is committed in 2f2d18e8912d51df. I made a few small changes to the wording under "Advanced". Thanks to everyone who contributed to this issue!

Status: Fixed » Closed (fixed)
Issue tags: +

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

Issue tags: +
  • Commit 2f2d18e on 7.x-3.x, process-deleted-refactor authored by Fabianx, committed by kostajh:
    Issue #1934790 by Fabianx, tsphethean, kostajh, epicflux, Haza, rickyoh...

Issue tags: +
  • Commit 2f2d18e on 7.x-3.x, mapped-object-ui authored by Fabianx, committed by kostajh:
    Issue #1934790 by Fabianx, tsphethean, kostajh, epicflux, Haza, rickyoh...