2.3 findThreaded with fields should auto-include parent_id #893

Closed
wants to merge 3 commits into
from

Projects

None yet

3 participants

@dereuromark
Member

see http://cakephp.lighthouseapp.com/projects/42648/tickets/3267-make-tree-behavior-work-with-fields-array-in-options

the documentation should also be updated:

"If you pass fields as a string you always have to manually include the parent_id".

@ADmad ADmad and 1 other commented on an outdated diff Oct 9, 2012
lib/Cake/Model/Behavior/TreeBehavior.php
@@ -108,8 +108,13 @@ public function afterSave(Model $Model, $created) {
* @return array
*/
public function beforeFind(Model $Model, $query) {
- if ($Model->findQueryType == 'threaded' && !isset($query['parent'])) {
- $query['parent'] = $this->settings[$Model->alias]['parent'];
+ if ($Model->findQueryType == 'threaded') {
+ if (!isset($query['parent'])) {
+ $query['parent'] = $this->settings[$Model->alias]['parent'];
+ }
+ if (!empty($query['fields']) && !in_array($query['parent'], $query['fields'])) {
+ $query['fields'][] = $query['parent'];
@ADmad
ADmad Oct 9, 2012 Member

Does $query['parent'] have the field name prefixed with model alias? If not it should be prefixed.

@dereuromark
dereuromark Oct 9, 2012 Member

it does not. we might also have to check on ModelAlias.field then.

@ADmad
ADmad Oct 9, 2012 Member

Does _findThreaded() itself work as expected if the results don't contain parent field? I think not. In that case the fix shouldn't be done in the TreeBehavior.

@markstory
Member

If people cannot include the proper fields their chances of getting the correct parent column are also low. I don't see why any of this beyond the documentation is really necessary.

This also omits the case where someone has included the quoted version of the parent field 😉

@ADmad
Member
ADmad commented Oct 9, 2012

..and cases when someone specifies fields as string. Regardless this is a documentation issue, not a bug. We shouldn't add more "magic" to fix people's oversight.

@dereuromark
Member

if you all agree on not wanting this included I will stand down :) I always manually add my parent_id.

@ADmad
Member
ADmad commented Oct 10, 2012

You can add the disclaimer in the manual and close the ticket :)

@dereuromark
Member

closing then in favor of the doc update

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment