From 206dc5c50a73c5402b90d7fdc8945d3ba9356758 Mon Sep 17 00:00:00 2001 From: erdgeist Date: Sat, 4 Jul 2026 01:24:55 +0200 Subject: Add self-service event creation from nodes#show nodes#show's events table now renders unconditionally (previously hidden entirely for a zero-event node) and gains an "add event" link. The suggested tag_list is derived from the page's own category tags via NodesHelper::DEFAULT_EVENT_TAG_BY_PAGE_TAG (erfa-detail/ chaostreff-detail -> open-day) rather than hardcoded, and degrades to a blank field for any node whose tags don't match - deliberately universal, not chapter-specific, since Updates have historically carried event dates the same way. events#new surfaces *why* the tag was pre-filled via flash.now (not flash - this is a same-request render, not a redirect), using an explicit auto_tag_source param passed alongside tag_list rather than having the controller re-derive the reason from node_id. No destroy link added to nodes#show's events list - deliberate, per existing subnav-semantics convention (destructive actions live on the resource's own views). Covered directly by test. Adds real coverage for EventsController, previously 100% commented-out scaffold, plus unit tests for the new tag-mapping helper and the weekday-abbreviation helpers from the prior commit. --- test/controllers/events_controller_test.rb | 167 ++++++++++++++++++++++------- test/controllers/nodes_controller_test.rb | 33 ++++++ 2 files changed, 159 insertions(+), 41 deletions(-) (limited to 'test/controllers') diff --git a/test/controllers/events_controller_test.rb b/test/controllers/events_controller_test.rb index 14e534e..9371ca7 100644 --- a/test/controllers/events_controller_test.rb +++ b/test/controllers/events_controller_test.rb @@ -1,45 +1,130 @@ require 'test_helper' class EventsControllerTest < ActionController::TestCase - # test "should get index" do - # get :index - # assert_response :success - # assert_not_nil assigns(:events) - # end - # - # test "should get new" do - # get :new - # assert_response :success - # end - # - # test "should create event" do - # assert_difference('Event.count') do - # post :create, params: { :event => { } } - # end - # - # assert_redirected_to event_path(assigns(:event)) - # end - # - # test "should show event" do - # get :show, params: { :id => events(:one).to_param } - # assert_response :success - # end - # - # test "should get edit" do - # get :edit, params: { :id => events(:one).to_param } - # assert_response :success - # end - # - # test "should update event" do - # put :update, params: { :id => events(:one).to_param, :event => { } } - # assert_redirected_to event_path(assigns(:event)) - # end - # - # test "should destroy event" do - # assert_difference('Event.count', -1) do - # delete :destroy, params: { :id => events(:one).to_param } - # end - # - # assert_redirected_to events_path - # end + + test "should get index" do + login_as :quentin + get :index + assert_response :success + assert_not_nil assigns(:events) + end + + test "should get new" do + login_as :quentin + get :new + assert_response :success + end + + test "new pre-fills tag_list and explains it via flash when auto_tag_source is given" do + login_as :quentin + node = create_node_with_published_page + + get :new, params: { node_id: node.id, tag_list: "open-day", auto_tag_source: "erfa-detail" } + + assert_response :success + assert_equal "open-day", assigns(:event).tag_list.to_s + assert_match "open-day", flash[:notice] + assert_match "erfa-detail", flash[:notice] + end + + test "new does not flash without an auto_tag_source" do + login_as :quentin + + get :new, params: { tag_list: "open-day" } + + assert_response :success + assert_nil flash[:notice] + end + + test "new with no params renders a blank, unflashed form" do + login_as :quentin + + get :new + + assert_response :success + assert_nil assigns(:event).tag_list.presence + assert_nil flash[:notice] + end + + test "should show event" do + login_as :quentin + node = create_node_with_published_page + event = Event.create!(node_id: node.id, start_time: Time.now, end_time: Time.now + 1.hour) + + get :show, params: { id: event.id } + assert_response :success + end + + test "should get edit" do + login_as :quentin + node = create_node_with_published_page + event = Event.create!(node_id: node.id, start_time: Time.now, end_time: Time.now + 1.hour) + + get :edit, params: { id: event.id } + assert_response :success + end + + test "should create event attached to a node" do + login_as :quentin + node = create_node_with_published_page + + assert_difference('Event.count') do + post :create, params: { + event: { + node_id: node.id, + start_time: Time.now, + end_time: Time.now + 1.hour, + tag_list: "open-day" + } + } + end + + assert_redirected_to edit_node_path(node) + assert_equal 'Event was successfully created.', flash[:notice] + end + + test "should not create an event without a title or a node_id" do + login_as :quentin + + assert_no_difference('Event.count') do + post :create, params: { event: { start_time: Time.now, end_time: Time.now + 1.hour } } + end + + assert_response :success # re-renders :new, not a redirect + end + + test "should honour return_to on create" do + login_as :quentin + node = create_node_with_published_page + + post :create, params: { + event: { node_id: node.id, start_time: Time.now, end_time: Time.now + 1.hour }, + return_to: node_path(node) + } + + assert_redirected_to node_path(node) + end + + test "should update event" do + login_as :quentin + node = create_node_with_published_page + event = Event.create!(node_id: node.id, start_time: Time.now, end_time: Time.now + 1.hour) + + put :update, params: { id: event.id, event: { title: "Updated title" } } + + assert_redirected_to events_path + assert_equal "Updated title", event.reload.title + end + + test "should destroy event" do + login_as :quentin + node = create_node_with_published_page + event = Event.create!(node_id: node.id, start_time: Time.now, end_time: Time.now + 1.hour) + + assert_difference('Event.count', -1) do + delete :destroy, params: { id: event.id } + end + + assert_redirected_to events_url + end end diff --git a/test/controllers/nodes_controller_test.rb b/test/controllers/nodes_controller_test.rb index 53799f1..f14e27c 100644 --- a/test/controllers/nodes_controller_test.rb +++ b/test/controllers/nodes_controller_test.rb @@ -379,4 +379,37 @@ class NodesControllerTest < ActionController::TestCase get :index end + test "show renders events row and add-link for zero-event chapter node" do + login_as :quentin + node = create_node_with_published_page + node.head.tag_list = "erfa-detail" + node.head.save! + + get :show, params: { id: node.id } + assert_response :success + assert_select "a", text: "add event" + assert_select "a[href*='tag_list=open-day']" + assert_select "a[href*='auto_tag_source=erfa-detail']" + end + + test "show renders events row without a tag default for untagged node" do + login_as :quentin + node = create_node_with_published_page + + get :show, params: { id: node.id } + assert_response :success + assert_select "a", text: "add event" + assert_select "a[href*='tag_list=']", count: 0 + end + + test "show never renders a destroy link for events" do + login_as :quentin + node = create_node_with_published_page + Event.create!(node_id: node.id, start_time: Time.now, end_time: Time.now + 1.hour) + + get :show, params: { id: node.id } + assert_response :success + assert_select "form.button_to.destructive", count: 0 + end + end -- cgit v1.3 From 970f10854bccee0528de8435e5a65cdcc18ba93e Mon Sep 17 00:00:00 2001 From: erdgeist Date: Mon, 6 Jul 2026 04:20:55 +0200 Subject: Remove dead custom_rrule references after column drop events#index still read event.custom_rrule - a live bug the column-drop migration introduced, not just stale test data. Caught by four test failures (three fixtures/direct attribute hashes still setting the removed column, one UnknownAttributeError from a stale fixture loaded before any test method runs), none of which were actually testing this view - "should get index" passed throughout with zero Event records present, meaning it could never have caught a per-row rendering bug. Strengthened to create one real event first, so a future stray reference to a dropped or renamed column fails loudly instead of silently passing on an empty table. --- app/views/events/index.html.erb | 2 -- test/controllers/events_controller_test.rb | 3 +++ test/models/event_test.rb | 4 ---- 3 files changed, 3 insertions(+), 6 deletions(-) (limited to 'test/controllers') diff --git a/app/views/events/index.html.erb b/app/views/events/index.html.erb index d0458d7..ae4f477 100644 --- a/app/views/events/index.html.erb +++ b/app/views/events/index.html.erb @@ -6,7 +6,6 @@ Start time End time Rrule - Custom rrule Allday Url Node @@ -18,7 +17,6 @@ <%=h event.start_time %> <%=h event.end_time %> <%=h event.rrule %> - <%=h event.custom_rrule %> <%=h event.allday %> <%=h event.url %> <%= event.node ? link_to(event.node_id, node_path(event.node)) : '' %> diff --git a/test/controllers/events_controller_test.rb b/test/controllers/events_controller_test.rb index 9371ca7..46f3f4f 100644 --- a/test/controllers/events_controller_test.rb +++ b/test/controllers/events_controller_test.rb @@ -4,6 +4,9 @@ class EventsControllerTest < ActionController::TestCase test "should get index" do login_as :quentin + node = create_node_with_published_page + Event.create!(node_id: node.id, start_time: Time.now, end_time: Time.now + 1.hour) + get :index assert_response :success assert_not_nil assigns(:events) diff --git a/test/models/event_test.rb b/test/models/event_test.rb index d85aadd..e98605e 100644 --- a/test/models/event_test.rb +++ b/test/models/event_test.rb @@ -29,7 +29,6 @@ class EventTest < ActiveSupport::TestCase :longitude => 13.378944, :rrule => "FOOBAR", :allday => false, - :custom_rrule => false, :node_id => @cal_node.id ) end @@ -44,7 +43,6 @@ class EventTest < ActiveSupport::TestCase :longitude => 13.378944, :rrule => nil, :allday => false, - :custom_rrule => false, :node_id => @cal_node.id ) @@ -62,7 +60,6 @@ class EventTest < ActiveSupport::TestCase :longitude => 13.378944, :rrule => "FREQ=WEEKLY;INTERVAL=1", :allday => false, - :custom_rrule => false, :node_id => @cal_node.id ) @@ -97,7 +94,6 @@ class EventTest < ActiveSupport::TestCase :longitude => 13.378944, :rrule => "FREQ=MONTHLY;INTERVAL=1;BYDAY=-1WE", :allday => false, - :custom_rrule => true, :node_id => @cal_node.id ) -- cgit v1.3 From 394a2b890686cadd7a3ecb0038ce5d0b744431f4 Mon Sep 17 00:00:00 2001 From: erdgeist Date: Mon, 6 Jul 2026 04:34:04 +0200 Subject: Strengthen event/occurrence cascade test with real assertions test_can_remove_a_node_with_an_event previously just called node.destroy and get :index with no assertions at all - would pass whether or not occurrences were actually cleaned up, or even if index rendered correctly afterward. Now confirms occurrences genuinely exist before destroy (otherwise a passing post-destroy count of zero is meaningless - indistinguishable from "nothing to cascade in the first place"), scopes the count to this event specifically rather than a global Occurrence.count that could coincidentally pass regardless of whether this cascade works, and checks the trailing index request actually succeeds rather than just not raising. First real test of the occurrences.event_id FK constraint added earlier this session, not just the application-level dependent: :destroy. --- test/controllers/nodes_controller_test.rb | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'test/controllers') diff --git a/test/controllers/nodes_controller_test.rb b/test/controllers/nodes_controller_test.rb index f14e27c..99e06ae 100644 --- a/test/controllers/nodes_controller_test.rb +++ b/test/controllers/nodes_controller_test.rb @@ -364,7 +364,7 @@ class NodesControllerTest < ActionController::TestCase test "can remove a node with an event" do node = create_node_with_published_page - Event.create!( + event = Event.create!( :start_time => "2009-01-01T15:23:42".to_time, :end_time => "2009-01-01T20:05:23".to_time, :url => "http://events.ccc.de/congress/2082", @@ -373,10 +373,16 @@ class NodesControllerTest < ActionController::TestCase :allday => true, :node_id => node.id ) + event_id = event.id + assert_operator Occurrence.where(event_id: event_id).count, :>, 0, "expected the event to have generated at least one occurrence before destroy" + node.destroy + assert_equal 0, Occurrence.where(event_id: event_id).count + login_as :quentin get :index + assert_response :success end test "show renders events row and add-link for zero-event chapter node" do -- cgit v1.3 From 1393b3de31d95b1aa5122d6da37bd3259830bdb6 Mon Sep 17 00:00:00 2001 From: erdgeist Date: Mon, 6 Jul 2026 16:51:27 +0200 Subject: Fix test to match the current layout --- test/controllers/nodes_controller_test.rb | 126 +++++++++++++++--------------- 1 file changed, 63 insertions(+), 63 deletions(-) (limited to 'test/controllers') diff --git a/test/controllers/nodes_controller_test.rb b/test/controllers/nodes_controller_test.rb index 99e06ae..61f7db4 100644 --- a/test/controllers/nodes_controller_test.rb +++ b/test/controllers/nodes_controller_test.rb @@ -9,13 +9,13 @@ class NodesControllerTest < ActionController::TestCase get :index assert_response :success end - + def test_new login_as :quentin get :new assert_response :success end - + test "create generic node with parent_id provided" do login_as :quentin before_count = Node.count @@ -33,7 +33,7 @@ class NodesControllerTest < ActionController::TestCase assert_equal Node.last.parent_id, Node.root.id assert_equal 1, Node.last.level end - + test "create update node" do login_as :quentin post( @@ -45,7 +45,7 @@ class NodesControllerTest < ActionController::TestCase ) assert_response :redirect end - + test "create top level node" do login_as :quentin before_count = Node.count @@ -62,64 +62,64 @@ class NodesControllerTest < ActionController::TestCase assert_equal expected, Node.last.unique_name assert_equal 1, Node.last.level end - + test "creating a top_level node without a title should not work" do login_as :quentin - + assert_no_difference "Node.count" do post(:create, params: { :kind => "top_level" } ) end end - + test "creating a generic node without a parent_id should not work" do login_as :quentin - + assert_no_difference "Node.count" do post(:create, params: { :kind => "generic" } ) end end - + test "editing a node" do login_as :quentin - + node = Node.find_by_unique_name("fourth_child") node.pages.create node.draft = node.pages.last node.save - + assert_equal 1, node.pages.length - + draft = node.find_or_create_draft( User.first ) draft.title = "Hello" draft.body = "World" draft.save node.publish_draft! - + get :edit, params: { :id => node.id } assert_response :success assert_select("#page_title[value='Hello']") assert_select("#page_body", "World") - + node.reload assert_equal 2, node.pages.length assert_equal "Hello", node.find_or_create_draft( User.first ).title assert_equal "World", node.find_or_create_draft( User.first ).body end - + test "editing a locked node raises LockedByAnotherUser Exception" do login_as :quentin - + node = create_node_with_draft node.lock_owner = User.last node.save - + assert node.locked? - + get :edit, params: { :id => node.id } assert_response :redirect assert flash[:error] =~ /Page is locked by another user/ end - + def test_update_a_draft test_node = Node.root.children.create! :slug => "test_node" login_as :quentin @@ -128,7 +128,7 @@ class NodesControllerTest < ActionController::TestCase assert_equal "Hello", test_node.draft.title assert_equal "There", test_node.draft.body end - + def test_update_a_draft_with_changing_the_template test_node = Node.root.children.create! :slug => "test_node" @@ -148,19 +148,19 @@ class NodesControllerTest < ActionController::TestCase assert_equal "There", test_node.head.body assert_equal "Foobar", test_node.head.template_name end - + test "publish draft with staged_slug unqueal slug" do login_as :quentin - + test_node = Node.root.children.create! :slug => "test_node", :staged_slug => "peter_pan" - + put :publish, params: { :id => test_node.id } - + test_node.reload assert_equal "peter_pan", test_node.slug assert_equal "peter_pan", test_node.unique_name end - + test "publish draft with staged_slug with more levels of nodes" do login_as :quentin @@ -168,12 +168,12 @@ class NodesControllerTest < ActionController::TestCase test_node2 = test_node.children.create! :slug => "test_node2" put :publish, params: { :id => test_node.id } - + test_node.reload; test_node2.reload assert_equal "peter_pan/test_node2", test_node2.unique_name assert_equal "peter_pan", test_node.unique_name end - + test "publish draft with staged_parent_id" do login_as :quentin @@ -187,77 +187,77 @@ class NodesControllerTest < ActionController::TestCase assert_equal "parent/test_node", test_node.unique_name assert_equal "parent/test_node/test_node2", test_node2.unique_name end - + test "publish draft with staged_parent_id and staged_slug" do login_as :quentin - + parent = Node.root.children.create! :slug => "parent" - + test_node = Node.root.children.create!( - :slug => "test_node", + :slug => "test_node", :staged_parent_id => parent.id, :staged_slug => "peter_pan" ) - + test_node2 = test_node.children.create! :slug => "test_node2" - + put :publish, params: { :id => test_node.id } - + test_node.reload; test_node2.reload assert_equal "parent/peter_pan", test_node.unique_name assert_equal "parent/peter_pan/test_node2", test_node2.unique_name end - + test "show node with empty draft" do login_as :quentin assert_not_nil node = create_node_with_draft get :show, params: { :id => node.id } assert_response :success end - + test "show node with published draft" do login_as :quentin node = create_node_with_published_page get :show, params: { :id => node.id } assert_response :success - assert_select "td", :text => "Test", :count => 3 + assert_select "div.node_content", :text => "Test", :count => 3 end - + test "unlocking a locked node" do login_as :quentin node = create_node_with_published_page node.find_or_create_draft User.first - + assert node.locked? - + put :unlock, params: { :id => node.id } assert_response :redirect assert !node.reload.locked? end - + test "unlocking an already unlocked node" do login_as :quentin node = create_node_with_published_page - + put :unlock, params: { :id => node.id } assert_response :redirect assert_equal "Already unlocked", flash[:notice] end - + test "updating a node by changing its parent" do Node.root.descendants.destroy_all login_as :quentin node = create_node_with_published_page node.find_or_create_draft User.first - + other_node = Node.root.children.create( :slug => "other" ) - + node.staged_parent_id = other_node.id node.publish_draft! assert Node.valid? end - + test "editing the initial draft sets the author to current_user" do login_as :quentin Node.root.descendants.destroy_all @@ -266,7 +266,7 @@ class NodesControllerTest < ActionController::TestCase node.reload assert_equal "quentin", node.draft.user.login end - + test "updating the author of a node with existing head" do login_as :quentin Node.root.descendants.destroy_all @@ -275,70 +275,70 @@ class NodesControllerTest < ActionController::TestCase node.find_or_create_draft users(:quentin) assert node.draft.valid? assert node.valid? - + put :update, params: { :id => node.id, :page => {:user_id => users(:aaron).id} } assert_response :redirect assert_equal "aaron", node.reload.draft.user.login end - + test "updating an existing page should not modify published_at" do login_as :quentin Node.root.descendants.destroy_all node = create_node_with_published_page - + get :edit, params: { :id => node.id } assert_response :success - + put :publish, params: { :id => node.id } - + node.reload assert_equal node.pages[0].published_at, node.pages[1].published_at end - + test "updating an exisiting page should not alter the author" do login_as :aaron Node.root.descendants.destroy_all node = create_node_with_published_page get :edit, params: { :id => node.id } - + put :publish, params: { :id => node.id } - + node.reload assert_equal node.pages[0].user, node.pages[1].user end - + test "editor and author are the same on a new node" do login_as :quentin node = create_node_with_draft get :edit, params: { :id => node.id } - + node.reload assert_equal "quentin", node.draft.user.login assert_equal "quentin", node.draft.editor.login end - + test "creating new draft alters the editor but keeps the author" do node = create_node_with_published_page assert_equal "quentin", node.head.user.login - + login_as :aaron get :edit, params: {:id => node.id } - + node.reload assert_equal "quentin", node.head.user.login assert_equal "aaron", node.draft.editor.login end - + test "unlocking and relocking changes editor if done by another user" do node = create_node_with_published_page draft = node.find_or_create_draft users(:quentin) assert_equal draft.user.login, draft.editor.login assert node.locked? node.unlock! - + login_as :aaron get :edit, params: { :id => node.id } - + node.reload assert_equal "quentin", node.draft.user.login assert_equal "aaron", node.draft.editor.login -- cgit v1.3 From c6bf63a82007c275d13e9e9e0857434b3b7890c0 Mon Sep 17 00:00:00 2001 From: erdgeist Date: Wed, 8 Jul 2026 16:23:01 +0200 Subject: Fix tests for the new autosave routing Route Save and autosave through the new Node methods update now promotes the current autosave into the draft via save_draft! rather than writing submitted params directly; autosave gets its own PUT member route so PATCH update can mean "deliberate save" specifically, matching every other custom action on this resource. Both now require the lock to already be held rather than acquiring it themselves, which is a deliberate narrowing: through the real UI this is always true, since neither can fire before edit has loaded. Two existing tests called update directly with no prior edit, which the old code tolerated by acquiring the lock as a side effect; updated to call edit first instead of loosening the guarantee back open. NodesController#edit itself is unchanged and still calls the old find_or_create_draft, so drafts are still created eagerly on entering edit for now -- switching that over is the next step, not this one. --- app/controllers/nodes_controller.rb | 39 +++++++++++++++++++++++-------- test/controllers/nodes_controller_test.rb | 2 ++ 2 files changed, 31 insertions(+), 10 deletions(-) (limited to 'test/controllers') diff --git a/app/controllers/nodes_controller.rb b/app/controllers/nodes_controller.rb index a72be68..042963b 100644 --- a/app/controllers/nodes_controller.rb +++ b/app/controllers/nodes_controller.rb @@ -11,7 +11,8 @@ class NodesController < ApplicationController :update, :destroy, :publish, - :unlock + :unlock, + :autosave ] def index @@ -72,17 +73,35 @@ class NodesController < ApplicationController def update @node.update(node_params) - @draft = @node.find_or_create_draft current_user - @draft.tag_list = params[:tag_list] - if @draft.update( page_params ) - flash[:notice] = "Draft has been saved: #{Time.now}" - respond_to do |format| - format.html { redirect_to edit_node_path(@node) } - format.js - end + @node.autosave!( page_params.merge(:tag_list => params[:tag_list]), current_user ) + @node.save_draft!(current_user) + + flash[:notice] = "Draft has been saved: #{Time.now}" + + if params[:commit] == "Save + Unlock + Exit" + @node.unlock! + redirect_to node_path(@node) else - render :action => :edit + redirect_to edit_node_path(@node) end + rescue LockedByAnotherUser => e + flash[:error] = e.message + redirect_to node_path(@node) + rescue ActiveRecord::RecordInvalid + @page = @node.autosave || @node.draft || @node.head + render :action => :edit + end + + def autosave + @node.update(node_params) + @node.autosave!( page_params.merge(:tag_list => params[:tag_list]), current_user ) + head :ok + rescue LockedByAnotherUser => e + render plain: e.message, status: :locked + rescue ActiveRecord::RecordInvalid => e + render plain: e.message, status: :unprocessable_entity + rescue StandardError => e + render plain: "Autosave failed", status: :internal_server_error end def destroy diff --git a/test/controllers/nodes_controller_test.rb b/test/controllers/nodes_controller_test.rb index 61f7db4..f0be8c9 100644 --- a/test/controllers/nodes_controller_test.rb +++ b/test/controllers/nodes_controller_test.rb @@ -123,6 +123,7 @@ class NodesControllerTest < ActionController::TestCase def test_update_a_draft test_node = Node.root.children.create! :slug => "test_node" login_as :quentin + get :edit, params: { :id => test_node.id } put :update, params: { :id => test_node.id, :page => {:title => "Hello", :body => "There"} } test_node.reload assert_equal "Hello", test_node.draft.title @@ -133,6 +134,7 @@ class NodesControllerTest < ActionController::TestCase test_node = Node.root.children.create! :slug => "test_node" login_as :quentin + get :edit, params: { :id => test_node.id } put :update, params: { :id => test_node.id, :page => { -- cgit v1.3 From a5d7fb9730aa629b3eb8244ee1b3dd863fbfbed5 Mon Sep 17 00:00:00 2001 From: erdgeist Date: Wed, 8 Jul 2026 16:24:52 +0200 Subject: Fix node_content count in show-with-published-draft test Fallout from the earlier nodes#show heading change, which replaced the Title row with an

-- unrelated to the autosave work in the preceding commits. --- test/controllers/nodes_controller_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'test/controllers') diff --git a/test/controllers/nodes_controller_test.rb b/test/controllers/nodes_controller_test.rb index f0be8c9..030cff0 100644 --- a/test/controllers/nodes_controller_test.rb +++ b/test/controllers/nodes_controller_test.rb @@ -222,7 +222,7 @@ class NodesControllerTest < ActionController::TestCase node = create_node_with_published_page get :show, params: { :id => node.id } assert_response :success - assert_select "div.node_content", :text => "Test", :count => 3 + assert_select "div.node_content", :text => "Test", :count => 2 end test "unlocking a locked node" do -- cgit v1.3 From 6ad96c44d04df01e6abde097c681e824dd5fe745 Mon Sep 17 00:00:00 2001 From: erdgeist Date: Wed, 8 Jul 2026 17:31:15 +0200 Subject: Fix authorship and published_at loss in autosave promotion 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. --- app/models/node.rb | 36 ++++++++++++++++++++++++++++--- test/controllers/nodes_controller_test.rb | 7 ++++-- test/models/node_test.rb | 3 +++ 3 files changed, 41 insertions(+), 5 deletions(-) (limited to 'test/controllers') diff --git a/app/models/node.rb b/app/models/node.rb index 9eb0fe4..f15c908 100644 --- a/app/models/node.rb +++ b/app/models/node.rb @@ -56,6 +56,11 @@ class Node < ApplicationRecord def lock_for_editing! current_user if self.lock_owner.nil? || self.lock_owner == current_user lock_for! current_user + if self.draft + self.draft.user = current_user if self.draft.user.nil? + self.draft.editor = current_user + self.draft.save! + end self else raise( @@ -95,15 +100,18 @@ class Node < ApplicationRecord return unless self.autosave if self.draft + preserved_published_at = self.draft.published_at self.draft.clone_attributes_from self.autosave + self.draft.published_at = preserved_published_at self.draft.user_id = self.autosave.user_id if self.autosave.user_id self.draft.editor = current_user self.draft.save! else - empty_page = self.pages.create! - empty_page.user = self.autosave.user_id ? self.autosave.user : (self.head ? self.head.user : current_user) - empty_page.editor = current_user + empty_page = self.pages.create! empty_page.clone_attributes_from self.autosave + empty_page.user = self.autosave.user_id ? self.autosave.user : (self.head ? self.head.user : current_user) + empty_page.editor = current_user + empty_page.published_at = self.head.published_at if self.head empty_page.save! self.draft = empty_page self.save! @@ -150,6 +158,28 @@ class Node < ApplicationRecord self.draft.reload end + # Discards exactly the topmost non-empty layer -- autosave if present, + # else draft -- and reveals whatever's beneath it. Releases the lock + # only once nothing is left to protect (no draft survives); leaves it + # alone whenever a draft remains, since #edit still has real content + # open. + def revert! current_user + assert_locked_by! current_user + + if self.autosave + self.autosave.destroy + self.autosave_id = nil + self.save! + elsif self.draft && self.head + self.draft.destroy + self.draft_id = nil + self.save! + end + + self.unlock! unless self.draft + self.reload + end + def staged_slug=(value) if head.blank? self.slug = value diff --git a/test/controllers/nodes_controller_test.rb b/test/controllers/nodes_controller_test.rb index 030cff0..f3e04c2 100644 --- a/test/controllers/nodes_controller_test.rb +++ b/test/controllers/nodes_controller_test.rb @@ -101,7 +101,7 @@ class NodesControllerTest < ActionController::TestCase assert_select("#page_body", "World") node.reload - assert_equal 2, node.pages.length + assert_equal 1, node.pages.length assert_equal "Hello", node.find_or_create_draft( User.first ).title assert_equal "World", node.find_or_create_draft( User.first ).body end @@ -291,6 +291,7 @@ class NodesControllerTest < ActionController::TestCase get :edit, params: { :id => node.id } assert_response :success + put :update, params: { :id => node.id, :page => { :title => "updated" } } put :publish, params: { :id => node.id } node.reload @@ -303,6 +304,7 @@ class NodesControllerTest < ActionController::TestCase node = create_node_with_published_page get :edit, params: { :id => node.id } + put :update, params: { :id => node.id, :page => { :title => "updated" } } put :publish, params: { :id => node.id } node.reload @@ -324,7 +326,8 @@ class NodesControllerTest < ActionController::TestCase assert_equal "quentin", node.head.user.login login_as :aaron - get :edit, params: {:id => node.id } + get :edit, params: { :id => node.id } + put :update, params: { :id => node.id, :page => { :title => "updated" } } node.reload assert_equal "quentin", node.head.user.login diff --git a/test/models/node_test.rb b/test/models/node_test.rb index 2953f8f..bdf556d 100644 --- a/test/models/node_test.rb +++ b/test/models/node_test.rb @@ -339,6 +339,9 @@ class NodeTest < ActiveSupport::TestCase assert_equal head_revision, node.head.revision assert_nil node.autosave assert_equal 2, node.pages.count + assert_equal node.head.user, node.draft.user + assert_equal @user1, node.draft.editor + assert_equal node.head.published_at, node.draft.published_at end test "autosave!, save_draft!, and lock_for_editing! raise LockedByAnotherUser for a second user" do -- cgit v1.3 From a25d90335ad3738b6831288190132c2f7498465c Mon Sep 17 00:00:00 2001 From: erdgeist Date: Fri, 10 Jul 2026 00:35:24 +0200 Subject: Retire vendored cacycle_diff.js for a diff-lcs-based diff view Revisions#diff now computes an inline or side-by-side word diff server-side (Page#diff_against, lib/html_word_diff.rb) instead of shipping raw/escaped content to the browser for a 2008-era vendored JS differ to mangle. View mode is picked via a `view` param, settable either on the diff page itself or directly from the revisions#index sticky bar next to "Diff revisions". Also fixes a pre-existing bug in RevisionsController#diff's single- revision branch, which set params[:start]/params[:end] instead of params[:start_revision]/params[:end_revision]. --- Gemfile | 3 +- Gemfile.lock | 3 + app/controllers/revisions_controller.rb | 12 +- app/models/page.rb | 16 + app/views/revisions/diff.html.erb | 78 +- app/views/revisions/index.html.erb | 8 + lib/html_word_diff.rb | 52 ++ public/javascripts/cacycle_diff.js | 1112 ------------------------- public/stylesheets/admin.css | 25 + test/controllers/revisions_controller_test.rb | 29 + test/models/page_test.rb | 34 + 11 files changed, 204 insertions(+), 1168 deletions(-) create mode 100644 lib/html_word_diff.rb delete mode 100644 public/javascripts/cacycle_diff.js (limited to 'test/controllers') diff --git a/Gemfile b/Gemfile index d136eb5..19d92d7 100644 --- a/Gemfile +++ b/Gemfile @@ -45,10 +45,11 @@ gem 'acts-as-taggable-on', git: 'https://github.com/mbleigh/acts-as-taggable-on.git', branch: 'master' -# ── XML / parsing ───────────────────────────────────────────────────────────── +# ── XML / parsing / diffing ─────────────────────────────────────────────────── gem 'libxml-ruby', '~> 5.0', require: 'xml' # body link rewriting in Page model gem 'nokogiri', '~> 1.18' +gem 'diff-lcs', require: 'diff/lcs' # ── Operational ─────────────────────────────────────────────────────────────── diff --git a/Gemfile.lock b/Gemfile.lock index 02c71ff..e6f5fa0 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -108,6 +108,7 @@ GEM connection_pool (3.0.2) crass (1.0.7) date (3.5.1) + diff-lcs (2.0.0) drb (2.2.3) erb (6.0.4) erubi (1.13.1) @@ -330,6 +331,7 @@ DEPENDENCIES chaos_calendar! coffee-rails (~> 4.0) concurrent-ruby (~> 1.3) + diff-lcs exception_notification (~> 4.5) globalize (~> 7.0) jquery-rails @@ -377,6 +379,7 @@ CHECKSUMS connection_pool (3.0.2) sha256=33fff5ba71a12d2aa26cb72b1db8bba2a1a01823559fb01d29eb74c286e62e0a crass (1.0.7) sha256=94868719948664c89ddcaf0a37c65048413dfcb1c869470a5f7a7ceb5390b295 date (3.5.1) sha256=750d06384d7b9c15d562c76291407d89e368dda4d4fff957eb94962d325a0dc0 + diff-lcs (2.0.0) sha256=708a5d52ec2945b50f8f53a181174aa1ef2c496edf81c05957fe956dabb363d5 drb (2.2.3) sha256=0b00d6fdb50995fe4a45dea13663493c841112e4068656854646f418fda13373 erb (6.0.4) sha256=38e3803694be357fe2bfe312487c74beaf9fb4e5beb3e22498952fe1645b95d9 erubi (1.13.1) sha256=a082103b0885dbc5ecf1172fede897f9ebdb745a4b97a5e8dc63953db1ee4ad9 diff --git a/app/controllers/revisions_controller.rb b/app/controllers/revisions_controller.rb index 42d667e..9acb26f 100644 --- a/app/controllers/revisions_controller.rb +++ b/app/controllers/revisions_controller.rb @@ -13,16 +13,18 @@ class RevisionsController < ApplicationController def diff @node = Node.find(params[:node_id]) - + if @node.pages.length > 1 params[:start_revision] ||= @node.pages.all[-2].revision params[:end_revision] ||= @node.pages.all[-1].revision else - params[:start], params[:end] = 1, 1 + params[:start_revision], params[:end_revision] = 1, 1 end - - @start = @node.pages.find_by_revision( params[:start_revision] ) - @end = @node.pages.find_by_revision( params[:end_revision] ) + + @start = @node.pages.find_by_revision( params[:start_revision] ) + @end = @node.pages.find_by_revision( params[:end_revision] ) + @diff_view = params[:view] == "side_by_side" ? :side_by_side : :inline + @diff = @end.diff_against( @start, view: @diff_view ) end def show diff --git a/app/models/page.rb b/app/models/page.rb index 1a98e08..ea04cd6 100644 --- a/app/models/page.rb +++ b/app/models/page.rb @@ -175,6 +175,22 @@ class Page < ApplicationRecord self.save end + def diff_against other, view: :inline + if view == :side_by_side + { + title: HtmlWordDiff.side_by_side(other.title.to_s, title.to_s), + abstract: HtmlWordDiff.side_by_side(other.abstract.to_s, abstract.to_s), + body: HtmlWordDiff.side_by_side(other.body.to_s, body.to_s) + } + else + { + title: HtmlWordDiff.inline(other.title.to_s, title.to_s), + abstract: HtmlWordDiff.inline(other.abstract.to_s, abstract.to_s), + body: HtmlWordDiff.inline(other.body.to_s, body.to_s) + } + end + end + def public? published_at.nil? ? true : published_at < Time.now end diff --git a/app/views/revisions/diff.html.erb b/app/views/revisions/diff.html.erb index d8c6a47..d7bb528 100644 --- a/app/views/revisions/diff.html.erb +++ b/app/views/revisions/diff.html.erb @@ -7,58 +7,36 @@ <%= form_tag diff_node_revisions_path do %> <%= select_tag :start_revision, options_for_select(@node.pages.map{|x| x.revision}, params[:start_revision].to_i) %> <%= select_tag :end_revision, options_for_select(@node.pages.map{|x| x.revision}, params[:end_revision].to_i) %> + <%= select_tag :view, options_for_select([['Inline', 'inline'], ['Side by side', 'side_by_side']], @diff_view) %> <%= submit_tag 'Diff' %> <% end %> - - - - - - - - -
-  
-
-
-  
-
- - - -
-

Title

-

- -

Abstract

-

- -

Body

-

+ <% if @diff_view == :side_by_side %> +
+
+

Title

+

<%= raw @diff[:title][0] %>

+

Abstract

+

<%= raw @diff[:abstract][0] %>

+

Body

+ <%= raw @diff[:body][0] %> +
+
+

Title

+

<%= raw @diff[:title][1] %>

+

Abstract

+

<%= raw @diff[:abstract][1] %>

+

Body

+ <%= raw @diff[:body][1] %> +
+
+ <% else %> +

Title

+

<%= raw @diff[:title] %>

+

Abstract

+

<%= raw @diff[:abstract] %>

+

Body

+ <%= raw @diff[:body] %> + <% end %>
diff --git a/app/views/revisions/index.html.erb b/app/views/revisions/index.html.erb index a6a981a..58c08b7 100644 --- a/app/views/revisions/index.html.erb +++ b/app/views/revisions/index.html.erb @@ -19,6 +19,8 @@ form: { id: 'diff_form', class: 'button_to computation' }, disabled: true %> + + @@ -68,6 +70,7 @@ document.getElementById('diff_form').addEventListener('submit', function(e) { var start = document.querySelector('input[name="start_revision"]:checked'); var end = document.querySelector('input[name="end_revision"]:checked'); + var view = document.querySelector('input[name="view"]:checked'); if (start) { var s = document.createElement('input'); s.type = 'hidden'; s.name = 'start_revision'; s.value = start.value; @@ -78,5 +81,10 @@ en.type = 'hidden'; en.name = 'end_revision'; en.value = end.value; this.appendChild(en); } + if (view) { + var v = document.createElement('input'); + v.type = 'hidden'; v.name = 'view'; v.value = view.value; + this.appendChild(v); + } }); diff --git a/lib/html_word_diff.rb b/lib/html_word_diff.rb new file mode 100644 index 0000000..1f28cf5 --- /dev/null +++ b/lib/html_word_diff.rb @@ -0,0 +1,52 @@ +require 'diff/lcs' + +module HtmlWordDiff + TOKEN_REGEXP = /<[^>]*>|[[:space:]]+|[[:alnum:]]+|[^[:space:][:alnum:]<>]/ + + def self.inline(old_html, new_html) + html = +'' + each_change(old_html, new_html) do |action, old_token, new_token| + case action + when '=' + html << new_token + when '-' + html << "#{old_token}" + when '+' + html << "#{new_token}" + when '!' + html << "#{old_token}#{new_token}" + end + end + html + end + + def self.side_by_side(old_html, new_html) + old_out = +'' + new_out = +'' + each_change(old_html, new_html) do |action, old_token, new_token| + case action + when '=' + old_out << old_token + new_out << new_token + when '-' + old_out << "#{old_token}" + when '+' + new_out << "#{new_token}" + when '!' + old_out << "#{old_token}" + new_out << "#{new_token}" + end + end + [old_out, new_out] + end + + def self.each_change(old_html, new_html) + Diff::LCS.sdiff(tokenize(old_html), tokenize(new_html)).each do |change| + yield change.action, change.old_element, change.new_element + end + end + + def self.tokenize(html) + html.to_s.scan(TOKEN_REGEXP) + end +end diff --git a/public/javascripts/cacycle_diff.js b/public/javascripts/cacycle_diff.js deleted file mode 100644 index 24f9d0b..0000000 --- a/public/javascripts/cacycle_diff.js +++ /dev/null @@ -1,1112 +0,0 @@ -//

- 
-/*
- 
-Name:    diff.js
-Version: 0.9.5a (April 6, 2008)
-Info:    http://en.wikipedia.org/wiki/User:Cacycle/diff
-Code:    http://en.wikipedia.org/wiki/User:Cacycle/diff.js
- 
-JavaScript diff algorithm by [[en:User:Cacycle]] (http://en.wikipedia.org/wiki/User_talk:Cacycle).
-Outputs html/css-formatted new text with highlighted deletions, inserts, and block moves.
- 
-The program uses cross-browser code and should work with all modern browsers. It has been tested with:
-* Mozilla Firefox 1.5.0.1
-* Mozilla SeaMonkey 1.0
-* Opera 8.53
-* Internet Explorer 6.0.2900.2180
-* Internet Explorer 7.0.5730.11
-This program is also compatibel with Greasemonkey
- 
-An implementation of the word-based algorithm from:
- 
-Communications of the ACM 21(4):264 (1978)
-http://doi.acm.org/10.1145/359460.359467
- 
-With the following additional feature:
- 
-* Word types have been optimized for MediaWiki source texts
-* Additional post-pass 5 code for resolving islands caused by adding
-  two common words at the end of sequences of common words
-* Additional detection of block borders and color coding of moved blocks and their original position
-* Optional "intelligent" omission of unchanged parts from the output
- 
-This code is used by the MediaWiki in-browser text editors [[en:User:Cacycle/editor]] and [[en:User:Cacycle/wikEd]]
-and the enhanced diff view tool wikEdDiff [[en:User:Cacycle/wikEd]].
- 
-Usage: var htmlText = WDiffString(oldText, newText);
- 
-This code has been released into the public domain.
- 
-Datastructures:
- 
-text: an object that holds all text related datastructures
-  .newWords: consecutive words of the new text (N)
-  .oldWords: consecutive words of the old text (O)
-  .newToOld: array of corresponding word number in old text (NA)
-  .oldToNew: array of corresponding word number in new text (OA)
-  .message:  output message for testing purposes
- 
-symbol['word']: symbol table for passes 1 - 3, holds words as a hash
-  .newCtr:  new word occurences counter (NC)
-  .oldCtr:  old word occurences counter (OC)
-  .toNew:   table last old word number
-  .toOld:   last new word number (OLNA)
- 
-block: an object that holds block move information
-  blocks indexed after new text:
-  .newStart:  new text word number of start of this block
-  .newLength: element number of this block including non-words
-  .newWords:  true word number of this block
-  .newNumber: corresponding block index in old text
-  .newBlock:  moved-block-number of a block that has been moved here
-  .newLeft:   moved-block-number of a block that has been moved from this border leftwards
-  .newRight:  moved-block-number of a block that has been moved from this border rightwards
-  .newLeftIndex:  index number of a block that has been moved from this border leftwards
-  .newRightIndex: index number of a block that has been moved from this border rightwards
-  blocks indexed after old text:
-  .oldStart:  word number of start of this block
-  .oldToNew:  corresponding new text word number of start
-  .oldLength: element number of this block including non-words
-  .oldWords:  true word number of this block
- 
-*/
- 
- 
-// css for change indicators
-if (typeof(wDiffStyleDelete) == 'undefined') { window.wDiffStyleDelete = 'font-weight: normal; text-decoration: none; color: #fff; background-color: #990033;'; }
-if (typeof(wDiffStyleInsert) == 'undefined') { window.wDiffStyleInsert = 'font-weight: normal; text-decoration: none; color: #fff; background-color: #009933;'; }
-if (typeof(wDiffStyleMoved)  == 'undefined') { window.wDiffStyleMoved  = 'font-weight: bold;  color: #000; vertical-align: text-bottom; font-size: xx-small; padding: 0; border: solid 1px;'; }
-if (typeof(wDiffStyleBlock)  == 'undefined') { window.wDiffStyleBlock  = [
-  'color: #000; background-color: #ffff80;',
-  'color: #000; background-color: #c0ffff;',
-  'color: #000; background-color: #ffd0f0;',
-  'color: #000; background-color: #ffe080;',
-  'color: #000; background-color: #aaddff;',
-  'color: #000; background-color: #ddaaff;',
-  'color: #000; background-color: #ffbbbb;',
-  'color: #000; background-color: #d8ffa0;',
-  'color: #000; background-color: #d0d0d0;'
-]; }
- 
-// html for change indicators, {number} is replaced by the block number
-// {block} is replaced by the block style, class and html comments are important for shortening the output
-if (typeof(wDiffHtmlMovedRight)  == 'undefined') { window.wDiffHtmlMovedRight  = ''; }
-if (typeof(wDiffHtmlMovedLeft)   == 'undefined') { window.wDiffHtmlMovedLeft   = ''; }
- 
-if (typeof(wDiffHtmlBlockStart)  == 'undefined') { window.wDiffHtmlBlockStart  = ''; }
-if (typeof(wDiffHtmlBlockEnd)    == 'undefined') { window.wDiffHtmlBlockEnd    = ''; }
- 
-if (typeof(wDiffHtmlDeleteStart) == 'undefined') { window.wDiffHtmlDeleteStart = ''; }
-if (typeof(wDiffHtmlDeleteEnd)   == 'undefined') { window.wDiffHtmlDeleteEnd   = ''; }
- 
-if (typeof(wDiffHtmlInsertStart) == 'undefined') { window.wDiffHtmlInsertStart = ''; }
-if (typeof(wDiffHtmlInsertEnd)   == 'undefined') { window.wDiffHtmlInsertEnd   = ''; }
- 
-// minimal number of real words for a moved block (0 for always displaying block move indicators)
-if (typeof(wDiffBlockMinLength) == 'undefined') { window.wDiffBlockMinLength = 3; }
- 
-// exclude identical sequence starts and endings from change marking
-if (typeof(wDiffWordDiff) == 'undefined') { window.wDiffWordDiff = true; }
- 
-// enable recursive diff to resolve problematic sequences
-if (typeof(wDiffRecursiveDiff) == 'undefined') { window.wDiffRecursiveDiff = true; }
- 
-// enable block move display
-if (typeof(wDiffShowBlockMoves) == 'undefined') { window.wDiffShowBlockMoves = true; }
- 
-// remove unchanged parts from final output
- 
-// characters before diff tag to search for previous heading, paragraph, line break, cut characters
-if (typeof(wDiffHeadingBefore)   == 'undefined') { window.wDiffHeadingBefore   = 1500; }
-if (typeof(wDiffParagraphBefore) == 'undefined') { window.wDiffParagraphBefore = 1500; }
-if (typeof(wDiffLineBeforeMax)   == 'undefined') { window.wDiffLineBeforeMax   = 1000; }
-if (typeof(wDiffLineBeforeMin)   == 'undefined') { window.wDiffLineBeforeMin   =  500; }
-if (typeof(wDiffBlankBeforeMax)  == 'undefined') { window.wDiffBlankBeforeMax  = 1000; }
-if (typeof(wDiffBlankBeforeMin)  == 'undefined') { window.wDiffBlankBeforeMin  =  500; }
-if (typeof(wDiffCharsBefore)     == 'undefined') { window.wDiffCharsBefore     =  500; }
- 
-// characters after diff tag to search for next heading, paragraph, line break, or characters
-if (typeof(wDiffHeadingAfter)   == 'undefined') { window.wDiffHeadingAfter   = 1500; }
-if (typeof(wDiffParagraphAfter) == 'undefined') { window.wDiffParagraphAfter = 1500; }
-if (typeof(wDiffLineAfterMax)   == 'undefined') { window.wDiffLineAfterMax   = 1000; }
-if (typeof(wDiffLineAfterMin)   == 'undefined') { window.wDiffLineAfterMin   =  500; }
-if (typeof(wDiffBlankAfterMax)  == 'undefined') { window.wDiffBlankAfterMax  = 1000; }
-if (typeof(wDiffBlankAfterMin)  == 'undefined') { window.wDiffBlankAfterMin  =  500; }
-if (typeof(wDiffCharsAfter)     == 'undefined') { window.wDiffCharsAfter     =  500; }
- 
-// maximal fragment distance to join close fragments
-if (typeof(wDiffFragmentJoin)  == 'undefined') { window.wDiffFragmentJoin = 1000; }
-if (typeof(wDiffOmittedChars)  == 'undefined') { window.wDiffOmittedChars = '…'; }
-if (typeof(wDiffOmittedLines)  == 'undefined') { window.wDiffOmittedLines = '
'; } -if (typeof(wDiffNoChange) == 'undefined') { window.wDiffNoChange = '
'; } - -// compatibility fix for old name of main function -window.StringDiff = window.WDiffString; - - -// WDiffString: main program -// input: oldText, newText, strings containing the texts -// returns: html diff - -window.WDiffString = function(oldText, newText) { - -// IE / Mac fix - oldText = oldText.replace(/(\r\n)/g, '\n'); - newText = newText.replace(/(\r\n)/g, '\n'); - - var text = {}; - text.newWords = []; - text.oldWords = []; - text.newToOld = []; - text.oldToNew = []; - text.message = ''; - var block = {}; - var outText = ''; - -// trap trivial changes: no change - if (oldText == newText) { - outText = newText; - outText = WDiffEscape(outText); - outText = WDiffHtmlFormat(outText); - return(outText); - } - -// trap trivial changes: old text deleted - if ( (oldText == null) || (oldText.length == 0) ) { - outText = newText; - outText = WDiffEscape(outText); - outText = WDiffHtmlFormat(outText); - outText = wDiffHtmlInsertStart + outText + wDiffHtmlInsertEnd; - return(outText); - } - -// trap trivial changes: new text deleted - if ( (newText == null) || (newText.length == 0) ) { - outText = oldText; - outText = WDiffEscape(outText); - outText = WDiffHtmlFormat(outText); - outText = wDiffHtmlDeleteStart + outText + wDiffHtmlDeleteEnd; - return(outText); - } - -// split new and old text into words - WDiffSplitText(oldText, newText, text); - -// calculate diff information - WDiffText(text); - -//detect block borders and moved blocks - WDiffDetectBlocks(text, block); - -// process diff data into formatted html text - outText = WDiffToHtml(text, block); - -// IE fix - outText = outText.replace(/> ( *) $1<'); - - return(outText); -} - - -// WDiffSplitText: split new and old text into words -// input: oldText, newText, strings containing the texts -// changes: text.newWords and text.oldWords, arrays containing the texts in arrays of words - -window.WDiffSplitText = function(oldText, newText, text) { - -// convert strange spaces - oldText = oldText.replace(/[\t\u000b\u00a0\u2028\u2029]+/g, ' '); - newText = newText.replace(/[\t\u000b\u00a0\u2028\u2029]+/g, ' '); - -// split old text into words - -// / | | | | | | | | | | | | | | / - var pattern = /[\w]+|\[\[|\]\]|\{\{|\}\}|\n+| +|&\w+;|'''|''|=+|\{\||\|\}|\|\-|./g; - var result; - do { - result = pattern.exec(oldText); - if (result != null) { - text.oldWords.push(result[0]); - } - } while (result != null); - -// split new text into words - do { - result = pattern.exec(newText); - if (result != null) { - text.newWords.push(result[0]); - } - } while (result != null); - - return; -} - - -// WDiffText: calculate diff information -// input: text.newWords and text.oldWords, arrays containing the texts in arrays of words -// optionally for recursive calls: newStart, newEnd, oldStart, oldEnd, recursionLevel -// changes: text.newToOld and text.oldToNew, containing the line numbers in the other version - -window.WDiffText = function(text, newStart, newEnd, oldStart, oldEnd, recursionLevel) { - - symbol = new Object(); - symbol.newCtr = []; - symbol.oldCtr = []; - symbol.toNew = []; - symbol.toOld = []; - -// set defaults - newStart = newStart || 0; - newEnd = newEnd || text.newWords.length; - oldStart = oldStart || 0; - oldEnd = oldEnd || text.oldWords.length; - recursionLevel = recursionLevel || 0; - -// limit recursion depth - if (recursionLevel > 10) { - return; - } - -// pass 1: parse new text into symbol table s - - var word; - for (var i = newStart; i < newEnd; i ++) { - word = text.newWords[i]; - -// add new entry to symbol table - if ( symbol[word] == null) { - symbol[word] = { newCtr: 0, oldCtr: 0, toNew: null, toOld: null }; - } - -// increment symbol table word counter for new text - symbol[word].newCtr ++; - -// add last word number in new text - symbol[word].toNew = i; - } - -// pass 2: parse old text into symbol table - - for (var j = oldStart; j < oldEnd; j ++) { - word = text.oldWords[j]; - -// add new entry to symbol table - if ( symbol[word] == null) { - symbol[word] = { newCtr: 0, oldCtr: 0, toNew: null, toOld: null }; - } - -// increment symbol table word counter for old text - symbol[word].oldCtr ++; - -// add last word number in old text - symbol[word].toOld = j; - } - -// pass 3: connect unique words - - for (var i in symbol) { - -// find words in the symbol table that occur only once in both versions - if ( (symbol[i].newCtr == 1) && (symbol[i].oldCtr == 1) ) { - var toNew = symbol[i].toNew; - var toOld = symbol[i].toOld; - -// do not use spaces as unique markers - if ( ! /\s/.test( text.newWords[toNew] ) ) { - -// connect from new to old and from old to new - text.newToOld[toNew] = toOld; - text.oldToNew[toOld] = toNew; - } - } - } - -// pass 4: connect adjacent identical words downwards - - for (var i = newStart; i < newEnd - 1; i ++) { - -// find already connected pairs - if (text.newToOld[i] != null) { - j = text.newToOld[i]; - -// check if the following words are not yet connected - if ( (text.newToOld[i + 1] == null) && (text.oldToNew[j + 1] == null) ) { - -// if the following words are the same connect them - if ( text.newWords[i + 1] == text.oldWords[j + 1] ) { - text.newToOld[i + 1] = j + 1; - text.oldToNew[j + 1] = i + 1; - } - } - } - } - -// pass 5: connect adjacent identical words upwards - - for (var i = newEnd - 1; i > newStart; i --) { - -// find already connected pairs - if (text.newToOld[i] != null) { - j = text.newToOld[i]; - -// check if the preceeding words are not yet connected - if ( (text.newToOld[i - 1] == null) && (text.oldToNew[j - 1] == null) ) { - -// if the preceeding words are the same connect them - if ( text.newWords[i - 1] == text.oldWords[j - 1] ) { - text.newToOld[i - 1] = j - 1; - text.oldToNew[j - 1] = i - 1; - } - } - } - } - -// recursively diff still unresolved regions downwards - - if (wDiffRecursiveDiff) { - i = newStart; - j = oldStart; - while (i < newEnd) { - if (text.newToOld[i - 1] != null) { - j = text.newToOld[i - 1] + 1; - } - -// check for the start of an unresolved sequence - if ( (text.newToOld[i] == null) && (text.oldToNew[j] == null) ) { - -// determine the ends of the sequences - var iStart = i; - var iEnd = i; - while ( (text.newToOld[iEnd] == null) && (iEnd < newEnd) ) { - iEnd ++; - } - var iLength = iEnd - iStart; - - var jStart = j; - var jEnd = j; - while ( (text.oldToNew[jEnd] == null) && (jEnd < oldEnd) ) { - jEnd ++; - } - var jLength = jEnd - jStart; - -// recursively diff the unresolved sequence - if ( (iLength > 0) && (jLength > 0) ) { - if ( (iLength > 1) || (jLength > 1) ) { - if ( (iStart != newStart) || (iEnd != newEnd) || (jStart != oldStart) || (jEnd != oldEnd) ) { - WDiffText(text, iStart, iEnd, jStart, jEnd, recursionLevel + 1); - } - } - } - i = iEnd; - } - else { - i ++; - } - } - } - -// recursively diff still unresolved regions upwards - - if (wDiffRecursiveDiff) { - i = newEnd - 1; - j = oldEnd - 1; - while (i >= newStart) { - if (text.newToOld[i + 1] != null) { - j = text.newToOld[i + 1] - 1; - } - -// check for the start of an unresolved sequence - if ( (text.newToOld[i] == null) && (text.oldToNew[j] == null) ) { - -// determine the ends of the sequences - var iStart = i; - var iEnd = i + 1; - while ( (text.newToOld[iStart - 1] == null) && (iStart >= newStart) ) { - iStart --; - } - var iLength = iEnd - iStart; - - var jStart = j; - var jEnd = j + 1; - while ( (text.oldToNew[jStart - 1] == null) && (jStart >= oldStart) ) { - jStart --; - } - var jLength = jEnd - jStart; - -// recursively diff the unresolved sequence - if ( (iLength > 0) && (jLength > 0) ) { - if ( (iLength > 1) || (jLength > 1) ) { - if ( (iStart != newStart) || (iEnd != newEnd) || (jStart != oldStart) || (jEnd != oldEnd) ) { - WDiffText(text, iStart, iEnd, jStart, jEnd, recursionLevel + 1); - } - } - } - i = iStart - 1; - } - else { - i --; - } - } - } - return; -} - - -// WDiffToHtml: process diff data into formatted html text -// input: text.newWords and text.oldWords, arrays containing the texts in arrays of words -// text.newToOld and text.oldToNew, containing the line numbers in the other version -// block data structure -// returns: outText, a html string - -window.WDiffToHtml = function(text, block) { - - var outText = text.message; - - var blockNumber = 0; - var i = 0; - var j = 0; - var movedAsInsertion; - -// cycle through the new text - do { - var movedIndex = []; - var movedBlock = []; - var movedLeft = []; - var blockText = ''; - var identText = ''; - var delText = ''; - var insText = ''; - var identStart = ''; - -// check if a block ends here and finish previous block - if (movedAsInsertion != null) { - if (movedAsInsertion == false) { - identStart += wDiffHtmlBlockEnd; - } - else { - identStart += wDiffHtmlInsertEnd; - } - movedAsInsertion = null; - } - -// detect block boundary - if ( (text.newToOld[i] != j) || (blockNumber == 0 ) ) { - if ( ( (text.newToOld[i] != null) || (i >= text.newWords.length) ) && ( (text.oldToNew[j] != null) || (j >= text.oldWords.length) ) ) { - -// block moved right - var moved = block.newRight[blockNumber]; - if (moved > 0) { - var index = block.newRightIndex[blockNumber]; - movedIndex.push(index); - movedBlock.push(moved); - movedLeft.push(false); - } - -// block moved left - moved = block.newLeft[blockNumber]; - if (moved > 0) { - var index = block.newLeftIndex[blockNumber]; - movedIndex.push(index); - movedBlock.push(moved); - movedLeft.push(true); - } - -// check if a block starts here - moved = block.newBlock[blockNumber]; - if (moved > 0) { - -// mark block as inserted text - if (block.newWords[blockNumber] < wDiffBlockMinLength) { - identStart += wDiffHtmlInsertStart; - movedAsInsertion = true; - } - -// mark block by color - else { - if (moved > wDiffStyleBlock.length) { - moved = wDiffStyleBlock.length; - } - identStart += WDiffHtmlCustomize(wDiffHtmlBlockStart, moved - 1); - movedAsInsertion = false; - } - } - - if (i >= text.newWords.length) { - i ++; - } - else { - j = text.newToOld[i]; - blockNumber ++; - } - } - } - -// get the correct order if moved to the left as well as to the right from here - if (movedIndex.length == 2) { - if (movedIndex[0] > movedIndex[1]) { - movedIndex.reverse(); - movedBlock.reverse(); - movedLeft.reverse(); - } - } - -// handle left and right block moves from this position - for (var m = 0; m < movedIndex.length; m ++) { - -// insert the block as deleted text - if (block.newWords[ movedIndex[m] ] < wDiffBlockMinLength) { - var movedStart = block.newStart[ movedIndex[m] ]; - var movedLength = block.newLength[ movedIndex[m] ]; - var str = ''; - for (var n = movedStart; n < movedStart + movedLength; n ++) { - str += text.newWords[n]; - } - str = WDiffEscape(str); - str = str.replace(/\n/g, '¶
'); - blockText += wDiffHtmlDeleteStart + str + wDiffHtmlDeleteEnd; - } - -// add a placeholder / move direction indicator - else { - if (movedBlock[m] > wDiffStyleBlock.length) { - movedBlock[m] = wDiffStyleBlock.length; - } - if (movedLeft[m]) { - blockText += WDiffHtmlCustomize(wDiffHtmlMovedLeft, movedBlock[m] - 1); - } - else { - blockText += WDiffHtmlCustomize(wDiffHtmlMovedRight, movedBlock[m] - 1); - } - } - } - -// collect consecutive identical text - while ( (i < text.newWords.length) && (j < text.oldWords.length) ) { - if ( (text.newToOld[i] == null) || (text.oldToNew[j] == null) ) { - break; - } - if (text.newToOld[i] != j) { - break; - } - identText += text.newWords[i]; - i ++; - j ++; - } - -// collect consecutive deletions - while ( (text.oldToNew[j] == null) && (j < text.oldWords.length) ) { - delText += text.oldWords[j]; - j ++; - } - -// collect consecutive inserts - while ( (text.newToOld[i] == null) && (i < text.newWords.length) ) { - insText += text.newWords[i]; - i ++; - } - -// remove leading and trailing similarities betweein delText and ins from highlighting - var preText = ''; - var postText = ''; - if (wDiffWordDiff) { - if ( (delText != '') && (insText != '') ) { - -// remove leading similarities - while ( delText.charAt(0) == insText.charAt(0) && (delText != '') && (insText != '') ) { - preText = preText + delText.charAt(0); - delText = delText.substr(1); - insText = insText.substr(1); - } - -// remove trailing similarities - while ( delText.charAt(delText.length - 1) == insText.charAt(insText.length - 1) && (delText != '') && (insText != '') ) { - postText = delText.charAt(delText.length - 1) + postText; - delText = delText.substr(0, delText.length - 1); - insText = insText.substr(0, insText.length - 1); - } - } - } - -// output the identical text, deletions and inserts - -// moved from here indicator - if (blockText != '') { - outText += blockText; - } - -// identical text - if (identText != '') { - outText += identStart + WDiffEscape(identText); - } - outText += preText; - -// deleted text - if (delText != '') { - delText = wDiffHtmlDeleteStart + WDiffEscape(delText) + wDiffHtmlDeleteEnd; - delText = delText.replace(/\n/g, '¶
'); - outText += delText; - } - -// inserted text - if (insText != '') { - insText = wDiffHtmlInsertStart + WDiffEscape(insText) + wDiffHtmlInsertEnd; - insText = insText.replace(/\n/g, '¶
'); - outText += insText; - } - outText += postText; - } while (i <= text.newWords.length); - - outText += '\n'; - outText = WDiffHtmlFormat(outText); - - return(outText); -} - - -// WDiffEscape: replaces html-sensitive characters in output text with character entities - -window.WDiffEscape = function(text) { - - text = text.replace(/&/g, '&'); - text = text.replace(//g, '>'); - text = text.replace(/\"/g, '"'); - - return(text); -} - - -// HtmlCustomize: customize indicator html: replace {number} with the block number, {block} with the block style - -window.WDiffHtmlCustomize = function(text, block) { - - text = text.replace(/\{number\}/, block); - text = text.replace(/\{block\}/, wDiffStyleBlock[block]); - - return(text); -} - - -// HtmlFormat: replaces newlines and multiple spaces in text with html code - -window.WDiffHtmlFormat = function(text) { - - text = text.replace(/ /g, '  '); - text = text.replace(/\n/g, '
'); - - return(text); -} - - -// WDiffDetectBlocks: detect block borders and moved blocks -// input: text object, block object - -window.WDiffDetectBlocks = function(text, block) { - - block.oldStart = []; - block.oldToNew = []; - block.oldLength = []; - block.oldWords = []; - block.newStart = []; - block.newLength = []; - block.newWords = []; - block.newNumber = []; - block.newBlock = []; - block.newLeft = []; - block.newRight = []; - block.newLeftIndex = []; - block.newRightIndex = []; - - var blockNumber = 0; - var wordCounter = 0; - var realWordCounter = 0; - -// get old text block order - if (wDiffShowBlockMoves) { - var j = 0; - var i = 0; - do { - -// detect block boundaries on old text - if ( (text.oldToNew[j] != i) || (blockNumber == 0 ) ) { - if ( ( (text.oldToNew[j] != null) || (j >= text.oldWords.length) ) && ( (text.newToOld[i] != null) || (i >= text.newWords.length) ) ) { - if (blockNumber > 0) { - block.oldLength[blockNumber - 1] = wordCounter; - block.oldWords[blockNumber - 1] = realWordCounter; - wordCounter = 0; - realWordCounter = 0; - } - - if (j >= text.oldWords.length) { - j ++; - } - else { - i = text.oldToNew[j]; - block.oldStart[blockNumber] = j; - block.oldToNew[blockNumber] = text.oldToNew[j]; - blockNumber ++; - } - } - } - -// jump over identical pairs - while ( (i < text.newWords.length) && (j < text.oldWords.length) ) { - if ( (text.newToOld[i] == null) || (text.oldToNew[j] == null) ) { - break; - } - if (text.oldToNew[j] != i) { - break; - } - i ++; - j ++; - wordCounter ++; - if ( /\w/.test( text.newWords[i] ) ) { - realWordCounter ++; - } - } - -// jump over consecutive deletions - while ( (text.oldToNew[j] == null) && (j < text.oldWords.length) ) { - j ++; - } - -// jump over consecutive inserts - while ( (text.newToOld[i] == null) && (i < text.newWords.length) ) { - i ++; - } - } while (j <= text.oldWords.length); - -// get the block order in the new text - var lastMin; - var currMinIndex; - lastMin = null; - -// sort the data by increasing start numbers into new text block info - for (var i = 0; i < blockNumber; i ++) { - currMin = null; - for (var j = 0; j < blockNumber; j ++) { - curr = block.oldToNew[j]; - if ( (curr > lastMin) || (lastMin == null) ) { - if ( (curr < currMin) || (currMin == null) ) { - currMin = curr; - currMinIndex = j; - } - } - } - block.newStart[i] = block.oldToNew[currMinIndex]; - block.newLength[i] = block.oldLength[currMinIndex]; - block.newWords[i] = block.oldWords[currMinIndex]; - block.newNumber[i] = currMinIndex; - lastMin = currMin; - } - -// detect not moved blocks - for (var i = 0; i < blockNumber; i ++) { - if (block.newBlock[i] == null) { - if (block.newNumber[i] == i) { - block.newBlock[i] = 0; - } - } - } - -// detect switches of neighbouring blocks - for (var i = 0; i < blockNumber - 1; i ++) { - if ( (block.newBlock[i] == null) && (block.newBlock[i + 1] == null) ) { - if (block.newNumber[i] - block.newNumber[i + 1] == 1) { - if ( (block.newNumber[i + 1] - block.newNumber[i + 2] != 1) || (i + 2 >= blockNumber) ) { - -// the shorter one is declared the moved one - if (block.newLength[i] < block.newLength[i + 1]) { - block.newBlock[i] = 1; - block.newBlock[i + 1] = 0; - } - else { - block.newBlock[i] = 0; - block.newBlock[i + 1] = 1; - } - } - } - } - } - -// mark all others as moved and number the moved blocks - j = 1; - for (var i = 0; i < blockNumber; i ++) { - if ( (block.newBlock[i] == null) || (block.newBlock[i] == 1) ) { - block.newBlock[i] = j++; - } - } - -// check if a block has been moved from this block border - for (var i = 0; i < blockNumber; i ++) { - for (var j = 0; j < blockNumber; j ++) { - - if (block.newNumber[j] == i) { - if (block.newBlock[j] > 0) { - -// block moved right - if (block.newNumber[j] < j) { - block.newRight[i] = block.newBlock[j]; - block.newRightIndex[i] = j; - } - -// block moved left - else { - block.newLeft[i + 1] = block.newBlock[j]; - block.newLeftIndex[i + 1] = j; - } - } - } - } - } - } - return; -} - - -// WDiffShortenOutput: remove unchanged parts from final output -// input: the output of WDiffString -// returns: the text with removed unchanged passages indicated by (...) - -window.WDiffShortenOutput = function(diffText) { - -// html
to newlines - diffText = diffText.replace(/]*>/g, '\n'); - -// scan for diff html tags - var regExpDiff = new RegExp('<\\w+ class=\\"(\\w+)\\"[^>]*>(.|\\n)*?', 'g'); - var tagStart = []; - var tagEnd = []; - var i = 0; - var found; - while ( (found = regExpDiff.exec(diffText)) != null ) { - -// combine consecutive diff tags - if ( (i > 0) && (tagEnd[i - 1] == found.index) ) { - tagEnd[i - 1] = found.index + found[0].length; - } - else { - tagStart[i] = found.index; - tagEnd[i] = found.index + found[0].length; - i ++; - } - } - -// no diff tags detected - if (tagStart.length == 0) { - return(wDiffNoChange); - } - -// define regexps - var regExpHeading = new RegExp('\\n=+.+?=+ *\\n|\\n\\{\\||\\n\\|\\}', 'g'); - var regExpParagraph = new RegExp('\\n\\n+', 'g'); - var regExpLine = new RegExp('\\n+', 'g'); - var regExpBlank = new RegExp('(<[^>]+>)*\\s+', 'g'); - -// determine fragment border positions around diff tags - var rangeStart = []; - var rangeEnd = []; - var rangeStartType = []; - var rangeEndType = []; - for (var i = 0; i < tagStart.length; i ++) { - var found; - -// find last heading before diff tag - var lastPos = tagStart[i] - wDiffHeadingBefore; - if (lastPos < 0) { - lastPos = 0; - } - regExpHeading.lastIndex = lastPos; - while ( (found = regExpHeading.exec(diffText)) != null ) { - if (found.index > tagStart[i]) { - break; - } - rangeStart[i] = found.index; - rangeStartType[i] = 'heading'; - } - -// find last paragraph before diff tag - if (rangeStart[i] == null) { - lastPos = tagStart[i] - wDiffParagraphBefore; - if (lastPos < 0) { - lastPos = 0; - } - regExpParagraph.lastIndex = lastPos; - while ( (found = regExpParagraph.exec(diffText)) != null ) { - if (found.index > tagStart[i]) { - break; - } - rangeStart[i] = found.index; - rangeStartType[i] = 'paragraph'; - } - } - -// find line break before diff tag - if (rangeStart[i] == null) { - lastPos = tagStart[i] - wDiffLineBeforeMax; - if (lastPos < 0) { - lastPos = 0; - } - regExpLine.lastIndex = lastPos; - while ( (found = regExpLine.exec(diffText)) != null ) { - if (found.index > tagStart[i] - wDiffLineBeforeMin) { - break; - } - rangeStart[i] = found.index; - rangeStartType[i] = 'line'; - } - } - -// find blank before diff tag - if (rangeStart[i] == null) { - lastPos = tagStart[i] - wDiffBlankBeforeMax; - if (lastPos < 0) { - lastPos = 0; - } - regExpBlank.lastIndex = lastPos; - while ( (found = regExpBlank.exec(diffText)) != null ) { - if (found.index > tagStart[i] - wDiffBlankBeforeMin) { - break; - } - rangeStart[i] = found.index; - rangeStartType[i] = 'blank'; - } - } - -// fixed number of chars before diff tag - if (rangeStart[i] == null) { - rangeStart[i] = tagStart[i] - wDiffCharsBefore; - rangeStartType[i] = 'chars'; - if (rangeStart[i] < 0) { - rangeStart[i] = 0; - } - } - -// find first heading after diff tag - regExpHeading.lastIndex = tagEnd[i]; - if ( (found = regExpHeading.exec(diffText)) != null ) { - if (found.index < tagEnd[i] + wDiffHeadingAfter) { - rangeEnd[i] = found.index + found[0].length; - rangeEndType[i] = 'heading'; - } - } - -// find first paragraph after diff tag - if (rangeEnd[i] == null) { - regExpParagraph.lastIndex = tagEnd[i]; - if ( (found = regExpParagraph.exec(diffText)) != null ) { - if (found.index < tagEnd[i] + wDiffParagraphAfter) { - rangeEnd[i] = found.index; - rangeEndType[i] = 'paragraph'; - } - } - } - -// find first line break after diff tag - if (rangeEnd[i] == null) { - regExpLine.lastIndex = tagEnd[i] + wDiffLineAfterMin; - if ( (found = regExpLine.exec(diffText)) != null ) { - if (found.index < tagEnd[i] + wDiffLineAfterMax) { - rangeEnd[i] = found.index; - rangeEndType[i] = 'break'; - } - } - } - -// find blank after diff tag - if (rangeEnd[i] == null) { - regExpBlank.lastIndex = tagEnd[i] + wDiffBlankAfterMin; - if ( (found = regExpBlank.exec(diffText)) != null ) { - if (found.index < tagEnd[i] + wDiffBlankAfterMax) { - rangeEnd[i] = found.index; - rangeEndType[i] = 'blank'; - } - } - } - -// fixed number of chars after diff tag - if (rangeEnd[i] == null) { - rangeEnd[i] = tagEnd[i] + wDiffCharsAfter; - if (rangeEnd[i] > diffText.length) { - rangeEnd[i] = diffText.length; - rangeEndType[i] = 'chars'; - } - } - } - -// remove overlaps, join close fragments - var fragmentStart = []; - var fragmentEnd = []; - var fragmentStartType = []; - var fragmentEndType = []; - fragmentStart[0] = rangeStart[0]; - fragmentEnd[0] = rangeEnd[0]; - fragmentStartType[0] = rangeStartType[0]; - fragmentEndType[0] = rangeEndType[0]; - var j = 1; - for (var i = 1; i < rangeStart.length; i ++) { - if (rangeStart[i] > fragmentEnd[j - 1] + wDiffFragmentJoin) { - fragmentStart[j] = rangeStart[i]; - fragmentEnd[j] = rangeEnd[i]; - fragmentStartType[j] = rangeStartType[i]; - fragmentEndType[j] = rangeEndType[i]; - j ++; - } - else { - fragmentEnd[j - 1] = rangeEnd[i]; - fragmentEndType[j - 1] = rangeEndType[i]; - } - } - -// assemble the fragments - var outText = ''; - for (var i = 0; i < fragmentStart.length; i ++) { - -// get text fragment - var fragment = diffText.substring(fragmentStart[i], fragmentEnd[i]); - var fragment = fragment.replace(/^\n+|\n+$/g, ''); - -// add inline marks for omitted chars and words - if (fragmentStart[i] > 0) { - if (fragmentStartType[i] == 'chars') { - fragment = wDiffOmittedChars + fragment; - } - else if (fragmentStartType[i] == 'blank') { - fragment = wDiffOmittedChars + ' ' + fragment; - } - } - if (fragmentEnd[i] < diffText.length) { - if (fragmentStartType[i] == 'chars') { - fragment = fragment + wDiffOmittedChars; - } - else if (fragmentStartType[i] == 'blank') { - fragment = fragment + ' ' + wDiffOmittedChars; - } - } - -// add omitted line separator - if (fragmentStart[i] > 0) { - outText += wDiffOmittedLines; - } - -// encapsulate span errors - outText += '
' + fragment + '
'; - } - -// add trailing omitted line separator - if (fragmentEnd[i - 1] < diffText.length) { - outText = outText + wDiffOmittedLines; - } - -// remove leading and trailing empty lines - outText = outText.replace(/^(
)\n+|\n+(<\/div>)$/g, '$1$2'); - -// convert to html linebreaks - outText = outText.replace(/\n/g, '
'); - - return(outText); -} - - -//

