| Age | Commit message (Collapse) | Author |
|---|
|
node_id/page_id/user_id are lookup and ordering only -- all three
nullify on delete, so an entry outlives its actor and its subject.
Everything that must survive those deletions lives in a mandatory
metadata jsonb written once at creation: the actor's username, the
node's human-readable name (pinned to the default locale), and
action-specific extras such as publish's title from/to.
NodeAction.record! is the single constructor, so every entry gets
the same baseline metadata without each call site re-implementing
it. occurred_at is one field for live and backfilled entries alike;
inferred_from distinguishes them -- nil means witnessed at the
moment it happened, populated names how a backfilled entry was
estimated.
Instrumented so far: publish (crediting the actual publisher,
threaded through from the controller -- previously nobody had the
act of publishing recorded anywhere), revert's discard_autosave and
destroy_draft branches, and translation destroy. publish_draft! now
runs in a transaction so the promotion and its log entry land
together. The remaining verbs follow once this mechanism has proven
itself.
|
|
Both had already lost their reason to exist as production API:
wipe_draft!'s one remaining callsite (nodes#show) was removed two
sessions ago, and find_or_create_draft had zero production callers
left at all -- confirmed by a fresh grep, not assumed -- every one of
its ~65 call sites was test setup, unrelated to what those tests
actually cover.
wipe_draft! is deleted outright, along with its two tests -- the
lock/draft/autosave cleanup it silently performed already has
explicit, always-visible manual equivalents (Unlock, Discard
Autosave, Destroy Draft), so nothing real is lost.
find_or_create_draft moves to test_helper.rb as a plain method on
ActiveSupport::TestCase, alongside the create_node_with_draft/
create_node_with_published_page helpers already living there --
extending the framework's own designated test-extension point
rather than reopening the Node model from test code. Its three
tests of real dispatch behavior (idempotency on repeat calls, and
raising when a second user contends for the lock) are kept, since
~65 other tests depend on this helper actually working correctly;
only the call syntax changed, from node.find_or_create_draft(user)
to find_or_create_draft(node, user).
|
|
Node#autosave! only copied assets forward onto a newly created
autosave, not translations. Invisible for single-locale nodes, but a
node with an existing multi-locale head/draft that gets locked and
edited in one locale would silently drop every other locale's
translation from the new autosave -- and from there into the draft
and eventually head, since save_draft! faithfully clones whatever
the autosave actually holds.
Fixed by using clone_attributes_from, which already does a complete
clone (translations, assets, tags, template, published_at) -- the
same method save_draft! already relies on two branches later, so no
new mechanism, just closing the one place still doing a partial,
hand-rolled version of it.
|
|
Replaces nodes#edit's old Images section -- a hidden panel dumping
every image asset in the system unfiltered (#image_browser) plus a
raw drag-and-drop box (#image_box) -- with a small search-and-click
picker built on the endpoint from the last two commits. Attaching
posts immediately and appends the new thumbnail via a cloned
<template> -- icons only render correctly through the Rails helper
server-side, so the template holds real, pre-rendered markup for JS
to clone rather than duplicating raw SVG in a JS string. Reordering is
jQuery UI sortable on the small attached list only, with a dedicated
drag handle rather than the whole thumbnail.
Two bugs caught while click-testing, fixed here rather than shipped
and patched after: the search panel never closed after attaching an
image, since the success handler re-triggered focus to keep it open
for attaching several in a row -- which meant it just re-populated
itself forever instead of signaling "done." Fixed to close explicitly;
a click-outside-closes handler was added alongside it, matching the
affordance the top-bar search already has.
A real, independent, pre-existing data bug surfaced during the same
testing: Node#autosave!'s first-time-creation branch never carried
related assets forward from whatever page was previously current --
attach an image, let autosave fire once, and it silently landed on a
fresh, assetless Page row. Long-dormant, not introduced by this work,
just finally exercised by something that made it visible. Fixed inside
the `unless self.autosave` guard specifically -- running this on every
call, not just creation, would overwrite anything attached directly to
an existing autosave in between, a worse bug than the one being fixed.
nodes#show gains a read-only Images section, rendered only when a page
actually has attached images, so an attachment can be confirmed
present without entering the edit/lock cycle -- useful on its own, and
specifically useful the next time an asset bug needs investigating.
Its thumbnail CSS is shared with the edit view's picker via a class
(.thumbnail_list) rather than duplicated under a second name.
|
|
Replaces the old admin#index wizard -- accreted over years, never
designed as a whole -- with the dashboard settled on this session:
a three-icon nav (dashboard/search/log out, no locale selector), a
nodes-first search bar, four task signposts, and two symmetric
widgets (drafts/autosaves, recent changes) with a quiet housekeeping
row beneath them.
Node.recently_changed now filters and orders by the head page's own
updated_at instead of the node's blanket timestamp, so a lock/unlock
cycle with no actual publish no longer surfaces here, and the
original publisher is no longer misattributed to someone else's
housekeeping action. This also restores the "published" qualifier on
each entry, which the query previously couldn't guarantee was true.
@mynodes and its dedicated "My Work" table are retired along with the
old wizard -- "Continue my work" is a link to the existing, already-
correct NodesController#mine instead of a second, duplicate query.
Its one dedicated test (dedup across multiple revisions by the same
user) is ported to nodes_controller_test.rb, since mine already
carries the same .distinct protection the old query did; it just had
no test of its own until now.
|
|
Node.drafts_and_autosaves and Node.recently_changed replace inline
query logic in NodesController#drafts/#recent -- pure refactor, no
behavior change for either action. The real reason: the dashboard's
upcoming abridged widgets need the exact same queries the full pages
already use, just limited and (for drafts) sorted with the current
user's own locked nodes first. Better to share one method than let a
widget and a full page quietly drift onto two versions of "what counts
as a current draft."
current_user_id: is an explicit, optional argument rather than a
separate method -- ordering by lock ownership only applies when a user
is given; omitted entirely, it's pure recency, exactly today's
behavior. Needed Arel.sql wrapping a sanitized CASE expression before
.order would accept it -- sanitize_sql_array only vouches for the
values inside the string, a separate Rails safety check still refuses
any string shaped like more than a plain column reference unless it's
explicitly marked as already-vetted.
|
|
Four NodesController actions -- drafts, recent, mine, chapters --
each building its own base scope, sharing one private method
(index_matching) for search narrowing and pagination. Wizard rewrite
to link into these instead of rendering its own tables is a separate,
later step.
Node.editor_search backs the shared "q" narrowing: an ILIKE substring
match against title/abstract on whichever of head or draft is
present, splitting the term on whitespace and requiring every word to
match somewhere independently, not as one phrase, since real words can
end up separated by markup in the underlying HTML. Deliberately
separate from Node.search, the public content search, which stays
tsvector-based and head-only.
chapters generalizes into /admin/nodes/tags/:tags for an arbitrary
tag list (OR'd, not AND'd), sharing the controller action but
rendering its own template rather than branching inside one view.
|
|
publish_draft! called move_to_child_of before validating the new
parent at all. Staging a node under one of its own descendants
created a real, if transient, cycle in parent_id the instant that
save landed -- and update_unique_names_of_children's after_save
callback, which recurses down through parent_id with no cycle check
and no depth limit, bounced between the two nodes forever, crashing
the whole process with a SystemStackError rather than just producing
bad data.
Now rejected up front with a normal ActiveRecord::RecordInvalid.
|
|
Both ran real code and checked nothing -- Minitest's "missing
assertions" warning has been firing on every run this session.
test_find_or_create_draft_if_draft_exists_and_is_owned_by_user called
the method twice but never checked what came back; now confirms the
second call returns the same draft rather than creating a new one,
and leaves the lock and page count untouched.
test_destroy_a_published_node destroyed a node and loaded the admin
index but never checked the response, unlike its two neighbors
covering the same destroy path (dangling pages, orphaned
occurrences). Added the assert_response :success its own shape
already implied.
No behavior changed -- both were passing before for the same reason
they're passing now, they just weren't proving anything.
|
|
Node#resolve_page_reference and #available_layer_pairs let
Page#diff_against compare named layers (head/draft/autosave), not
just numbered revisions -- autosave was never part of Node#pages, so
this was the missing piece.
Wired into nodes#show's Status section, nodes#edit right after an
autosave gets resurrected ("What changed?"), and the admin wizard's
current-drafts table, which now also lists autosave-only nodes it
previously never showed.
revisions#diff hides the numbered-revision picker when comparing
named layers (it can't represent them), shows a plain label instead,
and offers buttons to switch between whichever other pairs make
sense for the node's current state. Destroying the topmost layer is
available directly from the diff view, reusing the existing revert!
path.
"Discard changes" is renamed "Discard Autosave" everywhere it
appears, to match "Destroy Draft".
|
|
revert! discards exactly the topmost non-empty layer -- autosave if
present, else draft -- and reveals whatever's beneath it, releasing the
lock only once nothing is left to protect. Guards against destroying a
brand-new, never-published node's only draft, which would violate the
(head | draft) invariant every other method here assumes holds; the
view's Destroy/Discard button is gated the same way.
nodes#edit now calls lock_for_editing! instead of find_or_create_draft,
and always displays autosave || draft || head, resurrecting an
abandoned session's unsaved content by default with an explicit flash
explaining what's shown and how to get back to the last saved version.
The view drops content_for :subnavigation entirely: Show becomes
"Unlock + Back", Preview stays a plain link, metadata's own <details>
already replaced the old toggle, and Publish moves off this page for
good, per the earlier decision to manage the publish lifecycle
entirely from nodes#show. Save Draft and Save + Unlock + Exit appear
both above and below the form, given the body field alone runs 600px
on desktop.
|
|
Two real bugs surfaced by the full test suite, not by the new tests
written for this feature -- both were latent the moment lock_for_editing!
and save_draft! replaced find_or_create_draft, and only visible once an
existing test exercised the exact path each one lived in.
lock_for_editing! never stamped user/editor onto a draft that already
existed when the lock was acquired. find_or_create_draft used to do this
as part of acquiring the lock; splitting lock acquisition from draft
creation dropped it entirely, since it looked like draft-creation logic
rather than locking logic. Restored as an explicit step: claim authorship
only if none is set yet, editorship unconditionally, matching the old
behavior exactly.
save_draft!'s "no draft yet" branch set user/editor before calling
clone_attributes_from, whose first line is an unconditional self.reload
-- silently discarding both, since neither had been persisted yet.
clone_attributes_from also always copies published_at from its source
without the ||= guard used for template_name, and the source here is
always the autosave, whose published_at is never anything but nil --
meaning every single promotion, not just the first, was quietly
resetting a published page's published_at, which publish_draft!'s own
||= Time.now would then treat as never-published and re-stamp. Fixed by
running clone_attributes_from first on both branches, then applying
user/editor/published_at afterward, exactly as the existing-draft branch
already happened to do by accident.
Four controller tests updated to insert a real put :update between
get :edit and assertions that used to be true immediately after
visiting edit -- deferred draft creation means edit alone no longer
produces one, which is the intended consequence of this whole
redesign, not something these tests were meant to catch.
|
|
Introduces autosave_id as a third, unversioned layer above draft/head,
with lock_for_editing!, autosave!, and save_draft! as the new entry
points. Also fixes a real bug in wipe_draft!: its "no draft" branch
unconditionally released the lock, which was safe when "no draft" only
ever meant "nothing is happening" — no longer true now that a lock can
exist with only an autosave beneath it. lock_for_editing! deliberately
does not call wipe_draft! at all, for the same reason: an intruder
calling it while a lock was genuinely held would otherwise silently
steal it via wipe_draft!'s own unlock side effect, caught by the new
two-user lock test.
|
|
- Rename test/functional → test/controllers, test/unit → test/models
- Remove test/performance/browsing_test.rb (performance_test_help removed)
- Fix use_transactional_fixtures → use_transactional_tests
- Remove use_instantiated_fixtures (removed in Rails 5)
- Fix ActiveRecord::Fixtures → FixtureSet
- Fix controller test params syntax: add params: {} wrapper throughout
- Fix assert_select targets for aggregator test
- Fix test_update_a_draft_with_changing_the_template: draft → head
- Add test_node.reload after children.create! (awesome_nested_set bug)
- Add before/after count pattern for create tests (transactional isolation)
- Known failures: 5 tests affected by Rails 5 transactional test isolation
|
