Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Search Filter Split, Use Same Controller #2454

Merged
merged 36 commits into from
Feb 24, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
49f7c92
Introduce extensible filterer system, make ListUsersController and Li…
askvortsov1 Nov 13, 2020
f76de0b
Add filter counterparts to builtin gambits
askvortsov1 Nov 14, 2020
7c99276
Properly register filters, support negation (passed into filter inter…
askvortsov1 Nov 14, 2020
8fd3f11
Add tests for all user filters and gambits
askvortsov1 Nov 14, 2020
34594fc
Fix created filter
askvortsov1 Nov 14, 2020
619913c
Add tests for all discussion filters and gambits
askvortsov1 Nov 14, 2020
dd61e44
Apply fixes from StyleCI
askvortsov1 Nov 14, 2020
44e7431
Fix failing tests
askvortsov1 Jan 19, 2021
f644c69
Apply fixes from StyleCI
askvortsov1 Jan 19, 2021
7b409cb
Add types
askvortsov1 Jan 28, 2021
e970282
Expect eloquent builder
askvortsov1 Feb 1, 2021
6ccd7a5
Merge gambits into "FilterGambit"
askvortsov1 Feb 1, 2021
a8c9ae8
Use Arr::get in CreatedFilterGambit
askvortsov1 Feb 1, 2021
837253d
Inject filters and filter mutators instead of static
askvortsov1 Feb 1, 2021
60a1a2b
Apply fixes from StyleCI
askvortsov1 Feb 1, 2021
94823ba
Per-resource filter classes
askvortsov1 Feb 10, 2021
8703956
Apply fixes from StyleCI
askvortsov1 Feb 10, 2021
66976fc
List controllers no longer need repositories
askvortsov1 Feb 10, 2021
6b96457
Add missing imports
askvortsov1 Feb 10, 2021
6f27690
Move gambit pattern to method instead of attribute for consistency
askvortsov1 Feb 13, 2021
2838f11
Combine search and filter criteria classes
askvortsov1 Feb 13, 2021
ad38f79
Apply fixes from StyleCI
askvortsov1 Feb 13, 2021
9fcc7cd
Use null coalesce instead of shorthand ternary
askvortsov1 Feb 17, 2021
70ebed2
Replace WrappedFilter and AbstractSearch with FilterState and SearchS…
askvortsov1 Feb 17, 2021
a217968
Apply fixes from StyleCI
askvortsov1 Feb 17, 2021
41835a5
Remove unnecessary load
askvortsov1 Feb 17, 2021
40da823
Rename $load to $include
askvortsov1 Feb 17, 2021
2b5b14d
Typehint AbstractSearcher return value
askvortsov1 Feb 17, 2021
ba3994b
Move $include out of searching / filtering
askvortsov1 Feb 17, 2021
3d0d931
Fix stupid mistakes
askvortsov1 Feb 17, 2021
947e5da
More fixes for stupid stuff, also temporarily loosen interface to avo…
askvortsov1 Feb 17, 2021
993fbed
Apply fixes from StyleCI
askvortsov1 Feb 17, 2021
ff4f227
Don't pass unused $include to filterer/searcher
askvortsov1 Feb 17, 2021
5fb3f29
Remove includes param from AbstractSearcher
askvortsov1 Feb 17, 2021
5a4bc5a
Add filter mutator params to docblock
askvortsov1 Feb 17, 2021
97ac81a
Capitalization fix
askvortsov1 Feb 18, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 24 additions & 9 deletions src/Api/Controller/ListDiscussionsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@

use Flarum\Api\Serializer\DiscussionSerializer;
use Flarum\Discussion\Discussion;
use Flarum\Discussion\Filter\DiscussionFilterer;
use Flarum\Discussion\Search\DiscussionSearcher;
use Flarum\Http\UrlGenerator;
use Flarum\Search\SearchCriteria;
use Illuminate\Support\Arr;
use Psr\Http\Message\ServerRequestInterface;
use Tobscure\JsonApi\Document;

Expand Down Expand Up @@ -43,11 +43,21 @@ class ListDiscussionsController extends AbstractListController
'lastPost'
];

/**
* {@inheritDoc}
*/
public $sort = ['lastPostedAt' => 'desc'];

/**
* {@inheritdoc}
*/
public $sortFields = ['lastPostedAt', 'commentCount', 'createdAt'];

/**
* @var DiscussionFilterer
*/
protected $filterer;

/**
* @var DiscussionSearcher
*/
Expand All @@ -59,11 +69,13 @@ class ListDiscussionsController extends AbstractListController
protected $url;

