Skip to content

Commit 080969b

Browse files
committed
Improve players stats page
1 parent dffea3b commit 080969b

9 files changed

Lines changed: 226 additions & 51 deletions

File tree

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
import React from 'react';
2+
3+
import { act, fireEvent, render, screen } from '@testing-library/react';
4+
import '@testing-library/jest-dom';
5+
6+
import PopoverStickOnHover from '../widgets/components/PopoverStickOnHover';
7+
8+
describe('PopoverStickOnHover', () => {
9+
afterEach(() => {
10+
vi.useRealTimers();
11+
});
12+
13+
test('keeps the delayed hover timer across parent rerenders', () => {
14+
vi.useFakeTimers();
15+
16+
const component = <span>user details</span>;
17+
const view = render(
18+
<PopoverStickOnHover id="user-info" delay={400} component={component}>
19+
<button type="button">Ada</button>
20+
</PopoverStickOnHover>,
21+
);
22+
23+
fireEvent.mouseEnter(screen.getByRole('button', { name: 'Ada' }));
24+
25+
view.rerender(
26+
<PopoverStickOnHover id="user-info" delay={400} component={component}>
27+
<button type="button">Ada</button>
28+
</PopoverStickOnHover>,
29+
);
30+
31+
act(() => vi.advanceTimersByTime(399));
32+
expect(screen.queryByText('user details')).not.toBeInTheDocument();
33+
34+
act(() => vi.advanceTimersByTime(1));
35+
expect(screen.getByText('user details')).toBeInTheDocument();
36+
});
37+
});

apps/codebattle/assets/js/__tests__/UserInfo.test.tsx

Lines changed: 48 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import React, { type ReactNode } from 'react';
22

3-
import { render, screen } from '@testing-library/react';
3+
import { render, screen, waitFor } from '@testing-library/react';
44
import '@testing-library/jest-dom';
55

66
import UserInfo from '../widgets/components/UserInfo';
@@ -21,10 +21,20 @@ vi.mock('../widgets/slices', () => ({
2121
}));
2222

