From 7ff46af192b2276d11deeab0ad57488c4271f0c3 Mon Sep 17 00:00:00 2001 From: gsinghpal Date: Sun, 24 May 2026 13:30:29 -0400 Subject: [PATCH] =?UTF-8?q?fix(shopfloor):=20Phase=20D=20review=20findings?= =?UTF-8?q?=20=E2=80=94=20defensive=20cleanups=20+=20bootstrap=20test?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Important I1: tablet_session_manager.beginSession() now calls _removeListeners() (and clears any pending _tickHandle) defensively at start. Prevents DOM listener leak on dev hot-reload or any path that re-bootstraps without a clean endSession() first. Important I2: tablet_lock._checkIdle() early-returns in session_swap mode. The tablet_session_manager owns idle tracking there (5s poll, calls /fp/tablet/lock_session directly). Was previously dormant by accident because session_swap never populates the legacy techStore; explicit guard makes the decoupling intentional. Minor M5: session_swap unlock success now resets selectedTileUserId before window.location.reload(), matching the legacy path''s cleanup pattern. Cosmetic before reload kicks in. Minor M9: New test_tiles_bootstrap_fields with 3 HttpCase tests asserting /fp/tablet/tiles returns tablet_session_mode, kiosk_uid, and current_uid. The OWL lock screen branches on all three — a contract regression would silently break session_swap. Minor M10: Added inline comment near _sessionModeCache declaration in fp_rpc.js explaining the page-reload-invalidates-cache lifecycle. Deferred (for future polish, NOT in this commit): - I3 (_getSessionMode ACL gap for tech users — functionally correct, just suboptimal; cache fallback to ''legacy'' kicks in) - M6 (wrapper component Hand-Off buttons no-op in session_swap) - M7 (hardcoded idle/ceiling thresholds — server-configurable later) - M8 (timer divergence vs activity_tracker — unify later) Co-Authored-By: Claude Opus 4.7 (1M context) --- .../static/src/js/services/fp_rpc.js | 2 + .../src/js/services/tablet_session_manager.js | 5 ++ .../static/src/js/tablet_lock.js | 9 ++++ .../tests/__init__.py | 1 + .../tests/test_tiles_bootstrap_fields.py | 52 +++++++++++++++++++ 5 files changed, 69 insertions(+) create mode 100644 fusion_plating/fusion_plating_shopfloor/tests/test_tiles_bootstrap_fields.py diff --git a/fusion_plating/fusion_plating_shopfloor/static/src/js/services/fp_rpc.js b/fusion_plating/fusion_plating_shopfloor/static/src/js/services/fp_rpc.js index 1be24120..771fbbe2 100644 --- a/fusion_plating/fusion_plating_shopfloor/static/src/js/services/fp_rpc.js +++ b/fusion_plating/fusion_plating_shopfloor/static/src/js/services/fp_rpc.js @@ -26,6 +26,8 @@ import { rpc as baseRpc } from "@web/core/network/rpc"; +// Cached once per page load. Invalidated naturally by window.location.reload() +// after every lock/unlock (the JS bundle reinitializes, cache resets to null). let _sessionModeCache = null; // 'legacy' | 'session_swap' | null (unknown) async function _getSessionMode() { diff --git a/fusion_plating/fusion_plating_shopfloor/static/src/js/services/tablet_session_manager.js b/fusion_plating/fusion_plating_shopfloor/static/src/js/services/tablet_session_manager.js index a1aef82e..a64fd4b4 100644 --- a/fusion_plating/fusion_plating_shopfloor/static/src/js/services/tablet_session_manager.js +++ b/fusion_plating/fusion_plating_shopfloor/static/src/js/services/tablet_session_manager.js @@ -31,6 +31,11 @@ export const tabletSessionManager = { _touchHandler: null, beginSession(sessionStartedAtMs) { + // Defensive: if a prior session wasn't cleanly ended, remove its + // listeners before installing new ones. Prevents memory leaks + // during dev hot-reload / unexpected re-bootstrap paths. + this._removeListeners(); + if (this._tickHandle) clearInterval(this._tickHandle); this.sessionStartedAt = sessionStartedAtMs || Date.now(); this.lastActivity = Date.now(); this._installListeners(); diff --git a/fusion_plating/fusion_plating_shopfloor/static/src/js/tablet_lock.js b/fusion_plating/fusion_plating_shopfloor/static/src/js/tablet_lock.js index 4909bc2a..4ad1275b 100644 --- a/fusion_plating/fusion_plating_shopfloor/static/src/js/tablet_lock.js +++ b/fusion_plating/fusion_plating_shopfloor/static/src/js/tablet_lock.js @@ -130,6 +130,13 @@ export class FpTabletLock extends Component { } _checkIdle() { + // In session_swap mode, the tablet_session_manager owns the idle + // timer (it polls every 5s and calls /fp/tablet/lock_session + // directly). Skip this legacy 1s-poll path to avoid two parallel + // idle systems competing on the same tech session. + if (this.state.sessionMode === "session_swap") { + return; + } if (!this.techStore.currentTechId) { this.state.idleSecondsRemaining = null; return; @@ -167,6 +174,8 @@ export class FpTabletLock extends Component { // Cookie has swapped. Reload so OWL/services re-init // under the new (tech) session. The session manager // (Task D1) picks up on the next page load. + // Match the legacy path's cleanup before reload kicks in. + this.state.selectedTileUserId = null; window.location.reload(); // Return a pending state so the caller doesn't try to // navigate while we're tearing down. diff --git a/fusion_plating/fusion_plating_shopfloor/tests/__init__.py b/fusion_plating/fusion_plating_shopfloor/tests/__init__.py index ecf22138..cd1d0e2e 100644 --- a/fusion_plating/fusion_plating_shopfloor/tests/__init__.py +++ b/fusion_plating/fusion_plating_shopfloor/tests/__init__.py @@ -8,3 +8,4 @@ from . import test_tablet_session_event_model from . import test_tablet_pin_auth_manager from . import test_unlock_lock_session_endpoints from . import test_force_lock_cron +from . import test_tiles_bootstrap_fields diff --git a/fusion_plating/fusion_plating_shopfloor/tests/test_tiles_bootstrap_fields.py b/fusion_plating/fusion_plating_shopfloor/tests/test_tiles_bootstrap_fields.py new file mode 100644 index 00000000..ebbb950a --- /dev/null +++ b/fusion_plating/fusion_plating_shopfloor/tests/test_tiles_bootstrap_fields.py @@ -0,0 +1,52 @@ +from odoo.tests.common import HttpCase, tagged + + +@tagged('-at_install', 'post_install', 'fp_tablet') +class TestTilesBootstrapFields(HttpCase): + """Phase D added 3 bootstrap fields to /fp/tablet/tiles: + tablet_session_mode, kiosk_uid, current_uid. The OWL lock screen + branches on these, so a contract regression would silently + break the session-swap flow.""" + + def setUp(self): + super().setUp() + # Ensure kiosk password exists so we can authenticate as kiosk. + ICP = self.env['ir.config_parameter'].sudo() + ICP.set_param('fp.tablet.kiosk_password', 'tiles_test_pwd') + kiosk = self.env.ref('fusion_plating_shopfloor.user_fp_tablet_kiosk') + kiosk.sudo().password = 'tiles_test_pwd' + self.kiosk = kiosk + + def _jsonrpc(self, route, params): + import json + return self.url_open( + route, + data=json.dumps({'jsonrpc': '2.0', 'params': params}), + headers={'Content-Type': 'application/json'}, + ).json() + + def test_tiles_returns_session_mode(self): + self.authenticate('fp_tablet_kiosk@enplating.local', 'tiles_test_pwd') + resp = self._jsonrpc('/fp/tablet/tiles', {}) + result = resp.get('result', {}) + self.assertIn('tablet_session_mode', result, + 'tiles bootstrap must include tablet_session_mode for OWL branching') + # Default value is 'legacy' until rollout flips the flag + self.assertIn(result['tablet_session_mode'], ('legacy', 'session_swap')) + + def test_tiles_returns_kiosk_uid(self): + self.authenticate('fp_tablet_kiosk@enplating.local', 'tiles_test_pwd') + resp = self._jsonrpc('/fp/tablet/tiles', {}) + result = resp.get('result', {}) + self.assertIn('kiosk_uid', result, + 'tiles bootstrap must include kiosk_uid for OWL session detection') + self.assertEqual(result['kiosk_uid'], self.kiosk.id) + + def test_tiles_returns_current_uid(self): + self.authenticate('fp_tablet_kiosk@enplating.local', 'tiles_test_pwd') + resp = self._jsonrpc('/fp/tablet/tiles', {}) + result = resp.get('result', {}) + self.assertIn('current_uid', result, + 'tiles bootstrap must include current_uid so OWL can compare to kiosk_uid') + # When authenticated as kiosk, current_uid IS kiosk_uid + self.assertEqual(result['current_uid'], self.kiosk.id)