Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@ A comprehensive, offline-first rubric creation and grading tool built with React
- **View feedback**: Students see their grades, teacher comments, and attached files.
- **Submit essays**: Anonymous essay submission via submission codes.
- **Self-assessment**: Students complete CEFR self-assessments from their portal.
- **My Work**: A combined to-do list of assigned essays and tests, grouped into Overdue/Planned/Completed with per-item status (not started/in progress/submitted). Tests open in one click from the list, backed by a `test_assignments` Supabase table mirroring `essay_assignments`.
- **My Progress**: A radar chart of the student's own per-criterion scores, combined across every graded rubric (criteria with matching titles averaged together) or filtered to a single rubric.

### 7. Customisation & Accessibility

Expand Down
146 changes: 136 additions & 10 deletions src/components/Tests/TestAssignmentModal.tsx
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import React, { useState, useCallback, useMemo } from 'react';
import { X, Copy, Check, ClipboardCheck, Database, ExternalLink } from 'lucide-react';
import React, { useState, useCallback, useMemo, useEffect } from 'react';
import { X, Copy, Check, ClipboardCheck, Database, ExternalLink, AlertCircle } from 'lucide-react';
import { useTranslation } from 'react-i18next';
import Modal from '../ui/Modal';
import { useApp } from '../../context/AppContext';
import { useDbStatus } from '../../hooks/useDbStatus';
import { loadSupabaseConfig } from '../../services/database';
import { encodeTestAssignment } from '../../utils/shareCode';
import { nanoid } from '../../utils/nanoid';
import type { Test, TestAssignmentPayload } from '../../types';
import type { Test, TestAssignmentPayload, TestAssignment } from '../../types';

