mirror of
https://github.com/maziggy/bambuddy.git
synced 2026-10-08 23:21:58 +02:00
fix(failure-reason): consistent camelCase keys across UI surfaces
The Stats page's Failure Analysis widget and the per-archive run sub-table rendered the raw PrintLogEntry.failure_reason value without translating, so the camelCase keys saved by the new Print Log row editor (#1687 part 4) surfaced as literal "filamentRunout" / "cloggedNozzle" text. The Print Log table did translate the value, so the inconsistency was visible from one surface to the next. EditArchiveModal was also still saving the localised label as the column value while the new editor saved the key - same column, two formats, two failure modes (group fragmentation on language switch, new PATCH validation rejection on round-trip). Three surfaces fixed in one drop: 1. StatsPage.tsx and PrintLogTable.tsx wrap the value in t('editArchive.failureReasons.${reason}', { defaultValue: reason }) - the defaultValue path keeps legacy translated-text rows rendering unchanged. 2. EditArchiveModal stores the camelCase key on save and reverse- looks up any legacy translated-text value against the current locale on open. Every save thereafter converts that row forward to the key format, so the column self-heals over time. 3. Added htmlFor/id to the failure-reason label/select pair (a11y plus testability).
This commit is contained in:
maziggy
@@ -20,6 +20,8 @@ All notable changes to Bambuddy will be documented in this file.
|
||||
- **Print Log page: per-row delete (#1687 part 1, reported by @IndividualGhost1905)** — Reporter noted that the existing "Also remove this print from Quick Stats" toggle on archive delete is one-shot: if you tick "keep stats" at delete time, there was no later way to drop the row from /stats; and rows that aren't tied to an archive (errors, aborts, manual entries) had no delete affordance at all. **Fix:** every row in the Archives → Print Log table now has a trash icon next to the filament cell, gated on `archives:delete_own` (own rows) or `archives:delete_all` (any row), matching the archive-delete permission shape. Click → confirm modal → row is gone, and because /archives/stats aggregates over `PrintLogEntry` the filament / time / cost contribution drops out of Quick Stats in the same response cycle. The matching archive (if any) is untouched — the log row is a sibling, not a child. **Backend:** new `DELETE /print-log/{entry_id}` mirrors `delete_archive`'s ownership flow via `require_ownership_permission(ARCHIVES_DELETE_ALL, ARCHIVES_DELETE_OWN)`; owners can drop their own rows, admins can drop any row, missing IDs return 404 rather than 200-silently. **Frontend:** new `deletePrintLogEntry` API helper, per-row mutation that invalidates both `print-log` and `archives-stats` query keys so the totals re-render without a manual refresh. **i18n:** 4 new keys (`deleteEntryTitle`, `deleteEntryConfirm`, `entryDeleted`, `entryDeleteFailed`) translated across all 11 locales (de / en / es / fr / it / ja / ko / pt-BR / tr / zh-CN / zh-TW). **Tests:** 3 backend integration cases — delete drops the row from /stats while keeping the linked archive listed, missing ID returns 404, delete-one does not touch siblings (regression guard against an accidental `delete(PrintLogEntry)` without a `where`). Frontend ArchivesPage / PrintLogModal vitests stay green (31 / 31). i18n parity green (5099 leaves × 11 locales). Issue #1687 also asks for per-row tagging (already covered by `EditArchiveModal`'s tags field) and per-row filament-usage-history edits (deferred — see the issue thread for the reasoning).
|
||||
|
||||
### Fixed
|
||||
- **Stats page Failure Analysis widget rendered raw camelCase keys instead of translated reasons (#1687 follow-up, reported by @IndividualGhost1905)** — After #1687 part 4 shipped the per-row Print Log editor, the reporter classified a couple of failed runs and saw "filamentRunout" / "cloggedNozzle" (the literal camelCase keys) appear under Statistics → Failure Analysis → Top Failure Reasons, while the same rows rendered correctly as "Filament runout" / "Clogged nozzle" on the Print Log table. Surfaced an inconsistency I introduced when shipping the new editor: the new Print Log row editor saves the camelCase key (`filamentRunout`) which is what the new backend PATCH validates against, but the older `EditArchiveModal` was still saving the localised label (`"Filament runout"`) as the value — two formats landing in the same `PrintLogEntry.failure_reason` column from two different UI surfaces. The Failure Analysis widget at `frontend/src/pages/StatsPage.tsx:817` and the per-archive run history sub-table at `frontend/src/components/PrintLogTable.tsx:81` both rendered the raw column value without running it through i18n, so the new key-form values surfaced as literal keys. **Fix — three sites in one drop:** (1) `StatsPage.tsx` and (2) `PrintLogTable.tsx` now wrap the value in `t('editArchive.failureReasons.${reason}', { defaultValue: reason })` — same pattern already used at `ArchivesPage.tsx:3874` for the Print Log table. The `defaultValue` fallback keeps legacy translated-text rows rendering as-is, no regression. (3) `EditArchiveModal.tsx` now saves the camelCase key (`<option value={reasonKey}>`) instead of the localised label, matching the new editor's wire format. On modal open, a reverse-lookup against the current locale resolves any legacy translated-text value back to its key so the dropdown pre-selects the right option — every save thereafter converts that row forward to the key format, so the data set self-heals over time without a migration script. Added `htmlFor`/`id` linkage to the failure-reason `<label>`/`<select>` pair as a side benefit (lets `getByLabelText` in tests reach the control, plus a small a11y improvement). **What this also fixes invisibly:** German / Japanese / Turkish users who classified rows under one UI language and then switched languages would have seen their historical buckets fragment in the Failure Analysis widget (each translation = its own group). With keys as the storage format, language switch no longer reclassifies anything. **Tests:** 5 new vitest cases — StatsPage `translates camelCase failure-reason keys` and `renders legacy translated-text failure reasons unchanged`; EditArchiveModal `preselects the option when the stored value is already a camelCase key`, `reverse-looks-up a legacy translated value back to its key`, and `sends the camelCase key on save, not the translated label`; PrintLogModal `translates camelCase failure_reason keys`. The existing `shows failure_reason under failed runs` case (which checks legacy text path) keeps passing under the defaultValue fallback. Full vitest 58 / 58 across touched files. ESLint clean; frontend build clean (vite 9.61s); i18n parity 5118 leaves × 11 locales green (no new keys — reuses `editArchive.failureReasons.*`).
|
||||
|
||||
- **System page boot time was rendered with a doubled timezone offset (#1690 follow-up, reported by @IndividualGhost1905)** — After the original #1690 fix landed in 0.2.4.6, the reporter on UTC+3 (Turkey) confirmed uptime was correct but boot time displayed +3 hours ahead of reality. **Root cause:** `backend/app/api/routes/system.py` built `boot_time` as a NAIVE LOCAL datetime via `datetime.fromtimestamp(psutil.Process(1).create_time())` and serialised it with `.isoformat()`, which emits no timezone marker (e.g. `"2026-06-09T11:22:05"`). The frontend's `parseUTCDate()` helper at `frontend/src/utils/date.ts:206` is documented to append `'Z'` when no tz marker is present, treating the string as UTC, then `toLocaleString` converts UTC → local — applying the local offset on top of an already-local timestamp. Uptime was unaffected because it's computed entirely backend-side as `datetime.now() - boot_time`, two naive-local values whose delta is correct regardless of the missing tz info. **Fix:** make both boot_time and the uptime anchor tz-aware UTC — `datetime.fromtimestamp(ts, tz=timezone.utc)` on the main path and the `psutil.boot_time()` fallback, and `datetime.now(timezone.utc)` in the uptime subtraction. `isoformat()` then emits `"+00:00"` and the frontend's parseUTCDate uses the marker as-is. Same naive-datetime pattern surfaced in two adjacent `generated_at` fields — the storage-usage cache snapshot in `system.py` and the support bundle root in `support.py`. Neither is rendered as a wall-clock timestamp in the frontend today, but both now emit tz-aware UTC for consistency so any future surface that does render them won't recreate this bug. **Tests:** new `test_boot_time_isoformat_carries_utc_marker` regression case asserts the boot_time string ends in `+00:00` (or `Z`) — without that marker the frontend double-converts, which is exactly the reporter's symptom. Existing `test_boot_time_uses_pid1_create_time` and `test_boot_time_falls_back_to_psutil_boot_time_on_pid1_failure` still pass under the tz-aware values because `1700345600` is `2023-11-18T20:53:20+00:00` UTC, so the date-prefix assertion is unaffected. Full system API suite 21/21 green; support API 72/72 green; ruff clean.
|
||||
|
||||
- **A1 / A1 Mini internal-code map was swapped in `PRINTER_MODEL_ID_MAP` (surfaced while scoping A2L support, #1684)** — `backend/app/utils/printer_models.py` mapped `N1 → "A1"` and `N2S → "A1 Mini"`, but every other registry that names these codes — `firmware_check.py` (`N2S → "a1"`), `virtual_printer/manager.py` (both the model map and the serial-prefix map: `N2S → "03900A"` is the A1's `039` prefix, `N1 → "03000A"` is the A1 Mini's `030`), `printer_manager.py` `A1_MODELS` — consistently uses the opposite (correct) direction. Any path that resolved an A1-family printer by internal code rather than serial prefix would silently misclassify. **Fix:** swap `PRINTER_MODEL_ID_MAP` to `N1 → "A1 Mini"`, `N2S → "A1"`; the matching comment in `LINEAR_RAIL_MODELS` was also wrong and got the same swap (the frozenset's contents don't change — both codes were already in it — so this is cosmetic, but kept the file self-consistent). New regression test class `TestA1SeriesModelIds` pins both directions so a future re-flip fails loudly. Functional impact in practice is small (most A1 detection runs off the serial prefix), but the inconsistency was a footgun for any future caller that trusted `normalize_printer_model_id`. Backend printer-model suite 46 / 46 green; ruff clean.
|
||||
|
||||
@@ -210,4 +210,47 @@ describe('EditArchiveModal', () => {
|
||||
expect(nameInput).toHaveValue('New Name');
|
||||
});
|
||||
});
|
||||
|
||||
describe('failure_reason vocabulary (#1687 follow-up)', () => {
|
||||
// The Stats page's Failure Analysis widget groups by the raw column value.
|
||||
// Before this fix this modal saved the translated label, so a language
|
||||
// switch fragmented historical buckets and any round-trip through the
|
||||
// new PATCH /print-log endpoint (which validates against camelCase keys)
|
||||
// would reject the value. The dropdown now saves the key.
|
||||
|
||||
const failedArchive = { ...mockArchive, status: 'failed', failure_reason: 'filamentRunout' };
|
||||
const legacyArchive = { ...mockArchive, status: 'failed', failure_reason: 'Filament runout' };
|
||||
|
||||
it('preselects the option when the stored value is already a camelCase key', () => {
|
||||
render(<EditArchiveModal archive={failedArchive} onClose={mockOnClose} onSave={mockOnSave} />);
|
||||
const select = screen.getByLabelText(/failure reason/i) as HTMLSelectElement;
|
||||
expect(select.value).toBe('filamentRunout');
|
||||
});
|
||||
|
||||
it('reverse-looks-up a legacy translated value back to its key', () => {
|
||||
render(<EditArchiveModal archive={legacyArchive} onClose={mockOnClose} onSave={mockOnSave} />);
|
||||
const select = screen.getByLabelText(/failure reason/i) as HTMLSelectElement;
|
||||
expect(select.value).toBe('filamentRunout');
|
||||
});
|
||||
|
||||
it('sends the camelCase key on save, not the translated label', async () => {
|
||||
const user = userEvent.setup();
|
||||
let patched: { failure_reason?: string } | undefined;
|
||||
server.use(
|
||||
http.patch('/api/v1/archives/:id', async ({ request }) => {
|
||||
patched = (await request.json()) as { failure_reason?: string };
|
||||
return HttpResponse.json({ ...failedArchive, ...patched });
|
||||
}),
|
||||
);
|
||||
|
||||
render(<EditArchiveModal archive={failedArchive} onClose={mockOnClose} onSave={mockOnSave} />);
|
||||
const select = screen.getByLabelText(/failure reason/i);
|
||||
await user.selectOptions(select, 'cloggedNozzle');
|
||||
await user.click(screen.getByRole('button', { name: /save/i }));
|
||||
|
||||
await waitFor(() => {
|
||||
expect(patched?.failure_reason).toBe('cloggedNozzle');
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -97,6 +97,23 @@ describe('PrintLogModal', () => {
|
||||
});
|
||||
});
|
||||
|
||||
it('translates camelCase failure_reason keys (#1687 follow-up)', async () => {
|
||||
vi.mocked(api.getArchiveRuns).mockResolvedValue({
|
||||
total: 1,
|
||||
items: [
|
||||
{
|
||||
...sampleRuns.items[0],
|
||||
failure_reason: 'filamentRunout',
|
||||
},
|
||||
],
|
||||
});
|
||||
render(<PrintLogModal archiveId={42} archiveName="Benchy" onClose={vi.fn()} />);
|
||||
await waitFor(() => {
|
||||
expect(screen.getByText('Filament runout')).toBeInTheDocument();
|
||||
});
|
||||
expect(screen.queryByText('filamentRunout')).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('shows the empty state when there are no runs', async () => {
|
||||
vi.mocked(api.getArchiveRuns).mockResolvedValue({ total: 0, items: [] });
|
||||
render(<PrintLogModal archiveId={42} archiveName="Benchy" onClose={vi.fn()} />);
|
||||
|
||||
@@ -293,6 +293,49 @@ describe('StatsPage', () => {
|
||||
});
|
||||
});
|
||||
|
||||
it('translates camelCase failure-reason keys instead of rendering them raw (#1687 follow-up)', async () => {
|
||||
// The widget groups by the raw PrintLogEntry.failure_reason column.
|
||||
// The new editor stores camelCase keys (`filamentRunout`), so the widget
|
||||
// must translate them — otherwise users see the literal key text.
|
||||
server.use(
|
||||
http.get('/api/v1/archives/analysis/failures', () => {
|
||||
return HttpResponse.json({
|
||||
...mockFailureAnalysis,
|
||||
failures_by_reason: { filamentRunout: 2, cloggedNozzle: 1 },
|
||||
});
|
||||
}),
|
||||
);
|
||||
|
||||
render(<StatsPage />);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByText('Filament runout')).toBeInTheDocument();
|
||||
expect(screen.getByText('Clogged nozzle')).toBeInTheDocument();
|
||||
});
|
||||
expect(screen.queryByText('filamentRunout')).not.toBeInTheDocument();
|
||||
expect(screen.queryByText('cloggedNozzle')).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('renders legacy translated-text failure reasons unchanged (#1687 follow-up)', async () => {
|
||||
// Old rows from before the key/value migration stored the translated
|
||||
// text. The defaultValue fallback in the t() call must surface them
|
||||
// as-is rather than turning them into the literal key string.
|
||||
server.use(
|
||||
http.get('/api/v1/archives/analysis/failures', () => {
|
||||
return HttpResponse.json({
|
||||
...mockFailureAnalysis,
|
||||
failures_by_reason: { 'Custom legacy reason': 4 },
|
||||
});
|
||||
}),
|
||||
);
|
||||
|
||||
render(<StatsPage />);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByText('Custom legacy reason')).toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
||||
it('shows printer stats widget', async () => {
|
||||
render(<StatsPage />);
|
||||
|
||||
|
||||
@@ -51,7 +51,19 @@ export function EditArchiveModal({ archive, onClose, existingTags = [] }: EditAr
|
||||
const [projectId, setProjectId] = useState<number | null>(archive.project_id ?? null);
|
||||
const [notes, setNotes] = useState(archive.notes || '');
|
||||
const [tags, setTags] = useState(archive.tags || '');
|
||||
const [failureReason, setFailureReason] = useState(archive.failure_reason || '');
|
||||
// Failure reason is stored as a camelCase key (`filamentRunout`), but earlier
|
||||
// versions of this modal saved the translated label as the value. Reverse-
|
||||
// lookup any legacy translated text against the current locale so the
|
||||
// dropdown pre-selects the right option, then any save converts it forward.
|
||||
const [failureReason, setFailureReason] = useState(() => {
|
||||
const raw = archive.failure_reason || '';
|
||||
if (!raw) return '';
|
||||
if ((FAILURE_REASON_KEYS as readonly string[]).includes(raw)) return raw;
|
||||
const match = FAILURE_REASON_KEYS.find(
|
||||
(k) => t(`editArchive.failureReasons.${k}`) === raw,
|
||||
);
|
||||
return match || '';
|
||||
});
|
||||
const [status, setStatus] = useState(archive.status);
|
||||
const [quantity, setQuantity] = useState(archive.quantity ?? 1);
|
||||
const [photos, setPhotos] = useState<string[]>(archive.photos || []);
|
||||
@@ -417,15 +429,16 @@ export function EditArchiveModal({ archive, onClose, existingTags = [] }: EditAr
|
||||
{/* Failure Reason - only show for failed/aborted prints */}
|
||||
{(status === 'failed' || status === 'aborted') && (
|
||||
<div>
|
||||
<label className="block text-sm text-bambu-gray mb-1">{t('editArchive.failureReason')}</label>
|
||||
<label htmlFor="failure-reason-select" className="block text-sm text-bambu-gray mb-1">{t('editArchive.failureReason')}</label>
|
||||
<select
|
||||
id="failure-reason-select"
|
||||
value={failureReason}
|
||||
onChange={(e) => setFailureReason(e.target.value)}
|
||||
className="w-full px-3 py-2 bg-bambu-dark border border-bambu-dark-tertiary rounded-lg text-white focus:border-bambu-green focus:outline-none"
|
||||
>
|
||||
<option value="">{t('editArchive.selectReason')}</option>
|
||||
{FAILURE_REASON_KEYS.map((reasonKey) => (
|
||||
<option key={reasonKey} value={t(`editArchive.failureReasons.${reasonKey}`)}>
|
||||
<option key={reasonKey} value={reasonKey}>
|
||||
{t(`editArchive.failureReasons.${reasonKey}`)}
|
||||
</option>
|
||||
))}
|
||||
|
||||
@@ -78,7 +78,7 @@ export function PrintLogTable({ archiveId }: PrintLogTableProps) {
|
||||
{t(`archives.runLog.status.${run.status}`, { defaultValue: run.status })}
|
||||
{run.failure_reason && (
|
||||
<span className="block text-[10px] text-bambu-gray font-normal">
|
||||
{run.failure_reason}
|
||||
{t(`editArchive.failureReasons.${run.failure_reason}`, { defaultValue: run.failure_reason })}
|
||||
</span>
|
||||
)}
|
||||
</td>
|
||||
|
||||
@@ -814,7 +814,9 @@ function FailureAnalysisWidget({ size = 1, dateFrom, dateTo, createdById }: {
|
||||
{topReasons.map(([reason, count]) => (
|
||||
<div key={reason} className="flex items-center justify-between text-sm">
|
||||
<span className={`text-white truncate ${size === 4 ? 'max-w-[200px]' : 'max-w-[160px]'}`}>
|
||||
{reason || t('common.unknown')}
|
||||
{reason
|
||||
? t(`editArchive.failureReasons.${reason}`, { defaultValue: reason })
|
||||
: t('common.unknown')}
|
||||
</span>
|
||||
<span className="text-bambu-gray ml-2">{count}</span>
|
||||
</div>
|
||||
|
||||
File diff suppressed because one or more lines are too long
+1
-1
@@ -26,7 +26,7 @@
|
||||
|
||||
<!-- Splash screens for iOS -->
|
||||
<link rel="apple-touch-startup-image" href="/img/android-chrome-512x512.png" />
|
||||
<script type="module" crossorigin src="/assets/index-QVjYxA_R.js"></script>
|
||||
<script type="module" crossorigin src="/assets/index-DdAEkh5e.js"></script>
|
||||
<link rel="stylesheet" crossorigin href="/assets/index-7s3X35pi.css">
|
||||
</head>
|
||||
<body>
|
||||
|
||||
Reference in New Issue
Block a user