From 15047ffbaa13db223d69c71d82e40bc54aaba106 Mon Sep 17 00:00:00 2001 From: Ali <010109ali@gmail.com> Date: Mon, 15 Jun 2026 13:19:16 +0000 Subject: [PATCH] fix(amas): single continuous progress bar across both import phases The bar refilled 0->100% once per phase: the parallel fetch phase (the slow, network-bound one) reported 0..N, then the sequential save phase reported 0..N again. The client's monotonic guard then pinned the bar at 100% for the whole save phase, so real per-employee progress (with names) was only visible at the very end, and the bar looked stuck at 0% during the long fetch. Report one overall 0-100 percent across both phases (fetch = 0-50, save = 50-100) from the worker and drive the bar from it; show per-phase "N of M" + a clear phase label ("Fetching details" / "Saving employees") in the detail line. Poll every 1s for snappier updates. Bar now advances continuously and is meaningful from the first tick. Co-Authored-By: Claude Opus 4.8 (1M context) --- invoice_az/amas_api.py | 46 ++++++++++++++++++----- invoice_az/client/employee.js | 69 +++++++++++++++++++++-------------- 2 files changed, 77 insertions(+), 38 deletions(-) diff --git a/invoice_az/amas_api.py b/invoice_az/amas_api.py index 970bedb..967fc4e 100644 --- a/invoice_az/amas_api.py +++ b/invoice_az/amas_api.py @@ -2761,8 +2761,9 @@ def import_bulk_employees(asan_login_name, employees_data, company, create_desig frappe.db.commit() # Seed progress immediately so a bar that attaches before the worker emits - # its first tick already shows 0 / total instead of an empty "reconnecting". - _set_import_progress(asan_login_name, 0, len(employees_data), phase="queued") + # its first tick already shows the total and a "preparing" state instead of + # an empty "reconnecting". + _set_import_progress(asan_login_name, 0, len(employees_data), phase="queued", percent=0) user = frappe.session.user @@ -2799,6 +2800,7 @@ def get_amas_import_status(asan_login_name): "total": progress.get("total") or 0, "phase": progress.get("phase"), "employee_name": progress.get("employee_name"), + "percent": progress.get("percent") or 0, } @@ -2875,10 +2877,16 @@ def _import_progress_key(asan_login_name): return f"amas_import_progress:{asan_login_name}" -def _set_import_progress(asan_login_name, current, total, phase=None, employee_name=None): +def _set_import_progress(asan_login_name, current, total, phase=None, employee_name=None, percent=None): """Persist the latest progress tick to Redis so a reconnecting list-view bar can poll it (see get_amas_import_status). Best-effort: progress reporting - must never break the import itself.""" + must never break the import itself. + + ``percent`` is the overall 0-100 progress across BOTH import phases (fetch + then save). It drives the bar width directly so the bar advances + continuously instead of refilling 0->100 once per phase. ``current``/``total`` + stay per-phase, for the "N of M" detail text only. + """ try: frappe.cache().set_value( _import_progress_key(asan_login_name), @@ -2887,6 +2895,7 @@ def _set_import_progress(asan_login_name, current, total, phase=None, employee_n "total": total, "phase": phase, "employee_name": employee_name, + "percent": percent, }, expires_in_sec=3600, ) @@ -2901,6 +2910,20 @@ def _clear_import_progress(asan_login_name): pass +def _overall_percent(phase, current, total): + """Collapse the two import phases into one continuous 0-100 bar: + fetch = first half (0-50), save = second half (50-100). 'queued' sits at 0. + Keeps the bar moving forward instead of refilling once per phase.""" + if not total or total <= 0: + return 0 + frac = max(0.0, min(1.0, current / total)) + if phase == "fetch": + return round(frac * 50.0, 1) + if phase == "save": + return round(50.0 + frac * 50.0, 1) + return 0 + + def _process_bulk_employees_import(asan_login_name, employees, company, create_designation, user): """Background job: import employees with realtime progress. @@ -2997,10 +3020,11 @@ def _process_bulk_employees_import(asan_login_name, employees, company, create_d or outcome["emp_data"].get("identification_number") or "Employee" ) - _set_import_progress(asan_login_name, completed, total, "fetch", emp_name) + pct = _overall_percent("fetch", completed, total) + _set_import_progress(asan_login_name, completed, total, "fetch", emp_name, pct) frappe.publish_realtime( "amas_import_progress", - {"current": completed, "total": total, "employee_name": emp_name, "phase": "fetch"}, + {"current": completed, "total": total, "employee_name": emp_name, "phase": "fetch", "percent": pct}, user=user, ) @@ -3060,10 +3084,11 @@ def _process_bulk_employees_import(asan_login_name, employees, company, create_d if outcome["error"]: errors.append({"employee": emp_name, "error": outcome["error"]}) - _set_import_progress(asan_login_name, idx + 1, total, "save", emp_name) + pct = _overall_percent("save", idx + 1, total) + _set_import_progress(asan_login_name, idx + 1, total, "save", emp_name, pct) frappe.publish_realtime( "amas_import_progress", - {"current": idx + 1, "total": total, "employee_name": emp_name, "phase": "save"}, + {"current": idx + 1, "total": total, "employee_name": emp_name, "phase": "save", "percent": pct}, user=user, ) continue @@ -3087,10 +3112,11 @@ def _process_bulk_employees_import(asan_login_name, employees, company, create_d except Exception as e: errors.append({"employee": emp_name, "error": str(e)}) - _set_import_progress(asan_login_name, idx + 1, total, "save", emp_name) + pct = _overall_percent("save", idx + 1, total) + _set_import_progress(asan_login_name, idx + 1, total, "save", emp_name, pct) frappe.publish_realtime( "amas_import_progress", - {"current": idx + 1, "total": total, "employee_name": emp_name, "phase": "save"}, + {"current": idx + 1, "total": total, "employee_name": emp_name, "phase": "save", "percent": pct}, user=user, ) diff --git a/invoice_az/client/employee.js b/invoice_az/client/employee.js index 37b6de6..5216bb8 100644 --- a/invoice_az/client/employee.js +++ b/invoice_az/client/employee.js @@ -714,42 +714,55 @@ function attach_amas_import_listeners(listview, asan_login_name, total_hint) { // fire before the worker has even started (the running flag is set // server-side a beat after import_bulk_employees is called). let sawRunning = (total > 0); - // Highest "current" rendered so far. Realtime and polling both feed the bar; - // this keeps it monotonic so a stale poll can't rewind a fresher realtime - // tick (or vice-versa). - let lastCurrent = 0; - let knownTotal = total || 0; + // Highest overall percent (0-100) rendered so far. Realtime and polling both + // feed the bar; this keeps it monotonic so a stale poll can't rewind a + // fresher realtime tick (or vice-versa). + let lastPercent = 0; let dialog = frappe.show_progress( - __('Importing from ƏMAS'), 0, total, + __('Importing from ƏMAS'), 0, 100, total ? __('Starting import of {0} employees...', [total]) - : __('Importing employees, please wait...') + : __('Preparing import...') ); ensure_cancel_button(dialog); - // Single entry point for moving the bar, fed by both the realtime handler - // and the status poll. Never regresses; tolerates an unknown total (0). - const renderProgress = function(current, totalArg, phase, employee_name) { - if (importFinished) return; - if (totalArg && totalArg > 0) knownTotal = totalArg; - if (typeof current === 'number' && current > lastCurrent) lastCurrent = current; + const phaseLabel = function(phase) { + if (phase === 'save') return __('Saving employees'); + if (phase === 'fetch') return __('Fetching details'); + return __('Preparing import...'); + }; - const is_save = phase === 'save'; - const verb = is_save ? __('Saving: ') : __('Fetching: '); + // The bar tracks ONE overall percent across both import phases (fetch = + // 0-50, save = 50-100), computed server-side. Fall back to deriving it the + // same way if an older worker sends no `percent`. + const overallPercent = function(percent, current, total, phase) { + if (typeof percent === 'number' && percent >= 0) return percent; + if (!(total > 0)) return 0; + const frac = Math.max(0, Math.min(1, current / total)); + if (phase === 'save') return 50 + frac * 50; + if (phase === 'fetch') return frac * 50; + return 0; + }; + + // Single entry point for moving the bar, fed by both the realtime handler + // and the status poll. Never regresses. + const renderProgress = function(percent, current, total, phase, employee_name) { + if (importFinished) return; + const pct = overallPercent(percent, current, total, phase); + if (pct > lastPercent) lastPercent = pct; + + const label = phaseLabel(phase); let detail; - if (employee_name) { - detail = verb + employee_name; - } else if (knownTotal > 0) { - detail = verb + __('{0} of {1}', [lastCurrent, knownTotal]); + if (phase === 'queued' || !(total > 0)) { + detail = label; + } else if (employee_name) { + detail = label + ': ' + employee_name + ' (' + current + '/' + total + ')'; } else { - // Running, but no count yet (first tick not in, or server hasn't - // populated progress). Keep it reassuring rather than alarming. - detail = __('Importing employees, please wait...'); + detail = label + ': ' + __('{0} of {1}', [current, total]); } - // Guard against total=0 (NaN width); show_progress reuses the dialog. const d = frappe.show_progress( - __('Importing from ƏMAS'), lastCurrent, knownTotal > 0 ? knownTotal : 1, detail + __('Importing from ƏMAS'), Math.round(lastPercent), 100, detail ); ensure_cancel_button(d); }; @@ -758,7 +771,7 @@ function attach_amas_import_listeners(listview, asan_login_name, total_hint) { frappe.realtime.on('amas_import_progress', function(data) { if (importFinished) return; sawRunning = true; - renderProgress(data.current, data.total, data.phase, data.employee_name); + renderProgress(data.percent, data.current, data.total, data.phase, data.employee_name); }); // Polling fallback. Realtime events are transient — anything published @@ -787,7 +800,7 @@ function attach_amas_import_listeners(listview, asan_login_name, total_hint) { const m = r.message || {}; if (m.running) { sawRunning = true; - renderProgress(m.current, m.total, m.phase, m.employee_name); + renderProgress(m.percent, m.current, m.total, m.phase, m.employee_name); } else if (sawRunning) { // We watched it run and now it's gone — the realtime // 'complete' was missed. Wrap up gracefully. @@ -798,10 +811,10 @@ function attach_amas_import_listeners(listview, asan_login_name, total_hint) { }; if (amasPollTimer) clearInterval(amasPollTimer); - amasPollTimer = setInterval(pollStatus, 2000); + amasPollTimer = setInterval(pollStatus, 1000); // Reconnect path (no total hint): the caller already confirmed an import is // running, so seed the bar from real numbers immediately instead of waiting - // a full poll interval on the bare "reconnecting" text. + // a full poll interval on the bare "preparing" text. if (!total) pollStatus(); frappe.realtime.off('amas_import_complete');