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

Sort qualified beatmap listing according to qualified queue date #3154

Merged
merged 4 commits into from May 18, 2018

Conversation

3 participants
@notbakaneko
Contributor

notbakaneko commented May 9, 2018

fixes #3072

Maps that are not in the queue will be ordered below those that are, regardless of sort direction.

index mapping should be updated first so that the query doesn't die while the field doesn't exist
curl -XPUT -H "Content-Type: application/json" http://ES_HOST/beatmaps/_mapping/beatmaps -d '{ "properties": { "queued_at": { "type": "date" } } }'

@nanaya

nanaya approved these changes May 18, 2018

@@ -68,15 +68,15 @@ public function __construct(Request $request, ?User $user = null)
}
$sort = explode('_', $request['sort']);
$this->sort = static::normalizeSort(
$this->sort = $this->normalizeSort(

This comment has been minimized.

@nanaya

nanaya May 18, 2018

Collaborator

Not related with this pull request but this assignment and related functions seem a bit... interesting.

  • first it create sort with given params
  • and then the created sort is thrown out, replaced with another pretty much the same sort just the sort[0] mapped to correct field
  • it's put in array
  • the normalizeSort function loops the array which is always single element.
  • (kind of related to this pull request) goes to great length using array_unshift instead of just doing it before appending with provided sort
    • it prepends the return sort, potentially overriding any previous sorts (['artist', 'approved_date']['queued_at', 'artist', 'approved_date'])
    • except it doesn't matter because there's always only one sort

This comment has been minimized.

@notbakaneko

notbakaneko May 18, 2018

Contributor

probably should have added a comment there >_>

nanaya added some commits May 18, 2018

@nanaya nanaya merged commit 8b8e4ea into ppy:master May 18, 2018

2 checks passed

continuous-integration/styleci/pr The analysis has passed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@notbakaneko notbakaneko deleted the notbakaneko:feature/beatmap-search-qualified-expected-rank-sort branch Jun 29, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment