Skip to content

Support multiple mappers on beatmap difficulties #11377

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

Merged
merged 168 commits into from
Dec 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
168 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
19a8517
Merge branch 'master' into feature/beatmaps-multiple-mappers
notbakaneko Oct 15, 2024
26e5ac9
doesn't do anything at the current size when parent container doesn't…
notbakaneko Oct 15, 2024
ef2a2c6
increase gap
notbakaneko Oct 17, 2024
2d0679c
use function short hand; also break up lines
notbakaneko Oct 17, 2024
6094968
use joinComponents;
notbakaneko Oct 17, 2024
71bcc08
move tooltip to aoivd overlapping button/contents while editing
notbakaneko Oct 17, 2024
432b8c6
add confirm dialog if navigating away while editing
notbakaneko Oct 17, 2024
9961203
include owners with converts
notbakaneko Oct 17, 2024
79afce7
require owners property when using hasGuestMapper
notbakaneko Oct 17, 2024
315563d
use collection transformer
notbakaneko Oct 18, 2024
4cf6146
add compatibility user after handling deleted/missing users
notbakaneko Oct 18, 2024
e797273
set is iterable
notbakaneko Oct 18, 2024
2c8b429
limit maximum number of guest mappers
notbakaneko Oct 18, 2024
dedf45b
use only() to get values; id conveniently mapped to getKey()
notbakaneko Oct 18, 2024
3111767
wrap owners
notbakaneko Oct 21, 2024
e6963a8
hide button instead
notbakaneko Oct 21, 2024
334a910
use --input-border-colour
notbakaneko Oct 22, 2024
a5734ca
add a bit more padding, move everything into the container
notbakaneko Oct 22, 2024
32d4239
remove old method originally kept around to make it easer to test cha…
notbakaneko Oct 23, 2024
4f01de0
remove attribute accessor for now;
notbakaneko Oct 23, 2024
b6b02cb
test missing/deleted user is preserved during changes.
notbakaneko Oct 23, 2024
6ea93b4
use relation
notbakaneko Oct 24, 2024
d517e0f
add beatmap owners to index
notbakaneko Oct 25, 2024
dc6fd75
preload users for beatmap owners if necessary
notbakaneko Oct 25, 2024
5178c50
Merge branch 'master' into feature/beatmaps-multiple-mappers
notbakaneko Oct 25, 2024
430b091
lint fix
notbakaneko Oct 25, 2024
38a4536
localise too many guest mappers error
notbakaneko Oct 28, 2024
3d94a3f
apply default lookup
notbakaneko Oct 28, 2024
2f6e7ff
Add test case for existing restricted user;
notbakaneko Oct 28, 2024
b69335b
already in testMissingUser
notbakaneko Oct 28, 2024
1f78bab
test restricted users
notbakaneko Oct 28, 2024
0e26615
Merge branch 'master' into feature/beatmaps-multiple-mappers
notbakaneko Nov 15, 2024
b0e321b
owners always filled now
notbakaneko Nov 14, 2024
c0cf615
use array_map
notbakaneko Nov 14, 2024
6b4052c
use already queried data; add note
notbakaneko Nov 14, 2024
1167226
include multiple owners in BeatmapDiscussion::managedBy
notbakaneko Nov 14, 2024
b5933ef
explicitly compare with user_id
notbakaneko Nov 14, 2024
b6d904e
check all owners for checkBeatmapsetNominate
notbakaneko Nov 14, 2024
85a37d3
eager load beatmapOwners for beatmaps
notbakaneko Nov 14, 2024
5da7c56
rename type
notbakaneko Nov 15, 2024
0389253
clearer comment
notbakaneko Nov 15, 2024
191e312
unused now
notbakaneko Nov 18, 2024
2e3daee
split css class and add missing background
notbakaneko Nov 18, 2024
7b0adcc
no highlight on hover; don't override error colour on hover
notbakaneko Nov 18, 2024
a8e2635
don't show error highlight during first lookup of name
notbakaneko Nov 18, 2024
9a657eb
import order
notbakaneko Nov 18, 2024
48bb628
unneeded typing
notbakaneko Nov 18, 2024
0e1d857
add background colour to beatmap owner item
notbakaneko Nov 18, 2024
d263810
it's turbo now
notbakaneko Nov 18, 2024
78e1d0e
explicitly use BeatmapsetTransformer
notbakaneko Nov 19, 2024
cb7eb31
rename with more explicit name
notbakaneko Nov 19, 2024
2840b5d
Merge branch 'master' into feature/beatmaps-multiple-mappers
notbakaneko Nov 25, 2024
e589f63
use select()
notbakaneko Nov 25, 2024
d8f8883
o is after m;
notbakaneko Nov 25, 2024
ffe6e91
Revert "no highlight on hover; don't override error colour on hover"
notbakaneko Nov 25, 2024
06f602a
Revert "eagerload for userScore"
notbakaneko Nov 28, 2024
fdcbde7
simpler then trying to share getOwner and not loading owner when not …
notbakaneko Nov 28, 2024
ecc00eb
avatar_url not needed for beatmap list in discussion dropdown
notbakaneko Nov 28, 2024
b5e8c0a
owners should be included in score page
notbakaneko Nov 28, 2024
6ca51ce
not just includes, also the beatmapset event log
notbakaneko Nov 28, 2024
ea43258
use loadMissing to load relations instead of with->find
notbakaneko Nov 29, 2024
23d4942
set discussion relation preloads from already loaded data.
notbakaneko Nov 29, 2024
47f9a75
that's not supposed to be there
notbakaneko Nov 29, 2024
a8256c7
key by beatmap_id for lookup
notbakaneko Nov 29, 2024
11825d3
forgot about the comment
notbakaneko Nov 29, 2024
900ecb3
skip highlighting when not editing (it's not click to edit)
notbakaneko Nov 29, 2024
edbbabd
unique id for each input
notbakaneko Nov 29, 2024
e6537e4
add listener in correct event
notbakaneko Dec 2, 2024
8fa7949
move handling of cancelling navigation to parent; add confirmation to…
notbakaneko Dec 2, 2024
f81bb52
add guest badge on discussions
notbakaneko Dec 2, 2024
8ed9cab
update mapper note js permission
notbakaneko Dec 2, 2024
adf7bea
id not needed if first element?
notbakaneko Dec 2, 2024
a82db85
also check for multiple owners on nominate
notbakaneko Dec 3, 2024
20e791c
Merge branch 'master' into feature/beatmaps-multiple-mappers
notbakaneko Dec 3, 2024
755efc3
add deleted user if missing
notbakaneko Dec 4, 2024
454d9fb
Merge branch 'master' into feature/beatmaps-multiple-mappers
nanaya Dec 5, 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
1 change: 1 addition & 0 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ CLIENT_CHECK_VERSION=false
# SEARCH_MINIMUM_LENGTH=2

# BEATMAPS_DIFFICULTY_CACHE_SERVER_URL=http://localhost:5001
# BEATMAPS_OWNERS_MAX=10
# BEATMAPSET_DISCUSSION_KUDOSU_PER_USER=10
# BEATMAPSET_GUEST_ADVANCED_SEARCH=0
# BEATMAPSET_MAXIMUM_DISQUALIFIED_RANK_PENALTY_DAYS=7
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 <contact@ppy.sh>. 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('');
}
}
26 changes: 4 additions & 22 deletions app/Http/Controllers/BeatmapsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,13 @@

