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

Support multiple mappers on beatmap difficulties #11377

Open
wants to merge 92 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 79 commits
Commits
Show all changes
92 commits
Select commit Hold shift + click to select a range
55dbc21
support multiple owners on beatmap
notbakaneko Jul 2, 2024
c3ee82a
update phpdoc
notbakaneko Jul 5, 2024
72b80d7
allow array param
notbakaneko Jul 5, 2024
5c697f4
extract username input with lookup into component
notbakaneko Jul 8, 2024
c93753a
xhr error doesn't contain responseText when aborted
notbakaneko Jul 8, 2024
1195c44
initialize directly
notbakaneko Jul 10, 2024
43d9dbc
add option to ignore current user
notbakaneko Jul 10, 2024
79adc14
support updating to multiple beatmap owners
notbakaneko Jul 10, 2024
825a01a
show in events
notbakaneko Jul 11, 2024
498e3c7
only prepend for compatiblity if not already there.
notbakaneko Jul 11, 2024
2da23ff
wrong condition
notbakaneko Jul 12, 2024
5655d38
show multiple mappers
notbakaneko Jul 16, 2024
716957d
add mappers to beatmap for score
notbakaneko Jul 16, 2024
f9706cf
filtering mappers
notbakaneko Jul 16, 2024
e371792
should be id
notbakaneko Jul 17, 2024
0042a94
show multiple users on owner editor
notbakaneko Jul 17, 2024
93a51de
include avatar, also correct fields
notbakaneko Jul 17, 2024
fdaede5
component for multiple UserLink
notbakaneko Jul 17, 2024
696a102
add hasGuestMapper helper, remove unused imports
notbakaneko Jul 19, 2024
50b8c6d
update typings
notbakaneko Jul 19, 2024
fdbd894
handle enter key
notbakaneko Jul 19, 2024
926ee77
styling
notbakaneko Jul 19, 2024
eb4e738
custom render for valid usernames
notbakaneko Jul 22, 2024
ad768a5
handle removing users from list
notbakaneko Jul 22, 2024
6b9ab68
move key
notbakaneko Jul 22, 2024
5bb4a44
define minimum user properties
notbakaneko Jul 22, 2024
86dfd22
use own block style
notbakaneko Jul 25, 2024
581259a
separate mapper name component
notbakaneko Jul 25, 2024
6ec895d
initialize with users without looking up
notbakaneko Jul 25, 2024
187306d
just use UserJson so we don't have to handle UserCardBrick differently
notbakaneko Jul 26, 2024
0c84a6a
change input colour
notbakaneko Jul 26, 2024
c539574
lint
notbakaneko Jul 26, 2024
65ac60e
run in transaction
notbakaneko Jul 26, 2024
3732f20
move logic into function
notbakaneko Jul 26, 2024
2e25964
fix tests, make route post
notbakaneko Jul 26, 2024
9217224
fix import order
notbakaneko Jul 26, 2024
7f283ea
always force to array
notbakaneko Jul 26, 2024
edce318
move into class
notbakaneko Jul 29, 2024
bc706ef
move tests
notbakaneko Jul 29, 2024
736a853
move priv check
notbakaneko Jul 29, 2024
5e481aa
test call function directly instead of going through controller
notbakaneko Jul 29, 2024
9b807b8
use factory methods and expectExceptionCallable
notbakaneko Jul 29, 2024
b264688
test the mappers collection
notbakaneko Jul 29, 2024
4b91a83
remove controller tests
notbakaneko Jul 29, 2024
b3cb55a
lint
notbakaneko Jul 29, 2024
ecbd11a
need moderator permission to change owner of nominated map
notbakaneko Jul 29, 2024
e8d4d00
add dispatch assertions
notbakaneko Jul 30, 2024
1c64529
render user list
notbakaneko Jul 30, 2024
eacb7bc
include mappers related_users response
notbakaneko Jul 30, 2024
d7d1b0f
eager load users for compatibility
notbakaneko Jul 30, 2024
c629115
eagerload for userScore
notbakaneko Jul 30, 2024
405a091
xhrcollection not needed
notbakaneko Jul 30, 2024
7d4f3e2
move user lookup and add throttle
notbakaneko Jul 30, 2024
4f757d4
make input wider
notbakaneko Jul 30, 2024
78bff9f
update max-warnings
notbakaneko Jul 30, 2024
dcf1d1b
wrong size
notbakaneko Jul 30, 2024
58a28b4
show all mappers on info page
notbakaneko Jul 30, 2024
0d3d6f0
wrong namespace
notbakaneko Jul 30, 2024
bb48bf2
skip update if no change
notbakaneko Jul 31, 2024
9d7845b
fill leftover space with element
notbakaneko Jul 31, 2024
5d0faef
stop event from removing multiple users
notbakaneko Jul 31, 2024
4baa745
use InputContainer to handle error styling
notbakaneko Jul 31, 2024
9aa564e
less layout shifting when switching between edit modes
notbakaneko Jul 31, 2024
7e1e92f
maybe 80px is too big...
notbakaneko Jul 31, 2024
3c2129e
support error highlight without model
notbakaneko Jul 31, 2024
fa1e602
add command to copy mappers to new table
notbakaneko Aug 1, 2024
a70c617
use beatmap_owners to get guest mapper listing
notbakaneko Aug 1, 2024
a744a1f
skip users in no_profile group
notbakaneko Aug 1, 2024
eff4003
Add deleted or missing users into mappers response
notbakaneko Aug 1, 2024
2d19464
preserve ids of missing mappers; also freeze deleted user objects.
notbakaneko Aug 2, 2024
77807ac
reset showError state when cancelling
notbakaneko Aug 2, 2024
40ca43c
allow existing mappers to be preserved even if they don't exist anymore
notbakaneko Aug 2, 2024
31059d6
include all mappers in related_users
notbakaneko Aug 2, 2024
0a8891d
Merge branch 'master' into feature/beatmaps-multiple-mappers
notbakaneko Aug 2, 2024
da5cd9c
add test for updating with missing user
notbakaneko Aug 2, 2024
384d0b2
fix indentation
notbakaneko Aug 2, 2024
de8de24
also create BeatmapOwner
notbakaneko Aug 2, 2024
198e59a
unused relation
notbakaneko Aug 8, 2024
1f31a9d
Merge branch 'master' into feature/beatmaps-multiple-mappers
notbakaneko Aug 8, 2024
620c1f7
fix reset button disabled state
notbakaneko Sep 3, 2024
100f320
Merge branch 'master' into feature/beatmaps-multiple-mappers-revert-r…
notbakaneko Sep 12, 2024
de0841e
rename to owners
notbakaneko Sep 13, 2024
a24d80b
rename setOwner to setOwners
notbakaneko Sep 13, 2024
2a475b3
rename includes and json property to owners
notbakaneko Sep 13, 2024
d587edb
rename to beatmap-owner
notbakaneko Sep 13, 2024
2afa074
rename to hasGuestOwners
notbakaneko Sep 13, 2024
c7d6d02
rename css elass
notbakaneko Sep 13, 2024
9a63743
more renames
notbakaneko Sep 13, 2024
ef1721a
forgot to rename the other elements...
notbakaneko Sep 13, 2024
0de28c1
rename handler
notbakaneko Sep 13, 2024
0e306c3
Merge branch 'master' into feature/beatmaps-multiple-mappers
notbakaneko Sep 13, 2024
0601e19
lint fix
notbakaneko Sep 13, 2024
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
2 changes: 1 addition & 1 deletion .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ jobs:
- name: Install js dependencies
run: yarn --frozen-lockfile