\ No newline at end of file
diff --git a/public/stylesheets/admin.css b/public/stylesheets/admin.css
index 38c9e5a..1bb6cf4 100644
--- a/public/stylesheets/admin.css
+++ b/public/stylesheets/admin.css
@@ -511,6 +511,31 @@ table.revisions_table tr:hover {
   background-color: #f1f1f1;
 }
 
+#diffview del {
+  background: #ffd7d5;
+  color: #82071e;
+  text-decoration: line-through;
+  padding: 0 2px;
+  border-radius: 2px;
+}
+
+#diffview ins {
+  background: #ccffd8;
+  color: #055d20;
+  text-decoration: none;
+  padding: 0 2px;
+  border-radius: 2px;
+}
+
+#diffview .diff_side_by_side {
+  display: flex;
+  gap: 1rem;
+}
+
+#diffview .diff_column {
+  flex: 1;
+}
+
 table.user_table td.user_login {
   padding-right: 30px;
 }
diff --git a/test/controllers/revisions_controller_test.rb b/test/controllers/revisions_controller_test.rb
index b4dcd8f..e2fc976 100644
--- a/test/controllers/revisions_controller_test.rb
+++ b/test/controllers/revisions_controller_test.rb
@@ -59,4 +59,33 @@ class RevisionsControllerTest < ActionController::TestCase
     assert_equal @node.head, @node.pages.first
     assert_equal "first", @node.head.reload.body
   end
