Skip to content

Commit 2047fff

Browse files
Valentin Grünerclaude
andcommitted
refactor(profile): hand the profile a section, not a section's props
`ProfilePage` took the seven props of the practice-group card, read none of them, and spread them straight through. It now takes the rendered section, so it places a slot instead of carrying a type for a component it never touches — and the route that owns the queries also owns the rendering. The review-run card had one story for a component that branches on provider, artifact kind, whether the work links anywhere, and how many observations it holds. The four kinds that were wrong before this branch — a Slack thread, an Outline document, a GitLab merge request — now each have one, and two play functions cover the cases a snapshot cannot: no link means no anchor, and the disclosure counts what it is holding back. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MAQxdMw579siMh1wXKEEh4
1 parent 9f94032 commit 2047fff

4 files changed

Lines changed: 155 additions & 35 deletions

File tree

webapp/src/components/profile/ProfilePage.stories.tsx

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { STORY_NOW } from "@/components/common/story-clock";
77
import { withStandardPage } from "@/stories/decorators";
88
import { expectNoPageOverflow } from "@/test/reflow";
99

10+
import { PracticeGroupStandingCard } from "./PracticeGroupStandingCard";
1011
import { ProfilePage } from "./ProfilePage";
1112

1213
const now = new Date(STORY_NOW);
@@ -70,13 +71,15 @@ const groupPractices: PracticeStanding[] = [
7071
];
7172

7273
/** The section a developer sees on their own profile, above the activity monitor. */
73-
const practiceGroupStandings = {
74-
groups: [practiceGroup],
75-
standings: { [practiceGroup.slug]: practiceGroupStanding },
76-
practicesByGroup: { [practiceGroup.slug]: groupPractices },
77-
isLoading: false,
78-
onOpenDetails: fn(),
79-
};
74+
const practiceGroupStandings = (
75+
<PracticeGroupStandingCard
76+
groups={[practiceGroup]}
77+
standings={{ [practiceGroup.slug]: practiceGroupStanding }}
78+
practicesByGroup={{ [practiceGroup.slug]: groupPractices }}
79+
isLoading={false}
80+
onOpenDetails={fn()}
81+
/>
82+
);
8083