- run: 'yarn lint --max-warnings 89 > /dev/null'
- run: 'yarn lint --max-warnings 88 > /dev/null'

- run: yarn pretty

Expand Down
31 changes: 31 additions & 0 deletions app/Console/Commands/BeatmapsMigrateOwners.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
<?php

// Copyright (c) ppy Pty Ltd <[email protected]>. Licensed under the GNU Affero General Public License v3.0.
// See the LICENCE file in the repository root for full licence text.

namespace App\Console\Commands;

use App\Models\Beatmap;
use Illuminate\Console\Command;

class BeatmapsMigrateOwners extends Command
{
protected $signature = 'beatmaps:migrate-owners';

protected $description = 'Migrates beatmap owners to new table.';

public function handle()
{
$progress = $this->output->createProgressBar();

Beatmap::chunkById(1000, function ($beatmaps) use ($progress) {
foreach ($beatmaps as $beatmap) {
$beatmap->beatmapOwners()->firstOrCreate(['user_id' => $beatmap->user_id]);
$progress->advance();
}
});

$progress->finish();
$this->line('');
}
}
1 change: 1 addition & 0 deletions app/Console/Kernel.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ class Kernel extends ConsoleKernel
Commands\ModdingRankCommand::class,

Commands\UserForumStatSyncCommand::class,
Commands\BeatmapsMigrateOwners::class,
Commands\BeatmapsetsHypeSyncCommand::class,
Commands\BeatmapsetNominationSyncCommand::class,