interface Props {
test: Test;
Expand All @@ -16,7 +16,7 @@ interface Props {

export default function TestAssignmentModal({ test, onClose }: Props) {
const { t } = useTranslation();
const { students, classes, settings } = useApp();
const { students, classes, settings, saveTestAssignment } = useApp();
const dbStatus = useDbStatus();
const config = loadSupabaseConfig();

Expand All @@ -25,20 +25,49 @@ export default function TestAssignmentModal({ test, onClose }: Props) {
const [embedDb, setEmbedDb] = useState(dbStatus.isConnected);
const [copiedStudentId, setCopiedStudentId] = useState<string | null>(null);
const [copiedAll, setCopiedAll] = useState(false);
// Keyed by `${studentId}::${expiresAt}` rather than bare student id, so changing the
// deadline after an initial auto-save is treated as a new payload and re-saved, instead
// of silently leaving already-persisted rows stuck on their original (now stale) expiry.
const [savedKeys, setSavedKeys] = useState<Set<string>>(new Set());
const [saving, setSaving] = useState(false);
const [saveErrorCount, setSaveErrorCount] = useState(0);

const teacherKey = useMemo(() => nanoid(), []);
const savedKeyFor = useCallback((studentId: string) => `${studentId}::${expiresAt}`, [expiresAt]);

const classStudents = useMemo(
() => students.filter((s) => (classId ? s.classId === classId : true)),
[students, classId]
);

// One teacherKey per student — test_assignments rows are 1:1 with a single teacherKey
// server-side (same constraint essay_assignments has), so a whole-class share batch
// needs a distinct row id per student rather than the single shared key used for the
// offline/legacy link format. Keyed off the full `students` list (not the class-filtered
// one) so switching the class dropdown back and forth doesn't regenerate keys for
// students already saved under their original key — savedKeys tracks per-student save
// state, and a regenerated key for an already-saved student would silently un-sync the
// displayed link from what's actually persisted.
const teacherKeys = useMemo(() => {
const map: Record<string, string> = {};
students.forEach((s) => {
map[s.id] = nanoid();
});
return map;
}, [students]);

// Saved-progress display must be scoped to the CURRENT class, not the lifetime total
// across every class visited this session — savedKeys accumulates globally so the
// save logic can skip already-persisted students on a class revisit, but showing that
// raw total against the current (possibly smaller) class's count would read as nonsense
// (e.g. "3/1 saved").
const classSavedCount = classStudents.filter((s) => savedKeys.has(savedKeyFor(s.id))).length;

const buildAssignment = useCallback(
(studentId: string): TestAssignmentPayload => {
const base: TestAssignmentPayload = {
testId: test.id,
studentId,
teacherKey,
teacherKey: teacherKeys[studentId] ?? nanoid(),
requireSEB: test.requireSEB,
durationMinutes: test.durationMinutes,
createdAt: new Date().toISOString(),
Expand All @@ -52,14 +81,61 @@ export default function TestAssignmentModal({ test, onClose }: Props) {
}
return base;
},
[test, teacherKey, expiresAt, embedDb, dbStatus.isConnected, config]
[test, teacherKeys, expiresAt, embedDb, dbStatus.isConnected, config]
);

function buildUrl(studentId: string): string {
const code = encodeTestAssignment(buildAssignment(studentId));
return `${window.location.origin}${window.location.pathname}#/test/${code}`;
}

const handleSaveAllToDb = useCallback(async () => {
setSaving(true);
setSaveErrorCount(0);
try {
const nowSaved = new Set(savedKeys);
const pending = classStudents.filter((s) => !nowSaved.has(savedKeyFor(s.id)));
const results = await Promise.allSettled(
pending.map((s) =>
saveTestAssignment({
testId: test.id,
studentId: s.id,
teacherKey: teacherKeys[s.id],
testName: test.name,
requireSEB: test.requireSEB,
durationMinutes: test.durationMinutes,
createdAt: new Date().toISOString(),
expiresAt: expiresAt ? new Date(expiresAt).toISOString() : undefined,
} satisfies TestAssignment)
)
);
let errors = 0;
results.forEach((result, i) => {
if (result.status === 'fulfilled' && result.value.success) {
nowSaved.add(savedKeyFor(pending[i].id));
} else {
errors += 1;
}
});
setSavedKeys(nowSaved);
setSaveErrorCount(errors);
} finally {
setSaving(false);
}
}, [classStudents, teacherKeys, test, expiresAt, saveTestAssignment, savedKeys, savedKeyFor]);

// Auto-save as soon as a DB-mode batch of links becomes shareable, same rationale as
// EssayAssignmentModal: gating behind a separate button click leaves a window where a
// teacher could hand out a link before its row exists server-side. Re-runs when
// expiresAt changes too — savedKeyFor makes that a "new" payload per student, so
// editing the deadline after the first auto-save doesn't leave stale rows behind.
useEffect(() => {
if (embedDb && !saving) {
void handleSaveAllToDb();
}
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [embedDb, classId, expiresAt]);

async function handleCopyOne(studentId: string) {
try {
await navigator.clipboard.writeText(buildUrl(studentId));
Expand Down Expand Up @@ -175,9 +251,59 @@ export default function TestAssignmentModal({ test, onClose }: Props) {
{t('essay_assignment.db_embed_label')}
</label>
{embedDb && (
<p style={{ margin: 0, fontSize: '0.8rem', color: 'var(--text-muted)', lineHeight: 1.5 }}>
{t('essay_assignment.db_embed_help')}
</p>
<>
<p
style={{
margin: 0,
fontSize: '0.8rem',
color: 'var(--text-muted)',
lineHeight: 1.5,
}}
>
{t('tests.assignment_db_embed_help')}
</p>
<div style={{ display: 'flex', alignItems: 'center', gap: 8 }}>
<button
className="btn btn-secondary btn-sm"
onClick={handleSaveAllToDb}
disabled={saving || classSavedCount >= classStudents.length}
>
{classSavedCount >= classStudents.length && classStudents.length > 0 ? (
<>
<Check size={13} /> {t('essay_assignment.saved_to_db')}
</>
) : saving ? (
t('essay_assignment.saving')
) : (
<>
<Database size={13} /> {t('tests.assignment_save_all_to_db')}
</>
)}
</button>
{classStudents.length > 0 && (
<span style={{ fontSize: '0.78rem', color: 'var(--text-muted)' }}>
{t('tests.assignment_saved_count', {
saved: classSavedCount,
total: classStudents.length,
})}
</span>
)}
{saveErrorCount > 0 && (
<span
style={{
display: 'flex',
alignItems: 'center',
gap: 4,
color: 'var(--red)',
fontSize: '0.8rem',
}}
>
<AlertCircle size={12} />
{t('tests.assignment_save_partial_error', { count: saveErrorCount })}
</span>
)}
</div>
</>
)}
</div>
)}
Expand Down
81 changes: 74 additions & 7 deletions src/components/Tests/__tests__/TestAssignmentModal.test.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
import React from 'react';
import { screen } from '@testing-library/react';
import { describe, it, expect, vi } from 'vitest';
import { screen, waitFor, fireEvent } from '@testing-library/react';
import { describe, it, expect, vi, beforeEach } from 'vitest';
import { renderWithRouter } from '../../../test-utils/renderWithProviders';
import { DEFAULT_FORMAT } from '../../../types';
import type { AppSettings, Class, Student, Test as RmTest } from '../../../types';
import { decodeTestAssignment } from '../../../utils/shareCode';

const mockDbStatus = vi.hoisted(() => ({ isConnected: false }));

const mockSettings: AppSettings = {
defaultGradeScaleId: 'gs1',
theme: 'dark',
Expand All @@ -16,9 +18,11 @@ const mockSettings: AppSettings = {
};

const mockClass: Class = { id: 'c1', name: 'Class A' };
const mockClass2: Class = { id: 'c2', name: 'Class B' };
const mockStudents: Student[] = [
{ id: 's1', name: 'Alice', classId: 'c1' },
{ id: 's2', name: 'Bob', classId: 'c1' },
{ id: 's3', name: 'Carla', classId: 'c2' },
];

const mockTest: RmTest = {
Expand All @@ -31,11 +35,16 @@ const mockTest: RmTest = {
createdAt: '2024-01-01T00:00:00Z',
};

const classAStudents = mockStudents.filter((s) => s.classId === 'c1');

const mockSaveTestAssignment = vi.fn().mockResolvedValue({ success: true });

vi.mock('../../../context/AppContext', () => ({
useApp: () => ({
students: mockStudents,
classes: [mockClass],
classes: [mockClass, mockClass2],
settings: mockSettings,
saveTestAssignment: mockSaveTestAssignment,
}),
}));

Expand All @@ -50,19 +59,31 @@ vi.mock('react-i18next', () => ({
}));

vi.mock('../../../hooks/useDbStatus', () => ({
useDbStatus: () => ({ isConnected: false, status: 'idle', lastSyncAt: null, userId: null, currentUser: null }),
useDbStatus: () => ({
isConnected: mockDbStatus.isConnected,
status: 'idle',
lastSyncAt: null,
userId: null,
currentUser: null,
}),
}));

vi.mock('../../../services/database', () => ({
loadSupabaseConfig: vi.fn(() => null),
loadSupabaseConfig: vi.fn(() => ({ supabaseUrl: 'https://x.supabase.co', supabaseAnonKey: 'anon-key' })),
}));

describe('TestAssignmentModal', () => {
it('generates per-student share links that decode back to a valid TestAssignmentPayload', async () => {
beforeEach(() => {
mockDbStatus.isConnected = false;
mockSaveTestAssignment.mockClear();
});

it('generates per-student share links that decode back to a valid TestAssignmentPayload with distinct teacherKeys', async () => {
const { default: TestAssignmentModal } = await import('../TestAssignmentModal');
renderWithRouter(<TestAssignmentModal test={mockTest} onClose={vi.fn()} />);

for (const student of mockStudents) {
const teacherKeys = new Set<string>();
for (const student of classAStudents) {
const input = screen.getByLabelText(
`tests.assignment_link_for:{"name":"${student.name}"}`
) as HTMLInputElement;
Expand All @@ -78,6 +99,52 @@ describe('TestAssignmentModal', () => {
expect(decoded?.teacherKey).toBeTruthy();
expect(decoded?.requireSEB).toBe(true);
expect(decoded?.durationMinutes).toBe(30);
teacherKeys.add(decoded!.teacherKey);
}
// Each student's row must have its own id — a shared key would let one DB row
// (test_assignments is 1:1 per teacherKey) silently overwrite another's assignment.
expect(teacherKeys.size).toBe(classAStudents.length);
});

it('saves one test_assignments row per student when DB embedding is enabled', async () => {
mockDbStatus.isConnected = true;
const { default: TestAssignmentModal } = await import('../TestAssignmentModal');
renderWithRouter(<TestAssignmentModal test={mockTest} onClose={vi.fn()} />);

await waitFor(() => expect(mockSaveTestAssignment).toHaveBeenCalledTimes(classAStudents.length));

const savedStudentIds = mockSaveTestAssignment.mock.calls.map(([a]) => a.studentId).sort();
expect(savedStudentIds).toEqual(classAStudents.map((s) => s.id).sort());
const savedKeys = new Set(mockSaveTestAssignment.mock.calls.map(([a]) => a.teacherKey));
expect(savedKeys.size).toBe(classAStudents.length);
});

it('keeps a stable teacherKey per student when the class dropdown is switched back and forth', async () => {
mockDbStatus.isConnected = true;
const { default: TestAssignmentModal } = await import('../TestAssignmentModal');
renderWithRouter(<TestAssignmentModal test={mockTest} onClose={vi.fn()} />);

await waitFor(() => expect(mockSaveTestAssignment).toHaveBeenCalledTimes(classAStudents.length));
const firstVisitKey = mockSaveTestAssignment.mock.calls.find(([a]) => a.studentId === 's1')?.[0]?.teacherKey;
expect(firstVisitKey).toBeTruthy();

const classSelect = screen.getByLabelText('tests.assignment_class_label');
fireEvent.change(classSelect, { target: { value: 'c2' } });
await waitFor(() => expect(mockSaveTestAssignment).toHaveBeenCalledTimes(mockStudents.length));

// Class B has 1 student. The saved-progress count must read against class B alone
// (1/1), not the lifetime total across every class saved so far this session (3/1).
expect(screen.getByText('tests.assignment_saved_count:{"saved":1,"total":1}')).toBeInTheDocument();

fireEvent.change(classSelect, { target: { value: 'c1' } });
await waitFor(() => expect(screen.getByLabelText('tests.assignment_class_label')).toHaveValue('c1'));

// Revisiting class A must not re-save its students (they're already persisted),
// and the link shown must still point at the exact row that was saved the first
// time — in DB mode the share code IS the bare teacherKey (see shareCode.ts).
expect(mockSaveTestAssignment).toHaveBeenCalledTimes(mockStudents.length);
const input = screen.getByLabelText('tests.assignment_link_for:{"name":"Alice"}') as HTMLInputElement;
const code = input.value.split('#/test/')[1];
expect(code).toBe(firstVisitKey);
});
});
11 changes: 11 additions & 0 deletions src/context/AppContext.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import type {
GradingTask,
Test,
StudentTest,
TestAssignment,
UserRole,
} from '../types';
import {
Expand Down Expand Up @@ -756,6 +757,10 @@ interface AppContextValue extends StoreData {
) => Promise<Awaited<ReturnType<typeof storageSync.fetchEssayAssignmentByKey>>>;
deleteEssaySubmission: (submissionId: string, storagePath: string) => Promise<SyncResult>;
getEssaySignedUrl: (storagePath: string) => Promise<string | null>;
// Test assignments (teacher side)
saveTestAssignment: (a: TestAssignment) => Promise<SyncResult>;
fetchMyTestAssignments: () => Promise<Awaited<ReturnType<typeof storageSync.fetchMyTestAssignments>>>;
fetchAssignedTestContent: (testId: string) => Promise<Test | null>;
// Backup / restore
importBackup: (json: string) => Promise<boolean>;
// Landing / auth flow
Expand Down Expand Up @@ -1531,6 +1536,9 @@ export function AppProvider({ children }: { children: ReactNode }) {
[]
);
const getEssaySignedUrl = useCallback((path: string) => storageSync.getEssaySignedUrl(path), []);
const saveTestAssignment = useCallback((a: TestAssignment) => storageSync.saveTestAssignment(a), []);
const fetchMyTestAssignments = useCallback(() => storageSync.fetchMyTestAssignments(), []);
const fetchAssignedTestContent = useCallback((testId: string) => storageSync.fetchAssignedTestContent(testId), []);

// ─── Landing / auth flow ──────────────────────────────────────────
const enterLocalMode = useCallback(() => {
Expand Down Expand Up @@ -1732,6 +1740,9 @@ export function AppProvider({ children }: { children: ReactNode }) {
fetchEssayAssignmentByKey,
deleteEssaySubmission,
getEssaySignedUrl,
saveTestAssignment,
fetchMyTestAssignments,
fetchAssignedTestContent,
importBackup,
showLanding: landingState === 'show',
isCheckingSession: landingState === 'checking',
Expand Down
Loading
Loading