+
+  test "diffing two revisions renders real markup with only the changed words marked" do
+    login_as :quentin
+    post(
+      :diff, params: {
+        :node_id => @node.id,
+        :start_revision => @node.pages.first.revision,
+        :end_revision => @node.pages.last.revision
+      }
+    )
+    assert_response :success
+    assert_select "del", "first"
+    assert_select "ins", "second"
+    assert_no_match /</, response.body
+  end
+
+  test "diffing two revisions in side by side view renders two columns" do
+    login_as :quentin
+    post(
+      :diff, params: {
+        :node_id => @node.id,
+        :start_revision => @node.pages.first.revision,
+        :end_revision => @node.pages.last.revision,
+        :view => "side_by_side"
+      }
+    )
+    assert_response :success
+    assert_select ".diff_column", 2
+  end
 end
diff --git a/test/models/page_test.rb b/test/models/page_test.rb
index ad2742f..ac5691a 100644
--- a/test/models/page_test.rb
+++ b/test/models/page_test.rb
@@ -176,4 +176,38 @@ class PageTest < ActiveSupport::TestCase
     assert_not_equal first_page.id, first_page.node.head_id
     assert first_page.published_at.present?
   end
+
+  def test_diff_against_inline_keeps_tags_and_marks_only_the_changed_word
+    n = Node.root.children.create! :slug => "diff_against_test"
+    d = n.find_or_create_draft @user1
+    d.title = "Old heading"
+    d.save!
+    n.publish_draft!
+
+    d2 = n.find_or_create_draft @user1
+    d2.title = "New heading"
+    d2.save!
+
+    diff = d2.diff_against(n.head)
+
+    assert_match "Old", diff[:title]
+    assert_match "New", diff[:title]
+  end
+
+  def test_diff_against_side_by_side_returns_two_annotated_strings
+    n = Node.root.children.create! :slug => "diff_against_sbs_test"
+    d = n.find_or_create_draft @user1
+    d.title = "Old heading"
+    d.save!
+    n.publish_draft!
+
+    d2 = n.find_or_create_draft @user1
+    d2.title = "New heading"
+    d2.save!
+
+    old_html, new_html = d2.diff_against(n.head, view: :side_by_side)[:title]
+
+    assert_match "Old", old_html
+    assert_match "New", new_html
+  end
 end
-- 
cgit v1.3


From cdf861a23233afb955b3c722a73d9e6ac8de3942 Mon Sep 17 00:00:00 2001
From: erdgeist 
Date: Fri, 10 Jul 2026 00:44:22 +0200
Subject: Fix stale assertions and escaped body in revisions#show
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The table-based markup revisions#show's own test was asserting
against (, ) was retired when this view moved to the
node_description/node_content pattern; updated the assertions to
match. Found in the same pass: @page.body had no raw(), so any real
markup in a revision's body rendered as escaped entities on this
page — same bug class as revisions#diff, just not yet fixed here.
---
 app/views/revisions/show.html.erb             |  2 +-
 test/controllers/revisions_controller_test.rb | 24 ++++++++++++++++++------
 2 files changed, 19 insertions(+), 7 deletions(-)

(limited to 'test/controllers')

diff --git a/app/views/revisions/show.html.erb b/app/views/revisions/show.html.erb
index 4148096..92d959b 100644
--- a/app/views/revisions/show.html.erb
+++ b/app/views/revisions/show.html.erb
@@ -33,6 +33,6 @@
     
<%= @page.abstract %>
Body
-
<%= @page.body %>
+
<%= raw @page.body %>
diff --git a/test/controllers/revisions_controller_test.rb b/test/controllers/revisions_controller_test.rb index e2fc976..46fc220 100644 --- a/test/controllers/revisions_controller_test.rb +++ b/test/controllers/revisions_controller_test.rb @@ -28,21 +28,33 @@ class RevisionsControllerTest < ActionController::TestCase assert_response :success assert_select ".revision", 2 end - - test "showing one revision" do + + test "showing one revision" do login_as :quentin get :show, params: { :node_id => @node.id, :id => @node.pages.last.id } assert_response :success - assert_select "strong", "Body" - assert_select "td", {:count => 1, :text => "second"} + assert_select ".node_description", "Body" + assert_select ".node_content", {:count => 1, :text => "second"} end - + + test "showing a revision renders real markup in the body, not escaped entities" do + login_as :quentin + node = Node.root.children.create!(:slug => "show_markup_test") + draft = node.draft + draft.body = "

Hello