2323
vi.mock('../widgets/components/PopoverStickOnHover', () => ({
24-
default: ({ children, component }: { children: ReactNode; component: ReactNode }) => (
24+
default: ({
25+
children,
26+
component,
27+
delay,
28+
}: {
29+
children: ReactNode;
30+
component: ReactNode;
31+
delay?: number;
32+
}) => (
2533
<>
2634
{children}
27-
<div data-testid="popover-content">{component}</div>
35+
<div data-delay={delay} data-testid="popover-content">
36+
{component}
37+
</div>
2838
</>
2939
),
3040
}));
@@ -34,10 +44,16 @@ vi.mock('../widgets/components/UserName', () => ({
3444
}));
3545

3646
vi.mock('../widgets/components/UserStats', () => ({
37-
default: () => <div>user details</div>,
47+
default: ({ data }: { data?: unknown }) => (
48+
<div>{data ? 'loaded user details' : 'loading user details'}</div>
49+
),
3850
}));
3951

4052
describe('UserInfo', () => {
53+
beforeEach(() => {
54+
dispatch.mockClear();
55+
});
56+
4157
test('renders only bot text in the tooltip without preloading user details', () => {
4258
const fetchMock = vi.fn();
4359
globalThis.fetch = fetchMock;
@@ -47,4 +63,32 @@ describe('UserInfo', () => {
4763
expect(screen.getByTestId('popover-content')).toHaveTextContent(/^bot$/);
4864
expect(fetchMock).not.toHaveBeenCalled();
4965
});
66+
67+
test('delays user popovers and shares cached requests for the same user', async () => {
68+
const fetchMock = vi.fn().mockResolvedValue({
69+
ok: true,
70+
json: vi.fn().mockResolvedValue({ achievements: [], metrics: {} }),
71+
});
72+
globalThis.fetch = fetchMock;
73+
74+
const view = render(
75+
<>
76+
<UserInfo user={{ id: 101, name: 'Ada' }} />
77+
<UserInfo user={{ id: 101, name: 'Ada' }} />
78+
</>,
79+
);
80+
81+
expect(screen.getAllByTestId('popover-content')[0]).toHaveAttribute('data-delay', '150');
82+
expect(fetchMock).toHaveBeenCalledTimes(1);
83+
84+
await waitFor(() => {
85+
expect(screen.getAllByText('loaded user details')).toHaveLength(2);
86+
});
87+
88+
view.unmount();
89+
render(<UserInfo user={{ id: 101, name: 'Ada' }} />);
90+
91+
expect(fetchMock).toHaveBeenCalledTimes(1);
92+
expect(screen.getByText('loaded user details')).toBeInTheDocument();
93+
});
5094
});

apps/codebattle/assets/js/widgets/components/PopoverStickOnHover.tsx

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -23,23 +23,34 @@ function PopoverStickOnHover({
2323
}: PopoverStickOnHoverProps) {
2424
const [showPopover, setShowPopover] = useState(false);
2525
const childNode = useRef<unknown>(null);
26-
let setTimeoutConst: ReturnType<typeof setTimeout> | null = null;
26+
const showTimeout = useRef<ReturnType<typeof setTimeout> | null>(null);
2727

28-
useEffect(() => () => {
29-
if (setTimeoutConst) {
30-
clearTimeout(setTimeoutConst);
31-
}
32-
});
28+
useEffect(
29+
() => () => {
30+
if (showTimeout.current) {
31+
clearTimeout(showTimeout.current);
32+
}
33+
},
34+
[],
35+
);
3336

3437
const handleMouseEnter = () => {
35-
setTimeoutConst = setTimeout(() => {
38+
if (showTimeout.current) {
39+
clearTimeout(showTimeout.current);
40+
}
41+
42+
showTimeout.current = setTimeout(() => {
3643
setShowPopover(true);
3744
onMouseEnter();
3845
}, delay);
3946
};
4047

4148
const handleMouseLeave = () => {
42-
clearTimeout(setTimeoutConst as ReturnType<typeof setTimeout>);
49+
if (showTimeout.current) {
50+
clearTimeout(showTimeout.current);
51+
showTimeout.current = null;
52+
}
53+
4354
setShowPopover(false);
4455
};
4556

apps/codebattle/assets/js/widgets/components/UserInfo.tsx

Lines changed: 55 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -20,46 +20,69 @@ interface UserPopoverContentProps {
2020
user: UserNameUser;
2121
}
2222

23-
function UserPopoverContent({ user }: UserPopoverContentProps) {
24-
// TODO: store stats in global redux state
25-
const dispatch = useDispatch<AppDispatch>();
23+
type UserStatsData = React.ComponentProps<typeof UserStats>['data'];
2624

27-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
28-
const [stats, setStats] = useState<any>(null);
25+
const userStatsCache = new Map<UserNameUser['id'], UserStatsData>();
26+
const userStatsRequests = new Map<UserNameUser['id'], Promise<UserStatsData>>();
2927

30-
useEffect(() => {
31-
const userId = user.id;
32-
const controller = new AbortController();
28+
const fetchUserStats = (userId: UserNameUser['id']) => {
29+
const cached = userStatsCache.get(userId);
30+
31+
if (cached) {
32+
return Promise.resolve(cached);
33+
}
34+
35+
const pendingRequest = userStatsRequests.get(userId);
36+
37+
if (pendingRequest) {
38+
return pendingRequest;
39+
}
3340

34-
fetch(`/api/v1/user/${userId}/achievements`, {
35-
signal: controller.signal,
41+
const request = fetch(`/api/v1/user/${userId}/achievements`)
42+
.then(async (response) => {
43+
if (!response.ok) {
44+
throw new Error(`Request failed with status ${response.status}`);
45+
}
46+
47+
return camelizeKeys(await response.json()) as UserStatsData;
3648
})
37-
.then(async (response) => {
38-
if (!response.ok) {
39-
throw new Error(`Request failed with status ${response.status}`);
40-
}
49+
.then((data) => {
50+
userStatsCache.set(userId, data);
51+
return data;
52+
})
53+
.finally(() => {
54+
userStatsRequests.delete(userId);
55+
});
56+
57+
userStatsRequests.set(userId, request);
58+
return request;
59+
};
60+
61+
function UserPopoverContent({ user }: UserPopoverContentProps) {
62+
const dispatch = useDispatch<AppDispatch>();
63+
const [stats, setStats] = useState<UserStatsData>(() => userStatsCache.get(user.id));
4164

42-
const data = await response.json();
65+
useEffect(() => {
66+
const userId = user.id;
67+
let mounted = true;
4368

44-
if (!controller.signal.aborted) {
45-
setStats(camelizeKeys(data));
69+
setStats(userStatsCache.get(userId));
70+
fetchUserStats(userId)
71+
.then((data) => {
72+
if (mounted) {
73+
setStats(data);
4674
}
4775
})
4876
.catch((error) => {
49-
// Aborting the in-flight request on hover-out (below) rejects with an
50-
// AbortError — that's expected teardown, not a real failure, so don't
51-
// surface it as a global error.
52-
if (controller.signal.aborted || (error as { name?: string })?.name === 'AbortError') {
53-
return;
77+
if (mounted) {
78+
dispatch(actions.setError(error));
5479
}
55-
56-
dispatch(actions.setError(error));
5780
});
5881

5982
return () => {
60-
controller.abort();
83+
mounted = false;
6184
};
62-
}, [dispatch, setStats, user.id]);
85+
}, [dispatch, user.id]);
6386

6487
// UserStats expects a stricter user shape (numeric id); UserNameUser is broader.
6588
// eslint-disable-next-line @typescript-eslint/no-explicit-any
@@ -141,7 +164,12 @@ function UserInfo({
141164
}
142165

143166
return (
144-
<PopoverStickOnHover id={`user-info-${user?.id}`} placement={placement} component={content}>
167+
<PopoverStickOnHover
168+
id={`user-info-${user?.id}`}
169+
delay={150}
170+
placement={placement}
171+
component={content}
172+
>
145173
<div>
146174
<UserName
147175
className={userClassName}

apps/codebattle/lib/codebattle/game/context.ex

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -457,16 +457,15 @@ defmodule Codebattle.Game.Context do
457457
end
458458

459459
def get_active_game_id(user_id) do
460-
Game
461-
|> where([g], g.state == "playing")
462-
|> where([g], fragment("? = ANY(player_ids)", ^user_id))
463-
# |> where([g], g.inserted_at > fragment("now() - interval '30 minutes'"))
464-
|> order_by([g], desc: g.id)
465-
|> Repo.all()
466-
|> case do
467-
[%Game{id: id} | _] -> id
468-
_ -> nil
469-
end
460+
Repo.one(
461+
from(g in Game,
462+
where: g.state == "playing",
463+
where: fragment("? @> ARRAY[?]::integer[]", g.player_ids, ^user_id),
464+
order_by: [desc: g.id],
465+
limit: 1,
466+
select: g.id
467+
)
468+
)
470469
end
471470

472471
def report_on_player(game_id, reporter, offender_id) do

apps/codebattle/lib/codebattle/user/stats.ex

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ defmodule Codebattle.User.Stats do
1515
user_games_stats =
1616
Repo.all(
1717
from(ug in UserGame,
18-
select: %{result: ug.result, lang: ug.lang, count: count(ug.id)},
18+
select: %{result: ug.result, lang: ug.lang, count: count()},
1919
where: ug.user_id == ^user_id,
2020
where: ug.result in ["won", "lost", "gave_up"],
2121
group_by: [ug.result, ug.lang]

apps/codebattle/lib/codebattle_web/controllers/api/v1/user_controller.ex

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -276,8 +276,8 @@ defmodule CodebattleWeb.Api.V1.UserController do
276276

277277
user ->
278278
active_game_id = Game.Context.get_active_game_id(id)
279-
stats = UserStats.get_game_stats(id)
280279
achievements = Achievements.get_user_achievements(id)
280+
stats = if include_stats, do: UserStats.get_game_stats(id), else: %{games: %{}, all: []}
281281
metrics = build_metrics_from_achievements(achievements, stats)
282282

283283
payload = %{
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
defmodule Codebattle.Repo.Migrations.AddUserProfileQueryIndexes do
2+
@moduledoc false
3+
use Ecto.Migration
4+
5+
@disable_ddl_transaction true
6+
@disable_migration_lock true
7+
8+
def up do
9+
execute("""
10+
CREATE INDEX CONCURRENTLY IF NOT EXISTS user_games_user_id_result_lang_stats_idx
11+
ON user_games (user_id, result, lang)
12+
WHERE result IN ('won', 'lost', 'gave_up')
13+
""")
14+
15+
execute("""
16+
CREATE INDEX CONCURRENTLY IF NOT EXISTS games_playing_player_ids_idx
17+
ON games USING gin (player_ids)
18+
WHERE state = 'playing'
19+
""")
20+
end
21+
22+
def down do
23+
execute("DROP INDEX CONCURRENTLY IF EXISTS user_games_user_id_result_lang_stats_idx")
24+
execute("DROP INDEX CONCURRENTLY IF EXISTS games_playing_player_ids_idx")
25+
end
26+
end

apps/codebattle/test/codebattle_web/controllers/api/v1/user_controller_test.exs

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -300,6 +300,36 @@ defmodule CodebattleWeb.Api.V1.UserControllerTest do
300300
assert is_list(achievements)
301301
end
302302

303+
test "uses denormalized achievements without scanning user games", %{conn: conn} do
304+
user = insert(:user)
305+
game = insert(:game, state: "game_over")
306+
insert(:user_game, user: user, game: game, result: "won", lang: "js")
307+
:ok = Achievements.recalculate_user(user.id)
308+
309+
handler_id = "user-achievements-query-test-#{System.unique_integer()}"
310+
test_process = self()
311+
312+
:ok =
313+
:telemetry.attach(
314+
handler_id,
315+
[:codebattle, :repo, :query],
316+
fn _event, _measurements, %{query: query}, _config ->
317+
if String.contains?(query, ~s[FROM "user_games"]) do
318+
send(test_process, :queried_user_games)
319+
end
320+
end,
321+
nil
322+
)
323+
324+
on_exit(fn -> :telemetry.detach(handler_id) end)
325+
326+
conn
327+
|> get(Routes.api_v1_user_path(conn, :achievements, user.id))
328+
|> json_response(200)
329+
330+
refute_receive :queried_user_games
331+
end
332+
303333
test "returns 404 for missing user", %{conn: conn} do
304334
resp_body =
305335
conn

0 commit comments

Comments
 (0)