fix(shopfloor): Phase D review findings — defensive cleanups + bootstrap test
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) <noreply@anthropic.com>
This commit is contained in:
@@ -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() {
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
Reference in New Issue
Block a user