Displaying contact data by profile is currently inconsistent. Some profile fields display, but many do not.

The issue is the request fed by civinode_util_load_cdata(). crm_fetch_contact() is fed with just the first argument, the params to match. Because returnProperties is null, we get only a default set of contact properties returned, which doesn't contain some needed properties.

To fix this, I've completed some draft code in the function for specifying returnProperties. I've introduced a third argument to civinode_util_load_cdata(), $specify_return_properties. If set to true, the return properties are only that of the specified profile--so we're sure to have the data we need for display.

I've defaulted this to FALSE. Rob, you'll want to review this default. Maybe instead it should default to TRUE. Or, maybe (and as I say this I think this seems the most likely), we don't want a third parameter and instead the test should be, if a $pid is fed in then we fetch return the properties of that profile.

CommentFileSizeAuthor
#2 civinode-returnproperties_0.patch1.27 KBnedjo
civinode-returnproperties.patch2.44 KBnedjo

Comments

Torenware’s picture

Torenware commented

Nedjo,

I'll try out your patch. One question: are you pleased with the behavior you're getting with CiviCRM 1.6?

One of the reasons I did not actually use the $pid argument in earlier versions was a lack of consistency in how the the CiviCRM APIs handled the returnedParams type arguments. If we're getting better behavior in 1.6, it may make sense to use your new code as the default.

Thanks,
Rob

nedjo’s picture

nedjo
he/him/his
Primary language English
commented
StatusFileSize
new civinode-returnproperties_0.patch1.27 KB

I'm using this with 1.5, where it works. I haven't tested with 1.6.

Here's a revised patch that drops the $specify_return_properties and just says, if a profile id is specified, use its fields. Like the first version of the patch, this is on the 4.7 branch, because that's what I'm using and have tested on.

It may need a bit of adapting for HEAD, since there is a small difference in the versions. In HEAD the line $params = array('contact_id' => $cid); has become $params = array('id' => $cid);. I can confirm that the 4.7 version, with contact_id rather than just id, works, though intuitively I'd say the simple id seems more likely, since the id isn't a foreign key in the main contact record, and the CiviCRM naming convention uses the form contact_id only for foreign keys. Maybe they both work in a search query?

Torenware’s picture

Torenware commented

using array('contact_id' => $cid) and array('id' => $cid) should be equivalent; 'contact_id' is usually preferred. That said, there have been CiviCRM bugs where you got different behavior anyway :-)

It's probably worth using 'contact_id' consistently with both 5.1.x and 4.7.x, since IMNSHO, if 'contact_id' does not work, it's a CiviCRM issue and should get reported on OpenNGO's JIRA system as such. I can probably suss out how to fix it on the CiviCRM side.

I'm happy to hear this is working on CiviCRM 1.5. I'm totally upgraded to 1.6 at this point, but AFAIK none of the relevant APIs have changed.

Torenware’s picture

Torenware commented
Status: Needs review » Fixed

Patch works fine on 4.7 AFAICT, and is a definite improvement on previous behavior.

So I'm landing the patch. I'm also going to give nedjo direct access to the tree to make it easier for him to get his patches in.

Rob

Torenware’s picture

Torenware commented

Merged nedjo's changes and tested on HEAD and DRUPAL_5.

Nedjo's patch is now completely in. Thanks!

Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)