feat: rebuild student submissions UI - #67
joan-code6 wants to merge 5 commits into
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
LanisProject ID: Tip Preview deployments create instant URLs for every branch and commit |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 25643a00e1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const response = await apiClient.post<SubmissionUploadResponse>( | ||
| '/meinunterricht/submissions/upload', | ||
| form, | ||
| { headers: { 'X-Session-Token': token }, signal }, |
There was a problem hiding this comment.
For every real upload, this request inherits the API client's global Content-Type: application/json; Axios serializes FormData as JSON in that case instead of allowing the browser to generate a multipart body with a boundary, so the backend receives neither uploaded file bytes nor normal multipart fields. The delete request at lines 706–708 has the same problem. Remove or override the JSON content type for both FormData requests.
Useful? React with 👍 / 👎.
|
|
||
| {uploadAllowed ? ( | ||
| <div className="mt-5 rounded-2xl border border-primary-200 bg-primary-50/60 p-4 dark:border-primary-900/70 dark:bg-primary-950/20"> | ||
| <input ref={inputRef} type="file" className="sr-only" multiple={maxFiles > 1} accept={detail.allowed_file_types.map(type => `.${type.toLowerCase().replace(/^\./, '')}`).join(',')} onChange={chooseFiles} /> |
There was a problem hiding this comment.
When the backend returns allowed_file_types: ['Alle'], allowedExtension explicitly treats that sentinel as allowing every extension, but the input converts it into accept=".alle". File pickers then filter out ordinary files—and mobile pickers may provide no override—so users cannot select a file for an unrestricted assignment. Leave accept unset when the list is empty or contains Alle.
Useful? React with 👍 / 👎.
| if (u === '/meinunterricht/submissions/upload' && method === 'post') { | ||
| return { status: 200, data: { success: true, all_succeeded: true, files: [{ name: 'demo-upload.pdf', status: 'erfolgreich', message: null }] } }; | ||
| } | ||
| if (u === '/meinunterricht/submissions/file' && method === 'delete') { | ||
| return { status: 200, data: { success: true, code: '1', message: 'File deleted successfully' } }; |
There was a problem hiding this comment.
On /demo, these handlers report successful uploads and deletions without changing any mock state. The UI immediately refreshes the detail, which rebuilds own_files solely from the unchanged summary.uploaded_count, so an uploaded file disappears and a deleted file reappears. Update the mocked submission/file state before returning success so the refreshed demo reflects the operation.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a08d3f7cff
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const maxBytes = parseMaxBytes(detail.max_file_size); | ||
| const attemptClosed = detail.allows_multiple_attempts === false && detail.own_files.length > 0; | ||
| const uploadAllowed = detail.status === 'open' && detail.can_upload && !attemptClosed; | ||
| const maxFiles = detail.allows_multiple_files === false ? 1 : 5; |
There was a problem hiding this comment.
When allows_multiple_files is true, this imposes a client-only limit of five even though the assignment data supplies only a boolean and no maximum count. Selecting six or more valid files is rejected before reaching the server; for an assignment that also disallows multiple attempts, the student cannot work around this by uploading the remainder later. Allow the browser/backend to accept the unrestricted multiple-file selection unless the API provides an actual count limit.
Useful? React with 👍 / 👎.
| {isDetail && detail ? ( | ||
| <SubmissionDetailView detail={detail} token={token} onBack={() => navigate(listPath)} onRefresh={async () => { if (id) await loadDetail(id); }} /> |
There was a problem hiding this comment.
After a detail has loaded, a failed manual refresh—or the refresh following a successful upload/delete—sets the parent error but leaves detail populated. This branch therefore continues rendering SubmissionDetailView, whose separate local error state never receives that failure, so the user sees stale files with no indication that synchronization failed. Pass the refresh error into the detail view or clear the stale detail on failure.
Useful? React with 👍 / 👎.
| <span className="mt-2 font-semibold">Dateien auswählen</span> | ||
| <span className="mt-1 text-xs text-primary-700/80 dark:text-primary-300/80">{detail.allowed_file_types.join(', ') || 'Erlaubte Dateitypen ansehen'} · maximal {maxFiles} Datei{maxFiles === 1 ? '' : 'en'}</span> | ||
| </button> | ||
| {selectedFiles.length > 0 && <div className="mt-3 space-y-2">{selectedFiles.map(file => <FileRow key={`${file.name}-${file.lastModified}`} name={file.name} meta={formatBytes(file.size)} action={<button type="button" onClick={() => setSelectedFiles(current => current.filter(item => item !== file))} className="rounded-lg p-2 text-surface-500 hover:bg-surface-200 dark:hover:bg-surface-800" aria-label={`${file.name} entfernen`}><XMarkIcon className="h-4 w-4" /></button>} />)}</div>} |
There was a problem hiding this comment.
Removing a file only updates selectedFiles; the native input still contains the same FileList. If the user removes a file and then chooses that same file again, browsers do not emit change because the input value did not change, leaving the upload button disabled and preventing the file from being re-added. Clear inputRef.current.value when the selection is removed (or before reopening the picker).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0046d16720
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| <div className="flex flex-wrap items-end justify-between gap-4"><div><p className="text-sm font-semibold uppercase tracking-[0.14em] text-primary-700 dark:text-primary-300">Mein Unterricht</p><h1 className="mt-1 text-3xl font-bold text-surface-900 dark:text-surface-100">Abgaben</h1><p className="mt-2 text-sm text-surface-600 dark:text-surface-400">Alle Upload-Aufträge, Fristen und deine abgegebenen Dateien an einem Ort.</p></div><button type="button" onClick={() => void loadList()} className="btn btn-secondary inline-flex items-center gap-2"><ArrowPathIcon className="h-4 w-4" /> Aktualisieren</button></div> | ||
| {error && <div className="flex items-start gap-2 rounded-xl border border-red-200 bg-red-50 px-4 py-3 text-sm text-red-800 dark:border-red-900/70 dark:bg-red-950/30 dark:text-red-200"><ExclamationCircleIcon className="mt-0.5 h-5 w-5 shrink-0" />{error}</div>} | ||
| {!loading && !error && <div className="grid gap-3 sm:grid-cols-3"><div className="card border-l-4 border-l-primary-500"><p className="text-xs uppercase tracking-[0.14em] text-surface-500">Aufträge</p><p className="mt-1 text-2xl font-bold text-surface-900 dark:text-surface-100">{submissions.length}</p></div><div className="card border-l-4 border-l-emerald-500"><p className="text-xs uppercase tracking-[0.14em] text-surface-500">Offen</p><p className="mt-1 text-2xl font-bold text-surface-900 dark:text-surface-100">{openCount}</p></div><div className="card border-l-4 border-l-surface-300 dark:border-l-surface-700"><p className="text-xs uppercase tracking-[0.14em] text-surface-500">Abgegeben</p><p className="mt-1 text-2xl font-bold text-surface-900 dark:text-surface-100">{submissions.filter(item => (item.uploaded_count || 0) > 0).length}</p></div></div>} | ||
| {loading ? <div className="space-y-3">{Array.from({ length: 4 }).map((_, index) => <div key={index} className="card h-36 animate-pulse bg-surface-100 dark:bg-surface-900" />)}</div> : submissions.length === 0 ? <div className="card py-14 text-center"><DocumentArrowUpIcon className="mx-auto h-12 w-12 text-surface-400" /><h2 className="mt-3 text-lg font-semibold text-surface-900 dark:text-surface-100">Keine Abgaben</h2><p className="mx-auto mt-1 max-w-md text-sm text-surface-500 dark:text-surface-400">Sobald dir ein Upload-Auftrag zugewiesen wurde, erscheint er hier.</p></div> : <div className="space-y-3">{submissions.map(submission => <SubmissionCard key={submission.id} submission={submission} onOpen={() => openSubmission(submission)} />)}</div>} |
There was a problem hiding this comment.
When the initial submissions request fails, loading becomes false while submissions remains empty, so this ternary renders “Keine Abgaben” directly below the error alert. Users are therefore told both that loading failed and that they have no assignments, even though no successful response established that; gate the empty/list content on !error or render a dedicated retry state.
Useful? React with 👍 / 👎.
| files.push(createMockSubmissionFile(summary, nextIndex, name)); | ||
| return { name, status: 'erfolgreich', message: null }; | ||
| }); | ||
| summary.uploaded_count = files.length; |
There was a problem hiding this comment.
On /demo, this updates only the summary object, while the course-history badge reads its independent uploaded_count from mockCourseDetails[*].entries[*].uploads. After uploading a file and revisiting the course, the submission detail and list show the new count but the course badge still shows the original value. Fresh evidence beyond the earlier detail-refresh issue is this separately stored course-entry count, which should be updated alongside the summary.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: afda29f5d0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!response.success) { | ||
| setError(response.message || response.error || 'Die Datei konnte nicht gelöscht werden.'); |
There was a problem hiding this comment.
When deletion is rejected—for example because the password is incorrect—this sets the page-level error while leaving deleteTarget populated, so the fixed modal remains open over the alert rendered outside it. The user receives no visible explanation for the failed action and can only retry blindly; render the error inside the dialog or close the dialog when reporting it.
Useful? React with 👍 / 👎.
| return ( | ||
| <div className="p-6"> | ||
| {isDetail && detail ? ( | ||
| <SubmissionDetailView detail={detail} refreshError={error} token={token} onBack={() => navigate(listPath)} onRefresh={async () => { if (id) await loadDetail(id); }} /> |
There was a problem hiding this comment.
This refresh callback calls loadDetail without the abort controller used by the route effect or any request-identity check. If a user refreshes assignment A and then navigates to assignment B before A finishes, the late A response can overwrite B's detail and render the wrong assignment under B's URL; an overlapping manual and post-upload refresh can similarly restore the pre-upload file list. Abort these refreshes on route changes or only apply a response when its reference still matches the current id.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d35e0a578f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| setUploadStatuses(response.files || []); | ||
| setSelectedFiles([]); | ||
| if (inputRef.current) inputRef.current.value = ''; | ||
| await onRefresh(); |
There was a problem hiding this comment.
Fresh evidence beyond the earlier stale-refresh issue is that this refresh is started only after the upload promise resolves: if a user leaves assignment A while an upload is in flight and opens assignment B, the unmounted A handler can invoke its captured onRefresh(A) after B's request has begun, increment detailRequestRef, invalidate B's response, and render A under B's URL. Guard the callback against unmount or verify the current route before refreshing; the successful deletion path has the same problem.
Useful? React with 👍 / 👎.
| <FileRow | ||
| key={file.index} | ||
| name={file.name} | ||
| meta={file.time || file.comment} |
There was a problem hiding this comment.
When an uploaded file has both its normal upload time and a comment, file.time || file.comment always selects the timestamp. Since the own-file comment is not rendered anywhere else, students cannot see that feedback; render the comment separately or combine both metadata values.
Useful? React with 👍 / 👎.
1 participant
Summary
Validation
npx tsc --noEmitpassesNo production or mobile repository changes are included.