Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Fix issue with find(count) and translated conditions.

Because count queries did not have joins created for the translated
fields pagination would generate invalid queries. Checking the conditions
for translated fields and adding in the correct joins solves that.
Extract what would have been duplicated code into methods.

Add a few protected properties to keep method signatures sane. The code
could be even simpler if the existing find(count) join was removed.

Fixes #2349
  • Loading branch information...
commit f9bcc950a18b6e033ef4f30c1751162e23b4c000 1 parent 049b04d
@markstory markstory authored
View
136 cake/libs/model/behaviors/translate.php
@@ -35,6 +35,20 @@ class TranslateBehavior extends ModelBehavior {
var $runtime = array();
/**
+ * Stores the joinTable object for generating joins.
+ *
+ * @var object
+ */
+ var $_joinTable;
+
+/**
+ * Stores the runtime model for generating joins.
+ *
+ * @var Model
+ */
+ var $_runtimeModel;
+
+/**
* Callback
*
* $config for TranslateBehavior should be
@@ -96,6 +110,7 @@ function beforeFind(&$model, $query) {
}
$db =& ConnectionManager::getDataSource($model->useDbConfig);
$RuntimeModel =& $this->translateModel($model);
+
if (!empty($RuntimeModel->tablePrefix)) {
$tablePrefix = $RuntimeModel->tablePrefix;
} else {
@@ -105,8 +120,12 @@ function beforeFind(&$model, $query) {
$joinTable->tablePrefix = $tablePrefix;
$joinTable->table = $RuntimeModel->table;
- if (is_string($query['fields']) && 'COUNT(*) AS '.$db->name('count') == $query['fields']) {
+ $this->_joinTable = $joinTable;
+ $this->_runtimeModel = $RuntimeModel;
+
+ if (is_string($query['fields']) && 'COUNT(*) AS ' . $db->name('count') == $query['fields']) {
$query['fields'] = 'COUNT(DISTINCT('.$db->name($model->alias . '.' . $model->primaryKey) . ')) ' . $db->alias . 'count';
+
$query['joins'][] = array(
'type' => 'INNER',
'alias' => $RuntimeModel->alias,
@@ -117,6 +136,11 @@ function beforeFind(&$model, $query) {
$RuntimeModel->alias.'.locale' => $locale
)
);
+ $conditionFields = $this->_checkConditions($model, $query);
+ foreach ($conditionFields as $field) {
+ $query = $this->_addJoin($model, $query, $field, $locale, false);
+ }
+ unset($this->_joinTable, $this->_runtimeModel);
return $query;
}
$autoFields = false;
@@ -150,7 +174,6 @@ function beforeFind(&$model, $query) {
if (is_array($query['fields'])) {
foreach ($fields as $key => $value) {
$field = (is_numeric($key)) ? $value : $key;
-
if (in_array($model->alias.'.*', $query['fields']) || $autoFields || in_array($model->alias.'.'.$field, $query['fields']) || in_array($field, $query['fields'])) {
$addFields[] = $field;
}
@@ -166,39 +189,90 @@ function beforeFind(&$model, $query) {
unset($query['fields'][$key]);
}
}
+ $query = $this->_addJoin($model, $query, $field, $locale, true);
+ }
+ }
+ $this->runtime[$model->alias]['beforeFind'] = $addFields;
+ unset($this->_joinTable, $this->_runtimeModel);
+ return $query;
+ }
- if (is_array($locale)) {
- foreach ($locale as $_locale) {
- $query['fields'][] = 'I18n__'.$field.'__'.$_locale.'.content';
- $query['joins'][] = array(
- 'type' => 'LEFT',
- 'alias' => 'I18n__'.$field.'__'.$_locale,
- 'table' => $joinTable,
- 'conditions' => array(
- $model->alias . '.' . $model->primaryKey => $db->identifier("I18n__{$field}__{$_locale}.foreign_key"),
- 'I18n__'.$field.'__'.$_locale.'.model' => $model->name,
- 'I18n__'.$field.'__'.$_locale.'.'.$RuntimeModel->displayField => $field,
- 'I18n__'.$field.'__'.$_locale.'.locale' => $_locale
- )
- );
- }
- } else {
- $query['fields'][] = 'I18n__'.$field.'.content';
- $query['joins'][] = array(
- 'type' => 'INNER',
- 'alias' => 'I18n__'.$field,
- 'table' => $joinTable,
- 'conditions' => array(
- $model->alias . '.' . $model->primaryKey => $db->identifier("I18n__{$field}.foreign_key"),
- 'I18n__'.$field.'.model' => $model->name,
- 'I18n__'.$field.'.'.$RuntimeModel->displayField => $field,
- 'I18n__'.$field.'.locale' => $locale
- )
- );
+/**
+ * Check a query's conditions for translated fields.
+ * Return an array of translated fields found in the conditions.
+ *
+ * @param Model $model The model being read.
+ * @param array $query The query array.
+ * @return array The list of translated fields that are in the conditions.
+ */
+ function _checkConditions(&$model, $query) {
+ $conditionFields = array();
+ if (empty($query['conditions']) || (!empty($query['conditions']) && !is_array($query['conditions'])) ) {
+ return $conditionFields;
+ }
+ foreach ($query['conditions'] as $col => $val) {
+ foreach ($this->settings[$model->alias] as $field => $assoc) {
+ if (is_numeric($field)) {
+ $field = $assoc;
+ }
+ if (strpos($col, $field) !== false) {
+ $conditionFields[] = $field;
}
}
}
- $this->runtime[$model->alias]['beforeFind'] = $addFields;
+ return $conditionFields;
+ }
+
+/**
+ * Appends a join for translated fields and possibly a field.
+ *
+ * @param Model $model The model being worked on.
+ * @param object $joinTable The jointable object.
+ * @param array $query The query array to append a join to.
+ * @param string $field The field name being joined.
+ * @param mixed $locale The locale(s) having joins added.
+ * @param boolean $addField Whether or not to add a field.
+ * @return array The modfied query
+ */
+ function _addJoin(&$model, $query, $field, $locale, $addField = false) {
+ $db =& ConnectionManager::getDataSource($model->useDbConfig);
+
+ $RuntimeModel = $this->_runtimeModel;
+ $joinTable = $this->_joinTable;
+
+ if (is_array($locale)) {
+ foreach ($locale as $_locale) {
+ if ($addField) {
+ $query['fields'][] = 'I18n__'.$field.'__'.$_locale.'.content';
+ }
+ $query['joins'][] = array(
+ 'type' => 'LEFT',
+ 'alias' => 'I18n__'.$field.'__'.$_locale,
+ 'table' => $joinTable,
+ 'conditions' => array(
+ $model->alias . '.' . $model->primaryKey => $db->identifier("I18n__{$field}__{$_locale}.foreign_key"),
+ 'I18n__'.$field.'__'.$_locale.'.model' => $model->name,
+ 'I18n__'.$field.'__'.$_locale.'.'.$RuntimeModel->displayField => $field,
+ 'I18n__'.$field.'__'.$_locale.'.locale' => $_locale
+ )
+ );
+ }
+ } else {
+ if ($addField) {
+ $query['fields'][] = 'I18n__'.$field.'.content';
+ }
+ $query['joins'][] = array(
+ 'type' => 'INNER',
+ 'alias' => 'I18n__'.$field,
+ 'table' => $joinTable,
+ 'conditions' => array(
+ $model->alias . '.' . $model->primaryKey => $db->identifier("I18n__{$field}.foreign_key"),
+ 'I18n__'.$field.'.model' => $model->name,
+ 'I18n__'.$field.'.'.$RuntimeModel->displayField => $field,
+ 'I18n__'.$field.'.locale' => $locale
+ )
+ );
+ }
return $query;
}
View
19 cake/tests/cases/libs/model/behaviors/translate.test.php
@@ -63,6 +63,24 @@ function endTest() {
}
/**
+ * Test that count queries with conditions get the correct joins
+ *
+ * @return void
+ */
+ function testCountWithConditions() {
+ $this->loadFixtures('Translate', 'TranslatedItem');
+
+ $Model =& new TranslatedItem();
+ $Model->locale = 'eng';
+ $result = $Model->find('count', array(
+ 'conditions' => array(
+ 'I18n__content.locale' => 'eng'
+ )
+ ));
+ $this->assertEqual(3, $result);
+ }
+
+/**
* testTranslateModel method
*
* @access public
@@ -895,4 +913,5 @@ function testUnbindTranslationInfinteLoop() {
$this->assertFalse($result);
}
+
}
Please sign in to comment.
Something went wrong with that request. Please try again.