MM-70092: Preserve drafts when a page is soft-deleted

server/app/page_draft_test.go

@@ -212,9 +212,11 @@ func TestPublishStaleBaselineConflicts(t *testing.T) {
 	require.Greater(t, current.EditAt, staleEditAt, "current page must carry the advanced baseline")
 }
 
-// TestPublishAfterPageDeleteReturns404 verifies that deleting a page cascade-deletes its drafts,
-// so a later publish finds no draft (404) rather than writing to a tombstone. The page-deleted 409
-// path in PublishPageDraft guards only the concurrent-delete race, which this flow cannot produce.
+// TestPublishAfterPageDeleteReturns404 verifies that a draft whose page has been soft-deleted is
+// not publishable. The draft itself survives the reversible delete, but the page-liveness
+// filter hides it from the draft read, so PublishPageDraft's idempotency guard finds no visible
+// draft and returns 404 rather than writing to a tombstone. The page-deleted 409 path in
+// PublishPageDraft guards only the concurrent-delete race, which this flow cannot produce.
 func TestPublishAfterPageDeleteReturns404(t *testing.T) {
 	h := openTestService(t)
 	space := mustCreateSpace(t, h.store, mmmodel.NewId())
@@ -233,6 +235,50 @@ func TestPublishAfterPageDeleteReturns404(t *testing.T) {
 	require.Equal(t, http.StatusNotFound, appErr.StatusCode)
 }
 
+// TestPublishAfterPageDeleteRestoreConflictsThenForceSucceeds verifies the publish path reachable
+// only because a draft now survives a page delete: DeletePage and RestorePage each advance the
+// page's EditAt once, so a draft's write-once BaseEditAt — captured before the delete — is two
+// bumps stale by the time the page is restored. A non-force publish must 409 as a concurrent edit
+// (not a 404, since the draft and its page are both visible again); force=true bypasses the stale
+// baseline and publishes the draft's content.
+func TestPublishAfterPageDeleteRestoreConflictsThenForceSucceeds(t *testing.T) {
+	h := openTestService(t)
+	space := mustCreateSpace(t, h.store, mmmodel.NewId())
+	userID := mmmodel.NewId()
+
+	page := publishNewPage(t, h, space.Id, userID, "Doc", "v1")
+
+	// Start an edit session baselined at the current EditAt; the draft persists (it is not consumed
+	// by any publish) through the delete+restore round trip below.
+	_, appErr := h.svc.UpdatePageDraft(&model.Draft{
+		UserId: userID, SpaceId: space.Id, PageId: page.Id, Title: "Doc", Body: docWith("v2"),
+		BaseEditAt: page.EditAt,
+	}, nil, nil, nil, "")
+	require.Nil(t, appErr)
+
+	requireStoreDeletePage(t, h.store, page.Id, space.Id, userID)
+	_, restoreErr := h.store.RestorePage(page.Id, space.Id, userID, model.MaxPageDepth)
+	require.NoError(t, restoreErr)
+
+	// The draft's baseline is now stale: a non-force publish must 409 as a concurrent edit, not
+	// succeed or 404 (the page and draft are both live again).
+	_, _, appErr = h.svc.PublishPageDraft(userID, space.Id, page.Id, false)
+	require.NotNil(t, appErr)
+	require.Equal(t, http.StatusConflict, appErr.StatusCode)
+	require.Equal(t, "app.page_draft.publish.edit_conflict.app_error", appErr.Id)
+
+	// The rejected publish must not have consumed the draft.
+	stillDrafted, appErr := h.svc.GetPageDraft(userID, space.Id, page.Id)
+	require.Nil(t, appErr, "a rejected non-force publish must leave the draft in place")
+	require.Contains(t, stillDrafted.Body, "v2")
+
+	// force=true bypasses the stale baseline and publishes the draft's content.
+	forced, wasCreated, appErr := h.svc.PublishPageDraft(userID, space.Id, page.Id, true)
+	require.Nil(t, appErr)
+	require.False(t, wasCreated)
+	require.Contains(t, forced.Body, "v2")
+}
+
 func TestDeletePageDraftRejectsWrongSpace(t *testing.T) {
 	h := openTestService(t)
 	spaceA := mustCreateSpace(t, h.store, mmmodel.NewId())

server/store/draft_store.go

@@ -35,6 +35,13 @@ var draftMetaColumns = []string{
 // the draft read queries: the space must be live, and the draft's page must be either absent
 // (a new-page draft) or a live page in the draft's own space. The draft table
 // must be aliased "d".
+//
+// This filter is the sole mechanism hiding a draft whose page or space is deleted. Drafts carry
+// no DeleteAt of their own, and neither DeletePage nor DeleteSpace purges them — both deletes are
+// reversible, so destroying a draft would irreversibly discard unpublished work (including other
+// users') as a side effect. A preserved draft is therefore invisible to every read below while its
+// page or space is deleted, and visible again once it is restored. Any new query reading draft
+// rows must apply this filter, or it will surface drafts the product considers deleted.
 func applyDraftLivenessFilter(q sq.SelectBuilder) sq.SelectBuilder {
 	return q.
 		Join("DOCS_Space s ON s.Id = d.SpaceId AND s.DeleteAt = 0").
@@ -45,23 +52,10 @@ func applyDraftLivenessFilter(q sq.SelectBuilder) sq.SelectBuilder {
 		})
 }
 
-// deleteDraftsForPage hard-deletes every user's draft for pageID scoped to spaceID. Drafts have
-// no soft-delete and must be cleaned up when the page is deleted. The spaceID predicate prevents
-// a delete in one space from removing drafts for the same pageID in another space. Must run
-// inside tx.
-func (s *Store) deleteDraftsForPage(tx *sqlx.Tx, pageID, spaceID string) error {
-	query := s.getQueryBuilder().
-		Delete("DOCS_Draft").
-		Where(sq.Eq{"PageId": pageID, "SpaceId": spaceID})
-	if _, err := s.execBuilder(tx, query); err != nil {
-		return errors.Wrap(err, "failed to delete page drafts")
-	}
-	return nil
-}
-
 // reparentDraftsForPage reparents every new-page draft pointing at pageID to newParentID,
 // so drafts don't retain a soft-deleted page as their pending parent. The spaceID predicate
-// scopes the rewrite the same way deleteDraftsForPage scopes its delete. Must run inside tx.
+// keeps a delete in one space from rewriting drafts for the same pageID in another. Must run
+// inside tx.
 //
 // UpdateAt uses GREATEST(now, UpdateAt+1) for the same reason as UpsertDraft: it must be a
 // strictly-monotonic token so the publish CAS-delete (deletePublishedDraftTx) cannot match a row

server/store/draft_store_test.go

@@ -204,6 +204,37 @@ func TestDraft(t *testing.T) {
 		require.Equal(t, draft.PageId, kept.PageId)
 	})
 
+	t.Run("a draft on a live page is hidden while its space is soft-deleted", func(t *testing.T) {
+		// The subtest above drafts against a page id that has no page row, so the filter's
+		// p.Id IS NULL branch carries it. Here the draft sits on a live page row, so only the
+		// space-liveness JOIN can hide it.
+		s := openTestDB(t)
+		channelID := mmmodel.NewId()
+		space, err := s.CreateSpace(newSpace(channelID))
+		require.NoError(t, err)
+		userID := mmmodel.NewId()
+
+		page, err := s.CreatePage(newPage(space.Id, channelID, userID, ""), testDefaultMaxDepth)
+		require.NoError(t, err)
+
+		d := newDraft(userID, space.Id, page.Id, "")
+		d.BaseEditAt = page.EditAt
+		saved, _, err := s.UpsertDraft(d, nil, nil, nil)
+		require.NoError(t, err)
+
+		require.NoError(t, s.DeleteSpace(space.Id))
+
+		_, err = s.GetDraft(userID, page.Id)
+		require.True(t, store.IsErrNotFound(err), "the draft must be hidden while its space is soft-deleted")
+
+		require.NoError(t, s.RestoreSpace(space.Id))
+
+		got, err := s.GetDraft(userID, page.Id)
+		require.NoError(t, err, "the draft must survive the space delete+restore round trip")
+		require.Equal(t, saved.Body, got.Body)
+		require.Equal(t, saved.UpdateAt, got.UpdateAt)
+	})
+
 	t.Run("drafts for space excludes a draft whose page lives in another space", func(t *testing.T) {
 		s := openTestDB(t)
 		userID := mmmodel.NewId()
@@ -687,6 +718,207 @@ func TestDeletePageReparentsPendingDrafts(t *testing.T) {
 	require.NoError(t, err, "draft's reparented parent must be a valid live parent")
 }
 
+// TestDeletePagePreservesEditDraftAndReparentsChildDraft verifies the two draft cascades DeletePage
+// runs stay isolated when both apply to the same delete: the deleted page's own edit draft (keyed
+// PageId = pageID) is preserved-but-hidden, while a pending new-page draft parented under that page
+// (ParentId = pageID) is reparented to the deleted page's parent. reparentDraftsForPage matches only
+// on ParentId, never PageId, which is why it cannot touch the edit draft row.
+func TestDeletePagePreservesEditDraftAndReparentsChildDraft(t *testing.T) {
+	s := openTestDB(t)
+	channelID := mmmodel.NewId()
+	space, err := s.CreateSpace(newSpace(channelID))
+	require.NoError(t, err)
+	userID := mmmodel.NewId()
+
+	grandparent, err := s.CreatePage(newPage(space.Id, channelID, userID, ""), testDefaultMaxDepth)
+	require.NoError(t, err)
+	page, err := s.CreatePage(newPage(space.Id, channelID, userID, grandparent.Id), testDefaultMaxDepth)
+	require.NoError(t, err)
+
+	editDraft := newDraft(userID, space.Id, page.Id, "")
+	editDraft.BaseEditAt = page.EditAt
+	_, _, err = s.UpsertDraft(editDraft, nil, nil, nil)
+	require.NoError(t, err)
+
+	childPageID := mmmodel.NewId()
+	parentID := page.Id
+	_, _, err = s.UpsertDraft(newDraft(userID, space.Id, childPageID, parentID), &parentID, nil, nil)
+	require.NoError(t, err)
+
+	require.NoError(t, deletePageErr(s, page.Id, space.Id, userID))
+
+	_, err = s.GetDraft(userID, page.Id)
+	require.True(t, store.IsErrNotFound(err), "the edit draft must be hidden while its page is deleted")
+
+	child, err := s.GetDraft(userID, childPageID)
+	require.NoError(t, err, "the pending child draft must survive as a reparented new-page draft")
+	require.Equal(t, grandparent.Id, child.ParentId, "child draft must be reparented to the deleted page's parent")
+
+	// Restore brings the edit draft back; the child's reparent is a structural rewrite that
+	// RestorePage does not undo, since RestorePage never touches draft rows. Full field parity
+	// across the round trip is pinned by TestDeletePage in store_test.go.
+	_, err = s.RestorePage(page.Id, space.Id, userID, testDefaultMaxDepth)
+	require.NoError(t, err)
+
+	_, err = s.GetDraft(userID, page.Id)
+	require.NoError(t, err, "the edit draft must reappear after restore")
+
+	stillReparented, err := s.GetDraft(userID, childPageID)
+	require.NoError(t, err)
+	require.Equal(t, grandparent.Id, stillReparented.ParentId, "the child draft's reparent survives the page's own restore")
+}
+
+// TestUpsertDraftQuotaCountsDraftsHiddenByPageDelete pins the quota consequence of preserving a
+// draft whose page was deleted: countDraftsForUser has no liveness join, so the hidden row still
+// consumes a slot even though no read path lists it. This bounds total row growth, but it also means
+// the owner can be refused a new draft while their visible listing is short of the cap.
+func TestUpsertDraftQuotaCountsDraftsHiddenByPageDelete(t *testing.T) {
+	s := openTestDB(t)
+	channelID := mmmodel.NewId()
+	space, err := s.CreateSpace(newSpace(channelID))
+	require.NoError(t, err)
+	userID := mmmodel.NewId()
+
+	page, err := s.CreatePage(newPage(space.Id, channelID, userID, ""), testDefaultMaxDepth)
+	require.NoError(t, err)
+	d := newDraft(userID, space.Id, page.Id, "")
+	d.BaseEditAt = page.EditAt
+	_, _, err = s.UpsertDraft(d, nil, nil, nil)
+	require.NoError(t, err)
+
+	require.NoError(t, deletePageErr(s, page.Id, space.Id, userID))
+
+	// One hidden draft exists; fill the rest of the quota with visible new-page drafts.
+	for range model.MaxDraftsPerUserPerSpace - 1 {
+		_, _, err = s.UpsertDraft(newDraft(userID, space.Id, mmmodel.NewId(), ""), nil, nil, nil)
+		require.NoError(t, err)
+	}
+
+	visible, err := s.GetDraftsForSpace(userID, space.Id, 0, testDraftListLimit)
+	require.NoError(t, err)
+	require.Len(t, visible, model.MaxDraftsPerUserPerSpace-1, "the page-deleted draft stays excluded from the visible listing")
+
+	// The store holds the full quota (1 hidden + Max-1 visible), so one more upsert must be refused.
+	// A liveness-filtered count would wrongly allow it, since only Max-1 drafts are visible.
+	_, _, err = s.UpsertDraft(newDraft(userID, space.Id, mmmodel.NewId(), ""), nil, nil, nil)
+	require.True(t, store.IsErrLimitExceeded(err), "expected the hidden draft to count against the quota, got %T: %v", err, err)
+}
+
+// TestRestoreSpaceLeavesIndividuallyDeletedPageDraftHidden carries RestoreSpace's stamp-scoped
+// un-cascade through to draft visibility: RestoreSpace only revives pages carrying the space's own
+// DeleteAt stamp, so a page deleted individually beforehand keeps its earlier stamp and stays
+// deleted — and its draft stays hidden even though the space is live again.
+func TestRestoreSpaceLeavesIndividuallyDeletedPageDraftHidden(t *testing.T) {
+	s := openTestDB(t)
+	channelID := mmmodel.NewId()
+	space, err := s.CreateSpace(newSpace(channelID))
+	require.NoError(t, err)
+	userID := mmmodel.NewId()
+
+	individual, err := s.CreatePage(newPage(space.Id, channelID, userID, ""), testDefaultMaxDepth)
+	require.NoError(t, err)
+	cascaded, err := s.CreatePage(newPage(space.Id, channelID, userID, ""), testDefaultMaxDepth)
+	require.NoError(t, err)
+
+	dIndividual := newDraft(userID, space.Id, individual.Id, "")
+	dIndividual.BaseEditAt = individual.EditAt
+	_, _, err = s.UpsertDraft(dIndividual, nil, nil, nil)
+	require.NoError(t, err)
+
+	dCascaded := newDraft(userID, space.Id, cascaded.Id, "")
+	dCascaded.BaseEditAt = cascaded.EditAt
+	_, _, err = s.UpsertDraft(dCascaded, nil, nil, nil)
+	require.NoError(t, err)
+
+	// Delete one page first so its stamp predates the space's cascade stamp, which DeleteSpace
+	// computes to be strictly greater than any existing page DeleteAt.
+	require.NoError(t, deletePageErr(s, individual.Id, space.Id, userID))
+
+	require.NoError(t, s.DeleteSpace(space.Id))
+	require.NoError(t, s.RestoreSpace(space.Id))
+
+	_, err = s.GetDraft(userID, cascaded.Id)
+	require.NoError(t, err, "a draft on a page the space delete cascaded must reappear after RestoreSpace")
+
+	_, err = s.GetPage(individual.Id, false)
+	require.True(t, store.IsErrNotFound(err), "individually-deleted page must stay deleted after RestoreSpace")
+	_, err = s.GetDraft(userID, individual.Id)
+	require.True(t, store.IsErrNotFound(err), "its draft must stay hidden after RestoreSpace")
+}
+
+// TestDiscardPathsReachDraftHiddenByPageDelete verifies the explicit discard paths are not gated by
+// applyDraftLivenessFilter the way the read paths are, so a draft the reads currently hide is still
+// discardable. This is the only route by which an owner reclaims a hidden draft's quota slot, and it
+// requires knowing the page id — no read path will surface it.
+func TestDiscardPathsReachDraftHiddenByPageDelete(t *testing.T) {
+	t.Run("DeleteDraftVersion discards a draft hidden by its page's soft-delete", func(t *testing.T) {
+		s := openTestDB(t)
+		channelID := mmmodel.NewId()
+		space, err := s.CreateSpace(newSpace(channelID))
+		require.NoError(t, err)
+		userID := mmmodel.NewId()
+		page, err := s.CreatePage(newPage(space.Id, channelID, userID, ""), testDefaultMaxDepth)
+		require.NoError(t, err)
+
+		d := newDraft(userID, space.Id, page.Id, "")
+		d.BaseEditAt = page.EditAt
+		saved, _, err := s.UpsertDraft(d, nil, nil, nil)
+		require.NoError(t, err)
+
+		require.NoError(t, deletePageErr(s, page.Id, space.Id, userID))
+		_, err = s.GetDraft(userID, page.Id)
+		require.True(t, store.IsErrNotFound(err), "the draft must be hidden while its page is deleted")
+
+		discarded, delErr := s.DeleteDraftVersion(userID, page.Id, saved.UpdateAt)
+		require.NoError(t, delErr)
+		require.True(t, discarded, "the CAS delete must match the hidden row")
+
+		// Restoring proves the row is gone rather than merely hidden: it does not come back.
+		_, err = s.RestorePage(page.Id, space.Id, userID, testDefaultMaxDepth)
+		require.NoError(t, err)
+		_, err = s.GetDraft(userID, page.Id)
+		require.True(t, store.IsErrNotFound(err), "the discarded draft must not reappear after restore")
+	})
+
+	t.Run("DeleteDraftReparenting discards a draft hidden by its page's soft-delete", func(t *testing.T) {
+		s := openTestDB(t)
+		channelID := mmmodel.NewId()
+		space, err := s.CreateSpace(newSpace(channelID))
+		require.NoError(t, err)
+		userID := mmmodel.NewId()
+		page, err := s.CreatePage(newPage(space.Id, channelID, userID, ""), testDefaultMaxDepth)
+		require.NoError(t, err)
+
+		d := newDraft(userID, space.Id, page.Id, "")
+		d.BaseEditAt = page.EditAt
+		_, _, err = s.UpsertDraft(d, nil, nil, nil)
+		require.NoError(t, err)
+
+		childPageID := mmmodel.NewId()
+		parentID := page.Id
+		_, _, err = s.UpsertDraft(newDraft(userID, space.Id, childPageID, parentID), &parentID, nil, nil)
+		require.NoError(t, err)
+
+		require.NoError(t, deletePageErr(s, page.Id, space.Id, userID))
+
+		// pageExistsInSpace reports false for a soft-deleted page, so the discard reads the page as
+		// not-live and takes the new-page reparent branch. That branch is a no-op here: DeletePage's
+		// own reparentDraftsForPage already moved every ParentId pointing at this page, so nothing
+		// still matches. The return value is what carries the consequence — the caller uses it to pick
+		// the presence-broadcast audience.
+		pageWasLive, delErr := s.DeleteDraftReparenting(userID, space.Id, page.Id)
+		require.NoError(t, delErr)
+		require.False(t, pageWasLive, "a soft-deleted page must read as not-live to the discard path")
+
+		_, err = s.GetDraft(userID, page.Id)
+		require.True(t, store.IsErrNotFound(err), "the discarded edit draft must be gone")
+
+		child, err := s.GetDraft(userID, childPageID)
+		require.NoError(t, err, "the child draft must be untouched by the sibling discard")
+		require.Equal(t, "", child.ParentId, "the child was already reparented to root by the page delete")
+	})
+}
+
 // TestGetActiveEditorsForPage covers the presence window predicate: a draft updated at/after the
 // cutoff counts its user as active; one before the cutoff, or on another page, does not.
 func TestGetActiveEditorsForPage(t *testing.T) {

server/store/migrations/000003_create_drafts.up.sql

@@ -16,5 +16,7 @@ CREATE TABLE IF NOT EXISTS DOCS_Draft (
 -- UpdateAt DESC; the trailing column lets the index satisfy the sort, no filesort.
 CREATE INDEX IF NOT EXISTS idx_docs_draft_user_space ON DOCS_Draft (UserId, SpaceId, UpdateAt DESC);
 
--- Purge-by-page: supports DELETE FROM DOCS_Draft WHERE PageId = ?, not covered by the primary key.
+-- Lookups keyed on PageId without a UserId: the page's active-editor presence snapshot, and the
+-- recursive parent-chain walk that rejects draft cycles. The primary key leads with UserId, so it
+-- cannot serve either.
 CREATE INDEX IF NOT EXISTS idx_docs_draft_pageid ON DOCS_Draft (PageId);

server/store/page_store.go

@@ -434,12 +434,16 @@ func (s *Store) DeletePage(pageID, spaceID, userID string) (_ string, err error)
 		return "", rowsErr
 	}
 
-	// A draft is unpublished work on the page, so deleting the page ends its life; a new-page
-	// draft parented under this page is a pending child of it, so it is reparented rather than
-	// deleted (see reparentDraftsForPage). Both cascades run inside this transaction.
-	if draftErr := s.deleteDraftsForPage(tx, pageID, spaceID); draftErr != nil {
-		return "", draftErr
-	}
+	// An edit draft on this page is preserved, not destroyed, and hidden meanwhile by
+	// applyDraftLivenessFilter (which documents why). Only a draft whose ParentId is this page is
+	// rewritten: a pending child is reparented to the deleted page's parent rather than left
+	// dangling.
+	//
+	// Two consequences of preserving the row. First, countDraftsForUser is unfiltered, so a hidden
+	// draft still consumes one of its owner's MaxDraftsPerUserPerSpace slots: that bounds total row
+	// growth, but it also lets an owner reach the quota holding drafts no read path will list back
+	// to them. Second, its write-once BaseEditAt falls behind the EditAt bumped by this delete and
+	// by the restore, so publishing it after a restore takes the conflict path unless forced.
 	if draftErr := s.reparentDraftsForPage(tx, pageID, deleted.ParentID, spaceID, now); draftErr != nil {
 		return "", draftErr
 	}

server/store/space_store.go

@@ -256,8 +256,7 @@ func (s *Store) DeleteSpace(spaceID string) (err error) {
 	}
 
 	// Drafts are left untouched: this soft-delete is reversible (RestoreSpace), so a user's
-	// in-progress work must survive the round trip. Drafts have no DeleteAt, so they are purged
-	// only when their page is explicitly deleted.
+	// in-progress work must survive the round trip, hidden meanwhile by applyDraftLivenessFilter.
 	if err = tx.Commit(); err != nil {
 		return errors.Wrap(err, "commit_transaction")
 	}