mirror of
https://github.com/itsdave-de/msp.git
synced 2026-09-28 04:02:42 -03:00
fix(msp): Robustheits-Fixes nach Browser-Testrunde (Dropzone + Wizard)
10 End-to-End-Testszenarien durchgespielt (Drag&Drop, ZIP, Multi-File,
Idempotenz, Unmatched, Error, Doc-Link, Reload-ohne-Kontext, Format-Drift,
Gutschriften). 3 echte Bugs gefunden und behoben, 2 UX-Verbesserungen
ergänzt.
Fixes:
- ZIP-Auto-Detection war zu restriktiv: sniff() hat sich auf Namens-
Heuristik verlassen (confidence 0.85 nur bei 'rechnungen'/'433148' im
Dateinamen, sonst 0.3 → unter Cutoff). Umbenannte ZIPs wurden nicht
erkannt. Fix: ZIP in-memory öffnen, erste CSV-Zeile lesen,
Signatur-Spalten matchen — gleiche Logik wie bei direkten CSVs.
Zusätzlich liest analyze_file bei ZIPs die komplette Datei (ZIP
Central Directory steht am Ende und passt oft nicht in 8 KiB).
- Unmatched-Dateien landeten im Error-State statt im Dropdown-Flow.
Backend hat error="Kein passender Handler …" gesetzt, Frontend
triggerte dadurch set_error(). Fix: analyze_file gibt für Unmatched
jetzt error=null zurück, handler_key=null — das Frontend bietet den
Dropdown mit manueller Handler-Zuordnung an (war schon vorhanden,
wurde aber nie erreicht).
- Fehler-Meldungen im Wizard-Ergebnis zeigten "[object Object]" statt
Text. Frappe-RPC-Errors liefern die Nachricht in _server_messages
als JSON-String. Fix: extract_error_message() parst das sauber aus
(mit Fallback auf exception / message / JSON-Stringify).
UX-Verbesserungen:
- Drift-Checkbox ("Abweichendes Format akzeptieren") ist jetzt immer
sichtbar, nicht nur wenn die Analyse Drift erkannt hat. Hilfreich
beim manuellen Handler-Zuordnungsflow, wenn der User eine Datei
trotz Parser-Ablehnung forcen will. Der Hinweistext passt sich dem
Kontext an.
- Wizard lädt bei fehlender Preview (z. B. Aufruf ohne Dropzone-
Voranalyse) selbst analyze_file, um Stats und Format-Drift-Status
nachzuziehen. Kein "ohne Stats"-Leerzustand mehr im Standardpfad.
Testresultate im Überblick (alle 10 ✓):
1. Drag & Drop: Karte erscheint, 100% Confidence
2. ZIP: nach Fix 100% Confidence (vorher 0)
3. Multi-File (4 parallel): alle Karten nebeneinander
4. Idempotenz: 0 Docs, 116 Skipped, Status Succeeded
5. Unmatched-Flow: Dropdown + disabled Button bis Handler gewählt
6. Broken file: ❌-Karte, keine Crashes
7. Doc-Link: öffnet SINV-271766 als Formular
8. Reload ohne Kontext: "Kein Kontext" + Link zur Dropzone
9. Format-Drift: Drift-Checkbox aktivierbar, nach Bestätigen läuft
Import durch
10. Gutschrift: 2 Credit-Note-Zeilen → negative DN (−101,95 €) als
eigenes Dokument in Ergebnistabelle
This commit is contained in:
@@ -41,8 +41,11 @@ def analyze_file(file_url: str) -> dict:
|
|||||||
if not os.path.exists(path):
|
if not os.path.exists(path):
|
||||||
frappe.throw(f"Datei nicht gefunden: {file_url}")
|
frappe.throw(f"Datei nicht gefunden: {file_url}")
|
||||||
|
|
||||||
|
# ZIPs müssen komplett gelesen werden (Central Directory steht am Ende);
|
||||||
|
# bei CSVs reichen 8 KiB für den Header.
|
||||||
|
read_full = path.lower().endswith(".zip")
|
||||||
with open(path, "rb") as f:
|
with open(path, "rb") as f:
|
||||||
sample = f.read(8192)
|
sample = f.read() if read_full else f.read(8192)
|
||||||
filename = os.path.basename(path)
|
filename = os.path.basename(path)
|
||||||
|
|
||||||
best_cls, confidence, scores = detect_best_handler(sample, filename)
|
best_cls, confidence, scores = detect_best_handler(sample, filename)
|
||||||
@@ -59,7 +62,8 @@ def analyze_file(file_url: str) -> dict:
|
|||||||
"error": None,
|
"error": None,
|
||||||
}
|
}
|
||||||
if best_cls is None:
|
if best_cls is None:
|
||||||
result["error"] = "Kein passender Handler — Format nicht erkannt."
|
# Kein Fehler — sondern „Unmatched": Frontend soll eine manuelle
|
||||||
|
# Handler-Zuordnung über das Dropdown anbieten. error bleibt null.
|
||||||
return result
|
return result
|
||||||
|
|
||||||
result["handler_key"] = best_cls.handler_key
|
result["handler_key"] = best_cls.handler_key
|
||||||
|
|||||||
@@ -44,13 +44,17 @@ class ADNMonthlyCSVHandler(BaseFileHandler):
|
|||||||
def sniff(cls, sample_bytes: bytes, filename: str) -> float:
|
def sniff(cls, sample_bytes: bytes, filename: str) -> float:
|
||||||
name = (filename or "").lower()
|
name = (filename or "").lower()
|
||||||
|
|
||||||
# ZIP-Dateien können wir nur nach Extension & Namensheuristik erkennen —
|
# ZIP: Inhalt auspacken und die erste CSV-Kopfzeile prüfen. So funktioniert
|
||||||
# der tatsächliche Header kommt erst nach dem Entpacken. Hohe Confidence
|
# die Erkennung auch bei umbenannten ZIPs.
|
||||||
# bei typischen ADN-Dateinamen, moderate sonst.
|
|
||||||
if name.endswith(".zip"):
|
if name.endswith(".zip"):
|
||||||
if "rechnungen" in name or "433148" in name:
|
first_line = cls._peek_first_line_in_zip(sample_bytes)
|
||||||
return 0.85
|
if first_line is None:
|
||||||
return 0.3 # irgendein ZIP — lieber unbestimmt
|
# Kein CSV im ZIP zu sehen (vielleicht verschlüsselt oder zu groß
|
||||||
|
# im Sample) — greifen zurück auf Namens-Heuristik.
|
||||||
|
if "rechnungen" in name or "433148" in name:
|
||||||
|
return 0.65
|
||||||
|
return 0.0
|
||||||
|
return cls._score_first_line(first_line)
|
||||||
|
|
||||||
if not name.endswith(".csv"):
|
if not name.endswith(".csv"):
|
||||||
return 0.0
|
return 0.0
|
||||||
@@ -61,15 +65,42 @@ class ADNMonthlyCSVHandler(BaseFileHandler):
|
|||||||
except Exception:
|
except Exception:
|
||||||
return 0.0
|
return 0.0
|
||||||
|
|
||||||
first_line = head.split("\n", 1)[0].lstrip("\ufeff").strip()
|
first_line = head.split("\n", 1)[0]
|
||||||
|
return cls._score_first_line(first_line)
|
||||||
|
|
||||||
|
@classmethod
|
||||||
|
def _score_first_line(cls, first_line: str) -> float:
|
||||||
|
first_line = (first_line or "").lstrip("\ufeff").strip()
|
||||||
|
if not first_line:
|
||||||
|
return 0.0
|
||||||
cols = {c.strip().upper() for c in first_line.split(";")}
|
cols = {c.strip().upper() for c in first_line.split(";")}
|
||||||
matching = cls._SIGNATURE_COLUMNS & cols
|
matching = cls._SIGNATURE_COLUMNS & cols
|
||||||
if not matching:
|
if not matching:
|
||||||
return 0.0
|
return 0.0
|
||||||
ratio = len(matching) / len(cls._SIGNATURE_COLUMNS)
|
ratio = len(matching) / len(cls._SIGNATURE_COLUMNS)
|
||||||
# ratio=1.0 ⇒ alle Signatur-Spalten drin, perfekter Match
|
|
||||||
return min(1.0, 0.4 + 0.6 * ratio)
|
return min(1.0, 0.4 + 0.6 * ratio)
|
||||||
|
|
||||||
|
@classmethod
|
||||||
|
def _peek_first_line_in_zip(cls, sample_bytes: bytes) -> str | None:
|
||||||
|
"""Öffnet das ZIP in-memory und gibt die erste Zeile der enthaltenen CSV
|
||||||
|
zurück. None, wenn keine CSV gefunden oder der Sample zu klein ist."""
|
||||||
|
import io
|
||||||
|
import zipfile
|
||||||
|
|
||||||
|
if not sample_bytes:
|
||||||
|
return None
|
||||||
|
try:
|
||||||
|
with zipfile.ZipFile(io.BytesIO(sample_bytes)) as z:
|
||||||
|
csvs = [m for m in z.namelist()
|
||||||
|
if m.lower().endswith(".csv") and not m.endswith("/")]
|
||||||
|
if not csvs:
|
||||||
|
return None
|
||||||
|
with z.open(csvs[0]) as fh:
|
||||||
|
head = fh.read(2048).decode("utf-8", errors="replace")
|
||||||
|
return head.split("\n", 1)[0]
|
||||||
|
except (zipfile.BadZipFile, EOFError, KeyError):
|
||||||
|
return None
|
||||||
|
|
||||||
# ------------------------------------------------------------------
|
# ------------------------------------------------------------------
|
||||||
# Preview
|
# Preview
|
||||||
# ------------------------------------------------------------------
|
# ------------------------------------------------------------------
|
||||||
|
|||||||
@@ -35,6 +35,26 @@ frappe.pages["import-assistant"].on_page_load = function (wrapper) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
inject_style(root);
|
inject_style(root);
|
||||||
|
|
||||||
|
// Falls keine Preview mitgereicht wurde (z. B. bei manueller Handler-Zuordnung
|
||||||
|
// in der Dropzone), hole sie jetzt nach — damit Stats angezeigt und die
|
||||||
|
// Drift-Checkbox korrekt positioniert werden kann.
|
||||||
|
if (!ctx.preview) {
|
||||||
|
root.innerHTML = `<div class="msp-assistant-empty">${__("Analysiere Datei …")}</div>`;
|
||||||
|
frappe.call({
|
||||||
|
method: "msp.importers.assistant.analyze_file",
|
||||||
|
args: { file_url: ctx.file_url },
|
||||||
|
callback: (r) => {
|
||||||
|
const m = r.message || {};
|
||||||
|
if (m.preview) ctx.preview = m.preview;
|
||||||
|
if (m.display_name) ctx.display_name = ctx.display_name || m.display_name;
|
||||||
|
if (m.preview && m.preview.format_drift) ctx.format_drift = true;
|
||||||
|
render_page(root, ctx, page);
|
||||||
|
},
|
||||||
|
});
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
render_page(root, ctx, page);
|
render_page(root, ctx, page);
|
||||||
};
|
};
|
||||||
|
|
||||||
@@ -133,13 +153,17 @@ function render_config_panel(ctx) {
|
|||||||
</select>
|
</select>
|
||||||
<div class="hint">${__("Leer = der Customer.billing_mode entscheidet pro Kunde.")}</div>
|
<div class="hint">${__("Leer = der Customer.billing_mode entscheidet pro Kunde.")}</div>
|
||||||
</div>
|
</div>
|
||||||
<div class="form-row field-drift-wrapper" ${ctx.format_drift ? "" : "hidden"}>
|
<div class="form-row field-drift-wrapper">
|
||||||
<label>
|
<label>
|
||||||
<input type="checkbox" class="field-ack-drift"
|
<input type="checkbox" class="field-ack-drift"
|
||||||
${ctx.format_drift ? "checked" : ""} />
|
${ctx.format_drift ? "checked" : ""} />
|
||||||
${__("Abweichendes Format akzeptieren")}
|
${__("Abweichendes Format akzeptieren")}
|
||||||
</label>
|
</label>
|
||||||
<div class="hint warn">${__("Die Analyse hat ein abweichendes CSV-Layout erkannt. Nur aktivieren, wenn du die Quelle kennst.")}</div>
|
<div class="hint ${ctx.format_drift ? "warn" : ""}">
|
||||||
|
${ctx.format_drift
|
||||||
|
? __("Die Analyse hat ein abweichendes CSV-Layout erkannt. Nur aktivieren, wenn du die Quelle kennst.")
|
||||||
|
: __("Aktivieren, falls der Parser die Datei ablehnt, obwohl sie inhaltlich passt.")}
|
||||||
|
</div>
|
||||||
</div>
|
</div>
|
||||||
</div>
|
</div>
|
||||||
<footer>
|
<footer>
|
||||||
@@ -205,11 +229,28 @@ async function kick_off_import(root, state, ctx) {
|
|||||||
state.result = response.message || {};
|
state.result = response.message || {};
|
||||||
show_result(root, state, ctx);
|
show_result(root, state, ctx);
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
state.result = { status: "Failed", message: (err && err.message) || String(err) };
|
state.result = { status: "Failed", message: extract_error_message(err) };
|
||||||
show_result(root, state, ctx);
|
show_result(root, state, ctx);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function extract_error_message(err) {
|
||||||
|
if (!err) return "Unbekannter Fehler";
|
||||||
|
// Frappe-RPC-Errors kommen als { exc, exception, _server_messages, ... }
|
||||||
|
if (err._server_messages) {
|
||||||
|
try {
|
||||||
|
const msgs = JSON.parse(err._server_messages);
|
||||||
|
const first = Array.isArray(msgs) ? msgs[0] : msgs;
|
||||||
|
const parsed = typeof first === "string" ? JSON.parse(first) : first;
|
||||||
|
if (parsed && parsed.message) return parsed.message;
|
||||||
|
} catch (_) { /* fallthrough */ }
|
||||||
|
}
|
||||||
|
if (typeof err === "string") return err;
|
||||||
|
if (err.exception) return String(err.exception);
|
||||||
|
if (err.message && typeof err.message === "string") return err.message;
|
||||||
|
try { return JSON.stringify(err); } catch (_) { return String(err); }
|
||||||
|
}
|
||||||
|
|
||||||
function show_result(root, state, ctx) {
|
function show_result(root, state, ctx) {
|
||||||
set_step(root, 3);
|
set_step(root, 3);
|
||||||
const r = state.result || {};
|
const r = state.result || {};
|
||||||
|
|||||||
Reference in New Issue
Block a user