From bbe27999eb7a54a33c3c890e8f810438e1099f03 Mon Sep 17 00:00:00 2001 From: Jean-Fabien Barrois Date: Tue, 10 Feb 2015 10:38:24 +1300 Subject: [PATCH 1/2] BUGFIX Use correct query when searching for items managed by a tree dropdown field #3173 --- forms/TreeDropdownField.php | 15 +++-- tests/forms/TreeDropdownFieldTest.php | 97 +++++++++++++++++++++++++++ tests/forms/TreeDropdownFieldTest.yml | 25 +++++++ 3 files changed, 131 insertions(+), 6 deletions(-) create mode 100644 tests/forms/TreeDropdownFieldTest.php create mode 100644 tests/forms/TreeDropdownFieldTest.yml diff --git a/forms/TreeDropdownField.php b/forms/TreeDropdownField.php index 2f9f5d623..35d079e1a 100644 --- a/forms/TreeDropdownField.php +++ b/forms/TreeDropdownField.php @@ -460,15 +460,18 @@ class TreeDropdownField extends FormField { if ($row->ParentID) $parents[$row->ParentID] = true; $this->searchIds[$row->ID] = true; } + + $sourceObject = $this->sourceObject; + while (!empty($parents)) { - $res = DB::query('SELECT "ParentID", "ID" FROM "' . $this->sourceObject - . '" WHERE "ID" in ('.implode(',',array_keys($parents)).')'); + $items = $sourceObject::get() + ->filter("ID",array_keys($parents)); $parents = array(); - foreach($res as $row) { - if ($row['ParentID']) $parents[$row['ParentID']] = true; - $this->searchIds[$row['ID']] = true; - $this->searchExpanded[$row['ID']] = true; + foreach($items as $item) { + if ($item->ParentID) $parents[$item->ParentID] = true; + $this->searchIds[$item->ID] = true; + $this->searchExpanded[$item->ID] = true; } } } diff --git a/tests/forms/TreeDropdownFieldTest.php b/tests/forms/TreeDropdownFieldTest.php new file mode 100644 index 000000000..5e1f20c75 --- /dev/null +++ b/tests/forms/TreeDropdownFieldTest.php @@ -0,0 +1,97 @@ +'sub')); + $tree = $field->tree($request); + + $folder1 = $this->objFromFixture('Folder','folder1'); + $folder1Subfolder1 = $this->objFromFixture('Folder','folder1-subfolder1'); + + $parser = new CSSContentParser($tree); + $cssPath = 'ul.tree li#selector-TestTree-'.$folder1->ID.' li#selector-TestTree-'.$folder1Subfolder1->ID.' a span.item'; + $firstResult = $parser->getBySelector($cssPath); + $this->assertEquals( + (string)$firstResult[0], + $folder1Subfolder1->Name, + 'FileTest-folder1-subfolder1 is found, nested under folder1' + ); + + $subfolder = $this->objFromFixture('Folder','subfolder'); + $cssPath = 'ul.tree li#selector-TestTree-'.$subfolder->ID.' a span.item'; + $secondResult = $parser->getBySelector($cssPath); + $this->assertEquals( + (string)$secondResult[0], + $subfolder->Name, + 'FileTest-subfolder is found at root level' + ); + + // other folders which don't contain the keyword 'sub' are not returned in search results + $folder2 = $this->objFromFixture('Folder','folder2'); + $cssPath = 'ul.tree li#selector-TestTree-'.$folder2->ID.' a span.item'; + $noResult = $parser->getBySelector($cssPath); + $this->assertEquals( + $noResult, + array(), + 'FileTest-folder2 is not found' + ); + + $field = new TreeDropdownField('TestTree', 'Test tree', 'File'); + + // case insensitive search against keyword 'sub' for files + $request = new SS_HTTPRequest('GET','url',array('search'=>'sub')); + $tree = $field->tree($request); + + $parser = new CSSContentParser($tree); + + // Even if we used File as the source object, folders are still returned because Folder is a File + $cssPath = 'ul.tree li#selector-TestTree-'.$folder1->ID.' li#selector-TestTree-'.$folder1Subfolder1->ID.' a span.item'; + $firstResult = $parser->getBySelector($cssPath); + $this->assertEquals( + (string)$firstResult[0], + $folder1Subfolder1->Name, + 'FileTest-folder1-subfolder1 is found, nested under folder1' + ); + + // Looking for two files with 'sub' in their name, both under the same folder + $file1 = $this->objFromFixture('File','subfolderfile1'); + $file2 = $this->objFromFixture('File','subfolderfile2'); + $cssPath = 'ul.tree li#selector-TestTree-'.$subfolder->ID.' li#selector-TestTree-'.$file1->ID.' a'; + $firstResult = $parser->getBySelector($cssPath); + $this->assertEquals( + (string)$firstResult[0], + $file1->Name, + 'TestFile1InSubfolder is found nested under subfolder' + ); + + $cssPath = 'ul.tree li#selector-TestTree-'.$subfolder->ID.' li#selector-TestTree-'.$file2->ID.' a'; + $secondResult = $parser->getBySelector($cssPath); + $this->assertEquals( + (string)$secondResult[0], + $file2->Name, + 'TestFile2InSubfolder is found nested under subfolder' + ); + + // other files which don't include 'sub' are not returned in search results + $file3 = $this->objFromFixture('File','asdf'); + $cssPath = 'ul.tree li#selector-TestTree-'.$file3->ID; + $noResult = $parser->getBySelector($cssPath); + $this->assertEquals( + $noResult, + array(), + 'FileTest.txt is not found' + ); + } + +} + \ No newline at end of file diff --git a/tests/forms/TreeDropdownFieldTest.yml b/tests/forms/TreeDropdownFieldTest.yml new file mode 100644 index 000000000..0e77af985 --- /dev/null +++ b/tests/forms/TreeDropdownFieldTest.yml @@ -0,0 +1,25 @@ +Folder: + subfolder: + Name: FileTest-subfolder + folder1: + Name: FileTest-folder1 + folder2: + Name: FileTest-folder2 + folder1-subfolder1: + Name: FileTest-folder1-subfolder1 + ParentID: =>Folder.folder1 +File: + asdf: + Filename: assets/FileTest.txt + subfolderfile1: + Filename: assets/FileTest-subfolder/TestFile1InSubfolder.txt + Name: TestFile1InSubfolder + ParentID: =>Folder.subfolder + subfolderfile2: + Filename: assets/FileTest-subfolder/TestFile2InSubfolder.txt + Name: TestFile2InSubfolder + ParentID: =>Folder.subfolder + file1-folder1: + Filename: assets/FileTest-folder1/File1.txt + Name: File1.txt + ParentID: =>Folder.folder1 \ No newline at end of file From f9d493dff568b76db7f50c206a586ee80a92d0b5 Mon Sep 17 00:00:00 2001 From: Jean-Fabien Barrois Date: Mon, 2 Mar 2015 09:55:29 +1300 Subject: [PATCH 2/2] BUGFIX Fixes case insensitive search for postgres databases --- forms/TreeDropdownField.php | 16 +++++++--------- tests/forms/TreeDropdownFieldTest.php | 24 +++++++++++++++++------- 2 files changed, 24 insertions(+), 16 deletions(-) diff --git a/forms/TreeDropdownField.php b/forms/TreeDropdownField.php index 35d079e1a..3678df457 100644 --- a/forms/TreeDropdownField.php +++ b/forms/TreeDropdownField.php @@ -431,27 +431,25 @@ class TreeDropdownField extends FormField { $res = call_user_func($this->searchCallback, $this->sourceObject, $this->labelField, $this->search); } else { $sourceObject = $this->sourceObject; - $wheres = array(); + $filters = array(); if(singleton($sourceObject)->hasDatabaseField($this->labelField)) { - $wheres[] = "\"$this->labelField\" LIKE '%$this->search%'"; + $filters["{$this->labelField}:PartialMatch"] = $this->search; } else { if(singleton($sourceObject)->hasDatabaseField('Title')) { - $wheres[] = "\"Title\" LIKE '%$this->search%'"; + $filters["Title:PartialMatch"] = $this->search; } if(singleton($sourceObject)->hasDatabaseField('Name')) { - $wheres[] = "\"Name\" LIKE '%$this->search%'"; + $filters["Name:PartialMatch"] = $this->search; } - } - - if(!$wheres) { + } + if(empty($filters)) { throw new InvalidArgumentException(sprintf( 'Cannot query by %s.%s, not a valid database column', $sourceObject, $this->labelField )); } - - $res = DataObject::get($this->sourceObject, implode(' OR ', $wheres)); + $res = DataObject::get($this->sourceObject)->filterAny($filters); } if( $res ) { diff --git a/tests/forms/TreeDropdownFieldTest.php b/tests/forms/TreeDropdownFieldTest.php index 5e1f20c75..125bee465 100644 --- a/tests/forms/TreeDropdownFieldTest.php +++ b/tests/forms/TreeDropdownFieldTest.php @@ -24,7 +24,7 @@ class TreeDropdownFieldTest extends SapphireTest { $this->assertEquals( (string)$firstResult[0], $folder1Subfolder1->Name, - 'FileTest-folder1-subfolder1 is found, nested under folder1' + $folder1Subfolder1->Name.' is found, nested under '.$folder1->Name ); $subfolder = $this->objFromFixture('Folder','subfolder'); @@ -33,7 +33,7 @@ class TreeDropdownFieldTest extends SapphireTest { $this->assertEquals( (string)$secondResult[0], $subfolder->Name, - 'FileTest-subfolder is found at root level' + $subfolder->Name.' is found at root level' ); // other folders which don't contain the keyword 'sub' are not returned in search results @@ -43,7 +43,7 @@ class TreeDropdownFieldTest extends SapphireTest { $this->assertEquals( $noResult, array(), - 'FileTest-folder2 is not found' + $folder2.' is not found' ); $field = new TreeDropdownField('TestTree', 'Test tree', 'File'); @@ -60,7 +60,7 @@ class TreeDropdownFieldTest extends SapphireTest { $this->assertEquals( (string)$firstResult[0], $folder1Subfolder1->Name, - 'FileTest-folder1-subfolder1 is found, nested under folder1' + $folder1Subfolder1->Name.' is found, nested under '.$folder1->Name ); // Looking for two files with 'sub' in their name, both under the same folder @@ -68,18 +68,28 @@ class TreeDropdownFieldTest extends SapphireTest { $file2 = $this->objFromFixture('File','subfolderfile2'); $cssPath = 'ul.tree li#selector-TestTree-'.$subfolder->ID.' li#selector-TestTree-'.$file1->ID.' a'; $firstResult = $parser->getBySelector($cssPath); + $this->assertGreaterThan( + 0, + count($firstResult), + $file1->Name.' with ID '.$file1->ID.' is in search results' + ); $this->assertEquals( (string)$firstResult[0], $file1->Name, - 'TestFile1InSubfolder is found nested under subfolder' + $file1->Name.' is found nested under '.$subfolder->Name ); $cssPath = 'ul.tree li#selector-TestTree-'.$subfolder->ID.' li#selector-TestTree-'.$file2->ID.' a'; $secondResult = $parser->getBySelector($cssPath); + $this->assertGreaterThan( + 0, + count($secondResult), + $file2->Name.' with ID '.$file2->ID.' is in search results' + ); $this->assertEquals( (string)$secondResult[0], $file2->Name, - 'TestFile2InSubfolder is found nested under subfolder' + $file2->Name.' is found nested under '.$subfolder->Name ); // other files which don't include 'sub' are not returned in search results @@ -89,7 +99,7 @@ class TreeDropdownFieldTest extends SapphireTest { $this->assertEquals( $noResult, array(), - 'FileTest.txt is not found' + $file3->Name.' is not found' ); }