COmanage Developers List

[Benn Oshrin <>]

On 11/16/11 8:57 PM, Marie Huynh wrote:

> > > M       trunk/app/models/co_person.php
> > The logic in your foreach loop looks very similar to
> > libs/util.php:generateCn(). Perhaps you should update and use that function?
> Why was the honorific field ignored in generateCn?

For general display purposes, "Pat Smith" seems more appropriate than 
"Ms. Pat Smith". I suppose the counter argument is that if honorific is 
supplied we may as well render it, but it's probably a bit of a 
distraction in lists of people.

In sponsorList(), why do you need to explicitly join to get Names? 
Doesn't Cake automagically pull them as associated data?

> > > M       trunk/app/config/schema/schema.xml
> > Why is there a second index created on co_person_id?
> I could have sworn it was sponsor_co_person_id; it seemed like you had
> an index for all the other foreign keys so I threw that one in too.
> Does it make sense if there are two foreign keys to the same table?

It's not that indices are required for foreign keys, it's that indices 
are required for queries that are keyed on that column. So if you're 
SELECTing WHERE sponsor_co_person_id=x, then you should index that column.

Are there any queries that do that? If not, don't create the index.

Regarding the demographics table, let's hold off until tomorrow's call. 
However, if we encode the values as numbers, then the type should be I, 
not C.

> > Why is $sponsors set in both co_person_roles_controller and
> > co_people_controller?
>
> It's used for the views of both of these so need to be set in both 
> controllers:
> gears/co_people/edit/1/co:1 (to display name of Sponsor)
> gears/co_person_roles/edit/4/co:1 (to feed into dropdown for Sponsor 
> selection)

So if sponsor applies to a role, why is it displayed in co_people?

+++ trunk/app/controllers/co_person_roles_controller.php

Why do you need to loadModel('CoPerson')? Shouldn't it already be 
available to CoPersonRoles due to the association?

+++ trunk/app/models/demographic.php

Why isn't $belongsTo just

 $belongsTo = array("CoPerson")

since it follows the normal conventions?

+++ trunk/app/controllers/demographics_controller.php

This can't be right:

+    // Establish pagination parameters for HTML views
+    var $paginate = array(
+      'limit' => 25,
+      'order' => array(
+        'CoGroup.name' => 'asc'
+      )
+    );

CoGroup?

Also, it looks like you copy and pasted isAuthorized from a controller 
with more logic than Demographics needs. You should review what's going 
on there.

Similarly, editself().

+++ trunk/app/controllers/co_people_controller.php

Again, why are you calling loadModel('CoPerson')? You're in the CoPerson 
controller... the model should already be available.

+++ trunk/app/libs/lang.php

In the en.foo translations, key them on the Enum values. eg

 'en.gender' => array(GenderEnum::Female => 'Female'

Why do you define fd.de.persid? (See related comment below.)

For fd.sponsor.desc, the convention is not to use parens, but to render 
in <font class="desc">. See 
view/cmp_enrollment_configurations/fields.inc as an example. (Applies to 
views/co_person_roles/fields.inc and views/co_people/fields.inc.)

> > > M       trunk/app/libs/enum.php

Any reason not to use USPermanentResident instead of USPermRes?

+++ trunk/app/views/co_person_roles/fields.inc

Shouldn't you just be able to reference $sponsors instead of having to do

 $options = $this->viewVars['sponsors']

? (And again in co_people/fields.inc.)

Why is this line being removed:

-                echo '<a class="deletebutton" title="' . 
_txt('op.delete') . '" onclick="javascript:js_confirm_delete(\'' . 
_jtxt(Sanitize::html($t['number'])) . '\', \'' . 
$html->url(array('controller' => 'telephone_numbers', 'action' => 
'delete', $t['id'], 'co' => $cur_co['Co']['id'])) . '\')";>' . 
_txt('op.delete') . '</a>' . "\n";

?

+++ trunk/app/views/demographics/index.ctp

Why is person ID rendered in the table?

-Benn-