Skip to content

Commit

Permalink
Fixes for faceting on the wrong base class (#156)
Browse files Browse the repository at this point in the history
Add baseclass to the tests

Doc updates

Comment out the unused method. It's for reference
  • Loading branch information
Firesphere authored and marczhermo committed Dec 7, 2019
1 parent 23bae0d commit e3bd450
Show file tree
Hide file tree
Showing 8 changed files with 72 additions and 20 deletions.
1 change: 1 addition & 0 deletions .circleci/TestIndexTwo.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ Firesphere\SolrSearch\Indexes\BaseIndex:
DefaultField: _text
FacetFields:
Firesphere\SolrSearch\Tests\TestObject:
BaseClass: SilverStripe\CMS\Model\SiteTree
Field: ID
Title: TestObject
Firesphere\SolrSearch\Services\SolrCoreService:
Expand Down
13 changes: 8 additions & 5 deletions docs/03-Usage.md
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ Available methods are:
| addSortField | Field to sort by | No | `$this->addSortField('Created');` |
| addCopyField | Add a special copy field, besides the default _text | No | `$this->addCopyField('myCopy', ['Fields', 'To', 'Copy']);` |
| addStoredField | Add a field to be stored specifically | No | `$this->addStoredField('LastEdited');` |
| addFacetField | Field to build faceting on | No | `$this->addFacetField(SiteTree::class, ['Title' => 'FacetObject', 'Field' => 'FacetObjectID']);` |
| addFacetField | Field to build faceting on | No | `$this->addFacetField(SiteTree::class, ['BaseClass' => SiteTree::class, 'Title' => 'FacetObject', 'Field' => 'FacetObjectID']);` |


### Using YML
Expand Down Expand Up @@ -106,6 +106,7 @@ Firesphere\SolrSearch\Indexes\BaseIndex:
DefaultField: _text
FacetFields:
Firesphere\SolrSearch\Tests\TestObject:
BaseClass: SilverStripe\CMS\Model\SiteTree
Field: ID
Title: TestObject
Expand All @@ -121,12 +122,14 @@ You could also use PHP, just like it was, to set the config. For readability how
```php
protected $facetFields = [
RelatedObject::class => [
'Field' => 'RelatedObjectID',
'Title' => 'RelationOne'
'BaseClass' => SiteTree::class,
'Field' => 'RelatedObjectID',
'Title' => 'RelationOne'
],
OtherRelatedObject::class => [
'Field' => 'OtherRelatedObjectID',
'Title' => 'RelationTwo'
'BaseClass' => SiteTree::class,
'Field' => 'OtherRelatedObjectID',
'Title' => 'RelationTwo'
]
];
```
Expand Down
2 changes: 1 addition & 1 deletion docs/template.html
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@
.content {
margin: 50px auto;
padding: 0 2em;
max-width: 80%;
max-width: 90%;
line-height: 1.6em;
}

Expand Down
42 changes: 42 additions & 0 deletions src/Extensions/DataObjectExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,12 @@
use SilverStripe\Core\Injector\Injector;
use SilverStripe\ORM\ArrayList;
use SilverStripe\ORM\DataExtension;
use SilverStripe\ORM\DataList;
use SilverStripe\ORM\DataObject;
use SilverStripe\ORM\ValidationException;
use SilverStripe\Security\InheritedPermissionsExtension;
use SilverStripe\Security\Member;
use SilverStripe\Security\Security;
use SilverStripe\SiteConfig\SiteConfig;
use SilverStripe\Versioned\Versioned;

Expand All @@ -39,6 +42,10 @@ class DataObjectExtension extends DataExtension
* @var SiteConfig Current siteconfig
*/
protected static $siteConfig;
/**
* @var ArrayList|Member[] List of all the members in the system
*/
protected static $memberList;

/**
* Push the item to solr if it is not versioned
Expand Down Expand Up @@ -285,4 +292,39 @@ protected function getGroupViewPermissions($owner): array

return $return;
}

/**
* Get permissions for viewing per member
* @todo fix the implementation
* @param DataObject $owner
* @return array
*
protected function getMemberViewPermissions(DataObject $owner)
{
if (!static::$memberList) {
static::$memberList = ArrayList::create(Member::get()->toArray());
}
$currentUser = Security::getCurrentUser();
Security::setCurrentUser(null);
if ($owner->canView(null)) {
Security::setCurrentUser($currentUser);
return ['null'];
}
$return = ['false'];
foreach (static::$memberList as $member) {
if ($owner->canView($member)) {
$return[] = $member->ID;
}
}
if (!$owner->hasExtension(InheritedPermissionsExtension::class)) {
static::$cachedClasses[$owner->ClassName] = $return;
}
return $return;
}
/**/
}
1 change: 1 addition & 0 deletions src/Indexes/BaseIndex.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
use SilverStripe\Core\Extensible;
use SilverStripe\Core\Injector\Injectable;
use SilverStripe\Core\Injector\Injector;
use SilverStripe\Dev\Debug;
use SilverStripe\Dev\Deprecation;
use SilverStripe\ORM\ValidationException;
use SilverStripe\View\ArrayData;
Expand Down
4 changes: 2 additions & 2 deletions src/Traits/QueryComponentTraits/QueryComponentFacetTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ protected function buildFacets(): void
$facets = $this->clientQuery->getFacetSet();
// Facets should be set from the index configuration
foreach ($this->index->getFacetFields() as $class => $config) {
$shortClass = getShortFieldName($class);
$shortClass = getShortFieldName($config['BaseClass']);
$field = $shortClass . '_' . str_replace('.', '_', $config['Field']);
/** @var Field $facet */
$facet = $facets->createFacetField('facet-' . $config['Title']);
Expand All @@ -61,7 +61,7 @@ protected function buildFacetQuery()
) {
// @todo add unit tests for this bit. It's crucial but untested
$filter = is_array($filter) ? $filter : [$filter];
$shortClass = getShortFieldName($class);
$shortClass = getShortFieldName($config['BaseClass']);
$field = $shortClass . '_' . str_replace('.', '_', $config['Field']);
$criteria = Criteria::where($field)->in($filter);
$this->clientQuery
Expand Down
10 changes: 6 additions & 4 deletions tests/mocks/TestIndex.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,9 @@ class TestIndex extends BaseIndex implements TestOnly
{
protected $facetFields = [
SiteTree::class => [
'Title' => 'Parent',
'Field' => 'ParentID',
'BaseClass' => SiteTree::class,
'Title' => 'Parent',
'Field' => 'ParentID',
],
];

Expand All @@ -25,8 +26,9 @@ public function init(): void
$this->addFilterField('Created');
$this->addSortField('Created');
$this->addFacetField(SiteTree::class, [
'Title' => 'Parent',
'Field' => 'ParentID',
'BaseClass' => SiteTree::class,
'Title' => 'Parent',
'Field' => 'ParentID',
]);
}

Expand Down
19 changes: 11 additions & 8 deletions tests/unit/BaseIndexTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
use SilverStripe\Control\NullHTTPRequest;
use SilverStripe\Core\Config\Config;
use SilverStripe\Core\Injector\Injector;
use SilverStripe\Dev\Debug;
use SilverStripe\Dev\Deprecation;
use SilverStripe\Dev\SapphireTest;
use SilverStripe\ORM\ArrayList;
Expand Down Expand Up @@ -146,8 +145,9 @@ public function testFacetFields()
$task->run($request);
$facets = $index->getFacetFields();
$this->assertEquals([
'Title' => 'Parent',
'Field' => 'ParentID',
'BaseClass' => SiteTree::class,
'Title' => 'Parent',
'Field' => 'ParentID',
], $facets[SiteTree::class]);
$query = new BaseQuery();
$query->addTerm('Test');
Expand Down Expand Up @@ -338,16 +338,19 @@ public function testDoRetry()

public function testSetFacets()
{
$this->index->addFacetField(Page::class, ['Title' => 'Title', 'Field' => 'Content']);
$this->index->addFacetField(Page::class,
['BaseClass' => Page::class, 'Title' => 'Title', 'Field' => 'Content']);

$expected = [
SiteTree::class => [
'Title' => 'Parent',
'Field' => 'ParentID',
'BaseClass' => SiteTree::class,
'Title' => 'Parent',
'Field' => 'ParentID',
],
Page::class => [
'Title' => 'Title',
'Field' => 'Content',
'BaseClass' => Page::class,
'Title' => 'Title',
'Field' => 'Content',
],
];
$this->assertEquals($expected, $this->index->getFacetFields());
Expand Down

0 comments on commit e3bd450

Please sign in to comment.