From 51d9f45a724c1e801ecf20ae21d8ec9b7ba876d5 Mon Sep 17 00:00:00 2001 From: Anettkva Date: Fri, 30 Oct 2026 20:55:49 +0100 Subject: [PATCH] fix: show the vault group number instead of the description on the element page - Ask for elementGroup.number (and id, so the cache recognises the group) in the catalogItem query - Show "Group N" for a vault and the group name for an element, with the same groupLabel as the Elements list, so the two pages agree - Test an element and a vault separately on the element page - Add the issue to AI_USAGE.md Closes #125 Co-Authored-By: Claude Opus 5.5 --- docs/AI_USAGE.md | 6 ++ .../elements/ElementActions.test.tsx | 1 + .../elements/ElementDetailHeader.test.tsx | 1 + .../elements/ElementDetailPage.test.tsx | 57 ++++++++++++++++++- .../elements/ElementMetadataCard.tsx | 8 +-- src/lib/catalog.test.ts | 8 ++- src/lib/catalog.ts | 3 + src/lib/catalogOperations.ts | 9 ++- src/lib/elementList.ts | 7 ++- 9 files changed, 91 insertions(+), 9 deletions(-) diff --git a/docs/AI_USAGE.md b/docs/AI_USAGE.md index 5a70fb7..3f9294b 100644 --- a/docs/AI_USAGE.md +++ b/docs/AI_USAGE.md @@ -514,3 +514,9 @@ - **What the team did:** Read the requirement of the assignment ("a form of automated end-to-end testing ... a longer sequence of interactions, and testing of the API") and wrote issue #139. Decided that the tests are run against the virtual machine too, and that atypical use and accessibility are part of them. - **What Claude did:** Checked what the requirement means and what the repository already had (component tests and API tests, but no test that drives the whole application). Chose Playwright (ready-made, waits for the page by itself) with the official axe-core integration, and wrote the configuration so that the tests start the app themselves or run against any address (`E2E_BASE_URL`). Wrote one long flow, the vault bands, atypical use (wrong addresses, no result, a failing request, keyboard only, the disabled filter), axe-core checks of the page in its main states, and tests of the API over HTTP. Ran them against the real backend and the real catalog with Playwright's own Chromium and with Edge, and broke the application on purpose in four places to check that a test fails each time. Wrote the README section, including what the tests do not cover (a screen reader, and a run on every pull request, because the course server has no runners). - **What was found:** The first run had 4 failing tests, all in the tests and not in the application: Playwright does not click a control that is `aria-disabled`, one expectation was wrong, and a check ran before the new results had arrived (the page keeps the old results on the screen until the new ones are there). + +## The vault group on the element page (`src/components/elements/ElementMetadataCard.tsx`, `src/lib/catalogOperations.ts`) + +- **Tool:** Claude Code +- **What the team did:** Found in a review that the element page shows the long description of a vault group under "Element group", and wrote issue #125: show "Group N" for a vault, with the same rule as the Elements list. +- **What Claude did:** Added `number` (and `id`, so Apollo's cache can recognise the group across queries) to the `catalogItem` query, and let the element page use `groupLabel` from the Elements list, so the two pages cannot disagree. Wrote one test for an element and one for a vault (with the real group of VT-1.22-a), and checked that each fails when the page shows the name for every item or "Group N" for every item. After a review of its own work, split one test that checked two cases, and replaced test data for the vault that was half an element. diff --git a/src/components/elements/ElementActions.test.tsx b/src/components/elements/ElementActions.test.tsx index 188ca97..cf4306f 100644 --- a/src/components/elements/ElementActions.test.tsx +++ b/src/components/elements/ElementActions.test.tsx @@ -13,6 +13,7 @@ const element: ElementDetail = { apparatusId: 'UB', apparatusName: 'Uneven bars', elementGroupName: 'Casts and clear hip circles', + elementGroupNumber: 2, imageUrl: 'http://localhost:3001/images/UB/UB-2.305-a.webp', removed: false, variants: [], diff --git a/src/components/elements/ElementDetailHeader.test.tsx b/src/components/elements/ElementDetailHeader.test.tsx index 3aeb123..83331e5 100644 --- a/src/components/elements/ElementDetailHeader.test.tsx +++ b/src/components/elements/ElementDetailHeader.test.tsx @@ -14,6 +14,7 @@ const element: ElementDetail = { apparatusId: 'UB', apparatusName: 'Uneven bars', elementGroupName: 'Giant circles', + elementGroupNumber: 3, imageUrl: 'http://localhost:3001/images/UB/UB-2.305-a.webp', removed: false, variants: [], diff --git a/src/components/elements/ElementDetailPage.test.tsx b/src/components/elements/ElementDetailPage.test.tsx index f8c9ca8..f2c48ca 100644 --- a/src/components/elements/ElementDetailPage.test.tsx +++ b/src/components/elements/ElementDetailPage.test.tsx @@ -3,7 +3,13 @@ // src/tests/apollo.ts). import { ApolloProvider } from '@apollo/client/react' -import { cleanup, fireEvent, render, screen } from '@testing-library/react' +import { + cleanup, + fireEvent, + render, + screen, + within, +} from '@testing-library/react' import { MemoryRouter, Route, Routes } from 'react-router-dom' import { afterEach, describe, expect, it } from 'vitest' import { @@ -35,7 +41,9 @@ const elementAnswer = (id: string, overrides: object = {}) => removed: false, elementGroup: { __typename: 'ElementGroup', + id: 'UB-2', name: 'Casts and clear hip circles', + number: 2, apparatus: { __typename: 'Apparatus', id: 'UB', name: 'Uneven bars' }, }, variants: [variant(id === 'UB-2.305-a' ? 'UB-2.305-b' : 'UB-2.305-a')], @@ -43,6 +51,32 @@ const elementAnswer = (id: string, overrides: object = {}) => }, }) +/** + * What the server says to `CatalogItem` for a vault. The name of a vault group is a + * long description, which the page must not show as the group. + */ +const vaultAnswer = () => + ok({ + catalogItem: { + __typename: 'Vault', + id: 'VT-1.22-a', + code: '1.22', + name: null, + description: 'Handspring fwd with 1/2 turn on, 1/2 turn off', + dValue: 2.6, + imageUrl: 'http://images.test/VT/VT-1.22-a.webp', + removed: false, + elementGroup: { + __typename: 'ElementGroup', + id: 'VT-1', + name: 'Handspring, Yamashita, round-off with or without turn in 1st and/or 2nd flight phase', + number: 1, + apparatus: { __typename: 'Apparatus', id: 'VT', name: 'Vault' }, + }, + variants: [], + }, + }) + function renderPage(id: string, handler: Handler) { const { client } = createTestClient(handler) render( @@ -59,6 +93,14 @@ function renderPage(id: string, handler: Handler) { /** The id that the page asks `catalogItem` for. */ const idOf = (operation: Operation) => String(operation.variables['id']) +/** What the page shows under "Element group". */ +const groupShown = async () => { + const label = within( + await screen.findByRole('region', { name: 'Element metadata' }), + ).getByText('Element group') + return label.nextElementSibling?.textContent +} + const title = () => screen.getByRole('heading', { level: 1 }).textContent const button = (name: string) => screen.getByRole('button', { name }) as HTMLButtonElement @@ -105,6 +147,19 @@ describe('ElementDetailPage', () => { expect(button('Try in a routine').disabled).toBe(false) }) + it('shows the name of the group for an element', async () => { + renderPage('UB-2.305-a', (operation) => elementAnswer(idOf(operation))) + + expect(await groupShown()).toBe('Casts and clear hip circles') + }) + + it('shows "Group N" for a vault, and not the description of its group', async () => { + renderPage('VT-1.22-a', () => vaultAnswer()) + + expect(await groupShown()).toBe('Group 1') + expect(screen.queryByText(/Handspring, Yamashita/)).toBeNull() + }) + it('warns about a removed element and turns off both buttons', async () => { renderPage('UB-2.305-a', (operation) => elementAnswer(idOf(operation), { removed: true }), diff --git a/src/components/elements/ElementMetadataCard.tsx b/src/components/elements/ElementMetadataCard.tsx index 19768e0..773ec1f 100644 --- a/src/components/elements/ElementMetadataCard.tsx +++ b/src/components/elements/ElementMetadataCard.tsx @@ -4,6 +4,7 @@ import DifficultyBadge from '@/components/ui/DifficultyBadge' import ElementValueBadge from '@/components/ui/ElementValueBadge' import type { ElementDetail } from '@/lib/catalog' import { getDifficultyLetter } from '@/lib/difficulty' +import { groupLabel } from '@/lib/elementList' function MetadataItem({ label, @@ -40,10 +41,7 @@ export default function ElementMetadataCard({
{element.isVault || !difficulty ? ( - + ) : ( )} @@ -52,7 +50,7 @@ export default function ElementMetadataCard({ {element.apparatusName} - {element.elementGroupName} + {groupLabel(element)}
diff --git a/src/lib/catalog.test.ts b/src/lib/catalog.test.ts index edffe05..ef24f8e 100644 --- a/src/lib/catalog.test.ts +++ b/src/lib/catalog.test.ts @@ -7,7 +7,9 @@ const shared = { imageUrl: 'http://localhost:3001/images/UB/UB-2.305-a.webp', removed: false, elementGroup: { + id: 'UB-2', name: 'Casts and clear hip circles', + number: 2, apparatus: { id: 'UB', name: 'Uneven bars' }, }, } @@ -56,7 +58,11 @@ describe('toElementDetail', () => { variants: [], } - expect(toElementDetail(vault)).toMatchObject({ value: 2.6, isVault: true }) + expect(toElementDetail(vault)).toMatchObject({ + value: 2.6, + isVault: true, + elementGroupNumber: 2, + }) }) }) diff --git a/src/lib/catalog.ts b/src/lib/catalog.ts index 2a781fb..9b70f2b 100644 --- a/src/lib/catalog.ts +++ b/src/lib/catalog.ts @@ -13,6 +13,8 @@ export type ElementDetail = CatalogVariant & { apparatusId: string apparatusName: string elementGroupName: string + /** The number of the group on the apparatus, shown for a vault (see `groupLabel`). */ + elementGroupNumber: number imageUrl: string removed: boolean variants: CatalogVariant[] @@ -51,6 +53,7 @@ export function toElementDetail(item: CatalogItemResult): ElementDetail { apparatusId: item.elementGroup.apparatus.id, apparatusName: item.elementGroup.apparatus.name, elementGroupName: item.elementGroup.name, + elementGroupNumber: item.elementGroup.number, imageUrl: item.imageUrl, removed: item.removed, variants: item.variants.map(toCatalogVariant), diff --git a/src/lib/catalogOperations.ts b/src/lib/catalogOperations.ts index f02362d..51e86d6 100644 --- a/src/lib/catalogOperations.ts +++ b/src/lib/catalogOperations.ts @@ -14,7 +14,12 @@ export type VariantFields = { export type CatalogItemResult = VariantFields & { imageUrl: string removed: boolean - elementGroup: { name: string; apparatus: { id: string; name: string } } + elementGroup: { + id: string + name: string + number: number + apparatus: { id: string; name: string } + } } & ( | { __typename: 'Element' @@ -41,7 +46,9 @@ export const CATALOG_ITEM_QUERY: TypedDocumentNode< imageUrl removed elementGroup { + id name + number apparatus { id name diff --git a/src/lib/elementList.ts b/src/lib/elementList.ts index e0142d9..9f1f85e 100644 --- a/src/lib/elementList.ts +++ b/src/lib/elementList.ts @@ -25,5 +25,10 @@ export type ElementListRow = { * therefore shown by the number of its group ("Group 4"), and an element by the name * of its group ("Mounts"). */ -export const groupLabel = (row: ElementListRow): string => +export const groupLabel = ( + row: Pick< + ElementListRow, + 'isVault' | 'elementGroupName' | 'elementGroupNumber' + >, +): string => row.isVault ? `Group ${row.elementGroupNumber}` : row.elementGroupName