Package home | Report new bug | New search | Development Roadmap Status: Open | Feedback | All | Closed Since Version 1.0.2

Bug #3182 _getFieldsToRender() and _reorderElements() drop fields in certain situations
Submitted: 2005-01-12 21:22 UTC
From: ate2 at cornell dot edu Assigned: justinpatrin
Status: Closed Package: DB_DataObject_FormBuilder
PHP Version: Irrelevant OS: Irrelevant
Roadmaps: (Not assigned)    
Subscription  


 [2005-01-12 21:22 UTC] ate2 at cornell dot edu
Description: ------------ There a some related (I think) issues here: 1. _getFieldsToRender() drops special fields when fieldsToRender property is set. 2. _reorderElements() drops any fields that are not in preDefOrder, including both special fields and fieldsToRender fields 3. even if special elements could be reordered, it would be difficult to specify the correct field name for reordering or other purposes (e.g. field labels). 4. _getFieldsToRender() always includes values returned from keys() but sometimes there are values from keys() that I don't need/want in my form. so some requests: 1. fix problems 1 and 2 (I've got a solution that seems to be working so far for me (see reproduce code)) 2. implement a method or methods like getSpecialElementName($type,$tableName) to reference the element in question 3. consider removing the feature described in 4 or making it optional through a class property Reproduce code: --------------- $do =& new DB_DataObject(); $fb =& new DB_DataObject_FormBuilder($do); $fb->fieldsToRender=array("someField","someOtherField"); $fb->preDefOrder=array("someOtherField"); $fb->crossLinks=array(array("table"=>"links")); $form =& $fb->getForm(); echo $form->toHtml(); My fix for this issue is available at: http://environment.cornell.edu/source/DB.DataObject.FormBuilder.phps Expected result: ---------------- Form with elements: someOtherField keyField (from $do->keys()) otherField __crossLink_links Actual result: -------------- Form with element: someOtherField

Comments

 [2005-01-12 21:39 UTC] ate2 at cornell dot edu
Noticed another problem with my fix... the special elements that are rendered are frozen because of a glitch with _getUserEditableFields(). I added an example workaround that resolves this for me to the same place as the other fixes. It involves overriding that method.
 [2005-01-12 21:46 UTC] justinpatrin
The special fields are normal fields and can be specified in the fieldToRender, userEditableFields, fieldLabels, etc. just fine. __crossLink_tableName __tripleLink_tableName __reverseLink_tableName Yes, these should be in the docs, but I just didn't think of it. Workarounds are not needed for these as yu can specify them yourself. Consider points 1-3 "Bogus" bugs. We *may* include a getSpecialElementName method, but IMHO this isn't really needed as it's simple (see above). I can see a possible problem with point 4. We should probably just use the field from _getPrimaryKey instead of doing all of the keys. Markus, what do you think?
 [2005-01-12 22:35 UTC] ate2 at cornell dot edu
With point 3, I was trying to imagine a scenario down the road where the naming convention might change and refactoring might be an issue... really is just feature request. But I still don't like 2. preDefOrder is only supposed to specify the order in which fields are arranged; why should every field have to be explicitly list twice in code if there are one or more fields I don't want displayed? I especially see this as a bug because preDefOrder is doing something that is not part of its API - preventing elements from being rendered. As for 1, I guess it might just be my taste but it seems more intuitive to eliminate special element fields by not adding them to crossLinks, tripleLinks, etc. properties. Same sort of double-listing issue as with 2, but in this case no method actually does something the docs don't say it does. BTW, great package, don't mean to be a nag.
 [2005-01-12 22:56 UTC] justinpatrin
1 will stay as-is. It makes perfect sense to be able to specify the special fields in all of the config arrays. Forcing them to be viewable and only at the end of the form is IMHO much worse. I can understand the preDefOrder thing. I actually also had a similar problem a while ago, but never addressed it. IMHO it should do all ordering, then put all remaining elements to render after the others. This is what I did in a recent DataGrid patch and should port it to FB. It may be possible to have those names change, but the only time it would that I can think of is if we allowed multiple crosslink elements to the *same* crosslink table. This is sort of theoretically possible, but I don't see it being a particularly useful feature. This is actually already done with reverseLinks, my above naming scheme is incorrect. The correct scheme for reverseLinks is: __reverseLink_tableName_fieldName This allow reverseLinking multiple fields from one other table.
 [2005-01-13 00:09 UTC] ate2 at cornell dot edu
Thx. I'm not sure whether this changes anything but what I was talking about in 1 pertains only to fieldsToRender. In the workaround I'm using special elements can still be repositioned with preDefOrder. They just don't have to be included in fieldsToRender when that option is used.
 [2005-01-13 00:30 UTC] justinpatrin
I'd rather leave fieldsToRender alone. Allowing people to disable special fields in fieldsToRender is too good an option to get rid of.
 [2005-01-14 17:43 UTC] justinpatrin
I've put a fix in CVS which renders fields in fieldsToRender even if they're not in preDefOrder. Please check it out and see what you think.
 [2005-02-02 02:19 UTC] justinpatrin
This bug has been fixed in CVS. In case this was a documentation problem, the fix will show up at the end of next Sunday (CET) on pear.php.net. In case this was a pear.php.net website problem, the change will show up on the website in short time. Thank you for the report, and for helping us make PEAR better.