Expand Down
28 changes: 5 additions & 23 deletions app/Http/Controllers/BeatmapsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,12 @@

use App\Enums\Ruleset;
use App\Exceptions\InvariantException;
use App\Jobs\Notifications\BeatmapOwnerChange;
use App\Libraries\BeatmapDifficultyAttributes;
use App\Libraries\Score\BeatmapScores;
use App\Libraries\Score\UserRank;
use App\Libraries\Search\ScoreSearch;
use App\Libraries\Search\ScoreSearchParams;
use App\Models\Beatmap;
use App\Models\BeatmapsetEvent;
use App\Models\User;
use App\Transformers\BeatmapTransformer;
use App\Transformers\ScoreTransformer;
Expand All @@ -25,7 +23,7 @@
class BeatmapsController extends Controller
{
const DEFAULT_API_INCLUDES = ['beatmapset.ratings', 'failtimes', 'max_combo'];
const DEFAULT_SCORE_INCLUDES = ['user', 'user.country', 'user.cover'];
const DEFAULT_SCORE_INCLUDES = ['user', 'user.country', 'user.cover', 'beatmap.owners'];

public function __construct()
{
Expand Down Expand Up @@ -380,27 +378,11 @@ public function soloScores($id)

public function updateOwner($id)
{
$beatmap = Beatmap::findOrFail($id);
$currentUser = auth()->user();

priv_check('BeatmapUpdateOwner', $beatmap->beatmapset)->ensureCan();

$newUserId = get_int(request('beatmap.user_id'));
$newUserIds = get_arr(request('user_ids'), 'get_int');

$beatmap->getConnection()->transaction(function () use ($beatmap, $currentUser, $newUserId) {
$beatmap->setOwner($newUserId);

BeatmapsetEvent::log(BeatmapsetEvent::BEATMAP_OWNER_CHANGE, $currentUser, $beatmap->beatmapset, [
'beatmap_id' => $beatmap->getKey(),
'beatmap_version' => $beatmap->version,
'new_user_id' => $beatmap->user_id,
'new_user_username' => $beatmap->user->username,
])->saveOrExplode();
});
$beatmap = Beatmap::findOrFail($id);

if ($beatmap->user_id !== $currentUser->getKey()) {
(new BeatmapOwnerChange($beatmap, $currentUser))->dispatch();
}
$beatmap->setOwner($newUserIds ?? [], \Auth::user());

return $beatmap->beatmapset->defaultDiscussionJson();
}
Expand Down Expand Up @@ -459,7 +441,7 @@ public function userScore($beatmapId, $userId)
'score' => json_item(
$score,
new ScoreTransformer(),
['beatmap', ...static::DEFAULT_SCORE_INCLUDES]
['beatmap.mappers', ...static::DEFAULT_SCORE_INCLUDES]
),
];
}
Expand Down
2 changes: 2 additions & 0 deletions app/Http/Controllers/BeatmapsetsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -399,6 +399,7 @@ private function showJson($beatmapset)
"{$beatmapRelation}.baseDifficultyRatings",
"{$beatmapRelation}.baseMaxCombo",
"{$beatmapRelation}.failtimes",
"{$beatmapRelation}.beatmapOwners",
'genre',
'language',
'user',
Expand All @@ -410,6 +411,7 @@ private function showJson($beatmapset)
return json_item($beatmapset, $transformer, [
'beatmaps',
'beatmaps.failtimes',
'beatmaps.mappers',
'beatmaps.max_combo',
'converts',
'converts.failtimes',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,23 @@

declare(strict_types=1);

namespace App\Http\Controllers\Chat;
namespace App\Http\Controllers\Users;

use App\Http\Controllers\Controller as BaseController;
use App\Http\Controllers\Controller;
use App\Models\User;
use App\Transformers\UserCompactTransformer;

class UsersController extends BaseController
class LookupController extends Controller
{
public function index()
public function __construct()
{
priv_check('ChatAnnounce')->ensureCan();
$this->middleware('auth');
$this->middleware('throttle:30,1');
}

public function lookup()
{
// TODO: referer check?
$params = get_params(request()->all(), null, ['ids:string[]'], ['null_missing' => true]);
$ids = array_slice($params['ids'], 0, 50);

Expand All @@ -31,6 +36,7 @@ public function index()
}

$users = User::where(fn ($q) => $q->whereIn('user_id', $numericIds)->orWhereIn('username', $stringIds))
->where('group_id', '<>', app('groups')->byIdentifier('no_profile')->getKey())
->default()
->with(UserCompactTransformer::CARD_INCLUDES_PRELOAD)
->get();
Expand Down
80 changes: 80 additions & 0 deletions app/Libraries/Beatmapset/ChangeBeatmapOwners.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
<?php

// Copyright (c) ppy Pty Ltd <[email protected]>. Licensed under the GNU Affero General Public License v3.0.
// See the LICENCE file in the repository root for full licence text.

declare(strict_types=1);

namespace App\Libraries\Beatmapset;

use App\Exceptions\InvariantException;
use App\Jobs\Notifications\BeatmapOwnerChange;
use App\Models\Beatmap;
use App\Models\BeatmapOwner;
use App\Models\BeatmapsetEvent;
use App\Models\DeletedUser;
use App\Models\User;
use Ds\Set;

class ChangeBeatmapOwners
{
private Set $userIds;

public function __construct(private Beatmap $beatmap, array $newUserIds, private User $source)
{
priv_check_user($source, 'BeatmapUpdateOwner', $beatmap->beatmapset)->ensureCan();

$this->userIds = new Set($newUserIds);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some kind of limit would be useful


if ($this->userIds->isEmpty()) {
throw new InvariantException('user_ids must be specified');
}
}

public function handle(): void
{
$currentOwners = new Set($this->beatmap->mappers->pluck('user_id'));
if ($currentOwners->xor($this->userIds)->isEmpty()) {
return;
}

$newUserIds = $this->userIds->diff($currentOwners);

if (User::whereIn('user_id', $newUserIds->toArray())->count() !== $newUserIds->count()) {
throw new InvariantException('invalid user_id');
}

$this->beatmap->getConnection()->transaction(function () {
$userIds = $this->userIds->toArray();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not iterate over the set itself? (or even straight out map)

$params = [];

foreach ($userIds as $userId) {
$params[] = ['beatmap_id' => $this->beatmap->getKey(), 'user_id' => $userId];
}

$this->beatmap->fill(['user_id' => $userIds[0]])->saveOrExplode();
$this->beatmap->beatmapOwners()->delete();
BeatmapOwner::insert($params);

$this->beatmap->refresh();

// TODO: use select instead (needs newer laravel)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how would the deleted user username fallback work with select though?

$newUsers = $this->beatmap->mappers->map(
fn ($user) => ['id' => $user->user_id, 'username' => ($user ?? new DeletedUser())->username],
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wouldn't the ->user_id (->getKey()?) fail if $user is null?

)->all();
$beatmapset = $this->beatmap->beatmapset;

BeatmapsetEvent::log(BeatmapsetEvent::BEATMAP_OWNER_CHANGE, $this->source, $beatmapset, [
'beatmap_id' => $this->beatmap->getKey(),
'beatmap_version' => $this->beatmap->version,
'new_user_id' => $this->beatmap->user_id,
'new_user_username' => ($this->beatmap->user ?? new DeletedUser())->username,
'new_users' => $newUsers,
])->saveOrExplode();

$beatmapset->update(['eligible_main_rulesets' => null]);
});

(new BeatmapOwnerChange($this->beatmap, $this->source))->dispatch();
}
}
60 changes: 41 additions & 19 deletions app/Models/Beatmap.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,17 @@

use App\Exceptions\InvariantException;
use App\Jobs\EsDocument;
use App\Libraries\Beatmapset\ChangeBeatmapOwners;
use App\Libraries\Transactions\AfterCommit;
use DB;
use Ds\Set;
use Illuminate\Database\Eloquent\Builder;
use Illuminate\Database\Eloquent\Collection;
use Illuminate\Database\Eloquent\SoftDeletes;

/**
* @property int $approved
* @property \Illuminate\Database\Eloquent\Collection $beatmapDiscussions BeatmapDiscussion
* @property-read Collection<BeatmapDiscussion> $beatmapDiscussions
* @property int $beatmap_id
* @property Beatmapset $beatmapset
* @property int|null $beatmapset_id
Expand All @@ -29,20 +32,22 @@
* @property float $diff_drain
* @property float $diff_overall
* @property float $diff_size
* @property \Illuminate\Database\Eloquent\Collection $difficulty BeatmapDifficulty
* @property \Illuminate\Database\Eloquent\Collection $difficultyAttribs BeatmapDifficultyAttrib
* @property-read Collection<BeatmapDifficulty> $difficulty
* @property-read Collection<BeatmapDifficultyAttrib> $difficultyAttribs
* @property float $difficultyrating
* @property \Illuminate\Database\Eloquent\Collection $failtimes BeatmapFailtimes
* @property-read Collection<BeatmapFailtimes> $failtimes
* @property string|null $filename
* @property int $hit_length
* @property \Carbon\Carbon $last_update
* @property int $max_combo
* @property mixed $mode
* @property-read Collection<User> $mappers
* @property int $passcount
* @property int $playcount
* @property int $playmode
* @property int $score_version
* @property int $total_length
* @property User $user
* @property int $user_id
* @property string $version
* @property string|null $youtube_preview
Expand Down Expand Up @@ -107,6 +112,11 @@ public function baseMaxCombo()
return $this->difficultyAttribs()->noMods()->maxCombo();
}

public function beatmapOwners()
{
return $this->hasMany(BeatmapOwner::class);
}

public function beatmapset()
{
return $this->belongsTo(Beatmapset::class, 'beatmapset_id')->withTrashed();
Expand Down Expand Up @@ -256,6 +266,7 @@ public function getAttribute($key)

'diff_size' => $this->getDiffSize(),
'difficultyrating' => $this->getDifficultyrating(),
'mappers' => $this->getMappers(),
'mode' => $this->getMode(),
'version' => $this->getVersion(),

Expand Down Expand Up @@ -300,22 +311,9 @@ public function maxCombo()
return $maxCombo?->value;
}

public function setOwner($newUserId)
public function setOwner(array $newUserIds, User $source): void
{
if ($newUserId === null) {
throw new InvariantException('user_id must be specified');
}

if (User::find($newUserId) === null) {
throw new InvariantException('invalid user_id');
}

if ($newUserId === $this->user_id) {
throw new InvariantException('the specified user_id is already the owner');
}

$this->fill(['user_id' => $newUserId])->saveOrExplode();
$this->beatmapset->update(['eligible_main_rulesets' => null]);
(new ChangeBeatmapOwners($this, $newUserIds, $source))->handle();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it all that useful over just using the class directly

}

public function status()
Expand Down Expand Up @@ -376,6 +374,30 @@ private function getDiffSize()
return $value;
}

private function getMappers(): Collection
{
$beatmapOwners = $this->beatmapOwners()->pluck('user_id');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this will ignore preloaded relation


$mappers = User::whereIn('user_id', $beatmapOwners)->get();
// compatiblity for anything that isn't writing to beatmap_owners yet.
if ($mappers->find($this->user_id) === null && $this->user !== null) {
$mappers->prepend($this->user);
}

// Add deleted/missing users.
if ($beatmapOwners->count() !== $mappers->count()) {
$beatmapOwnersSet = new Set($beatmapOwners);
$mappersSet = new Set($mappers->pluck('user_id')->toArray());

$missingIds = $beatmapOwnersSet->diff($mappersSet);
foreach ($missingIds as $id) {
$mappers->push(new DeletedUser(['user_id' => $id]));
}
}

return $mappers;
}

private function getMode()
{
return static::modeStr($this->playmode);
Expand Down
31 changes: 31 additions & 0 deletions app/Models/BeatmapOwner.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
<?php

// Copyright (c) ppy Pty Ltd <[email protected]>. Licensed under the GNU Affero General Public License v3.0.
// See the LICENCE file in the repository root for full licence text.

namespace App\Models;

/**
* @property Beatmap $beatmap
* @property int $beatmap_id
* @property User $user
* @property int $user_id
*/
class BeatmapOwner extends Model
{
public $incrementing = false;
public $timestamps = false;

protected $primaryKey = ':composite';
protected $primaryKeys = ['beatmap_id', 'user_id'];

public function beatmap()
{
return $this->belongsTo(Beatmap::class, 'beatmap_id');
}

public function user()
{
return $this->belongsTo(User::class, 'user_id');
}
}
Loading