growilabs/page-save-origin-semantics

Page Save Origin Semantics

Problem

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.

Solution

Understanding the two-stage origin check mechanism:

Stage 1: Frontend Determines revisionId Requirement

// 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:

• If origin === undefined (legacy/API save) → Send revisionId
• If origin === "editor" or "view" → Do NOT send revisionId

Stage 2: Backend Determines Conflict Check Behavior

// 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:

• If origin === "editor" AND latest is "editor" or "view" → Bypass revision check
• Otherwise → Enforce strict revision ID matching

Origin Values

Three 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
  • Examples: HandsontableModal, DrawioModal editing
• undefined - API-based saves or legacy pages

Origin Strength (強弱)

Basic 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:

• UX Goal: Avoid Posted param "revisionId" is outdated errors when multiple members are using the Editor and View changes interrupt them
• Special Case: When the latest revision's origin is View, Editor origin requests can update WITHOUT requiring revision ID

Origin Strength Matrix

| | 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:

• ⭕️ = Revision check bypassed (revisionId not required)
• ❌ = Strict revision check required (revisionId must match)

Behavior by Scenario

| 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 |

Example: Server-Side Logic Respecting Origin Semantics

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

When to Apply

Always consider this pattern when:

• Modifying page save/update API handlers
• Adding functionality that needs previous revision data
• Working on conflict detection or revision validation logic
• Implementing features that interact with page history
• Debugging save operation issues

Key Principles:

1. Do NOT modify frontend revisionId logic unless explicitly required for conflict detection
2. Do NOT modify isUpdatable() logic unless fixing conflict detection bugs
3. Separate concerns: Conflict detection ≠ Other revision-based features (diff detection, history, etc.)
4. Server-side fallback: If you need previous revision data when revisionId is not provided, fetch from currentPage.revision

Detailed Scenario Analysis

Scenario A: Normal Editor Mode (Most Common Case)

Latest revision has origin=editor:

1. Frontend Logic:

   • isRevisionIdRequiredForPageUpdate = false (latest revision origin is not undefined)
   • Does NOT send revisionId in request
   • Sends origin: Origin.Editor

2. API Layer:

   previousRevision = await Revision.findById(undefined);  // → null
   

   Result: No previousRevision fetched via revisionId

3. 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

4. Impact on Other Features:

   • If you need previousRevision data (e.g., for diff detection), it won't be available unless you implement server-side fallback
   • This is where currentPage.revision fallback becomes necessary

Scenario B: Legacy Page in Editor Mode

Latest revision has origin=undefined:

1. Frontend Logic:

   • isRevisionIdRequiredForPageUpdate = true (latest revision origin is undefined)
   • Sends revisionId in request
   • Sends origin: Origin.Editor

2. API Layer:

   previousRevision = await Revision.findById(sanitizeRevisionId);  // → revision object
   

   Result: previousRevision fetched successfully

3. 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

4. Impact on Other Features:

   • previousRevision data is available
   • All revision-based features work correctly

Scenario C: API-Based Save

Request has origin=undefined or omitted:

1. Frontend: Not applicable (API client)

2. API Layer:

   • API client MUST send revisionId in request
   • previousRevision = await Revision.findById(sanitizeRevisionId)

3. Backend Conflict Check (isUpdatable):

   ignoreLatestRevision =
     (undefined === Origin.Editor) && ...
   // → false
   
   // Strict revision check
   if (revision != sanitizeRevisionId) {
     return false;
   }
   return true;
   

   Result: Strict validation enforced

Root Cause: Why This Separation Matters

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:

• Conflict detection needs to know "Is this save conflicting with another user's changes?" (Answered by revision check)
• Diff detection needs to know "Did the content actually change?" (Answered by comparing body)
• Current implementation conflates these: When conflict detection is bypassed, previousRevision is not fetched, breaking diff detection

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
}

Reference

Official Documentation:

• https://dev.growi.org/651a6f4a008fee2f99187431#origin-%E3%81%AE%E5%BC%B7%E5%BC%B1

Related Files:

• Frontend: apps/app/src/client/components/PageEditor/PageEditor.tsx (lines 158, 240, 308-310)
• Backend: apps/app/src/server/models/obsolete-page.js (lines 159-182, isUpdatable method)
• API: apps/app/src/server/routes/apiv3/page/update-page.ts (lines 260-282, conflict check)
• Interface: packages/core/src/interfaces/revision.ts (lines 6-11, Origin definition)

Common Pitfalls

1. Assuming revisionId is always available: It's not! Editor mode with recent editor/view saves omits it by design.
2. Conflating conflict detection with other features: They serve different purposes and need separate logic.
3. Breaking Yjs collaborative editing: Forcing revisionId to always be sent breaks the bypass mechanism.
4. Ignoring origin values: The system behavior changes significantly based on origin combinations.

Lessons Learned

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:

• The frontend logic is correct for conflict detection and should NOT be changed
• Server-side fallback is the correct approach to get previous revision data
• Two-stage checking is intentional and critical for Yjs collaborative editing
• Conflict detection and diff detection must be separated

Key Takeaway: Always understand the existing architectural patterns before proposing changes. What appears to be a "fix" might actually break carefully designed functionality.