Draft and publish: staged edits to published records - #109
Merged
Conversation
Records a draft/publish design so an in-progress edit (typically an unfinished translation) no longer forces the whole record out of public view. Live rows are never touched by editing; a pending-changes table holds the working copy and Publish replays it through the normal update path. Records why PaperTrail is not the storage mechanism, despite looking like a fit: versions.object holds pre-change state, recordHistory ships in production and would fill with edits that never happened, and the payload is unqueryable YAML. PaperTrail is used for conflict detection instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnYLUscETuPwsQzU9nCSxu
paper_trail-association_tracking looks like the way to bring relations into drafts. It is not: drafts live in record_drafts.data, not in PaperTrail. The gem also recommends against itself, breaks on transactional tests and STI (both of which we have), and duplicates VersionedUnderRoot, which already surfaces relation, common-name and image changes in recordHistory. Staging relations is deferred rather than rejected, with the reasoning for why that is not a one-way door. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnYLUscETuPwsQzU9nCSxu
Eight tasks: the record_drafts table, a single-source draftable whitelist, the in-memory overlay, saveAsDraft on the four update mutations, the perspective lens, conflict detection, publish/discard, and the hasPendingChanges filter. Task 2 also fixes a pre-existing bug it would otherwise inherit: ChangeHistory::Restorer's whitelist never gained family_id when botanical families landed, so restoring a version silently leaves the family assignment untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnYLUscETuPwsQzU9nCSxu
Adds the storage table and model that every later draft/publish task
builds on: draftable (polymorphic), jsonb data, base_updated_at, and
author/last_editor principal ids, with a DB-enforced one-draft-per-record
unique index.
RecordDraft opts out of the ApplicationRecord-wide has_paper_trail by
overriding the inherited paper_trail_options with an always-true :unless
guard, since paper_trail 17's has_paper_trail on: [] raises ("must be
called only once") once a subclass already inherits
PaperTrail::Model::InstanceMethods, and paper_trail.disable is
request-scoped rather than a permanent class-level switch.
Implements Drafts::ConflictDetector to detect fields changed on the live record after a draft was created, using PaperTrail's immutable version audit trail. Queries versions strictly after base_updated_at to exclude the snapshot moment itself, then intersects with draft.changed_fields for field-level detection (not timestamp-level, which fires constantly on SourceSynchronizer runs and trains editors to dismiss warnings). Includes a dedicated test for RecordDraft#changed_fields to pin the method the detector depends on. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnYLUscETuPwsQzU9nCSxu
Adds a hasPendingChanges: Boolean option to the plants, varieties, categories, and families collection resolvers. Filters via a correlated EXISTS/NOT EXISTS subquery against record_drafts (keyed on draftable_type + draftable_id) rather than joining, so the filter cannot change row multiplicity even if the one-draft-per-record uniqueness constraint is ever relaxed for named drafts. value.nil? is checked explicitly: search_object still passes an argument bound to an explicit-null GraphQL variable through as `nil` (it only omits params for arguments never supplied at all), so skipping the nil guard would have silently treated "$v: null" the same as "$v: false" and excluded every draft-bearing record from the unfiltered case. The N+1 guard for a future per-row draft field (task 8 step 5) is deliberately not implemented here: the codebase has no established batch-loading primitive (no GraphQL::Dataloader, graphql-batch, or BatchLoader gem; the prior perf-plants-nplusone work solved its N+1 via plain ActiveRecord includes: on known static associations, which does not generalize to a keyed lookup for a field that does not exist yet). See task-8-report.md for detail; task 5's implementer should pick the batching approach once the draft field itself is designed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnYLUscETuPwsQzU9nCSxu
Applies a draft's staged values to a loaded record without saving, so a resolver can serialize the draft view of a record straight from the dirty instance. The Mobility container column needs two things a plain attribute write does not: a deep merge (a draft stages only the locales an editor touched, so assigning the blob wholesale would blank every other locale), and a drop of Mobility's per-locale read cache (memoised on the backend instance and only cleared by reload/changes_applied, so a read that happened before the overlay would otherwise pin the pre-draft value for the rest of the request).
Adds Mutations::Concerns::DraftWriting, included by UpdatePlant, UpdateVariety, UpdateFamily and UpdateCategory. When saveAsDraft is true, the mutation stages the supplied attributes onto the record's RecordDraft (creating it on first use) instead of writing the live row, and leaves no PaperTrail version. Translatable fields are staged under a translations blob keyed by locale, merged against the record's current translations (or the draft's own if one already exists) so other locales are never clobbered. base_updated_at is set only on draft creation. Omitting saveAsDraft is unchanged: mobile and importers keep writing live directly.
UpdatePlant's familyId argument uses loads:, so graphql-ruby hands resolve a loaded Family record (or explicit nil) under :family, never a literal family_id string. The generic permitted-keys slice in stageable_data could therefore never see it, silently dropping familyId + saveAsDraft on the floor. Adds DraftWriting#stage_family_id, guarded on the whitelist so it only fires for models that draft family_id, mirroring the attributes.key?(:family) idiom FamilyAssignment#apply_family already uses so an explicit familyId: null stages a clear (family_id => nil) rather than being skipped as absent. Per docs/superpowers/specs/2026-08-07-draft-publish-design.md: plants.family_id is an ordinary column and must be staged.
Review found the empty-container guard was testing the wrong value, and the
mechanism behind it was misdiagnosed.
Type::Serialized inherits the json subtype's mutable-value cast
(deserialize(serialize(value))), so Container::Coder's presence-stripping runs
at ASSIGNMENT time, not at save time. A draft clearing the last surviving
translated field assigns {"en" => {"description" => nil}} and leaves the record
holding exactly {}, which the writer turns into SQL NULL against a NOT NULL
column: the next save! raises NotNullViolation. The old guard inspected the
merged hash before assignment, where the emptiness is not yet visible, and its
comment wrongly called the branch unreachable.
The check now runs on the post-assignment container, so it cannot drift from
the coder's presence rules, and the staged locales are re-seeded in place -- the
same operation the backend's own reader performs -- so the clear is honoured and
the container persists as the empty default rather than as NULL.
Also drops a staged locale whose value is not a hash: Container::Coder#dump
calls #each on it and would raise mid-response.
Adds the Perspective enum (PUBLISHED default, DRAFT) as an argument on the four single-record queries (plant, variety, category, family), wired through a shared apply_perspective helper on QueryType that overlays a draft only when the caller may edit the record -- otherwise it silently falls back to published content rather than erroring, since perspective is a view preference and 403ing would break ordinary anonymous/read-only page loads. Adds the `draft` field (RecordDraftInfoType, via Types::Concerns::DraftFields) to PlantType, VarietyType, FamilyType and CategoryType, gated per-object by Pundit update? so an editor never sees another org's draft metadata. is_stale wires directly to Drafts::ConflictDetector (Task 6), no stub. Since `draft` is a plain type field it is reachable through the plants, varieties, categories and families list resolvers, not just the four single-record queries. Added :record_draft to each resolver's eager-load includes to avoid a per-row N+1, proven by a query-count spec against a 20-row list. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnYLUscETuPwsQzU9nCSxu
Drafts::Publisher takes the row lock, re-checks the conflict authoritatively inside it, applies the draft through Drafts::Overlay, reproduces the FamilyAssignment family_names mirror, flips publication_state on a first publish, saves ONCE (one honest PaperTrail version) and only then destroys the draft. A failed publish keeps the draft. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnYLUscETuPwsQzU9nCSxu
CI gates schema drift (graphql:schema:dump + git diff --exit-code). The dump had not been regenerated since the saveAsDraft and hasPendingChanges waves merged, so this carries their additions alongside publishDraft and discardDraft. Insertions only; nothing removed or reshaped. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnYLUscETuPwsQzU9nCSxu
The wave-D branches regenerated schema.graphql independently; the sdd/t7 dump predates sdd/t5's perspective argument and draft field. This is the dump of the merged tree, which is what the CI drift gate checks. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnYLUscETuPwsQzU9nCSxu
Addresses all findings from the final draft/publish review: - Permission-gate hasPendingChanges on plants/varieties/categories/families: callers without write access get scope.none for true rather than an existence oracle over other people's drafts (I2), with spec coverage for anonymous and readonly callers (M6 also fixes the dead trust_level: param in favor of the :superadmin factory trait). - Seed the draft translations blob with only staged locales/fields, not a full live snapshot, since Overlay/Publisher deep-merge onto live anyway (I3), with a new Family saveAsDraft spec covering the mobility_attributes path outside Plant (M8). - Warn that isStale queries the audit trail per-record (I1). - Derive PublishDraft/DiscardDraft's DRAFTABLE_TYPES from DraftableAttributes::BY_MODEL instead of a hand-copied list (M5). - Correct PublishDraftPayload#record's description for the validation-failure case, not just conflict refusal (M2), and document that saveAsDraft ignores workflow/non-column arguments (M3). - Narrow ConflictDetector's rescue to per-version so one undeserializable changeset doesn't blank the whole conflict result (M7). - Correct the design doc's "replays through the normal update path" claims to describe the actual Overlay + explicit family-mirror mechanism (M1), and add a corrections note to the implementation plan (M9). Full suite (2304 examples), RuboCop, and schema drift are all clean; schema.graphql re-dumped for the description-string changes.
Contributor
Author
|
Pre-deploy gate from the review: CLEARED (2026-08-07). Ran a read-only one-off Fargate task in the production cluster (task
The NULL-trio demotion path ( 🤖 Generated with Claude Code |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements the design in #108 (whose branch this includes): an editor can stage
changes to a published record and the current version stays live until they
publish. The originating case — a half-finished translation no longer forces
the whole record out of public view — works end-to-end.
Surface
record_drafts: one polymorphic working-copy row per record (unique index),deliberately excluded from PaperTrail.
perspective: PUBLISHED | DRAFTon the four single-record queries. DefaultPUBLISHED; DRAFT requires
update?and degrades to published contentotherwise. Anonymous and mobile callers structurally cannot receive draft
content — they never pass the argument.
draft { updatedAt author lastEditor changedFields isStale }on Plant,Variety, Family, Category; null unless the caller may edit.
saveAsDraft: Booleanon the four update mutations — same arguments, sameauthorization, live row untouched.
publishDraft(recordId, accessLevel, force)/discardDraft(recordId).Publish runs inside
with_lock: authoritative conflict re-check, in-memoryoverlay, family-names mirror, publication-state flip, one save (= exactly one
PaperTrail version), draft destroyed. Validation failure returns payload
errors and keeps the draft.
draft's base, intersected with the draft's fields. Field-level on purpose —
a coarse timestamp check would fire on every SourceSynchronizer run.
hasPendingChangesfilter on the four list resolvers, permission-gated.Also in this PR
ChangeHistory::Restorernever gainedfamily_idwhenbotanical families landed, so restoring a version silently skipped the
family assignment. Fixed via a single shared whitelist
(
DraftableAttributes) that Restorer and the draft system both consume —the two can no longer drift. Regression spec included.
docs/superpowers/,including a "corrections applied during execution" record.
Verification
spec/contracts/(the frozen mobile contract) green and untouched throughoutschema.graphqlregenerated on the merged tree)enumerated; draft content reaches only callers holding
update?Deploy gate — read before shipping
Production must be verified to have zero NULL
publication_state/access_levelrows across the five owned models before deploy.rake ownership:verifyexplicitly skips NULL rows, so a green verify is notevidence. Publishing with an access level on a NULL-trio legacy-public record
would demote it to private via
VisibilityBridge's else-branch. Userake ownership:report(nil bucket) as a one-off task against prod.Known deferred items
Tracked for follow-up, none load-bearing:
isStaleis per-record expensive(batching follow-up; field description warns); the draft row itself is not
locked during publish (ms-window, worst case one lost draft edit); no
unattributable-principal guard in draft writes (established degraded-DB path);
DraftableAttributes::MODEL_NAMESrefactor.🤖 Generated with Claude Code
https://claude.ai/code/session_01RnYLUscETuPwsQzU9nCSxu