Merge pull request #11371 from nanaya/vote-down-everyone

Update beatmap discussion downvoting behaviour
ppy · Aug 7, 2024 · e0970c7 · e0970c7
2 parents 80d90de + c577606
commit e0970c7
Show file tree

Hide file tree

Showing 6 changed files with 27 additions and 43 deletions.
diff --git a/app/Http/Controllers/BeatmapDiscussionsController.php b/app/Http/Controllers/BeatmapDiscussionsController.php
@@ -176,10 +176,6 @@ public function vote($id)
         $params = get_params(Request::all(), 'beatmap_discussion_vote', ['score:int']);
         $params['user_id'] = Auth::user()->user_id;
 
-        if ($params['score'] < 0) {
-            priv_check('BeatmapDiscussionVoteDown', $discussion)->ensureCan();
-        }
-
         if ($discussion->vote($params)) {
             return $discussion->beatmapset->defaultDiscussionJson();
         } else {

diff --git a/app/Models/BeatmapDiscussion.php b/app/Models/BeatmapDiscussion.php
@@ -274,7 +274,7 @@ public function refreshKudosu($event, $eventExtraData = [])
 
         // inb4 timing problem
         $currentVotes = $this->canGrantKudosu() ?
-            (int) $this->beatmapDiscussionVotes()->sum('score') :
+            (int) $this->beatmapDiscussionVotes()->where('score', '>', 0)->sum('score') :
             0;
         // remove kudosu by bots here instead of in canGrantKudosu due to
         // the function is also called by transformer without user preloaded

diff --git a/app/Singletons/OsuAuthorize.php b/app/Singletons/OsuAuthorize.php
@@ -393,34 +393,6 @@ public function checkBeatmapDiscussionVote(?User $user, BeatmapDiscussion $discu
         return 'ok';
     }
 
-    /**
-     * @param User|null $user
-     * @param BeatmapDiscussion $discussion
-     * @return string
-     * @throws AuthorizationCheckException
-     */
-    public function checkBeatmapDiscussionVoteDown(?User $user, BeatmapDiscussion $discussion): string
-    {
-        $prefix = 'beatmap_discussion.vote.';
-
-        if ($discussion->user !== null && $discussion->user->isBot()) {
-            return $prefix.'bot';
-        }
-
-        $this->ensureLoggedIn($user);
-        $this->ensureCleanRecord($user);
-
-        if ($discussion->user_id === $user->user_id) {
-            return $prefix.'owner';
-        }
-
-        if ($user->isBNG() || $user->isModerator()) {
-            return 'ok';
-        }
-
-        return 'unauthorized';
-    }
-
     /**
      * @param User|null $user
      * @param BeatmapDiscussionPost $post

diff --git a/resources/js/beatmap-discussions/discussion-vote-buttons.tsx b/resources/js/beatmap-discussions/discussion-vote-buttons.tsx
@@ -6,9 +6,8 @@ import { BeatmapsetDiscussionJsonForShow } from 'interfaces/beatmapset-discussio
 import BeatmapsetWithDiscussionsJson from 'interfaces/beatmapset-with-discussions-json';
 import UserJson from 'interfaces/user-json';
 import { route } from 'laroute';
-import { action, computed, makeObservable, observable } from 'mobx';
+import { action, makeObservable, observable } from 'mobx';
 import { observer } from 'mobx-react';
-import core from 'osu-core-singleton';
 import * as React from 'react';
 import { onError } from 'utils/ajax';
 import { classWithModifiers } from 'utils/css';
@@ -31,11 +30,6 @@ export default class DiscussionVoteButtons extends React.Component<Props> {
   private readonly tooltipDisposers: Partial<Record<VoteType, () => void>> = {};
   @observable private voteXhr: JQuery.jqXHR<BeatmapsetWithDiscussionsJson> | null = null;
 
-  @computed
-  private get canDownvote() {
-    return core.currentUser != null && (core.currentUser.is_admin || core.currentUser.is_moderator || core.currentUser.is_bng);
-  }
-
   constructor(props: Props) {
     super(props);
     makeObservable(this);
@@ -109,7 +103,7 @@ export default class DiscussionVoteButtons extends React.Component<Props> {
     const [baseScore, icon] = type === 'up' ? [1, 'thumbs-up'] : [-1, 'thumbs-down'];
     const currentVote = this.props.discussion.current_user_attributes?.vote_score;
     const score = currentVote === baseScore ? 0 : baseScore;
-    const disabled = this.voteXhr != null || this.props.cannotVote || (type === 'down' && !this.canDownvote);
+    const disabled = this.voteXhr != null || this.props.cannotVote;
 
     return (
       <button

diff --git a/tests/Controllers/BeatmapDiscussionsControllerTest.php b/tests/Controllers/BeatmapDiscussionsControllerTest.php
@@ -223,15 +223,15 @@ public static function putVoteChangeToDownDataProvider()
     {
         return [
             'bng can change to down vote' => ['bng', 200, -2],
-            'regular user cannot change to down vote' => [null, 403, 0],
+            'regular user can change to down vote' => [null, 200, -2],
         ];
     }
 
     public static function putVoteDownDataProvider()
     {
         return [
             'bng can down vote' => ['bng', 200, 1, -1],
-            'regular user cannot down vote' => [null, 403, 0, 0],
+            'regular user can down vote' => [null, 200, 1, -1],
         ];
     }
 

diff --git a/tests/Models/BeatmapDiscussionTest.php b/tests/Models/BeatmapDiscussionTest.php
@@ -8,6 +8,7 @@
 use App\Models\Beatmap;
 use App\Models\BeatmapDiscussion;
 use App\Models\Beatmapset;
+use App\Models\KudosuHistory;
 use App\Models\User;
 use Illuminate\Database\Eloquent\Builder;
 use Tests\TestCase;
@@ -184,6 +185,27 @@ public function testSoftDeleteOrExplode()
         $this->assertFalse($discussion->trashed());
     }
 
+    public function testRefreshKudosu(): void
+    {
+        $discussion = BeatmapDiscussion::factory()->problem()->create([
+            'beatmapset_id' => Beatmapset::factory()->create(),
+            'timestamp' => null,
+            'user_id' => User::factory()->create(),
+        ]);
+        $discussion->beatmapDiscussionVotes()->create([
+            'score' => 1,
+            'user_id' => User::factory()->create()->getKey(),
+        ]);
+        $discussion->beatmapDiscussionVotes()->create([
+            'score' => -1,
+            'user_id' => User::factory()->create()->getKey(),
+        ]);
+
+        $this->expectCountChange(fn () => KudosuHistory::count(), 1);
+        $this->expectCountChange(fn () => $discussion->user->fresh()->osu_kudostotal, 1);
+        $discussion->fresh()->refreshKudosu('recalculate');
+    }
+
     public static function validBeatmapsetStatuses()
     {
         return array_map(function ($status) {