/**
* @param DiscussionFilterer $filterer
* @param DiscussionSearcher $searcher
* @param UrlGenerator $url
*/
public function __construct(DiscussionSearcher $searcher, UrlGenerator $url)
public function __construct(DiscussionFilterer $filterer, DiscussionSearcher $searcher, UrlGenerator $url)
{
$this->filterer = $filterer;
$this->searcher = $searcher;
$this->url = $url;
}
Expand All @@ -74,16 +86,19 @@ public function __construct(DiscussionSearcher $searcher, UrlGenerator $url)
protected function data(ServerRequestInterface $request, Document $document)
{
$actor = $request->getAttribute('actor');
$query = Arr::get($this->extractFilter($request), 'q');
$filters = $this->extractFilter($request);
$sort = $this->extractSort($request);

$criteria = new SearchCriteria($actor, $query, $sort);

$limit = $this->extractLimit($request);
$offset = $this->extractOffset($request);
$load = array_merge($this->extractInclude($request), ['state']);
$include = array_merge($this->extractInclude($request), ['state']);

$results = $this->searcher->search($criteria, $limit, $offset);
$criteria = new SearchCriteria($actor, $filters, $sort);
if (array_key_exists('q', $filters)) {
$results = $this->searcher->search($criteria, $limit, $offset);
} else {
$results = $this->filterer->filter($criteria, $limit, $offset);
}

$document->addPaginationLinks(
$this->url->to('api')->route('discussions.index'),
Expand All @@ -95,9 +110,9 @@ protected function data(ServerRequestInterface $request, Document $document)

Discussion::setStateUser($actor);

$results = $results->getResults()->load($load);
$results = $results->getResults()->load($include);

if ($relations = array_intersect($load, ['firstPost', 'lastPost'])) {
if ($relations = array_intersect($include, ['firstPost', 'lastPost'])) {
foreach ($results as $discussion) {
foreach ($relations as $relation) {
if ($discussion->$relation) {
Expand Down
26 changes: 18 additions & 8 deletions src/Api/Controller/ListUsersController.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@
use Flarum\Api\Serializer\UserSerializer;
use Flarum\Http\UrlGenerator;
use Flarum\Search\SearchCriteria;
use Flarum\User\Filter\UserFilterer;
use Flarum\User\Search\UserSearcher;
use Illuminate\Support\Arr;
use Psr\Http\Message\ServerRequestInterface;
use Tobscure\JsonApi\Document;

Expand All @@ -40,6 +40,11 @@ class ListUsersController extends AbstractListController
'joinedAt'
];

/**
* @var UserFilterer
*/
protected $filterer;

/**
* @var UserSearcher
*/
Expand All @@ -51,11 +56,13 @@ class ListUsersController extends AbstractListController
protected $url;

/**
* @param UserFilterer $filterer
* @param UserSearcher $searcher
* @param UrlGenerator $url
*/
public function __construct(UserSearcher $searcher, UrlGenerator $url)
public function __construct(UserFilterer $filterer, UserSearcher $searcher, UrlGenerator $url)
{
$this->filterer = $filterer;
$this->searcher = $searcher;
$this->url = $url;
}
Expand All @@ -69,16 +76,19 @@ protected function data(ServerRequestInterface $request, Document $document)

$actor->assertCan('viewUserList');

$query = Arr::get($this->extractFilter($request), 'q');
$filters = $this->extractFilter($request);
$sort = $this->extractSort($request);

$criteria = new SearchCriteria($actor, $query, $sort);

$limit = $this->extractLimit($request);
$offset = $this->extractOffset($request);
$load = $this->extractInclude($request);
$include = $this->extractInclude($request);

$results = $this->searcher->search($criteria, $limit, $offset, $load);
$criteria = new SearchCriteria($actor, $filters, $sort);
if (array_key_exists('q', $filters)) {
$results = $this->searcher->search($criteria, $limit, $offset);
} else {
$results = $this->filterer->filter($criteria, $limit, $offset);
}

$document->addPaginationLinks(
$this->url->to('api')->route('users.index'),
Expand All @@ -88,6 +98,6 @@ protected function data(ServerRequestInterface $request, Document $document)
$results->areMoreResults() ? null : 0
);

return $results->getResults();
return $results->getResults()->load($include);
}
}
8 changes: 4 additions & 4 deletions src/Discussion/Event/Searching.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,16 @@

namespace Flarum\Discussion\Event;

use Flarum\Discussion\Search\DiscussionSearch;
use Flarum\Search\SearchCriteria;
use Flarum\Search\SearchState;

/**
* @deprecated beta 16, remove beta 17
*/
class Searching
{
/**
* @var DiscussionSearch
* @var SearchState
*/
public $search;

Expand All @@ -28,10 +28,10 @@ class Searching
public $criteria;

/**
* @param DiscussionSearch $search
* @param SearchState $search
* @param \Flarum\Search\SearchCriteria $criteria
*/
public function __construct(DiscussionSearch $search, SearchCriteria $criteria)
public function __construct(SearchState $search, SearchCriteria $criteria)
{
$this->search = $search;
$this->criteria = $criteria;
Expand Down
72 changes: 72 additions & 0 deletions src/Discussion/Filter/AuthorFilterGambit.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
<?php

/*
* This file is part of Flarum.
*
* For detailed copyright and license information, please view the
* LICENSE file that was distributed with this source code.
*/

namespace Flarum\Discussion\Filter;

use Flarum\Filter\FilterInterface;
use Flarum\Filter\FilterState;
use Flarum\Search\AbstractRegexGambit;
use Flarum\Search\SearchState;
use Flarum\User\UserRepository;
use Illuminate\Database\Query\Builder;

class AuthorFilterGambit extends AbstractRegexGambit implements FilterInterface
{
/**
* @var \Flarum\User\UserRepository
*/
protected $users;

/**
* @param \Flarum\User\UserRepository $users
*/
public function __construct(UserRepository $users)
{
$this->users = $users;
}

/**
* {@inheritdoc}
*/
public function getGambitPattern()
{
return 'author:(.+)';
}

/**
* {@inheritdoc}
*/
protected function conditions(SearchState $search, array $matches, $negate)
{
$this->constrain($search->getQuery(), $matches[1], $negate);
}

public function getFilterKey(): string
{
return 'author';
}

public function filter(FilterState $filterState, string $filterValue, bool $negate)
{
$this->constrain($filterState->getQuery(), $filterValue, $negate);
}

protected function constrain(Builder $query, $rawUsernames, $negate)
{
$usernames = trim($rawUsernames, '"');
$usernames = explode(',', $usernames);

$ids = [];
foreach ($usernames as $username) {
$ids[] = $this->users->getIdForUsername($username);
}

$query->whereIn('discussions.user_id', $ids, 'and', $negate);
}
}
61 changes: 61 additions & 0 deletions src/Discussion/Filter/CreatedFilterGambit.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
<?php

/*
* This file is part of Flarum.
*
* For detailed copyright and license information, please view the
* LICENSE file that was distributed with this source code.
*/

namespace Flarum\Discussion\Filter;

use Flarum\Filter\FilterInterface;
use Flarum\Filter\FilterState;
use Flarum\Search\AbstractRegexGambit;
use Flarum\Search\SearchState;
use Illuminate\Database\Query\Builder;
use Illuminate\Support\Arr;

class CreatedFilterGambit extends AbstractRegexGambit implements FilterInterface
{
/**
* {@inheritdoc}
*/
public function getGambitPattern()
{
return 'created:(\d{4}\-\d\d\-\d\d)(\.\.(\d{4}\-\d\d\-\d\d))?';
}

/**
* {@inheritdoc}
*/
protected function conditions(SearchState $search, array $matches, $negate)
{
$this->constrain($search->getQuery(), Arr::get($matches, 1), Arr::get($matches, 3), $negate);
}

public function getFilterKey(): string
{
return 'created';
}

public function filter(FilterState $filterState, string $filterValue, bool $negate)
{
preg_match('/^'.$this->getGambitPattern().'$/i', 'created:'.$filterValue, $matches);

$this->constrain($filterState->getQuery(), Arr::get($matches, 1), Arr::get($matches, 3), $negate);
}

public function constrain(Builder $query, ?string $firstDate, ?string $secondDate, $negate)
{
// If we've just been provided with a single YYYY-MM-DD date, then find
// discussions that were started on that exact date. But if we've been
// provided with a YYYY-MM-DD..YYYY-MM-DD range, then find discussions
// that were started during that period.
if (empty($secondDate)) {
$query->whereDate('created_at', $negate ? '!=' : '=', $firstDate);
} else {
$query->whereBetween('created_at', [$firstDate, $secondDate], 'and', $negate);
}
}
}
40 changes: 40 additions & 0 deletions src/Discussion/Filter/DiscussionFilterer.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
<?php

/*
* This file is part of Flarum.
*
* For detailed copyright and license information, please view the
* LICENSE file that was distributed with this source code.
*/

namespace Flarum\Discussion\Filter;

use Flarum\Discussion\DiscussionRepository;
use Flarum\Filter\AbstractFilterer;
use Flarum\User\User;
use Illuminate\Database\Eloquent\Builder;

class DiscussionFilterer extends AbstractFilterer
{
/**
* @var DiscussionRepository
*/
protected $discussions;

/**
* @param DiscussionRepository $discussions
* @param array $filters
* @param array $filterMutators
*/
public function __construct(DiscussionRepository $discussions, array $filters, array $filterMutators)
{
parent::__construct($filters, $filterMutators);

$this->discussions = $discussions;
}

protected function getQuery(User $actor): Builder
{
return $this->discussions->query()->select('discussions.*')->whereVisibleTo($actor);
}
}
Loading