Add live polling and optimistic locking for concurrent inline edits
Two-part fix so multiple people can edit /lignes at once without clobbering each other, and see each other's changes without a manual reload -- no Socket.io/websocket infra, just what fits the existing fetch-based architecture: 1. Polling (POLL_INTERVAL_MS = 8s): pollForChanges() asks JSON:API for any ligne_comptable changed since the last check (filtering on the `changed` field -- confirmed live that JSON:API only accepts a raw Unix timestamp for this, not the ISO string it returns in responses, silently matching everything otherwise) and mergeChangedRows() patches matching rows in place via Object.assign (not a `rows` reassignment, so it doesn't re-trigger the reconciliation-resolution watcher for routine polls). A row currently being edited is skipped entirely rather than overwritten out from under an in-progress keystroke. 2. Optimistic locking: every row now carries its `changed` timestamp, sent back on every inline edit (updateType/updateField). A new checkConflict() compares it against the node's actual changed time before saving and rejects with 409 if they differ -- someone else saved this exact line in between. On a 409, refreshSingleRow() re-fetches just that node and patches it in place so the view self-corrects instead of staying stuck on the stale state that caused the rejection. Verified live end-to-end against an isolated temporary test node (not real data): an external edit correctly appeared in the browser within one poll cycle with no reload; a save using a stale `changed` value was rejected with the conflict error, confirmed via direct DB query that it left the node's data completely untouched, and the view auto-corrected to show the other edit. Test node and its paragraph fully cleaned up afterward.
This commit is contained in:
@@ -57,6 +57,12 @@
|
||||
// more than enough rounds to walk the entire MIN_LOADABLE_DATE..
|
||||
// MAX_LOADABLE_DATE span at EXTEND_MONTHS per round.
|
||||
const MAX_ENSURE_SCROLLABLE_ROUNDS = 30;
|
||||
// How often to poll for other users' changes (see pollForChanges()) --
|
||||
// a plain setInterval rather than anything push-based (no Socket.io/
|
||||
// websocket infra for a 6-person internal tool); this is far more
|
||||
// frequent than table content actually changes, but the requests are
|
||||
// cheap and it keeps everyone's view close to live.
|
||||
const POLL_INTERVAL_MS = 8000;
|
||||
|
||||
// Absolute bounds on how far the window can extend. Without these,
|
||||
// loadOlder()/loadNewer() kept pushing windowStart/windowEnd outward
|
||||
@@ -148,6 +154,23 @@
|
||||
return fetchLignes(API_BASE + '?' + params.toString());
|
||||
}
|
||||
|
||||
// Any ligne_comptable changed after `tsSeconds` (a Unix timestamp) --
|
||||
// backs the polling in pollForChanges() below, so one user's edit
|
||||
// shows up in everyone else's table without a manual reload. `changed`
|
||||
// is stored as a Unix timestamp internally; JSON:API exposes it as an
|
||||
// ISO 8601 string in *responses* but only accepts the raw timestamp as
|
||||
// a *filter value* -- confirmed live (an ISO string filter value
|
||||
// silently matched everything instead of narrowing the query).
|
||||
async function fetchChangedSince(tsSeconds) {
|
||||
const params = new URLSearchParams();
|
||||
params.set('include', 'field_repartition,field_repartition.field_compte,field_client,field_entree_liee');
|
||||
params.set('filter[changedFilter][condition][path]', 'changed');
|
||||
params.set('filter[changedFilter][condition][operator]', '>');
|
||||
params.set('filter[changedFilter][condition][value]', String(tsSeconds));
|
||||
params.set('sort', 'changed');
|
||||
return fetchLignes(API_BASE + '?' + params.toString());
|
||||
}
|
||||
|
||||
async function fetchClientNames() {
|
||||
// page[limit]=200 is silently clamped to core's hard cap of 50 by
|
||||
// JSON:API (Query\OffsetPage::SIZE_MAX) -- with 106 client terms,
|
||||
@@ -186,32 +209,45 @@
|
||||
// full edit form. Fetches a fresh CSRF token each time (core's
|
||||
// /session/token) rather than caching one -- this is an occasional
|
||||
// action, not worth the complexity of handling a stale cached token.
|
||||
async function updateLigneType(nid, type) {
|
||||
// `changed` (the row's last-known changed timestamp) lets the server
|
||||
// reject a save-over-a-stale-view instead of silently overwriting
|
||||
// someone else's concurrent edit -- see LedgerActionsController::
|
||||
// checkConflict(). The thrown error's `.status` lets callers tell a
|
||||
// 409 conflict apart from any other failure.
|
||||
async function updateLigneType(nid, type, changed) {
|
||||
const tokenRes = await fetch('/session/token');
|
||||
const token = await tokenRes.text();
|
||||
const res = await fetch('/lignes/' + nid + '/type', {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json', 'X-CSRF-Token': token },
|
||||
body: JSON.stringify({ type }),
|
||||
body: JSON.stringify({ type, changed }),
|
||||
});
|
||||
const json = await res.json().catch(() => ({}));
|
||||
if (!res.ok) throw new Error(json.error || ('/lignes/' + nid + '/type a répondu ' + res.status));
|
||||
if (!res.ok) {
|
||||
const err = new Error(json.error || ('/lignes/' + nid + '/type a répondu ' + res.status));
|
||||
err.status = res.status;
|
||||
throw err;
|
||||
}
|
||||
return json;
|
||||
}
|
||||
|
||||
// POST /lignes/{nid}/champ -- change client/facture/libellé without
|
||||
// opening the full edit form. Same fresh-token-per-call reasoning as
|
||||
// updateLigneType() above.
|
||||
async function updateLigneField(nid, field, value) {
|
||||
// opening the full edit form. Same fresh-token-per-call and conflict-
|
||||
// detection reasoning as updateLigneType() above.
|
||||
async function updateLigneField(nid, field, value, changed) {
|
||||
const tokenRes = await fetch('/session/token');
|
||||
const token = await tokenRes.text();
|
||||
const res = await fetch('/lignes/' + nid + '/champ', {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json', 'X-CSRF-Token': token },
|
||||
body: JSON.stringify({ field, value }),
|
||||
body: JSON.stringify({ field, value, changed }),
|
||||
});
|
||||
const json = await res.json().catch(() => ({}));
|
||||
if (!res.ok) throw new Error(json.error || ('/lignes/' + nid + '/champ a répondu ' + res.status));
|
||||
if (!res.ok) {
|
||||
const err = new Error(json.error || ('/lignes/' + nid + '/champ a répondu ' + res.status));
|
||||
err.status = res.status;
|
||||
throw err;
|
||||
}
|
||||
return json;
|
||||
}
|
||||
|
||||
@@ -291,6 +327,13 @@
|
||||
rows.push({
|
||||
id: node.id,
|
||||
nid: attrs.drupal_internal__nid,
|
||||
// ISO 8601 string (ok as an opaque token here, only ever
|
||||
// compared for equality/passed straight back to the server) --
|
||||
// sent back on the next inline edit so the server can detect
|
||||
// "someone else saved this line in between" and reject instead
|
||||
// of silently overwriting. See LedgerActionsController::
|
||||
// checkConflict() and pollForChanges()/mergeChangedRows() below.
|
||||
changed: attrs.changed,
|
||||
date: attrs.field_date_ligne,
|
||||
type: attrs.field_type_ligne,
|
||||
client: clientTerm ? clientTerm.attributes.name : null,
|
||||
@@ -352,6 +395,9 @@
|
||||
// group again for every other sortie that happens to link to
|
||||
// the same entrée.
|
||||
resolvedLinkGroups: new Set(),
|
||||
// Unix timestamp (seconds) -- everything with `changed` after
|
||||
// this has appeared since the last poll. See pollForChanges().
|
||||
lastPollTs: null,
|
||||
// Which row's type badge is currently showing its inline <select>
|
||||
// instead of the badge -- only one at a time.
|
||||
editingTypeId: null,
|
||||
@@ -817,6 +863,57 @@
|
||||
}
|
||||
await Promise.all(toFetch.map((nid) => this.fetchGroupFromNid(nid).catch(() => {})));
|
||||
},
|
||||
// Live multi-user updates without websocket/Socket.io infra --
|
||||
// plain polling (see POLL_INTERVAL_MS) is simple, fits the
|
||||
// existing fetch-based architecture, and is plenty for a
|
||||
// 6-person internal tool. A failed poll is silent and just tried
|
||||
// again next tick (matches other background-refresh code here);
|
||||
// lastPollTs is deliberately *not* advanced on failure, so a
|
||||
// transient network blip doesn't miss whatever changed during it.
|
||||
startPolling() {
|
||||
setInterval(() => this.pollForChanges(), POLL_INTERVAL_MS);
|
||||
},
|
||||
async pollForChanges() {
|
||||
try {
|
||||
const { data, includedMap } = await fetchChangedSince(this.lastPollTs);
|
||||
if (data.length) {
|
||||
this.mergeChangedRows(buildRows(data, includedMap));
|
||||
}
|
||||
this.lastPollTs = Math.floor(Date.now() / 1000);
|
||||
} catch (err) {
|
||||
// silent -- see comment above.
|
||||
}
|
||||
},
|
||||
// Patches already-loaded rows in place (mutates each row's own
|
||||
// properties via Object.assign, not the `rows`/`groupExtraRows`
|
||||
// arrays themselves) so Vue's reactivity updates just the
|
||||
// affected cells, without re-triggering the `rows` watcher (and
|
||||
// so ensureLinkedReconciliationResolved()) for routine polls that
|
||||
// only touch content already known locally. A row currently being
|
||||
// edited is left alone entirely -- overwriting its value out from
|
||||
// under an in-progress keystroke would be worse than a few
|
||||
// seconds of staleness; it'll pick up on the next poll once
|
||||
// editing finishes. Genuinely new lines (not loaded at all yet)
|
||||
// that fall inside the current window get appended -- that's the
|
||||
// one case that *does* need the array reassigned, since there's
|
||||
// no existing row object to mutate.
|
||||
mergeChangedRows(changedRows) {
|
||||
const isBeingEdited = (id) => this.editingTypeId === id || (this.editingCell && this.editingCell.id === id);
|
||||
const known = new Map([...this.rows, ...this.groupExtraRows].map((r) => [r.id, r]));
|
||||
const newOnes = [];
|
||||
for (const fresh of changedRows) {
|
||||
if (isBeingEdited(fresh.id)) continue;
|
||||
const existing = known.get(fresh.id);
|
||||
if (existing) {
|
||||
Object.assign(existing, fresh);
|
||||
} else if (fresh.date >= this.windowStart && fresh.date < this.windowEnd) {
|
||||
newOnes.push(fresh);
|
||||
}
|
||||
}
|
||||
if (newOnes.length) {
|
||||
this.rows = [...this.rows, ...newOnes].sort((a, b) => (a.date || '').localeCompare(b.date || ''));
|
||||
}
|
||||
},
|
||||
startEditType(item) {
|
||||
this.typeUpdateError = null;
|
||||
this.editingTypeId = item.id;
|
||||
@@ -832,11 +929,12 @@
|
||||
this.editingTypeId = null;
|
||||
if (newType === item.type) return;
|
||||
try {
|
||||
const result = await updateLigneType(item.nid, newType);
|
||||
const result = await updateLigneType(item.nid, newType, item.changed);
|
||||
const row = this.rows.find((r) => r.id === item.id);
|
||||
if (row) {
|
||||
row.type = newType;
|
||||
row.linkable = LINKABLE_TYPES.includes(newType);
|
||||
row.changed = result.changed;
|
||||
if (result.entree_liee_cleared) {
|
||||
row.entreeLieeIds = [];
|
||||
row.entreeLieeLabels = [];
|
||||
@@ -845,6 +943,7 @@
|
||||
this.reloadWindow();
|
||||
} catch (err) {
|
||||
this.typeUpdateError = err.message;
|
||||
if (err.status === 409) this.refreshSingleRow(item.id, item.nid);
|
||||
}
|
||||
},
|
||||
startEditCell(item, field) {
|
||||
@@ -863,9 +962,12 @@
|
||||
this.editingCell = null;
|
||||
if (newValue === (item[field] || '')) return;
|
||||
try {
|
||||
const result = await updateLigneField(item.nid, field, newValue);
|
||||
const result = await updateLigneField(item.nid, field, newValue, item.changed);
|
||||
const row = this.rows.find((r) => r.id === item.id);
|
||||
if (row) row[field] = result.value;
|
||||
if (row) {
|
||||
row[field] = result.value;
|
||||
row.changed = result.changed;
|
||||
}
|
||||
// A client name with no existing match gets created on the fly
|
||||
// (server-side) rather than rejected -- reflect it in the
|
||||
// filter dropdown/datalist immediately instead of only after a
|
||||
@@ -875,6 +977,25 @@
|
||||
}
|
||||
} catch (err) {
|
||||
this.typeUpdateError = err.message;
|
||||
if (err.status === 409) this.refreshSingleRow(item.id, item.nid);
|
||||
}
|
||||
},
|
||||
// 409 conflict from saveType()/saveCell() above -- someone else
|
||||
// saved this exact line in between. Re-fetch just this one node
|
||||
// and patch it in place, so the row reflects their change right
|
||||
// away instead of staying stuck on the stale view that caused the
|
||||
// rejection (the user would otherwise hit the same conflict again
|
||||
// on retry without understanding why).
|
||||
async refreshSingleRow(id, nid) {
|
||||
try {
|
||||
const { data, includedMap } = await fetchLignesByNids([nid]);
|
||||
if (!data.length) return;
|
||||
const [freshRow] = buildRows(data, includedMap);
|
||||
const existing = this.rows.find((r) => r.id === id) || this.groupExtraRows.find((r) => r.id === id);
|
||||
if (existing) Object.assign(existing, freshRow);
|
||||
} catch (err) {
|
||||
// Best effort -- the error banner from the conflict itself
|
||||
// already told the user to reload if this doesn't work out.
|
||||
}
|
||||
},
|
||||
onCellHover(evt) {
|
||||
@@ -1343,6 +1464,9 @@
|
||||
fetchYearsList().then((years) => { this.allYearsList = years; }).catch(() => {});
|
||||
fetchOuvertureEcarts().then((ecarts) => { this.ouvertureEcarts = ecarts; }).catch(() => {});
|
||||
jQuery(document).on('dialog:afterclose', () => this.reloadWindow());
|
||||
|
||||
this.lastPollTs = Math.floor(Date.now() / 1000);
|
||||
this.startPolling();
|
||||
},
|
||||
};
|
||||
|
||||
|
||||
@@ -74,6 +74,10 @@ class LedgerActionsController extends ControllerBase {
|
||||
return new JsonResponse(['error' => 'Type de ligne invalide.'], 400);
|
||||
}
|
||||
|
||||
if ($conflict = $this->checkConflict($request, $node)) {
|
||||
return $conflict;
|
||||
}
|
||||
|
||||
$node->set('field_type_ligne', $type);
|
||||
|
||||
// A type that's no longer linkable shouldn't keep a stale
|
||||
@@ -110,6 +114,7 @@ class LedgerActionsController extends ControllerBase {
|
||||
'success' => TRUE,
|
||||
'type' => $type,
|
||||
'entree_liee_cleared' => !in_array($type, self::LINKABLE_TYPES, TRUE),
|
||||
'changed' => date(DATE_ATOM, $node->getChangedTime()),
|
||||
]);
|
||||
}
|
||||
|
||||
@@ -137,6 +142,10 @@ class LedgerActionsController extends ControllerBase {
|
||||
}
|
||||
$fieldName = self::INLINE_EDITABLE_FIELDS[$field];
|
||||
|
||||
if ($conflict = $this->checkConflict($request, $node)) {
|
||||
return $conflict;
|
||||
}
|
||||
|
||||
if ($field === 'client') {
|
||||
if ($value === '') {
|
||||
$node->set('field_client', NULL);
|
||||
@@ -186,7 +195,37 @@ class LedgerActionsController extends ControllerBase {
|
||||
'success' => TRUE,
|
||||
'field' => $field,
|
||||
'value' => $newValue,
|
||||
'changed' => date(DATE_ATOM, $node->getChangedTime()),
|
||||
]);
|
||||
}
|
||||
|
||||
/**
|
||||
* Optimistic-locking guard shared by both endpoints above: the
|
||||
* frontend sends the `changed` timestamp of the row it last saw (see
|
||||
* buildRows() in home.js), captured at load/last-successful-save
|
||||
* time. If the node's *actual* changed time has since moved on --
|
||||
* someone else saved this same line in between -- the two won't
|
||||
* match, and we reject rather than silently overwrite whatever that
|
||||
* other save touched. Absent entirely (older cached frontend, or a
|
||||
* request that genuinely doesn't know it, e.g. a fresh row from
|
||||
* autocreate) skips the check rather than blocking on a false
|
||||
* mismatch -- the check is a safety net for the common case, not a
|
||||
* hard requirement of the API contract.
|
||||
*/
|
||||
private function checkConflict(Request $request, NodeInterface $node) {
|
||||
$data = json_decode($request->getContent(), TRUE);
|
||||
$clientChanged = is_array($data) ? ($data['changed'] ?? NULL) : NULL;
|
||||
if ($clientChanged === NULL) {
|
||||
return NULL;
|
||||
}
|
||||
$clientChangedTs = strtotime($clientChanged);
|
||||
if ($clientChangedTs !== FALSE && $clientChangedTs !== (int) $node->getChangedTime()) {
|
||||
return new JsonResponse([
|
||||
'error' => 'Cette ligne a été modifiée par quelqu\'un d\'autre entre-temps. Rechargez la page pour voir les dernières modifications.',
|
||||
'conflict' => TRUE,
|
||||
], 409);
|
||||
}
|
||||
return NULL;
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user