Fix supporter being required when selecting default options in beatmap listing

resources/js/beatmaps/beatmapset-search-controller.ts

@@ -11,6 +11,7 @@ import core from 'osu-core-singleton';
 import { trans, transArray } from 'utils/lang';
 import { popup } from 'utils/popup';
 import { currentUrl } from 'utils/turbolinks';
+import { updateQueryString } from 'utils/url';
 
 
 const expandFilters: FilterKey[] = ['genre', 'language', 'extra', 'rank', 'played'];
@@ -150,10 +151,9 @@ export class BeatmapsetSearchController {
     });
   }
 
+  @action
   private readonly filterChangedHandler = (change: IObjectDidChange<BeatmapsetSearchFilters>) => {
     if (change.type === 'update' && change.oldValue === change.newValue) return;
-    // FIXME: sort = null changes ignored because search triggered too early during filter update.
-    if (change.type !== 'remove' && change.name === 'sort' && change.newValue == null) return;
 
     this.searchStatus.state = 'input';
     this.debouncedFilterChangedSearch();
@@ -178,6 +178,10 @@ export class BeatmapsetSearchController {
       this.filtersObserver();
     }
     this.filters = new BeatmapsetSearchFilters(url);
+
+    // normalize url
+    Turbolinks.controller.replaceHistory(updateQueryString(null, { ...this.filters.queryParams }));
+
     this.filtersObserver = observe(this.filters, this.filterChangedHandler);
 
     this.isExpanded = intersection(Object.keys(filtersFromUrl(url)), expandFilters).length > 0;

resources/js/beatmapset-search-filters.ts

@@ -91,7 +91,8 @@ export class BeatmapsetSearchFilters {
   constructor(url: string) {
     const filters = filtersFromUrl(url);
     for (const key of keyNames) {
-      this[key] = filters[key] ?? null;
+      const value = filters[key] ?? null;
+      this[key] = value === this.getDefault(key) ? null : value;
     }
 
     makeObservable(this);
@@ -145,6 +146,6 @@ export class BeatmapsetSearchFilters {
       this.sort = null;
     }
 
-    this[key] = value;
+    this[key] = value === this.getDefault(key) ? null : value;
   }
 }

tests/karma/beatmapset-search-filters.spec.ts

@@ -2,11 +2,21 @@
 // See the LICENCE file in the repository root for full licence text.
 
 import { BeatmapsetSearchFilters } from 'beatmapset-search-filters';
+import core from 'osu-core-singleton';
+import testCurrentUserJson from './test-current-user-json';
 
 describe('BeatmapsetSearchFilters', () => {
   let subject: BeatmapsetSearchFilters;
 
   describe('.query', () => {
+    beforeAll(() => {
+      core.setCurrentUser(testCurrentUserJson);
+    });
+
+    afterAll(() => {
+      core.setCurrentUser({ id: undefined });
+    });
+
     beforeEach(() => {
       subject = new BeatmapsetSearchFilters('https://notarealdomain');
     });

tests/karma/globals.js

@@ -1,20 +1,5 @@
-/**
- *    Copyright (c) ppy Pty Ltd <[email protected]>.
- *
- *    This file is part of osu!web. osu!web is distributed with the hope of
- *    attracting more community contributions to the core ecosystem of osu!.
- *
- *    osu!web is free software: you can redistribute it and/or modify
- *    it under the terms of the Affero GNU General Public License version 3
- *    as published by the Free Software Foundation.
- *
- *    osu!web is distributed WITHOUT ANY WARRANTY; without even the implied
- *    warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
- *    See the GNU Affero General Public License for more details.
- *
- *    You should have received a copy of the GNU Affero General Public License
- *    along with osu!web.  If not, see <http://www.gnu.org/licenses/>.
- */
+// 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.
 
 'use strict';
 

tests/karma/osu-core.spec.ts

@@ -1,37 +1,30 @@
 // 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.
 
-import UserJson from 'interfaces/user-json';
 import User from 'models/user';
 import OsuCore from 'osu-core';
+import testCurrentUserJson from './test-current-user-json';
 
-describe('OsuCore user:update subscriber testing thing', () => {
-  it('user:update should update the user store from a JSON value', () => {
-    const core = new OsuCore();
+const expectedUser = new User(1);
+expectedUser.updateWithJson(testCurrentUserJson);
 
-    const json: UserJson = {
-      avatar_url: '',
-      country_code: '',
-      cover: { custom_url: null, id: null, url: null },
-      default_group: '',
-      id: 1,
-      is_active: true,
-      is_bot: false,
-      is_deleted: false,
-      is_online: true,
-      is_supporter: true,
-      last_visit: null,
-      pm_friends_only: false,
-      profile_colour: null,
-      username: 'foo',
-    };
+describe('OsuCore user update', () => {
+  it('.setCurrentUser should update the user store from a JSON value', () => {
+    const core = new OsuCore();
+    core.setCurrentUser(testCurrentUserJson);
 
-    const user = new User(json.id);
-    user.updateWithJson(json);
+    expect(core.dataStore.userStore.users.get(1)).toEqual(expectedUser);
+    expect(core.dataStore.userStore.users.size).toEqual(1);
+  });
 
-    $.publish('user:update', json);
+  it('user:update subscriber should update the user store from a JSON value', () => {
+    const core = new OsuCore();
+    $.publish('user:update', testCurrentUserJson);
 
-    expect(core.dataStore.userStore.users.get(1)).toEqual(user);
+    expect(core.dataStore.userStore.users.get(1)).toEqual(expectedUser);
     expect(core.dataStore.userStore.users.size).toEqual(1);
+
+    // unset the singleton
+    $.publish('user:update', { id: undefined });
   });
 });

tests/karma/test-current-user-json.ts

@@ -0,0 +1,64 @@
+// 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.
+
+import CurrentUserJson from 'interfaces/current-user-json';
+import { defaultUserPreferencesJson } from 'interfaces/user-preferences-json';
+
+const testCurrentUserJson: CurrentUserJson = {
+  avatar_url: '',
+  blocks: [],
+  country: {
+    code: 'AU',
+    name: 'Australia',
+  },
+  country_code: 'AU',
+  cover: { custom_url: null, id: null, url: null },
+  default_group: '',
+  discord: null,
+  follow_user_mapping: [],
+  friends: [],
+  groups: [],
+  has_supported: false,
+  id: 1,
+  interests: null,
+  is_active: true,
+  is_admin: false,
+  is_bng: false,
+  is_bot: false,
+  is_deleted: false,
+  is_full_bn: false,
+  is_gmt: false,
+  is_limited_bn: false,
+  is_moderator: false,
+  is_nat: false,
+  is_online: true,
+  is_restricted: false,
+  is_silenced: false,
+  is_supporter: true,
+  join_date: '2020-01-01T12:34:56+00:00',
+  kudosu: {
+    available: 0,
+    total: 0,
+  },
+  last_visit: null,
+  location: null,
+  max_blocks: 1,
+  max_friends: 1,
+  occupation: null,
+  playmode: 'osu',
+  playstyle: [],
+  pm_friends_only: false,
+  post_count: 0,
+  profile_colour: null,
+  profile_hue: null,
+  profile_order: [],
+  title: null,
+  title_url: null,
+  twitter: null,
+  unread_pm_count: 0,
+  user_preferences: defaultUserPreferencesJson,
+  username: 'foo',
+  website: null,
+};
+
+export default testCurrentUserJson;

tests/karma/utils/beatmapset-discussion-helper.spec.ts

@@ -5,9 +5,10 @@ import BeatmapsetDiscussionJson from 'interfaces/beatmapset-discussion-json';
 import GameMode from 'interfaces/ruleset';
 import UserGroupJson from 'interfaces/user-group-json';
 import UserJson from 'interfaces/user-json';
-import User from 'models/user';
 import * as moment from 'moment';
+import core from 'osu-core-singleton';
 import { discussionMode, isUserFullNominator, maxLengthTimeline, nearbyDiscussions, validMessageLength } from 'utils/beatmapset-discussion-helper';
+import testCurrentUserJson from '../test-current-user-json';
 
 interface TestCase<T> {
   description: string;
@@ -33,24 +34,6 @@ const template: BeatmapsetDiscussionJson = Object.freeze({
   user_id: 1,
 });
 
-const currentUser = new User(1);
-currentUser.updateWithJson({
-  avatar_url: '',
-  country_code: '',
-  cover: { custom_url: null, id: null, url: null },
-  default_group: '',
-  id: 1,
-  is_active: true,
-  is_bot: false,
-  is_deleted: false,
-  is_online: true,
-  is_supporter: true,
-  last_visit: null,
-  pm_friends_only: false,
-  profile_colour: null,
-  username: 'foo',
-});
-
 describe('utils/beatmapset-discussion-helper', () => {
   describe('.discussionMode', () => {
     const cases = [
@@ -89,7 +72,7 @@ describe('utils/beatmapset-discussion-helper', () => {
   });
 
   describe('.isUserFullNominator', () => {
-    const userTemplate = currentUser.toJson();
+    const userTemplate = structuredClone(testCurrentUserJson);
     const groupsTemplate: UserGroupJson = {
       colour: null,
       has_listing: true,
@@ -184,13 +167,12 @@ describe('utils/beatmapset-discussion-helper', () => {
         },
       ];
 
-      // FIXME: need a better way of setting user in osu-core for tests.
       beforeAll(() => {
-        $.publish('user:update', currentUser);
+        core.setCurrentUser(testCurrentUserJson);
       });
 
       afterAll(() => {
-        $.publish('user:update', {});
+        core.setCurrentUser({ id: undefined });
       });
 
       cases.forEach((test) => {