use App\Enums\Ruleset;
use App\Exceptions\InvariantException;
use App\Jobs\Notifications\BeatmapOwnerChange;
use App\Libraries\BeatmapDifficultyAttributes;
use App\Libraries\Beatmapset\ChangeBeatmapOwners;
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 Down Expand Up @@ -381,26 +380,9 @@ public function soloScores($id)
public function updateOwner($id)
{
$beatmap = Beatmap::findOrFail($id);
$currentUser = auth()->user();
$newUserIds = get_arr(request('user_ids'), 'get_int');

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

$newUserId = get_int(request('beatmap.user_id'));

$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();
});

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

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.owners', ...static::DEFAULT_SCORE_INCLUDES]
),
];
}
Expand Down
3 changes: 3 additions & 0 deletions app/Http/Controllers/BeatmapsetsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,7 @@ private function showJson($beatmapset)
"{$beatmapRelation}.baseDifficultyRatings",
"{$beatmapRelation}.baseMaxCombo",
"{$beatmapRelation}.failtimes",
"{$beatmapRelation}.beatmapOwners.user",
'genre',
'language',
'user',
Expand All @@ -402,8 +403,10 @@ private function showJson($beatmapset)
'beatmaps',
'beatmaps.failtimes',
'beatmaps.max_combo',
'beatmaps.owners',
'converts',
'converts.failtimes',
'converts.owners',
Copy link
Collaborator

Choose a reason for hiding this comment

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

duplicating same content three times (four if counting related_users) doesn't seem like a good idea...?

(and I don't think I've seen comment on why .owners is a thing, partially duplicating related_users and not just ids)

Copy link
Collaborator

Choose a reason for hiding this comment

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

as a bonus(?), if just need the user ids, it can potentially be selected together with the beatmap itself with something like in

public function scopeWithRecentParticipantIds($query, ?int $limit = null)
{
$limit ??= 10;
if ($query->getQuery()->columns === null) {
$query = $query->select();
}
$highScore = new UserScoreAggregate();
return $query->selectSub("
SELECT json_arrayagg(user_id)
FROM (
SELECT user_id
FROM {$highScore->getTable()}
WHERE
{$highScore->qualifyColumn('room_id')} = {$this->qualifyColumn($this->getKeyName())}
AND (
{$this->qualifyColumn('type')} = {$this->getGrammar()->quoteString(static::PLAYLIST_TYPE)}
OR {$highScore->qualifyColumn('in_room')}
)
ORDER BY updated_at DESC
LIMIT {$limit}
) recent_participants
", 'recent_participant_ids');
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

owners is included on converts so the UI side doesn't have to go digging the for the non-convert to check for mappers.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

owners is also contains the minimum to display on pages that don't have related_users like the score page instead of a separate include for just the ids, otherwise, the score page should sideload the users instead.

Copy link
Collaborator

Choose a reason for hiding this comment

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

owners is included on converts so the UI side doesn't have to go digging the for the non-convert to check for mappers.

just include the ids then?

owners is also contains the minimum to display on pages that don't have related_users like the score page instead of a separate include for just the ids, otherwise, the score page should sideload the users instead.

include it only on that specific page then? or add the related users.

seems like a lot of data being returned for everything else just to deal with single page with its single beatmap

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

owners is just the id and username...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm going to remove the duplicated data from converts but it may or not be part of this PR since it needs some extra type and client-side beatmap lookup changes

'current_nominations',
'current_user_attributes',
'description',
Expand Down
1 change: 1 addition & 0 deletions app/Http/Controllers/ScoresController.php
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ public function show($rulesetOrSoloId, $legacyId = null)
$scoreJson = json_item($score, new ScoreTransformer(), array_merge([
'beatmap.max_combo',
'beatmap.user',
'beatmap.owners',
'beatmapset',
'rank_global',
], $userIncludes));
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 <contact@ppy.sh>. 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\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();
Copy link
Collaborator

Choose a reason for hiding this comment

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

seems like this should be part of controller (as it previously was) as indicated by the need to create additional user in test


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

if ($this->userIds->count() > $GLOBALS['cfg']['osu']['beatmaps']['owners_max']) {
throw new InvariantException(osu_trans('beatmaps.change_owner.too_many'));
}

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

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

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

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

$this->beatmap->getConnection()->transaction(function () {
$params = array_map(
fn ($userId) => ['beatmap_id' => $this->beatmap->getKey(), 'user_id' => $userId],
$this->userIds->toArray()
);

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

$this->beatmap->refresh();

$newUsers = $this->beatmap->getOwners()->select('id', 'username')->all();
$beatmapset = $this->beatmap->beatmapset;
$firstMapper = $newUsers[0];

BeatmapsetEvent::log(BeatmapsetEvent::BEATMAP_OWNER_CHANGE, $this->source, $beatmapset, [
'beatmap_id' => $this->beatmap->getKey(),
'beatmap_version' => $this->beatmap->version,
// TODO: mainly for compatibility during dev when switching branches, can be removed after deployed.
'new_user_id' => $firstMapper['id'],
'new_user_username' => $firstMapper['username'],
'new_users' => $newUsers,
])->saveOrExplode();

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

(new BeatmapOwnerChange($this->beatmap, $this->source))->dispatch();
}
}
64 changes: 42 additions & 22 deletions app/Models/Beatmap.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,13 @@
use App\Libraries\Transactions\AfterCommit;
use DB;
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-read Collection<BeatmapOwner> $beatmapOwners
* @property int $beatmap_id
* @property Beatmapset $beatmapset
* @property int|null $beatmapset_id
Expand All @@ -29,20 +31,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> $owners
* @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 +111,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 @@ -267,6 +276,7 @@ public function getAttribute($key)
'baseDifficultyRatings',
'baseMaxCombo',
'beatmapDiscussions',
'beatmapOwners',
'beatmapset',
'difficulty',
'difficultyAttribs',
Expand All @@ -279,6 +289,34 @@ public function getAttribute($key)
};
}

/**
* @return Collection<User>
*/
public function getOwners(): Collection
{
$owners = $this->beatmapOwners->loadMissing('user')->map(
fn ($beatmapOwner) => $beatmapOwner->user ?? new DeletedUser(['user_id' => $beatmapOwner->user_id])
);

// TODO: remove when everything writes to beatmap_owners.
Copy link
Collaborator

Choose a reason for hiding this comment

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

isn't it enough to just create an entry if there's no owners at all? any further changes go through osu-web anyway

Copy link
Collaborator

Choose a reason for hiding this comment

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

more like should eventually decide whether an empty collection is acceptable or not (in case of broken owners or just simplifying simple cases)

if (!$owners->contains(fn ($beatmapOwner) => $beatmapOwner->user_id === $this->user_id)) {
$owners->prepend($this->user ?? new DeletedUser(['user_id' => $this->user_id]));
}

return $owners;
}

public function isOwner(User $user): bool
{
if ($this->user_id === $user->getKey()) {
return true;
}

return $this->relationLoaded('beatmapOwners')
? $this->beatmapOwners->contains('user_id', $user->getKey())
: $this->beatmapOwners()->where('user_id', $user->getKey())->exists();
}

public function maxCombo()
{
if (!$this->convert) {
Expand All @@ -305,24 +343,6 @@ public function maxCombo()
return $maxCombo?->value;
}

public function setOwner($newUserId)
{
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]);
}

public function status()
{
return array_search($this->approved, Beatmapset::STATES, true);
Expand Down
6 changes: 2 additions & 4 deletions app/Models/BeatmapDiscussion.php
Original file line number Diff line number Diff line change
Expand Up @@ -632,10 +632,8 @@ public function denyKudosu($deniedBy)

public function managedBy(User $user): bool
{
$id = $user->getKey();

return $this->beatmapset->user_id === $id
|| ($this->beatmap !== null && $this->beatmap->user_id === $id);
return $this->beatmapset->user_id === $user->getKey()
|| ($this->beatmap !== null && $this->beatmap->isOwner($user));
}

public function userRecentVotesCount($user, $increment = false)
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 <contact@ppy.sh>. 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
Loading