From b9cef64860928674a39117dc1c41a4f5c7fcfc6f Mon Sep 17 00:00:00 2001 From: Kouji Takao Date: Thu, 21 May 2026 13:27:28 +0900 Subject: [PATCH 01/10] fix(classroom): migrate session persistence from sessionStorage to localStorage Student session was persisted to sessionStorage even though the docs and the in-app behavior promised localStorage-style persistence. As a result: - A new tab opened from a `?classcode=` link saw an empty session and offered the seat-selection screen, where the student's own seat was shown as occupied (because the original tab still held the membership on the server). - A browser restart silently lost the membership state on the client, while the server-side seat remained taken until TTL. Move persistence to localStorage so that new tabs and browser restarts restore the student session, matching the documented behavior. A one-shot migration on module load promotes any pre-existing sessionStorage value to localStorage (without overwriting an existing localStorage value) and always clears the legacy sessionStorage entry so it stops resurfacing. Refs #692 Co-Authored-By: Claude Opus 4.7 (1M context) --- .../scratch-gui/src/reducers/classroom.js | 93 +++++++--- .../unit/reducers/classroom-reducer.test.js | 160 ++++++++++++++++++ 2 files changed, 229 insertions(+), 24 deletions(-) diff --git a/packages/scratch-gui/src/reducers/classroom.js b/packages/scratch-gui/src/reducers/classroom.js index 4fa5cc320cf..c9b978737a5 100644 --- a/packages/scratch-gui/src/reducers/classroom.js +++ b/packages/scratch-gui/src/reducers/classroom.js @@ -11,51 +11,96 @@ const CLEAR_TEACHER_SELECTION = 'scratch-gui/classroom/CLEAR_TEACHER_SELECTION'; const STORAGE_KEY = 'smalruby:classroom'; -/** - * Load classroom session from localStorage. - * @returns {object|null} Stored session or null - */ -const loadSession = () => { - if (typeof window === 'undefined' || !window.sessionStorage) return null; +const hasLocalStorage = () => typeof window !== 'undefined' && !!window.localStorage; +const hasSessionStorage = () => typeof window !== 'undefined' && !!window.sessionStorage; + +const parseStoredSession = (raw) => { + if (!raw) return null; try { - const raw = window.sessionStorage.getItem(STORAGE_KEY); - if (!raw) return null; const data = JSON.parse(raw); if (data && data.sessionToken && data.classroomId) return data; - return null; + } catch { + // fall through + } + return null; +}; + +// One-shot migration from the legacy sessionStorage location to localStorage. +// Older builds persisted the student session to sessionStorage, which meant a +// fresh tab opened from a `?classcode=` link had no session and the student's +// own seat appeared occupied. We promote the legacy value to localStorage on +// load, and always clear the legacy key so it stops resurfacing. +const migrateLegacySessionStorage = () => { + if (!hasSessionStorage()) return; + let legacyRaw = null; + try { + legacyRaw = window.sessionStorage.getItem(STORAGE_KEY); + } catch { + return; + } + if (legacyRaw === null) return; + try { + window.sessionStorage.removeItem(STORAGE_KEY); + } catch { + // ignore + } + if (!hasLocalStorage()) return; + const existing = (() => { + try { + return window.localStorage.getItem(STORAGE_KEY); + } catch { + return null; + } + })(); + if (existing) return; + const parsed = parseStoredSession(legacyRaw); + if (!parsed) return; + try { + window.localStorage.setItem(STORAGE_KEY, JSON.stringify(parsed)); + } catch { + // ignore + } +}; + +const loadSession = () => { + migrateLegacySessionStorage(); + if (!hasLocalStorage()) return null; + try { + return parseStoredSession(window.localStorage.getItem(STORAGE_KEY)); } catch { return null; } }; -/** - * Save classroom session to localStorage. - * @param {object} session - Session data to save - */ const saveSession = (session) => { - if (typeof window === 'undefined' || !window.sessionStorage) return; + if (!hasLocalStorage()) return; try { - window.sessionStorage.setItem(STORAGE_KEY, JSON.stringify(session)); + window.localStorage.setItem(STORAGE_KEY, JSON.stringify(session)); } catch { // Ignore storage errors } }; -/** - * Clear classroom session from localStorage. - */ const clearStoredSession = () => { - if (typeof window === 'undefined' || !window.sessionStorage) return; - try { - window.sessionStorage.removeItem(STORAGE_KEY); - } catch { - // Ignore storage errors + if (hasLocalStorage()) { + try { + window.localStorage.removeItem(STORAGE_KEY); + } catch { + // ignore + } + } + if (hasSessionStorage()) { + try { + window.sessionStorage.removeItem(STORAGE_KEY); + } catch { + // ignore + } } }; // teacherSelection is intentionally NOT persisted across page reloads. The // teacher idToken (use-teacher-auth.js) is module-scope in-memory only and -// clears on reload, so persisting the selection in sessionStorage would leave +// clears on reload, so persisting the selection in localStorage would leave // the Mesh domain bound to a class the teacher is no longer logged in for. const storedSession = loadSession(); diff --git a/packages/scratch-gui/test/unit/reducers/classroom-reducer.test.js b/packages/scratch-gui/test/unit/reducers/classroom-reducer.test.js index 1d1e4e3f89e..bfb3a337098 100644 --- a/packages/scratch-gui/test/unit/reducers/classroom-reducer.test.js +++ b/packages/scratch-gui/test/unit/reducers/classroom-reducer.test.js @@ -9,7 +9,14 @@ import reducer, { clearTeacherSelection, } from '../../../src/reducers/classroom'; +const STORAGE_KEY = 'smalruby:classroom'; + describe('classroom reducer', () => { + beforeEach(() => { + window.localStorage.clear(); + window.sessionStorage.clear(); + }); + test('should return initial state', () => { const state = reducer(undefined, { type: 'UNKNOWN' }); expect(state.modalVisible).toBe(false); @@ -161,4 +168,157 @@ describe('classroom reducer', () => { expect(next.teacherSelection).toEqual(state.teacherSelection); }); }); + + describe('storage persistence (localStorage)', () => { + const baseSession = { + role: 'student', + classroomId: 'class-123', + className: '2年A組', + assignmentName: '宿題1', + joinCode: 'abc234', + seatNumber: 5, + memberId: 'seat-05', + sessionToken: 'token-abc', + joinedAt: '2026-05-21T01:00:00.000Z', + }; + + test('writes session to localStorage (not sessionStorage) on SET_SESSION', () => { + reducer(classroomInitialState, setClassroomSession(baseSession)); + const stored = window.localStorage.getItem(STORAGE_KEY); + expect(stored).not.toBeNull(); + expect(JSON.parse(stored)).toMatchObject({ + role: 'student', + classroomId: 'class-123', + seatNumber: 5, + sessionToken: 'token-abc', + }); + expect(window.sessionStorage.getItem(STORAGE_KEY)).toBeNull(); + }); + + test('removes session from localStorage on CLEAR_SESSION', () => { + reducer(classroomInitialState, setClassroomSession(baseSession)); + expect(window.localStorage.getItem(STORAGE_KEY)).not.toBeNull(); + + reducer( + { + ...classroomInitialState, + role: 'student', + sessionToken: 'token-abc', + }, + clearClassroomSession(), + ); + expect(window.localStorage.getItem(STORAGE_KEY)).toBeNull(); + }); + + test('clears any leftover sessionStorage on CLEAR_SESSION', () => { + window.sessionStorage.setItem(STORAGE_KEY, JSON.stringify(baseSession)); + window.localStorage.setItem(STORAGE_KEY, JSON.stringify(baseSession)); + + reducer( + { + ...classroomInitialState, + role: 'student', + sessionToken: 'token-abc', + }, + clearClassroomSession(), + ); + expect(window.localStorage.getItem(STORAGE_KEY)).toBeNull(); + expect(window.sessionStorage.getItem(STORAGE_KEY)).toBeNull(); + }); + + test('persists submission status updates to localStorage', () => { + reducer(classroomInitialState, setClassroomSession(baseSession)); + const stateWithSession = { + ...classroomInitialState, + role: 'student', + sessionToken: 'token-abc', + classroomId: 'class-123', + seatNumber: 5, + memberId: 'seat-05', + joinCode: 'abc234', + className: '2年A組', + assignmentName: '宿題1', + joinedAt: baseSession.joinedAt, + }; + reducer(stateWithSession, setSubmissionStatus('submitted', '2026-05-21T01:30:00.000Z')); + + const stored = JSON.parse(window.localStorage.getItem(STORAGE_KEY)); + expect(stored.submissionStatus).toBe('submitted'); + expect(stored.lastSubmittedAt).toBe('2026-05-21T01:30:00.000Z'); + }); + }); + + describe('module-load migration (sessionStorage → localStorage)', () => { + const legacySession = { + role: 'student', + classroomId: 'class-legacy', + className: '6年2組', + assignmentName: '宿題X', + joinCode: 'legacy', + seatNumber: 7, + memberId: 'seat-07', + sessionToken: 'legacy-token', + joinedAt: '2026-05-20T00:00:00.000Z', + }; + + beforeEach(() => { + jest.resetModules(); + window.localStorage.clear(); + window.sessionStorage.clear(); + }); + + test('migrates legacy sessionStorage session to localStorage and clears sessionStorage on load', () => { + window.sessionStorage.setItem(STORAGE_KEY, JSON.stringify(legacySession)); + + // Re-import to trigger module-load migration logic. + const { classroomInitialState: freshInitialState } = require('../../../src/reducers/classroom'); + + expect(window.sessionStorage.getItem(STORAGE_KEY)).toBeNull(); + const migrated = window.localStorage.getItem(STORAGE_KEY); + expect(migrated).not.toBeNull(); + expect(JSON.parse(migrated)).toMatchObject({ + classroomId: 'class-legacy', + seatNumber: 7, + sessionToken: 'legacy-token', + }); + + // Initial state should reflect the restored session. + expect(freshInitialState.role).toBe('student'); + expect(freshInitialState.classroomId).toBe('class-legacy'); + expect(freshInitialState.sessionToken).toBe('legacy-token'); + }); + + test('does not overwrite an existing localStorage session with sessionStorage value', () => { + const newSession = { ...legacySession, sessionToken: 'localstorage-token' }; + window.localStorage.setItem(STORAGE_KEY, JSON.stringify(newSession)); + window.sessionStorage.setItem(STORAGE_KEY, JSON.stringify(legacySession)); + + const { classroomInitialState: freshInitialState } = require('../../../src/reducers/classroom'); + + const stored = JSON.parse(window.localStorage.getItem(STORAGE_KEY)); + expect(stored.sessionToken).toBe('localstorage-token'); + // Stale sessionStorage value is cleared either way to avoid future confusion. + expect(window.sessionStorage.getItem(STORAGE_KEY)).toBeNull(); + expect(freshInitialState.sessionToken).toBe('localstorage-token'); + }); + + test('initial state is null when neither storage has a session', () => { + const { classroomInitialState: freshInitialState } = require('../../../src/reducers/classroom'); + expect(freshInitialState.role).toBeNull(); + expect(freshInitialState.classroomId).toBeNull(); + expect(freshInitialState.sessionToken).toBeNull(); + }); + + test('ignores malformed sessionStorage payload without throwing', () => { + window.sessionStorage.setItem(STORAGE_KEY, '{ not valid json'); + + const { classroomInitialState: freshInitialState } = require('../../../src/reducers/classroom'); + + expect(freshInitialState.role).toBeNull(); + expect(freshInitialState.sessionToken).toBeNull(); + // Malformed legacy data should still be cleared so it stops surfacing. + expect(window.sessionStorage.getItem(STORAGE_KEY)).toBeNull(); + expect(window.localStorage.getItem(STORAGE_KEY)).toBeNull(); + }); + }); }); From 1e80b67612576652ed0ad2d725568d027c6cb3c8 Mon Sep 17 00:00:00 2001 From: Kouji Takao Date: Thu, 21 May 2026 13:52:58 +0900 Subject: [PATCH 02/10] feat(classroom): mark kicked members and surface reason via verify-session MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a teacher removes a student via DELETE /classrooms/{id}/members/{memberId}, the next thing the student does is open the classroom modal and watch the UI say "session expired" — the same message they would see for a TTL expiry or a deleted classroom. They have no way to tell that the teacher just kicked them on purpose, and the seat-selection flow can't auto-rehydrate to let them pick a different number. Switch the kick from a hard delete to a tombstone so the next request from the kicked sessionToken can be distinguished from the generic auth failure: - handleDeleteMember now SETs `kicked=true, kickedAt, kickJoinCode, kickClassName, kickSeatNumber` and shortens the row's TTL to KICK_TOMBSTONE_TTL_SECONDS (1h). If the row was already gone, we treat the request as success (idempotent). - verifySessionToken throws a new KickedError when it sees `kicked=true`. The top-level handler converts KickedError to a 410 response carrying `{reason: 'kicked', joinCode, className, seatNumber}` so the student UI can navigate straight back into seat selection for the same classroom. - handleListMembers and handleLookupClassroom add a FilterExpression to skip tombstones — the seat appears free to the teacher's grid and to the takenSeats list immediately after the kick. - handleJoinClassroom relaxes its ConditionExpression to `attribute_not_exists(memberId) OR kicked = :true`, so a freshly kicked seat can be re-occupied without a separate cleanup pass. The new row drops the kick attributes, so the old sessionToken naturally stops resolving via the sessionToken-index and falls back to the standard 401 path. Integration tests added against the stg endpoint exercise the full sequence (join → verify=200 → kick → verify=410 + payload → listMembers excludes → lookup takenSeats excludes → another student joins same seat → original sessionToken now resolves to 401). Note: one pre-existing test failure remains in the lookup describe block (expects `data.className`/`assignmentName` to be undefined while the handler returns them); it predates this commit (added by aa35f9a7b9) and is left for a separate cleanup. Refs #692 Co-Authored-By: Claude Opus 4.7 (1M context) --- infra/smalruby-classroom/lambda/handler.ts | 117 ++++++++++++++++-- .../lambda/tests/handler.integration.test.ts | 110 ++++++++++++++++ 2 files changed, 216 insertions(+), 11 deletions(-) diff --git a/infra/smalruby-classroom/lambda/handler.ts b/infra/smalruby-classroom/lambda/handler.ts index 214ae853961..1ff9527a6c1 100644 --- a/infra/smalruby-classroom/lambda/handler.ts +++ b/infra/smalruby-classroom/lambda/handler.ts @@ -185,6 +185,29 @@ class GoogleAPIError extends Error { } } +// Tombstone error: the member's row exists but is flagged kicked. Surface this +// to the student so the UI can show a specific "you were removed by the +// teacher" banner instead of the generic "session expired" alert. +class KickedError extends Error { + joinCode: string; + className: string; + seatNumber: number; + constructor(joinCode: string, className: string, seatNumber: number) { + super('You were removed from the classroom by the teacher'); + this.name = 'KickedError'; + this.joinCode = joinCode; + this.className = className; + this.seatNumber = seatNumber; + } +} + +// Kick tombstone TTL: how long after a teacher kick we keep the row around so +// the kicked student's next verify-session can read the reason. Anything beyond +// this (1 hour) and we don't bother — the student will hit the regular "session +// expired" path. The tombstone is also consumed proactively when another +// student joins the seat. +const KICK_TOMBSTONE_TTL_SECONDS = parseInt(process.env.KICK_TOMBSTONE_TTL_SECONDS || '3600', 10); + // --- Google Classroom API proxy --- async function callGoogleClassroomAPI( @@ -557,7 +580,12 @@ async function handleJoinClassroom(sourceIp: string, body: Record :true', + ExpressionAttributeValues: { ':cid': classroomId, ':true': true }, })), docClient.send(new QueryCommand({ TableName: SUBMISSIONS_TABLE, @@ -717,11 +749,13 @@ async function handleLookupClassroom(sourceIp: string, body: Record :true', + ExpressionAttributeValues: { ':cid': classroom.classroomId, ':true': true }, ProjectionExpression: 'memberId', })); @@ -753,10 +787,44 @@ async function handleDeleteMember(teacherSub: string, classroomId: string, membe throw new AuthError('Not authorized to modify this classroom'); } - await docClient.send(new DeleteCommand({ - TableName: MEMBERSHIPS_TABLE, - Key: { classroomId, memberId }, - })); + // Soft-kick: mark the row as kicked instead of hard-deleting so the next + // verify-session call can return reason='kicked'. Hard-delete would yield + // the same 401 as a TTL expiry and the student would see a generic message. + // The tombstone TTL is shortened to KICK_TOMBSTONE_TTL_SECONDS (1h) so we + // don't keep dead rows around forever; the seat is freed immediately because + // handleLookupClassroom / handleListMembers / handleJoinClassroom all treat + // `kicked === true` as "gone". + const seatMatch = memberId.match(/^seat-(\d+)$/); + const seatNumber = seatMatch ? parseInt(seatMatch[1], 10) : 0; + try { + await docClient.send(new UpdateCommand({ + TableName: MEMBERSHIPS_TABLE, + Key: { classroomId, memberId }, + UpdateExpression: + 'SET kicked = :true, kickedAt = :now, kickJoinCode = :jc, kickClassName = :cn, kickSeatNumber = :sn, #ttl = :ttl', + ConditionExpression: 'attribute_exists(memberId)', + ExpressionAttributeNames: { '#ttl': 'ttl' }, + ExpressionAttributeValues: { + ':true': true, + ':now': new Date().toISOString(), + ':jc': classroom.Item.joinCode, + ':cn': classroom.Item.className, + ':sn': seatNumber, + ':ttl': Math.floor(Date.now() / 1000) + KICK_TOMBSTONE_TTL_SECONDS, + }, + })); + } catch (err: unknown) { + // If the row was already gone (e.g. student left first), nothing to do. + if ( + err && + typeof err === 'object' && + 'name' in err && + err.name === 'ConditionalCheckFailedException' + ) { + return { statusCode: 204, body: '' }; + } + throw err; + } return { statusCode: 204, body: '' }; } @@ -777,6 +845,16 @@ export async function verifySessionToken(sessionToken: string): Promise<{ classr } const item = result.Items[0]; + // Surface kick tombstones as a distinct error so callers can return 410 + // with the reason. Non-verify-session callers (e.g. submission endpoints) + // also benefit: a kicked student shouldn't be able to submit any more. + if (item.kicked === true) { + throw new KickedError( + (item.kickJoinCode as string) || '', + (item.kickClassName as string) || '', + (item.kickSeatNumber as number) || 0, + ); + } return { classroomId: item.classroomId as string, memberId: item.memberId as string }; } @@ -1261,7 +1339,11 @@ async function handlePostAssignment( } async function handleVerifySession(sessionToken: string): Promise { - // verifySessionToken will throw AuthError if invalid + // verifySessionToken throws AuthError for unknown/expired tokens and + // KickedError when the row exists but the teacher removed the student. + // KickedError is caught in the top-level handler and converted to a 410 + // response with reason='kicked' + joinCode/className/seatNumber so the + // student UI can navigate back to seat selection with the right context. const session = await verifySessionToken(sessionToken); // Update lastActiveAt and extend TTL on each verify call @@ -1456,6 +1538,19 @@ export const handler = async (event: APIGatewayProxyEventV2): Promise { expect(status).toBe(204); }); }); + +// --------------------------------------------------------------------------- +// 教師フロー — 強制退室 (kick) と verify-session の reason 透過 +// --------------------------------------------------------------------------- +describeIfToken('教師フロー — 強制退室 (kick) と verify-session の reason 透過', () => { + let classroomId: string; + let joinCode: string; + let sessionTokenA: string; + let memberIdA: string; + + test('セットアップ: クラス作成', async () => { + const { status, data } = await request( + 'POST', + '/classrooms', + { className: 'Kick Test クラス', assignmentName: 'Kick 課題', studentCount: 5 }, + teacherHeaders, + ); + expect(status).toBe(201); + classroomId = data.classroomId as string; + joinCode = data.joinCode as string; + }); + + test('セットアップ: 生徒A が席1で参加', async () => { + const { status, data } = await request('POST', '/classrooms/join', { + joinCode, + seatNumber: 1, + nickname: '生徒A', + }); + expect(status).toBe(200); + sessionTokenA = data.sessionToken as string; + memberIdA = data.memberId as string; + }); + + test('生徒A の verify-session は 200', async () => { + const { status } = await request('POST', '/classrooms/verify-session', null, { + Authorization: `Bearer ${sessionTokenA}`, + }); + expect(status).toBe(200); + }); + + test('教師が生徒A を kick (DELETE /classrooms/{id}/members/{memberId}) → 204', async () => { + const { status } = await request( + 'DELETE', + `/classrooms/${classroomId}/members/${memberIdA}`, + null, + teacherHeaders, + ); + expect(status).toBe(204); + }); + + test('生徒A の verify-session は 410 reason=kicked + joinCode/className/seatNumber を返す', async () => { + const { status, data } = await request('POST', '/classrooms/verify-session', null, { + Authorization: `Bearer ${sessionTokenA}`, + }); + expect(status).toBe(410); + expect(data.reason).toBe('kicked'); + expect(data.joinCode).toBe(joinCode); + expect(data.className).toBe('Kick Test クラス'); + expect(data.seatNumber).toBe(1); + }); + + test('kick された席はメンバー一覧 (listMembers) には出てこない', async () => { + const { status, data } = await request( + 'GET', + `/classrooms/${classroomId}/members`, + null, + teacherHeaders, + ); + expect(status).toBe(200); + const members = data.members as Array<{ memberId: string }>; + expect(members.find(m => m.memberId === memberIdA)).toBeUndefined(); + }); + + test('kick された席は lookup の takenSeats に含まれない (席が空く)', async () => { + const { status, data } = await request('POST', '/classrooms/lookup', { joinCode }); + expect(status).toBe(200); + const takenSeats = data.takenSeats as number[]; + expect(takenSeats).not.toContain(1); + }); + + test('別の生徒B が同じ席1で join できる (kicked 行が上書きされる)', async () => { + const { status, data } = await request('POST', '/classrooms/join', { + joinCode, + seatNumber: 1, + nickname: '生徒B', + }); + expect(status).toBe(200); + expect(data.seatNumber).toBe(1); + expect(data.sessionToken).toBeDefined(); + expect(data.sessionToken).not.toBe(sessionTokenA); + }); + + test('上書き後、元の sessionToken (A) は 401 になる (tombstone は B の join で消滅)', async () => { + const { status } = await request('POST', '/classrooms/verify-session', null, { + Authorization: `Bearer ${sessionTokenA}`, + }); + expect(status).toBe(401); + }); + + // Cleanup + test('クリーンアップ: クラス削除', async () => { + const { status } = await request( + 'DELETE', + `/classrooms/${classroomId}`, + null, + teacherHeaders, + ); + expect(status).toBe(204); + }); +}); From 012b4389fd2c2a140df071e476b9a4f09b43ecf1 Mon Sep 17 00:00:00 2001 From: Kouji Takao Date: Thu, 21 May 2026 14:40:02 +0900 Subject: [PATCH 03/10] feat(classroom): show kick banner and auto-navigate to seat selection on forced leave MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pair with the Phase 1 backend change so a student who was kicked by the teacher gets a specific in-modal banner instead of the generic "session expired" alert, and lands on the seat-selection screen for the same classroom — ready to pick a different seat in one click instead of having to re-type the join code. - classroom-api.js: attach the parsed response body to thrown Errors so callers can read response-specific fields (the 410 kick body carries reason/joinCode/className/seatNumber). - classroom-error-utils.js: new `extractKickReason(err)` helper that returns the kick context for a 410 + reason='kicked', or null otherwise. Pure function, unit-tested in isolation. - classroom-modal.jsx (container): when refreshStudentStatus catches an error, detect kick via extractKickReason. On kick: clear the local session, save a `kickedNotice` state, and call handleJoinWithCode with the kicked classroom's joinCode — which lookups the seat grid and flips phase to student-seat. On non-kick errors, keep the existing generic-alert path. - ClassroomModal (component) + StudentSeatSelector: forward kickedNotice and onDismissKickedNotice. The selector renders a dismissible orange banner above the seat grid ("先生によってクラスから退室させられました ...") using the existing CSS-module styling vocabulary. - locales: add `gui.classroom.kicked.banner.title` and `.subtitle` in en / ja / ja-Hira. - Tests: * 6 cases for extractKickReason (happy path, missing body, non-kick 410, null/undefined err, …) * 3 cases for classroom-api error-body propagation - Playwright smoke verified end-to-end on localhost against the deployed stg backend: join seat 2 → teacher kicks via API → reopen modal → banner appears + seat-selection auto-displayed + seat 2 immediately re-selectable → re-join lands student back in student-joined. Refs #692 Co-Authored-By: Claude Opus 4.7 (1M context) --- .../scratch-gui/smalruby-prettier-files.md | 2 + packages/scratch-gui/.prettierignore | 2 + .../classroom-modal/classroom-modal.css | 32 ++++++++ .../classroom-modal/classroom-modal.jsx | 10 +++ .../classroom-modal/student-seat-selector.jsx | 40 ++++++++++ .../src/containers/classroom-error-utils.js | 23 +++++- .../src/containers/classroom-modal.jsx | 31 ++++++-- packages/scratch-gui/src/lib/classroom-api.js | 4 + packages/scratch-gui/src/locales/en.js | 2 + packages/scratch-gui/src/locales/ja-Hira.js | 3 + packages/scratch-gui/src/locales/ja.js | 2 + .../containers/classroom-error-utils.test.js | 56 ++++++++++++++ .../test/unit/lib/classroom-api.test.js | 75 +++++++++++++++++++ 13 files changed, 276 insertions(+), 6 deletions(-) create mode 100644 packages/scratch-gui/test/unit/containers/classroom-error-utils.test.js create mode 100644 packages/scratch-gui/test/unit/lib/classroom-api.test.js diff --git a/.claude/rules/scratch-gui/smalruby-prettier-files.md b/.claude/rules/scratch-gui/smalruby-prettier-files.md index 68aa5e9f575..8982db57dab 100644 --- a/.claude/rules/scratch-gui/smalruby-prettier-files.md +++ b/.claude/rules/scratch-gui/smalruby-prettier-files.md @@ -217,6 +217,7 @@ upstream (Scratch) ファイルは対象外。 - `test/unit/components/student-join-form.test.js` - `test/unit/containers/backpack.test.jsx` - `test/unit/containers/cards.test.jsx` +- `test/unit/containers/classroom-error-utils.test.js` - `test/unit/containers/connection-modal-connected-message.test.jsx` - `test/unit/containers/connection-modal-smalrubot-s1.test.jsx` - `test/unit/containers/connection-modal.test.jsx` @@ -233,6 +234,7 @@ upstream (Scratch) ファイルは対象外。 - `test/unit/lib/block-display-initialization.test.js` - `test/unit/lib/blockly-private-api.test.js` - `test/unit/lib/blocks-gesture-recovery.test.js` +- `test/unit/lib/classroom-api.test.js` - `test/unit/lib/deck-setup.test.js` - `test/unit/lib/blocks-screenshot.test.js` - `test/unit/lib/calculate-popup-position.test.js` diff --git a/packages/scratch-gui/.prettierignore b/packages/scratch-gui/.prettierignore index b9f9f9dfaf4..eab76502489 100644 --- a/packages/scratch-gui/.prettierignore +++ b/packages/scratch-gui/.prettierignore @@ -284,6 +284,7 @@ test/unit/containers/* !test/unit/containers/ruby-tab/ !test/unit/containers/backpack.test.jsx !test/unit/containers/cards.test.jsx +!test/unit/containers/classroom-error-utils.test.js !test/unit/containers/connection-modal-connected-message.test.jsx !test/unit/containers/connection-modal-smalrubot-s1.test.jsx !test/unit/containers/connection-modal.test.jsx @@ -310,6 +311,7 @@ test/unit/lib/* !test/unit/lib/block-display-initialization.test.js !test/unit/lib/blockly-private-api.test.js !test/unit/lib/blocks-gesture-recovery.test.js +!test/unit/lib/classroom-api.test.js !test/unit/lib/deck-setup.test.js !test/unit/lib/blocks-screenshot.test.js !test/unit/lib/calculate-popup-position.test.js diff --git a/packages/scratch-gui/src/components/classroom-modal/classroom-modal.css b/packages/scratch-gui/src/components/classroom-modal/classroom-modal.css index b7e58a899cd..3ee849315fd 100644 --- a/packages/scratch-gui/src/components/classroom-modal/classroom-modal.css +++ b/packages/scratch-gui/src/components/classroom-modal/classroom-modal.css @@ -515,6 +515,38 @@ margin-top: 0.5rem; } +.kicked-banner { + display: flex; + align-items: flex-start; + gap: 0.5rem; + background-color: #fff4e5; + border: 1px solid #ffa000; + border-radius: 8px; + padding: 0.8rem 1rem; + margin-bottom: 1rem; + color: #4a3500; +} + +.kicked-banner-text { + flex: 1; + font-size: 0.9rem; + line-height: 1.5; +} + +.kicked-banner-dismiss { + background: none; + border: none; + cursor: pointer; + font-size: 1.2rem; + line-height: 1; + color: #4a3500; + padding: 0 0.2rem; +} + +.kicked-banner-dismiss:hover { + color: #000; +} + .error-box { background-color: #fff0f3; border: 1px solid #ff6680; diff --git a/packages/scratch-gui/src/components/classroom-modal/classroom-modal.jsx b/packages/scratch-gui/src/components/classroom-modal/classroom-modal.jsx index 2373a48f297..3d7ba0fade3 100644 --- a/packages/scratch-gui/src/components/classroom-modal/classroom-modal.jsx +++ b/packages/scratch-gui/src/components/classroom-modal/classroom-modal.jsx @@ -27,6 +27,7 @@ const ClassroomModal = ({ takenSeats, selectedSeat, joinedInfo, + kickedNotice, error, errorActionHandler, errorActionLabel, @@ -38,6 +39,7 @@ const ClassroomModal = ({ onSelectSeat, onConfirmJoin, onClose, + onDismissKickedNotice, onLeaveClassroom, onStartSubmit, onConfirmSubmit, @@ -79,10 +81,12 @@ const ClassroomModal = ({ error={error} errorTitle={errorTitle} isLoading={isLoading} + kickedNotice={kickedNotice} seatCount={seatCount} selectedSeat={selectedSeat} takenSeats={takenSeats} onConfirmJoin={onConfirmJoin} + onDismissKickedNotice={onDismissKickedNotice} onSelectSeat={onSelectSeat} /> )} @@ -147,10 +151,16 @@ ClassroomModal.propTypes = { className: PropTypes.string, seatNumber: PropTypes.number, }), + kickedNotice: PropTypes.shape({ + joinCode: PropTypes.string, + className: PropTypes.string, + seatNumber: PropTypes.number, + }), onCancelSubmit: PropTypes.func.isRequired, onClose: PropTypes.func.isRequired, onConfirmJoin: PropTypes.func.isRequired, onConfirmSubmit: PropTypes.func.isRequired, + onDismissKickedNotice: PropTypes.func, onJoinWithCode: PropTypes.func.isRequired, onLeaveClassroom: PropTypes.func.isRequired, onRefreshStudentStatus: PropTypes.func.isRequired, diff --git a/packages/scratch-gui/src/components/classroom-modal/student-seat-selector.jsx b/packages/scratch-gui/src/components/classroom-modal/student-seat-selector.jsx index ae11c367915..698e9de2fd1 100644 --- a/packages/scratch-gui/src/components/classroom-modal/student-seat-selector.jsx +++ b/packages/scratch-gui/src/components/classroom-modal/student-seat-selector.jsx @@ -13,8 +13,10 @@ const StudentSeatSelector = ({ isLoading, error, errorTitle, + kickedNotice, onSelectSeat, onConfirmJoin, + onDismissKickedNotice, }) => { const handleSeatClick = useCallback( (e) => { @@ -25,6 +27,38 @@ const StudentSeatSelector = ({ return (
+ {kickedNotice && ( +
+
+ + + +
+ +
+
+ {onDismissKickedNotice && ( + + )} +
+ )}
{ ); }; -export { translateError }; +/** + * If the error represents a kick (verify-session returned 410 with + * reason='kicked'), return the kick context so the UI can navigate the + * student back into seat selection for the same classroom. Returns null + * otherwise. + * @param {*} err - Value thrown from a classroom API call + * @returns {?{joinCode: string, className: string, seatNumber: number}} + * Kick context, or null if the error is not a kick. + */ +const extractKickReason = (err) => { + if (!err || typeof err !== 'object') return null; + if (err.status !== 410) return null; + const body = err.body; + if (!body || body.reason !== 'kicked') return null; + return { + joinCode: body.joinCode || '', + className: body.className || '', + seatNumber: body.seatNumber || 0, + }; +}; + +export { translateError, extractKickReason }; export default translateError; diff --git a/packages/scratch-gui/src/containers/classroom-modal.jsx b/packages/scratch-gui/src/containers/classroom-modal.jsx index 6de9f20442d..818355ea3c0 100644 --- a/packages/scratch-gui/src/containers/classroom-modal.jsx +++ b/packages/scratch-gui/src/containers/classroom-modal.jsx @@ -15,7 +15,7 @@ import { setSubmissionStatus, } from '../reducers/classroom.js'; import { setProjectTitle } from '../reducers/project-title.js'; -import translateError from './classroom-error-utils.js'; +import translateError, { extractKickReason } from './classroom-error-utils.js'; import useStudentSubmit from './use-student-submit.js'; const ClassroomModal = () => { @@ -115,6 +115,12 @@ const ClassroomModal = () => { const [selectedSeat, setSelectedSeat] = useState(null); const [pendingClassroomInfo, setPendingClassroomInfo] = useState(null); const [joinedInfo, setJoinedInfo] = useState(null); + // Set to {joinCode, className, seatNumber} when the student arrives at the + // seat-selection screen because the teacher kicked them. The seat-selector + // shows a dismissible banner so the student knows the reason rather than + // seeing the generic "session expired" alert. + const [kickedNotice, setKickedNotice] = useState(null); + const handleDismissKickedNotice = useCallback(() => setKickedNotice(null), []); // --- Join with code --- @@ -197,6 +203,7 @@ const ClassroomModal = () => { assignmentName: data.assignmentName || null, seatNumber: data.seatNumber, }); + setKickedNotice(null); setPhase('student-joined'); } catch (err) { if (err.status === 409) { @@ -222,13 +229,25 @@ const ClassroomModal = () => { dispatch(setSubmissionStatus(result.submission.status, result.submission.submittedAt)); setStudentTeacherComment(result.submission.teacherComment || null); } - } catch { - dispatch(clearClassroomSession()); - showSessionExpiredError(translateError(intl, { status: 401 }, 'session')); + } catch (err) { + const kick = extractKickReason(err); + if (kick) { + // Teacher removed the student. Clear the dead session, jump + // straight to seat selection for the same classroom, and let + // the seat-selector show a "you were removed" banner so the + // student knows what happened instead of seeing the generic + // "session expired" alert. + dispatch(clearClassroomSession()); + setKickedNotice(kick); + handleJoinWithCode(kick.joinCode); + } else { + dispatch(clearClassroomSession()); + showSessionExpiredError(translateError(intl, { status: 401 }, 'session')); + } } finally { setIsLoading(false); } - }, [classroomState.sessionToken, dispatch, showSessionExpiredError, intl]); + }, [classroomState.sessionToken, dispatch, showSessionExpiredError, intl, handleJoinWithCode]); useEffect(() => { if (phase === 'student-status' && classroomState.sessionToken) { @@ -284,6 +303,7 @@ const ClassroomModal = () => { isLoading={isLoading} joinCodeHistory={joinCodeHistory} joinedInfo={joinedInfo} + kickedNotice={kickedNotice} phase={phase} seatCount={seatCount} selectedSeat={selectedSeat} @@ -295,6 +315,7 @@ const ClassroomModal = () => { onClose={handleClose} onConfirmJoin={handleConfirmJoin} onConfirmSubmit={submit.handleConfirmSubmit} + onDismissKickedNotice={handleDismissKickedNotice} onJoinWithCode={handleJoinWithCode} onLeaveClassroom={handleLeaveClassroom} onRefreshStudentStatus={refreshStudentStatus} diff --git a/packages/scratch-gui/src/lib/classroom-api.js b/packages/scratch-gui/src/lib/classroom-api.js index 34e4b66ea30..3ea8543604b 100644 --- a/packages/scratch-gui/src/lib/classroom-api.js +++ b/packages/scratch-gui/src/lib/classroom-api.js @@ -283,6 +283,10 @@ class ClassroomAPI { const errorData = await response.json().catch(() => ({})); lastError = new Error(errorData.error || `API error ${response.status}`); lastError.status = response.status; + // Surface the parsed body so callers can read response-specific + // fields (e.g. 410 + reason='kicked' carries joinCode/className/ + // seatNumber that the kick-banner UI needs). + lastError.body = errorData; throw lastError; } diff --git a/packages/scratch-gui/src/locales/en.js b/packages/scratch-gui/src/locales/en.js index ceb332a3a78..946c8ea540d 100644 --- a/packages/scratch-gui/src/locales/en.js +++ b/packages/scratch-gui/src/locales/en.js @@ -77,6 +77,8 @@ export default { 'Teachers can find the join code in {settingsIcon} Settings → Class Management.', 'gui.classroom.studentSeat.prompt': 'Select your seat number', 'gui.classroom.studentSeat.join': 'Join', + 'gui.classroom.kicked.banner.title': 'You were removed from this class by your teacher.', + 'gui.classroom.kicked.banner.subtitle': 'Pick your seat again to rejoin {className}.', 'gui.classroom.studentJoined.success': 'Joined successfully!', 'gui.classroom.studentJoined.seat': 'Seat {seatNumber}', 'gui.classroom.studentJoined.start': 'Start', diff --git a/packages/scratch-gui/src/locales/ja-Hira.js b/packages/scratch-gui/src/locales/ja-Hira.js index 69bd73e44e6..4cf869fbc98 100644 --- a/packages/scratch-gui/src/locales/ja-Hira.js +++ b/packages/scratch-gui/src/locales/ja-Hira.js @@ -92,6 +92,9 @@ export default { 'gui.classroom.studentJoin.teacherLink': 'せんせいのかたはこちら(クラスかんり)', 'gui.classroom.studentSeat.prompt': 'しゅっせきばんごうをえらんでください', 'gui.classroom.studentSeat.join': 'さんかする', + 'gui.classroom.kicked.banner.title': 'せんせいによってクラスからたいしつさせられました。', + 'gui.classroom.kicked.banner.subtitle': + '「{className}」にもういちどさんかするには、しゅっせきばんごうをえらびなおしてください。', 'gui.classroom.studentJoined.success': 'さんかしました!', 'gui.classroom.studentJoined.seat': 'しゅっせきばんごう{seatNumber}', 'gui.classroom.studentJoined.start': 'はじめる', diff --git a/packages/scratch-gui/src/locales/ja.js b/packages/scratch-gui/src/locales/ja.js index 0bc1e08feff..48c32f36170 100644 --- a/packages/scratch-gui/src/locales/ja.js +++ b/packages/scratch-gui/src/locales/ja.js @@ -91,6 +91,8 @@ export default { 'gui.classroom.studentJoin.teacherLink': '先生の方はこちら(クラス管理)', 'gui.classroom.studentSeat.prompt': '出席番号を選んでください', 'gui.classroom.studentSeat.join': '参加する', + 'gui.classroom.kicked.banner.title': '先生によってクラスから退室させられました。', + 'gui.classroom.kicked.banner.subtitle': '「{className}」に再度参加するには出席番号を選びなおしてください。', 'gui.classroom.studentJoined.success': '参加しました!', 'gui.classroom.studentJoined.seat': '出席番号{seatNumber}', 'gui.classroom.studentJoined.start': 'はじめる', diff --git a/packages/scratch-gui/test/unit/containers/classroom-error-utils.test.js b/packages/scratch-gui/test/unit/containers/classroom-error-utils.test.js new file mode 100644 index 00000000000..947ffa6a13c --- /dev/null +++ b/packages/scratch-gui/test/unit/containers/classroom-error-utils.test.js @@ -0,0 +1,56 @@ +import { extractKickReason } from '../../../src/containers/classroom-error-utils.js'; + +describe('extractKickReason', () => { + test('returns null for a non-410 error', () => { + const err = new Error('Invalid or expired session token'); + err.status = 401; + expect(extractKickReason(err)).toBeNull(); + }); + + test('returns null for a 410 without a kicked reason', () => { + const err = new Error('Gone'); + err.status = 410; + err.body = { error: 'Gone', reason: 'other' }; + expect(extractKickReason(err)).toBeNull(); + }); + + test('returns kick context when the server flags reason=kicked', () => { + const err = new Error('You were removed from the classroom by the teacher'); + err.status = 410; + err.body = { + error: 'You were removed from the classroom by the teacher', + reason: 'kicked', + joinCode: 'btgyal', + className: 'Phase1検証', + seatNumber: 5, + }; + expect(extractKickReason(err)).toEqual({ + joinCode: 'btgyal', + className: 'Phase1検証', + seatNumber: 5, + }); + }); + + test('falls back gracefully when body fields are missing', () => { + const err = new Error('You were removed from the classroom by the teacher'); + err.status = 410; + err.body = { reason: 'kicked' }; + expect(extractKickReason(err)).toEqual({ + joinCode: '', + className: '', + seatNumber: 0, + }); + }); + + test('returns null when body is missing entirely', () => { + const err = new Error('Gone'); + err.status = 410; + expect(extractKickReason(err)).toBeNull(); + }); + + test('returns null for a non-Error-shaped value', () => { + expect(extractKickReason(null)).toBeNull(); + expect(extractKickReason(undefined)).toBeNull(); + expect(extractKickReason('boom')).toBeNull(); + }); +}); diff --git a/packages/scratch-gui/test/unit/lib/classroom-api.test.js b/packages/scratch-gui/test/unit/lib/classroom-api.test.js new file mode 100644 index 00000000000..321c690be11 --- /dev/null +++ b/packages/scratch-gui/test/unit/lib/classroom-api.test.js @@ -0,0 +1,75 @@ +/** + * Unit tests for ClassroomAPI + * + * Focused on error-body propagation — the response body for 4xx/5xx + * needs to surface on the thrown Error so callers (notably the kick + * detection in classroom-modal) can read reason / joinCode / className / + * seatNumber from a 410 response. + */ + +global.fetch = jest.fn(); + +describe('ClassroomAPI._request error handling', () => { + let classroomAPI; + + beforeEach(() => { + jest.resetModules(); + global.fetch = jest.fn(); + classroomAPI = require('../../../src/lib/classroom-api.js').default; + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + test('attaches HTTP status to the thrown Error', async () => { + global.fetch.mockResolvedValue({ + ok: false, + status: 401, + json: jest.fn().mockResolvedValue({ error: 'Invalid or expired session token' }), + }); + + await expect(classroomAPI.verifySession('bad-token')).rejects.toMatchObject({ + status: 401, + message: 'Invalid or expired session token', + }); + }); + + test('410 errors expose the full response body so callers can read reason / kick context', async () => { + const kickBody = { + error: 'You were removed from the classroom by the teacher', + reason: 'kicked', + joinCode: 'btgyal', + className: 'Phase1検証', + seatNumber: 5, + }; + global.fetch.mockResolvedValue({ + ok: false, + status: 410, + json: jest.fn().mockResolvedValue(kickBody), + }); + + let thrown; + try { + await classroomAPI.verifySession('kicked-token'); + } catch (err) { + thrown = err; + } + expect(thrown).toBeDefined(); + expect(thrown.status).toBe(410); + expect(thrown.body).toEqual(kickBody); + }); + + test('errors with no JSON body still surface status and message', async () => { + global.fetch.mockResolvedValue({ + ok: false, + status: 500, + json: jest.fn().mockRejectedValue(new Error('not json')), + }); + + await expect(classroomAPI.verifySession('any')).rejects.toMatchObject({ + status: 500, + message: 'API error 500', + }); + }); +}); From 692d586a1b628186a60ef056dfbc5aff3a0c0f64 Mon Sep 17 00:00:00 2001 From: Kouji Takao Date: Thu, 21 May 2026 15:03:51 +0900 Subject: [PATCH 04/10] fix(classroom): leave old session on server when classcode URL points to different classroom MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, opening a `?classcode=` URL while still holding a session for a different classroom only cleared the local Redux state — the previous seat stayed occupied on the server until TTL, so a returning student or the teacher's grid kept treating the old seat as "taken." With the Phase 0 move to localStorage this became much more likely to bite, since the session survives tab close / browser restart. Extract the URL-parameter decision into a pure helper `decideClasscodeAction()` and unit-test it across the five cases: fresh_join (no session), same_class (matching joinCode), switch_class (different joinCode → release old seat + re-lookup), case-insensitive join code match, and the malformed-state fallback (sessionToken but no classroomId). Wire the helper into the classcode useEffect so the switch_class branch fires `classroomAPI.leaveClassroom(oldToken, oldClassroomId)` as a best-effort, non-blocking call. We do not await it: a slow API or a 401 (e.g. server already expired the session) must not stall the new lookup or block the student from joining the new classroom. The new join takes precedence either way. Playwright verified on localhost frontend against the deployed stg backend: 1. Join class A (seat 3) → `GET /members` shows seat-03 on A. 2. Navigate to `?classcode=` (different class) → land on seat selection for B, `GET /members` on A is now empty, lookup `takenSeats` on A returns []. 3. Join class B (seat 2) → revisit `?classcode=` → straight to student-status (same_class branch, no leave fired, lastActiveAt ticked on B). Refs #692 Co-Authored-By: Claude Opus 4.7 (1M context) --- .../scratch-gui/smalruby-prettier-files.md | 2 + packages/scratch-gui/.prettierignore | 2 + .../containers/classroom-classcode-utils.js | 46 +++++++++++++++ .../src/containers/classroom-modal.jsx | 19 +++++-- .../classroom-classcode-utils.test.js | 56 +++++++++++++++++++ 5 files changed, 121 insertions(+), 4 deletions(-) create mode 100644 packages/scratch-gui/src/containers/classroom-classcode-utils.js create mode 100644 packages/scratch-gui/test/unit/containers/classroom-classcode-utils.test.js diff --git a/.claude/rules/scratch-gui/smalruby-prettier-files.md b/.claude/rules/scratch-gui/smalruby-prettier-files.md index 8982db57dab..6b7c52d9312 100644 --- a/.claude/rules/scratch-gui/smalruby-prettier-files.md +++ b/.claude/rules/scratch-gui/smalruby-prettier-files.md @@ -63,6 +63,7 @@ upstream (Scratch) ファイルは対象外。 **個別ファイル:** - `src/containers/block-display-modal.jsx` +- `src/containers/classroom-classcode-utils.js` - `src/containers/classroom-error-utils.js` - `src/containers/classroom-modal.jsx` - `src/containers/classroom-teacher-modal.jsx` @@ -217,6 +218,7 @@ upstream (Scratch) ファイルは対象外。 - `test/unit/components/student-join-form.test.js` - `test/unit/containers/backpack.test.jsx` - `test/unit/containers/cards.test.jsx` +- `test/unit/containers/classroom-classcode-utils.test.js` - `test/unit/containers/classroom-error-utils.test.js` - `test/unit/containers/connection-modal-connected-message.test.jsx` - `test/unit/containers/connection-modal-smalrubot-s1.test.jsx` diff --git a/packages/scratch-gui/.prettierignore b/packages/scratch-gui/.prettierignore index eab76502489..1878105a42c 100644 --- a/packages/scratch-gui/.prettierignore +++ b/packages/scratch-gui/.prettierignore @@ -88,6 +88,7 @@ src/containers/* !src/containers/ruby-tab/ # Individual Smalruby files !src/containers/block-display-modal.jsx +!src/containers/classroom-classcode-utils.js !src/containers/classroom-error-utils.js !src/containers/classroom-modal.jsx !src/containers/classroom-teacher-modal.jsx @@ -284,6 +285,7 @@ test/unit/containers/* !test/unit/containers/ruby-tab/ !test/unit/containers/backpack.test.jsx !test/unit/containers/cards.test.jsx +!test/unit/containers/classroom-classcode-utils.test.js !test/unit/containers/classroom-error-utils.test.js !test/unit/containers/connection-modal-connected-message.test.jsx !test/unit/containers/connection-modal-smalrubot-s1.test.jsx diff --git a/packages/scratch-gui/src/containers/classroom-classcode-utils.js b/packages/scratch-gui/src/containers/classroom-classcode-utils.js new file mode 100644 index 00000000000..3b6995e1ff9 --- /dev/null +++ b/packages/scratch-gui/src/containers/classroom-classcode-utils.js @@ -0,0 +1,46 @@ +/** + * Decide what to do when the editor is opened with a `?classcode=XXXX` URL + * parameter, given any session the student already has in localStorage. + * + * - `fresh_join`: no existing session, or the existing session is malformed + * (no classroomId to release). Just lookup + seat select. + * - `same_class`: the URL classcode matches the existing session — drop the + * student straight into student-status (no leave, no re-pick). + * - `switch_class`: the existing session is for a different classroom. The + * container should call leaveClassroom() on the server (best effort) before + * clearing the local session and starting the new join flow. Without the + * server-side leave, the previous seat would stay occupied until TTL. + * + * Pure function with no side effects; container glue performs the action. + * @param {object} currentSession - {sessionToken, joinCode, classroomId} + * @param {string} code - The classcode from the URL parameter + * @returns {object} Action descriptor for the container + */ +const decideClasscodeAction = (currentSession, code) => { + const { sessionToken, joinCode, classroomId } = currentSession || {}; + if (!sessionToken) { + return { type: 'fresh_join', code }; + } + // Join codes are case-insensitive (server normalises to lowercase). Without + // this, opening the URL `?classcode=ABCDEF` while holding a session for + // `abcdef` would falsely look like a class switch and the student would be + // bounced through seat selection on their own seat. + if (joinCode && joinCode.toLowerCase() === code.toLowerCase()) { + return { type: 'same_class' }; + } + if (!classroomId) { + // No classroomId means we can't issue a server-side leave anyway. + // Fall back to fresh_join so we at least clear any stale local state + // and don't refuse to switch. + return { type: 'fresh_join', code }; + } + return { + type: 'switch_class', + leaveSessionToken: sessionToken, + leaveClassroomId: classroomId, + code, + }; +}; + +export { decideClasscodeAction }; +export default decideClasscodeAction; diff --git a/packages/scratch-gui/src/containers/classroom-modal.jsx b/packages/scratch-gui/src/containers/classroom-modal.jsx index 818355ea3c0..ae32f313c06 100644 --- a/packages/scratch-gui/src/containers/classroom-modal.jsx +++ b/packages/scratch-gui/src/containers/classroom-modal.jsx @@ -15,6 +15,7 @@ import { setSubmissionStatus, } from '../reducers/classroom.js'; import { setProjectTitle } from '../reducers/project-title.js'; +import { decideClasscodeAction } from './classroom-classcode-utils.js'; import translateError, { extractKickReason } from './classroom-error-utils.js'; import useStudentSubmit from './use-student-submit.js'; @@ -281,15 +282,25 @@ const ClassroomModal = () => { window.history.replaceState({}, '', url.toString()); clearClasscode(); - if (classroomState.sessionToken && classroomState.joinCode === code) { + const action = decideClasscodeAction(classroomState, code); + if (action.type === 'same_class') { setPhase('student-status'); return; } - - if (classroomState.sessionToken) { + if (action.type === 'switch_class') { + // Best-effort release of the old seat before switching. We do not + // await this so the new lookup UI is not blocked by a slow API + // call (or by a 401 in case the old session already expired on + // the server). Without this call, opening a new classcode URL + // while still holding an old session would leave the previous + // seat occupied until TTL. + classroomAPI.leaveClassroom(action.leaveSessionToken, action.leaveClassroomId).catch(() => { + // Ignore — we have no UI to surface this to and the new + // join takes precedence either way. + }); dispatch(clearClassroomSession()); } - + // 'fresh_join' and 'switch_class' both fall through to a join lookup. handleJoinWithCode(code); }, []); // Run once on mount diff --git a/packages/scratch-gui/test/unit/containers/classroom-classcode-utils.test.js b/packages/scratch-gui/test/unit/containers/classroom-classcode-utils.test.js new file mode 100644 index 00000000000..d0ac6e32b43 --- /dev/null +++ b/packages/scratch-gui/test/unit/containers/classroom-classcode-utils.test.js @@ -0,0 +1,56 @@ +import { decideClasscodeAction } from '../../../src/containers/classroom-classcode-utils.js'; + +describe('decideClasscodeAction', () => { + test('returns fresh_join when there is no existing session', () => { + expect(decideClasscodeAction({ sessionToken: null, joinCode: null, classroomId: null }, 'abcdef')).toEqual({ + type: 'fresh_join', + code: 'abcdef', + }); + }); + + test('returns same_class when the URL classcode matches the existing session', () => { + expect( + decideClasscodeAction( + { + sessionToken: 'tok-a', + joinCode: 'abcdef', + classroomId: 'cid-a', + }, + 'abcdef', + ), + ).toEqual({ type: 'same_class' }); + }); + + test('returns switch_class when an existing session points to a different classroom', () => { + expect( + decideClasscodeAction( + { + sessionToken: 'tok-old', + joinCode: 'oldcde', + classroomId: 'cid-old', + }, + 'newcde', + ), + ).toEqual({ + type: 'switch_class', + leaveSessionToken: 'tok-old', + leaveClassroomId: 'cid-old', + code: 'newcde', + }); + }); + + test('case-insensitive match treats upper/lower joinCode as same class', () => { + // Server / DB stores join codes in lower-case (handler.ts validateJoinCode), + // but the URL parameter may arrive in any case. Treat 'ABCDEF' and 'abcdef' + // as the same class so we do not bounce the student off their own seat. + expect( + decideClasscodeAction({ sessionToken: 'tok', joinCode: 'abcdef', classroomId: 'cid' }, 'ABCDEF'), + ).toEqual({ type: 'same_class' }); + }); + + test('returns fresh_join when sessionToken exists but classroomId is missing (malformed state)', () => { + expect( + decideClasscodeAction({ sessionToken: 'tok', joinCode: 'oldcde', classroomId: null }, 'newcde'), + ).toEqual({ type: 'fresh_join', code: 'newcde' }); + }); +}); From fee079c522e7a5df91db12ccef19d0e72befc1c3 Mon Sep 17 00:00:00 2001 From: Kouji Takao Date: Thu, 21 May 2026 15:26:34 +0900 Subject: [PATCH 05/10] =?UTF-8?q?feat(classroom):=20show=20"=E3=82=AF?= =?UTF-8?q?=E3=83=A9=E3=82=B9:=E5=87=BA=E5=B8=AD=E7=95=AA=E5=8F=B7NN"=20in?= =?UTF-8?q?=20the=20menu=20bar=20instead=20of=20"=E8=AA=B2=E9=A1=8C?= =?UTF-8?q?=E5=90=8D/NN"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit While joined, the menu-bar button used to render "{assignmentName || className} / {seatNumber}" (e.g. "第1回チャットアプリ を作ろう / 03"). In user testing this turned out to be confusing — the left half looked like a project title and it wasn't obvious that the two-digit suffix was the seat number rather than a version or a date. Replace the joined label with a fixed-format "クラス:出席番号NN" (English: "Class: Seat NN"). The class / assignment name still lives in the modal once opened, but is dropped from the always-visible bar so the bar unambiguously identifies *what the number means*. The unjoined bar remains "クラス" alone. - menu-bar.jsx: switch the joined branch to a single FormattedMessage `gui.menuBar.classroomJoined` (with the seat-number span passed as a formatter value so the `classroom-menu-seat-number` testid is preserved for Playwright). The old `classroom-menu-class-name` span is removed — no test or tool referenced it. - locales: add `gui.menuBar.classroomJoined` in en/ja/ja-Hira. - docs: update docs/classroom/ui-ux.md, docs/classroom/testing.md and .claude/rules/scratch-gui/e2e-test.md to match the new format and drop the removed testid. Playwright verified end-to-end on localhost: not-joined → "クラス"; joined to seat 4 → "クラス:出席番号04"; `classroom-menu-seat-number` is still a child span; `classroom-menu-class-name` is no longer in the DOM. Refs #692 Co-Authored-By: Claude Opus 4.7 (1M context) --- .claude/rules/scratch-gui/e2e-test.md | 5 ++--- docs/classroom/testing.md | 5 ++--- docs/classroom/ui-ux.md | 8 ++++--- .../src/components/menu-bar/menu-bar.jsx | 21 +++++++++---------- packages/scratch-gui/src/locales/en.js | 1 + packages/scratch-gui/src/locales/ja-Hira.js | 1 + packages/scratch-gui/src/locales/ja.js | 1 + 7 files changed, 22 insertions(+), 20 deletions(-) diff --git a/.claude/rules/scratch-gui/e2e-test.md b/.claude/rules/scratch-gui/e2e-test.md index a3f0bf93b88..c690b048255 100644 --- a/.claude/rules/scratch-gui/e2e-test.md +++ b/.claude/rules/scratch-gui/e2e-test.md @@ -108,9 +108,8 @@ expect(await isActiveByTestId(driver, 'ruby-toolbar-mode-dncl')).toBe(true); | data-testid | 要素 | 値の内容 | |------------|------|----------| -| `classroom-menu-label` | span | メニューバーのクラス表示テキスト | -| `classroom-menu-class-name` | span | 課題名(またはクラス名) | -| `classroom-menu-seat-number` | span | 出席番号(0埋め2桁) | +| `classroom-menu-label` | span | メニューバーのクラス表示テキスト全体(参加中は「クラス:出席番号NN」、未参加は「クラス」) | +| `classroom-menu-seat-number` | span | 出席番号(0埋め2桁、参加中のみレンダリング) | | `classroom-list` | ul | クラス一覧 | | `classroom-item-{id}` | li | クラス一覧の各項目 | | `classroom-item-name-{id}` | span | クラス名 | diff --git a/docs/classroom/testing.md b/docs/classroom/testing.md index 497baa7bca9..9ec914482ea 100644 --- a/docs/classroom/testing.md +++ b/docs/classroom/testing.md @@ -163,9 +163,8 @@ Playwright MCP および Selenium integration tests で使用する `data-testid | data-testid | 要素 | 説明 | |------------|------|------| | `classroom-menu-button` | div | クラスボタン(コンテナ) | -| `classroom-menu-label` | span | メニューバーのクラス表示テキスト | -| `classroom-menu-class-name` | span | 課題名(またはクラス名) | -| `classroom-menu-seat-number` | span | 出席番号(0埋め2桁) | +| `classroom-menu-label` | span | メニューバーのクラス表示テキスト全体(参加中は「クラス:出席番号NN」、未参加時は「クラス」)| +| `classroom-menu-seat-number` | span | 出席番号(0埋め2桁、参加中のみレンダリングされる) | ### 汎用 diff --git a/docs/classroom/ui-ux.md b/docs/classroom/ui-ux.md index fa0c3f8df1a..99397248157 100644 --- a/docs/classroom/ui-ux.md +++ b/docs/classroom/ui-ux.md @@ -62,12 +62,14 @@ stateDiagram-v2 | クラスボタン | 「クラス」(未参加時) | `classroom-menu-button` | | ラベル | — | `classroom-menu-label` | -生徒がクラスに参加中の場合、ボタンのテキストが課題名と出席番号に変わります: +生徒がクラスに参加中の場合、ボタンのテキストは固定の「クラス:出席番号NN」表記に変わります (NN は 0 埋め 2 桁): | 要素 | テキスト/内容 | data-testid | |------|-------------|-------------| -| 課題名(またはクラス名) | 例: 「第1回チャットアプリを作ろう」 | `classroom-menu-class-name` | -| 出席番号 | 例: 「/ 03」(0埋め2桁) | `classroom-menu-seat-number` | +| ラベル全体 | 例: 「クラス:出席番号03」 | `classroom-menu-label` | +| 出席番号 (内側 span) | 例: 「03」 | `classroom-menu-seat-number` | + +未参加時はラベルが「クラス」のみになる。 課題名 / クラス名はメニューに表示せず、モーダル内でのみ確認する設計。 --- diff --git a/packages/scratch-gui/src/components/menu-bar/menu-bar.jsx b/packages/scratch-gui/src/components/menu-bar/menu-bar.jsx index 2ad1fb800d3..6a88677b056 100644 --- a/packages/scratch-gui/src/components/menu-bar/menu-bar.jsx +++ b/packages/scratch-gui/src/components/menu-bar/menu-bar.jsx @@ -1266,20 +1266,19 @@ class MenuBar extends React.Component { onClick={this.props.onOpenClassroomModal} > - {this.props.classroomClassName ? ( - - - {this.props.classroomAssignmentName || this.props.classroomClassName} - - {this.props.classroomSeatNumber && ( - - {' / '} + {this.props.classroomClassName && this.props.classroomSeatNumber ? ( + {String(this.props.classroomSeatNumber).padStart(2, '0')} - - )} - + ), + }} + /> ) : ( Date: Thu, 21 May 2026 15:41:07 +0900 Subject: [PATCH 06/10] feat(classroom): kick request endpoints for student-initiated seat reclaim MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 4 of #692 — student-driven path for the case where a wrong-seat classmate is occupying the seat the student wants. The student opens the seat-selection screen, taps their own seat (currently grey/taken), fires a kick request to the teacher, and waits; the teacher sees the request next to the seat in the class-detail view and either approves (which kicks the occupant, reusing Phase 1's tombstone path so the kicked student gets the "you were removed" banner) or rejects (the request disappears, the occupant stays). CDK: - New `ClassroomKickRequests{stageSuffix}` DynamoDB table with PK=classroomId, SK=requestId (UUID) so a single seat can hold multiple pending requests simultaneously. A `classroomId-seatNumber-index` GSI lets the approve handler delete all sibling requests for the same seat in one query so duplicates don't ghost-linger after a kick. - KICK_REQUESTS_TABLE_NAME env wired into the Lambda; grantReadWriteData added. - Four new HTTP API v2 routes: POST /classrooms/lookup/kick-request (no auth — joinCode + seatNumber) GET /classrooms/{classroomId}/kick-requests (teacher auth) POST /classrooms/{classroomId}/kick-requests/{requestId}/approve DELETE /classrooms/{classroomId}/kick-requests/{requestId} (= reject) Handler: - KICK_REQUEST_TTL_SECONDS = 3600 (1 hour); rows expire automatically. - MAX_KICK_REQUEST_REASON_LENGTH = 200; `validateKickRequestReason()` trims and enforces. - `findClassroomWithSeatOccupied()` refuses requests for a seat that is already empty (or only holds a kick tombstone) so the teacher doesn't see noise from misclicks or stale clients. - `handleApproveKickRequest()` calls into the existing `handleDeleteMember()` so the kicked student hits exactly the same 410 reason='kicked' verify-session path as a direct teacher kick, then batch-deletes every kick request that targeted the same seat (the approved one and any duplicates). - `handleRejectKickRequest()` deletes the single request row; the occupant is untouched. - Anonymous create endpoint reuses the join-IP rate limiter; explicit abuse-prevention beyond that (e.g. dedupe by IP) is intentionally skipped per the Issue's "規制なし — 複数件のリクエストを許可" decision. Integration tests against the deployed stg endpoint cover the full flow plus auth boundaries — 60/60 new+existing tests green except for one pre-existing failure (`/classrooms/lookup` expecting className to be undefined, see aa35f9a7b9) unrelated to this change. Refs #692 Co-Authored-By: Claude Opus 4.7 (1M context) --- infra/smalruby-classroom/lambda/handler.ts | 236 ++++++++++++++++++ .../lambda/tests/handler.integration.test.ts | 213 ++++++++++++++++ .../smalruby-classroom/lib/classroom-stack.ts | 66 +++++ 3 files changed, 515 insertions(+) diff --git a/infra/smalruby-classroom/lambda/handler.ts b/infra/smalruby-classroom/lambda/handler.ts index 1ff9527a6c1..f4f7820a550 100644 --- a/infra/smalruby-classroom/lambda/handler.ts +++ b/infra/smalruby-classroom/lambda/handler.ts @@ -20,6 +20,7 @@ import * as crypto from 'crypto'; const CLASSROOMS_TABLE = process.env.CLASSROOMS_TABLE_NAME || 'Classrooms'; const MEMBERSHIPS_TABLE = process.env.MEMBERSHIPS_TABLE_NAME || 'ClassroomMemberships'; const SUBMISSIONS_TABLE = process.env.SUBMISSIONS_TABLE_NAME || 'ClassroomSubmissions'; +const KICK_REQUESTS_TABLE = process.env.KICK_REQUESTS_TABLE_NAME || 'ClassroomKickRequests'; const SUBMISSIONS_BUCKET = process.env.SUBMISSIONS_BUCKET_NAME || 'smalruby-classroom-submissions'; const GOOGLE_CLIENT_ID = process.env.GOOGLE_CLIENT_ID || ''; const MICROSOFT_CLIENT_ID = process.env.MICROSOFT_CLIENT_ID || ''; @@ -56,6 +57,10 @@ const PRESIGNED_URL_DOWNLOAD_EXPIRY = parseInt(process.env.PRESIGNED_URL_DOWNLOA const MAX_FILE_SIZE = 10 * 1024 * 1024; // 10 MB const MAX_SCREENSHOT_COUNT = 20; const MAX_TEACHER_COMMENT_LENGTH = 500; +// Kick request TTL: short-lived (1h) since the student is actively waiting +// for the teacher to act. Expired requests are removed by DynamoDB TTL. +const KICK_REQUEST_TTL_SECONDS = parseInt(process.env.KICK_REQUEST_TTL_SECONDS || '3600', 10); +const MAX_KICK_REQUEST_REASON_LENGTH = 200; // --- DynamoDB Client --- @@ -829,6 +834,209 @@ async function handleDeleteMember(teacherSub: string, classroomId: string, membe return { statusCode: 204, body: '' }; } +// --- Kick request handlers --- + +export function validateKickRequestReason(reason: unknown): string | undefined { + if (reason === undefined || reason === null || reason === '') return undefined; + if (typeof reason !== 'string') { + throw new ValidationError('Kick request reason must be a string'); + } + const trimmed = reason.trim(); + if (trimmed.length > MAX_KICK_REQUEST_REASON_LENGTH) { + throw new ValidationError(`Kick request reason must be ${MAX_KICK_REQUEST_REASON_LENGTH} characters or less`); + } + return trimmed || undefined; +} + +// Lookup classroom by joinCode + ensure a non-kicked occupant exists at the +// given seat. Used by handleCreateKickRequest to refuse requests for seats +// that are already empty (so the teacher doesn't see noise from misclicks +// or stale UI). Returns the classroom row. +async function findClassroomWithSeatOccupied( + joinCode: string, + seatNumber: number, +): Promise> { + const classroomResult = await docClient.send(new QueryCommand({ + TableName: CLASSROOMS_TABLE, + IndexName: 'joinCode-index', + KeyConditionExpression: 'joinCode = :jc', + ExpressionAttributeValues: { ':jc': joinCode }, + Limit: 1, + })); + if (!classroomResult.Items || classroomResult.Items.length === 0) { + throw new NotFoundError('Invalid join code'); + } + const classroom = classroomResult.Items[0]; + if (classroom.status !== 'active') { + throw new NotFoundError('This classroom is no longer active'); + } + if (seatNumber < 1 || seatNumber > (classroom.studentCount as number)) { + throw new ValidationError(`Seat number must be between 1 and ${classroom.studentCount}`); + } + const memberId = `seat-${String(seatNumber).padStart(2, '0')}`; + const memberResult = await docClient.send(new GetCommand({ + TableName: MEMBERSHIPS_TABLE, + Key: { classroomId: classroom.classroomId, memberId }, + })); + if (!memberResult.Item || memberResult.Item.kicked === true) { + // Seat already empty (or only holds a kick tombstone) — nothing to free up. + throw new NotFoundError(`Seat ${seatNumber} is not currently occupied`); + } + return classroom; +} + +async function handleCreateKickRequest( + sourceIp: string, + body: Record, +): Promise { + // Reuse the join endpoint's IP-based rate limit — same threat model + // (anonymous endpoint, abuse risk if open). The body fields are validated + // independently below so we surface friendlier errors than the limit. + checkJoinRateLimit(sourceIp); + const joinCode = validateJoinCode(body.joinCode); + const seatRaw = body.seatNumber; + const seatNumber = + typeof seatRaw === 'number' ? seatRaw : parseInt(String(seatRaw), 10); + if (isNaN(seatNumber)) { + throw new ValidationError('Seat number is required'); + } + const reason = validateKickRequestReason(body.reason); + const classroom = await findClassroomWithSeatOccupied(joinCode, seatNumber); + if (seatNumber < 1 || seatNumber > (classroom.studentCount as number)) { + throw new ValidationError(`Seat number must be between 1 and ${classroom.studentCount}`); + } + + const requestId = crypto.randomUUID(); + const now = new Date().toISOString(); + await docClient.send(new PutCommand({ + TableName: KICK_REQUESTS_TABLE, + Item: { + classroomId: classroom.classroomId, + requestId, + seatNumber, + reason: reason || null, + sourceIpHash: crypto.createHash('sha256').update(sourceIp).digest('hex').slice(0, 16), + createdAt: now, + ttl: Math.floor(Date.now() / 1000) + KICK_REQUEST_TTL_SECONDS, + }, + })); + + return { + statusCode: 201, + body: JSON.stringify({ + requestId, + classroomId: classroom.classroomId, + seatNumber, + }), + }; +} + +async function handleListKickRequests( + teacherSub: string, + classroomId: string, +): Promise { + // Verify ownership: only the owning teacher may list requests. + const classroom = await docClient.send(new GetCommand({ + TableName: CLASSROOMS_TABLE, + Key: { classroomId }, + })); + if (!classroom.Item || classroom.Item.teacherSub !== teacherSub) { + throw new AuthError('Not authorized to view kick requests for this classroom'); + } + + const result = await docClient.send(new QueryCommand({ + TableName: KICK_REQUESTS_TABLE, + KeyConditionExpression: 'classroomId = :cid', + ExpressionAttributeValues: { ':cid': classroomId }, + })); + const requests = (result.Items || []).map(item => ({ + requestId: item.requestId, + seatNumber: item.seatNumber, + reason: item.reason || null, + createdAt: item.createdAt, + })); + + return { + statusCode: 200, + body: JSON.stringify({ requests }), + }; +} + +async function handleApproveKickRequest( + teacherSub: string, + classroomId: string, + requestId: string, +): Promise { + // Verify ownership and read the request to learn which seat to kick. + const classroom = await docClient.send(new GetCommand({ + TableName: CLASSROOMS_TABLE, + Key: { classroomId }, + })); + if (!classroom.Item || classroom.Item.teacherSub !== teacherSub) { + throw new AuthError('Not authorized to modify kick requests for this classroom'); + } + const reqResult = await docClient.send(new GetCommand({ + TableName: KICK_REQUESTS_TABLE, + Key: { classroomId, requestId }, + })); + if (!reqResult.Item) { + // Request already gone (TTL or someone else acted on it). Treat as success. + return { statusCode: 204, body: '' }; + } + const seatNumber = reqResult.Item.seatNumber as number; + const memberId = `seat-${String(seatNumber).padStart(2, '0')}`; + + // Reuse the existing kick logic so kicked students get the same + // 410 reason='kicked' from verify-session that a direct DELETE + // /members/{memberId} would produce. + await handleDeleteMember(teacherSub, classroomId, memberId); + + // Delete all requests targeting this seat (including the approved one + // and any duplicates the same seat may have accumulated). Otherwise + // the teacher would see ghost rows asking to kick a seat that is now + // empty. + const siblings = await docClient.send(new QueryCommand({ + TableName: KICK_REQUESTS_TABLE, + IndexName: 'classroomId-seatNumber-index', + KeyConditionExpression: 'classroomId = :cid AND seatNumber = :sn', + ExpressionAttributeValues: { ':cid': classroomId, ':sn': seatNumber }, + ProjectionExpression: 'requestId', + })); + if (siblings.Items && siblings.Items.length > 0) { + for (let i = 0; i < siblings.Items.length; i += 25) { + const batch = siblings.Items.slice(i, i + 25); + await docClient.send(new BatchWriteCommand({ + RequestItems: { + [KICK_REQUESTS_TABLE]: batch.map(item => ({ + DeleteRequest: { Key: { classroomId, requestId: item.requestId as string } }, + })), + }, + })); + } + } + + return { statusCode: 204, body: '' }; +} + +async function handleRejectKickRequest( + teacherSub: string, + classroomId: string, + requestId: string, +): Promise { + const classroom = await docClient.send(new GetCommand({ + TableName: CLASSROOMS_TABLE, + Key: { classroomId }, + })); + if (!classroom.Item || classroom.Item.teacherSub !== teacherSub) { + throw new AuthError('Not authorized to modify kick requests for this classroom'); + } + await docClient.send(new DeleteCommand({ + TableName: KICK_REQUESTS_TABLE, + Key: { classroomId, requestId }, + })); + return { statusCode: 204, body: '' }; +} + // --- Session Token Auth --- export async function verifySessionToken(sessionToken: string): Promise<{ classroomId: string; memberId: string }> { @@ -1424,6 +1632,14 @@ export const handler = async (event: APIGatewayProxyEventV2): Promise { + let classroomId: string; + let joinCode: string; + let sessionTokenA: string; + let requestId: string; + let request2Id: string; + + test('セットアップ: クラス作成', async () => { + const { status, data } = await request( + 'POST', + '/classrooms', + { className: 'Kick Request Test', assignmentName: '退室依頼テスト', studentCount: 5 }, + teacherHeaders, + ); + expect(status).toBe(201); + classroomId = data.classroomId as string; + joinCode = data.joinCode as string; + }); + + test('セットアップ: 生徒A が席1で参加', async () => { + const { status, data } = await request('POST', '/classrooms/join', { + joinCode, + seatNumber: 1, + nickname: '生徒A', + }); + expect(status).toBe(200); + sessionTokenA = data.sessionToken as string; + }); + + test('GET /classrooms/{id}/kick-requests — 初期状態は空', async () => { + const { status, data } = await request( + 'GET', + `/classrooms/${classroomId}/kick-requests`, + null, + teacherHeaders, + ); + expect(status).toBe(200); + expect(data.requests).toEqual([]); + }); + + test('POST /classrooms/lookup/kick-request — joinCode + seatNumber でリクエスト送信 (認証不要)', async () => { + const { status, data } = await request('POST', '/classrooms/lookup/kick-request', { + joinCode, + seatNumber: 1, + reason: 'これは私の席です', + }); + expect(status).toBe(201); + expect(data.requestId).toBeDefined(); + expect(data.classroomId).toBe(classroomId); + expect(data.seatNumber).toBe(1); + requestId = data.requestId as string; + }); + + test('POST /classrooms/lookup/kick-request — 不正な joinCode で 404', async () => { + const { status } = await request('POST', '/classrooms/lookup/kick-request', { + joinCode: 'zzzzzz', + seatNumber: 1, + }); + expect(status).toBe(404); + }); + + test('POST /classrooms/lookup/kick-request — 範囲外の seatNumber で 400', async () => { + const { status } = await request('POST', '/classrooms/lookup/kick-request', { + joinCode, + seatNumber: 99, + }); + expect(status).toBe(400); + }); + + test('GET /classrooms/{id}/kick-requests — リクエスト 1 件返る (reason 付き)', async () => { + const { status, data } = await request( + 'GET', + `/classrooms/${classroomId}/kick-requests`, + null, + teacherHeaders, + ); + expect(status).toBe(200); + const requests = data.requests as Array<{ + requestId: string; + seatNumber: number; + reason: string | null; + createdAt: string; + }>; + expect(requests).toHaveLength(1); + expect(requests[0].requestId).toBe(requestId); + expect(requests[0].seatNumber).toBe(1); + expect(requests[0].reason).toBe('これは私の席です'); + }); + + test('POST /classrooms/lookup/kick-request — 同じ席に対する 2 回目のリクエストは別レコードとして許可される (abuse 規制なし、複数件許可仕様)', async () => { + const { status, data } = await request('POST', '/classrooms/lookup/kick-request', { + joinCode, + seatNumber: 1, + }); + expect(status).toBe(201); + expect(data.requestId).toBeDefined(); + expect(data.requestId).not.toBe(requestId); + request2Id = data.requestId as string; + }); + + test('GET /classrooms/{id}/kick-requests — リクエスト 2 件返る', async () => { + const { status, data } = await request( + 'GET', + `/classrooms/${classroomId}/kick-requests`, + null, + teacherHeaders, + ); + expect(status).toBe(200); + const requests = data.requests as Array<{ requestId: string }>; + expect(requests.map(r => r.requestId).sort()).toEqual([requestId, request2Id].sort()); + }); + + test('DELETE /classrooms/{id}/kick-requests/{requestId} — 教師が却下 → リクエスト削除のみでメンバー残る', async () => { + const { status } = await request( + 'DELETE', + `/classrooms/${classroomId}/kick-requests/${request2Id}`, + null, + teacherHeaders, + ); + expect(status).toBe(204); + + // 却下後: リクエストは 1 件に減る + const list = await request( + 'GET', + `/classrooms/${classroomId}/kick-requests`, + null, + teacherHeaders, + ); + const requests = list.data.requests as Array<{ requestId: string }>; + expect(requests.map(r => r.requestId)).toEqual([requestId]); + + // メンバー A はまだ参加中 (kick されていない) + const verifyA = await request('POST', '/classrooms/verify-session', null, { + Authorization: `Bearer ${sessionTokenA}`, + }); + expect(verifyA.status).toBe(200); + }); + + test('POST /classrooms/{id}/kick-requests/{requestId}/approve — 教師が承認 → 該当メンバー kick + リクエスト削除', async () => { + const { status } = await request( + 'POST', + `/classrooms/${classroomId}/kick-requests/${requestId}/approve`, + null, + teacherHeaders, + ); + expect(status).toBe(204); + + // 承認後: リクエスト一覧は空 + const list = await request( + 'GET', + `/classrooms/${classroomId}/kick-requests`, + null, + teacherHeaders, + ); + expect(list.data.requests).toEqual([]); + + // メンバー A の verify-session は 410 reason=kicked を返す (Phase 1 と同じ挙動) + const verifyA = await request('POST', '/classrooms/verify-session', null, { + Authorization: `Bearer ${sessionTokenA}`, + }); + expect(verifyA.status).toBe(410); + expect(verifyA.data.reason).toBe('kicked'); + }); + + test('POST /classrooms/lookup/kick-request — kick 済みの席に対するリクエストは席が空くので拒否される (404 — 席に占有者なし)', async () => { + const { status } = await request('POST', '/classrooms/lookup/kick-request', { + joinCode, + seatNumber: 1, + }); + expect(status).toBe(404); + }); + + test('認証エラー: 生徒がリクエスト一覧を見ようとすると 401', async () => { + const { status } = await request( + 'GET', + `/classrooms/${classroomId}/kick-requests`, + null, + { Authorization: `Bearer ${sessionTokenA}` }, + ); + expect(status).toBe(401); + }); + + test('認証エラー: 生徒が承認エンドポイントを叩こうとすると 401', async () => { + const { status } = await request( + 'POST', + `/classrooms/${classroomId}/kick-requests/anything/approve`, + null, + { Authorization: `Bearer ${sessionTokenA}` }, + ); + expect(status).toBe(401); + }); + + test('認証エラー: 別教師トークンで kick-requests を見ようとすると 401', async () => { + // 別 teacherSub をシミュレートできないので、無効トークンで代替 + const { status } = await request( + 'GET', + `/classrooms/${classroomId}/kick-requests`, + null, + { Authorization: 'Bearer invalid-teacher-token' }, + ); + expect(status).toBe(401); + }); + + // Cleanup + test('クリーンアップ: クラス削除', async () => { + const { status } = await request('DELETE', `/classrooms/${classroomId}`, null, teacherHeaders); + expect(status).toBe(204); + }); +}); diff --git a/infra/smalruby-classroom/lib/classroom-stack.ts b/infra/smalruby-classroom/lib/classroom-stack.ts index bd2d178739a..fe046e2e846 100644 --- a/infra/smalruby-classroom/lib/classroom-stack.ts +++ b/infra/smalruby-classroom/lib/classroom-stack.ts @@ -16,6 +16,7 @@ export class ClassroomStack extends cdk.Stack { public readonly classroomsTable: dynamodb.Table; public readonly membershipsTable: dynamodb.Table; public readonly submissionsTable: dynamodb.Table; + public readonly kickRequestsTable: dynamodb.Table; public readonly submissionsBucket: s3.Bucket; public readonly api: apigatewayv2.HttpApi; @@ -156,6 +157,44 @@ export class ClassroomStack extends cdk.Stack { cdk.Tags.of(this.submissionsTable).add('ResourceType', 'DynamoDB'); + // Kick requests table — short-lived (1h TTL) records of students asking + // the teacher to free up a specific seat. PK is classroomId; SK is the + // requestId (UUID) so multiple pending requests for the same seat + // coexist. A GSI on (classroomId, seatNumber) lets the approve handler + // delete sibling requests for the same seat in one query after kick. + this.kickRequestsTable = new dynamodb.Table(this, 'KickRequestsTable', { + tableName: `ClassroomKickRequests${stageSuffix}`, + partitionKey: { + name: 'classroomId', + type: dynamodb.AttributeType.STRING, + }, + sortKey: { + name: 'requestId', + type: dynamodb.AttributeType.STRING, + }, + billingMode: dynamodb.BillingMode.PAY_PER_REQUEST, + removalPolicy: cdk.RemovalPolicy.DESTROY, + pointInTimeRecoverySpecification: { + pointInTimeRecoveryEnabled: false, + }, + timeToLiveAttribute: 'ttl', + }); + + this.kickRequestsTable.addGlobalSecondaryIndex({ + indexName: 'classroomId-seatNumber-index', + partitionKey: { + name: 'classroomId', + type: dynamodb.AttributeType.STRING, + }, + sortKey: { + name: 'seatNumber', + type: dynamodb.AttributeType.NUMBER, + }, + projectionType: dynamodb.ProjectionType.ALL, + }); + + cdk.Tags.of(this.kickRequestsTable).add('ResourceType', 'DynamoDB'); + // --- S3 Bucket for submissions --- this.submissionsBucket = new s3.Bucket(this, 'SubmissionsBucket', { @@ -201,6 +240,7 @@ export class ClassroomStack extends cdk.Stack { CLASSROOMS_TABLE_NAME: this.classroomsTable.tableName, MEMBERSHIPS_TABLE_NAME: this.membershipsTable.tableName, SUBMISSIONS_TABLE_NAME: this.submissionsTable.tableName, + KICK_REQUESTS_TABLE_NAME: this.kickRequestsTable.tableName, SUBMISSIONS_BUCKET_NAME: this.submissionsBucket.bucketName, GOOGLE_CLIENT_ID: googleClientId, MICROSOFT_CLIENT_ID: microsoftClientId, @@ -225,6 +265,7 @@ export class ClassroomStack extends cdk.Stack { this.classroomsTable.grantReadWriteData(handlerFn); this.membershipsTable.grantReadWriteData(handlerFn); this.submissionsTable.grantReadWriteData(handlerFn); + this.kickRequestsTable.grantReadWriteData(handlerFn); this.submissionsBucket.grantPut(handlerFn); this.submissionsBucket.grantRead(handlerFn); @@ -326,6 +367,31 @@ export class ClassroomStack extends cdk.Stack { integration, }); + // Kick requests — students ask the teacher to free a specific seat. + this.api.addRoutes({ + path: '/classrooms/lookup/kick-request', + methods: [apigatewayv2.HttpMethod.POST], + integration, + }); + + this.api.addRoutes({ + path: '/classrooms/{classroomId}/kick-requests', + methods: [apigatewayv2.HttpMethod.GET], + integration, + }); + + this.api.addRoutes({ + path: '/classrooms/{classroomId}/kick-requests/{requestId}', + methods: [apigatewayv2.HttpMethod.DELETE], + integration, + }); + + this.api.addRoutes({ + path: '/classrooms/{classroomId}/kick-requests/{requestId}/approve', + methods: [apigatewayv2.HttpMethod.POST], + integration, + }); + this.api.addRoutes({ path: '/classrooms/{classroomId}/submissions', methods: [apigatewayv2.HttpMethod.POST, apigatewayv2.HttpMethod.GET], From 4ef1bfad8cda540ad3e352d5770401d8aec2a4ae Mon Sep 17 00:00:00 2001 From: Kouji Takao Date: Thu, 21 May 2026 19:25:56 +0900 Subject: [PATCH 07/10] feat(classroom): allow students to request seat reclaim from teacher MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wire the student-side UI for the Phase 4 kick-request endpoints. When a student opens the seat-selection screen and finds the seat they actually want already occupied (e.g. someone picked the wrong seat number first), they can now tap the grey-tinted seat to send a request to the teacher. - classroom-api.js: add createKickRequest, listKickRequests, approveKickRequest, rejectKickRequest methods. - classroom-kick-request-storage.js: small localStorage helper that persists {requestId, joinCode, seatNumber, reason, createdAt} so the "依頼中" state survives a tab close or reload. Records older than the backend's 1h TTL are dropped on load so we don't keep showing the pending banner forever. - StudentSeatSelector: render occupied seats as tappable when an onRequestKick handler is provided AND no pending request is already outstanding (we cap at one in-flight request to keep the teacher's view manageable). Show a blue "Waiting for the teacher to free seat NN..." banner above the grid while a request is pending. The dialog replaces the grid (vs. overlaying) so users on small viewports can still tap the buttons. - KickRequestConfirmDialog (new): confirm dialog with an optional reason textarea (200 char max enforced both client- and server-side) and Cancel / Send buttons. The dialog inherits the modal's container styling so it doesn't introduce a new portal. - classroom-modal.jsx container: holds dialog/pending state, calls the API, persists to localStorage, and polls lookupClassroom every 5s while a request is outstanding. When the targeted seat is no longer in takenSeats, the teacher acted (approve = handleDeleteMember kicked the occupant; reject = request just disappeared from the list; TTL = backend GC'd it after an hour) — we clear pending state so the student can re-pick the seat. Submitting a successful join also clears pending state. - locales: gui.classroom.kickRequest.{title,body,reasonPlaceholder, cancel,submit,pendingBanner} in en/ja/ja-Hira. End-to-end Playwright check on localhost against stg backend: 1. seat 2 occupied by another student via API. 2. Open `?classcode=...` → student-seat. Seat 2 is grey but clickable; `data-taken="1"` exposed for tests. 3. Tap seat 2 → kick-request-confirm-dialog opens with title "出席番号02の人に退室を依頼しますか?". 4. Type "私の席です" reason → submit → 201 from /classrooms/lookup/kick-request. Banner appears, seat 2 is no longer tappable (`data-taken="0"`), localStorage holds the pending record. 5. Teacher approves via API. Within one polling tick (5s) the banner disappears, seat 2 becomes available, localStorage is cleared, and the student can now select it. Unit tests added (14 total): * createKickRequest: payload shape, omits reason, no Authorization header * Storage helper: load/save/clear, stale-record drop, malformed JSON, missing fields, no-op for invalid input Refs #692 Co-Authored-By: Claude Opus 4.7 (1M context) --- .../scratch-gui/smalruby-prettier-files.md | 2 + packages/scratch-gui/.prettierignore | 2 + .../classroom-modal/classroom-modal.css | 78 +++++++++++++ .../classroom-modal/classroom-modal.jsx | 22 ++++ .../kick-request-confirm-dialog.jsx | 103 ++++++++++++++++++ .../classroom-modal/student-seat-selector.jsx | 70 ++++++++++-- .../src/containers/classroom-modal.jsx | 99 +++++++++++++++++ packages/scratch-gui/src/lib/classroom-api.js | 50 +++++++++ .../src/lib/classroom-kick-request-storage.js | 87 +++++++++++++++ packages/scratch-gui/src/locales/en.js | 8 ++ packages/scratch-gui/src/locales/ja-Hira.js | 8 ++ packages/scratch-gui/src/locales/ja.js | 7 ++ .../test/unit/lib/classroom-api.test.js | 54 +++++++++ .../classroom-kick-request-storage.test.js | 91 ++++++++++++++++ 14 files changed, 674 insertions(+), 7 deletions(-) create mode 100644 packages/scratch-gui/src/components/classroom-modal/kick-request-confirm-dialog.jsx create mode 100644 packages/scratch-gui/src/lib/classroom-kick-request-storage.js create mode 100644 packages/scratch-gui/test/unit/lib/classroom-kick-request-storage.test.js diff --git a/.claude/rules/scratch-gui/smalruby-prettier-files.md b/.claude/rules/scratch-gui/smalruby-prettier-files.md index 6b7c52d9312..96c8097eee5 100644 --- a/.claude/rules/scratch-gui/smalruby-prettier-files.md +++ b/.claude/rules/scratch-gui/smalruby-prettier-files.md @@ -100,6 +100,7 @@ upstream (Scratch) ファイルは対象外。 - `src/lib/auto-correct.js` - `src/lib/backpack-mesh-v1-migration.js` - `src/lib/classroom-api.js` +- `src/lib/classroom-kick-request-storage.js` - `src/lib/deck-setup.js` - `src/lib/google-classroom-auth.js` - `src/lib/block-utils.js` @@ -237,6 +238,7 @@ upstream (Scratch) ファイルは対象外。 - `test/unit/lib/blockly-private-api.test.js` - `test/unit/lib/blocks-gesture-recovery.test.js` - `test/unit/lib/classroom-api.test.js` +- `test/unit/lib/classroom-kick-request-storage.test.js` - `test/unit/lib/deck-setup.test.js` - `test/unit/lib/blocks-screenshot.test.js` - `test/unit/lib/calculate-popup-position.test.js` diff --git a/packages/scratch-gui/.prettierignore b/packages/scratch-gui/.prettierignore index 1878105a42c..82d3fa86f96 100644 --- a/packages/scratch-gui/.prettierignore +++ b/packages/scratch-gui/.prettierignore @@ -124,6 +124,7 @@ src/lib/* !src/lib/backpack-mesh-v1-migration.js !src/lib/deck-setup.js !src/lib/classroom-api.js +!src/lib/classroom-kick-request-storage.js !src/lib/google-classroom-auth.js !src/lib/block-utils.js !src/lib/blocks-gesture-recovery.js @@ -314,6 +315,7 @@ test/unit/lib/* !test/unit/lib/blockly-private-api.test.js !test/unit/lib/blocks-gesture-recovery.test.js !test/unit/lib/classroom-api.test.js +!test/unit/lib/classroom-kick-request-storage.test.js !test/unit/lib/deck-setup.test.js !test/unit/lib/blocks-screenshot.test.js !test/unit/lib/calculate-popup-position.test.js diff --git a/packages/scratch-gui/src/components/classroom-modal/classroom-modal.css b/packages/scratch-gui/src/components/classroom-modal/classroom-modal.css index 3ee849315fd..b4fda37933e 100644 --- a/packages/scratch-gui/src/components/classroom-modal/classroom-modal.css +++ b/packages/scratch-gui/src/components/classroom-modal/classroom-modal.css @@ -527,6 +527,84 @@ color: #4a3500; } +.kick-request-pending-banner { + display: flex; + align-items: center; + gap: 0.5rem; + background-color: #e3f2fd; + border: 1px solid #2196f3; + border-radius: 8px; + padding: 0.8rem 1rem; + margin-bottom: 1rem; + color: #0d47a1; + font-size: 0.9rem; + line-height: 1.5; +} + +.kick-request-dialog { + background-color: #fff; + border-radius: 8px; + padding: 1rem 1.2rem; + margin: 0 auto; + max-width: 28rem; +} + +.kick-request-dialog-title { + font-size: 1.05rem; + margin: 0 0 0.6rem 0; + color: #1a1a4e; +} + +.kick-request-dialog-body { + font-size: 0.9rem; + color: #575e75; + margin: 0 0 0.8rem 0; + line-height: 1.5; +} + +.kick-request-reason-input { + width: 100%; + box-sizing: border-box; + font-size: 0.9rem; + font-family: inherit; + padding: 0.5rem; + border: 1px solid #d9d9d9; + border-radius: 4px; + resize: vertical; +} + +.kick-request-dialog-error { + margin-top: 0.5rem; + color: #d32f2f; + font-size: 0.85rem; +} + +.kick-request-dialog-buttons { + display: flex; + justify-content: flex-end; + gap: 0.6rem; + margin-top: 1rem; +} + +.secondary-button { + background-color: #fff; + color: #575e75; + border: 1px solid #b8b8b8; + border-radius: 0.5rem; + padding: 0.5rem 1rem; + cursor: pointer; + font-size: 0.9rem; +} + +.secondary-button:hover { + background-color: #f0f0f0; +} + +.secondary-button:disabled { + opacity: 0.5; + cursor: not-allowed; +} + .kicked-banner-text { flex: 1; font-size: 0.9rem; diff --git a/packages/scratch-gui/src/components/classroom-modal/classroom-modal.jsx b/packages/scratch-gui/src/components/classroom-modal/classroom-modal.jsx index 3d7ba0fade3..6b5862d3920 100644 --- a/packages/scratch-gui/src/components/classroom-modal/classroom-modal.jsx +++ b/packages/scratch-gui/src/components/classroom-modal/classroom-modal.jsx @@ -28,6 +28,9 @@ const ClassroomModal = ({ selectedSeat, joinedInfo, kickedNotice, + kickRequestDialogSeat, + kickRequestPending, + kickRequestError, error, errorActionHandler, errorActionLabel, @@ -40,6 +43,9 @@ const ClassroomModal = ({ onConfirmJoin, onClose, onDismissKickedNotice, + onRequestKick, + onConfirmKickRequest, + onCancelKickRequest, onLeaveClassroom, onStartSubmit, onConfirmSubmit, @@ -82,11 +88,17 @@ const ClassroomModal = ({ errorTitle={errorTitle} isLoading={isLoading} kickedNotice={kickedNotice} + kickRequestDialogSeat={kickRequestDialogSeat} + kickRequestError={kickRequestError} + kickRequestPending={kickRequestPending} seatCount={seatCount} selectedSeat={selectedSeat} takenSeats={takenSeats} + onCancelKickRequest={onCancelKickRequest} onConfirmJoin={onConfirmJoin} + onConfirmKickRequest={onConfirmKickRequest} onDismissKickedNotice={onDismissKickedNotice} + onRequestKick={onRequestKick} onSelectSeat={onSelectSeat} /> )} @@ -156,12 +168,22 @@ ClassroomModal.propTypes = { className: PropTypes.string, seatNumber: PropTypes.number, }), + kickRequestDialogSeat: PropTypes.number, + kickRequestError: PropTypes.string, + kickRequestPending: PropTypes.shape({ + requestId: PropTypes.string, + joinCode: PropTypes.string, + seatNumber: PropTypes.number, + }), + onCancelKickRequest: PropTypes.func, onCancelSubmit: PropTypes.func.isRequired, onClose: PropTypes.func.isRequired, onConfirmJoin: PropTypes.func.isRequired, + onConfirmKickRequest: PropTypes.func, onConfirmSubmit: PropTypes.func.isRequired, onDismissKickedNotice: PropTypes.func, onJoinWithCode: PropTypes.func.isRequired, + onRequestKick: PropTypes.func, onLeaveClassroom: PropTypes.func.isRequired, onRefreshStudentStatus: PropTypes.func.isRequired, onSelectSeat: PropTypes.func.isRequired, diff --git a/packages/scratch-gui/src/components/classroom-modal/kick-request-confirm-dialog.jsx b/packages/scratch-gui/src/components/classroom-modal/kick-request-confirm-dialog.jsx new file mode 100644 index 00000000000..5ec2b6a8163 --- /dev/null +++ b/packages/scratch-gui/src/components/classroom-modal/kick-request-confirm-dialog.jsx @@ -0,0 +1,103 @@ +import { FormattedMessage, defineMessages, useIntl } from 'react-intl'; +import PropTypes from 'prop-types'; +import React, { useCallback, useState } from 'react'; + +import styles from './classroom-modal.css'; + +const messages = defineMessages({ + reasonPlaceholder: { + defaultMessage: 'Optional: tell the teacher why you need this seat (max 200 chars).', + description: 'Placeholder text in the kick-request reason textarea', + id: 'gui.classroom.kickRequest.reasonPlaceholder', + }, +}); + +const MAX_REASON_LENGTH = 200; + +const KickRequestConfirmDialog = ({ seatNumber, isLoading, error, onCancel, onConfirm }) => { + const intl = useIntl(); + const [reason, setReason] = useState(''); + + const handleReasonChange = useCallback((e) => { + const value = e.target.value; + setReason(value.length > MAX_REASON_LENGTH ? value.slice(0, MAX_REASON_LENGTH) : value); + }, []); + + const handleConfirm = useCallback(() => { + const trimmed = reason.trim(); + onConfirm(trimmed === '' ? null : trimmed); + }, [onConfirm, reason]); + + return ( +
+

+ +

+

+ +

+