" + node.publish_draft! + + get :show, params: { :node_id => node.id, :id => node.head.id } + assert_response :success + assert_select ".node_content h3", "Hello" + end + test "diffing two revisions" do login_as :quentin post( :diff, params: { :node_id => @node.id, - :start_revision => @node.pages.first.revision, + :start_revision => @node.pages.first.revision, :end_revision => @node.pages.last.revision } ) -- cgit v1.3 From 81769557804ce59ce3e4603e8f47e731b61b180d Mon Sep 17 00:00:00 2001 From: erdgeist Date: Fri, 10 Jul 2026 00:44:41 +0200 Subject: Whitespace fixes --- test/controllers/revisions_controller_test.rb | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) (limited to 'test/controllers') diff --git a/test/controllers/revisions_controller_test.rb b/test/controllers/revisions_controller_test.rb index 46fc220..caca6bf 100644 --- a/test/controllers/revisions_controller_test.rb +++ b/test/controllers/revisions_controller_test.rb @@ -1,12 +1,12 @@ require 'test_helper' class RevisionsControllerTest < ActionController::TestCase - + def setup Node.root.descendants.destroy_all @user = User.find_by_login("aaron") @node = Node.root.children.create!( :slug => "version_me" ) - + draft = @node.draft draft.body = "first" @node.publish_draft! @@ -15,13 +15,13 @@ class RevisionsControllerTest < ActionController::TestCase draft.update(:body => "second") @node.publish_draft! end - + test "setup" do assert_equal 2, Node.count assert_equal 2, @node.pages.count assert_equal ["first", "second"], @node.pages.map {|p| p.body} end - + test "get list of revisions for a given node" do login_as :quentin get :index, params: { :node_id => @node.id } @@ -60,13 +60,13 @@ class RevisionsControllerTest < ActionController::TestCase ) assert_response :success end - + test "restoring a revision" do assert_equal "second", @node.head.body - + login_as :aaron put( :restore, params: { :node_id => @node.id, :id => @node.pages.first.id } ) - + @node.reload assert_equal @node.head, @node.pages.first assert_equal "first", @node.head.reload.body -- cgit v1.3 From 205e6216fc7850fe717122c189e5003d1f9e8afe Mon Sep 17 00:00:00 2001 From: erdgeist Date: Fri, 10 Jul 2026 02:03:19 +0200 Subject: Add head/draft/autosave layer comparison at three UI entry points 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". --- app/controllers/admin_controller.rb | 4 +-- app/controllers/nodes_controller.rb | 2 +- app/controllers/revisions_controller.rb | 16 ++++++++--- app/helpers/revisions_helper.rb | 5 ++++ app/models/node.rb | 21 ++++++++++++++ app/views/admin/index.html.erb | 13 +++++++-- app/views/nodes/edit.html.erb | 9 +++++- app/views/nodes/show.html.erb | 12 +++++++- app/views/revisions/diff.html.erb | 40 +++++++++++++++++++++++---- config/routes.rb | 1 + test/controllers/admin_controller_test.rb | 15 ++++++++-- test/controllers/revisions_controller_test.rb | 39 ++++++++++++++++++++++++++ test/models/node_test.rb | 22 +++++++++++++++ 13 files changed, 179 insertions(+), 20 deletions(-) (limited to 'test/controllers') diff --git a/app/controllers/admin_controller.rb b/app/controllers/admin_controller.rb index 3fa0519..6ab2135 100644 --- a/app/controllers/admin_controller.rb +++ b/app/controllers/admin_controller.rb @@ -5,10 +5,10 @@ class AdminController < ApplicationController before_action :login_required def index - @drafts = Node.where("draft_id IS NOT NULL") + @drafts = Node.where("draft_id IS NOT NULL OR autosave_id IS NOT NULL") .limit(50).order("updated_at desc") - @drafts_count = Node.where("draft_id IS NOT NULL").count + @drafts_count = Node.where("draft_id IS NOT NULL OR autosave_id IS NOT NULL").count @recent_changes = Node.where( "updated_at < ? AND updated_at > ? AND parent_id IS NOT NULL", diff --git a/app/controllers/nodes_controller.rb b/app/controllers/nodes_controller.rb index 38d42d9..d1538e1 100644 --- a/app/controllers/nodes_controller.rb +++ b/app/controllers/nodes_controller.rb @@ -72,7 +72,7 @@ class NodesController < ApplicationController if @node.autosave flash.now[:notice] = "This page has unsaved changes from a previous session, shown below. " \ - "Save to keep them, or use \"Discard changes\" below to go back to the last saved version." + "Save to keep them, or use \"Discard Autosave\" below to go back to the last saved version." elsif freshly_locked flash.now[:notice] = "Node locked and ready to edit" end diff --git a/app/controllers/revisions_controller.rb b/app/controllers/revisions_controller.rb index 9acb26f..4b0c549 100644 --- a/app/controllers/revisions_controller.rb +++ b/app/controllers/revisions_controller.rb @@ -21,10 +21,18 @@ class RevisionsController < ApplicationController params[:start_revision], params[:end_revision] = 1, 1 end - @start = @node.pages.find_by_revision( params[:start_revision] ) - @end = @node.pages.find_by_revision( params[:end_revision] ) - @diff_view = params[:view] == "side_by_side" ? :side_by_side : :inline - @diff = @end.diff_against( @start, view: @diff_view ) + @start = @node.resolve_page_reference(params[:start_revision]) + @end = @node.resolve_page_reference(params[:end_revision]) + + if @start.nil? || @end.nil? + flash[:error] = "That comparison is no longer available." + redirect_to(node_path(@node)) and return + end + + @diff_view = params[:view] == "side_by_side" ? :side_by_side : :inline + @diff = @end.diff_against(@start, view: @diff_view) + @available_layer_pairs = @node.available_layer_pairs + @locked_by_other = @node.locked? && @node.lock_owner != current_user end def show diff --git a/app/helpers/revisions_helper.rb b/app/helpers/revisions_helper.rb index fdb51f8..a629013 100644 --- a/app/helpers/revisions_helper.rb +++ b/app/helpers/revisions_helper.rb @@ -1,2 +1,7 @@ module RevisionsHelper + # Human-readable label for a diff endpoint -- "head"/"draft"/"autosave" + # get their name; anything else is a revision number. + def describe_page_reference(ref) + %w[head draft autosave].include?(ref.to_s) ? ref.to_s.capitalize : "revision #{ref}" + end end diff --git a/app/models/node.rb b/app/models/node.rb index 7675ab6..82d9954 100644 --- a/app/models/node.rb +++ b/app/models/node.rb @@ -123,6 +123,27 @@ class Node < ApplicationRecord self.draft.reload end + def resolve_page_reference ref + case ref.to_s + when "head" then head + when "draft" then draft + when "autosave" then autosave + else pages.find_by_revision(ref) + end + end + + # Which layer-pairs are meaningful to compare right now, given this + # node's actual state. Head vs autosave only shows up when no draft + # sits between them -- with a draft present, autosave is compared + # against the draft, never past it straight to head. + def available_layer_pairs + pairs = [] + pairs << [:head, :draft] if head && draft + pairs << [:draft, :autosave] if draft && autosave + pairs << [:head, :autosave] if head && autosave && !draft + pairs + end + def find_or_create_draft current_user self.wipe_draft! if draft && self.lock_owner == current_user diff --git a/app/views/admin/index.html.erb b/app/views/admin/index.html.erb index 77b45f4..c67ccb3 100644 --- a/app/views/admin/index.html.erb +++ b/app/views/admin/index.html.erb @@ -82,9 +82,9 @@
- -

Current Drafts (<%= @drafts_count %>)

- + +

Current Drafts & Autosaves (<%= @drafts_count %>)

+ @@ -92,6 +92,7 @@ + <% @drafts.each do |node| %> "> @@ -103,6 +104,9 @@ + <% end %>
IDActions Locked by Rev.Autosave
<%= link_to 'show', node_path(node) %> <%= link_to 'Revisions', node_revisions_path(node) %> + <% if pair = node.available_layer_pairs.last %> + <%= link_to 'diff', diff_node_revisions_path(node, start_revision: pair.first, end_revision: pair.last) %> + <% end %> <%= node.lock_owner.login if node.lock_owner %> @@ -110,6 +114,9 @@ <%= link_to ( node.draft ? node.draft.revision : (node.head ? node.head.revision : "EMPTY" ) ), node_revisions_path(node) %> + <%= node.autosave ? "unsaved changes" : "" %> +
diff --git a/app/views/nodes/edit.html.erb b/app/views/nodes/edit.html.erb index cdc9b36..1c19410 100644 --- a/app/views/nodes/edit.html.erb +++ b/app/views/nodes/edit.html.erb @@ -6,9 +6,16 @@ disabled: @node.autosave.present? %> <% if @node.autosave || (@node.draft && @node.head) %> - <%= button_to (@node.draft && !@node.autosave ? 'Destroy Draft' : 'Discard changes'), + <%= button_to (@node.draft && !@node.autosave ? 'Destroy Draft' : 'Discard Autosave'), revert_node_path(@node), method: :put, form: { data: { confirm: "This cannot be undone. Continue?" }, class: 'button_to destructive' } %> + <% if pair = @node.available_layer_pairs.find { |p| p.include?(:autosave) } %> + <%= button_to 'What changed?', + diff_node_revisions_path(@node), + method: :get, + params: { start_revision: pair.first, end_revision: pair.last }, + form: { class: 'button_to computation' } %> + <% end %> <% end %> <%= submit_tag "Save Draft", form: dom_id(@node, :edit) %> diff --git a/app/views/nodes/show.html.erb b/app/views/nodes/show.html.erb index 036caf2..2469310 100644 --- a/app/views/nodes/show.html.erb +++ b/app/views/nodes/show.html.erb @@ -32,6 +32,16 @@ <% end %>
+ <% @node.available_layer_pairs.each do |pair| %> +
+ <%= button_to "Diff #{pair.first.to_s.capitalize} vs. #{pair.last.to_s.capitalize}", + diff_node_revisions_path(@node), + method: :get, + params: { start_revision: pair.first, end_revision: pair.last }, + form: { class: 'button_to computation' } %> +
+ <% end %> + <% unless locked_by_other %> <% if @node.draft && !@node.autosave %>
@@ -41,7 +51,7 @@ <% end %> <% if @node.draft || @node.autosave %>
- <%= button_to (@node.draft && !@node.autosave ? 'Destroy Draft' : 'Discard changes'), + <%= button_to (@node.draft && !@node.autosave ? 'Destroy Draft' : 'Discard Autosave'), revert_node_path(@node), method: :put, form: { data: { confirm: "This cannot be undone. Continue?" }, class: 'button_to destructive' } %>
diff --git a/app/views/revisions/diff.html.erb b/app/views/revisions/diff.html.erb index d7bb528..3157dca 100644 --- a/app/views/revisions/diff.html.erb +++ b/app/views/revisions/diff.html.erb @@ -4,11 +4,41 @@ <%= link_to 'Revisions', node_revisions_path(@node) %>

-<%= form_tag diff_node_revisions_path do %> - <%= select_tag :start_revision, options_for_select(@node.pages.map{|x| x.revision}, params[:start_revision].to_i) %> - <%= select_tag :end_revision, options_for_select(@node.pages.map{|x| x.revision}, params[:end_revision].to_i) %> - <%= select_tag :view, options_for_select([['Inline', 'inline'], ['Side by side', 'side_by_side']], @diff_view) %> - <%= submit_tag 'Diff' %> +

+ Comparing <%= describe_page_reference(params[:start_revision]) %> + against <%= describe_page_reference(params[:end_revision]) %> +

+ +<% numeric_comparison = params[:start_revision].to_s =~ /\A\d+\z/ && params[:end_revision].to_s =~ /\A\d+\z/ %> + +<% if numeric_comparison %> + <%= form_tag diff_node_revisions_path do %> + <%= select_tag :start_revision, options_for_select(@node.pages.map{|x| x.revision}, params[:start_revision].to_i) %> + <%= select_tag :end_revision, options_for_select(@node.pages.map{|x| x.revision}, params[:end_revision].to_i) %> + <%= select_tag :view, options_for_select([['Inline', 'inline'], ['Side by side', 'side_by_side']], @diff_view) %> + <%= submit_tag 'Diff' %> + <% end %> +<% else %> +

<%= link_to 'Compare two numbered revisions instead', node_revisions_path(@node) %>

+<% end %> + +<% if @available_layer_pairs.present? %> +

+ <% @available_layer_pairs.each do |pair| %> + <% next if [params[:start_revision].to_s, params[:end_revision].to_s].sort == pair.map(&:to_s).sort %> + <%= button_to "Diff #{pair.first.to_s.capitalize} vs. #{pair.last.to_s.capitalize}", + diff_node_revisions_path(@node), + method: :get, + params: { start_revision: pair.first, end_revision: pair.last, view: @diff_view }, + form: { class: 'button_to computation' } %> + <% end %> + + <% if !@locked_by_other && (@node.autosave || @node.draft) %> + <%= button_to (@node.draft && !@node.autosave ? 'Destroy Draft' : 'Discard Autosave'), + revert_node_path(@node), method: :put, + form: { data: { confirm: "This cannot be undone. Continue?" }, class: 'button_to destructive' } %> + <% end %> +

<% end %>
diff --git a/config/routes.rb b/config/routes.rb index c0aef2f..da6b626 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -47,6 +47,7 @@ Cccms::Application.routes.draw do resources :revisions do collection do post :diff + get :diff end member do put :restore diff --git a/test/controllers/admin_controller_test.rb b/test/controllers/admin_controller_test.rb index 9bbf29b..d6005ba 100644 --- a/test/controllers/admin_controller_test.rb +++ b/test/controllers/admin_controller_test.rb @@ -1,8 +1,17 @@ require 'test_helper' class AdminControllerTest < ActionController::TestCase - # Replace this with your real tests. - test "the truth" do - assert true + test "current drafts includes nodes with only an autosave" do + node = Node.root.children.create!(:slug => "admin_autosave_only") + node.lock_for_editing!(User.find_by_login("aaron")) + node.autosave!({title: "in progress"}, User.find_by_login("aaron")) + node.save_draft!(User.find_by_login("aaron")) + node.publish_draft! + node.lock_for_editing!(User.find_by_login("aaron")) + node.autosave!({title: "editing again"}, User.find_by_login("aaron")) + + login_as :quentin + get :index + assert_includes assigns(:drafts), node end end diff --git a/test/controllers/revisions_controller_test.rb b/test/controllers/revisions_controller_test.rb index caca6bf..162e6f1 100644 --- a/test/controllers/revisions_controller_test.rb +++ b/test/controllers/revisions_controller_test.rb @@ -100,4 +100,43 @@ class RevisionsControllerTest < ActionController::TestCase assert_response :success assert_select ".diff_column", 2 end + + test "diffing head against draft by name" do + login_as :quentin + @node.find_or_create_draft(@user) + @node.draft.update(:body => "draft body") + + post(:diff, params: { :node_id => @node.id, :start_revision => "head", :end_revision => "draft" }) + assert_response :success + end + + test "diffing a layer pair that no longer exists redirects with a flash" do + login_as :quentin + post(:diff, params: { :node_id => @node.id, :start_revision => "draft", :end_revision => "autosave" }) + assert_redirected_to node_path(@node) + assert flash[:error].present? + end + + test "diffing by name shows a clear comparison label instead of a misleading revision picker" do + login_as :quentin + @node.find_or_create_draft(@user) + + post(:diff, params: { :node_id => @node.id, :start_revision => "head", :end_revision => "draft" }) + assert_response :success + assert_select "strong", "Head" + assert_select "strong", "Draft" + assert_select "select[name=?]", "start_revision", :count => 0 + end + + test "pair-switcher buttons carry their params as real hidden fields, not a query string" do + login_as :quentin + @node.find_or_create_draft(@user) + @node.lock_for_editing!(@user) + @node.autosave!({ :body => "unsaved" }, @user) + + post(:diff, params: { :node_id => @node.id, :start_revision => "head", :end_revision => "draft" }) + assert_response :success + assert_select "form.computation input[type=hidden][name=start_revision]" + assert_select "form.computation input[type=hidden][name=end_revision]" + end end diff --git a/test/models/node_test.rb b/test/models/node_test.rb index 2138c19..9e71dec 100644 --- a/test/models/node_test.rb +++ b/test/models/node_test.rb @@ -473,4 +473,26 @@ class NodeTest < ActiveSupport::TestCase node.publish_draft! end end + + test "available_layer_pairs matches the six-state table" do + node = Node.root.children.create!(:slug => "layer_pairs_test") + user = @user1 || User.find_by_login("aaron") + + assert_equal [[:draft, :autosave]], (node.lock_for_editing!(user); node.autosave!({title: "v1"}, user); node.available_layer_pairs) # state F + + node.save_draft!(user) + node.publish_draft! + assert_equal [], node.available_layer_pairs # state A + + node.lock_for_editing!(user) + node.autosave!({title: "v2"}, user) + assert_equal [[:head, :autosave]], node.available_layer_pairs # state B + + node.save_draft!(user) + assert_equal [[:head, :draft]], node.available_layer_pairs # state C + + node.lock_for_editing!(user) + node.autosave!({title: "v3"}, user) + assert_equal [[:head, :draft], [:draft, :autosave]], node.available_layer_pairs # state D + end end -- cgit v1.3 From c2b2648d327e1c1749c37fe2e58cd051ed871547 Mon Sep 17 00:00:00 2001 From: erdgeist Date: Fri, 10 Jul 2026 02:13:02 +0200 Subject: Destroying Draft or Discarding Autosave drops you where you left --- app/controllers/nodes_controller.rb | 5 +++- app/views/nodes/show.html.erb | 1 + app/views/revisions/diff.html.erb | 1 + test/controllers/nodes_controller_test.rb | 39 +++++++++++++++++++++++++++++++ 4 files changed, 45 insertions(+), 1 deletion(-) (limited to 'test/controllers') diff --git a/app/controllers/nodes_controller.rb b/app/controllers/nodes_controller.rb index d1538e1..6fcd930 100644 --- a/app/controllers/nodes_controller.rb +++ b/app/controllers/nodes_controller.rb @@ -128,7 +128,10 @@ class NodesController < ApplicationController def revert @node.lock_for_editing!(current_user) @node.revert!(current_user) - if @node.draft + + if params[:return_to].present? + redirect_to safe_return_to(params[:return_to]) + elsif @node.draft redirect_to edit_node_path(@node) else redirect_to node_path(@node) diff --git a/app/views/nodes/show.html.erb b/app/views/nodes/show.html.erb index 2469310..8b9e98b 100644 --- a/app/views/nodes/show.html.erb +++ b/app/views/nodes/show.html.erb @@ -53,6 +53,7 @@
<%= button_to (@node.draft && !@node.autosave ? 'Destroy Draft' : 'Discard Autosave'), revert_node_path(@node), method: :put, + params: { return_to: request.path }, form: { data: { confirm: "This cannot be undone. Continue?" }, class: 'button_to destructive' } %>
<% end %> diff --git a/app/views/revisions/diff.html.erb b/app/views/revisions/diff.html.erb index 3157dca..490cf17 100644 --- a/app/views/revisions/diff.html.erb +++ b/app/views/revisions/diff.html.erb @@ -36,6 +36,7 @@ <% if !@locked_by_other && (@node.autosave || @node.draft) %> <%= button_to (@node.draft && !@node.autosave ? 'Destroy Draft' : 'Discard Autosave'), revert_node_path(@node), method: :put, + params: { return_to: request.fullpath }, form: { data: { confirm: "This cannot be undone. Continue?" }, class: 'button_to destructive' } %> <% end %>

diff --git a/test/controllers/nodes_controller_test.rb b/test/controllers/nodes_controller_test.rb index f3e04c2..ce3419c 100644 --- a/test/controllers/nodes_controller_test.rb +++ b/test/controllers/nodes_controller_test.rb @@ -423,4 +423,43 @@ class NodesControllerTest < ActionController::TestCase assert_select "form.button_to.destructive", count: 0 end + test "reverting from nodes#show returns to the show page, not the editor, even if a draft remains" do + user = User.find_by_login("aaron") + node = Node.root.children.create!(:slug => "revert_return_to_test") + node.lock_for_editing!(user) + node.autosave!({:title => "v1"}, user) + node.save_draft!(user) + node.publish_draft! + node.lock_for_editing!(user) + node.autosave!({:title => "v2"}, user) + node.save_draft!(user) + node.lock_for_editing!(user) + node.autosave!({:title => "v3"}, user) + # state D: head, draft, and autosave all present, locked by aaron + + login_as :aaron + put :revert, params: { :id => node.id, :return_to => node_path(node) } + assert_redirected_to node_path(node) + node.reload + assert node.draft.present? + assert node.autosave.blank? + end + + test "reverting from nodes#edit without return_to still lands back in the editor when a draft remains" do + user = User.find_by_login("aaron") + node = Node.root.children.create!(:slug => "revert_default_test") + node.lock_for_editing!(user) + node.autosave!({:title => "v1"}, user) + node.save_draft!(user) + node.publish_draft! + node.lock_for_editing!(user) + node.autosave!({:title => "v2"}, user) + node.save_draft!(user) + node.lock_for_editing!(user) + node.autosave!({:title => "v3"}, user) + + login_as :aaron + put :revert, params: { :id => node.id } + assert_redirected_to edit_node_path(node) + end end -- cgit v1.3 From cf730b751b590ca80695492f5d62c4fb43efb4d3 Mon Sep 17 00:00:00 2001 From: erdgeist Date: Fri, 10 Jul 2026 03:40:48 +0200 Subject: Ensure that comparison view toggle is there for all diff modes --- app/views/revisions/diff.html.erb | 17 ++++++++++++++++- public/stylesheets/admin.css | 1 + test/controllers/revisions_controller_test.rb | 9 +++++++++ 3 files changed, 26 insertions(+), 1 deletion(-) (limited to 'test/controllers') diff --git a/app/views/revisions/diff.html.erb b/app/views/revisions/diff.html.erb index 490cf17..d70503c 100644 --- a/app/views/revisions/diff.html.erb +++ b/app/views/revisions/diff.html.erb @@ -9,13 +9,28 @@ against <%= describe_page_reference(params[:end_revision]) %>

+

+ View: + <% if @diff_view == :inline %> + Inline + <% else %> + <%= link_to 'Inline', diff_node_revisions_path(@node, start_revision: params[:start_revision], end_revision: params[:end_revision], view: 'inline') %> + <% end %> + · + <% if @diff_view == :side_by_side %> + Side by side + <% else %> + <%= link_to 'Side by side', diff_node_revisions_path(@node, start_revision: params[:start_revision], end_revision: params[:end_revision], view: 'side_by_side') %> + <% end %> +

+ <% numeric_comparison = params[:start_revision].to_s =~ /\A\d+\z/ && params[:end_revision].to_s =~ /\A\d+\z/ %> <% if numeric_comparison %> <%= form_tag diff_node_revisions_path do %> <%= select_tag :start_revision, options_for_select(@node.pages.map{|x| x.revision}, params[:start_revision].to_i) %> <%= select_tag :end_revision, options_for_select(@node.pages.map{|x| x.revision}, params[:end_revision].to_i) %> - <%= select_tag :view, options_for_select([['Inline', 'inline'], ['Side by side', 'side_by_side']], @diff_view) %> + <%= hidden_field_tag :view, @diff_view %> <%= submit_tag 'Diff' %> <% end %> <% else %> diff --git a/public/stylesheets/admin.css b/public/stylesheets/admin.css index 56470bc..510e3cc 100644 --- a/public/stylesheets/admin.css +++ b/public/stylesheets/admin.css @@ -32,6 +32,7 @@ table.user_table a, table.revisions_table a, .add_child_links a, .standalone_action_bar a, +.diff_view_toggle a, div#draft_list table td.actions a { text-decoration: underline; -webkit-text-decoration-style: wavy; diff --git a/test/controllers/revisions_controller_test.rb b/test/controllers/revisions_controller_test.rb index 162e6f1..bf92c5b 100644 --- a/test/controllers/revisions_controller_test.rb +++ b/test/controllers/revisions_controller_test.rb @@ -139,4 +139,13 @@ class RevisionsControllerTest < ActionController::TestCase assert_select "form.computation input[type=hidden][name=start_revision]" assert_select "form.computation input[type=hidden][name=end_revision]" end + + test "the view toggle is available even when comparing named layers" do + login_as :quentin + @node.find_or_create_draft(@user) + + post(:diff, params: { :node_id => @node.id, :start_revision => "head", :end_revision => "draft" }) + assert_response :success + assert_select "a", "Side by side" + end end -- cgit v1.3 From 30ed9fd9a8bffb44e6ab91dfedb8c0e33837769a Mon Sep 17 00:00:00 2001 From: erdgeist Date: Fri, 10 Jul 2026 04:41:54 +0200 Subject: Include assets, tags and template in diff between revisions --- app/models/page.rb | 33 ++++++++++++-------- app/views/revisions/diff.html.erb | 27 +++++++++++++++++ public/stylesheets/admin.css | 11 +++++++ test/controllers/revisions_controller_test.rb | 13 ++++++++ test/models/page_test.rb | 43 +++++++++++++++++++++++++++ 5 files changed, 114 insertions(+), 13 deletions(-) (limited to 'test/controllers') diff --git a/app/models/page.rb b/app/models/page.rb index ea04cd6..740d42e 100644 --- a/app/models/page.rb +++ b/app/models/page.rb @@ -176,19 +176,26 @@ class Page < ApplicationRecord end def diff_against other, view: :inline - if view == :side_by_side - { - title: HtmlWordDiff.side_by_side(other.title.to_s, title.to_s), - abstract: HtmlWordDiff.side_by_side(other.abstract.to_s, abstract.to_s), - body: HtmlWordDiff.side_by_side(other.body.to_s, body.to_s) - } - else - { - title: HtmlWordDiff.inline(other.title.to_s, title.to_s), - abstract: HtmlWordDiff.inline(other.abstract.to_s, abstract.to_s), - body: HtmlWordDiff.inline(other.body.to_s, body.to_s) - } - end + text_diffs = + if view == :side_by_side + { + title: HtmlWordDiff.side_by_side(other.title.to_s, title.to_s), + abstract: HtmlWordDiff.side_by_side(other.abstract.to_s, abstract.to_s), + body: HtmlWordDiff.side_by_side(other.body.to_s, body.to_s) + } + else + { + title: HtmlWordDiff.inline(other.title.to_s, title.to_s), + abstract: HtmlWordDiff.inline(other.abstract.to_s, abstract.to_s), + body: HtmlWordDiff.inline(other.body.to_s, body.to_s) + } + end + + text_diffs.merge( + tags: { added: tag_list.to_a - other.tag_list.to_a, removed: other.tag_list.to_a - tag_list.to_a }, + template_name: { from: other.template_name, to: template_name, changed: template_name != other.template_name }, + assets: { added: assets.to_a - other.assets.to_a, removed: other.assets.to_a - assets.to_a } + ) end def public? diff --git a/app/views/revisions/diff.html.erb b/app/views/revisions/diff.html.erb index d70503c..b9ce6bd 100644 --- a/app/views/revisions/diff.html.erb +++ b/app/views/revisions/diff.html.erb @@ -85,4 +85,31 @@

Body

<%= raw @diff[:body] %> <% end %> + +

Tags

+ <% if @diff[:tags][:added].empty? && @diff[:tags][:removed].empty? %> +

No change.

+ <% else %> +
    + <% @diff[:tags][:added].each do |tag| %>
  • <%= tag %>
  • <% end %> + <% @diff[:tags][:removed].each do |tag| %>
  • <%= tag %>
  • <% end %> +
+ <% end %> + +

Template

+ <% if @diff[:template_name][:changed] %> +

<%= @diff[:template_name][:from] || '(none)' %> <%= @diff[:template_name][:to] || '(none)' %>

+ <% else %> +

No change.

+ <% end %> + +

Assets

+ <% if @diff[:assets][:added].empty? && @diff[:assets][:removed].empty? %> +

No change.

+ <% else %> +
    + <% @diff[:assets][:added].each do |asset| %>
  • <%= asset.upload_file_name %>
  • <% end %> + <% @diff[:assets][:removed].each do |asset| %>
  • <%= asset.upload_file_name %>
  • <% end %> +
+ <% end %>
diff --git a/public/stylesheets/admin.css b/public/stylesheets/admin.css index e04499d..7b39c72 100644 --- a/public/stylesheets/admin.css +++ b/public/stylesheets/admin.css @@ -549,6 +549,17 @@ table.revisions_table tr:hover { cursor: help; } +.diff_unchanged { + color: #969696; + font-style: italic; +} + +.diff_set_list { + list-style: none; + padding-left: 0; + margin: 0; +} + table.user_table td.user_login { padding-right: 30px; } diff --git a/test/controllers/revisions_controller_test.rb b/test/controllers/revisions_controller_test.rb index bf92c5b..53927b4 100644 --- a/test/controllers/revisions_controller_test.rb +++ b/test/controllers/revisions_controller_test.rb @@ -148,4 +148,17 @@ class RevisionsControllerTest < ActionController::TestCase assert_response :success assert_select "a", "Side by side" end + + test "diffing two revisions also shows tag, template, and asset changes" do + login_as :quentin + @node.find_or_create_draft(@user) + @node.draft.tag_list = "update" + @node.draft.save! + + post(:diff, params: { :node_id => @node.id, :start_revision => @node.pages.first.revision, :end_revision => @node.pages.last.revision }) + assert_response :success + assert_select "h3", "Tags" + assert_select "h3", "Template" + assert_select "h3", "Assets" + end end diff --git a/test/models/page_test.rb b/test/models/page_test.rb index 3868a53..bcddc89 100644 --- a/test/models/page_test.rb +++ b/test/models/page_test.rb @@ -228,4 +228,47 @@ class PageTest < ActiveSupport::TestCase assert_equal 2, fragment.css('ins.diff_structural').length assert_match "der Zugang erfolgt über den Hinterhof.", fragment.text end + + test "diff_against reports tag and template changes" do + n = Node.root.children.create! :slug => "field_diff_test" + d = n.find_or_create_draft @user1 + d.tag_list = "update" + d.template_name = "standard_template" + d.save! + n.publish_draft! + + d2 = n.find_or_create_draft @user1 + d2.tag_list = "update, pressemitteilung" + d2.template_name = "title_only" + d2.save! + + diff = d2.diff_against(n.head) + + assert_equal ["pressemitteilung"], diff[:tags][:added] + assert_equal [], diff[:tags][:removed] + assert diff[:template_name][:changed] + assert_equal "standard_template", diff[:template_name][:from] + assert_equal "title_only", diff[:template_name][:to] + end + + test "diff_against reports added and removed assets by filename" do + n = Node.root.children.create! :slug => "asset_diff_test" + d = n.find_or_create_draft @user1 + d.save! + n.publish_draft! + + kept_asset = Asset.create!(:upload_file_name => "kept.png", :upload_content_type => "image/png", :upload_file_size => 1) + removed_asset = Asset.create!(:upload_file_name => "removed.pdf", :upload_content_type => "application/pdf", :upload_file_size => 1) + n.head.update_assets([kept_asset.id, removed_asset.id]) + + d2 = n.find_or_create_draft @user1 + added_asset = Asset.create!(:upload_file_name => "added.png", :upload_content_type => "image/png", :upload_file_size => 1) + d2.update_assets([kept_asset.id, added_asset.id]) + d2.save! + + diff = d2.diff_against(n.head) + + assert_equal [added_asset], diff[:assets][:added] + assert_equal [removed_asset], diff[:assets][:removed] + end end -- cgit v1.3 From d12598f17547d6b6ad817f32bc6419556ecdf019 Mon Sep 17 00:00:00 2001 From: erdgeist Date: Fri, 10 Jul 2026 04:44:58 +0200 Subject: Add test for backlink in revisions#index --- test/controllers/revisions_controller_test.rb | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'test/controllers') diff --git a/test/controllers/revisions_controller_test.rb b/test/controllers/revisions_controller_test.rb index 53927b4..76d4e8c 100644 --- a/test/controllers/revisions_controller_test.rb +++ b/test/controllers/revisions_controller_test.rb @@ -161,4 +161,11 @@ class RevisionsControllerTest < ActionController::TestCase assert_select "h3", "Template" assert_select "h3", "Assets" end + + test "revisions#index links back to the node" do + login_as :quentin + get :index, params: { :node_id => @node.id } + assert_response :success + assert_select "a[href=?]", node_path(@node) + end end -- cgit v1.3 From 48eed5f35ea798c91f1e0a77c13e751145acfc48 Mon Sep 17 00:00:00 2001 From: erdgeist Date: Fri, 10 Jul 2026 18:10:57 +0200 Subject: Add the missing assertions two tests were silently running without 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. --- test/controllers/nodes_controller_test.rb | 1 + test/models/node_test.rb | 14 +++++++++----- 2 files changed, 10 insertions(+), 5 deletions(-) (limited to 'test/controllers') diff --git a/test/controllers/nodes_controller_test.rb b/test/controllers/nodes_controller_test.rb index ce3419c..37091d5 100644 --- a/test/controllers/nodes_controller_test.rb +++ b/test/controllers/nodes_controller_test.rb @@ -355,6 +355,7 @@ class NodesControllerTest < ActionController::TestCase login_as :quentin get :index + assert_response :success end test "no dangling pages remain after node removal" do diff --git a/test/models/node_test.rb b/test/models/node_test.rb index 9e71dec..280614e 100644 --- a/test/models/node_test.rb +++ b/test/models/node_test.rb @@ -111,15 +111,19 @@ class NodeTest < ActiveSupport::TestCase node.publish_draft! assert_not_nil node.find_or_create_draft( @user1 ) end - + def test_find_or_create_draft_if_draft_exists_and_is_owned_by_user node = Node.root.children.create :slug => "xyz" node.publish_draft! - - node.find_or_create_draft @user1 - node.find_or_create_draft @user1 + + first_call = node.find_or_create_draft @user1 + second_call = node.find_or_create_draft @user1 + + assert_equal first_call, second_call + assert_equal 2, node.pages.count + assert_equal @user1, node.lock_owner end - + def test_exception_if_draft_exists_but_locked_by_another_user node = Node.root.children.create :slug => "xyz" node.publish_draft! -- cgit v1.3 From 7d6a665b896252537b8b8df2f15e204d04417231 Mon Sep 17 00:00:00 2001 From: erdgeist Date: Fri, 10 Jul 2026 21:57:58 +0200 Subject: Fix current_user/uniq inconsistencies in admin#index, remove dead query @mynodes used the bare @current_user ivar instead of the current_user method (works today only because login_required already forces that memoization; every other query and controller in this app uses the method), and .uniq, which is plain Enumerable#uniq here, not Relation#distinct -- it materializes every joined row into an Array before deduplicating, rather than deduplicating in SQL. @mypages had the same ivar pattern, no .order/.limit unlike every sibling query in this action, and -- confirmed via a full grep -- is referenced nowhere else in the codebase. Removed rather than bounded; nothing was ever rendering it. --- app/controllers/admin_controller.rb | 8 +++----- test/controllers/admin_controller_test.rb | 20 ++++++++++++++++++++ 2 files changed, 23 insertions(+), 5 deletions(-) (limited to 'test/controllers') diff --git a/app/controllers/admin_controller.rb b/app/controllers/admin_controller.rb index 6ab2135..435df57 100644 --- a/app/controllers/admin_controller.rb +++ b/app/controllers/admin_controller.rb @@ -20,12 +20,10 @@ class AdminController < ApplicationController ordered_with_level.each { |node, level| @sitemap_depth[node.id] = level } @sitemap = ordered_with_level.map(&:first).reject(&:update?) - @mypages = Page.where("user_id = ? or editor_id = ?", @current_user, @current_user) - @mynodes = Node.joins(:pages) - .where("pages.user_id = ? or pages.editor_id = ?", @current_user, @current_user) - .order("updated_at desc") - .uniq.first(50) + .where("pages.user_id = ? or pages.editor_id = ?", current_user, current_user) + .order("updated_at desc") + .distinct.first(50) end def conventions diff --git a/test/controllers/admin_controller_test.rb b/test/controllers/admin_controller_test.rb index d6005ba..13cc1bb 100644 --- a/test/controllers/admin_controller_test.rb +++ b/test/controllers/admin_controller_test.rb @@ -14,4 +14,24 @@ class AdminControllerTest < ActionController::TestCase get :index assert_includes assigns(:drafts), node end + + test "my work list shows each matching node only once, even with several revisions by the same user" do + login_as :quentin + user = User.find_by_login("quentin") + node = Node.root.children.create!(:slug => "dedup_test") + node.lock_for_editing!(user) + node.autosave!({:title => "v1"}, user) + node.save_draft!(user) + node.publish_draft! + node.lock_for_editing!(user) + node.autosave!({:title => "v2"}, user) + node.save_draft!(user) + node.publish_draft! + # three pages now exist on this node, all touched by quentin -- + # without DISTINCT, the join would return this node three times + + get :index + matches = assigns(:mynodes).select { |n| n.id == node.id } + assert_equal 1, matches.length + end end -- cgit v1.3 From 15aff3eff51809d1ce21caab406a0ef8b13624b6 Mon Sep 17 00:00:00 2001 From: erdgeist Date: Sat, 11 Jul 2026 01:33:07 +0200 Subject: Do not offer to destroy the only draft of a never-published node --- app/views/nodes/show.html.erb | 2 +- test/controllers/nodes_controller_test.rb | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) (limited to 'test/controllers') diff --git a/app/views/nodes/show.html.erb b/app/views/nodes/show.html.erb index 5756649..07cb5d1 100644 --- a/app/views/nodes/show.html.erb +++ b/app/views/nodes/show.html.erb @@ -49,7 +49,7 @@ form: { data: { confirm: "Publish this draft?" }, class: 'button_to state_changing' } %>
<% end %> - <% if @node.draft || @node.autosave %> + <% if @node.autosave || (@node.draft && @node.head) %>
<%= button_to (@node.draft && !@node.autosave ? 'Destroy Draft' : 'Discard Autosave'), revert_node_path(@node), method: :put, diff --git a/test/controllers/nodes_controller_test.rb b/test/controllers/nodes_controller_test.rb index 37091d5..b563d4d 100644 --- a/test/controllers/nodes_controller_test.rb +++ b/test/controllers/nodes_controller_test.rb @@ -463,4 +463,12 @@ class NodesControllerTest < ActionController::TestCase put :revert, params: { :id => node.id } assert_redirected_to edit_node_path(node) end + + test "nodes#show does not offer to destroy the only draft of a never-published node" do + node = Node.root.children.create!(:slug => "draft_only_test") + login_as :quentin + get :show, params: { :id => node.id } + assert_response :success + assert_select "form.destructive", :count => 0 + end end -- cgit v1.3 From 92c394b43a0603743b914c6298aab986805ca990 Mon Sep 17 00:00:00 2001 From: erdgeist Date: Sat, 11 Jul 2026 23:43:02 +0200 Subject: Add drafts/recent/mine/chapters admin node views 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. --- app/controllers/nodes_controller.rb | 45 +++++++++++++ app/models/node.rb | 22 +++++++ app/views/nodes/_node_list.html.erb | 39 +++++++++++ app/views/nodes/chapters.html.erb | 9 +++ app/views/nodes/drafts.html.erb | 3 + app/views/nodes/mine.html.erb | 3 + app/views/nodes/recent.html.erb | 3 + app/views/nodes/tags.html.erb | 3 + config/routes.rb | 5 ++ public/stylesheets/admin.css | 15 +++++ test/controllers/nodes_controller_test.rb | 104 ++++++++++++++++++++++++++++++ test/models/node_test.rb | 26 ++++++++ 12 files changed, 277 insertions(+) create mode 100644 app/views/nodes/_node_list.html.erb create mode 100644 app/views/nodes/chapters.html.erb create mode 100644 app/views/nodes/drafts.html.erb create mode 100644 app/views/nodes/mine.html.erb create mode 100644 app/views/nodes/recent.html.erb create mode 100644 app/views/nodes/tags.html.erb (limited to 'test/controllers') diff --git a/app/controllers/nodes_controller.rb b/app/controllers/nodes_controller.rb index 6fcd930..ede91ad 100644 --- a/app/controllers/nodes_controller.rb +++ b/app/controllers/nodes_controller.rb @@ -183,6 +183,39 @@ class NodesController < ApplicationController render plain: slug_for(params[:title]) end + # Filter functions for admin views + def drafts + base = Node.where("draft_id IS NOT NULL OR autosave_id IS NOT NULL") + @nodes = index_matching(base) + end + + def recent + base = Node.where( + "nodes.updated_at < ? AND nodes.updated_at > ? AND nodes.parent_id IS NOT NULL", + Time.now, Time.now - 14.days + ) + @nodes = index_matching(base) + end + + def mine + base = Node.joins(:pages) + .where("pages.user_id = ? or pages.editor_id = ?", current_user, current_user) + .distinct + @nodes = index_matching(base) + end + + def chapters + @kind_keys = Array(params[:kinds]) & %w[erfa chaostreff] + @kind_keys = %w[erfa chaostreff] if @kind_keys.empty? + tags = @kind_keys.flat_map { |key| CccConventions::NODE_KINDS[key][:tags] } + @nodes = nodes_matching_tags(tags) + end + + def tags + tags = params[:tags].to_s.split(',').map(&:strip).reject(&:blank?) + @nodes = nodes_matching_tags(tags) + end + private def slug_for(title) @@ -214,4 +247,16 @@ class NodesController < ApplicationController config && config[:parent] ? config[:parent].call.id : nil end end + + def nodes_matching_tags(tags) + matching_pages = Page.tagged_with(tags, any: true).reselect(:id) + base = Node.where(head_id: matching_pages).or(Node.where(draft_id: matching_pages)) + index_matching(base) + end + + def index_matching(base_scope) + scope = base_scope.includes(:head, :draft) + scope = scope.merge(Node.editor_search(params[:q])) if params[:q].present? + scope.order("nodes.updated_at desc").paginate(page: params[:page], per_page: 25) + end end diff --git a/app/models/node.rb b/app/models/node.rb index 9cb4840..24f3cd0 100644 --- a/app/models/node.rb +++ b/app/models/node.rb @@ -345,6 +345,28 @@ class Node < ApplicationRecord .distinct end + # This one is for admin-only views, where finding a draft is the point. + # Substring match on whichever of head/draft is present. + def self.editor_search(term) + words = term.to_s.split(/\s+/).reject(&:blank?) + return none if words.empty? + + conditions = [] + binds = {} + + words.each_with_index do |word, i| + key = "term#{i}" + binds[key.to_sym] = "%#{sanitize_sql_like(word)}%" + conditions << "(head_translations.title ILIKE :#{key} OR head_translations.abstract ILIKE :#{key} " \ + "OR draft_translations.title ILIKE :#{key} OR draft_translations.abstract ILIKE :#{key})" + end + + joins("LEFT JOIN page_translations head_translations ON head_translations.page_id = nodes.head_id") + .joins("LEFT JOIN page_translations draft_translations ON draft_translations.page_id = nodes.draft_id") + .where(conditions.join(" AND "), binds) + .distinct + end + protected def lock_for! current_user self.lock_owner = current_user diff --git a/app/views/nodes/_node_list.html.erb b/app/views/nodes/_node_list.html.erb new file mode 100644 index 0000000..03f38b4 --- /dev/null +++ b/app/views/nodes/_node_list.html.erb @@ -0,0 +1,39 @@ +<%= form_tag url_for(controller: params[:controller], action: params[:action]), method: :get, class: "node_search_form" do %> + <% Array(params[:kinds]).each do |kind| %> + <%= hidden_field_tag "kinds[]", kind %> + <% end %> + <%= hidden_field_tag :tags, params[:tags] if params[:tags].present? %> + <%= text_field_tag :q, params[:q], placeholder: "Search title, abstract, body…" %> + <%= submit_tag "Search", class: "action_button" %> + <% if params[:q].present? || params[:kinds].present? || params[:tags].present? %> + <%= link_to "Reset", url_for(controller: params[:controller], action: params[:action]) %> + <% end %> +<% end %> + +<%= will_paginate @nodes %> + + + + + + + + + <% @nodes.each do |node| %> + "> + + + + + + + <% end %> +
IDTitleActionsLocked byRev.
<%= node.id %> +

<%= link_to title_for_node(node), node_path(node) %>

+

<%= link_to_path(node.unique_name, node.unique_name) %>

+
+ <%= link_to 'show', node_path(node) %> + <%= link_to 'edit', edit_node_path(node) %> + <%= link_to 'revisions', node_revisions_path(node) %> + <%= node.lock_owner.login if node.lock_owner %><%= node.draft ? node.draft.revision : (node.head ? node.head.revision : "EMPTY") %>
+<%= will_paginate @nodes %> diff --git a/app/views/nodes/chapters.html.erb b/app/views/nodes/chapters.html.erb new file mode 100644 index 0000000..939f19c --- /dev/null +++ b/app/views/nodes/chapters.html.erb @@ -0,0 +1,9 @@ +

Chapters

+ +<%= form_tag chapters_nodes_path, method: :get do %> + + + <%= submit_tag "Filter", class: "action_button" %> +<% end %> + +<%= render 'node_list' %> diff --git a/app/views/nodes/drafts.html.erb b/app/views/nodes/drafts.html.erb new file mode 100644 index 0000000..6d0830c --- /dev/null +++ b/app/views/nodes/drafts.html.erb @@ -0,0 +1,3 @@ +

Nodes with drafts or autosaves

+ +<%= render 'node_list' %> diff --git a/app/views/nodes/mine.html.erb b/app/views/nodes/mine.html.erb new file mode 100644 index 0000000..fc5680a --- /dev/null +++ b/app/views/nodes/mine.html.erb @@ -0,0 +1,3 @@ +

My work

+ +<%= render 'node_list' %> diff --git a/app/views/nodes/recent.html.erb b/app/views/nodes/recent.html.erb new file mode 100644 index 0000000..3602775 --- /dev/null +++ b/app/views/nodes/recent.html.erb @@ -0,0 +1,3 @@ +

Recently changed

+ +<%= render 'node_list' %> diff --git a/app/views/nodes/tags.html.erb b/app/views/nodes/tags.html.erb new file mode 100644 index 0000000..0ebc6ce --- /dev/null +++ b/app/views/nodes/tags.html.erb @@ -0,0 +1,3 @@ +

Nodes tagged: <%= params[:tags] %>

+ +<%= render 'node_list' %> diff --git a/config/routes.rb b/config/routes.rb index aebce90..2c165d2 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -37,7 +37,12 @@ Cccms::Application.routes.draw do resources :nodes do collection do + get 'tags/:tags', action: :tags, as: :tags, constraints: { tags: /[^\/]+/ } get :parameterize_preview + get :drafts + get :recent + get :mine + get :chapters end member do diff --git a/public/stylesheets/admin.css b/public/stylesheets/admin.css index 28b8494..aa8b288 100644 --- a/public/stylesheets/admin.css +++ b/public/stylesheets/admin.css @@ -748,6 +748,21 @@ form.button_to button[type="submit"] { margin-bottom: 0; } +.node_search_form { + display: flex; + align-items: center; + gap: 0.5rem; + margin-bottom: 0.75rem; +} + +.node_search_form input[type=text] { + padding: 4px 12px; + box-sizing: border-box; + height: 2.25rem; + width: 22rem; + border-radius: 2px; +} + /* Layout only -- the at-rest visibility (wavy underline) for these links comes from the scoped rule in Base elements above. */ .add_child_links { diff --git a/test/controllers/nodes_controller_test.rb b/test/controllers/nodes_controller_test.rb index b563d4d..05cb195 100644 --- a/test/controllers/nodes_controller_test.rb +++ b/test/controllers/nodes_controller_test.rb @@ -471,4 +471,108 @@ class NodesControllerTest < ActionController::TestCase assert_response :success assert_select "form.destructive", :count => 0 end + + test "drafts includes a never-published node with only a draft" do + node = Node.root.children.create!(:slug => "drafts_action_test") + login_as :quentin + get :drafts + assert_includes assigns(:nodes), node + end + + test "chapters filters by kind, matching head or draft, and shows both by default" do + erfa_node = Node.root.children.create!(:slug => "chapters_erfa_test") + erfa_node.find_or_create_draft(@user1) + erfa_node.draft.tag_list = "erfa-detail" + erfa_node.draft.save! + erfa_node.publish_draft! + + chaostreff_node = Node.root.children.create!(:slug => "chapters_chaostreff_test") + chaostreff_node.find_or_create_draft(@user1) + chaostreff_node.draft.tag_list = "chaostreff-detail" + chaostreff_node.draft.save! + chaostreff_node.publish_draft! + + login_as :quentin + + get :chapters, params: { :kinds => "erfa" } + assert_includes assigns(:nodes), erfa_node + assert_not_includes assigns(:nodes), chaostreff_node + + get :chapters + assert_includes assigns(:nodes), erfa_node + assert_includes assigns(:nodes), chaostreff_node + end + + test "recent combined with a search term does not raise an ambiguous column error" do + login_as :quentin + get :recent, params: { :q => "Zombies" } + assert_response :success + end + + test "drafts combined with a search term does not raise an ambiguous column error" do + login_as :quentin + get :drafts, params: { :q => "Zombies" } + assert_response :success + end + + test "mine combined with a search term does not raise an ambiguous column error" do + login_as :quentin + get :mine, params: { :q => "Zombies" } + assert_response :success + end + + test "chapters combined with a search term does not raise an ambiguous column error" do + login_as :quentin + get :chapters, params: { :q => "Zombies" } + assert_response :success + end + + test "tags path filters by an arbitrary raw tag, generalizing chapters" do + presse_node = Node.root.children.create!(:slug => "tags_path_presse_test") + presse_node.find_or_create_draft(@user1) + presse_node.draft.tag_list = "pressemitteilung" + presse_node.draft.save! + presse_node.publish_draft! + + erfa_node = Node.root.children.create!(:slug => "tags_path_erfa_test") + erfa_node.find_or_create_draft(@user1) + erfa_node.draft.tag_list = "erfa-detail" + erfa_node.draft.save! + erfa_node.publish_draft! + + login_as :quentin + get :tags, params: { :tags => "pressemitteilung" } + + assert_includes assigns(:nodes), presse_node + assert_not_includes assigns(:nodes), erfa_node + + assert_select "h1", "Nodes tagged: pressemitteilung" + assert_select "h1", :text => "Chapters", :count => 0 + end + + test "tags path with multiple tags matches any of them, not all" do + erfa_node = Node.root.children.create!(:slug => "tags_path_multi_erfa_test") + erfa_node.find_or_create_draft(@user1) + erfa_node.draft.tag_list = "erfa-detail" + erfa_node.draft.save! + erfa_node.publish_draft! + + chaostreff_node = Node.root.children.create!(:slug => "tags_path_multi_chaostreff_test") + chaostreff_node.find_or_create_draft(@user1) + chaostreff_node.draft.tag_list = "chaostreff-detail" + chaostreff_node.draft.save! + chaostreff_node.publish_draft! + + login_as :quentin + get :tags, params: {:tags => "erfa-detail,chaostreff-detail" } + + assert_includes assigns(:nodes), erfa_node + assert_includes assigns(:nodes), chaostreff_node + end + + test "chapters renders the curated heading" do + login_as :quentin + get :chapters + assert_select "h1", "Chapters" + end end diff --git a/test/models/node_test.rb b/test/models/node_test.rb index 5626384..15e908b 100644 --- a/test/models/node_test.rb +++ b/test/models/node_test.rb @@ -511,4 +511,30 @@ class NodeTest < ActiveSupport::TestCase a.reload assert_equal Node.root.id, a.parent_id end + + test "editor_search matches a partial substring, not just a whole word" do + node = Node.root.children.create!(:slug => "editor_search_substring_test") + node.find_or_create_draft(@user1) + node.draft.update(:title => "Biometrics Conference") + node.publish_draft! + + assert_includes Node.editor_search("bio"), node + assert_includes Node.editor_search("Conf"), node + end + + test "editor_search returns an empty relation for a blank term, not every node" do + assert_equal 0, Node.editor_search("").count + assert_equal 0, Node.editor_search(nil).count + assert_equal 0, Node.editor_search(" ").count + end + + test "editor_search requires every word to match, but each word can match a different field" do + node = Node.root.children.create!(:slug => "editor_search_multiword_test") + node.find_or_create_draft(@user1) + node.draft.update(:title => "Backspace e.V. Bamberg", :abstract => "Spiegelgraben 41, 96052 Bamberg") + node.publish_draft! + + assert_includes Node.editor_search("Backspace Spiegelgraben"), node + assert_equal 0, Node.editor_search("Backspace Nonexistentstreet").count + end end -- cgit v1.3 From 45bf65d04d046c0ea4a1150096b2a9b846d6c0b8 Mon Sep 17 00:00:00 2001 From: erdgeist Date: Sun, 12 Jul 2026 02:15:44 +0200 Subject: Give the sitemap its own view, with collapse and descendant counts Extracted from admin#index's inline table into NodesController#sitemap. Nested
/ per branch, one linear pass over the existing flat [node, level] list (no added queries) -- each node's own descendant count computed the same way, via a small stack rather than re-walking the tree per node. Branches under updates/, club/erfas, club/chaostreffs, and disclosure start collapsed by default (CccConventions::SITEMAP_COLLAPSED_PATHS); any branch currently collapsed, whether by that default or because someone just closed it, is highlighted via a plain :not([open]) selector -- no state tracked outside the DOM itself. Dropped the update?-post exclusion this view used to rely on -- no longer needed now that updates/ collapses instead of being filtered out, so its real children (previously silently absent) now show up correctly. admin#index's own, separate @sitemap query is unchanged; that view has no collapse mechanism to compensate and wasn't part of this. --- app/controllers/nodes_controller.rb | 23 ++++++++++++++ app/helpers/nodes_helper.rb | 5 ++- app/views/nodes/sitemap.html.erb | 32 +++++++++++++++++++ config/routes.rb | 1 + lib/ccc_conventions.rb | 1 + public/stylesheets/admin.css | 40 ++++++++++++++++++++++++ test/controllers/nodes_controller_test.rb | 52 +++++++++++++++++++++++++++++++ test/models/helpers/nodes_helper_test.rb | 11 +++++++ 8 files changed, 164 insertions(+), 1 deletion(-) create mode 100644 app/views/nodes/sitemap.html.erb (limited to 'test/controllers') diff --git a/app/controllers/nodes_controller.rb b/app/controllers/nodes_controller.rb index ede91ad..772bf4b 100644 --- a/app/controllers/nodes_controller.rb +++ b/app/controllers/nodes_controller.rb @@ -216,6 +216,11 @@ class NodesController < ApplicationController @nodes = nodes_matching_tags(tags) end + def sitemap + @sitemap = Node.root.self_and_descendants_ordered_with_level + @sitemap_descendant_counts = descendant_counts_for(@sitemap) + end + private def slug_for(title) @@ -248,6 +253,24 @@ class NodesController < ApplicationController end end + def descendant_counts_for(ordered_with_level) + counts = Hash.new(0) + stack = [] # [node, level, index] + ordered_with_level.each_with_index do |(node, level), index| + while stack.any? && stack.last[1] >= level + ancestor_node, _ancestor_level, ancestor_index = stack.pop + counts[ancestor_node.id] = index - ancestor_index - 1 + end + stack << [node, level, index] + end + total = ordered_with_level.length + while stack.any? + ancestor_node, _ancestor_level, ancestor_index = stack.pop + counts[ancestor_node.id] = total - ancestor_index - 1 + end + counts + end + def nodes_matching_tags(tags) matching_pages = Page.tagged_with(tags, any: true).reselect(:id) base = Node.where(head_id: matching_pages).or(Node.where(draft_id: matching_pages)) diff --git a/app/helpers/nodes_helper.rb b/app/helpers/nodes_helper.rb index a89f879..1268b63 100644 --- a/app/helpers/nodes_helper.rb +++ b/app/helpers/nodes_helper.rb @@ -12,7 +12,6 @@ module NodesHelper end end - def truncated_title_for_node node if (title = title_for_node node) && title.size > 20 "#{truncate(title, 40)}" @@ -63,4 +62,8 @@ module NodesHelper path = node.unique_path CccConventions::NODE_KINDS.select { |_, config| config[:parent_match]&.call(path) } end + + def sitemap_node_open?(node) + !CccConventions::SITEMAP_COLLAPSED_PATHS.include?(node.unique_name) + end end diff --git a/app/views/nodes/sitemap.html.erb b/app/views/nodes/sitemap.html.erb new file mode 100644 index 0000000..a799c17 --- /dev/null +++ b/app/views/nodes/sitemap.html.erb @@ -0,0 +1,32 @@ +

Sitemap

+ +
+<% + open_details = [] # levels with a currently-open
+%> +<% @sitemap.each_with_index do |(node, level), index| %> + <% while open_details.any? && open_details.last >= level %> +
+ <% open_details.pop %> + <% end %> + +
+

<%= link_to title_for_node(node), node_path(node) %>

+ <%= link_to_path("#{node.unique_name} ↗", node.unique_name) %> +

+ <%= link_to 'Show', node_path(node) %> + <%= link_to 'Create Child', new_node_path(:parent_id => node.id) %> +

+
+ + <% next_level = @sitemap[index + 1]&.last %> + <% if next_level && next_level > level %> + > + + <%= pluralize(@sitemap_descendant_counts[node.id], 'descendant', 'descendants') %> + + <% open_details.push(level) %> + <% end %> +<% end %> +<% open_details.length.times { %>
<% } %> +
diff --git a/config/routes.rb b/config/routes.rb index 2c165d2..bb34bd8 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -43,6 +43,7 @@ Cccms::Application.routes.draw do get :recent get :mine get :chapters + get :sitemap end member do diff --git a/lib/ccc_conventions.rb b/lib/ccc_conventions.rb index b420452..352dd3c 100644 --- a/lib/ccc_conventions.rb +++ b/lib/ccc_conventions.rb @@ -1,6 +1,7 @@ module CccConventions ERFA_PARENT_NAME = "club/erfas" CHAOSTREFF_PARENT_NAME = "club/chaostreffs" + SITEMAP_COLLAPSED_PATHS = %w[updates club/erfas club/chaostreffs disclosure].freeze NODE_KINDS = { "top_level" => { diff --git a/public/stylesheets/admin.css b/public/stylesheets/admin.css index aa8b288..5c1e489 100644 --- a/public/stylesheets/admin.css +++ b/public/stylesheets/admin.css @@ -25,6 +25,7 @@ a:hover { body content, node listing tables, the child-creation shortcuts on nodes#show, and the dashboard draft list's Show/Revisions links. */ #page_editor a, +#sitemap a, table.node_table a, table.assets_table a, table.events_table a, @@ -104,6 +105,11 @@ input[type=radio] { width: 100%; box-sizing: border-box; } + + #sitemap details details { + margin-left: 0.75rem; + padding-left: 0.5rem; + } } #metadata, @@ -773,6 +779,40 @@ form.button_to button[type="submit"] { white-space: nowrap; } +.sitemap_node { + padding-bottom: 0.5rem; + margin-bottom: 0.5rem; + border-bottom: 1px solid #ececec; +} + +.sitemap_node h4 { + margin: 0; +} + +.sitemap_node .field_hint { + display: block; + margin: 2px 0 0; +} + +.sitemap_node p { + margin: 2px 0 0; +} + +#sitemap details details { + margin-left: 1.5rem; + border-left: 1px solid #e8e8e8; + padding-left: 0.75rem; +} + +#sitemap summary { + padding: 4px 0; +} + +#sitemap details:not([open]) > summary { + color: #ff9600; + font-weight: bold; +} + /* ============================================================ Page editor / metadata forms ============================================================ */ diff --git a/test/controllers/nodes_controller_test.rb b/test/controllers/nodes_controller_test.rb index 05cb195..b43d2de 100644 --- a/test/controllers/nodes_controller_test.rb +++ b/test/controllers/nodes_controller_test.rb @@ -575,4 +575,56 @@ class NodesControllerTest < ActionController::TestCase get :chapters assert_select "h1", "Chapters" end + + test "sitemap collapses configured paths but leaves others open" do + club = Node.root.children.create!(:slug => "club") + erfas = club.children.create!(:slug => "erfas") + erfas.children.create!(:slug => "one_chapter") + other = Node.root.children.create!(:slug => "sitemap_controller_open_test") + other.children.create!(:slug => "sitemap_controller_open_child") + + login_as :quentin + get :sitemap + assert_response :success + + doc = Nokogiri::HTML::DocumentFragment.parse(response.body) + + erfas_node_div = doc.css('.sitemap_node').find { |div| div.at_css('.field_hint')&.text&.include?(erfas.unique_name) } + other_node_div = doc.css('.sitemap_node').find { |div| div.at_css('.field_hint')&.text&.include?(other.unique_name) } + + erfas_details = erfas_node_div.next_element + other_details = other_node_div.next_element + + assert_equal 'details', erfas_details.name + assert_equal 'details', other_details.name + assert_not erfas_details.key?('open') + assert other_details.key?('open') + end + + test "sitemap shows how many descendants a collapsed branch is hiding" do + club = Node.root.children.create!(:slug => "club") + erfas = club.children.create!(:slug => "erfas") + erfas.children.create!(:slug => "one_chapter") + erfas.children.create!(:slug => "another_chapter") + + login_as :quentin + get :sitemap + assert_response :success + + doc = Nokogiri::HTML::DocumentFragment.parse(response.body) + erfas_node_div = doc.css('.sitemap_node').find { |div| div.at_css('.field_hint')&.text&.include?(erfas.unique_name) } + erfas_details = erfas_node_div.next_element + + assert_equal 'details', erfas_details.name + assert_match "2 descendants", erfas_details.at_css('summary').text + end + + test "sitemap shows Show and Create Child, not Revisions" do + node = Node.root.children.create!(:slug => "sitemap_actions_test") + login_as :quentin + get :sitemap + assert_select ".sitemap_node_actions", :text => /Show/ + assert_select ".sitemap_node_actions", :text => /Create Child/ + assert_select ".sitemap_node_actions", :text => /Revisions/, :count => 0 + end end diff --git a/test/models/helpers/nodes_helper_test.rb b/test/models/helpers/nodes_helper_test.rb index 5d91a88..5ab924f 100644 --- a/test/models/helpers/nodes_helper_test.rb +++ b/test/models/helpers/nodes_helper_test.rb @@ -22,4 +22,15 @@ class NodesHelperTest < ActionView::TestCase page = FakePage.new([]) assert_nil default_event_tag_list(page) end + + test "sitemap_node_open? is false for a configured collapsed path" do + club = Node.root.children.create!(:slug => "club") + erfas = club.children.create!(:slug => "erfas") + assert_equal false, sitemap_node_open?(erfas) + end + + test "sitemap_node_open? is true for anything not configured as collapsed" do + node = Node.root.children.create!(:slug => "sitemap_open_test") + assert_equal true, sitemap_node_open?(node) + end end -- cgit v1.3 From 4ec70e5ecf792a56e868538738d6c7f63bb6cf24 Mon Sep 17 00:00:00 2001 From: erdgeist Date: Sun, 12 Jul 2026 16:33:12 +0200 Subject: Add a grouped tag+node search endpoint for the upcoming dashboard AdminController#dashboard_search returns tags and nodes as separate, labeled groups (via ActsAsTaggableOn::Tag.named_like and Node.editor_search) rather than the flat per-node list every existing picker returns. initSearchPicker gains a renderResults callback option that, when given, replaces the default per-node rendering entirely -- lets the dashboard render its two labeled groups without a second, parallel picker implementation. resultsHeaderHtml (the "Press Enter to see all results" hint) is now threaded through as a third argument to renderResults, so a picker with custom rendering can still show it -- previously only the default per-node branch ever did. Tags link straight to the existing /admin/nodes/tags/:tag view rather than becoming an in-place filter chip. --- app/controllers/admin_controller.rb | 18 +++++ app/views/layouts/admin.html.erb | 1 + config/routes.rb | 9 +-- public/javascripts/admin_interface.js | 4 ++ public/javascripts/admin_search.js | 109 +++++++++++++++++++++--------- public/stylesheets/admin.css | 10 +++ test/controllers/admin_controller_test.rb | 24 +++++++ 7 files changed, 139 insertions(+), 36 deletions(-) (limited to 'test/controllers') diff --git a/app/controllers/admin_controller.rb b/app/controllers/admin_controller.rb index 3c45c49..8445997 100644 --- a/app/controllers/admin_controller.rb +++ b/app/controllers/admin_controller.rb @@ -49,6 +49,24 @@ class AdminController < ApplicationController end end + def dashboard_search + term = params[:search_term] + + if term.blank? + render json: { tags: [], nodes: [] } + return + end + + render json: { + tags: ActsAsTaggableOn::Tag.named_like(term).limit(5).map { |tag| + { name: tag.name, tag_path: tags_nodes_path(tags: tag.name) } + }, + nodes: Node.editor_search(term).limit(10).map { |node| + { node_id: node.id, title: node.title, unique_name: node.unique_name, node_path: node_path(node) } + } + } + end + def menu_search if params[:search_term] == "Root" @results = [Node.root] diff --git a/app/views/layouts/admin.html.erb b/app/views/layouts/admin.html.erb index 079346b..00d7075 100644 --- a/app/views/layouts/admin.html.erb +++ b/app/views/layouts/admin.html.erb @@ -17,6 +17,7 @@ var ADMIN_SEARCH_URL = "<%= admin_search_path %>"; var ADMIN_MENU_SEARCH_URL = "<%= admin_menu_search_path %>"; var PARAMETERIZE_PREVIEW_URL = "<%= parameterize_preview_nodes_path %>"; + var DASHBOARD_SEARCH_URL = "<%= admin_dashboard_search_path %>"; diff --git a/config/routes.rb b/config/routes.rb index bb34bd8..92301e5 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -66,10 +66,11 @@ Cccms::Application.routes.draw do end end - match '' => 'admin#index', :as => :admin, :via => :get - match 'search' => 'admin#search', :as => :admin_search, :via => :get - match 'menu_search' => 'admin#menu_search', :as => :admin_menu_search, :via => :get - match 'conventions' => 'admin#conventions', :as => :admin_conventions, :via => :get + match '' => 'admin#index', :as => :admin, :via => :get + match 'search' => 'admin#search', :as => :admin_search, :via => :get + match 'menu_search' => 'admin#menu_search', :as => :admin_menu_search, :via => :get + match 'conventions' => 'admin#conventions', :as => :admin_conventions, :via => :get + match 'dashboard_search' => 'admin#dashboard_search', :as => :admin_dashboard_search, :via => :get end match '/logout' => 'sessions#destroy', :as => :logout, :via => :delete diff --git a/public/javascripts/admin_interface.js b/public/javascripts/admin_interface.js index 8ca5b5e..3f6a0a9 100644 --- a/public/javascripts/admin_interface.js +++ b/public/javascripts/admin_interface.js @@ -31,6 +31,10 @@ $(document).ready(function () { } }); + if ($("#dashboard_search_term").length != 0) { + dashboard_search.initialize(); + } + if ($("#menu_search_term").length != 0) { menu_items.initialize_search(); } diff --git a/public/javascripts/admin_search.js b/public/javascripts/admin_search.js index f553334..53bcb5e 100644 --- a/public/javascripts/admin_search.js +++ b/public/javascripts/admin_search.js @@ -42,9 +42,10 @@ function initSearchPicker(options) { var inputSelector = options.inputSelector; var resultsSelector = options.resultsSelector; var url = options.url || ADMIN_MENU_SEARCH_URL; - var onSelect = options.onSelect; // omit for a real-navigation search like admin_search - var isActive = options.isActive; // optional guard, checked before every search + var onSelect = options.onSelect; // omit for a real-navigation search like admin_search + var isActive = options.isActive; // optional guard, checked before every search var resultsHeaderHtml = options.resultsHeaderHtml; // optional, prepended only when results exist + var renderResults = options.renderResults; // optional, replaces the default per-node rendering entirely var requestId = 0; var timeout; @@ -68,39 +69,44 @@ function initSearchPicker(options) { url: url, data: "search_term=" + term, dataType: "json", - success: function(nodes) { + success: function(data) { if (thisRequest !== requestId) return; results.empty(); - if (nodes.length && resultsHeaderHtml) { - results.append(resultsHeaderHtml); - } - var found = false; - for (var i = 0; i < nodes.length; i++) { - (function(node) { - var link; - if (onSelect) { - link = $( - "

" + node.title + - "" + node.unique_name + "" + - "

" - ); - link.find("a").bind("click", function() { - onSelect(node); - results.slideUp(); - results.empty(); - return false; - }); - } else { - link = $( - "

" + node.title + - "" + node.unique_name + "" + - "

" - ); - } - results.append(link); - found = true; - })(nodes[i]); + + var found; + if (renderResults) { + found = renderResults(data, results, resultsHeaderHtml); + } else { + if (data.length && resultsHeaderHtml) { + results.append(resultsHeaderHtml); + } + found = false; + for (var i = 0; i < data.length; i++) { + (function(node) { + var link; + if (onSelect) { + link = $( + "

" + node.title + + "" + node.unique_name + "

" + ); + link.find("a").bind("click", function() { + onSelect(node); + results.slideUp(); + results.empty(); + return false; + }); + } else { + link = $( + "

" + node.title + + "" + node.unique_name + "

" + ); + } + results.append(link); + found = true; + })(data[i]); + } } + if (found) { results.slideDown(); } else { @@ -115,6 +121,45 @@ function initSearchPicker(options) { }); } +dashboard_search = { + initialize : function() { + initSearchPicker({ + inputSelector: "#dashboard_search_term", + resultsSelector: "#dashboard_search_results", + url: DASHBOARD_SEARCH_URL, + resultsHeaderHtml: "

Press Enter to see all results ⏎

", + renderResults: function(data, results, resultsHeaderHtml) { + var found = false; + + if ((data.tags.length || data.nodes.length) && resultsHeaderHtml) { + results.append(resultsHeaderHtml); + } + + if (data.tags.length) { + results.append("

Tags

"); + data.tags.forEach(function(tag) { + results.append("

" + tag.name + "

"); + found = true; + }); + } + + if (data.nodes.length) { + results.append("

Pages

"); + data.nodes.forEach(function(node) { + results.append( + "

" + node.title + + "" + node.unique_name + "

" + ); + found = true; + }); + } + + return found; + } + }); + } +}; + menu_items = { initialize_search : function() { initSearchPicker({ diff --git a/public/stylesheets/admin.css b/public/stylesheets/admin.css index da31535..ade3a62 100644 --- a/public/stylesheets/admin.css +++ b/public/stylesheets/admin.css @@ -1052,6 +1052,16 @@ div#draft_list table td.actions a { border-bottom: none; } +.search_results p.search_group_label { + margin: 8px 0 2px; + padding: 0; + border-bottom: none; + font-size: 0.75rem; + color: #969696; + text-transform: lowercase; + font-weight: normal; +} + /* ============================================================ Menu items (navigation editor) ============================================================ */ diff --git a/test/controllers/admin_controller_test.rb b/test/controllers/admin_controller_test.rb index 13cc1bb..9beaf58 100644 --- a/test/controllers/admin_controller_test.rb +++ b/test/controllers/admin_controller_test.rb @@ -34,4 +34,28 @@ class AdminControllerTest < ActionController::TestCase matches = assigns(:mynodes).select { |n| n.id == node.id } assert_equal 1, matches.length end + + test "dashboard_search returns matching tags and nodes grouped separately" do + node = Node.root.children.create!(:slug => "dashboard_search_test") + node.find_or_create_draft(User.find_by_login("aaron")) + node.draft.update(:title => "Biometrics Workshop") + node.draft.tag_list = "biometrics-workshop" + node.draft.save! + + login_as :quentin + get :dashboard_search, params: { :search_term => "biometr" }, :format => :json + + json = JSON.parse(response.body) + assert json["tags"].any? { |t| t["name"] == "biometrics-workshop" } + assert json["nodes"].any? { |n| n["title"] == "Biometrics Workshop" } + end + + test "dashboard_search returns empty results for a blank term" do + login_as :quentin + get :dashboard_search, params: { :search_term => "" }, :format => :json + + json = JSON.parse(response.body) + assert_equal [], json["tags"] + assert_equal [], json["nodes"] + end end -- cgit v1.3 From b271648f89cba7cafafa1b73b1658b1c1bc0e4b0 Mon Sep 17 00:00:00 2001 From: erdgeist Date: Sun, 12 Jul 2026 23:42:43 +0200 Subject: Rebuild the admin dashboard: icon nav, search, signposts, widgets 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. --- app/controllers/admin_controller.rb | 21 +-- app/helpers/nodes_helper.rb | 12 ++ app/models/node.rb | 6 +- app/views/admin/_menu.html.erb | 14 +- app/views/admin/index.html.erb | 210 ++++++++++-------------------- test/controllers/admin_controller_test.rb | 20 --- test/controllers/nodes_controller_test.rb | 20 +++ test/models/node_test.rb | 24 ++++ 8 files changed, 134 insertions(+), 193 deletions(-) (limited to 'test/controllers') diff --git a/app/controllers/admin_controller.rb b/app/controllers/admin_controller.rb index 8445997..37fd78b 100644 --- a/app/controllers/admin_controller.rb +++ b/app/controllers/admin_controller.rb @@ -5,25 +5,8 @@ class AdminController < ApplicationController before_action :login_required def index - @drafts = Node.where("draft_id IS NOT NULL OR autosave_id IS NOT NULL") - .limit(50).order("updated_at desc") - - @drafts_count = Node.where("draft_id IS NOT NULL OR autosave_id IS NOT NULL").count - - @recent_changes = Node.where( - "updated_at < ? AND updated_at > ? AND parent_id IS NOT NULL", - Time.now, Time.now - 14.days - ).limit(50).order("updated_at desc") - - ordered_with_level = Node.root.self_and_descendants_ordered_with_level - @sitemap_depth = {} - ordered_with_level.each { |node, level| @sitemap_depth[node.id] = level } - @sitemap = ordered_with_level.map(&:first).reject(&:update?) - - @mynodes = Node.joins(:pages) - .where("pages.user_id = ? or pages.editor_id = ?", current_user, current_user) - .order("updated_at desc") - .distinct.first(50) + @drafts = Node.drafts_and_autosaves(current_user_id: current_user.id).limit(5) + @recent_changes = Node.recently_changed.limit(5) end def conventions diff --git a/app/helpers/nodes_helper.rb b/app/helpers/nodes_helper.rb index 1268b63..5884c8c 100644 --- a/app/helpers/nodes_helper.rb +++ b/app/helpers/nodes_helper.rb @@ -28,6 +28,18 @@ module NodesHelper User.all.map {|u| [u.login, u.id]} end + def node_last_editor(node) + editor = node.draft&.editor || node.head&.editor + return nil unless editor + editor == current_user ? t("editor_self") : editor.login + end + + def node_head_editor(node) + editor = node.head&.editor + return nil unless editor + editor == current_user ? t("publisher_self") : editor.login + end + DEFAULT_EVENT_TAG_BY_PAGE_TAG = { 'erfa-detail' => 'open-day', 'chaostreff-detail' => 'open-day' diff --git a/app/models/node.rb b/app/models/node.rb index aa2f7f3..1d0a089 100644 --- a/app/models/node.rb +++ b/app/models/node.rb @@ -377,10 +377,10 @@ class Node < ApplicationRecord end def self.recently_changed - where( - "nodes.updated_at < ? AND nodes.updated_at > ? AND nodes.parent_id IS NOT NULL", + includes(:head).where( + "pages.updated_at < ? AND pages.updated_at > ? AND nodes.parent_id IS NOT NULL", Time.now, Time.now - 14.days - ) + ).order("pages.updated_at desc").references(:head) end protected diff --git a/app/views/admin/_menu.html.erb b/app/views/admin/_menu.html.erb index 4302987..970429d 100644 --- a/app/views/admin/_menu.html.erb +++ b/app/views/admin/_menu.html.erb @@ -1,9 +1,5 @@ -<%= language_selector %> -<%= button_to 'Logout', logout_path, method: :delete %> -<%= link_to 'Overview', admin_path %> -search -<%= link_to 'Nodes', nodes_path, selected?('nodes') %> -<%= link_to 'Assets', assets_path, selected?('assets') %> -<%= link_to 'Events', events_path, selected?('events') %> -<%= link_to 'User', users_path, selected?('users') %> -<%= link_to 'Navigation', menu_items_path, selected?('menu_items') %> +<%= link_to icon("home", library: "tabler", "aria-hidden": true), admin_path, "aria-label": "Dashboard" %> +<%= icon("search", library: "tabler", "aria-hidden": true) %> +<%= button_to logout_path, method: :delete, aria: { label: "Log out" } do %> + <%= icon("logout", library: "tabler", "aria-hidden": true) %> +<% end %> diff --git a/app/views/admin/index.html.erb b/app/views/admin/index.html.erb index c67ccb3..bdb555e 100644 --- a/app/views/admin/index.html.erb +++ b/app/views/admin/index.html.erb @@ -1,155 +1,81 @@ -
-

Quick links

-
<%= link_to 'Create post or page', new_node_path, :only_path => false %>
- - -
<%= link_to("Upload a new asset", new_asset_path, :only_path => false) %>
+

cccms dashboard

+ -

- recent changes - my work - current drafts (<%= @drafts_count %>) - site map -

+
+ <%= link_to new_node_path, class: "action_button" do %> + <%= icon("plus", library: "tabler", "aria-hidden": true) %> Create post + <% end %> + <%= link_to chapters_nodes_path, class: "action_button" do %> + <%= icon("map-pin", library: "tabler", "aria-hidden": true) %> Find a chapter + <% end %> + <%= link_to new_asset_path, class: "action_button" do %> + <%= icon("upload", library: "tabler", "aria-hidden": true) %> Upload asset + <% end %> + <%= link_to sitemap_nodes_path, class: "action_button" do %> + <%= icon("list-tree", library: "tabler", "aria-hidden": true) %> Add a page in the page tree + <% end %> +
-
- -

Recent Changes

+
+
+

Drafts and autosaves

+
    + <% @drafts.each do |node| %> +
  • +
    + <%= link_to title_for_node(node), node_path(node) %> + <%= link_to_path("#{node.unique_name} ↗", node.unique_name) %> +
    + + <% if (editor = node_last_editor(node)) %><%= editor %>, <% end %><%= relative_time_phrase(node.updated_at) %> + +
  • + <% end %> +
+ <%= link_to "See all drafts →", drafts_nodes_path %> +
- - - - - - - - +
+

Recent changes

+
    <% @recent_changes.each do |node| %> -
"> - - - - - - +
  • +
    + <%= link_to title_for_node(node), node_path(node) %> + <%= link_to_path("#{node.unique_name} ↗", node.unique_name) %> +
    + + <%= (editor = node_head_editor(node)) ? t("published_by", editor: editor) : t("published") %>, <%= relative_time_phrase(node.head.updated_at) %> + +
  • <% end %> -
    IDTitleActionsLocked byRev.
    <%= node.id %> -

    <%= link_to title_for_node(node), node_path(node) %>

    -

    <%= link_to_path(node.unique_name, node.unique_name) %>

    -
    - <%= link_to 'show', node_path(node) %> - <%= link_to 'Revisions', node_revisions_path(node) %> - - <%= node.lock_owner.login if node.lock_owner %> - - <%= link_to ( node.draft ? node.draft.revision : (node.head ? node.head.revision : "EMPTY" ) ), node_revisions_path(node) %> -
    + + <%= link_to "See all recent changes →", recent_nodes_path %> +
    -
    - -

    My Work

    - - - - - - - - - - <% @mynodes.each do |node| %> - "> - - - - - - +
    +

    Housekeeping

    +
    + <%= link_to nodes_path, class: "action_button" do %> + <%= icon("list", library: "tabler", "aria-hidden": true) %> Nodes <% end %> -
    IDTitleActionsLocked byRev.
    <%= node.id %> -

    <%= link_to title_for_node(node), node_path(node) %>

    -

    <%= link_to_path(node.unique_name, node.unique_name) %>

    -
    - <%= link_to 'show', node_path(node) %> - <%= link_to 'Revisions', node_revisions_path(node) %> - - <%= node.lock_owner.login if node.lock_owner %> - - <%= link_to ( node.draft ? node.draft.revision : (node.head ? node.head.revision : "EMPTY" ) ), node_revisions_path(node) %> -
    -
    - -
    - -

    Current Drafts & Autosaves (<%= @drafts_count %>)

    - - - - - - - - - - - <% @drafts.each do |node| %> - "> - - - - - - - + <%= link_to events_path, class: "action_button" do %> + <%= icon("calendar", library: "tabler", "aria-hidden": true) %> Events <% end %> -
    IDTitleActionsLocked byRev.Autosave
    <%= node.id %> -

    <%= link_to title_for_node(node), node_path(node) %>

    -

    <%= link_to_path(node.unique_name, node.unique_name) %>

    -
    - <%= link_to 'show', node_path(node) %> - <%= link_to 'Revisions', node_revisions_path(node) %> - <% if pair = node.available_layer_pairs.last %> - <%= link_to 'diff', diff_node_revisions_path(node, start_revision: pair.first, end_revision: pair.last) %> - <% end %> - - <%= node.lock_owner.login if node.lock_owner %> - - <%= link_to ( node.draft ? node.draft.revision : (node.head ? node.head.revision : "EMPTY" ) ), node_revisions_path(node) %> - - <%= node.autosave ? "unsaved changes" : "" %> -
    -
    - -
    - -

    Sitemap

    - - - - - - - - - <% @sitemap.each do |node| %> - <% if !node.nil? %> - "> - - - - - + <%= link_to assets_path, class: "action_button" do %> + <%= icon("folder", library: "tabler", "aria-hidden": true) %> Assets + <% end %> + <%= link_to users_path, class: "action_button" do %> + <%= icon("users", library: "tabler", "aria-hidden": true) %> Users <% end %> + <%= link_to menu_items_path, class: "action_button" do %> + <%= icon("menu-2", library: "tabler", "aria-hidden": true) %> Navigation <% end %> -
    IDTitleActionsLocked by
    <%= node.id %> -

    <%= link_to title_for_node(node), node_path(node) %>

    -

    <%= link_to_path(node.unique_name, node.unique_name) %>

    -
    - <%= link_to 'create subpage', new_node_path(:parent_id => node.id) %> - <%= link_to 'Revisions', node_revisions_path(node) %> - - <%= node.lock_owner.login if node.lock_owner %> -
    +
    diff --git a/test/controllers/admin_controller_test.rb b/test/controllers/admin_controller_test.rb index 9beaf58..00a51e2 100644 --- a/test/controllers/admin_controller_test.rb +++ b/test/controllers/admin_controller_test.rb @@ -15,26 +15,6 @@ class AdminControllerTest < ActionController::TestCase assert_includes assigns(:drafts), node end - test "my work list shows each matching node only once, even with several revisions by the same user" do - login_as :quentin - user = User.find_by_login("quentin") - node = Node.root.children.create!(:slug => "dedup_test") - node.lock_for_editing!(user) - node.autosave!({:title => "v1"}, user) - node.save_draft!(user) - node.publish_draft! - node.lock_for_editing!(user) - node.autosave!({:title => "v2"}, user) - node.save_draft!(user) - node.publish_draft! - # three pages now exist on this node, all touched by quentin -- - # without DISTINCT, the join would return this node three times - - get :index - matches = assigns(:mynodes).select { |n| n.id == node.id } - assert_equal 1, matches.length - end - test "dashboard_search returns matching tags and nodes grouped separately" do node = Node.root.children.create!(:slug => "dashboard_search_test") node.find_or_create_draft(User.find_by_login("aaron")) diff --git a/test/controllers/nodes_controller_test.rb b/test/controllers/nodes_controller_test.rb index b43d2de..81e3f45 100644 --- a/test/controllers/nodes_controller_test.rb +++ b/test/controllers/nodes_controller_test.rb @@ -521,6 +521,26 @@ class NodesControllerTest < ActionController::TestCase assert_response :success end + test "mine shows each matching node only once, even with several revisions by the same user" do + login_as :quentin + user = User.find_by_login("quentin") + node = Node.root.children.create!(:slug => "dedup_test") + node.lock_for_editing!(user) + node.autosave!({:title => "v1"}, user) + node.save_draft!(user) + node.publish_draft! + node.lock_for_editing!(user) + node.autosave!({:title => "v2"}, user) + node.save_draft!(user) + node.publish_draft! + # three pages now exist on this node, all touched by quentin -- + # without DISTINCT, the join would return this node three times + + get :mine + matches = assigns(:nodes).select { |n| n.id == node.id } + assert_equal 1, matches.length + end + test "chapters combined with a search term does not raise an ambiguous column error" do login_as :quentin get :chapters, params: { :q => "Zombies" } diff --git a/test/models/node_test.rb b/test/models/node_test.rb index de540f8..0bce222 100644 --- a/test/models/node_test.rb +++ b/test/models/node_test.rb @@ -561,4 +561,28 @@ class NodeTest < ActiveSupport::TestCase result = Node.drafts_and_autosaves(current_user_id: @user1.id).to_a assert result.index(mine) < result.index(someone_elses_newer) end + + test "recently_changed includes a node whose head was recently published" do + node = Node.root.children.create!(:slug => "recent_changed_published") + node.find_or_create_draft(@user1) + node.autosave!({:title => "v1"}, @user1) + node.save_draft!(@user1) + node.publish_draft! + + assert_includes Node.recently_changed, node + end + + test "recently_changed excludes a node only touched by locking or unlocking after an old publish" do + node = Node.root.children.create!(:slug => "recent_changed_lock_only") + node.find_or_create_draft(@user1) + node.autosave!({:title => "v1"}, @user1) + node.save_draft!(@user1) + node.publish_draft! + node.head.update_column(:updated_at, 20.days.ago) + + node.lock_for_editing!(@user1) + node.unlock! + + assert_not_includes Node.recently_changed, node + end end -- cgit v1.3 From 848ce18303f927bfadaf0965a769cae2c62492a0 Mon Sep 17 00:00:00 2001 From: erdgeist Date: Sun, 12 Jul 2026 23:43:02 +0200 Subject: Normalize action-button wording and add icons app-wide "New X" becomes "Create X" throughout (users, events, assets, menu items, nodes), matching the verb-first pattern the dashboard's own signposts already established, with a shared plus icon rather than a document-flavored one that only made sense next to "post". The "Destroy"/"destroy"/"Delete" family is normalized to "Destroy" everywhere, with a shared trash icon; occurrences#index and pages#index also pick up the destructive button class they'd been silently missing. Filter and Search convert from submit_tag to button_tag, the only way either can hold an icon alongside its label. Edit and the node editor's three dynamic labels (Continue Editing / Edit Draft / Lock + Edit) share one icon without touching their wording -- unlike Destroy's family, the state nuance in the text is real information, not just inconsistent phrasing. --- app/views/assets/index.html.erb | 8 ++++++-- app/views/assets/show.html.erb | 4 +++- app/views/events/edit.html.erb | 6 ++++-- app/views/events/index.html.erb | 8 ++++++-- app/views/events/show.html.erb | 8 ++++++-- app/views/events/without_node.html.erb | 4 +++- app/views/menu_items/index.html.erb | 11 +++++++---- app/views/nodes/_node_list.html.erb | 4 +++- app/views/nodes/chapters.html.erb | 6 ++++-- app/views/nodes/edit.html.erb | 8 +++++--- app/views/nodes/index.html.erb | 4 +++- app/views/nodes/show.html.erb | 15 ++++++++++----- app/views/occurrences/index.html.erb | 4 +++- app/views/pages/index.html.erb | 4 +++- app/views/revisions/diff.html.erb | 10 ++++++---- app/views/users/_user.html.erb | 7 ++++--- app/views/users/index.html.erb | 8 ++++++-- app/views/users/show.html.erb | 4 +++- test/controllers/users_controller_test.rb | 2 +- 19 files changed, 86 insertions(+), 39 deletions(-) (limited to 'test/controllers') diff --git a/app/views/assets/index.html.erb b/app/views/assets/index.html.erb index 6591091..8c35561 100644 --- a/app/views/assets/index.html.erb +++ b/app/views/assets/index.html.erb @@ -1,6 +1,8 @@

    Assets

    -<%= link_to 'New asset', new_asset_path, class: 'action_button' %> +<%= link_to new_asset_path, class: 'action_button' do %> + <%= icon("plus", library: "tabler", "aria-hidden": true) %> Create asset +<% end %> <%= will_paginate @assets %> @@ -20,7 +22,9 @@ <%= asset.upload.content_type %> <%= link_to 'Show', asset %> <%= link_to 'Edit', edit_asset_path(asset) %> - <%= button_to 'Destroy', asset, method: :delete, form: { data: { confirm: 'Are you sure?' }, class: 'button_to destructive' } %> + <%= button_to asset, method: :delete, form: { data: { confirm: 'Are you sure?' }, class: 'button_to destructive' } do %> + <%= icon("trash", library: "tabler", "aria-hidden": true) %> Destroy + <% end %> <% end %> diff --git a/app/views/assets/show.html.erb b/app/views/assets/show.html.erb index 0286678..5717dd9 100644 --- a/app/views/assets/show.html.erb +++ b/app/views/assets/show.html.erb @@ -6,7 +6,9 @@
    - <%= link_to 'Edit', edit_asset_path(@asset), class: 'action_button' %> + <%= link_to edit_asset_path(@asset), class: 'action_button' do %> + <%= icon("edit", library: "tabler", "aria-hidden": true) %> Edit + <% end %>
    <%= link_to 'Back', assets_path %> diff --git a/app/views/events/edit.html.erb b/app/views/events/edit.html.erb index 45b084f..b6564a4 100644 --- a/app/views/events/edit.html.erb +++ b/app/views/events/edit.html.erb @@ -2,8 +2,10 @@
    <%= link_to 'Back', safe_return_to(params[:return_to] || events_path) %> - <%= button_to 'Destroy', event_path(@event), method: :delete, - form: { data: { confirm: 'Delete this event?' }, class: 'button_to destructive' } %> + <%= button_to event_path(@event), method: :delete, + form: { data: { confirm: 'Delete this event?' }, class: 'button_to destructive' } do %> + <%= icon("trash", library: "tabler", "aria-hidden": true) %> Destroy + <% end %>
    <%= form_for(@event) do |f| %> diff --git a/app/views/events/index.html.erb b/app/views/events/index.html.erb index 3d953d5..dde897e 100644 --- a/app/views/events/index.html.erb +++ b/app/views/events/index.html.erb @@ -1,7 +1,11 @@

    Events

    -<%= link_to 'New event', new_event_path, class: 'action_button' %> -<%= link_to 'View events without a page →', without_node_events_path %> +
    + <%= link_to new_event_path, class: 'action_button' do %> + <%= icon("plus", library: "tabler", "aria-hidden": true) %> Create event + <% end %> + <%= link_to 'View events without a page →', without_node_events_path %> +
    <%= will_paginate @events %> diff --git a/app/views/events/show.html.erb b/app/views/events/show.html.erb index 5812228..ac084c9 100644 --- a/app/views/events/show.html.erb +++ b/app/views/events/show.html.erb @@ -11,10 +11,14 @@
    <% end %>
    - <%= link_to 'Edit', edit_event_path(@event, return_to: request.path), class: 'action_button' %> + <%= link_to edit_event_path(@event, return_to: request.path), class: 'action_button' do %> + <%= icon("edit", library: "tabler", "aria-hidden": true) %> Edit + <% end %>
    - <%= button_to 'Destroy', event_path(@event), method: :delete, form: { data: { confirm: 'Delete this event?' }, class: 'button_to destructive' } %> + <%= button_to event_path(@event), method: :delete, form: { data: { confirm: 'Delete this event?' }, class: 'button_to destructive' } do %> + <%= icon("trash", library: "tabler", "aria-hidden": true) %> Destroy + <% end %>
    diff --git a/app/views/events/without_node.html.erb b/app/views/events/without_node.html.erb index 52514ab..bb27173 100644 --- a/app/views/events/without_node.html.erb +++ b/app/views/events/without_node.html.erb @@ -1,7 +1,9 @@

    Events without a node

    <%= link_to '← All events', events_path %> -<%= link_to 'New event', new_event_path, class: 'action_button' %> +<%= link_to new_event_path, class: 'action_button' do %> + <%= icon("plus", library: "tabler", "aria-hidden": true) %> Create event +<% end %> <%= will_paginate @events %> diff --git a/app/views/menu_items/index.html.erb b/app/views/menu_items/index.html.erb index e84199b..591eb68 100644 --- a/app/views/menu_items/index.html.erb +++ b/app/views/menu_items/index.html.erb @@ -1,6 +1,8 @@

    Menu Items

    -<%= link_to 'New menu item', new_menu_item_path, class: 'action_button' %> +<%= link_to new_menu_item_path, class: 'action_button' do %> + <%= icon("plus", library: "tabler", "aria-hidden": true) %> Create menu item +<% end %> <% @menu_items.each do |menu_item| %> @@ -11,9 +13,10 @@ <% end %> diff --git a/app/views/nodes/_node_list.html.erb b/app/views/nodes/_node_list.html.erb index 03f38b4..f7210f1 100644 --- a/app/views/nodes/_node_list.html.erb +++ b/app/views/nodes/_node_list.html.erb @@ -4,7 +4,9 @@ <% end %> <%= hidden_field_tag :tags, params[:tags] if params[:tags].present? %> <%= text_field_tag :q, params[:q], placeholder: "Search title, abstract, body…" %> - <%= submit_tag "Search", class: "action_button" %> + <%= button_tag type: "submit", class: "action_button" do %> + <%= icon("search", library: "tabler", "aria-hidden": true) %> Search + <% end %> <% if params[:q].present? || params[:kinds].present? || params[:tags].present? %> <%= link_to "Reset", url_for(controller: params[:controller], action: params[:action]) %> <% end %> diff --git a/app/views/nodes/chapters.html.erb b/app/views/nodes/chapters.html.erb index 939f19c..543bc53 100644 --- a/app/views/nodes/chapters.html.erb +++ b/app/views/nodes/chapters.html.erb @@ -1,9 +1,11 @@

    Chapters

    -<%= form_tag chapters_nodes_path, method: :get do %> +<%= form_tag chapters_nodes_path, method: :get, class: "node_search_form" do %> - <%= submit_tag "Filter", class: "action_button" %> + <%= button_tag type: "submit", class: "action_button" do %> + <%= icon("filter", library: "tabler", "aria-hidden": true) %> Filter + <% end %> <% end %> <%= render 'node_list' %> diff --git a/app/views/nodes/edit.html.erb b/app/views/nodes/edit.html.erb index 13b78fc..1ac05e0 100644 --- a/app/views/nodes/edit.html.erb +++ b/app/views/nodes/edit.html.erb @@ -6,9 +6,11 @@ disabled: @node.autosave.present? %> <% if @node.autosave || (@node.draft && @node.head) %> - <%= button_to (@node.draft && !@node.autosave ? 'Destroy Draft' : 'Discard Autosave'), - revert_node_path(@node), method: :put, - form: { data: { confirm: "This cannot be undone. Continue?" }, class: 'button_to destructive' } %> + <%= button_to revert_node_path(@node), method: :put, + form: { data: { confirm: "This cannot be undone. Continue?" }, class: 'button_to destructive' } do %> + <%= icon("trash", library: "tabler", "aria-hidden": true) %> + <%= @node.draft && !@node.autosave ? "Destroy Draft" : "Discard Autosave" %> + <% end %> <% if pair = @node.available_layer_pairs.find { |p| p.include?(:autosave) } %> <%= button_to 'What changed?', diff_node_revisions_path(@node), diff --git a/app/views/nodes/index.html.erb b/app/views/nodes/index.html.erb index 2688ea1..3bb251c 100644 --- a/app/views/nodes/index.html.erb +++ b/app/views/nodes/index.html.erb @@ -1,7 +1,9 @@

    Nodes

    - <%= link_to 'New node', new_node_path, class: 'action_button' %> + <%= link_to new_node_path, class: 'action_button' do %> + <%= icon("plus", library: "tabler", "aria-hidden": true) %> Create node + <% end %> Creates a node with no parent or kind set — for a specific page, use the wizard's create flow or "add child" from an existing node instead.
    diff --git a/app/views/nodes/show.html.erb b/app/views/nodes/show.html.erb index 07cb5d1..6e20258 100644 --- a/app/views/nodes/show.html.erb +++ b/app/views/nodes/show.html.erb @@ -25,7 +25,10 @@ <% if locked_by_other %> <%= edit_label %> <% else %> - <%= link_to (@node.autosave ? "Continue Editing" : (@node.draft ? "Edit Draft" : "Lock + Edit")), edit_node_path(@node), class: "action_button" %> + <%= link_to edit_node_path(@node), class: "action_button" do %> + <%= icon("edit", library: "tabler", "aria-hidden": true) %> + <%= @node.autosave ? "Continue Editing" : (@node.draft ? "Edit Draft" : "Lock + Edit") %> + <% end %> <% if !@node.draft && !@node.autosave %> Nothing pending — this will start a fresh draft. <% end %> @@ -51,10 +54,12 @@ <% end %> <% if @node.autosave || (@node.draft && @node.head) %>
    - <%= button_to (@node.draft && !@node.autosave ? 'Destroy Draft' : 'Discard Autosave'), - revert_node_path(@node), method: :put, - params: { return_to: request.path }, - form: { data: { confirm: "This cannot be undone. Continue?" }, class: 'button_to destructive' } %> + <%= button_to revert_node_path(@node), method: :put, + params: { return_to: request.path }, + form: { data: { confirm: "This cannot be undone. Continue?" }, class: 'button_to destructive' } do %> + <%= icon("trash", library: "tabler", "aria-hidden": true) %> + <%= @node.draft && !@node.autosave ? "Destroy Draft" : "Discard Autosave" %> + <% end %>
    <% end %> <% end %> diff --git a/app/views/occurrences/index.html.erb b/app/views/occurrences/index.html.erb index 82818d0..aa084ed 100644 --- a/app/views/occurrences/index.html.erb +++ b/app/views/occurrences/index.html.erb @@ -18,7 +18,9 @@ - + <% end %> diff --git a/app/views/pages/index.html.erb b/app/views/pages/index.html.erb index 05c7311..da67871 100644 --- a/app/views/pages/index.html.erb +++ b/app/views/pages/index.html.erb @@ -12,7 +12,9 @@ <%=h page.title %> <%= link_to 'Show', content_path(:page_path => page.node.unique_path) %> <%= link_to 'Edit', edit_page_path(page) %> - <%= button_to 'Destroy', page, method: :delete, form: { data: { confirm: 'Are you sure?' } } %> + <%= button_to page, method: :delete, form: { data: { confirm: 'Are you sure?' }, class: 'button_to destructive' } do %> + <%= icon("trash", library: "tabler", "aria-hidden": true) %> Destroy + <% end %> <% end %> diff --git a/app/views/revisions/diff.html.erb b/app/views/revisions/diff.html.erb index b9ce6bd..5638b7a 100644 --- a/app/views/revisions/diff.html.erb +++ b/app/views/revisions/diff.html.erb @@ -49,10 +49,12 @@ <% end %> <% if !@locked_by_other && (@node.autosave || @node.draft) %> - <%= button_to (@node.draft && !@node.autosave ? 'Destroy Draft' : 'Discard Autosave'), - revert_node_path(@node), method: :put, - params: { return_to: request.fullpath }, - form: { data: { confirm: "This cannot be undone. Continue?" }, class: 'button_to destructive' } %> + <%= button_to revert_node_path(@node), method: :put, + params: { return_to: request.fullpath }, + form: { data: { confirm: "This cannot be undone. Continue?" }, class: 'button_to destructive' } do %> + <%= icon("trash", library: "tabler", "aria-hidden": true) %> + <%= @node.draft && !@node.autosave ? "Destroy Draft" : "Discard Autosave" %> + <% end %> <% end %>

    <% end %> diff --git a/app/views/users/_user.html.erb b/app/views/users/_user.html.erb index 798b82b..3a88f5e 100644 --- a/app/views/users/_user.html.erb +++ b/app/views/users/_user.html.erb @@ -7,9 +7,10 @@ <%= link_to "edit", edit_user_path(user) %> - <%= button_to "destroy", user_path(user), - method: :delete, - form: { data: { confirm: "Do you really want to delete user #{user.login}?" }, class: 'button_to destructive' } %> + <%= button_to user_path(user), method: :delete, + form: { data: { confirm: "Do you really want to destroy user #{user.login}?" }, class: 'button_to destructive' } do %> + <%= icon("trash", library: "tabler", "aria-hidden": true) %> Destroy + <% end %> <% end %> diff --git a/app/views/users/index.html.erb b/app/views/users/index.html.erb index 46e958b..4f5266d 100644 --- a/app/views/users/index.html.erb +++ b/app/views/users/index.html.erb @@ -1,5 +1,7 @@

    Admins

    -<%= link_to 'New admin user', new_user_path(admin: true), class: 'action_button' %> +<%= link_to new_user_path(admin: true), class: 'action_button' do %> + <%= icon("plus", library: "tabler", "aria-hidden": true) %> Create admin user +<% end %> @@ -11,7 +13,9 @@
    Login

    Users

    -<%= link_to 'New user', new_user_path, class: 'action_button' %> +<%= link_to new_user_path, class: 'action_button' do %> + <%= icon("plus", library: "tabler", "aria-hidden": true) %> Create user +<% end %> diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb index e744efe..70881db 100644 --- a/app/views/users/show.html.erb +++ b/app/views/users/show.html.erb @@ -6,7 +6,9 @@
    - <%= link_to 'Edit', edit_user_path(@user), class: 'action_button' %> + <%= link_to edit_user_path(@user), class: 'action_button' do %> + <%= icon("edit", library: "tabler", "aria-hidden": true) %> Edit + <% end %>
    diff --git a/test/controllers/users_controller_test.rb b/test/controllers/users_controller_test.rb index 5cd5ad4..58f8a86 100644 --- a/test/controllers/users_controller_test.rb +++ b/test/controllers/users_controller_test.rb @@ -13,7 +13,7 @@ class UsersControllerTest < ActionController::TestCase login_as :aaron get :index assert_response :success - assert_select "button[type=submit]", "destroy" + assert_select "button[type=submit]", "Destroy" assert_select "a", "show" end -- cgit v1.3 From 70653b681d10917b77dced08f577446ced7568f1 Mon Sep 17 00:00:00 2001 From: erdgeist Date: Mon, 13 Jul 2026 03:22:37 +0200 Subject: Add RelatedAssetsController: search, attach, detach, reorder Backend for the asset-picker rebuild -- replaces the plan to dump every image asset in the system into a hidden, unfiltered browse panel on every node edit (the actual current behavior, confirmed by reading nodes/edit.html.erb directly) with a small, name-scoped search endpoint plus create/destroy/update for attach, detach, and reorder. No schema change needed -- RelatedAsset already had everything this requires (asset_id, page_id, an acts_as_list position). search excludes assets already attached to the page, keeping results meaningfully small given hundreds of assets total but only a handful per node in practice. create is find_or_create_by! rather than a bare create!, guarding against the same asset being attached twice from two separate search results. update leans on acts_as_list's own insert_at rather than custom position-shifting logic. Node#editable_page (autosave || draft || head) is extracted since this is now its third call site with identical logic -- deliberately not touching nodes#show, which resolves draft || head without autosave on purpose, a different and correct semantic for "current committed state" versus "what's actively being edited." --- app/controllers/related_assets_controller.rb | 50 +++++++++++++ app/models/node.rb | 4 ++ config/routes.rb | 6 ++ test/controllers/related_assets_controller_test.rb | 83 ++++++++++++++++++++++ 4 files changed, 143 insertions(+) create mode 100644 app/controllers/related_assets_controller.rb create mode 100644 test/controllers/related_assets_controller_test.rb (limited to 'test/controllers') diff --git a/app/controllers/related_assets_controller.rb b/app/controllers/related_assets_controller.rb new file mode 100644 index 0000000..906c02c --- /dev/null +++ b/app/controllers/related_assets_controller.rb @@ -0,0 +1,50 @@ +class RelatedAssetsController < ApplicationController + before_action :login_required + before_action :find_node + + def search + term = params[:q].to_s.strip + if term.blank? + render json: [] + return + end + + attached_ids = @node.editable_page.related_assets.pluck(:asset_id) + results = Asset.images + .where("name ILIKE ?", "%#{term}%") + .where.not(id: attached_ids) + .limit(10) + + render json: results.map { |a| + { id: a.id, name: a.name, thumb_url: a.upload.url(:thumb) } + } + end + + def create + asset = Asset.find(params[:asset_id]) + related = @node.editable_page.related_assets.find_or_create_by!(asset: asset) + + render json: { + id: related.id, + asset_id: asset.id, + name: asset.name, + thumb_url: asset.upload.url(:thumb) + } + end + + def destroy + @node.editable_page.related_assets.find(params[:id]).destroy + head :ok + end + + def update + @node.editable_page.related_assets.find(params[:id]).insert_at(params[:position].to_i) + head :ok + end + + private + + def find_node + @node = Node.find(params[:node_id]) + end +end diff --git a/app/models/node.rb b/app/models/node.rb index 1d0a089..0361c1e 100644 --- a/app/models/node.rb +++ b/app/models/node.rb @@ -328,6 +328,10 @@ class Node < ApplicationRecord unique_path.length == 3 && unique_path[0] == "updates" end + def editable_page + autosave || draft || head + end + # Returns immutable node id for all new nodes so that the atom feed entry ids # stay the same eventhough the slug or positions changes. # Can be removed after a year or so ;) diff --git a/config/routes.rb b/config/routes.rb index 92301e5..5d61bae 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -55,6 +55,12 @@ Cccms::Application.routes.draw do put :revert end + resources :related_assets, only: [:create, :destroy, :update] do + collection do + get :search + end + end + resources :revisions do collection do post :diff diff --git a/test/controllers/related_assets_controller_test.rb b/test/controllers/related_assets_controller_test.rb new file mode 100644 index 0000000..a3082c2 --- /dev/null +++ b/test/controllers/related_assets_controller_test.rb @@ -0,0 +1,83 @@ +require 'test_helper' + +class RelatedAssetsControllerTest < ActionController::TestCase + test "search finds assets by name, excluding ones already attached" do + login_as :quentin + node = Node.root.children.create!(:slug => "related_assets_search_test") + + attached = Asset.create!(:name => "biometrics-scan", :upload_content_type => "image/png") + node.draft.assets << attached + + Asset.create!(:name => "biometrics-poster", :upload_content_type => "image/png") + Asset.create!(:name => "chaostreff-flyer", :upload_content_type => "image/png") + + get :search, params: { :node_id => node.id, :q => "biometrics" } + + json = JSON.parse(response.body) + names = json.map { |a| a["name"] } + assert_includes names, "biometrics-poster" + assert_not_includes names, "biometrics-scan" + assert_not_includes names, "chaostreff-flyer" + end + + test "search returns an empty list for a blank term" do + login_as :quentin + node = Node.root.children.create!(:slug => "related_assets_blank_search_test") + + get :search, params: { :node_id => node.id, :q => "" } + + assert_equal [], JSON.parse(response.body) + end + + test "create attaches an asset to the node's editable page" do + login_as :quentin + node = Node.root.children.create!(:slug => "related_assets_create_test") + asset = Asset.create!(:name => "erfa-photo", :upload_content_type => "image/png") + + post :create, params: { :node_id => node.id, :asset_id => asset.id } + + assert_response :success + assert_includes node.draft.reload.related_assets.map(&:asset_id), asset.id + end + + test "create does not duplicate an already-attached asset" do + login_as :quentin + node = Node.root.children.create!(:slug => "related_assets_dup_test") + asset = Asset.create!(:name => "erfa-photo-2", :upload_content_type => "image/png") + node.draft.assets << asset + + post :create, params: { :node_id => node.id, :asset_id => asset.id } + + assert_response :success + assert_equal 1, node.draft.reload.related_assets.count + end + + test "destroy removes the attached asset" do + login_as :quentin + node = Node.root.children.create!(:slug => "related_assets_destroy_test") + asset = Asset.create!(:name => "old-photo", :upload_content_type => "image/png") + node.draft.assets << asset + related = node.draft.related_assets.first + + delete :destroy, params: { :node_id => node.id, :id => related.id } + + assert_response :success + assert_equal 0, node.draft.reload.related_assets.count + end + + test "update reorders the attached assets" do + login_as :quentin + node = Node.root.children.create!(:slug => "related_assets_reorder_test") + first = Asset.create!(:name => "first-photo", :upload_content_type => "image/png") + second = Asset.create!(:name => "second-photo", :upload_content_type => "image/png") + node.draft.assets << first + node.draft.assets << second + + second_related = node.draft.related_assets.find_by(:asset_id => second.id) + patch :update, params: { :node_id => node.id, :id => second_related.id, :position => 1 } + + assert_response :success + ordered_asset_ids = node.draft.reload.related_assets.map(&:asset_id) + assert_equal [second.id, first.id], ordered_asset_ids + end +end -- cgit v1.3 From 5f56fa867f073d8d1aecbfbba4a340ea414cfb0d Mon Sep 17 00:00:00 2001 From: erdgeist Date: Mon, 13 Jul 2026 03:48:28 +0200 Subject: Add loadOnFocus to initSearchPicker; asset search shows recent by default initSearchPicker's input handler has always short-circuited on an empty term -- correct for every existing picker, none of which want results appearing before anything's typed. The asset picker does want exactly that (show the last few uploaded images without requiring a search first), so this adds an opt-in loadOnFocus option instead of changing the shared default: it fires once when the input gains focus with no term yet, reusing the same request/render path a real search uses rather than a separate code path. The AJAX call itself was pulled out into a named runSearch function so both triggers could share it without duplicating the success/render logic. RelatedAssetsController#search now treats a blank term as "show the most recently created, unattached images" (limit 5) rather than returning nothing, matching the new client behavior. A real search term still returns up to 10 name-matched results, unchanged. --- app/controllers/related_assets_controller.rb | 18 ++- public/javascripts/admin_search.js | 133 +++++++++++---------- test/controllers/related_assets_controller_test.rb | 16 ++- 3 files changed, 91 insertions(+), 76 deletions(-) (limited to 'test/controllers') diff --git a/app/controllers/related_assets_controller.rb b/app/controllers/related_assets_controller.rb index 906c02c..ae37d2f 100644 --- a/app/controllers/related_assets_controller.rb +++ b/app/controllers/related_assets_controller.rb @@ -3,17 +3,15 @@ class RelatedAssetsController < ApplicationController before_action :find_node def search - term = params[:q].to_s.strip - if term.blank? - render json: [] - return - end - + term = params[:search_term].to_s.strip attached_ids = @node.editable_page.related_assets.pluck(:asset_id) - results = Asset.images - .where("name ILIKE ?", "%#{term}%") - .where.not(id: attached_ids) - .limit(10) + scope = Asset.images.where.not(id: attached_ids) + + results = if term.present? + scope.where("name ILIKE ?", "%#{term}%").limit(10) + else + scope.order(created_at: :desc).limit(5) + end render json: results.map { |a| { id: a.id, name: a.name, thumb_url: a.upload.url(:thumb) } diff --git a/public/javascripts/admin_search.js b/public/javascripts/admin_search.js index 0c768a3..7604bb7 100644 --- a/public/javascripts/admin_search.js +++ b/public/javascripts/admin_search.js @@ -42,18 +42,76 @@ function initSearchPicker(options) { var inputSelector = options.inputSelector; var resultsSelector = options.resultsSelector; var url = options.url || ADMIN_MENU_SEARCH_URL; - var onSelect = options.onSelect; // omit for a real-navigation search like admin_search - var isActive = options.isActive; // optional guard, checked before every search - var resultsHeaderHtml = options.resultsHeaderHtml; // optional, prepended only when results exist - var renderResults = options.renderResults; // optional, replaces the default per-node rendering entirely + var onSelect = options.onSelect; + var isActive = options.isActive; + var resultsHeaderHtml = options.resultsHeaderHtml; + var renderResults = options.renderResults; + var loadOnFocus = options.loadOnFocus; // optional, fires an initial search on focus with no term typed yet var requestId = 0; var timeout; + var results = $(resultsSelector); + + function runSearch(term) { + var thisRequest = ++requestId; + $.ajax({ + type: "GET", + url: url, + data: "search_term=" + term, + dataType: "json", + success: function(data) { + if (thisRequest !== requestId) return; + results.empty(); + + var found; + if (renderResults) { + found = renderResults(data, results, resultsHeaderHtml); + } else { + if (data.length && resultsHeaderHtml) { + results.append(resultsHeaderHtml); + } + found = false; + for (var i = 0; i < data.length; i++) { + (function(node) { + var link; + if (onSelect) { + link = $( + "

    " + node.title + + "" + node.unique_name + "

    " + ); + link.find("a").bind("click", function() { + onSelect(node); + results.slideUp(); + results.empty(); + return false; + }); + } else { + link = $( + "

    " + node.title + + "" + node.unique_name + "

    " + ); + } + results.append(link); + found = true; + })(data[i]); + } + } + + if (found) { + results.slideDown(); + } else { + results.slideUp(); + } + }, + error: function(xhr, status, error) { + console.log("Ajax error:", status, error, xhr.status, xhr.responseText); + } + }); + } $(inputSelector).bind("input", function() { if (isActive && !isActive()) return; var term = $(this).val(); - var results = $(resultsSelector); clearTimeout(timeout); if (!term) { @@ -62,63 +120,16 @@ function initSearchPicker(options) { return; } - timeout = setTimeout(function() { - var thisRequest = ++requestId; - $.ajax({ - type: "GET", - url: url, - data: "search_term=" + term, - dataType: "json", - success: function(data) { - if (thisRequest !== requestId) return; - results.empty(); - - var found; - if (renderResults) { - found = renderResults(data, results, resultsHeaderHtml); - } else { - if (data.length && resultsHeaderHtml) { - results.append(resultsHeaderHtml); - } - found = false; - for (var i = 0; i < data.length; i++) { - (function(node) { - var link; - if (onSelect) { - link = $( - "

    " + node.title + - "" + node.unique_name + "

    " - ); - link.find("a").bind("click", function() { - onSelect(node); - results.slideUp(); - results.empty(); - return false; - }); - } else { - link = $( - "

    " + node.title + - "" + node.unique_name + "

    " - ); - } - results.append(link); - found = true; - })(data[i]); - } - } - - if (found) { - results.slideDown(); - } else { - results.slideUp(); - } - }, - error: function(xhr, status, error) { - console.log("Ajax error:", status, error, xhr.status, xhr.responseText); - } - }); - }, 250); + timeout = setTimeout(function() { runSearch(term); }, 250); }); + + if (loadOnFocus) { + $(inputSelector).bind("focus", function() { + if (isActive && !isActive()) return; + if ($(this).val()) return; // the input handler above already covers a real term + runSearch(""); + }); + } } dashboard_search = { diff --git a/test/controllers/related_assets_controller_test.rb b/test/controllers/related_assets_controller_test.rb index a3082c2..7f6d487 100644 --- a/test/controllers/related_assets_controller_test.rb +++ b/test/controllers/related_assets_controller_test.rb @@ -11,7 +11,7 @@ class RelatedAssetsControllerTest < ActionController::TestCase Asset.create!(:name => "biometrics-poster", :upload_content_type => "image/png") Asset.create!(:name => "chaostreff-flyer", :upload_content_type => "image/png") - get :search, params: { :node_id => node.id, :q => "biometrics" } + get :search, params: { :node_id => node.id, :search_term => "biometrics" } json = JSON.parse(response.body) names = json.map { |a| a["name"] } @@ -20,13 +20,19 @@ class RelatedAssetsControllerTest < ActionController::TestCase assert_not_includes names, "chaostreff-flyer" end - test "search returns an empty list for a blank term" do + test "search with a blank term returns the most recently created assets" do + Asset.delete_all + RelatedAsset.delete_all login_as :quentin - node = Node.root.children.create!(:slug => "related_assets_blank_search_test") + node = Node.root.children.create!(:slug => "related_assets_recent_test") - get :search, params: { :node_id => node.id, :q => "" } + Asset.create!(:name => "older-photo", :upload_content_type => "image/png", :created_at => 2.days.ago) + Asset.create!(:name => "newer-photo", :upload_content_type => "image/png", :created_at => 1.hour.ago) - assert_equal [], JSON.parse(response.body) + get :search, params: { :node_id => node.id, :search_term => "" } + + json = JSON.parse(response.body) + assert_equal ["newer-photo", "older-photo"], json.map { |a| a["name"] } end test "create attaches an asset to the node's editable page" do -- cgit v1.3 From 8baac265059b70da0148487458ee4077b15f155e Mon Sep 17 00:00:00 2001 From: erdgeist Date: Mon, 13 Jul 2026 04:54:33 +0200 Subject: Asset picker: attach/detach/reorder UI, read-only view, autosave fix 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
    Login