Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Making btm marshalling use table aliases #6867

Merged
merged 2 commits into from Jun 25, 2015

Conversation

patrickconroy
Copy link
Contributor

I tried making a test for this, but it'd involve overriding the findAll for TestApp\Model\Table\TagsTable, which causes a bunch of failures and is probably too unexpected. I could duplicate everything and make Tags2Table and Articles2Table, but that seems bad.

Refs #6866, #6826

@markstory markstory added this to the 3.0.8 milestone Jun 24, 2015
@markstory markstory added the ORM label Jun 24, 2015
@@ -295,7 +295,9 @@ protected function _belongsToMany(Association $assoc, array $data, $options = []
if (array_intersect_key($primaryKey, $row) === $primaryKey) {
$keys = array_intersect_key($row, $primaryKey);
if (count($keys) === $primaryCount) {
$conditions[] = $keys;
foreach ($keys as $key => $value) {
$conditions[][$assoc->alias() . '.' . $key] = $value;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can do

$target = $assoc->target();
foreach ($keys as $key => $value) {
  $conditions[$target->aliasfield($key)] = $value
}

It saves a few __call() methods, and uses a method that people can customize if necessary.

@markstory markstory merged commit 2238303 into cakephp:master Jun 25, 2015
markstory added a commit that referenced this pull request Jun 25, 2015
Call target() fewer times when marshalling belongsToMany associations.

Refs #6867
@markstory
Copy link
Member

Thanks @patrickconroy

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants