Merge pull request #11764 from notbakaneko/feature/change-beatmap-own…

…er-exclude-bot

Exclude bot and no profile special groups from being set as guest mappers

app/Http/Controllers/Users/LookupController.php

@@ -22,7 +22,12 @@ public function __construct()
     public function index()
     {
         // TODO: referer check?
-        $ids = array_slice(array_reject_null(get_arr(request('ids'), presence(...)) ?? []), 0, 50);
+        $params = get_params(\Request::all(), null, [
+            'exclude_bots:bool',
+            'ids:string[]',
+        ]);
+
+        $ids = array_slice(array_reject_null(get_arr($params['ids'] ?? [], presence(...))), 0, 50);
 
         $numericIds = [];
         $stringIds = [];
@@ -35,13 +40,16 @@ 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();
+            ->withoutNoProfile()
+            ->with(UserCompactTransformer::CARD_INCLUDES_PRELOAD);
+
+        if ($params['exclude_bots'] ?? false) {
+            $users = $users->withoutBots();
+        }
 
         return [
-            'users' => json_collection($users, new UserCompactTransformer(), UserCompactTransformer::CARD_INCLUDES),
+            'users' => json_collection($users->get(), new UserCompactTransformer(), UserCompactTransformer::CARD_INCLUDES),
         ];
     }
 }

app/Libraries/Beatmapset/ChangeBeatmapOwners.php

@@ -43,7 +43,7 @@ public function handle(): void
 
         $newUserIds = $this->userIds->diff($currentOwners);
 
-        if (User::whereIn('user_id', $newUserIds->toArray())->default()->count() !== $newUserIds->count()) {
+        if (User::whereIn('user_id', $newUserIds->toArray())->default()->withoutBots()->withoutNoProfile()->count() !== $newUserIds->count()) {
             throw new InvariantException('invalid user_id');
         }
 

app/Models/User.php

@@ -2039,6 +2039,16 @@ public function scopeOnline($query)
             ->where('user_lastvisit', '>', time() - $GLOBALS['cfg']['osu']['user']['online_window']);
     }
 
+    public function scopeWithoutBots(Builder $query): Builder
+    {
+        return $query->whereNot('group_id', app('groups')->byIdentifier('bot')->getKey());
+    }
+
+    public function scopeWithoutNoProfile(Builder $query): Builder
+    {
+        return $query->whereNot('group_id', app('groups')->byIdentifier('no_profile')->getKey());
+    }
+
     public function checkPassword($password)
     {
         return Hash::check($password, $this->getAuthPassword());

resources/js/beatmap-discussions/beatmap-owner-editor.tsx

@@ -189,6 +189,7 @@ export default class BeatmapOwnerEditor extends React.Component<Props> {
         <div className={classWithModifiers('beatmap-owner-editor-owners', { editing: this.editing })}>
           {this.editing ? (
             <UsernameInput
+              excludeBots
               initialUsers={this.owners}
               // initialValue not set for owner editor as value is reset when cancelled.
               modifiers='beatmap-owner-editor'

resources/js/chat/create-announcement.tsx

@@ -81,6 +81,7 @@ export default class CreateAnnouncement extends React.Component<Props> {
               <div className='chat-form-users'>
                 <UserCardBrick user={core.currentUserOrFail} />
                 <UsernameInput
+                  excludeBots
                   id='chat-form-users'
                   ignoreCurrentUser
                   name='users'

resources/js/components/username-input.tsx

@@ -15,6 +15,7 @@ import { Spinner } from './spinner';
 import UserCardBrick from './user-card-brick';
 
 interface Props {
+  excludeBots?: boolean;
   id?: string;
   ignoreCurrentUser?: boolean;
   initialUsers?: UserJson[];
@@ -166,7 +167,7 @@ export default class UsernameInput extends React.PureComponent<Props> {
     }
 
     try {
-      this.xhr = apiLookupUsers(userIds);
+      this.xhr = apiLookupUsers(userIds, this.props.excludeBots);
       const response = await this.xhr;
       this.extractValidUsers(response.users);
     } catch (error) {

resources/js/store/store-supporter-tag.tsx

@@ -224,7 +224,7 @@ export default class StoreSupporterTag extends React.Component<Props> {
 
   @action
   private readonly getUser = (username: string) => {
-    this.xhr = apiLookupUsers([`@${username}`]);
+    this.xhr = apiLookupUsers([`@${username}`], true);
 
     this.xhr
       .done((response) => runInAction(() => {

resources/js/utils/user.ts

@@ -4,9 +4,9 @@
 import UserJson from 'interfaces/user-json';
 import { route } from 'laroute';
 
-export function apiLookupUsers(idsOrUsernames: (string | null | undefined)[]) {
+export function apiLookupUsers(idsOrUsernames: (string | null | undefined)[], excludeBots?: boolean) {
   return $.ajax(route('users.lookup'), {
-    data: { ids: idsOrUsernames },
+    data: { exclude_bots: excludeBots, ids: idsOrUsernames },
     dataType: 'json',
   }) as JQuery.jqXHR<{ users: UserJson[] }>;
 }

tests/Libraries/Beatmapset/ChangeBeatmapOwnersTest.php

@@ -23,6 +23,14 @@
 
 class ChangeBeatmapOwnersTest extends TestCase
 {
+    public static function dataProviderForInvalidUser(): array
+    {
+        return [
+            ['bot'],
+            ['no_profile'],
+        ];
+    }
+
     public static function dataProviderForUpdateOwner(): array
     {
         return [
@@ -68,6 +76,33 @@ public function testMissingUser(): void
         Bus::assertDispatched(BeatmapOwnerChange::class);
     }
 
+    /**
+     * @dataProvider dataProviderForInvalidUser
+     */
+    public function testInvalidUser(string $group): void
+    {
+        $moderator = User::factory()->withGroup('nat')->create();
+        $owner = User::factory()->create();
+        $invalidUser = User::factory()->withGroup($group)->create();
+
+        $beatmap = Beatmap::factory()
+            ->for(Beatmapset::factory()->pending()->owner($owner))
+            ->owner($owner)
+            ->create();
+
+        $this->expectCountChange(fn () => BeatmapsetEvent::count(), 0);
+
+        $this->expectExceptionCallable(
+            fn () => (new ChangeBeatmapOwners($beatmap, [$invalidUser->getKey()], $moderator))->handle(),
+            InvariantException::class,
+        );
+
+        $beatmap = $beatmap->fresh();
+        $this->assertEqualsCanonicalizing([$owner->getKey()], $beatmap->getOwners()->pluck('user_id')->toArray());
+
+        Bus::assertNotDispatched(BeatmapOwnerChange::class);
+    }
+
     /**
      * @dataProvider dataProviderForUpdateOwner
      */