MinnPost / object-sync-for-salesforce

WordPress plugin that maps and syncs data between Salesforce objects and WordPress objects.
https://wordpress.org/plugins/object-sync-for-salesforce/
GNU General Public License v2.0
94 stars 51 forks source link

if( empty($params) ) in salesforce_push_sync_rest fails silently #362

Closed OfficeBureau closed 3 years ago

OfficeBureau commented 4 years ago

We discovered this weekend that when map_params runs in salesforce_push_sync_rest, if all params get unset (in our case because our systems integrator didn't make any of the fields we need to update updateable, which is a whole other conversation), the object sync will fail silently. This includes when all levels of logging are enabled.

It's an edge case to be sure, but one of our devs literally had to go line by line through the salesforce_push_sync_rest class adding error_logging to figure out where things were failing. Would be good to get some sort of notice that no updateable fields are available, either in error logs, or when setting initial field maps (can non-updateable fields be disabled in the field mappings dropdown?)

Also, we agree that it should be mentioned often that salesforce spelling it "updateable" is bad.

jonathanstegall commented 4 years ago

I'd like to make sure I understand your request. Here's what I think you're suggesting:

  1. When a data push has no updateable values, we should create a log entry to that effect.
  2. When setting field maps, either show that none of the fields are updateable, and/or provide a way to only show fields that are updateable when mapping fields.

Am I right on that?

If so, here are my initial thoughts.

  1. I don't have any objection to such a log entry. I guess it would be an error log?
  2. This one is a little trickier because many users will pull fields from Salesforce and save them in WordPress even though they cannot be updated in Salesforce. I don't think I'd be inclined to remove those fields, even with a setting. But maybe there's some kind of icon or label or something else that we could use to show whether or not a field can be updated. This way we could show that indicator whether the field was being pushed or pulled.
OfficeBureau commented 4 years ago

Absolutely right on both fronts. An error message would suffice, and an indication in mapping that a field is unupdateable would be very helpful.

An icon's a smart idea. Perhaps a combination of left pointing arrow, right pointing arrow or both? Or up/down/both? I guess the question then becomes which direction is which. Also, I don't think there's any WordPress field equivalent to not updateable.

Might be simpler to just indicate that a field is locked on salesforce (which for all intents and purposes 'updateable' = false is.)

An asterisk makes it clear a field is required, so a lock or an x could communicate it can't be updated?

jonathanstegall commented 4 years ago

I'll look into that. I had previously been working on a somewhat large interface update, especially to the fieldmap screen, when I got pulled into other things. I'm hoping to get back to that soon and this would be a good thing to add at that time.

jonathanstegall commented 3 years ago

I'm trying to make a new release (specifically limited to upgrading ActionScheduler, renaming files to match the WordPress code standards, and other code standard fixes). I'm looking at this issue and I can't remember why I left it open. How is this working for you since it was merged last year, @OfficeBureau? Does it still need further work (probably not in this upcoming release, but in general)?

jonathanstegall commented 3 years ago

Reading it over again, I realize there isn't a log entry for an edge case where there is a push event that occurs when no fields are updateable. I think that's still worth adding as a debug message.