Fix relevance sort #2773
Fix relevance sort #2773
Conversation
[ci skip] [skip ci]
[ci skip] [skip ci]
| @@ -85,12 +85,13 @@ protected function data(ServerRequestInterface $request, Document $document) | |||
|
|
|||
| $filters = $this->extractFilter($request); | |||
| $sort = $this->extractSort($request); | |||
| $sortIsDefault = $this->sortIsDefault($request); | |||
There was a problem hiding this comment.
can we rename sortIsDefault to isDefaultSorting or maybe isSortingByDefault.
There was a problem hiding this comment.
Mixed feelings about this one. Any use of sorting describes whether or not any sort is being applied. But this variable should describe whether the sort currently being used (whatever that is, could be no sort at all) is the controller's default sort.
Alternatively, we could use userRequestedSort or userSpecifiedSort and invert everything, but that sounds very verbose.
There was a problem hiding this comment.
True, we only want to know if we're falling back to the default sort.
Though I was more looking for just a isSomething naming, since that's the usual convention for boolean variables/methods where possible. But isDefaultSort might be weird.
Let's keep it as is then!
This was introduced in flarum/framework#2773, and allows us to more cleanly determine whether the sort requested is the default one, while taking extension modifications into account.
This was introduced in flarum/framework#2773, and allows us to more cleanly determine whether the sort requested is the default one, while taking extension modifications into account.
This was introduced in flarum/framework#2773, and allows us to more cleanly determine whether the sort requested is the default one, while taking extension modifications into account.
Fixes #2772
Changes proposed in this pull request:
QueryCriteriathat determines whether the sort provided is the controller's default sorttrueiffsortnot in query params. Default it to falseConfirmed
composer test).