From 03bf24e8d4b70cb779713f21ea97aa4d35fbc9b4 Mon Sep 17 00:00:00 2001 From: Brian Krabach Date: Sun, 17 May 2026 10:10:05 -0700 Subject: [PATCH] refactor(views): introduce operation layer with pure and user-intent ops (Phase 2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Backend (muxplex/views.py): - Added five pure data operations that mutate settings dict in place: add_membership, remove_membership, remove_from_all_views, hide, unhide. Each operation is isolated and affects only its named field. - Added 16 new tests under 'Pure data operations (Phase 2)' in test_views.py. Isolation verified: each operation only modifies its target field. Frontend (muxplex/frontend/app.js): - Added five pure ops (_opAddMembership, _opRemoveMembership, _opRemoveFromAllViews, _opHide, _opUnhide) and _cloneOpState helper. - Added four user-intent operations with intentional asymmetry: * hideSessionOp = hide + removeFromAllViews (federation-safe; matches current UX; see design doc for rationale) * unhideSessionOp = unhide (orthogonal; does NOT change view membership) * addSessionToViewOp = unhide + addMembership (auto-unhide on add; expressed as explicit composition) * removeSessionFromViewOp = removeMembership (orthogonal; does NOT hide) - Refactored 7 call sites to use user-intent ops: _doHideSession, _doUnhideSession, _doRemoveFromView, flyout submenu toggle, 'New View' flow, mobile picker toggle, Manage View checkbox. - All new operations exported in module.exports. Frontend tests (muxplex/frontend/tests/test_app.mjs): - Added 30 new tests under 'Phase 2 operation layer'. - Cover pure-op isolation, idempotency, no-op behavior, user-intent op composition, and guarantee that _serverSettings is never mutated. Verification: - muxplex/tests/: 1239 passed (1223 prior + 16 new pure-op tests) - test_app.mjs: 372 passed, 10 pre-existing failures (Phase 1 had 21; Phase 2 reduces to 10; none Phase-2-related; all test unrelated details of createNewSession, renderFilterBar, loadGridViewMode) Intentional inline-PATCH constructions (NOT regressions): - Creating empty views (~1263, 1347, 1595) - Moving views up/down in Settings tab (~2520, 2577) - _saveViewsAndRerender (full views array persistence) - createNewSession auto-add-to-active-view (session-creation side effect) These are view-management operations, not session-in-view management. Builds on Phase 0+1 (commit 0f9623d). No behavior change — purely structural refactor. Generated with Amplifier Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> --- muxplex/frontend/app.js | 320 +++++++++++++++++----------- muxplex/frontend/tests/test_app.mjs | 283 ++++++++++++++++++++++++ muxplex/tests/test_views.py | 165 ++++++++++++++ muxplex/views.py | 59 +++++ 4 files changed, 703 insertions(+), 124 deletions(-) diff --git a/muxplex/frontend/app.js b/muxplex/frontend/app.js index f56c299..7214bfb 100644 --- a/muxplex/frontend/app.js +++ b/muxplex/frontend/app.js @@ -694,6 +694,139 @@ function getVisibleSessions(sessions) { return filterVisible(sessions, _serverSettings, _activeView); } +// ============================================================================= +// Operation layer (Phase 2) +// +// Two layers: +// 1. Pure data ops: _opAddMembership/_opRemoveMembership/_opHide/_opUnhide — +// narrow, composable, no side effects beyond their name. Operate on a +// local settings object (typically a clone of _serverSettings) so callers +// can compose multiple operations before PATCHing. +// 2. User-intent ops: hideSessionOp/unhideSessionOp/addSessionToViewOp/ +// removeSessionFromViewOp — express what the *user* meant. Some compose +// multiple pure ops: +// - hideSessionOp = hide + removeFromAllViews (asymmetric: v1 +// federation-safe; matches current UX) +// - addSessionToViewOp = unhide + addMembership (auto-unhide on add, +// explicit composition) +// - unhideSessionOp = unhide (orthogonal — does NOT touch membership) +// - removeSessionFromViewOp = removeMembership (orthogonal — does NOT +// hide) +// +// The asymmetry between hideSessionOp (which removes from views) and +// addSessionToViewOp (which unhides) is intentional. See +// docs/plans/2026-05-17-hidden-state-redesign-design.md. +// ============================================================================= + +// --- Pure data ops --- + +// Add `key` to view's session list if absent. No-op if view doesn't exist. +function _opAddMembership(state, viewName, key) { + var views = state.views || []; + for (var i = 0; i < views.length; i++) { + if (views[i].name === viewName) { + var sessions = views[i].sessions || []; + if (sessions.indexOf(key) === -1) { + sessions.push(key); + views[i].sessions = sessions; + } + break; + } + } +} + +// Remove `key` from view's session list. No-op if view or key absent. +function _opRemoveMembership(state, viewName, key) { + var views = state.views || []; + for (var i = 0; i < views.length; i++) { + if (views[i].name === viewName) { + var sessions = views[i].sessions || []; + var pos = sessions.indexOf(key); + if (pos !== -1) { + sessions.splice(pos, 1); + views[i].sessions = sessions; + } + break; + } + } +} + +// Remove `key` from every view's session list. +function _opRemoveFromAllViews(state, key) { + var views = state.views || []; + for (var i = 0; i < views.length; i++) { + var sessions = views[i].sessions || []; + var pos = sessions.indexOf(key); + if (pos !== -1) { + sessions.splice(pos, 1); + views[i].sessions = sessions; + } + } +} + +// Append `key` to hidden_sessions if absent. +function _opHide(state, key) { + if (state.hidden_sessions.indexOf(key) === -1) { + state.hidden_sessions.push(key); + } +} + +// Remove `key` from hidden_sessions. No-op if absent. +function _opUnhide(state, key) { + var pos = state.hidden_sessions.indexOf(key); + if (pos !== -1) { + state.hidden_sessions.splice(pos, 1); + } +} + +// Helper: deep-clone the bits we mutate (avoid touching the cached +// _serverSettings before the PATCH confirms). +function _cloneOpState(settings) { + return JSON.parse(JSON.stringify({ + hidden_sessions: (settings && settings.hidden_sessions) || [], + views: (settings && settings.views) || [] + })); +} + +// --- User-intent ops --- +// Each returns the patch body for /api/settings. + +// hideSessionOp: hide(k) + removeFromAllViews(k). +// Asymmetric — removes from all views. This is the v1 federation-safe +// behaviour; matching the current UX. Returns { hidden_sessions, views }. +function hideSessionOp(settings, key) { + var state = _cloneOpState(settings); + _opHide(state, key); + _opRemoveFromAllViews(state, key); + return { hidden_sessions: state.hidden_sessions, views: state.views }; +} + +// unhideSessionOp: unhide(k) only. +// Orthogonal — does NOT touch view membership. Returns { hidden_sessions }. +function unhideSessionOp(settings, key) { + var state = _cloneOpState(settings); + _opUnhide(state, key); + return { hidden_sessions: state.hidden_sessions }; +} + +// addSessionToViewOp: unhide(k) + addMembership(k, viewName). +// Auto-unhide on add is explicit composition here, not an invariant +// enforced elsewhere. Returns { hidden_sessions, views }. +function addSessionToViewOp(settings, viewName, key) { + var state = _cloneOpState(settings); + _opUnhide(state, key); + _opAddMembership(state, viewName, key); + return { hidden_sessions: state.hidden_sessions, views: state.views }; +} + +// removeSessionFromViewOp: removeMembership(k, viewName) only. +// Orthogonal — does NOT hide the session. Returns { views }. +function removeSessionFromViewOp(settings, viewName, key) { + var state = _cloneOpState(settings); + _opRemoveMembership(state, viewName, key); + return { views: state.views }; +} + /** * Resolve the active view name against the known views list. * @@ -1924,33 +2057,22 @@ function _openMobileViewPicker(sessionKey, sessionName, unhideFirst) { if (!viewBtn) return; var idx = parseInt(viewBtn.dataset.viewIndex, 10); - var updatedViews = JSON.parse(JSON.stringify((_serverSettings && _serverSettings.views) || [])); - var view = updatedViews[idx]; + var views = (_serverSettings && _serverSettings.views) || []; + var view = views[idx]; if (!view) return; var sessions = view.sessions || []; - var pos = sessions.indexOf(sessionKey); + var isAlreadyInView = sessions.indexOf(sessionKey) !== -1; + + var patch; var nowIn; - if (pos !== -1) { - sessions.splice(pos, 1); + if (isAlreadyInView) { + patch = removeSessionFromViewOp(_serverSettings, view.name, sessionKey); nowIn = false; } else { - sessions.push(sessionKey); + patch = addSessionToViewOp(_serverSettings, view.name, sessionKey); nowIn = true; } - view.sessions = sessions; - - var patch = { views: updatedViews }; - if (unhideFirst && nowIn) { - var hidden = (_serverSettings && _serverSettings.hidden_sessions) || []; - var hiddenIdx = hidden.indexOf(sessionKey); - if (hiddenIdx !== -1) { - var updatedHidden = hidden.slice(); - updatedHidden.splice(hiddenIdx, 1); - patch.hidden_sessions = updatedHidden; - unhideFirst = false; // only unhide on first successful add - } - } // Update checkmark immediately for responsiveness var checkEl = viewBtn.querySelector('span'); @@ -1959,10 +2081,10 @@ function _openMobileViewPicker(sessionKey, sessionName, unhideFirst) { api('PATCH', '/api/settings', patch) .then(function() { if (_serverSettings) { - _serverSettings.views = updatedViews; + _serverSettings.views = patch.views; if (patch.hidden_sessions) _serverSettings.hidden_sessions = patch.hidden_sessions; } - if (patch.hidden_sessions) renderGrid(_currentSessions || []); + if (nowIn && patch.hidden_sessions) renderGrid(_currentSessions || []); }) .catch(function(err) { showToast('Couldn\u2019t save \u2014 try again'); @@ -2086,7 +2208,6 @@ function _openFlyoutSubmenu(triggerItem, unhideFirst) { var newViewAction = e.target.closest('[data-action="new-view-in-flyout"]'); if (newViewAction) { var capturedKey = sessionKey; - var capturedUnhide = unhideFirst; closeFlyoutMenu(); var newName = prompt('View name:'); if (!newName || !newName.trim()) return; @@ -2100,23 +2221,21 @@ function _openFlyoutSubmenu(triggerItem, unhideFirst) { showToast('View \'' + newName + '\' already exists'); return; } + // New-view creation: addSessionToViewOp doesn't model view creation, but + // we use it on a temp settings (with the new view already appended) so + // that the hidden_sessions update is expressed via the op layer. var newView = { name: newName, sessions: [capturedKey] }; var newViews = existViews.concat([newView]); - var flyoutPatch = { views: newViews }; - if (capturedUnhide) { - var hiddenList = (_serverSettings && _serverSettings.hidden_sessions) || []; - var hi = hiddenList.indexOf(capturedKey); - if (hi !== -1) { - var updHidden = hiddenList.slice(); - updHidden.splice(hi, 1); - flyoutPatch.hidden_sessions = updHidden; - } - } + var tempSettings = { + hidden_sessions: (_serverSettings && _serverSettings.hidden_sessions) || [], + views: newViews + }; + var flyoutPatch = addSessionToViewOp(tempSettings, newName, capturedKey); api('PATCH', '/api/settings', flyoutPatch) .then(function() { if (_serverSettings) { - _serverSettings.views = newViews; - if (flyoutPatch.hidden_sessions) _serverSettings.hidden_sessions = flyoutPatch.hidden_sessions; + _serverSettings.views = flyoutPatch.views; + _serverSettings.hidden_sessions = flyoutPatch.hidden_sessions; } switchView(newName); }) @@ -2130,35 +2249,24 @@ function _openFlyoutSubmenu(triggerItem, unhideFirst) { if (!btn) return; var idx = parseInt(btn.dataset.viewIndex, 10); - var updatedViews = JSON.parse(JSON.stringify((_serverSettings && _serverSettings.views) || [])); - var view = updatedViews[idx]; + var views = (_serverSettings && _serverSettings.views) || []; + var view = views[idx]; if (!view) return; var sessions = view.sessions || []; - var pos = sessions.indexOf(sessionKey); - if (pos !== -1) { - sessions.splice(pos, 1); + var isAlreadyInView = sessions.indexOf(sessionKey) !== -1; + + var patch; + if (isAlreadyInView) { + patch = removeSessionFromViewOp(_serverSettings, view.name, sessionKey); } else { - sessions.push(sessionKey); - } - view.sessions = sessions; - - var patch = { views: updatedViews }; - - if (unhideFirst) { - var hidden = (_serverSettings && _serverSettings.hidden_sessions) || []; - var hiddenIdx = hidden.indexOf(sessionKey); - if (hiddenIdx !== -1) { - var updatedHidden = hidden.slice(); - updatedHidden.splice(hiddenIdx, 1); - patch.hidden_sessions = updatedHidden; - } + patch = addSessionToViewOp(_serverSettings, view.name, sessionKey); } api('PATCH', '/api/settings', patch) .then(function() { if (_serverSettings) { - _serverSettings.views = updatedViews; + _serverSettings.views = patch.views; if (patch.hidden_sessions) _serverSettings.hidden_sessions = patch.hidden_sessions; } // Update checkmarks in submenu @@ -2167,12 +2275,13 @@ function _openFlyoutSubmenu(triggerItem, unhideFirst) { for (var ci = 0; ci < checkItems.length; ci++) { var vi = parseInt(checkItems[ci].dataset.viewIndex, 10); var checkEl = checkItems[ci].querySelector('.flyout-submenu__check'); - if (checkEl && updatedViews[vi]) { - checkEl.textContent = (updatedViews[vi].sessions || []).indexOf(sessionKey) !== -1 ? '\u2713' : ''; + var updViews = (_serverSettings && _serverSettings.views) || []; + if (checkEl && updViews[vi]) { + checkEl.textContent = (updViews[vi].sessions || []).indexOf(sessionKey) !== -1 ? '\u2713' : ''; } } } - if (unhideFirst) { + if (!isAlreadyInView && patch.hidden_sessions) { renderGrid(_currentSessions || []); } }) @@ -2191,30 +2300,15 @@ function _doHideSession() { var sessionKey = _flyoutSessionKey; if (!sessionKey) return; - var hidden = (_serverSettings && _serverSettings.hidden_sessions) || []; - var views = (_serverSettings && _serverSettings.views) || []; - - // Add to hidden_sessions - var updatedHidden = hidden.slice(); - if (updatedHidden.indexOf(sessionKey) === -1) { - updatedHidden.push(sessionKey); - } - - // Remove from all views (mutual exclusion) - var updatedViews = JSON.parse(JSON.stringify(views)); - for (var i = 0; i < updatedViews.length; i++) { - var sessions = updatedViews[i].sessions || []; - var idx = sessions.indexOf(sessionKey); - if (idx !== -1) sessions.splice(idx, 1); - } + var patch = hideSessionOp(_serverSettings, sessionKey); closeFlyoutMenu(); - api('PATCH', '/api/settings', { hidden_sessions: updatedHidden, views: updatedViews }) + api('PATCH', '/api/settings', patch) .then(function() { if (_serverSettings) { - _serverSettings.hidden_sessions = updatedHidden; - _serverSettings.views = updatedViews; + _serverSettings.hidden_sessions = patch.hidden_sessions; + _serverSettings.views = patch.views; } renderGrid(_currentSessions || []); renderViewDropdown(); @@ -2233,18 +2327,17 @@ function _doUnhideSession() { var sessionKey = _flyoutSessionKey; if (!sessionKey) return; + // Early-exit if session is not hidden (no PATCH needed). var hidden = (_serverSettings && _serverSettings.hidden_sessions) || []; - var idx = hidden.indexOf(sessionKey); - if (idx === -1) { closeFlyoutMenu(); return; } + if (hidden.indexOf(sessionKey) === -1) { closeFlyoutMenu(); return; } - var updatedHidden = hidden.slice(); - updatedHidden.splice(idx, 1); + var patch = unhideSessionOp(_serverSettings, sessionKey); closeFlyoutMenu(); - api('PATCH', '/api/settings', { hidden_sessions: updatedHidden }) + api('PATCH', '/api/settings', patch) .then(function() { - if (_serverSettings) _serverSettings.hidden_sessions = updatedHidden; + if (_serverSettings) _serverSettings.hidden_sessions = patch.hidden_sessions; renderGrid(_currentSessions || []); renderViewDropdown(); }) @@ -2262,24 +2355,13 @@ function _doRemoveFromView() { var sessionKey = _flyoutSessionKey; if (!sessionKey || _activeView === 'all' || _activeView === 'hidden') return; - var views = (_serverSettings && _serverSettings.views) || []; - var updatedViews = JSON.parse(JSON.stringify(views)); - - // Find the active view and remove the session - for (var i = 0; i < updatedViews.length; i++) { - if (updatedViews[i].name === _activeView) { - var sessions = updatedViews[i].sessions || []; - var idx = sessions.indexOf(sessionKey); - if (idx !== -1) sessions.splice(idx, 1); - break; - } - } + var patch = removeSessionFromViewOp(_serverSettings, _activeView, sessionKey); closeFlyoutMenu(); - api('PATCH', '/api/settings', { views: updatedViews }) + api('PATCH', '/api/settings', patch) .then(function() { - if (_serverSettings) _serverSettings.views = updatedViews; + if (_serverSettings) _serverSettings.views = patch.views; renderGrid(_currentSessions || []); }) .catch(function(err) { @@ -2606,40 +2688,18 @@ function renderManageViewList() { if (!cb) return; var sessionKey = cb.dataset.sessionKey; var isChecked = cb.checked; - var isHiddenSession = cb.dataset.isHidden === '1'; - var views = (_serverSettings && _serverSettings.views) || []; - var updatedViews = JSON.parse(JSON.stringify(views)); - - for (var vi = 0; vi < updatedViews.length; vi++) { - if (updatedViews[vi].name === _activeView) { - var vs = updatedViews[vi].sessions || []; - if (isChecked) { - if (vs.indexOf(sessionKey) === -1) vs.push(sessionKey); - } else { - var pos = vs.indexOf(sessionKey); - if (pos !== -1) vs.splice(pos, 1); - } - updatedViews[vi].sessions = vs; - break; - } - } - - var patch = { views: updatedViews }; - if (isHiddenSession && isChecked) { - var hiddenList = (_serverSettings && _serverSettings.hidden_sessions) || []; - var hi = hiddenList.indexOf(sessionKey); - if (hi !== -1) { - var updatedHidden = hiddenList.slice(); - updatedHidden.splice(hi, 1); - patch.hidden_sessions = updatedHidden; - } + var patch; + if (isChecked) { + patch = addSessionToViewOp(_serverSettings, _activeView, sessionKey); + } else { + patch = removeSessionFromViewOp(_serverSettings, _activeView, sessionKey); } api('PATCH', '/api/settings', patch) .then(function() { if (_serverSettings) { - _serverSettings.views = updatedViews; + _serverSettings.views = patch.views; if (patch.hidden_sessions) _serverSettings.hidden_sessions = patch.hidden_sessions; } // Update summary count in-place — do NOT re-render the full list (avoids layout thrash) @@ -4528,6 +4588,18 @@ if (typeof module !== 'undefined' && module.exports) { isHidden, filterVisible, visibleCount, + // Operation layer (Phase 2) — pure data ops + _opAddMembership, + _opRemoveMembership, + _opRemoveFromAllViews, + _opHide, + _opUnhide, + _cloneOpState, + // Operation layer (Phase 2) — user-intent ops + hideSessionOp, + unhideSessionOp, + addSessionToViewOp, + removeSessionFromViewOp, // Federation tiles buildStatusTileHTML, // Constants diff --git a/muxplex/frontend/tests/test_app.mjs b/muxplex/frontend/tests/test_app.mjs index 65d15c0..096cfd1 100644 --- a/muxplex/frontend/tests/test_app.mjs +++ b/muxplex/frontend/tests/test_app.mjs @@ -5066,3 +5066,286 @@ test('visibleCount always equals filterVisible().length for the same inputs', () ); } }); + +// ============================================================================= +// Phase 2 operation layer +// ============================================================================= + +// Helper: build a settings-like object with both views and hidden_sessions. +function _settingsWithState() { + return { + hidden_sessions: ['dev1:a', 'dev1:b'], + views: [ + { name: 'Work', sessions: ['dev1:a', 'dev1:c'] }, + { name: 'Personal', sessions: ['dev1:b', 'dev1:d'] }, + ], + }; +} + +// --- Exports check --- + +test('app.js exports all Phase 2 operation layer functions', () => { + const fns = [ + '_opAddMembership', '_opRemoveMembership', '_opRemoveFromAllViews', + '_opHide', '_opUnhide', '_cloneOpState', + 'hideSessionOp', 'unhideSessionOp', 'addSessionToViewOp', 'removeSessionFromViewOp', + ]; + for (const fn of fns) { + assert.ok(fn in app, `app.js should export "${fn}"`); + assert.strictEqual(typeof app[fn], 'function', `"${fn}" should be a function`); + } +}); + +// --- _cloneOpState --- + +test('_cloneOpState returns a deep clone — mutations do not affect the original', () => { + const settings = _settingsWithState(); + const clone = app._cloneOpState(settings); + clone.hidden_sessions.push('dev1:new'); + clone.views[0].sessions.push('dev1:extra'); + // Original untouched + assert.strictEqual(settings.hidden_sessions.indexOf('dev1:new'), -1); + assert.strictEqual(settings.views[0].sessions.indexOf('dev1:extra'), -1); +}); + +// --- Pure op isolation: each op affects ONLY its named field --- + +test('_opAddMembership does not touch hidden_sessions', () => { + const settings = _settingsWithState(); + const state = app._cloneOpState(settings); + const hiddenBefore = JSON.stringify(state.hidden_sessions); + app._opAddMembership(state, 'Work', 'dev1:new'); + assert.strictEqual(JSON.stringify(state.hidden_sessions), hiddenBefore); +}); + +test('_opRemoveMembership does not touch hidden_sessions', () => { + const settings = _settingsWithState(); + const state = app._cloneOpState(settings); + const hiddenBefore = JSON.stringify(state.hidden_sessions); + app._opRemoveMembership(state, 'Work', 'dev1:a'); + assert.strictEqual(JSON.stringify(state.hidden_sessions), hiddenBefore); +}); + +test('_opRemoveFromAllViews does not touch hidden_sessions', () => { + const settings = _settingsWithState(); + const state = app._cloneOpState(settings); + const hiddenBefore = JSON.stringify(state.hidden_sessions); + app._opRemoveFromAllViews(state, 'dev1:a'); + assert.strictEqual(JSON.stringify(state.hidden_sessions), hiddenBefore); +}); + +test('_opHide does not touch views', () => { + const settings = _settingsWithState(); + const state = app._cloneOpState(settings); + const viewsBefore = JSON.stringify(state.views); + app._opHide(state, 'dev1:new'); + assert.strictEqual(JSON.stringify(state.views), viewsBefore); +}); + +test('_opUnhide does not touch views', () => { + const settings = _settingsWithState(); + const state = app._cloneOpState(settings); + const viewsBefore = JSON.stringify(state.views); + app._opUnhide(state, 'dev1:a'); + assert.strictEqual(JSON.stringify(state.views), viewsBefore); +}); + +// --- _opAddMembership --- + +test('_opAddMembership adds key when absent', () => { + const state = app._cloneOpState(_settingsWithState()); + app._opAddMembership(state, 'Work', 'dev1:new'); + assert.ok(state.views[0].sessions.includes('dev1:new')); +}); + +test('_opAddMembership is idempotent — no duplicate', () => { + const state = app._cloneOpState(_settingsWithState()); + app._opAddMembership(state, 'Work', 'dev1:a'); // already present + assert.strictEqual(state.views[0].sessions.filter(k => k === 'dev1:a').length, 1); +}); + +test('_opAddMembership is no-op for unknown view', () => { + const settings = _settingsWithState(); + const state = app._cloneOpState(settings); + const viewsBefore = JSON.stringify(state.views); + app._opAddMembership(state, 'DoesNotExist', 'dev1:new'); + assert.strictEqual(JSON.stringify(state.views), viewsBefore); +}); + +// --- _opRemoveMembership --- + +test('_opRemoveMembership removes key when present', () => { + const state = app._cloneOpState(_settingsWithState()); + app._opRemoveMembership(state, 'Work', 'dev1:a'); + assert.ok(!state.views[0].sessions.includes('dev1:a')); +}); + +test('_opRemoveMembership is no-op when key absent', () => { + const settings = _settingsWithState(); + const state = app._cloneOpState(settings); + const workBefore = JSON.stringify(state.views[0].sessions); + app._opRemoveMembership(state, 'Work', 'dev1:nothere'); + assert.strictEqual(JSON.stringify(state.views[0].sessions), workBefore); +}); + +test('_opRemoveMembership is no-op for unknown view', () => { + const settings = _settingsWithState(); + const state = app._cloneOpState(settings); + const viewsBefore = JSON.stringify(state.views); + app._opRemoveMembership(state, 'DoesNotExist', 'dev1:a'); + assert.strictEqual(JSON.stringify(state.views), viewsBefore); +}); + +// --- _opRemoveFromAllViews --- + +test('_opRemoveFromAllViews removes key from every view', () => { + const state = { + hidden_sessions: ['dev1:x'], + views: [ + { name: 'Work', sessions: ['dev1:x', 'dev1:y'] }, + { name: 'Personal', sessions: ['dev1:x', 'dev1:z'] }, + ], + }; + app._opRemoveFromAllViews(state, 'dev1:x'); + assert.ok(!state.views[0].sessions.includes('dev1:x')); + assert.ok(!state.views[1].sessions.includes('dev1:x')); + // Other keys untouched + assert.ok(state.views[0].sessions.includes('dev1:y')); + assert.ok(state.views[1].sessions.includes('dev1:z')); +}); + +// --- _opHide --- + +test('_opHide adds key to hidden_sessions when absent', () => { + const state = { hidden_sessions: ['dev1:b'], views: [] }; + app._opHide(state, 'dev1:new'); + assert.ok(state.hidden_sessions.includes('dev1:new')); +}); + +test('_opHide is idempotent — no duplicate', () => { + const state = { hidden_sessions: ['dev1:a'], views: [] }; + app._opHide(state, 'dev1:a'); + assert.strictEqual(state.hidden_sessions.filter(k => k === 'dev1:a').length, 1); +}); + +// --- _opUnhide --- + +test('_opUnhide removes key from hidden_sessions', () => { + const state = { hidden_sessions: ['dev1:a', 'dev1:b'], views: [] }; + app._opUnhide(state, 'dev1:a'); + assert.ok(!state.hidden_sessions.includes('dev1:a')); + assert.ok(state.hidden_sessions.includes('dev1:b')); +}); + +test('_opUnhide is no-op when key absent', () => { + const state = { hidden_sessions: ['dev1:b'], views: [] }; + app._opUnhide(state, 'dev1:nothere'); + assert.deepStrictEqual(state.hidden_sessions, ['dev1:b']); +}); + +// --- hideSessionOp (user-intent, asymmetric) --- + +test('hideSessionOp puts key in hidden_sessions AND removes from all views', () => { + const settings = _settingsWithState(); + // dev1:a is in Work and in hidden_sessions already; dev1:c is in Work, not hidden + // Let's use a clean state where dev1:c is visible (in Work, not hidden) + const s = { + hidden_sessions: [], + views: [ + { name: 'Work', sessions: ['dev1:c', 'dev1:d'] }, + { name: 'Personal', sessions: ['dev1:c', 'dev1:e'] }, + ], + }; + const patch = app.hideSessionOp(s, 'dev1:c'); + assert.ok(patch.hidden_sessions.includes('dev1:c'), 'hidden_sessions must contain dev1:c'); + assert.ok(!patch.views[0].sessions.includes('dev1:c'), 'Work must not contain dev1:c'); + assert.ok(!patch.views[1].sessions.includes('dev1:c'), 'Personal must not contain dev1:c'); + // Other keys unaffected + assert.ok(patch.views[0].sessions.includes('dev1:d')); + assert.ok(patch.views[1].sessions.includes('dev1:e')); +}); + +test('hideSessionOp returns both hidden_sessions and views in patch', () => { + const patch = app.hideSessionOp(_settingsWithState(), 'dev1:x'); + assert.ok('hidden_sessions' in patch, 'patch must have hidden_sessions'); + assert.ok('views' in patch, 'patch must have views'); +}); + +// --- unhideSessionOp (user-intent, orthogonal) --- + +test('unhideSessionOp removes key from hidden_sessions', () => { + const settings = _settingsWithState(); // dev1:a is in hidden_sessions + const patch = app.unhideSessionOp(settings, 'dev1:a'); + assert.ok(!patch.hidden_sessions.includes('dev1:a')); +}); + +test('unhideSessionOp returns ONLY hidden_sessions — views key is absent', () => { + const patch = app.unhideSessionOp(_settingsWithState(), 'dev1:a'); + assert.ok('hidden_sessions' in patch, 'patch must have hidden_sessions'); + assert.ok(!('views' in patch), 'patch must NOT have views'); +}); + +// --- addSessionToViewOp (user-intent, auto-unhide) --- + +test('addSessionToViewOp adds key to named view AND removes from hidden_sessions', () => { + // dev1:b is hidden AND not in Work view + const settings = { + hidden_sessions: ['dev1:b'], + views: [ + { name: 'Work', sessions: ['dev1:a'] }, + ], + }; + const patch = app.addSessionToViewOp(settings, 'Work', 'dev1:b'); + assert.ok(patch.views[0].sessions.includes('dev1:b'), 'Work must now contain dev1:b'); + assert.ok(!patch.hidden_sessions.includes('dev1:b'), 'hidden_sessions must not contain dev1:b'); +}); + +test('addSessionToViewOp returns both hidden_sessions and views', () => { + const patch = app.addSessionToViewOp(_settingsWithState(), 'Work', 'dev1:z'); + assert.ok('hidden_sessions' in patch, 'patch must have hidden_sessions'); + assert.ok('views' in patch, 'patch must have views'); +}); + +// --- removeSessionFromViewOp (user-intent, orthogonal) --- + +test('removeSessionFromViewOp removes key from named view', () => { + const settings = _settingsWithState(); // dev1:a is in Work + const patch = app.removeSessionFromViewOp(settings, 'Work', 'dev1:a'); + assert.ok(!patch.views[0].sessions.includes('dev1:a')); +}); + +test('removeSessionFromViewOp returns ONLY views — hidden_sessions is absent', () => { + const patch = app.removeSessionFromViewOp(_settingsWithState(), 'Work', 'dev1:a'); + assert.ok('views' in patch, 'patch must have views'); + assert.ok(!('hidden_sessions' in patch), 'patch must NOT have hidden_sessions'); +}); + +// --- No mutation of _serverSettings --- + +test('hideSessionOp does not mutate its settings argument', () => { + const settings = _settingsWithState(); + const before = JSON.stringify(settings); + app.hideSessionOp(settings, 'dev1:c'); + assert.strictEqual(JSON.stringify(settings), before, 'settings must not be mutated'); +}); + +test('unhideSessionOp does not mutate its settings argument', () => { + const settings = _settingsWithState(); + const before = JSON.stringify(settings); + app.unhideSessionOp(settings, 'dev1:a'); + assert.strictEqual(JSON.stringify(settings), before, 'settings must not be mutated'); +}); + +test('addSessionToViewOp does not mutate its settings argument', () => { + const settings = _settingsWithState(); + const before = JSON.stringify(settings); + app.addSessionToViewOp(settings, 'Work', 'dev1:b'); + assert.strictEqual(JSON.stringify(settings), before, 'settings must not be mutated'); +}); + +test('removeSessionFromViewOp does not mutate its settings argument', () => { + const settings = _settingsWithState(); + const before = JSON.stringify(settings); + app.removeSessionFromViewOp(settings, 'Work', 'dev1:a'); + assert.strictEqual(JSON.stringify(settings), before, 'settings must not be mutated'); +}); diff --git a/muxplex/tests/test_views.py b/muxplex/tests/test_views.py index 0c62b17..d03c3cf 100644 --- a/muxplex/tests/test_views.py +++ b/muxplex/tests/test_views.py @@ -3,10 +3,15 @@ Tests for muxplex/views.py — views invariant enforcement and v2 visibility hel """ from muxplex.views import ( + add_membership, enforce_mutual_exclusion, filter_visible, + hide, is_hidden, normalize_session_keys, + remove_from_all_views, + remove_membership, + unhide, validate_view_name, visible_count, ) @@ -404,3 +409,163 @@ def test_normalize_handles_cross_device_name_collisions_safely(): # implementation uses setdefault, so first-seen wins. Document the behavior # in this test so the choice is visible. assert result["hidden_sessions"][0] in {"dev1:a", "dev2:a"} + + +# --------------------------------------------------------------------------- +# Pure data operations (Phase 2) +# See docs/plans/2026-05-17-hidden-state-redesign-design.md +# --------------------------------------------------------------------------- + + +def _settings_with_state() -> dict: + """Return a settings dict with both views and hidden_sessions populated.""" + return { + "hidden_sessions": ["dev1:a", "dev1:b"], + "views": [ + {"name": "Work", "sessions": ["dev1:a", "dev1:c"]}, + {"name": "Personal", "sessions": ["dev1:b", "dev1:d"]}, + ], + } + + +# --- add_membership --- + + +def test_add_membership_adds_key_when_absent(): + settings = _settings_with_state() + result = add_membership(settings, "Work", "dev1:new") + assert "dev1:new" in result["views"][0]["sessions"] + + +def test_add_membership_is_idempotent(): + settings = _settings_with_state() + add_membership(settings, "Work", "dev1:a") # already present + assert settings["views"][0]["sessions"].count("dev1:a") == 1 + + +def test_add_membership_is_noop_for_unknown_view(): + settings = _settings_with_state() + original_work = settings["views"][0]["sessions"][:] + original_personal = settings["views"][1]["sessions"][:] + result = add_membership(settings, "DoesNotExist", "dev1:new") + assert result["views"][0]["sessions"] == original_work + assert result["views"][1]["sessions"] == original_personal + + +def test_add_membership_does_not_touch_hidden_sessions(): + """Pure op: add_membership must not touch hidden_sessions.""" + settings = _settings_with_state() + original_hidden = settings["hidden_sessions"][:] + add_membership(settings, "Work", "dev1:new") + assert settings["hidden_sessions"] == original_hidden + + +# --- remove_membership --- + + +def test_remove_membership_removes_the_key(): + settings = _settings_with_state() + result = remove_membership(settings, "Work", "dev1:a") + assert "dev1:a" not in result["views"][0]["sessions"] + + +def test_remove_membership_is_noop_when_key_absent(): + settings = _settings_with_state() + before = settings["views"][0]["sessions"][:] + remove_membership(settings, "Work", "dev1:nothere") + assert settings["views"][0]["sessions"] == before + + +def test_remove_membership_is_noop_for_unknown_view(): + settings = _settings_with_state() + original_work = settings["views"][0]["sessions"][:] + original_personal = settings["views"][1]["sessions"][:] + result = remove_membership(settings, "DoesNotExist", "dev1:a") + assert result["views"][0]["sessions"] == original_work + assert result["views"][1]["sessions"] == original_personal + + +def test_remove_membership_does_not_touch_hidden_sessions(): + """Pure op: remove_membership must not touch hidden_sessions.""" + settings = _settings_with_state() + original_hidden = settings["hidden_sessions"][:] + remove_membership(settings, "Work", "dev1:a") + assert settings["hidden_sessions"] == original_hidden + + +# --- remove_from_all_views --- + + +def test_remove_from_all_views_clears_key_from_all_views(): + settings = { + "hidden_sessions": ["dev1:x"], + "views": [ + {"name": "Work", "sessions": ["dev1:x", "dev1:y"]}, + {"name": "Personal", "sessions": ["dev1:x", "dev1:z"]}, + ], + } + result = remove_from_all_views(settings, "dev1:x") + assert "dev1:x" not in result["views"][0]["sessions"] + assert "dev1:x" not in result["views"][1]["sessions"] + assert "dev1:y" in result["views"][0]["sessions"] + assert "dev1:z" in result["views"][1]["sessions"] + + +def test_remove_from_all_views_does_not_touch_hidden_sessions(): + """Pure op: remove_from_all_views must not touch hidden_sessions.""" + settings = _settings_with_state() + original_hidden = settings["hidden_sessions"][:] + remove_from_all_views(settings, "dev1:a") + assert settings["hidden_sessions"] == original_hidden + + +# --- hide --- + + +def test_hide_adds_to_hidden_sessions_when_absent(): + settings = {"hidden_sessions": ["dev1:b"], "views": []} + result = hide(settings, "dev1:new") + assert "dev1:new" in result["hidden_sessions"] + + +def test_hide_is_idempotent(): + settings = {"hidden_sessions": ["dev1:a"], "views": []} + hide(settings, "dev1:a") # already present + assert settings["hidden_sessions"].count("dev1:a") == 1 + + +def test_hide_does_not_touch_views(): + """Pure op: hide must not touch views.""" + settings = _settings_with_state() + original_work = settings["views"][0]["sessions"][:] + original_personal = settings["views"][1]["sessions"][:] + hide(settings, "dev1:new") + assert settings["views"][0]["sessions"] == original_work + assert settings["views"][1]["sessions"] == original_personal + + +# --- unhide --- + + +def test_unhide_removes_from_hidden_sessions(): + settings = {"hidden_sessions": ["dev1:a", "dev1:b"], "views": []} + result = unhide(settings, "dev1:a") + assert "dev1:a" not in result["hidden_sessions"] + assert "dev1:b" in result["hidden_sessions"] + + +def test_unhide_is_noop_when_absent(): + settings = {"hidden_sessions": ["dev1:b"], "views": []} + before = settings["hidden_sessions"][:] + unhide(settings, "dev1:nothere") + assert settings["hidden_sessions"] == before + + +def test_unhide_does_not_touch_views(): + """Pure op: unhide must not touch views.""" + settings = _settings_with_state() + original_work = settings["views"][0]["sessions"][:] + original_personal = settings["views"][1]["sessions"][:] + unhide(settings, "dev1:a") + assert settings["views"][0]["sessions"] == original_work + assert settings["views"][1]["sessions"] == original_personal diff --git a/muxplex/views.py b/muxplex/views.py index 5b4cf61..e26e818 100644 --- a/muxplex/views.py +++ b/muxplex/views.py @@ -203,6 +203,65 @@ def enforce_mutual_exclusion(settings: dict) -> dict: return settings +# --------------------------------------------------------------------------- +# Pure data ops (Phase 2) +# +# Pure data ops. Composable. No tangling of concerns. User-intent ops live on +# the frontend (where the PATCH boundary is) and call these to build the final +# state. +# +# Each mutates the settings dict in place and returns it. No side effects +# beyond the named operation. +# --------------------------------------------------------------------------- + + +def add_membership(settings: dict, view_name: str, key: str) -> dict: + """Add `key` to view's session list if absent. No-op if view doesn't exist.""" + for view in settings.get("views") or []: + if view.get("name") == view_name: + sessions = view.setdefault("sessions", []) + if key not in sessions: + sessions.append(key) + break + return settings + + +def remove_membership(settings: dict, view_name: str, key: str) -> dict: + """Remove `key` from view's session list. No-op if view or key absent.""" + for view in settings.get("views") or []: + if view.get("name") == view_name: + sessions = view.get("sessions") or [] + if key in sessions: + sessions.remove(key) + break + return settings + + +def remove_from_all_views(settings: dict, key: str) -> dict: + """Remove `key` from every view's session list.""" + for view in settings.get("views") or []: + sessions = view.get("sessions") or [] + if key in sessions: + sessions.remove(key) + return settings + + +def hide(settings: dict, key: str) -> dict: + """Append `key` to hidden_sessions if absent.""" + hidden = settings.setdefault("hidden_sessions", []) + if key not in hidden: + hidden.append(key) + return settings + + +def unhide(settings: dict, key: str) -> dict: + """Remove `key` from hidden_sessions. No-op if absent.""" + hidden = settings.get("hidden_sessions") or [] + if key in hidden: + hidden.remove(key) + return settings + + def validate_view_name(name: str, existing_views: list[dict]) -> str | None: """Validate a view name. Returns an error message string, or None if valid.