github-mirror/framework
2
0
Fork 0
mirror of https://github.com/flarum/framework.git synced 2025-02-06 14:51:01 +08:00
Code Issues Actions Packages Projects Releases Wiki Activity

Fix relevance sort (#2773)

Browse Source 
- Adds a field to QueryCriteria that determines whether the sort provided is the controller's default sort
- Set this field to true iff sort not in query params. Default it to false
- Override $sort if a new default sort has been set on search state, and the param is true.
- Add tests!
This commit is contained in:
Alexander Skvortsov 2021-04-11 22:21:56 -04:00 committed by GitHub
parent 598bb94657
commit 0fb3548f46
9 changed files with 42 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
framework/core/src/Api/Controller/AbstractSerializeController.php
View File

@ -12,6 +12,7 @@ namespace Flarum\Api\Controller;
use Flarum\Api\JsonApiResponse;
use Illuminate\Contracts\Container\Container;
use Illuminate\Database\Eloquent\Collection;
use Illuminate\Support\Arr;
use Illuminate\Support\Str;
use Psr\Http\Message\ResponseInterface;
use Psr\Http\Message\ServerRequestInterface;
@ -255,6 +256,11 @@ abstract class AbstractSerializeController implements RequestHandlerInterface
return new Parameters($request->getQueryParams());
}
protected function sortIsDefault(ServerRequestInterface $request): bool
{
return ! Arr::get($request->getQueryParams(), 'sort');
}
/**
* Set the serializer that will serialize data for the endpoint.
*

3
framework/core/src/Api/Controller/ListDiscussionsController.php
View File

@ -89,12 +89,13 @@ class ListDiscussionsController extends AbstractListController
$actor = RequestUtil::getActor($request);
$filters = $this->extractFilter($request);
$sort = $this->extractSort($request);
$sortIsDefault = $this->sortIsDefault($request);
$limit = $this->extractLimit($request);
$offset = $this->extractOffset($request);
$include = array_merge($this->extractInclude($request), ['state']);
$criteria = new QueryCriteria($actor, $filters, $sort);
$criteria = new QueryCriteria($actor, $filters, $sort, $sortIsDefault);
if (array_key_exists('q', $filters)) {
$results = $this->searcher->search($criteria, $limit, $offset);
} else {

3
framework/core/src/Api/Controller/ListPostsController.php
View File

@ -79,12 +79,13 @@ class ListPostsController extends AbstractListController
$filters = $this->extractFilter($request);
$sort = $this->extractSort($request);
$sortIsDefault = $this->sortIsDefault($request);
$limit = $this->extractLimit($request);
$offset = $this->extractOffset($request);
$include = $this->extractInclude($request);
$results = $this->filterer->filter(new QueryCriteria($actor, $filters, $sort), $limit, $offset);
$results = $this->filterer->filter(new QueryCriteria($actor, $filters, $sort, $sortIsDefault), $limit, $offset);
$document->addPaginationLinks(
$this->url->to('api')->route('posts.index'),

3
framework/core/src/Api/Controller/ListUsersController.php
View File

@ -86,12 +86,13 @@ class ListUsersController extends AbstractListController
$filters = $this->extractFilter($request);
$sort = $this->extractSort($request);
$sortIsDefault = $this->sortIsDefault($request);
$limit = $this->extractLimit($request);
$offset = $this->extractOffset($request);
$include = $this->extractInclude($request);
$criteria = new QueryCriteria($actor, $filters, $sort);
$criteria = new QueryCriteria($actor, $filters, $sort, $sortIsDefault);
if (array_key_exists('q', $filters)) {
$results = $this->searcher->search($criteria, $limit, $offset);
} else {

2
framework/core/src/Filter/AbstractFilterer.php
View File

@ -65,7 +65,7 @@ abstract class AbstractFilterer
}
}
$this->applySort($filterState, $criteria->sort);
$this->applySort($filterState, $criteria->sort, $criteria->sortIsDefault);
$this->applyOffset($filterState, $offset);
$this->applyLimit($filterState, $limit + 1);

12
framework/core/src/Query/ApplyQueryParametersTrait.php
View File

@ -11,6 +11,9 @@ namespace Flarum\Query;
use Illuminate\Support\Str;
/**
* @internal
*/
trait ApplyQueryParametersTrait
{
/**
@ -18,15 +21,18 @@ trait ApplyQueryParametersTrait
*
* @param AbstractQueryState $query
* @param array $sort
* @param bool $sortIsDefault
*/
protected function applySort(AbstractQueryState $query, array $sort = null)
protected function applySort(AbstractQueryState $query, array $sort = null, bool $sortIsDefault = false)
{
$sort = $sort ?: $query->getDefaultSort();
if ($sortIsDefault && ! empty($query->getDefaultSort())) {
$sort = $query->getDefaultSort();
}
if (is_callable($sort)) {
$sort($query->getQuery());
} else {
foreach ($sort as $field => $order) {
foreach ((array) $sort as $field => $order) {
if (is_array($order)) {
foreach ($order as $value) {
$query->getQuery()->orderByRaw(Str::snake($field).' != ?', [$value]);

11
framework/core/src/Query/QueryCriteria.php
View File

@ -41,6 +41,14 @@ class QueryCriteria
*/
public $sort;
/**
* Is the sort for this request the default sort from the controller?
* If false, the current request specifies a sort.
*
* @var bool
*/
public $sortIsDefault;
/**
* @param User $actor The user performing the query.
* @param array $query The query params.
@ -48,10 +56,11 @@ class QueryCriteria
* key, and the order is the value. The order may be 'asc', 'desc', or
* an array of IDs to order by.
*/
public function __construct(User $actor, $query, array $sort = null)
public function __construct(User $actor, $query, array $sort = null, bool $sortIsDefault = false)
{
$this->actor = $actor;
$this->query = $query;
$this->sort = $sort;
$this->sortIsDefault = $sortIsDefault;
}
}

2
framework/core/src/Search/AbstractSearcher.php
View File

@ -54,7 +54,7 @@ abstract class AbstractSearcher
$search = new SearchState($query->getQuery(), $actor);
$this->gambits->apply($search, $criteria->query['q']);
$this->applySort($search, $criteria->sort);
$this->applySort($search, $criteria->sort, $criteria->sortIsDefault);
$this->applyOffset($search, $offset);
$this->applyLimit($search, $limit + 1);

17
framework/core/tests/integration/api/discussions/ListTestWithFulltextSearch.php → framework/core/tests/integration/api/discussions/ListWithFulltextSearchTest.php
View File

@ -13,7 +13,7 @@ use Carbon\Carbon;
use Flarum\Testing\integration\RetrievesAuthorizedUsers;
use Flarum\Testing\integration\TestCase;
class ListTest extends TestCase
class ListWithFulltextSearchTest extends TestCase
{
use RetrievesAuthorizedUsers;
@ -31,12 +31,15 @@ class ListTest extends TestCase
// We clean it up explcitly at the end.
$this->database()->table('discussions')->insert([
['id' => 1, 'title' => 'lightsail in title', 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 1, 'comment_count' => 1],
['id' => 2, 'title' => 'not in title', 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 1, 'comment_count' => 1],
['id' => 2, 'title' => 'not in title and older', 'created_at' => Carbon::createFromDate(2020, 01, 01)->toDateTimeString(), 'user_id' => 1, 'comment_count' => 1],
['id' => 3, 'title' => 'not in title either', 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 1, 'comment_count' => 1],
]);
$this->database()->table('posts')->insert([
['id' => 1, 'discussion_id' => 1, 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 1, 'type' => 'comment', 'content' => '<t><p>not in text</p></t>'],
['id' => 2, 'discussion_id' => 2, 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 1, 'type' => 'comment', 'content' => '<t><p>lightsail in text</p></t>'],
['id' => 3, 'discussion_id' => 2, 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 1, 'type' => 'comment', 'content' => '<t><p>another lightsail for discussion 2!</p></t>'],
['id' => 4, 'discussion_id' => 3, 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 1, 'type' => 'comment', 'content' => '<t><p>just one lightsail for discussion 3.</p></t>'],
]);
// We need to call these again, since we rolled back the transaction started by `::app()`.
@ -52,8 +55,8 @@ class ListTest extends TestCase
{
parent::tearDown();
$this->database()->table('discussions')->whereIn('id', [1, 2])->delete();
$this->database()->table('posts')->whereIn('id', [1, 2])->delete();
$this->database()->table('discussions')->whereIn('id', [1, 2, 3])->delete();
$this->database()->table('posts')->whereIn('id', [1, 2, 3, 4])->delete();
}
/**
@ -74,8 +77,7 @@ class ListTest extends TestCase
return $row['id'];
}, $data['data']);
// Order-independent comparison
$this->assertEquals(['3'], $ids, 'IDs do not match');
$this->assertEquals(['2', '3'], $ids, 'IDs do not match');
}
/**
@ -96,8 +98,7 @@ class ListTest extends TestCase
return $row['id'];
}, $data['data']);
// Order-independent comparison
$this->assertEquals(['3'], $ids, 'IDs do not match');
$this->assertEquals(['2', '3'], $ids, 'IDs do not match');
}
/**