#1246 new
cricket

Containable should keep assoc conditions

Reported by cricket | October 29th, 2010 @ 10:46 PM | in RFC

(1.3.5)

In beforeFind(), Containable unbinds associated models briefly. If an association has been declared with conditions, those will not be used unless those conditions are also included in the contain array. I think it would be better to merge the association conditions (if any) with those (if any) included in 'contain'.

scenario:

// Book
public $hasMany = array(
    'Image' => array(
        'className' => 'Image',    
        'foreignKey' => 'foreign_key',
        'conditions' => array('Image.model' => 'Book'),
        'dependent' => true
    )
);

// Image
public $belongsTo = array(
    'Book' => array(
        'className' => 'Book',
        'foreignKey' => false,
        'conditions' => array(
            'Image.model' => 'Book',
            'Image.foreign_key' => 'Book.id'
        ),
        'dependent' => true
    ),
    'Journal' => array(
        'className' => 'Site',
        'foreignKey' => false,
        'conditions' => array(
            'Image.model' => 'Journal',
            'Image.foreign_key' => 'Journal.id'
        ),
        'dependent' => true
    )
);

// BooksController
public $paginate = array(
    'fields' => array('*'),
    'contain' => array(
        'Image' => array(
            'conditions' => array('Image.sort_order' => 1),
            'Thumbnail'
        )
    )
    //, etc ...
);

What I'm after here is to limit the number of Images returned to just one when paginating a list of Books (yeah, I guess that's obvious). Because we can't have limit inside of contain the condition is used to do effectively the same thing.

However, Containable will use only the condition in $paginate, not also 'Image.model' => 'Book', resulting in Images from other models (that have sort_order == 1) being returned. This can be remedied by including the conditions before calling paginate():

$this->paginate['contain']['Image']['conditions'] = array_merge(
    $this->Book->hasMany['Image']['conditions'],
    array('Image.sort_order' => 1)
);

// or, keep the original conditions in $paginate, and:

$this->paginate['contain']['Image']['conditions'] = array_merge(
    $this->Book->hasMany['Image']['conditions'],
    $this->paginate['contain']['Image']['conditions']
);

$this->set('data', $this->paginate());

This works but it's inelegant. A (possible) fix for Containable::beforeFind() ...

/* FIX
 */
if (isset($instance->__backAssociation[$type][$assoc]['conditions']))
{
    if (!isset($model['keep'][$assoc]['conditions'])) {
        $model['keep'][$assoc]['conditions'] = array();
    }
    $model['keep'][$assoc]['conditions'] = array_merge(
        $model['keep'][$assoc]['conditions'],
        $instance->__backAssociation[$type][$assoc]['conditions']
    );
}
/* END FIX
 */
$instance->{$type}[$assoc] = array_merge($instance->{$type}[$assoc], $model['keep'][$assoc]);

I say "possible" for the same reason I'm not including a test. While I understand $backAssociation, I'm a bit hazy on $reset, $backContainableAssociation, $backOriginalAssociation, and $backInnerAssociation (getting hazier), so I wouldn't be surprised if a little more work would be necessary.

Comments and changes to this ticket

Please Sign in or create a free account to add a new ticket.

With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.

New-ticket Create new ticket

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

Source available from github

Repository is at http://github.com/cakephp/cakephp

Creating a bug report

When creating a bug report, please include as much relevant information as possible. Please include code to reproduce the issue. Or even better, make a unit test. Either change an existing test or add a new test to show that the expected behavior is not occuring.

People watching this ticket

Attachments

Tags