Skip to content
This repository has been archived by the owner on Jun 1, 2021. It is now read-only.

Bug with fieldList method dealing with custom fields. #12

Open
RickKukiela opened this issue Apr 25, 2018 · 4 comments
Open

Bug with fieldList method dealing with custom fields. #12

RickKukiela opened this issue Apr 25, 2018 · 4 comments

Comments

@RickKukiela
Copy link

I'm not sure if this is a problem because of the way the API returns objects when there is only one instance of said object type but I ran into this and I only have 1 custom field defined. I did not try the existing code with multiple fields but I did come up with a fix / workaround for this.

I really dont have time to do a PR and I'm also not sure if this is the best way to fix this however in CRM.php at Line 162 you have the following code:

if ($resource == 'activityType') { //yet another API inconsistency to deal with
	$list = array_merge($list->user, $list->system);
}

$result = array_column($list, $detailed ? null : 'name', 'id');

if ($detailed && $resource == 'customFieldDefinition') {
	array_walk($result, function (&$field) {
		if (isset($field->options) && $field->options) {
			$field->options = array_column($field->options, 'name', 'id');
		}
	});
}

return $result;

The problem here is that in my particular instance the $list variable was of type ProsperWorks\Resrouces\BareResource and not Array as expected by the call to array_column within this block.

I was able to fix / work around this by changing the above block to this:

if ($resource == 'activityType') { //yet another API inconsistency to deal with
	$list = array_merge($list->user, $list->system);
}


// START NEW
if ($resource == 'customFieldDefinition') {
	if($detailed) {
		$result = [$list->id => $list];
	} else {
		$result = [$list->id => $list->name];
	}
} else {
	$result = array_column($list, $detailed ? null : 'name', 'id');
}
// END NEW


if ($detailed && $resource == 'customFieldDefinition') {
	array_walk($result, function (&$field) {
		if (isset($field->options) && $field->options) {
			$field->options = array_column($field->options, 'name', 'id');
		}
	});
}

return $result;

What do you think?

@RickKukiela
Copy link
Author

I feel like this will break if another field is added though...

@RickKukiela
Copy link
Author

Changing my if statement to this will probably prevent it from breaking, and allow it to run the existing code if a valid array type is returned from the api:

if ($resource == 'customFieldDefinition' && is_object($list)) {

@igorsantos07
Copy link
Contributor

igorsantos07 commented Apr 25, 2018

Disclaimer: I'm not working on S&C anymore (for quite some time already), nor maintaining this project.

But here's my opinion :)

  1. Please, take your time to fork and write a PR :) In the end the fork can be used in your own composer file and will help keep your own project organized
  2. Not sure if that's the best fix. We must understand first why in this specific case it's returning the wrong thing. Is that an API change from them, or some hidden bug on the library?
  3. The part where you create a manual array seems weird, as it has a single item. It would make more sense if you looped the $list and created an array from it, right? You may also try to cast the object into array and see if that passes correctly through the existing array_column() call.
  4. Finally, your check from the last comment seems correct, but if the issue is actually on having an object instead of an array... Why not skipping the $resource check? Right?

@RickKukiela
Copy link
Author

Thanks for getting back to me. I had a feeling you might not be working on this anymore. I'll probably have to do a fork since modifying files in vendor is really poor form and risky.

I guess you're right I'm manually doing the work array_column() is doing essentially, I should be able to cast it accordingly and pass it to the existing array_column() call.

I'll implement something better and you're right about skipping the resource check. I have to add another custom field today so I'll see if the array is returned from the api now.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants