From 1fa2f3821497d5529a6753cee84cf5ca679e8bcc Mon Sep 17 00:00:00 2001 From: erdgeist Date: Sun, 5 Jul 2026 21:49:21 +0200 Subject: Fix admin search results colliding with other search widgets layouts/admin.html.erb's top-bar search results div shared id "search_results" with nodes#new/nodes#edit's parent-search widget - since the layout renders on every admin page, whichever widget's JS ran last silently won the shared ID, putting top-bar results into the parent-search box on affected pages. Renamed to "menu_search_results" and updated admin_search.js to match. Also modernizes admin_search.js's event bindings from keyup/keydown/ keypress/paste/cut + .attr("value") to input + .val() throughout (menu_items, parent_search, move_to_search) - the multi-event binding was working around old IE compatibility that .val() + "input" already handles correctly. parent_search's result rendering now matches menu_search's convention (real

/ markup with a .result_path span) instead of a bare in a throwaway wrapper div, so the same CSS rule now correctly applies to both. menu_search's JSON response gains node_path per result, matching what admin_search's own results already provide - not yet consumed by parent_search/move_to_search, which still render click-to-select links rather than navigable ones (correct for their purpose - selecting a value, not leaving the page). Known remaining gap, not fixed here: menu_items, parent_search, and move_to_search still all target the literal id "search_results" between themselves. No live collision today since none of the three currently share a page, but it's the same fragility as the bug above - tracked alongside the existing menu_items/parent_search/move_to_search consolidation backlog item rather than treated as resolved. --- app/controllers/admin_controller.rb | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) (limited to 'app/controllers/admin_controller.rb') diff --git a/app/controllers/admin_controller.rb b/app/controllers/admin_controller.rb index d671384..e0098b0 100644 --- a/app/controllers/admin_controller.rb +++ b/app/controllers/admin_controller.rb @@ -29,46 +29,49 @@ class AdminController < ApplicationController .order("updated_at desc") .uniq.first(50) end - + + def conventions + @node_kinds = CccConventions::NODE_KINDS + end + def search @results = Node.search params[:search_term], :per_page => 1000 - + respond_to do |format| format.html do render :template => 'admin/search_results' end - format.js do - render( :json => @results.map do |node| + format.js do + render( :json => @results.map do |node| if node { :id => node.id, :title => node.title, :unique_name => node.unique_name, :node_path => node_path(node) } end end ) - - end + + end end end - + def menu_search if params[:search_term] == "Root" @results = [Node.root] else @results = Node.search params[:search_term] end - + respond_to do |format| format.html do render :partial => 'admin/menu_search_results' end - - format.js do - render( :json => @results.map do |node| - {:node_id => node.id, :title => node.title, :unique_name => node.unique_name} + + format.js do + render( :json => @results.map do |node| + {:node_id => node.id, :title => node.title, :unique_name => node.unique_name, :node_path => node_path(node)} end ) - - end + + end end end - end -- cgit v1.3 From d95947f9d738fd74d1fc16c6a1843b7f46bc0484 Mon Sep 17 00:00:00 2001 From: erdgeist Date: Sun, 5 Jul 2026 21:52:56 +0200 Subject: Whitespace fixes --- app/controllers/admin_controller.rb | 4 ++-- app/controllers/nodes_controller.rb | 28 ++++++++++++++-------------- 2 files changed, 16 insertions(+), 16 deletions(-) (limited to 'app/controllers/admin_controller.rb') diff --git a/app/controllers/admin_controller.rb b/app/controllers/admin_controller.rb index e0098b0..bfc6cd8 100644 --- a/app/controllers/admin_controller.rb +++ b/app/controllers/admin_controller.rb @@ -1,7 +1,7 @@ class AdminController < ApplicationController - + # Private - + before_action :login_required def index diff --git a/app/controllers/nodes_controller.rb b/app/controllers/nodes_controller.rb index 1e1def2..380a659 100644 --- a/app/controllers/nodes_controller.rb +++ b/app/controllers/nodes_controller.rb @@ -1,14 +1,14 @@ class NodesController < ApplicationController - + # Private - + layout 'admin' - + before_action :login_required before_action :find_node, :only => [ - :show, - :edit, - :update, + :show, + :edit, + :update, :destroy, :publish, :unlock @@ -27,10 +27,10 @@ class NodesController < ApplicationController @parent_name = Node.find(@parent_id).title end end - + def create params[:title] ||= "" - + @node = Node.new @node.parent_id = find_parent @node.slug = slug_for(params[:title]) @@ -49,7 +49,7 @@ class NodesController < ApplicationController render :new end end - + def show node = Node.find(params[:id]) node.wipe_draft! @@ -87,20 +87,20 @@ class NodesController < ApplicationController def destroy @node.destroy end - + def publish @node.publish_draft! flash[:notice] = "Draft has been published" redirect_to node_path(@node) end - + def unlock if @node.unlock! flash[:notice] = "Node unlocked" else flash[:notice] = "Already unlocked" end - + redirect_to node_path(@node) end @@ -121,11 +121,11 @@ class NodesController < ApplicationController def page_params params.fetch(:page, {}).permit(:title, :abstract, :body, :template_name, :published_at, :user_id) end - + def find_node @node = Node.find(params[:id]) end - + def find_parent case params[:kind] when "generic" -- cgit v1.3 From 7b6af89509e8439fe2474e623ee97e4db67ab011 Mon Sep 17 00:00:00 2001 From: erdgeist Date: Tue, 7 Jul 2026 02:46:37 +0200 Subject: Replace awesome_nested_set with a plain parent_id-based NestedTree concern Root-caused this session: appending a child to any node never widened that parent's own rgt boundary, on the pinned revision (Gemfile tracked main directly, chasing a too-conservative gemspec constraint - not, as first assumed, a deliberate pin to avoid a known bug). Reproduced cleanly on a single ordinary create with no concurrency and no bulk operation involved, confirmed via the gem's own SetValidator, then confirmed as the root cause of nodes_controller_test.rb's 3 long-standing "pre-existing" failures - not three separate mysteries, one bug. admin_controller's sitemap needed its own real conversion, not just a drop-in: awesome_nested_set's lft column implicitly provided correct depth-first tree order for free, which the old code combined with a separate class-level each_with_level iterator. Both replaced by one method, self_and_descendants_ordered_with_level, computing an ordered [node, level] list in a single query-then-walk pass - checked against the actual view template first (admin/index.html.erb) rather than assumed, since it relies on list order alone to render correct visual nesting. lft/rgt/depth columns intentionally left in schema, unused - dropping them is a separate, deliberately deferred migration once this is proven running for a while, not bundled with the behavior change. --- Gemfile | 3 -- Gemfile.lock | 10 ----- app/controllers/admin_controller.rb | 8 ++-- app/models/concerns/nested_tree.rb | 86 +++++++++++++++++++++++++++++++++++++ app/models/node.rb | 25 +++-------- db/seeds/chapters.rb | 3 -- 6 files changed, 95 insertions(+), 40 deletions(-) create mode 100644 app/models/concerns/nested_tree.rb (limited to 'app/controllers/admin_controller.rb') diff --git a/Gemfile b/Gemfile index 13bb4ca..d136eb5 100644 --- a/Gemfile +++ b/Gemfile @@ -44,9 +44,6 @@ gem 'will_paginate', '~> 3.0' gem 'acts-as-taggable-on', git: 'https://github.com/mbleigh/acts-as-taggable-on.git', branch: 'master' -gem 'awesome_nested_set', - git: 'https://github.com/collectiveidea/awesome_nested_set.git', - branch: 'main' # ── XML / parsing ───────────────────────────────────────────────────────────── diff --git a/Gemfile.lock b/Gemfile.lock index 2e9a1ce..02c71ff 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,11 +1,3 @@ -GIT - remote: https://github.com/collectiveidea/awesome_nested_set.git - revision: 7a642749af74519b4bf3642d939061beb36835e1 - branch: main - specs: - awesome_nested_set (3.9.0) - activerecord (>= 4.0.0, < 8.2) - GIT remote: https://github.com/erdgeist/chaoscalendar.git revision: 48fe5c517ae3895cc78d9d2fefbdd35b90d52ba7 @@ -335,7 +327,6 @@ PLATFORMS DEPENDENCIES acts-as-taggable-on! acts_as_list - awesome_nested_set! chaos_calendar! coffee-rails (~> 4.0) concurrent-ruby (~> 1.3) @@ -374,7 +365,6 @@ CHECKSUMS activesupport (8.1.3) sha256=21a5e0dfbd4c3ddd9e1317ec6a4d782fa226e7867dc70b0743acda81a1dca20e acts-as-taggable-on (13.0.0) acts_as_list (1.2.6) sha256=8345380900b7bee620c07ad00991ccee59af3d8c9e8574f426e321da2865fdc8 - awesome_nested_set (3.9.0) base64 (0.3.0) sha256=27337aeabad6ffae05c265c450490628ef3ebd4b67be58257393227588f5a97b bigdecimal (4.1.2) sha256=53d217666027eab4280346fba98e7d5b66baaae1b9c3c1c0ffe89d48188a3fbd builder (3.3.0) sha256=497918d2f9dca528fdca4b88d84e4ef4387256d984b8154e9d5d3fe5a9c8835f diff --git a/app/controllers/admin_controller.rb b/app/controllers/admin_controller.rb index bfc6cd8..3fa0519 100644 --- a/app/controllers/admin_controller.rb +++ b/app/controllers/admin_controller.rb @@ -15,12 +15,10 @@ class AdminController < ApplicationController Time.now, Time.now - 14.days ).limit(50).order("updated_at desc") - all_nodes = Node.root.self_and_descendants + ordered_with_level = Node.root.self_and_descendants_ordered_with_level @sitemap_depth = {} - Node.each_with_level(all_nodes) do |node, level| - @sitemap_depth[node.id] = level - end - @sitemap = all_nodes.to_a.sort! { |node1,node2| node1.lft <=> node2.lft }.delete_if { |node| node.update? } + 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) diff --git a/app/models/concerns/nested_tree.rb b/app/models/concerns/nested_tree.rb new file mode 100644 index 0000000..9a2eb0e --- /dev/null +++ b/app/models/concerns/nested_tree.rb @@ -0,0 +1,86 @@ +# app/models/concerns/nested_tree.rb +# +# Minimal parent_id-based replacement for the tree-traversal subset of +# awesome_nested_set actually used by this app (descendants, ancestors, +# level, root, move_to_child_of) +module NestedTree + extend ActiveSupport::Concern + + included do + belongs_to :parent, class_name: name, foreign_key: :parent_id, optional: true, inverse_of: :children + has_many :children, -> { order(:id) }, class_name: name, foreign_key: :parent_id, inverse_of: :parent + + before_destroy :delete_descendants + end + + class_methods do + def root + roots.first + end + + def roots + where(parent_id: nil) + end + + def valid? + # Every non-root row's parent_id must point at a real, existing row. + where.not(parent_id: nil).where.missing(:parent).none? + end + end + + def root? + parent_id.nil? + end + + def ancestors + result = [] + current = parent + while current + result << current + current = current.parent + end + result + end + + def level + ancestors.size + end + + def descendants + ids = [] + queue = self.class.where(parent_id: id).pluck(:id) + until queue.empty? + ids.concat(queue) + queue = self.class.where(parent_id: queue).pluck(:id) + end + self.class.where(id: ids) + end + + def self_and_descendants + self.class.where(id: [id] + descendants.pluck(:id)) + end + + def self_and_descendants_ordered_with_level + nodes = [self] + descendants.to_a + children_by_parent = nodes.group_by(&:parent_id) + children_by_parent.each_value { |list| list.sort_by!(&:id) } + + result = [] + visit = ->(node, level) do + result << [node, level] + (children_by_parent[node.id] || []).each { |child| visit.call(child, level + 1) } + end + visit.call(self, 0) + result + end + + def move_to_child_of(new_parent) + update!(parent_id: new_parent.id) + end + + private + + def delete_descendants + self.class.where(id: descendants.pluck(:id)).delete_all + end +end diff --git a/app/models/node.rb b/app/models/node.rb index 2177f15..fc23dc1 100644 --- a/app/models/node.rb +++ b/app/models/node.rb @@ -1,6 +1,6 @@ class Node < ApplicationRecord # Mixins and Plugins - acts_as_nested_set + include NestedTree # Associations has_many :pages, -> { order("revision ASC") }, :dependent => :destroy @@ -21,16 +21,6 @@ class Node < ApplicationRecord validates_uniqueness_of :slug, :scope => :parent_id, :unless => -> { parent_id.nil? } validates_presence_of :parent_id, :unless => -> { Node.root.nil? } - validate :borders # This should never ever happen. - - # Index for Fulltext Search - # define_index do - # indexes head.translations.title - # indexes slug - # indexes unique_name - # indexes head.translations.body - # end - # Class methods # Returns a page for a given node. If no revision is supplied, it returns @@ -254,10 +244,13 @@ class Node < ApplicationRecord # Watch out recursion ahead! update_unique_name itself triggers this # after_save callback which invokes update_unique_name on its children. # Hopefully until no childrens occur + # + # Queries parent_id directly rather than the NestedTree#children + # association out of habit from the old awesome_nested_set-avoidance + # workaround - no longer strictly necessary now that children is + # equally safe, but left as-is since it already works correctly. def update_unique_names_of_children unless root? - # Use parent_id-based traversal instead of lft/rgt descendants - # due to awesome_nested_set not refreshing parent lft/rgt in memory Node.where(:parent_id => self.id).each do |child| child.reload child.update_unique_name @@ -265,10 +258,4 @@ class Node < ApplicationRecord end end end - - def borders - if lft && rgt && (lft > rgt) - errors.add("Fuck!. lft should never be smaller than rgt") - end - end end diff --git a/db/seeds/chapters.rb b/db/seeds/chapters.rb index bebc8da..4d81e3c 100644 --- a/db/seeds/chapters.rb +++ b/db/seeds/chapters.rb @@ -1409,6 +1409,3 @@ puts "Nodes flagged for review:" (erfas + chaostreffs).select { |e| e[:review] }.each do |e| puts " #{e[:slug]}" end - -Node.rebuild!(false) - -- 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 'app/controllers/admin_controller.rb') 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 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 'app/controllers/admin_controller.rb') 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 19e0ee821d5b2b6d3397a81411f4f3a4cbd2fa83 Mon Sep 17 00:00:00 2001 From: erdgeist Date: Sun, 12 Jul 2026 13:58:36 +0200 Subject: Unify the four field-level pickers, fix search visibility menu_items/parent_search/move_to_search/event_search each duplicated their own debounce-free AJAX call and result rendering. Replaced with one shared initSearchPicker, taking each picker's distinctive behavior (parent_search's live path preview, event_search's title hint) as a callback rather than a whole reimplementation. Adds a real debounce with an out-of-order-response guard -- none of the four had either before. admin_search (the Alt+F top-bar search) now shares the same function via its own url/isActive/header options, gaining the same guard and fixing an inconsistency of its own (it previously always slid its panel open, even on zero results). Each picker's results container gets its own id instead of sharing was ever supposed to be on screen at once, an assumption with no actual enforcement behind it. Styling moved from an id-pair (#menu_search_results, #search_results) to a shared .search_results class so a future picker never needs this file touched again. menu_search and the admin top-bar search now call Node.editor_search instead of the public, head-only Node.search -- both are admin-only, authenticated views, and had no reason to inherit the public search's "can't find a draft" limitation. The always-ignored :per_page => 1000 on the latter is gone too; Node.search's second argument was never read by the method at all. Also removed a stale #metadata a { text-transform: lowercase } rule, found while verifying the above -- written for the pre-subnav-removal expand-toggle, which no longer exists; it had been silently lowercasing nodes#edit's own, unrelated #metadata div (including move_to_search's results) by id coincidence ever since. #main_navigation and #overview_toggle intentionally left capitalized rather than special-cased -- both belong to the nav bar already slated to shrink to three icons, not worth polishing on the way out. --- app/controllers/admin_controller.rb | 4 +- app/views/events/edit.html.erb | 4 +- app/views/events/new.html.erb | 2 +- app/views/layouts/admin.html.erb | 4 +- app/views/menu_items/edit.html.erb | 2 +- app/views/menu_items/new.html.erb | 4 +- app/views/nodes/edit.html.erb | 14 +- app/views/nodes/new.html.erb | 2 +- public/javascripts/admin_search.js | 355 ++++++++++++------------------------ public/stylesheets/admin.css | 13 +- 10 files changed, 137 insertions(+), 267 deletions(-) (limited to 'app/controllers/admin_controller.rb') diff --git a/app/controllers/admin_controller.rb b/app/controllers/admin_controller.rb index 435df57..3c45c49 100644 --- a/app/controllers/admin_controller.rb +++ b/app/controllers/admin_controller.rb @@ -31,7 +31,7 @@ class AdminController < ApplicationController end def search - @results = Node.search params[:search_term], :per_page => 1000 + @results = Node.editor_search(params[:search_term]) respond_to do |format| format.html do @@ -53,7 +53,7 @@ class AdminController < ApplicationController if params[:search_term] == "Root" @results = [Node.root] else - @results = Node.search params[:search_term] + @results = Node.editor_search(params[:search_term]) end respond_to do |format| diff --git a/app/views/events/edit.html.erb b/app/views/events/edit.html.erb index 6cb04bd..45b084f 100644 --- a/app/views/events/edit.html.erb +++ b/app/views/events/edit.html.erb @@ -19,12 +19,12 @@
Change node <%= text_field_tag :event_node_search_term %> -
+
This will re-link the event to a different node — rarely needed.
<% else %> <%= text_field_tag :event_node_search_term %> -
+
Optional — search and pick a node to associate this event with a page. <% end %> <%= f.hidden_field :node_id %> diff --git a/app/views/events/new.html.erb b/app/views/events/new.html.erb index b20fe48..7a1ee7a 100644 --- a/app/views/events/new.html.erb +++ b/app/views/events/new.html.erb @@ -10,7 +10,7 @@
Node
<%= text_field_tag :event_node_search_term %> -
+
<%= f.hidden_field :node_id %> Optional — search and pick a node to associate this event with a page.
diff --git a/app/views/layouts/admin.html.erb b/app/views/layouts/admin.html.erb index e714c28..079346b 100644 --- a/app/views/layouts/admin.html.erb +++ b/app/views/layouts/admin.html.erb @@ -56,9 +56,7 @@ Search: <%= text_field_tag :search_term, nil, autocomplete: "off" %> <% end %>
- +
diff --git a/app/views/menu_items/edit.html.erb b/app/views/menu_items/edit.html.erb index dc5e8f9..44e5a79 100644 --- a/app/views/menu_items/edit.html.erb +++ b/app/views/menu_items/edit.html.erb @@ -6,7 +6,7 @@
Search
<%= text_field_tag :menu_search_term %> -
+
Node Id
diff --git a/app/views/menu_items/new.html.erb b/app/views/menu_items/new.html.erb index 68081d0..223cb8e 100644 --- a/app/views/menu_items/new.html.erb +++ b/app/views/menu_items/new.html.erb @@ -7,9 +7,7 @@ Search <%= text_field_tag :menu_search_term %> -
- -
+ diff --git a/app/views/nodes/edit.html.erb b/app/views/nodes/edit.html.erb index 1c19410..13b78fc 100644 --- a/app/views/nodes/edit.html.erb +++ b/app/views/nodes/edit.html.erb @@ -45,14 +45,12 @@
parent
<%= text_field_tag :move_to_search_term, @node.parent.title rescue "" %> -
- -
- <%= f.hidden_field( - :staged_parent_id, - :value => @node.staged_parent_id || @node.parent_id - ) - %> +
+ <%= f.hidden_field( + :staged_parent_id, + :value => @node.staged_parent_id || @node.parent_id + ) + %>
<%= fields_for @page do |d| %> diff --git a/app/views/nodes/new.html.erb b/app/views/nodes/new.html.erb index afc632f..bb7e078 100644 --- a/app/views/nodes/new.html.erb +++ b/app/views/nodes/new.html.erb @@ -34,7 +34,7 @@
<%= text_field_tag :parent_search_term, @parent_name %> <%= hidden_field_tag :parent_id, @parent_id %> -
+
diff --git a/public/javascripts/admin_search.js b/public/javascripts/admin_search.js index 0e70845..f553334 100644 --- a/public/javascripts/admin_search.js +++ b/public/javascripts/admin_search.js @@ -1,5 +1,4 @@ admin_search = { - initialize : function() { $(document).bind("keydown", 'Alt+f', function(){ admin_search.display_toggle(); @@ -20,25 +19,12 @@ admin_search = { } }); - $("#search_term").bind("input", function() { - if (!$('#search_widget').is(':visible')) return; - if ($(this).val()) { - $.ajax({ - type: "GET", - url: ADMIN_SEARCH_URL, - data: "search_term=" + $(this).val(), - dataType: "json", - success: function(results) { - admin_search.show_results(results); - }, - error: function(xhr, status, error) { - console.log("Ajax error:", status, error, xhr.status, xhr.responseText); - } - }); - } else { - $('#menu_search_results').slideUp(); - $('#menu_search_results').empty(); - } + initSearchPicker({ + inputSelector: "#search_term", + resultsSelector: "#menu_search_results", + url: ADMIN_SEARCH_URL, + isActive: function() { return $('#search_widget').is(':visible'); }, + resultsHeaderHtml: "

Press Enter to see all results ⏎

" }); }, @@ -49,75 +35,97 @@ admin_search = { $('#search_widget').fadeIn(); $('#search_term').focus(); } - }, - - show_results : function(results) { - $('#menu_search_results').empty(); - if (results.length) { - $('#menu_search_results').append( - "

Press Enter to see all results ⏎

" - ); - } - for (result in results) { - $('#menu_search_results').append( - "

" + - results[result].title + - "" + results[result].unique_name + "" + - "

" - ); - } - $('#menu_search_results').slideDown(); } }; -menu_items = { +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 requestId = 0; + var timeout; + + $(inputSelector).bind("input", function() { + if (isActive && !isActive()) return; + + var term = $(this).val(); + var results = $(resultsSelector); + clearTimeout(timeout); + + if (!term) { + results.slideUp(); + results.empty(); + return; + } - initialize_search : function() { - $("#menu_search_term").bind("input", function() { - if ($(this).val()) { - $.ajax({ - type: "GET", - url: ADMIN_MENU_SEARCH_URL, - data: "search_term=" + $(this).val(), - dataType: "json", - success : function(results) { - menu_items.show_results(results); + timeout = setTimeout(function() { + var thisRequest = ++requestId; + $.ajax({ + type: "GET", + url: url, + data: "search_term=" + term, + dataType: "json", + success: function(nodes) { + if (thisRequest !== requestId) return; + results.empty(); + if (nodes.length && resultsHeaderHtml) { + results.append(resultsHeaderHtml); } - }); - } - else { - $('#search_results').slideUp(); - $('#search_results').empty(); + 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]); + } + if (found) { + results.slideDown(); + } else { + results.slideUp(); + } + }, + error: function(xhr, status, error) { + console.log("Ajax error:", status, error, xhr.status, xhr.responseText); + } + }); + }, 250); + }); +} + +menu_items = { + initialize_search : function() { + initSearchPicker({ + inputSelector: "#menu_search_term", + resultsSelector: "#menu_item_search_results", + onSelect: function(node) { + $("#menu_item_node_id").val(node.node_id); + $("#menu_item_path").val("/" + node.unique_name); + $("#menu_item_title").val(node.title); } }); - }, - - show_results : function(results) { - $("#search_results").empty(); - for (result in results) { - var link = $((""+ results[result].title + "")); - $(link).bind("click", menu_items.link_closure(results[result])); - - - // Sometimes I don't get jquery; wrap() didn't work *sigh* - // Guess I'll need a book someday or another framework - var wrapper = $("
"); - $(wrapper).append(link) - - $("#search_results").append(wrapper); - - } - }, - - link_closure : function(node) { - var barf = function(){ - $("#menu_item_node_id").val(node.node_id); - $("#menu_item_path").val("/" + node.unique_name); - $("#menu_item_title").val(node.title); - return false; - } - - return barf; } }; @@ -125,21 +133,13 @@ parent_search = { initialize_search : function() { parent_search.initialize_radio_buttons(); - $("#parent_search_term").bind("input", function() { - if ($(this).val()) { - $.ajax({ - type: "GET", - url: ADMIN_MENU_SEARCH_URL, - data: "search_term=" + $(this).val(), - dataType: "json", - success : function(results) { - parent_search.show_results(results); - } - }); - } - else { - $('#search_results').slideUp(); - $('#search_results').empty(); + initSearchPicker({ + inputSelector: "#parent_search_term", + resultsSelector: "#parent_search_results", + onSelect: function(node) { + $("#parent_search_term").val(node.title); + $("#parent_id").val(node.node_id).attr("data-unique-name", node.unique_name); + parent_search.update_resulting_path(); } }); @@ -154,38 +154,6 @@ parent_search = { }); }, - show_results : function(results) { - $("#search_results").empty(); - var found = false; - for (result in results) { - - var link = $(( - "

" + results[result].title + - "" + results[result].unique_name + "" + - "

")); - - $(link).bind("click", parent_search.link_closure(results[result])); - - $("#search_results").append(link); - found = true; - } - if (found) - $('#search_results').slideDown(); - }, - - link_closure : function(node) { - var barf = function(){ - $("#parent_search_term").val(node.title); - $("#parent_id").val(node.node_id).attr("data-unique-name", node.unique_name); - $('#search_results').slideUp(); - $('#search_results').empty(); - parent_search.update_resulting_path(); - return false; - } - - return barf; - }, - update_resulting_path : function() { var kind = $("input[name='kind']:checked"); var title = $("#title").val(); @@ -223,122 +191,35 @@ parent_search = { $("#parent_search_field").hide(); } } -} +}; move_to_search = { initialize_search : function() { - $("#move_to_search_term").bind("input", function() {move_to_search.do_search($(this))}); - }, - - do_search : function(_this) { - if (_this.val()) { - $.ajax({ - type: "GET", - url: ADMIN_MENU_SEARCH_URL, - data: "search_term=" + _this.val(), - dataType: "json", - success : function(results) { - move_to_search.show_results(results); - } - }); - } - else { - $('#search_results').slideUp(); - $('#search_results').empty(); - } - }, - - show_results : function(results) { - $("#search_results").empty(); - var found = false; - for (result in results) { - var link = $((""+ results[result].title + "")); - $(link).bind("click", move_to_search.link_closure(results[result])); - - - // Sometimes I don't get jquery; wrap() didn't work *sigh* - // Guess I'll need a book someday or another framework - var wrapper = $("
"); - $(wrapper).append(link) - - $("#search_results").append(wrapper); - found = true; - } - if (found) - $('#search_results').slideDown(); - else - $('#search_results').slideUp(); - - }, - - link_closure : function(node) { - var barf = function(){ - $("#move_to_search_term").val(node.title); - $("#node_staged_parent_id").val(node.node_id); - $('#search_results').slideUp(); - $('#search_results').empty(); - return false; - } - - return barf; + initSearchPicker({ + inputSelector: "#move_to_search_term", + resultsSelector: "#move_to_search_results", + onSelect: function(node) { + $("#move_to_search_term").val(node.title); + $("#node_staged_parent_id").val(node.node_id); + } + }); } -} +}; event_search = { initialize_search : function() { - $("#event_node_search_term").bind("input", function() { - if ($(this).val()) { - $.ajax({ - type: "GET", - url: ADMIN_MENU_SEARCH_URL, - data: "search_term=" + $(this).val(), - dataType: "json", - success : function(results) { - event_search.show_results(results); - } - }); - } - else { - $('#search_results').slideUp(); - $('#search_results').empty(); + initSearchPicker({ + inputSelector: "#event_node_search_term", + resultsSelector: "#event_search_results", + onSelect: function(node) { + $("#event_node_search_term").val(node.title); + $("#event_node_id").val(node.node_id); + + var title_field = $("#event_title"); + if (title_field.val() === "") { + $("#event_title_hint").text("Using \"" + node.title + "\" from the associated node."); + } } }); - }, - - show_results : function(results) { - $("#search_results").empty(); - var found = false; - for (result in results) { - var link = $(( - "

" + results[result].title + - "" + results[result].unique_name + "" + - "

")); - - $(link).bind("click", event_search.link_closure(results[result])); - - $("#search_results").append(link); - found = true; - } - if (found) - $('#search_results').slideDown(); - else - $('#search_results').slideUp(); - }, - - link_closure : function(node) { - var barf = function(){ - $("#event_node_search_term").val(node.title); - $("#event_node_id").val(node.node_id); - $('#search_results').slideUp(); - $('#search_results').empty(); - - var title_field = $("#event_title"); - if (title_field.val() === "") { - $("#event_title_hint").text("Using \"" + node.title + "\" from the associated node."); - } - - return false; - } - return barf; } }; diff --git a/public/stylesheets/admin.css b/public/stylesheets/admin.css index 5c1e489..da31535 100644 --- a/public/stylesheets/admin.css +++ b/public/stylesheets/admin.css @@ -155,7 +155,6 @@ input[type=radio] { padding-right: 5px; padding-top: 1px; padding-bottom: 1px; - text-transform: lowercase; } #metadata a:hover { @@ -1023,22 +1022,19 @@ div#draft_list table td.actions a { font-size: 18px; } -#menu_search_results p, -#search_results p { +.search_results p { margin: 0; padding: 4px 4px 0 4px; border-bottom: 1px solid #e8e8e8; } -#menu_search_results p a, -#search_results p a { +.search_results p a { display: block; font-weight: bold; font-size: 0.95rem; } -#menu_search_results p span.result_path, -#search_results p span.result_path { +.search_results p span.result_path { display: block; margin-top: 2px; font-size: 0.75rem; @@ -1047,8 +1043,7 @@ div#draft_list table td.actions a { padding-bottom: 4px; } -#menu_search_results p.search_more, -#search_results p.search_more { +.search_results p.search_more { margin: 0; padding: 6px 4px; font-size: 0.8rem; -- 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 'app/controllers/admin_controller.rb') 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 'app/controllers/admin_controller.rb') 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 %>
-
Continue my work
-
Add a page in the page tree
-
<%= 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