From ef31852fd8f2c6e65186f8fc8175c5c6e19a19ec Mon Sep 17 00:00:00 2001 From: bonamin Date: Sun, 22 Feb 2026 15:32:45 +0200 Subject: [PATCH] Further improvements --- backend/builder/router.py | 2 +- backend/builder/service.py | 15 ++++ frontend/src/index.css | 8 +- frontend/src/melodies/MelodyDetail.jsx | 45 ++++++++--- .../melodies/builder/BuildOnTheFlyModal.jsx | 57 ++++++++++++- frontend/src/melodies/builder/BuilderForm.jsx | 80 ++++++++++++++++--- 6 files changed, 179 insertions(+), 28 deletions(-) diff --git a/backend/builder/router.py b/backend/builder/router.py index 526371e..b6d6596 100644 --- a/backend/builder/router.py +++ b/backend/builder/router.py @@ -95,7 +95,7 @@ async def download_binary( raise HTTPException(status_code=404, detail="Binary not built yet for this melody") melody = await service.get_built_melody(melody_id) - filename = f"{melody.name}.bsm" + filename = f"{melody.pid or melody.name}.bsm" return FileResponse( path=str(path), diff --git a/backend/builder/service.py b/backend/builder/service.py index 9fc128b..7d91f9f 100644 --- a/backend/builder/service.py +++ b/backend/builder/service.py @@ -130,7 +130,20 @@ async def get_built_melody(melody_id: str) -> BuiltMelodyInDB: return _row_to_built_melody(row) +async def _check_unique(name: str, pid: str, exclude_id: Optional[str] = None) -> None: + """Raise 409 if name or PID is already taken by another archetype.""" + rows = await db.list_built_melodies() + for row in rows: + if exclude_id and row["id"] == exclude_id: + continue + if row["name"].lower() == name.lower(): + raise HTTPException(status_code=409, detail=f"An archetype with the name '{name}' already exists.") + if pid and row.get("pid") and row["pid"].lower() == pid.lower(): + raise HTTPException(status_code=409, detail=f"An archetype with the PID '{pid}' already exists.") + + async def create_built_melody(data: BuiltMelodyCreate) -> BuiltMelodyInDB: + await _check_unique(data.name, data.pid or "") melody_id = str(uuid.uuid4()) await db.insert_built_melody( melody_id=melody_id, @@ -150,6 +163,8 @@ async def update_built_melody(melody_id: str, data: BuiltMelodyUpdate) -> BuiltM new_pid = data.pid if data.pid is not None else row["pid"] new_steps = data.steps if data.steps is not None else row["steps"] + await _check_unique(new_name, new_pid or "", exclude_id=melody_id) + await db.update_built_melody(melody_id, name=new_name, pid=new_pid, steps=new_steps) return await get_built_melody(melody_id) diff --git a/frontend/src/index.css b/frontend/src/index.css index 92682b4..ae8f922 100644 --- a/frontend/src/index.css +++ b/frontend/src/index.css @@ -93,13 +93,17 @@ select { background-color: var(--bg-input) !important; border-color: var(--border-input) !important; color: var(--text-primary) !important; + -webkit-appearance: none !important; + appearance: none !important; +} + +/* Only select elements get the dropdown arrow */ +select { padding-right: 2rem !important; background-image: url("data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' width='12' height='12' viewBox='0 0 24 24' fill='none' stroke='%239ca3af' stroke-width='2.5' stroke-linecap='round' stroke-linejoin='round'%3E%3Cpath d='M6 9l6 6 6-6'/%3E%3C/svg%3E") !important; background-repeat: no-repeat !important; background-position: right 0.6rem center !important; background-size: 12px !important; - -webkit-appearance: none !important; - appearance: none !important; } input::placeholder, textarea::placeholder { diff --git a/frontend/src/melodies/MelodyDetail.jsx b/frontend/src/melodies/MelodyDetail.jsx index 46c89fa..22fcca6 100644 --- a/frontend/src/melodies/MelodyDetail.jsx +++ b/frontend/src/melodies/MelodyDetail.jsx @@ -383,17 +383,40 @@ export default function MelodyDetail() {

Files

- {files.binary_url ? ( - - Download binary - - ) : ( + {files.binary_url ? (() => { + const binaryPid = builtMelody?.pid || melody.pid || "binary"; + const binaryFilename = `${binaryPid}.bsm`; + const handleDownload = async (e) => { + e.preventDefault(); + try { + const token = localStorage.getItem("access_token"); + const res = await fetch(files.binary_url, { + headers: token ? { Authorization: `Bearer ${token}` } : {}, + }); + if (!res.ok) throw new Error(`Download failed: ${res.statusText}`); + const blob = await res.blob(); + const url = URL.createObjectURL(blob); + const a = document.createElement("a"); + a.href = url; + a.download = binaryFilename; + a.click(); + URL.revokeObjectURL(url); + } catch (err) { + // surface error in page error state if possible + console.error(err); + } + }; + return ( + + {binaryFilename} + + ); + })() : ( Not uploaded )} diff --git a/frontend/src/melodies/builder/BuildOnTheFlyModal.jsx b/frontend/src/melodies/builder/BuildOnTheFlyModal.jsx index da5a924..2b2b51f 100644 --- a/frontend/src/melodies/builder/BuildOnTheFlyModal.jsx +++ b/frontend/src/melodies/builder/BuildOnTheFlyModal.jsx @@ -23,6 +23,29 @@ function computeStepsAndNotes(stepsStr) { return { steps: tokens.length, totalNotes: bellSet.size || 1 }; } +/** + * Validate steps string. Each comma-separated token must be: + * - "0" (silence) + * - a number 1-16 + * - multiple of the above joined by "+" + * Returns null if valid, or an error string if invalid. + */ +function validateSteps(stepsStr) { + if (!stepsStr || !stepsStr.trim()) return "Steps are required."; + const tokens = stepsStr.trim().split(","); + for (const token of tokens) { + const parts = token.split("+"); + for (const part of parts) { + const trimmed = part.trim(); + if (trimmed === "") return `Invalid token near: "${token.trim()}" — empty part found.`; + const n = parseInt(trimmed, 10); + if (isNaN(n)) return `Invalid step value: "${trimmed}" — must be a number 0–16.`; + if (n < 0 || n > 16) return `Step value "${trimmed}" is out of range (must be 0–16).`; + } + } + return null; +} + export default function BuildOnTheFlyModal({ open, melodyId, currentMelody, defaultName, defaultPid, onClose, onSuccess }) { const [name, setName] = useState(defaultName || ""); const [pid, setPid] = useState(defaultPid || ""); @@ -34,14 +57,39 @@ export default function BuildOnTheFlyModal({ open, melodyId, currentMelody, defa if (!open) return null; const handleBuildAndUpload = async () => { + // Mandatory field checks if (!name.trim()) { setError("Name is required."); return; } + if (!pid.trim()) { setError("PID is required."); return; } if (!steps.trim()) { setError("Steps are required."); return; } + + // Steps format validation + const stepsError = validateSteps(steps); + if (stepsError) { setError(stepsError); return; } + setBuilding(true); setError(""); setStatusMsg(""); - let builtId = null; try { + // Uniqueness check: fetch all existing archetypes + setStatusMsg("Checking for conflicts..."); + const existing = await api.get("/builder/melodies"); + const list = existing.melodies || []; + const dupName = list.find((m) => m.name.toLowerCase() === name.trim().toLowerCase()); + if (dupName) { + setError(`An archetype with the name "${name.trim()}" already exists.`); + setBuilding(false); + setStatusMsg(""); + return; + } + const dupPid = list.find((m) => m.pid && m.pid.toLowerCase() === pid.trim().toLowerCase()); + if (dupPid) { + setError(`An archetype with the PID "${pid.trim()}" already exists.`); + setBuilding(false); + setStatusMsg(""); + return; + } + // Step 1: Create the built melody record setStatusMsg("Creating melody record..."); const created = await api.post("/builder/melodies", { @@ -49,7 +97,7 @@ export default function BuildOnTheFlyModal({ open, melodyId, currentMelody, defa pid: pid.trim(), steps: steps.trim(), }); - builtId = created.id; + const builtId = created.id; // Step 2: Build the binary setStatusMsg("Building binary..."); @@ -63,7 +111,8 @@ export default function BuildOnTheFlyModal({ open, melodyId, currentMelody, defa }); if (!res.ok) throw new Error(`Failed to fetch binary: ${res.statusText}`); const blob = await res.blob(); - const file = new File([blob], `${name.trim()}.bsm`, { type: "application/octet-stream" }); + // File is named by PID, not friendly name + const file = new File([blob], `${pid.trim()}.bsm`, { type: "application/octet-stream" }); await api.upload(`/melodies/${melodyId}/upload/binary`, file); // Step 4: Assign to this melody @@ -134,7 +183,7 @@ export default function BuildOnTheFlyModal({ open, melodyId, currentMelody, defa setName(e.target.value)} placeholder="e.g. Doksologia_3k" className={inputClass} disabled={building} />
- + setPid(e.target.value)} placeholder="e.g. builtin_doksologia_3k" className={inputClass} disabled={building} />
diff --git a/frontend/src/melodies/builder/BuilderForm.jsx b/frontend/src/melodies/builder/BuilderForm.jsx index 973f63e..9384e01 100644 --- a/frontend/src/melodies/builder/BuilderForm.jsx +++ b/frontend/src/melodies/builder/BuilderForm.jsx @@ -30,6 +30,29 @@ function countSteps(stepsStr) { return stepsStr.trim().split(",").length; } +/** + * Validate steps string. Each comma-separated token must be: + * - "0" (silence) + * - a number 1-16 + * - multiple of the above joined by "+" + * Returns null if valid, or an error string if invalid. + */ +function validateSteps(stepsStr) { + if (!stepsStr || !stepsStr.trim()) return "Steps are required."; + const tokens = stepsStr.trim().split(","); + for (const token of tokens) { + const parts = token.split("+"); + for (const part of parts) { + const trimmed = part.trim(); + if (trimmed === "") return `Invalid token near: "${token.trim()}" — empty part found.`; + const n = parseInt(trimmed, 10); + if (isNaN(n)) return `Invalid step value: "${trimmed}" — must be a number 0–16.`; + if (n < 0 || n > 16) return `Step value "${trimmed}" is out of range (must be 0–16).`; + } + } + return null; +} + async function downloadBinary(binaryUrl, filename) { const token = localStorage.getItem("access_token"); const res = await fetch(`/api${binaryUrl}`, { @@ -50,10 +73,14 @@ export default function BuilderForm() { const isEdit = Boolean(id); const navigate = useNavigate(); + // Form state (what the user is editing) const [name, setName] = useState(""); const [pid, setPid] = useState(""); const [steps, setSteps] = useState(""); + // Saved state (what's actually stored — build actions use this) + const [savedPid, setSavedPid] = useState(""); + const [loading, setLoading] = useState(false); const [saving, setSaving] = useState(false); const [buildingBinary, setBuildingBinary] = useState(false); @@ -66,6 +93,9 @@ export default function BuilderForm() { const [progmemCode, setProgmemCode] = useState(""); const [copied, setCopied] = useState(false); + // Track whether the form has unsaved changes relative to what's built + const [hasUnsavedChanges, setHasUnsavedChanges] = useState(false); + const codeRef = useRef(null); useEffect(() => { @@ -79,9 +109,11 @@ export default function BuilderForm() { setName(data.name || ""); setPid(data.pid || ""); setSteps(data.steps || ""); + setSavedPid(data.pid || ""); setBinaryBuilt(Boolean(data.binary_path)); setBinaryUrl(data.binary_url || null); setProgmemCode(data.progmem_code || ""); + setHasUnsavedChanges(false); } catch (err) { setError(err.message); } finally { @@ -89,16 +121,37 @@ export default function BuilderForm() { } }; + // Track form dirtiness whenever name/pid/steps change after initial load + const handleNameChange = (v) => { setName(v); setHasUnsavedChanges(true); }; + const handlePidChange = (v) => { setPid(v); setHasUnsavedChanges(true); }; + const handleStepsChange = (v) => { setSteps(v); setHasUnsavedChanges(true); }; + const handleSave = async () => { if (!name.trim()) { setError("Name is required."); return; } - if (!steps.trim()) { setError("Steps are required."); return; } + if (!pid.trim()) { setError("PID is required."); return; } + + const stepsError = validateSteps(steps); + if (stepsError) { setError(stepsError); return; } + setSaving(true); setError(""); setSuccessMsg(""); try { + // Uniqueness check + const existing = await api.get("/builder/melodies"); + const list = existing.melodies || []; + + const dupName = list.find((m) => m.id !== id && m.name.toLowerCase() === name.trim().toLowerCase()); + if (dupName) { setError(`An archetype with the name "${name.trim()}" already exists.`); return; } + + const dupPid = list.find((m) => m.id !== id && m.pid && m.pid.toLowerCase() === pid.trim().toLowerCase()); + if (dupPid) { setError(`An archetype with the PID "${pid.trim()}" already exists.`); return; } + const body = { name: name.trim(), pid: pid.trim(), steps: steps.trim() }; if (isEdit) { await api.put(`/builder/melodies/${id}`, body); + setSavedPid(pid.trim()); + setHasUnsavedChanges(false); setSuccessMsg("Saved."); } else { const created = await api.post("/builder/melodies", body); @@ -113,6 +166,7 @@ export default function BuilderForm() { const handleBuildBinary = async () => { if (!isEdit) { setError("Save the melody first before building."); return; } + if (hasUnsavedChanges) { setError("You have unsaved changes. Save before rebuilding."); return; } setBuildingBinary(true); setError(""); setSuccessMsg(""); @@ -130,6 +184,7 @@ export default function BuilderForm() { const handleBuildBuiltin = async () => { if (!isEdit) { setError("Save the melody first before building."); return; } + if (hasUnsavedChanges) { setError("You have unsaved changes. Save before regenerating."); return; } setBuildingBuiltin(true); setError(""); setSuccessMsg(""); @@ -201,12 +256,12 @@ export default function BuilderForm() {
- setName(e.target.value)} placeholder="e.g. Doksologia_3k" className={inputClass} /> + handleNameChange(e.target.value)} placeholder="e.g. Doksologia_3k" className={inputClass} />
- - setPid(e.target.value)} placeholder="e.g. builtin_doksologia_3k" className={inputClass} /> -

Used as the built-in firmware identifier

+ + handlePidChange(e.target.value)} placeholder="e.g. builtin_doksologia_3k" className={inputClass} /> +

Used as the built-in firmware identifier. Must be unique.

@@ -215,7 +270,7 @@ export default function BuilderForm() {