Merge pull request #10863 from bancer/paginator-i18n-not

Pagination 'NOT' notation fails with i18n

lib/Cake/Model/Behavior/TranslateBehavior.php

@@ -147,7 +147,8 @@ class TranslateBehavior extends ModelBehavior {
 		$this->_joinTable = $joinTable;
 		$this->_runtimeModel = $RuntimeModel;
 
-		if (is_string($query['fields']) && $query['fields'] === "COUNT(*) AS {$db->name('count')}") {
+		if (is_string($query['fields'])) {
+			if ($query['fields'] === "COUNT(*) AS {$db->name('count')}") {
 				$query['fields'] = "COUNT(DISTINCT({$db->name($Model->escapeField())})) {$db->alias}count";
 				$query['joins'][] = array(
 					'type' => $this->runtime[$Model->alias]['joinType'],
@@ -165,10 +166,26 @@ class TranslateBehavior extends ModelBehavior {
 				}
 				unset($this->_joinTable, $this->_runtimeModel);
 				return $query;
-		} elseif (is_string($query['fields'])) {
+			} else {
 				$query['fields'] = CakeText::tokenize($query['fields']);
 			}
+		}
+		$addFields = $this->_getFields($Model, $query);
+		$this->runtime[$Model->alias]['virtualFields'] = $Model->virtualFields;
+		$query = $this->_addAllJoins($Model, $query, $addFields);
+		$this->runtime[$Model->alias]['beforeFind'] = $addFields;
+		unset($this->_joinTable, $this->_runtimeModel);
+		return $query;
+	}
 
+/**
+ * Gets fields to be retrieved.
+ *
+ * @param Model $Model The model being worked on.
+ * @param array $query The query array to take fields from.
+ * @return array The fields.
+ */
+	protected function _getFields(Model $Model, $query) {
 		$fields = array_merge(
 			$this->settings[$Model->alias],
 			$this->runtime[$Model->alias]['fields']
@@ -191,15 +208,24 @@ class TranslateBehavior extends ModelBehavior {
 				}
 			}
 		}
+		return $addFields;
+	}
 
-		$this->runtime[$Model->alias]['virtualFields'] = $Model->virtualFields;
+/**
+ * Appends all necessary joins for translated fields.
+ *
+ * @param Model $Model The model being worked on.
+ * @param array $query The query array to append joins to.
+ * @param array $addFields The fields being joined.
+ * @return array The modified query
+ */
+	protected function _addAllJoins(Model $Model, $query, $addFields) {
+		$locale = $this->_getLocale($Model);
 		if ($addFields) {
 			foreach ($addFields as $_f => $field) {
 				$aliasField = is_numeric($_f) ? $field : $_f;
-
 				foreach (array($aliasField, $Model->alias . '.' . $aliasField) as $_field) {
 					$key = array_search($_field, (array)$query['fields']);
-
 					if ($key !== false) {
 						unset($query['fields'][$key]);
 					}
@@ -207,8 +233,6 @@ class TranslateBehavior extends ModelBehavior {
 				$query = $this->_addJoin($Model, $query, $field, $aliasField, $locale);
 			}
 		}
-		$this->runtime[$Model->alias]['beforeFind'] = $addFields;
-		unset($this->_joinTable, $this->_runtimeModel);
 		return $query;
 	}
 
@@ -221,11 +245,26 @@ class TranslateBehavior extends ModelBehavior {
  * @return array The list of translated fields that are in the conditions.
  */
 	protected function _checkConditions(Model $Model, $query) {
-		$conditionFields = array();
 		if (empty($query['conditions']) || (!empty($query['conditions']) && !is_array($query['conditions']))) {
-			return $conditionFields;
+			return array();
+		}
+		return $this->_getConditionFields($Model, $query['conditions']);
+	}
+
+/**
+ * Extracts condition field names recursively.
+ *
+ * @param Model $Model The model being read.
+ * @param array $conditions The conditions array.
+ * @return array The list of condition fields.
+ */
+	protected function _getConditionFields(Model $Model, $conditions) {
+		$conditionFields = array();
+		foreach ($conditions as $col => $val) {
+			if (is_array($val)) {
+				$subConditionFields = $this->_getConditionFields($Model, $val);
+				$conditionFields = array_merge($conditionFields, $subConditionFields);
 			}
-		foreach ($query['conditions'] as $col => $val) {
 			foreach ($this->settings[$Model->alias] as $field => $assoc) {
 				if (is_numeric($field)) {
 					$field = $assoc;

lib/Cake/Test/Case/Controller/Component/PaginatorComponentTest.php

@@ -496,6 +496,25 @@ class PaginatorComponentTest extends CakeTestCase {
 		$this->assertCount(3, $result);
 	}
 
+	public function testPaginateI18nConditionNotTitleWithLimit() {
+		$Request = new CakeRequest('articles/index');
+		$Controller = new PaginatorTestController($Request);
+		$Controller->uses = array('TranslatedArticle');
+		$Controller->constructClasses();
+		$Controller->TranslatedArticle->locale = 'eng';
+		$Controller->Paginator->settings = array(
+			'recursive' => 0,
+			'conditions' => array(
+				'NOT' => array('I18n__title.content' => ''),
+			),
+			'limit' => 2,
+		);
+		$result = $Controller->Paginator->paginate('TranslatedArticle');
+		$this->assertEquals('Title (eng) #1', $result[0]['TranslatedArticle']['title']);
+		$this->assertEquals('Title (eng) #2', $result[1]['TranslatedArticle']['title']);
+		$this->assertCount(2, $result);
+	}
+
 /**
  * Test that non-numeric values are rejected for page, and limit
  *

lib/Cake/Test/Case/Model/Behavior/TranslateBehaviorTest.php

@@ -1443,4 +1443,166 @@ class TranslateBehaviorTest extends CakeTestCase {
 		$this->assertEquals('name', $results[0]['TranslateTestModel']['field']);
 	}
 
+	public function testBeforeFindAllI18nConditions() {
+		$this->skipIf(!$this->db instanceof Mysql, 'This test is only compatible with Mysql.');
+		$this->loadFixtures('TranslateArticle', 'TranslatedArticle', 'User');
+		$TestModel = new TranslatedArticle();
+		$TestModel->cacheQueries = false;
+		$TestModel->locale = 'eng';
+		$expected = array(
+			'conditions' => array(
+				'NOT' => array('I18n__title.content' => ''),
+			),
+			'fields' => null,
+			'joins' => array(
+				array(
+					'type' => 'INNER',
+					'alias' => 'I18n__title',
+					'table' => (object)array(
+						'tablePrefix' => '',
+						'table' => 'article_i18n',
+						'schemaName' => 'cakephp_test',
+					),
+					'conditions' => array(
+						'TranslatedArticle.id' => (object)array(
+							'type' => 'identifier',
+							'value' => 'I18n__title.foreign_key',
+						),
+						'I18n__title.model' => 'TranslatedArticle',
+						'I18n__title.field' => 'title',
+						'I18n__title.locale' => 'eng',
+					),
+				),
+				array(
+					'type' => 'INNER',
+					'alias' => 'I18n__body',
+					'table' => (object)array(
+						'tablePrefix' => '',
+						'table' => 'article_i18n',
+						'schemaName' => 'cakephp_test',
+					),
+					'conditions' => array(
+						'TranslatedArticle.id' => (object)array(
+							'type' => 'identifier',
+							'value' => 'I18n__body.foreign_key',
+						),
+						'I18n__body.model' => 'TranslatedArticle',
+						'I18n__body.field' => 'body',
+						'I18n__body.locale' => 'eng',
+					),
+				),
+			),
+			'limit' => 2,
+			'offset' => null,
+			'order' => array(
+				'TranslatedArticle.id' => 'ASC',
+			),
+			'page' => 1,
+			'group' => null,
+			'callbacks' => true,
+			'recursive' => 0,
+		);
+		$query = array(
+			'conditions' => array(
+				'NOT' => array(
+					'I18n__title.content' => '',
+				),
+			),
+			'fields' => null,
+			'joins' => array(),
+			'limit' => 2,
+			'offset' => null,
+			'order' => array(
+				'TranslatedArticle.id' => 'ASC',
+			),
+			'page' => 1,
+			'group' => null,
+			'callbacks' => true,
+			'recursive' => 0,
+		);
+		$TranslateBehavior = ClassRegistry::getObject('TranslateBehavior');
+		$result = $TranslateBehavior->beforeFind($TestModel, $query);
+		$this->assertEquals($expected, $result);
+	}
+
+	public function testBeforeFindCountI18nConditions() {
+		$this->skipIf(!$this->db instanceof Mysql, 'This test is only compatible with Mysql.');
+		$this->loadFixtures('TranslateArticle', 'TranslatedArticle', 'User');
+		$TestModel = new TranslatedArticle();
+		$TestModel->cacheQueries = false;
+		$TestModel->locale = 'eng';
+		$expected = array(
+			'conditions' => array(
+				'NOT' => array('I18n__title.content' => ''),
+			),
+			'fields' => 'COUNT(DISTINCT(`TranslatedArticle`.`id`)) AS count',
+			'joins' => array(
+				array(
+					'type' => 'INNER',
+					'alias' => 'TranslateArticleModel',
+					'table' => (object)array(
+						'tablePrefix' => '',
+						'table' => 'article_i18n',
+						'schemaName' => 'cakephp_test',
+					),
+					'conditions' => array(
+						'`TranslatedArticle`.`id`' => (object)array(
+							'type' => 'identifier',
+							'value' => '`TranslateArticleModel`.`foreign_key`',
+						),
+						'`TranslateArticleModel`.`model`' => 'TranslatedArticle',
+						'`TranslateArticleModel`.`locale`' => 'eng',
+					),
+				),
+				array(
+					'type' => 'INNER',
+					'alias' => 'I18n__title',
+					'table' => (object)array(
+						'tablePrefix' => '',
+						'table' => 'article_i18n',
+						'schemaName' => 'cakephp_test',
+					),
+					'conditions' => array(
+						'TranslatedArticle.id' => (object)array(
+							'type' => 'identifier',
+							'value' => 'I18n__title.foreign_key',
+						),
+						'I18n__title.model' => 'TranslatedArticle',
+						'I18n__title.field' => 'title',
+						'I18n__title.locale' => 'eng',
+					),
+				),
+			),
+			'limit' => 2,
+			'offset' => null,
+			'order' => array(
+				0 => false,
+			),
+			'page' => 1,
+			'group' => null,
+			'callbacks' => true,
+			'recursive' => 0,
+		);
+		$query = array(
+			'conditions' => array(
+				'NOT' => array(
+					'I18n__title.content' => '',
+				)
+			),
+			'fields' => 'COUNT(*) AS `count`',
+			'joins' => array(),
+			'limit' => 2,
+			'offset' => null,
+			'order' => array(
+				0 => false
+			),
+			'page' => 1,
+			'group' => null,
+			'callbacks' => true,
+			'recursive' => 0,
+		);
+		$TranslateBehavior = ClassRegistry::getObject('TranslateBehavior');
+		$result = $TranslateBehavior->beforeFind($TestModel, $query);
+		$this->assertEquals($expected, $result);
+	}
 }

lib/Cake/Test/Case/Model/ModelReadTest.php

@@ -6457,6 +6457,145 @@ class ModelReadTest extends BaseModelTest {
 		$this->assertEquals($expected, $result);
 	}
 
+	public function testBuildQueryAllI18nConditions() {
+		$this->skipIf(!$this->db instanceof Mysql, 'This test is only compatible with Mysql.');
+		$this->loadFixtures('TranslateArticle', 'TranslatedArticle', 'User');
+		$TestModel = new TranslatedArticle();
+		$TestModel->cacheQueries = false;
+		$TestModel->locale = 'eng';
+		$expected = array(
+			'conditions' => array(
+				'NOT' => array('I18n__title.content' => ''),
+			),
+			'fields' => null,
+			'joins' => array(
+				array(
+					'type' => 'INNER',
+					'alias' => 'I18n__title',
+					'table' => (object)array(
+						'tablePrefix' => '',
+						'table' => 'article_i18n',
+						'schemaName' => 'cakephp_test',
+					),
+					'conditions' => array(
+						'TranslatedArticle.id' => (object)array(
+							'type' => 'identifier',
+							'value' => 'I18n__title.foreign_key',
+						),
+						'I18n__title.model' => 'TranslatedArticle',
+						'I18n__title.field' => 'title',
+						'I18n__title.locale' => 'eng',
+					),
+				),
+				array(
+					'type' => 'INNER',
+					'alias' => 'I18n__body',
+					'table' => (object)array(
+						'tablePrefix' => '',
+						'table' => 'article_i18n',
+						'schemaName' => 'cakephp_test',
+					),
+					'conditions' => array(
+						'TranslatedArticle.id' => (object)array(
+							'type' => 'identifier',
+							'value' => 'I18n__body.foreign_key',
+						),
+						'I18n__body.model' => 'TranslatedArticle',
+						'I18n__body.field' => 'body',
+						'I18n__body.locale' => 'eng',
+					),
+				),
+			),
+			'limit' => 2,
+			'offset' => null,
+			'order' => array(
+				'TranslatedArticle.id' => 'ASC',
+			),
+			'page' => 1,
+			'group' => null,
+			'callbacks' => true,
+			'recursive' => 0,
+		);
+		$query = array(
+			'recursive' => 0,
+			'conditions' => array(
+				'NOT' => array('I18n__title.content' => ''),
+			),
+			'limit' => 2,
+		);
+		$result = $TestModel->buildQuery('all', $query);
+		$this->assertEquals($expected, $result);
+	}
+
+	public function testBuildQueryCountI18nConditions() {
+		$this->skipIf(!$this->db instanceof Mysql, 'This test is only compatible with Mysql.');
+		$this->loadFixtures('TranslateArticle', 'TranslatedArticle', 'User');
+		$TestModel = new TranslatedArticle();
+		$TestModel->cacheQueries = false;
+		$TestModel->locale = 'eng';
+		$expected = array(
+			'conditions' => array(
+				'NOT' => array('I18n__title.content' => ''),
+			),
+			'fields' => 'COUNT(DISTINCT(`TranslatedArticle`.`id`)) AS count',
+			'joins' => array(
+				array(
+					'type' => 'INNER',
+					'alias' => 'TranslateArticleModel',
+					'table' => (object)array(
+						'tablePrefix' => '',
+						'table' => 'article_i18n',
+						'schemaName' => 'cakephp_test',
+					),
+					'conditions' => array(
+						'`TranslatedArticle`.`id`' => (object)array(
+							'type' => 'identifier',
+							'value' => '`TranslateArticleModel`.`foreign_key`',
+						),
+						'`TranslateArticleModel`.`model`' => 'TranslatedArticle',
+						'`TranslateArticleModel`.`locale`' => 'eng',
+					),
+				),
+				array(
+					'type' => 'INNER',
+					'alias' => 'I18n__title',
+					'table' => (object)array(
+						'tablePrefix' => '',
+						'table' => 'article_i18n',
+						'schemaName' => 'cakephp_test',
+					),
+					'conditions' => array(
+						'TranslatedArticle.id' => (object)array(
+							'type' => 'identifier',
+							'value' => 'I18n__title.foreign_key',
+						),
+						'I18n__title.model' => 'TranslatedArticle',
+						'I18n__title.field' => 'title',
+						'I18n__title.locale' => 'eng',
+					),
+				),
+			),
+			'limit' => 2,
+			'offset' => null,
+			'order' => array(
+				0 => false,
+			),
+			'page' => 1,
+			'group' => null,
+			'callbacks' => true,
+			'recursive' => 0,
+		);
+		$query = array(
+			'recursive' => 0,
+			'conditions' => array(
+				'NOT' => array('I18n__title.content' => ''),
+			),
+			'limit' => 2,
+		);
+		$result = $TestModel->buildQuery('count', $query);
+		$this->assertEquals($expected, $result);
+	}
+
 /**
  * test find('all') method
  *
@@ -6717,6 +6856,62 @@ class ModelReadTest extends BaseModelTest {
 		$this->assertEquals($expected, $result);
 	}
 
+	public function testFindAllI18nConditions() {
+		$this->loadFixtures('TranslateArticle', 'TranslatedArticle', 'User');
+		$TestModel = new TranslatedArticle();
+		$TestModel->cacheQueries = false;
+		$TestModel->locale = 'eng';
+		$options = array(
+			'recursive' => 0,
+			'conditions' => array(
+				'NOT' => array('I18n__title.content' => ''),
+			),
+			'limit' => 2,
+		);
+		$result = $TestModel->find('all', $options);
+		$expected = array(
+			array(
+				'TranslatedArticle' => array(
+					'id' => '1',
+					'user_id' => '1',
+					'published' => 'Y',
+					'created' => '2007-03-18 10:39:23',
+					'updated' => '2007-03-18 10:41:31',
+					'locale' => 'eng',
+					'title' => 'Title (eng) #1',
+					'body' => 'Body (eng) #1',
+				),
+				'User' => array(
+					'id' => '1',
+					'user' => 'mariano',
+					'password' => '5f4dcc3b5aa765d61d8327deb882cf99',
+					'created' => '2007-03-17 01:16:23',
+					'updated' => '2007-03-17 01:18:31',
+				),
+			),
+			array(
+				'TranslatedArticle' => array(
+					'id' => '2',
+					'user_id' => '3',
+					'published' => 'Y',
+					'created' => '2007-03-18 10:41:23',
+					'updated' => '2007-03-18 10:43:31',
+					'locale' => 'eng',
+					'title' => 'Title (eng) #2',
+					'body' => 'Body (eng) #2',
+				),
+				'User' => array(
+					'id' => '3',
+					'user' => 'larry',
+					'password' => '5f4dcc3b5aa765d61d8327deb882cf99',
+					'created' => '2007-03-17 01:20:23',
+					'updated' => '2007-03-17 01:22:31',
+				),
+			),
+		);
+		$this->assertEquals($expected, $result);
+	}
+
 /**
  * test find('list') method
  *
@@ -7150,6 +7345,22 @@ class ModelReadTest extends BaseModelTest {
 		$this->assertEquals($expected, $result);
 	}
 
+	public function testFindCountI18nConditions() {
+		$this->loadFixtures('TranslateArticle', 'TranslatedArticle', 'User');
+		$TestModel = new TranslatedArticle();
+		$TestModel->cacheQueries = false;
+		$TestModel->locale = 'eng';
+		$options = array(
+			'recursive' => 0,
+			'conditions' => array(
+				'NOT' => array('I18n__title.content' => ''),
+			),
+			'limit' => 2,
+		);
+		$result = $TestModel->find('count', $options);
+		$this->assertEquals(3, $result);
+	}
+
 /**
  * Test that find('first') does not use the id set to the object.
  *

lib/Cake/Test/Case/Model/ModelTestBase.php

@@ -73,6 +73,7 @@ abstract class BaseModelTest extends CakeTestCase {
 		'core.bidding', 'core.bidding_message', 'core.site', 'core.domain', 'core.domains_site',
 		'core.uuidnativeitem', 'core.uuidnativeportfolio', 'core.uuidnativeitems_uuidnativeportfolio',
 		'core.uuidnativeitems_uuidnativeportfolio_numericid',
+		'core.translated_article', 'core.translate_article',
 	);
 
 /**