8184
export const Default: Story = {
8285
args: {

webapp/src/components/profile/ProfilePage.tsx

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { XCircleIcon } from "lucide-react";
2+
import type { ReactNode } from "react";
23

34
import type { Profile, ProfileActivityMonitor } from "@/api/types.gen";
45
import { Alert, AlertDescription, AlertTitle } from "@/components/ui/alert";
@@ -7,10 +8,6 @@ import type { ActivityMonitorFilters } from "@/lib/activity-monitor";
78
import type { ProviderType } from "@/lib/provider";
89
import type { LeaderboardSchedule } from "@/lib/timeframe";
910

10-
import {
11-
PracticeGroupStandingCard,
12-
type PracticeGroupStandingSectionProps,
13-
} from "./PracticeGroupStandingCard";
1411
import { ProfileContent } from "./ProfileContent";
1512
import { ProfileHeader } from "./ProfileHeader";
1613

@@ -32,7 +29,11 @@ interface ProfileProps {
3229
achievementsEnabled?: boolean;
3330
progressionEnabled?: boolean;
3431
leaguesEnabled?: boolean;
35-
practiceGroupStandings?: PracticeGroupStandingSectionProps;
32+
/**
33+
* The practice-group section, rendered by the route that has its data. A slot rather than that
34+
* component's props, so this page does not carry a type for a section it only places.
35+
*/
36+
practiceGroupStandings?: ReactNode;
3637
}
3738

3839
export function ProfilePage({
@@ -83,7 +84,7 @@ export function ProfilePage({
8384
/>
8485
{practiceGroupStandings && (
8586
<>
86-
<PracticeGroupStandingCard {...practiceGroupStandings} />
87+
{practiceGroupStandings}
8788
<Separator />
8889
</>
8990
)}

webapp/src/components/profile/ReviewRunCard.stories.tsx

Lines changed: 114 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
import type { Meta, StoryObj } from "@storybook/react";
2+
import { expect, fn } from "storybook/test";
23
import type { PracticeGroupReviewRun } from "@/api/types.gen";
4+
import { daysBefore } from "@/components/common/story-clock";
35
import { ReviewRunCard } from "./ReviewRunCard";
46

57
const run: PracticeGroupReviewRun = {
68
reviewId: "00000000-0000-0000-0000-000000000101",
7-
reviewedAt: new Date("2026-08-12T10:26:00Z"),
9+
reviewedAt: daysBefore(2),
810
reviewedWork: {
911
type: "scm.pull_request",
1012
id: 902,
@@ -53,3 +55,114 @@ export default meta;
5355
type Story = StoryObj<typeof meta>;
5456

5557
export const Default: Story = { args: { run } };
58+
59+
/**
60+
* A Slack thread. Its channel is the identity, and the mark is Slack's — the card reads the artifact
61+
* kind off the wire, where it arrives as `chat.conversation_thread` rather than a constant's name.
62+
*/
63+
export const SlackConversation: Story = {
64+
args: {
65+
run: {
66+
...run,
67+
reviewId: "00000000-0000-0000-0000-000000000201",
68+
reviewedWork: {
69+
type: "chat.conversation_thread",
70+
id: 41,
71+
provider: "SLACK",
72+
channelName: "backend-guild",
73+
url: "https://example.slack.com/archives/C01/p1700000000",
74+
},
75+
},
76+
},
77+
};
78+
79+
/** An Outline document: its own mark, not the one the default provider would have lent it. */
80+
export const OutlineDocument: Story = {
81+
args: {
82+
run: {
83+
...run,
84+
reviewId: "00000000-0000-0000-0000-000000000202",
85+
reviewedWork: {
86+
type: "docs.document",
87+
id: 77,
88+
provider: "OUTLINE",
89+
title: "Runbook: rotating the signing key",
90+
url: "https://outline.example.com/doc/runbook-rotating-the-signing-key",
91+
},
92+
},
93+
},
94+
};
95+
96+
/** A merge request on GitLab, where the number and title carry the identity. */
97+
export const GitLabMergeRequest: Story = {
98+
args: {
99+
run: {
100+
...run,
101+
reviewId: "00000000-0000-0000-0000-000000000203",
102+
reviewedWork: {
103+
...run.reviewedWork,
104+
provider: "GITLAB",
105+
number: 128,
106+
title: "Move the export retention sweep into a transaction",
107+
repositoryName: "aet/hephaestus",
108+
url: "https://gitlab.example.com/aet/hephaestus/-/merge_requests/128",
109+
},
110+
},
111+
},
112+
};
113+
114+
/** No link to follow: the identity stays plain text rather than a dead anchor. */
115+
export const WithoutALink: Story = {
116+
args: {
117+
run: {
118+
...run,
119+
reviewId: "00000000-0000-0000-0000-000000000204",
120+
reviewedWork: { ...run.reviewedWork, url: undefined, repositoryName: undefined },
121+
},
122+
},
123+
play: async ({ canvas }) => {
124+
await expect(canvas.queryByRole("link")).toBeNull();
125+
},
126+
};
127+
128+
const denseRun: PracticeGroupReviewRun = {
129+
...run,
130+
reviewId: "00000000-0000-0000-0000-000000000205",
131+
observations: [
132+
...run.observations,
133+
{
134+
observationId: "00000000-0000-0000-0000-000000000104",
135+
practiceSlug: "small-changes",
136+
practiceName: "Keep changes focused",
137+
title: "The refactor and the fix arrived together",
138+
presence: "PRESENT",
139+
assessment: "BAD",
140+
severity: "MAJOR",
141+
},
142+
{
143+
observationId: "00000000-0000-0000-0000-000000000105",
144+
practiceSlug: "covers-new-behavior",
145+
practiceName: "Cover new behavior with a test",
146+
title: "The new branch has no test exercising it",
147+
presence: "ABSENT",
148+
assessment: "BAD",
149+
severity: "CRITICAL",
150+
},
151+
],
152+
};
153+
154+
/** More observations than a card shows at once: it says how many it is holding back. */
155+
export const ManyObservations: Story = {
156+
args: { run: denseRun, onToggleObservation: fn() },
157+
play: async ({ canvas, userEvent }) => {
158+
// The fourth observation is the one held back, and the button counts it rather than saying "more".
159+
const held = "The new branch has no test exercising it";
160+
await expect(canvas.queryByText(held)).toBeNull();
161+
162+
await userEvent.click(canvas.getByRole("button", { name: "Show more (1)" }));
163+
await expect(canvas.getByText(held)).toBeVisible();
164+
165+
await userEvent.click(canvas.getByRole("button", { name: "Show less" }));
166+
await expect(canvas.queryByText(held)).toBeNull();
167+
},
168+
};

webapp/src/routes/_authenticated/w/$workspaceSlug/user/$username/index.tsx

Lines changed: 24 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
import type { PracticeStanding } from "@/api/types.gen";
1414
import { QueryErrorAlert } from "@/components/common/QueryErrorAlert";
1515
import { useNow } from "@/components/common/use-now";
16+
import { PracticeGroupStandingCard } from "@/components/profile/PracticeGroupStandingCard";
1617
import { ProfilePage } from "@/components/profile/ProfilePage";
1718
import { useWorkspaceFeatures } from "@/hooks/use-workspace-features";
1819
import { useAuth } from "@/integrations/auth/AuthContext";
@@ -218,28 +219,30 @@ function UserProfile() {
218219
progressionEnabled={progressionEnabled === true}
219220
leaguesEnabled={leaguesEnabled === true}
220221
practiceGroupStandings={
221-
currUserIsDashboardUser
222-
? {
223-
groups: practiceGroups,
224-
standings: groupStandings,
225-
practicesByGroup,
226-
isLoading:
227-
groupsQuery.isPending || groupStandingsQuery.isPending || standingsQuery.isPending,
228-
error:
229-
groupsQuery.error ?? groupStandingsQuery.error ?? standingsQuery.error ?? undefined,
230-
onRetry: () => {
231-
if (groupsQuery.isError) void groupsQuery.refetch();
232-
if (groupStandingsQuery.isError) void groupStandingsQuery.refetch();
233-
if (standingsQuery.isError) void standingsQuery.refetch();
234-
},
235-
onOpenDetails: (group) => {
236-
void navigate({
237-
to: "/w/$workspaceSlug/user/$username/practice-groups/$groupSlug",
238-
params: { workspaceSlug, username, groupSlug: group.slug },
239-
});
240-
},
222+
currUserIsDashboardUser ? (
223+
<PracticeGroupStandingCard
224+
groups={practiceGroups}
225+
standings={groupStandings}
226+
practicesByGroup={practicesByGroup}
227+
isLoading={
228+
groupsQuery.isPending || groupStandingsQuery.isPending || standingsQuery.isPending
241229
}
242-
: undefined
230+
error={
231+
groupsQuery.error ?? groupStandingsQuery.error ?? standingsQuery.error ?? undefined
232+
}
233+
onRetry={() => {
234+
if (groupsQuery.isError) void groupsQuery.refetch();
235+
if (groupStandingsQuery.isError) void groupStandingsQuery.refetch();
236+
if (standingsQuery.isError) void standingsQuery.refetch();
237+
}}
238+
onOpenDetails={(group) => {
239+
void navigate({
240+
to: "/w/$workspaceSlug/user/$username/practice-groups/$groupSlug",
241+
params: { workspaceSlug, username, groupSlug: group.slug },
242+
});
243+
}}
244+
/>
245+
) : undefined
243246
}
244247
/>
245248
);

0 commit comments

Comments
 (0)