ENHANCEMENT Respecting SiteTree->canView() settings in results (see #3052). This causes wrong counts, bugreport in #3120

ENHANCEMENT Added $data parameter to SearchForm->getResults() to remove dependency on $_REQUEST for better testability
ENHANCEMENT Added SearchForm->numPerPage - this shouldn't need to be passed around methods on every call, its instance specific
ENHANCEMENT Added SearchFormTest
MINOR Documentation for SearchForm


git-svn-id: svn://svn.silverstripe.com/silverstripe/open/modules/sapphire/branches/2.3@66322 467b73ca-7a2a-4603-9d3b-597d59a354a9
This commit is contained in:
Ingo Schommer 2008-11-20 22:07:36 +00:00 committed by Sam Minnee
parent 5e3726ff43
commit e772a4c1d4
3 changed files with 238 additions and 13 deletions

View File

@ -9,8 +9,27 @@
*/
class SearchForm extends Form {
/**
* @var boolean $showInSearchTurnOn
* @deprecated 2.3 SiteTree->ShowInSearch should always be respected
*/
protected $showInSearchTurnOn;
/**
* @var int $numPerPage How many results are shown per page.
* Relies on pagination being implemented in the search results template.
*/
protected $numPerPage = 10;
/**
*
* @param Controller $controller
* @param string $name The name of the form (used in URL addressing)
* @param FieldSet $fields Optional, defaults to a single field named "Search". Search logic needs to be customized
* if fields are added to the form.
* @param FieldSet $actions Optional, defaults to a single field named "Go".
* @param boolean $showInSearchTurnOn DEPRECATED 2.3
*/
function __construct($controller, $name, $fields = null, $actions = null, $showInSearchTurnOn = true) {
$this->showInSearchTurnOn = $showInSearchTurnOn;
@ -53,10 +72,17 @@ class SearchForm extends Form {
/**
* Return dataObjectSet of the results using $_REQUEST to get info from form.
* Wraps around {@link searchEngine()}
* Wraps around {@link searchEngine()}.
*
* @param int $numPerPage DEPRECATED 2.3 Use SearchForm->numPerPage
* @param array $data Request data as an associative array. Should contain at least a key 'Search' with all searched keywords.
* @return DataObjectSet
*/
public function getResults($numPerPage = 10){
$keywords = $_REQUEST['Search'];
public function getResults($numPerPage = null, $data = null){
// legacy usage: $data was defaulting to $_REQUEST, parameter not passed in doc.silverstripe.com tutorials
if(!isset($data)) $data = $_REQUEST;
$keywords = $data['Search'];
$andProcessor = create_function('$matches','
return " +" . $matches[2] . " +" . $matches[4] . " ";
@ -73,14 +99,20 @@ class SearchForm extends Form {
$keywords = $this->addStarsToKeywords($keywords);
if(strpos($keywords, '"') !== false || strpos($keywords, '+') !== false || strpos($keywords, '-') !== false || strpos($keywords, '*') !== false) {
return $this->searchEngine($keywords, $numPerPage, "Relevance DESC", "", true);
$results = $this->searchEngine($keywords, $numPerPage, "Relevance DESC", "", true);
} else {
return $this->searchEngine($keywords, $numPerPage);
$sortBy = "Relevance DESC";
}
$results = $this->searchEngine($keywords, $numPerPage);
}
// filter by permission
if($results) foreach($results as $result) {
if(!$result->canView()) $results->remove($result);
}
return $results;
}
function addStarsToKeywords($keywords) {
protected function addStarsToKeywords($keywords) {
if(!trim($keywords)) return "";
// Add * to each keyword
$splitWords = split(" +" , trim($keywords));
@ -97,13 +129,18 @@ class SearchForm extends Form {
}
return implode(" ", $newWords);
}
/**
* The core search engine, used by this class and its subclasses to do fun stuff.
* Searches both SiteTree and File.
*
* @param string $keywords Keywords as a string.
*/
public function searchEngine($keywords, $numPerPage = 10, $sortBy = "Relevance DESC", $extraFilter = "", $booleanSearch = false, $alternativeFileFilter = "", $invertedMatch = false) {
public function searchEngine($keywords, $numPerPage = null, $sortBy = "Relevance DESC", $extraFilter = "", $booleanSearch = false, $alternativeFileFilter = "", $invertedMatch = false) {
if(!$numPerPage) $numPerPage = $this->numPerPage;
$fileFilter = '';
$keywords = addslashes($keywords);
@ -139,14 +176,14 @@ class SearchForm extends Form {
$baseClass = reset($queryContent->from);
// There's no need to do all that joining
$queryContent->from = array(str_replace('`','',$baseClass) => $baseClass);
$queryContent->select = array("ClassName","$baseClass.ID","ParentID","Title","URLSegment","Content","LastEdited","Created","_utf8'' AS Filename", "_utf8'' AS Name", "$relevanceContent AS Relevance");
$queryContent->select = array("ClassName","$baseClass.ID","ParentID","Title","URLSegment","Content","LastEdited","Created","_utf8'' AS Filename", "_utf8'' AS Name", "$relevanceContent AS Relevance", "CanViewType");
$queryContent->orderby = null;
$queryFiles = singleton('File')->extendedSQL($notMatch . $matchFile . $fileFilter, "");
$baseClass = reset($queryFiles->from);
// There's no need to do all that joining
$queryFiles->from = array(str_replace('`','',$baseClass) => $baseClass);
$queryFiles->select = array("ClassName","$baseClass.ID","_utf8'' AS ParentID","Title","_utf8'' AS URLSegment","Content","LastEdited","Created","Filename","Name","$relevanceFile AS Relevance");
$queryFiles->select = array("ClassName","$baseClass.ID","_utf8'' AS ParentID","Title","_utf8'' AS URLSegment","Content","LastEdited","Created","Filename","Name","$relevanceFile AS Relevance","NULL AS CanViewType");
$queryFiles->orderby = null;
$fullQuery = $queryContent->sql() . " UNION " . $queryFiles->sql() . " ORDER BY $sortBy LIMIT $limit";
@ -164,8 +201,17 @@ class SearchForm extends Form {
return $doSet;
}
public function getSearchQuery() {
return Convert::raw2xml($_REQUEST['Search']);
/**
* Get the search query for display in a "You searched for ..." sentence.
*
* @param array $data
* @return string
*/
public function getSearchQuery($data = null) {
// legacy usage: $data was defaulting to $_REQUEST, parameter not passed in doc.silverstripe.com tutorials
if(!isset($data)) $data = $_REQUEST;
return Convert::raw2xml($data['Search']);
}
}

View File

@ -0,0 +1,146 @@
<?php
/**
* @package sapphire
* @subpackage testing
*
* @todo Fix unpublished pages check in testPublishedPagesMatchedByTitle()
* @todo All tests run on unpublished pages at the moment, due to the searchform not distinguishing between them
*/
class SearchFormTest extends FunctionalTest {
static $fixture_file = 'sapphire/tests/search/SearchFormTest.yml';
protected $mockController;
function setUp() {
parent::setUp();
$holderPage = $this->objFromFixture('SiteTree', 'searchformholder');
$this->mockController = new ContentController($holderPage);
$this->mockController->setSession(new Session(Controller::curr()->getSession()));
$this->mockController->pushCurrent();
}
function tearDown() {
$this->mockController->popCurrent();
parent::tearDown();
}
function testPublishedPagesMatchedByTitle() {
$sf = new SearchForm($this->mockController, 'SearchForm');
$publishedPage = $this->objFromFixture('SiteTree', 'publicPublishedPage');
$publishedPage->publish('Stage', 'Live');
$results = $sf->getResults(null, array('Search'=>'publicPublishedPage'));
$this->assertContains(
$publishedPage->ID,
$results->column('ID'),
'Published pages are found by searchform'
);
}
/*
function testUnpublishedPagesNotIncluded() {
$sf = new SearchForm($this->mockController, 'SearchForm');
$results = $sf->getResults(null, array('Search'=>'publicUnpublishedPage'));
$unpublishedPage = $this->objFromFixture('SiteTree', 'publicUnpublishedPage');
$this->assertNotContains(
$unpublishedPage->ID,
$results->column('ID'),
'Unpublished pages are not found by searchform'
);
}
*/
function testPagesRestrictedToLoggedinUsersNotIncluded() {
$sf = new SearchForm($this->mockController, 'SearchForm');
$page = $this->objFromFixture('SiteTree', 'restrictedViewLoggedInUsers');
$results = $sf->getResults(null, array('Search'=>'restrictedViewLoggedInUsers'));
$this->assertNotContains(
$page->ID,
$results->column('ID'),
'Page with "Restrict to logged in users" doesnt show without valid login'
);
$member = $this->objFromFixture('Member', 'randomuser');
$member->logIn();
$results = $sf->getResults(null, array('Search'=>'restrictedViewLoggedInUsers'));
$this->assertContains(
$page->ID,
$results->column('ID'),
'Page with "Restrict to logged in users" shows if login is present'
);
$member->logOut();
}
function testPagesRestrictedToSpecificGroupNotIncluded() {
$sf = new SearchForm($this->mockController, 'SearchForm');
$page = $this->objFromFixture('SiteTree', 'restrictedViewOnlyWebsiteUsers');
$results = $sf->getResults(null, array('Search'=>'restrictedViewOnlyWebsiteUsers'));
$this->assertNotContains(
$page->ID,
$results->column('ID'),
'Page with "Restrict to these users" doesnt show without valid login'
);
$member = $this->objFromFixture('Member', 'randomuser');
$member->logIn();
$results = $sf->getResults(null, array('Search'=>'restrictedViewOnlyWebsiteUsers'));
$this->assertNotContains(
$page->ID,
$results->column('ID'),
'Page with "Restrict to these users" doesnt show if logged in user is not in the right group'
);
$member->logOut();
$member = $this->objFromFixture('Member', 'websiteuser');
$member->logIn();
$results = $sf->getResults(null, array('Search'=>'restrictedViewOnlyWebsiteUsers'));
$this->assertContains(
$page->ID,
$results->column('ID'),
'Page with "Restrict to these users" shows if user in this group is logged in'
);
$member->logOut();
}
function testInheritedRestrictedPagesNotInlucded() {
$sf = new SearchForm($this->mockController, 'SearchForm');
$page = $this->objFromFixture('SiteTree', 'inheritRestrictedView');
$results = $sf->getResults(null, array('Search'=>'inheritRestrictedView'));
$this->assertNotContains(
$page->ID,
$results->column('ID'),
'Page inheriting "Restrict to loggedin users" doesnt show without valid login'
);
$member = $this->objFromFixture('Member', 'websiteuser');
$member->logIn();
$results = $sf->getResults(null, array('Search'=>'inheritRestrictedView'));
$this->assertContains(
$page->ID,
$results->column('ID'),
'Page inheriting "Restrict to loggedin users" shows if user in this group is logged in'
);
$member->logOut();
}
function testDisabledShowInSearchFlagNotIncluded() {
$sf = new SearchForm($this->mockController, 'SearchForm');
$page = $this->objFromFixture('SiteTree', 'dontShowInSearchPage');
$results = $sf->getResults(null, array('Search'=>'dontShowInSearchPage'));
$this->assertNotContains(
$page->ID,
$results->column('ID'),
'Page with "Show in Search" disabled doesnt show'
);
}
}
?>

View File

@ -0,0 +1,33 @@
Group:
websiteusers:
Title: View certain restricted pages
Member:
randomuser:
Email: randomuser@test.com
Password: test
websiteuser:
Email: websiteuser@test.com
Password: test
Groups: =>Group.websiteusers
SiteTree:
searchformholder:
URLSegment: searchformholder
Title: searchformholder
publicPublishedPage:
Title: publicPublishedPage
publicUnpublishedPage:
Title: publicUnpublishedPage
restrictedViewLoggedInUsers:
CanViewType: LoggedInUsers
Title: restrictedViewLoggedInUsers
restrictedViewOnlyWebsiteUsers:
CanViewType: OnlyTheseUsers
ViewerGroups: =>Group.websiteusers
Title: restrictedViewOnlyWebsiteUsers
inheritRestrictedView:
CanViewType: Inherit
Parent: =>SiteTree.restrictedViewLoggedInUsers
Title: inheritRestrictedView
dontShowInSearchPage:
Title: dontShowInSearchPage
ShowInSearch: 0