COmanage Developers List

[Benn Oshrin <>]

On 11/14/11 9:27 PM, Marie Huynh wrote:

> M       trunk/app/models/co_person.php

Minor nit: Capitalize 'retrieves' at the start of the comment.

The logic in your foreach loop looks very similar to 
libs/util.php:generateCn(). Perhaps you should update and use that function?

> M       trunk/app/config/schema/schema.xml

Why does sponsor_co_person_id have a NOTNULL constraint?

Why is there a second index created on co_person_id?

> M       trunk/app/controllers/co_person_roles_controller.php

My guess is for larger COs we don't want to populate $sponsors if 
sponsors aren't in use, since it could be a big chunk of data. We don't 
necessarily need to do anything with this now, but we should at least 
add a low priority JIRA for a future release so we don't forget.

Why is $sponsors set in both co_person_roles_controller and 
co_people_controller?

> A       trunk/app/models/demographic.php

Note your new files ('A') aren't included in the diff.

> M       trunk/app/libs/lang.php

fd.sponsor should just be 'Sponsor'. If additional descriptive text is 
needed, it should be added as a second entry (eg: 'fd.sponsor.desc').

> M       trunk/app/libs/enum.php

I realize this is how the standard defines things, but it bothers me 
that we have a 'RaceEnum'. I'd rather it be called 'EthnicityEnum', but 
that apparently means something else. (This also goes for lang.php.) Can 
we change the names in the code and just use the terms 'Ethnicity' and 
'Race' in the actual display (ie: translated) text?

Something like

 class HispanicEnum
 {
   const Hispanic = "H";
   const NotHispanic = "H";   <-- note not NotHis
 }

 class EthnicityEnum
 {
   Asian
   AmericanIndian    <-- any reason not to spell these out?
   Black
   NativeHawaiian
   White
 }

 // These might even only be defined this way in the LIGO installation
 fd.de.hispanic => "Ethinicity"
 fd.de.ethnicity => "Race"

> M       trunk/app/controllers/co_person_roles/fields.inc

Sanitize::html($org_identities[0]['OrgIdentity']['sponsor_co_person_id']

This is just going to output a number (literally the sponsor's person 
ID). It should instead output the person's name (in both places). I'm 
sure there's an example of this somewhere, probably in 
co_people/fields.inc, where it also needs to be updated.

-Benn-