Correct me if I'm wrong, but I am under the assumption that both at least of the push/pull modules need to be enabled, but not both. In my case, we are only going to be using the pull functionality, so we didn't enable the push module.

Even though the push module is not enabled, the add mapping admin page still has all the options for it. I haven't tested it, but selecting a push option (such as Drupal to SF direction for a mapped field) will probably result in an error.

What's worse is that since we aren't planning on push, we haven't enabled our SF objects to be "updateable" which is what the Salesforce object selectbox expects, so our desired objects weren't even showing up!

CommentFileSizeAuthor
#8 1980688-8.patch8.03 KBmpotter
#6 1980688-6.patch8.01 KBpianomansam
#5 1980688-3.patch8.01 KBpianomansam
#3 1980688-2.patch2.26 KBpianomansam
#1 1980688.patch2.35 KBpianomansam

Comments

pianomansam’s picture

pianomansam commented
Status: Needs review » Active
StatusFileSize
new 1980688.patch2.35 KB

Attached is a patch to beta4 that does three things to alleviate this issue.

  1. Only filter on "updateable" SF objects if push module is enabled.
  2. Only show "Drupal to SF" and "Sync" direction options if push module is enabled.
  3. Only show "SF to Drupal" direction option if pull module is enabled.
pianomansam’s picture

pianomansam commented
Title: Add mapping admin page assumes both push and pull modules are enabled » "Add salesforce mapping" admin page assumes both push and pull modules are enabled
Status: Active » Needs review
pianomansam’s picture

pianomansam commented
Status: Active » Needs review
StatusFileSize
new 1980688-2.patch2.26 KB

Rerolled patch as I apparently patched it the first time against beta3 not beta4.

kostajh’s picture

kostajh commented
Version: 7.x-3.0-beta4 » 7.x-3.x-dev
Status: Needs review » Needs work

Hi @pianomansam, best practice is to develop against 7.x-3.x-dev. Can you please re-roll and submit the patch again?

pianomansam’s picture

pianomansam commented
Title: "Add salesforce mapping" admin page assumes both push and pull modules are enabled » Mapping module assumes both Push and Pull modules are enabled
Status: Needs work » Needs review
StatusFileSize
new 1980688-3.patch8.01 KB

Thanks for the direction, kostajh. I've checkout the dev branch, worked on my patch a bit more, and re-rolled it. Here's a summary of my work:

  1. When Mapping is first installed, a notice is shown to remind that Push or Pull needs to be enabled.
  2. Access is denied (salesforce_mappings_access) when Push or Pull is not enabled. This makes the admin table page, mapping add page, and mapping import page inaccessible.
  3. When users attempt to visit admin/structure/salesforce and a sync method isn't enabled, an error is shown instructing them to enable one.
  4. SF objects are no longer filtered on whether they are "updateable", the original cause of this bug/patch. This may mean we need to validate somewhere else that data being pushed to SF fields are to fields that are writable.
  5. Field map direction now only lists options available based on which sync module(s) are enabled. For Pull, SF to Drupal is enabled. For Push, Drupal to SF and Sync are enabled. As this is a required field, the default is now set to the last option available.
  6. Action triggers also only list available options. For Pull, these are the Salesforce triggers, and for Push these are the Drupal triggers.
pianomansam’s picture

pianomansam commented
Title: Mapping module assumes both Push and Pull modules are enabled » Better UI when only one mapping direction enabled
Issue summary: View changes
StatusFileSize
new 1980688-6.patch8.01 KB

Previous patch no longer applied cleanly to 7.x-3.x-dev. Here's a new patch. Also better issue title.

mpotter’s picture

mpotter commented
Status: Needs review » Reviewed & tested by the community

This works well for me and fixed a big issue with Salesforce objects not showing up in the list.

mpotter’s picture

mpotter commented
StatusFileSize
new 1980688-8.patch8.03 KB

Actually, one minor change. There was a warning with PHP 5.6:

Strict warning: Only variables should be passed by reference in _salesforce_mapping_get_default_value() (line 902 of /var/www/docroot/sites/all/modules/contrib/salesforce/modules/salesforce_mapping/includes/salesforce_mapping.admin.inc).

so here is an updated patch to fix that line in the patch from #6.

  • aaronbauman committed bddbefb on 7.x-3.x authored by mpotter 
    Issue #1980688 by pianomansam, mpotter: Better UI when only one mapping...
aaronbauman’s picture

aaronbauman
he/him
commented
Status: Reviewed & tested by the community » Fixed

committed

Status: Fixed » Closed (fixed)

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