name: page-save-origin-semantics
When modifying page save logic, it's easy to accidentally break the carefully designed origin-based conflict detection system. The system uses a two-stage check mechanism (frontend + backend) to determine when revision validation should be enforced vs. bypassed for collaborative editing (Yjs).
Key Insight: Conflict detection (revision check) and diff detection (hasDiffToPrev) serve different purposes and require separate logic.
Understanding the two-stage origin check mechanism:
// apps/app/src/client/components/PageEditor/PageEditor.tsx:158
const isRevisionIdRequiredForPageUpdate = currentPage?.revision?.origin === undefined;
// lines 308-310
const revisionId = isRevisionIdRequiredForPageUpdate
? currentRevisionId
: undefined;
Logic: Check the latest revision's origin on the page:
origin === undefined (legacy/API save) → Send revisionIdorigin === "editor" or "view" → Do NOT send revisionId// apps/app/src/server/models/obsolete-page.js:167-172
const ignoreLatestRevision =
origin === Origin.Editor &&
(latestRevisionOrigin === Origin.Editor || latestRevisionOrigin === Origin.View);
if (ignoreLatestRevision) {
return true; // Bypass revision check
}
// Otherwise, enforce strict revision matching
if (revision != previousRevision) {
return false; // Reject save
}
return true;
Logic: Check current request's origin AND latest revision's origin:
origin === "editor" AND latest is "editor" or "view" → Bypass revision checkThree types of page update methods (called "origin"):
Origin.Editor = "editor" - Save from editor mode (collaborative editing via Yjs)Origin.View = "view" - Save from view mode
undefined - API-based saves or legacy pagesBasic Rule: Page updates require the previous revision ID in the request. If the latest revision doesn't match, the server rejects the request.
Exception - Editor origin is stronger than View origin:
Posted param "revisionId" is outdated errors when multiple members are using the Editor and View changes interrupt them| Latest Revision: Editor | Latest Revision: View | Latest Revision: API | |
|---|---|---|---|
| Request: Editor | ⭕️ Bypass revision check | ⭕️ Bypass revision check | ❌ Strict check |
| Request: View | ❌ Strict check | ❌ Strict check | ❌ Strict check |
| Request: API | ❌ Strict check | ❌ Strict check | ❌ Strict check |
Reading the table:
| Latest Revision Origin | Request Origin | revisionId Sent? | Revision Check | Use Case |
|---|---|---|---|---|
editor or view |
editor |
❌ No | ✅ Bypassed | Normal Editor use (most common) |
undefined |
editor |
✅ Yes | ✅ Enforced | Legacy page in Editor |
undefined |
undefined (API) |
✅ Yes (required) | ✅ Enforced | API save |
When adding server-side functionality that needs previous revision data:
// ✅ CORRECT: Separate concerns - conflict detection vs. diff detection
let previousRevision: IRevisionHasId | null = null;
// Priority 1: Use provided revisionId (for conflict detection)
if (sanitizeRevisionId != null) {
previousRevision = await Revision.findById(sanitizeRevisionId);
}
// Priority 2: Fallback to currentPage.revision (for other purposes like diff detection)
if (previousRevision == null && currentPage.revision != null) {
previousRevision = await Revision.findById(currentPage.revision);
}
const previousBody = previousRevision?.body ?? null;
// Continue with existing conflict detection logic (unchanged)
if (currentPage != null && !(await currentPage.isUpdatable(sanitizeRevisionId, origin))) {
// ... return conflict error
}
// Use previousBody for diff detection or other purposes
updatedPage = await crowi.pageService.updatePage(
currentPage,
body,
previousBody, // ← Available regardless of conflict detection logic
req.user,
options,
);
// ❌ WRONG: Forcing frontend to always send revisionId
const revisionId = currentRevisionId; // Always send, regardless of origin
// This breaks Yjs collaborative editing semantics!
// ❌ WRONG: Changing backend conflict detection logic
// Don't modify isUpdatable() unless you fully understand the implications
// for collaborative editing
Always consider this pattern when:
Key Principles:
currentPage.revisionLatest revision has origin=editor:
Frontend Logic:
isRevisionIdRequiredForPageUpdate = false (latest revision origin is not undefined)revisionId in requestorigin: Origin.EditorAPI Layer:
previousRevision = await Revision.findById(undefined); // → null
Result: No previousRevision fetched via revisionId
Backend Conflict Check (isUpdatable):
ignoreLatestRevision =
(Origin.Editor === Origin.Editor) &&
(latestRevisionOrigin === Origin.Editor || latestRevisionOrigin === Origin.View)
// → true (latest revision is editor)
return true; // Bypass revision check
Result: ✅ Save succeeds without revision validation
Impact on Other Features:
currentPage.revision fallback becomes necessaryLatest revision has origin=undefined:
Frontend Logic:
isRevisionIdRequiredForPageUpdate = true (latest revision origin is undefined)revisionId in requestorigin: Origin.EditorAPI Layer:
previousRevision = await Revision.findById(sanitizeRevisionId); // → revision object
Result: previousRevision fetched successfully
Backend Conflict Check (isUpdatable):
ignoreLatestRevision =
(Origin.Editor === Origin.Editor) &&
(latestRevisionOrigin === undefined)
// → false (latest revision is undefined, not editor/view)
// Strict revision check
if (revision != sanitizeRevisionId) {
return false; // Reject if mismatch
}
return true;
Result: ✅ Save succeeds only if revisionId matches
Request has origin=undefined or omitted:
Frontend: Not applicable (API client)
API Layer:
revisionId in requestpreviousRevision = await Revision.findById(sanitizeRevisionId)Backend Conflict Check (isUpdatable):
ignoreLatestRevision =
(undefined === Origin.Editor) && ...
// → false
// Strict revision check
if (revision != sanitizeRevisionId) {
return false;
}
return true;
Result: Strict validation enforced
Historical Context: At some point, the frontend stopped sending previousRevision (revisionId) for certain scenarios to support Yjs collaborative editing. This broke features that relied on previousRevision data being available.
The Core Issue:
The Solution Pattern:
// Separate the two concerns:
// 1. Fetch previousRevision for data purposes (diff detection, history, etc.)
let previousRevision: IRevisionHasId | null = null;
if (sanitizeRevisionId != null) {
previousRevision = await Revision.findById(sanitizeRevisionId);
} else if (currentPage.revision != null) {
previousRevision = await Revision.findById(currentPage.revision); // Fallback
}
// 2. Use previousRevision data for your feature
const previousBody = previousRevision?.body ?? null;
// 3. Conflict detection happens independently via isUpdatable()
if (currentPage != null && !(await currentPage.isUpdatable(sanitizeRevisionId, origin))) {
// Return conflict error
}
Official Documentation:
Related Files:
apps/app/src/client/components/PageEditor/PageEditor.tsx (lines 158, 240, 308-310)apps/app/src/server/models/obsolete-page.js (lines 159-182, isUpdatable method)apps/app/src/server/routes/apiv3/page/update-page.ts (lines 260-282, conflict check)packages/core/src/interfaces/revision.ts (lines 6-11, Origin definition)This pattern was identified during the "improve-unchanged-revision" feature implementation, where the initial assumption was that frontend should always send revisionId for diff detection. Deep analysis revealed:
Key Takeaway: Always understand the existing architectural patterns before proposing changes. What appears to be a "fix" might actually break carefully designed functionality.