From a7a6ad786eeb9f94f7882462bccbdd31e1bb4743 Mon Sep 17 00:00:00 2001 From: erdgeist Date: Tue, 30 Jun 2026 19:15:22 +0200 Subject: Phase 1: standalone events, external_url on nodes - Migration: node_id nullable on events and occurrences, add title/description/is_primary to events, external_url to nodes - Existing events marked is_primary: true (were all 1:1 with nodes) - Node: has_one :event -> has_many :events - Event: belongs_to :node optional, validates title presence for standalone events, is_primary uniqueness scoped to node_id, display_title helper falling back through node title - Occurrence: belongs_to :node optional, summary falls back to event.display_title - nodes_helper: event_information uses events.first (interim; will be replaced in Phase 3 event UI) - Tests: fix node.event -> node.events.first in event_test --- test/models/event_test.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'test/models') diff --git a/test/models/event_test.rb b/test/models/event_test.rb index f310af8..d85aadd 100644 --- a/test/models/event_test.rb +++ b/test/models/event_test.rb @@ -74,17 +74,17 @@ class EventTest < ActiveSupport::TestCase assert_equal "2009-12-24T15:23:42".to_time, scoped_occurrences[51].start_time assert_equal "2009-12-24T20:05:23".to_time, scoped_occurrences[51].end_time - assert_equal @cal_node.event, scoped_occurrences[51].event + assert_equal @cal_node.events.first, scoped_occurrences[51].event assert_equal @cal_node, scoped_occurrences[51].node assert_equal "2009-03-19T15:23:42".to_time, scoped_occurrences[11].start_time assert_equal "2009-03-19T20:05:23".to_time, scoped_occurrences[11].end_time - assert_equal @cal_node.event, scoped_occurrences[11].event + assert_equal @cal_node.events.first, scoped_occurrences[11].event assert_equal @cal_node, scoped_occurrences[11].node assert_equal "2009-01-01T15:23:42".to_time, scoped_occurrences[0].start_time assert_equal "2009-01-01T20:05:23".to_time, scoped_occurrences[0].end_time - assert_equal @cal_node.event, scoped_occurrences[11].event + assert_equal @cal_node.events.first, scoped_occurrences[11].event assert_equal @cal_node, scoped_occurrences[11].node end @@ -111,4 +111,4 @@ class EventTest < ActiveSupport::TestCase chaosradio_days = scoped_occurrences.map {|x| x.start_time.day} assert_equal expected_days, chaosradio_days end -end \ No newline at end of file +end -- cgit v1.3 From 95955abaa339098755a214cfcadf87c90211fe64 Mon Sep 17 00:00:00 2001 From: erdgeist Date: Wed, 1 Jul 2026 00:24:10 +0200 Subject: Add RRULE humanizer and wire events into nodes#show - app/models/concerns/rrule_humanizer.rb: new concern included into Event, renders recurring schedule as natural-language German or English from RRULE string; handles WEEKLY/MONTHLY, biweekly (INTERVAL=2), ordinal weekday positions (1TU, -1TH, -2WE), BYMONTH single-month exclusions (December pause convention); gracefully returns nil for COUNT/UNTIL/unrecognized shapes - test/models/concerns/rrule_humanizer_test.rb: 15 tests covering all distinct RRULE shapes found in production data - app/helpers/nodes_helper.rb: add event_schedule_text helper combining humanize_rrule with start_time formatting - app/views/nodes/show.html.erb: add events row, conditionally rendered when node has associated events - config/locales/de.yml, en.yml: add event_schedule_time, event_schedule_unrecognized, event_schedule_none keys --- app/helpers/nodes_helper.rb | 16 ++++++ app/models/concerns/rrule_humanizer.rb | 82 +++++++++++++++++++++++++++ app/models/event.rb | 1 + app/views/nodes/show.html.erb | 14 ++++- config/locales/de.yml | 3 + config/locales/en.yml | 3 + test/models/concerns/rrule_humanizer_test.rb | 84 ++++++++++++++++++++++++++++ 7 files changed, 202 insertions(+), 1 deletion(-) create mode 100644 app/models/concerns/rrule_humanizer.rb create mode 100644 test/models/concerns/rrule_humanizer_test.rb (limited to 'test/models') diff --git a/app/helpers/nodes_helper.rb b/app/helpers/nodes_helper.rb index 4293628..329bcc5 100644 --- a/app/helpers/nodes_helper.rb +++ b/app/helpers/nodes_helper.rb @@ -43,4 +43,20 @@ module NodesHelper link_to('add event', new_event_path(:node_id => @node.id)) ]) end + + def event_schedule_text(event) + if event.rrule.present? + recurrence = event.humanize_rrule(I18n.locale) + if recurrence + time = event.start_time&.strftime("%H:%M") + time ? "#{recurrence} #{t(:event_schedule_time, time: time)}" : recurrence + else + "#{event.rrule} (#{t(:event_schedule_unrecognized)})" + end + elsif event.start_time + I18n.l(event.start_time, format: :long) + else + t(:event_schedule_none) + end + end end diff --git a/app/models/concerns/rrule_humanizer.rb b/app/models/concerns/rrule_humanizer.rb new file mode 100644 index 0000000..6cee711 --- /dev/null +++ b/app/models/concerns/rrule_humanizer.rb @@ -0,0 +1,82 @@ +module RruleHumanizer + extend ActiveSupport::Concern + + WEEKDAY_NAMES = { + de: { "MO"=>"Montag","TU"=>"Dienstag","WE"=>"Mittwoch","TH"=>"Donnerstag","FR"=>"Freitag","SA"=>"Samstag","SU"=>"Sonntag" }, + en: { "MO"=>"Monday","TU"=>"Tuesday","WE"=>"Wednesday","TH"=>"Thursday","FR"=>"Friday","SA"=>"Saturday","SU"=>"Sunday" } + }.freeze + + WEEKDAY_NAMES_ADVERBIAL = { + de: { "MO"=>"montags","TU"=>"dienstags","WE"=>"mittwochs","TH"=>"donnerstags","FR"=>"freitags","SA"=>"samstags","SU"=>"sonntags" } + }.freeze + + ORDINAL_NAMES = { + de: { 1=>"ersten", 2=>"zweiten", 3=>"dritten", 4=>"vierten", -1=>"letzten", -2=>"vorletzten" }, + en: { 1=>"first", 2=>"second", 3=>"third", 4=>"fourth", -1=>"last", -2=>"second-to-last" } + }.freeze + + MONTH_NAMES = { + de: %w[Januar Februar März April Mai Juni Juli August September Oktober November Dezember], + en: %w[January February March April May June July August September October November December] + }.freeze + + def humanize_rrule(locale = I18n.locale) + return nil if rrule.blank? + parts = Hash[rrule.split(";").map { |p| p.split("=", 2) }] + return nil if parts["COUNT"] || parts["UNTIL"] # old one-off data, don't guess + + freq, interval, byday, bymonth = parts["FREQ"], parts["INTERVAL"].to_i, parts["BYDAY"], parts["BYMONTH"] + loc = locale.to_sym + weekdays = WEEKDAY_NAMES[loc] || WEEKDAY_NAMES[:en] + ordinals = ORDINAL_NAMES[loc] || ORDINAL_NAMES[:en] + months = MONTH_NAMES[loc] || MONTH_NAMES[:en] + + days = byday&.split(",")&.map do |d| + if d =~ /^(-?\d+)([A-Z]{2})$/ + "#{ordinals[$1.to_i]} #{weekdays[$2]}" + else + weekdays[d] + end + end + + base = + case loc + when :de + case freq + when "WEEKLY" + if days + if interval == 2 + adverbial = byday.split(",").map { |d| WEEKDAY_NAMES_ADVERBIAL[:de][d] } + "Alle zwei Wochen #{adverbial.join(' und ')}" + else + "Jeden #{days.join(' und ')}" + end + else + interval == 2 ? "Alle zwei Wochen" : "Wöchentlich" + end + when "MONTHLY" + days ? "Jeden #{days.join(' und ')} im Monat" : "Monatlich" + end + else + case freq + when "WEEKLY" + days ? "#{interval == 2 ? 'Every other' : 'Every'} #{days.join(' and ')}" : (interval == 2 ? "Every other week" : "Weekly") + when "MONTHLY" + days ? "Every #{days.join(' and ')} of the month" : "Monthly" + end + end + return nil unless base + + if bymonth + included = bymonth.split(",").map(&:to_i) + missing = ((1..12).to_a - included) + if missing.size == 1 + excluded_name = months[missing.first - 1] + base += (loc == :de ? ", außer im #{excluded_name}" : ", except in #{excluded_name}") + end + # more than one missing month: bymonth pattern more complex than we handle, leave base as-is silently + end + + base + end +end diff --git a/app/models/event.rb b/app/models/event.rb index 26c79e4..de82674 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -1,4 +1,5 @@ class Event < ApplicationRecord + include RruleHumanizer belongs_to :node, optional: true has_many :occurrences diff --git a/app/views/nodes/show.html.erb b/app/views/nodes/show.html.erb index de4d8a2..7223219 100644 --- a/app/views/nodes/show.html.erb +++ b/app/views/nodes/show.html.erb @@ -42,6 +42,18 @@ Tagged with: <%= @page.tag_list %> + <% if @node.events.any? %> + + Events + + + + + <% end %> Title <%= sanitize( @page.title ) %> @@ -59,4 +71,4 @@ - \ No newline at end of file + diff --git a/config/locales/de.yml b/config/locales/de.yml index 5f77d79..0b42dd3 100644 --- a/config/locales/de.yml +++ b/config/locales/de.yml @@ -7,6 +7,9 @@ de: sponsors: Sponsoren show_tag_headline: "Seiten mit dem Tag:" old_ccc_de: das alte ccc.de + event_schedule_time: "um %{time} Uhr" + event_schedule_unrecognized: "Termin-Regel nicht automatisch lesbar" + event_schedule_none: "Kein Termin" date: formats: default: "%d.%m.%Y" diff --git a/config/locales/en.yml b/config/locales/en.yml index 2458d4d..93a0d55 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -8,6 +8,9 @@ en: hello: "Hello world" show_tag_headline: "Pages tagged with:" old_ccc_de: the old ccc.de + event_schedule_time: "at %{time}" + event_schedule_unrecognized: "Schedule not automatically readable" + event_schedule_none: "No schedule" time: formats: diff --git a/test/models/concerns/rrule_humanizer_test.rb b/test/models/concerns/rrule_humanizer_test.rb new file mode 100644 index 0000000..500dbc7 --- /dev/null +++ b/test/models/concerns/rrule_humanizer_test.rb @@ -0,0 +1,84 @@ +require 'test_helper' + +class RruleHumanizerTest < ActiveSupport::TestCase + def humanize(rrule, locale = :de) + Event.new(rrule: rrule).humanize_rrule(locale) + end + + test "weekly single day" do + assert_equal "Jeden Dienstag", humanize("FREQ=WEEKLY;BYDAY=TU") + assert_equal "Every Tuesday", humanize("FREQ=WEEKLY;BYDAY=TU", :en) + end + + test "weekly two days" do + assert_equal "Jeden Mittwoch und Freitag", humanize("FREQ=WEEKLY;BYDAY=WE,FR") + assert_equal "Every Wednesday and Friday", humanize("FREQ=WEEKLY;BYDAY=WE,FR", :en) + end + + test "weekly no byday" do + assert_equal "Wöchentlich", humanize("FREQ=WEEKLY") + assert_equal "Weekly", humanize("FREQ=WEEKLY", :en) + end + + test "biweekly with day" do + assert_equal "Alle zwei Wochen donnerstags", humanize("FREQ=WEEKLY;INTERVAL=2;BYDAY=TH") + assert_equal "Every other Thursday", humanize("FREQ=WEEKLY;INTERVAL=2;BYDAY=TH", :en) + end + + test "biweekly no day" do + assert_equal "Alle zwei Wochen", humanize("FREQ=WEEKLY;INTERVAL=2") + assert_equal "Every other week", humanize("FREQ=WEEKLY;INTERVAL=2", :en) + end + + test "monthly nth weekday" do + assert_equal "Jeden ersten Dienstag im Monat", humanize("FREQ=MONTHLY;BYDAY=1TU") + assert_equal "Jeden zweiten Freitag im Monat", humanize("FREQ=MONTHLY;BYDAY=2FR") + assert_equal "Jeden dritten Sonntag im Monat", humanize("FREQ=MONTHLY;BYDAY=3SU") + assert_equal "Jeden letzten Mittwoch im Monat", humanize("FREQ=MONTHLY;BYDAY=-1WE") + end + + test "monthly nth weekday english" do + assert_equal "Every first Tuesday of the month", humanize("FREQ=MONTHLY;BYDAY=1TU", :en) + assert_equal "Every last Wednesday of the month", humanize("FREQ=MONTHLY;BYDAY=-1WE", :en) + end + + test "monthly second-to-last" do + assert_equal "Jeden vorletzten Donnerstag im Monat", humanize("FREQ=MONTHLY;BYDAY=-2TH") + assert_equal "Every second-to-last Thursday of the month", humanize("FREQ=MONTHLY;BYDAY=-2TH", :en) + end + + test "monthly no byday" do + assert_equal "Monatlich", humanize("FREQ=MONTHLY") + assert_equal "Monthly", humanize("FREQ=MONTHLY", :en) + end + + test "monthly with single excluded month" do + assert_equal "Jeden letzten Donnerstag im Monat, außer im Dezember", + humanize("FREQ=MONTHLY;BYDAY=-1TH;BYMONTH=1,2,3,4,5,6,7,8,9,10,11") + assert_equal "Every last Thursday of the month, except in December", + humanize("FREQ=MONTHLY;BYDAY=-1TH;BYMONTH=1,2,3,4,5,6,7,8,9,10,11", :en) + end + + test "monthly excluding january" do + assert_equal "Jeden zweiten Mittwoch im Monat, außer im Januar", + humanize("FREQ=MONTHLY;BYMONTH=2,3,4,5,6,7,8,9,10,11,12;BYDAY=2WE") + end + + test "blank rrule returns nil" do + assert_nil humanize(nil) + assert_nil humanize("") + end + + test "count and until are not guessed at" do + assert_nil humanize("FREQ=MONTHLY;BYDAY=1WE;COUNT=36") + assert_nil humanize("FREQ=MONTHLY;BYDAY=1WE;UNTIL=20050105T222222Z") + end + + test "unrecognized freq returns nil" do + assert_nil humanize("FREQ=YEARLY;BYMONTH=12") + end + + test "falls back to english for unknown locale" do + assert_equal "Every Tuesday", humanize("FREQ=WEEKLY;BYDAY=TU", :fr) + end +end -- cgit v1.3 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. --- app/controllers/events_controller.rb | 4 + app/helpers/nodes_helper.rb | 14 +++ app/views/nodes/show.html.erb | 4 +- test/controllers/events_controller_test.rb | 167 ++++++++++++++++++++------- test/controllers/nodes_controller_test.rb | 33 ++++++ test/models/concerns/rrule_humanizer_test.rb | 15 +++ test/models/helpers/content_helper_test.rb | 4 + test/models/helpers/nodes_helper_test.rb | 21 ++++ 8 files changed, 219 insertions(+), 43 deletions(-) (limited to 'test/models') diff --git a/app/controllers/events_controller.rb b/app/controllers/events_controller.rb index 9bed5dd..b98a38e 100644 --- a/app/controllers/events_controller.rb +++ b/app/controllers/events_controller.rb @@ -37,6 +37,10 @@ class EventsController < ApplicationController tag_list: params[:tag_list] ) + if params[:tag_list].present? && params[:auto_tag_source].present? + flash.now[:notice] = "Tag '#{params[:tag_list]}' was pre-filled because this page is tagged '#{params[:auto_tag_source]}'. You can remove it below." + end + respond_to do |format| format.html # new.html.erb format.xml { render :xml => @event } diff --git a/app/helpers/nodes_helper.rb b/app/helpers/nodes_helper.rb index 329bcc5..093bfc6 100644 --- a/app/helpers/nodes_helper.rb +++ b/app/helpers/nodes_helper.rb @@ -44,6 +44,20 @@ module NodesHelper ]) end + DEFAULT_EVENT_TAG_BY_PAGE_TAG = { + 'erfa-detail' => 'open-day', + 'chaostreff-detail' => 'open-day' + }.freeze + + def default_event_tag_mapping(page) + page_tags = page.tag_list + DEFAULT_EVENT_TAG_BY_PAGE_TAG.find { |page_tag, _| page_tags.include?(page_tag) } + end + + def default_event_tag_list(page) + default_event_tag_mapping(page)&.last + end + def event_schedule_text(event) if event.rrule.present? recurrence = event.humanize_rrule(I18n.locale) diff --git a/app/views/nodes/show.html.erb b/app/views/nodes/show.html.erb index 2ce3853..c533a55 100644 --- a/app/views/nodes/show.html.erb +++ b/app/views/nodes/show.html.erb @@ -42,7 +42,6 @@ Tagged with: <%= @page.tag_list %> - <% if @node.events.any? %> Events @@ -55,9 +54,10 @@ <% end %> + <% mapping = default_event_tag_mapping(@page) %> + <%= link_to 'add event', new_event_path(node_id: @node.id, tag_list: mapping&.last, auto_tag_source: mapping&.first, return_to: request.path) %> - <% end %> Title <%= sanitize( @page.title ) %> 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 diff --git a/test/models/concerns/rrule_humanizer_test.rb b/test/models/concerns/rrule_humanizer_test.rb index 500dbc7..279ff73 100644 --- a/test/models/concerns/rrule_humanizer_test.rb +++ b/test/models/concerns/rrule_humanizer_test.rb @@ -81,4 +81,19 @@ class RruleHumanizerTest < ActiveSupport::TestCase test "falls back to english for unknown locale" do assert_equal "Every Tuesday", humanize("FREQ=WEEKLY;BYDAY=TU", :fr) end + + test "wday_abbr returns the correct German abbreviation for each day" do + monday = Time.parse("2026-07-06") # confirmed Monday + assert_equal "Mo", RruleHumanizer.wday_abbr(monday, :de) + assert_equal "Di", RruleHumanizer.wday_abbr(monday + 1.day, :de) + assert_equal "Mi", RruleHumanizer.wday_abbr(monday + 2.days, :de) + assert_equal "Do", RruleHumanizer.wday_abbr(monday + 3.days, :de) + assert_equal "Fr", RruleHumanizer.wday_abbr(monday + 4.days, :de) + assert_equal "Sa", RruleHumanizer.wday_abbr(monday + 5.days, :de) + assert_equal "So", RruleHumanizer.wday_abbr(monday + 6.days, :de) + end + + test "wday_abbr falls back to :de for an unrecognized locale" do + assert_equal "Mo", RruleHumanizer.wday_abbr(Time.parse("2026-07-06"), :fr) + end end diff --git a/test/models/helpers/content_helper_test.rb b/test/models/helpers/content_helper_test.rb index 2da82d7..a7ed478 100644 --- a/test/models/helpers/content_helper_test.rb +++ b/test/models/helpers/content_helper_test.rb @@ -1,4 +1,8 @@ require 'test_helper' class ContentHelperTest < ActionView::TestCase + test "weekday_abbr delegates through the current I18n locale" do + I18n.locale = :de + assert_equal "Mo", weekday_abbr(Time.parse("2026-07-06")) + end end diff --git a/test/models/helpers/nodes_helper_test.rb b/test/models/helpers/nodes_helper_test.rb index 13011de..5d91a88 100644 --- a/test/models/helpers/nodes_helper_test.rb +++ b/test/models/helpers/nodes_helper_test.rb @@ -1,4 +1,25 @@ require 'test_helper' class NodesHelperTest < ActionView::TestCase + FakePage = Struct.new(:tag_list) + + test "default_event_tag_mapping matches erfa-detail" do + page = FakePage.new(["erfa-detail"]) + assert_equal ["erfa-detail", "open-day"], default_event_tag_mapping(page) + end + + test "default_event_tag_mapping matches chaostreff-detail" do + page = FakePage.new(["chaostreff-detail"]) + assert_equal ["chaostreff-detail", "open-day"], default_event_tag_mapping(page) + end + + test "default_event_tag_mapping returns nil for unrelated tags" do + page = FakePage.new(["update"]) + assert_nil default_event_tag_mapping(page) + end + + test "default_event_tag_list is nil without a matching tag" do + page = FakePage.new([]) + assert_nil default_event_tag_list(page) + 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/models') 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 889e15eabbe107d2642fdd8aa3f03821058c00dc Mon Sep 17 00:00:00 2001 From: erdgeist Date: Mon, 6 Jul 2026 13:52:28 +0200 Subject: Fix shared preview links breaking after a second publish A token's page could stop being node.head_id (superseded by a newer draft) while still being published_at.present? - the old check only compared against the current head, so a superseded page fell through to direct rendering instead of redirecting, serving a stale frozen snapshot indefinitely instead of the current live content. Also handles scheduled publishing correctly: a page can be head_id but not yet public? (published_at in the future) - that case must still render directly, not redirect into a 404 on the not-yet-live public URL. --- app/controllers/shared_previews_controller.rb | 9 +++++-- test/models/page_test.rb | 35 ++++++++++++++++++++++++++- 2 files changed, 41 insertions(+), 3 deletions(-) (limited to 'test/models') diff --git a/app/controllers/shared_previews_controller.rb b/app/controllers/shared_previews_controller.rb index a8a540f..d482466 100644 --- a/app/controllers/shared_previews_controller.rb +++ b/app/controllers/shared_previews_controller.rb @@ -1,9 +1,14 @@ class SharedPreviewsController < ApplicationController def show @page = Page.find_by!(preview_token: params[:token]) + node = @page.node - if @page.node && @page.node.head_id == @page.id - redirect_to node_path(@page.node) + was_published = @page.published_at.present? + superseded = was_published && node && node.head_id != @page.id + currently_public = was_published && node && node.head_id == @page.id && @page.public? + + if superseded || currently_public + redirect_to node_path(node) return end diff --git a/test/models/page_test.rb b/test/models/page_test.rb index afba8b5..ad2742f 100644 --- a/test/models/page_test.rb +++ b/test/models/page_test.rb @@ -142,5 +142,38 @@ class PageTest < ActiveSupport::TestCase update = updates2009.children.create!( :slug => "my-first-update" ) assert_equal "update", update.draft.template_name end - + + test "a page scheduled for future publication is not yet public even after being published" do + node = Node.root.children.create!(slug: "preview-scheduled-test") + draft = node.find_or_create_draft(@user1) + draft.title = "Scheduled test" + draft.published_at = 1.day.from_now + draft.save! + token = draft.ensure_preview_token! + + node.publish_draft! + page = Page.find_by(preview_token: token) + + assert_equal page.id, page.node.head_id + assert_not page.public? + end + + test "a superseded page is no longer the head, even though it was once published" do + node = Node.root.children.create!(slug: "preview-superseded-test") + first_draft = node.find_or_create_draft(@user1) + first_draft.title = "First version" + first_draft.save! + first_token = first_draft.ensure_preview_token! + node.publish_draft! + + second_draft = node.find_or_create_draft(@user1) + second_draft.title = "Second version" + second_draft.save! + node.publish_draft! + + first_page = Page.find_by(preview_token: first_token) + + assert_not_equal first_page.id, first_page.node.head_id + assert first_page.published_at.present? + end end -- cgit v1.3 From a006440f59bf99380e179cc2963cb98d4d894d3a Mon Sep 17 00:00:00 2001 From: erdgeist Date: Wed, 8 Jul 2026 16:22:17 +0200 Subject: Add head/draft/autosave hierarchy to Node MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduces autosave_id as a third, unversioned layer above draft/head, with lock_for_editing!, autosave!, and save_draft! as the new entry points. Also fixes a real bug in wipe_draft!: its "no draft" branch unconditionally released the lock, which was safe when "no draft" only ever meant "nothing is happening" — no longer true now that a lock can exist with only an autosave beneath it. lock_for_editing! deliberately does not call wipe_draft! at all, for the same reason: an intruder calling it while a lock was genuinely held would otherwise silently steal it via wipe_draft!'s own unlock side effect, caught by the new two-user lock test. --- app/models/node.rb | 80 ++++++++++++++++++ .../20260708095943_add_autosave_id_to_nodes.rb | 5 ++ test/models/node_test.rb | 96 ++++++++++++++++++++++ 3 files changed, 181 insertions(+) create mode 100644 db/migrate/20260708095943_add_autosave_id_to_nodes.rb (limited to 'test/models') diff --git a/app/models/node.rb b/app/models/node.rb index fc23dc1..9eb0fe4 100644 --- a/app/models/node.rb +++ b/app/models/node.rb @@ -6,6 +6,7 @@ class Node < ApplicationRecord has_many :pages, -> { order("revision ASC") }, :dependent => :destroy belongs_to :head, :class_name => "Page", :foreign_key => :head_id, :dependent => :destroy, optional: true belongs_to :draft, :class_name => "Page", :foreign_key => :draft_id, :dependent => :destroy, optional: true + belongs_to :autosave, :class_name => "Page", :foreign_key => :autosave_id, :dependent => :destroy, optional: true has_many :permissions, :dependent => :destroy has_many :events, :dependent => :destroy belongs_to :lock_owner, :class_name => "User", :foreign_key => :locking_user_id, optional: true @@ -49,6 +50,71 @@ class Node < ApplicationRecord # Instance Methods + # Acquires (or reaffirms) the editing lock without creating a draft or + # an autosave -- both are now deferred until there is real content to + # hold. + def lock_for_editing! current_user + if self.lock_owner.nil? || self.lock_owner == current_user + lock_for! current_user + self + else + raise( + LockedByAnotherUser, + "Page is locked by another user who is working on it! " \ + "Last modification: #{(self.autosave || self.draft || self.head).updated_at.to_fs(:db)}" + ) + end + end + + # Creates or updates the autosave buffer from the given attributes. + # Autosave rows are never associated to the node via node_id -- they + # must never appear in self.pages / the revisions list, which is the + # whole reason autosave exists as a separate, unversioned layer. + def autosave! attributes, current_user + assert_locked_by! current_user + + unless self.autosave + self.autosave = Page.create!(:editor => current_user) + self.save! + end + self.autosave.assign_attributes(attributes) + self.autosave.save! + self.autosave + end + + # Promotes the current autosave into the draft (creating the draft if + # none exists yet) and destroys the autosave afterward. This is what + # the explicit "Save" action does; it never creates a new revision -- + # same as any other in-place draft edit. The new draft is created via + # self.pages.create! rather than by repointing the autosave's own + # node_id, because acts_as_list assigns the revision number at create + # time, scoped to node_id -- a page created with node_id nil and + # reassigned afterward would carry a wrong or missing revision number. + def save_draft! current_user + assert_locked_by! current_user + return unless self.autosave + + if self.draft + self.draft.clone_attributes_from self.autosave + 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.clone_attributes_from self.autosave + empty_page.save! + self.draft = empty_page + self.save! + end + + self.autosave.destroy + self.autosave_id = nil + self.save! + self.draft.reload + end + def find_or_create_draft current_user self.wipe_draft! if draft && self.lock_owner == current_user @@ -129,8 +195,13 @@ class Node < ApplicationRecord # removes a draft and the lock if it is older than a day and still # identical to head def wipe_draft! + return if self.autosave && self.autosave.updated_at > 1.day.ago + unless self.draft + self.autosave&.destroy + self.autosave_id = nil self.unlock! + self.save! return end return unless self.head @@ -224,6 +295,15 @@ class Node < ApplicationRecord self.save end + def assert_locked_by! current_user + return if self.lock_owner == current_user + raise( + LockedByAnotherUser, + "Page is locked by another user who is working on it! " \ + "Last modification: #{(self.autosave || self.draft || self.head).updated_at.to_fs(:db)}" + ) + end + # Creates an empty page and associates it to the given node. This means # freshly created node has an empty draft. A user can create nodes as he # wants to which will not appear on the public page until the author edits diff --git a/db/migrate/20260708095943_add_autosave_id_to_nodes.rb b/db/migrate/20260708095943_add_autosave_id_to_nodes.rb new file mode 100644 index 0000000..eedcc00 --- /dev/null +++ b/db/migrate/20260708095943_add_autosave_id_to_nodes.rb @@ -0,0 +1,5 @@ +class AddAutosaveIdToNodes < ActiveRecord::Migration[8.1] + def change + add_column :nodes, :autosave_id, :integer + end +end diff --git a/test/models/node_test.rb b/test/models/node_test.rb index 514ba3f..2953f8f 100644 --- a/test/models/node_test.rb +++ b/test/models/node_test.rb @@ -98,6 +98,7 @@ class NodeTest < ActiveSupport::TestCase assert_not_nil node.draft assert_nil node.draft.user assert_nil node.head + assert_nil node.autosave end def test_create_new_draft_of_published_page @@ -280,6 +281,101 @@ class NodeTest < ActiveSupport::TestCase node = Node.root.children.create( :slug => "wow" ) assert_nil node.draft.published_at end + + test "lock_for_editing! acquires the lock without creating a draft or autosave" do + node = create_node_with_published_page + + node.lock_for_editing!(@user1) + + assert_equal @user1, node.lock_owner + assert_nil node.draft + assert_nil node.autosave + end + + test "autosave! creates a buffer that never appears among a node's pages, leaving the draft untouched" do + node = create_node_with_draft + node.lock_for_editing!(@user1) + page_count_before = node.pages.count + + node.autosave!({ :title => "in progress" }, @user1) + node.reload + + assert_not_nil node.autosave + assert_nil node.autosave.node_id + assert_equal page_count_before, node.pages.count + assert_not_equal "in progress", node.draft.title + end + + test "save_draft! promotes an autosave into an existing draft without creating a new revision" do + node = create_node_with_draft + node.lock_for_editing!(@user1) + node.autosave!({ :title => "in progress" }, @user1) + page_count_before = node.pages.count + + node.save_draft!(@user1) + node.reload + + assert_nil node.autosave + assert_equal "in progress", node.draft.title + assert_equal page_count_before, node.pages.count + end + + test "save_draft! promotes an autosave into a brand new, correctly-revisioned draft when none exists" do + node = create_node_with_published_page + head_revision = node.head.revision + + node.lock_for_editing!(@user1) + node.autosave!({ :title => "updated version" }, @user1) + node.reload + + assert_nil node.draft + assert_nil node.autosave.node_id + + node.save_draft!(@user1) + node.reload + + assert_not_nil node.draft + assert_equal head_revision + 1, node.draft.revision + assert_equal head_revision, node.head.revision + assert_nil node.autosave + assert_equal 2, node.pages.count + end + + test "autosave!, save_draft!, and lock_for_editing! raise LockedByAnotherUser for a second user" do + node = create_node_with_published_page + node.lock_for_editing!(@user1) + + assert_raise(LockedByAnotherUser) { node.autosave!({ :title => "x" }, @user2) } + assert_raise(LockedByAnotherUser) { node.save_draft!(@user2) } + assert_raise(LockedByAnotherUser) { node.lock_for_editing!(@user2) } + + assert_equal @user1, node.reload.lock_owner + end + + test "wipe_draft! leaves a fresh autosave and its lock untouched" do + node = create_node_with_published_page + node.lock_for_editing!(@user1) + node.autosave!({ :title => "still typing" }, @user1) + + node.wipe_draft! + node.reload + + assert_not_nil node.autosave + assert_equal @user1, node.lock_owner + end + + test "wipe_draft! destroys a stale, orphaned autosave and releases its lock" do + node = create_node_with_published_page + node.lock_for_editing!(@user1) + node.autosave!({ :title => "abandoned mid-session" }, @user1) + node.autosave.update_column(:updated_at, 2.days.ago) + + node.wipe_draft! + node.reload + + assert_nil node.autosave + assert_nil node.lock_owner + end def create_revisions node, count count.times 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/models') 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 bc03601ee5c7acd4ef012ec4a404bd7b76bceaa0 Mon Sep 17 00:00:00 2001 From: erdgeist Date: Wed, 8 Jul 2026 17:32:17 +0200 Subject: Add revert!/discard and rebuild nodes#edit around the new hierarchy revert! discards exactly the topmost non-empty layer -- autosave if present, else draft -- and reveals whatever's beneath it, releasing the lock only once nothing is left to protect. Guards against destroying a brand-new, never-published node's only draft, which would violate the (head | draft) invariant every other method here assumes holds; the view's Destroy/Discard button is gated the same way. nodes#edit now calls lock_for_editing! instead of find_or_create_draft, and always displays autosave || draft || head, resurrecting an abandoned session's unsaved content by default with an explicit flash explaining what's shown and how to get back to the last saved version. The view drops content_for :subnavigation entirely: Show becomes "Unlock + Back", Preview stays a plain link, metadata's own
already replaced the old toggle, and Publish moves off this page for good, per the earlier decision to manage the publish lifecycle entirely from nodes#show. Save Draft and Save + Unlock + Exit appear both above and below the form, given the body field alone runs 600px on desktop. --- app/controllers/nodes_controller.rb | 34 ++++++++++----- config/routes.rb | 1 + test/models/node_test.rb | 87 +++++++++++++++++++++++++++++++++++++ 3 files changed, 112 insertions(+), 10 deletions(-) (limited to 'test/models') diff --git a/app/controllers/nodes_controller.rb b/app/controllers/nodes_controller.rb index 042963b..6fd000b 100644 --- a/app/controllers/nodes_controller.rb +++ b/app/controllers/nodes_controller.rb @@ -12,7 +12,8 @@ class NodesController < ApplicationController :destroy, :publish, :unlock, - :autosave + :autosave, + :revert ] def index @@ -59,16 +60,17 @@ class NodesController < ApplicationController end def edit - begin - @draft = @node.find_or_create_draft( current_user ) - rescue LockedByAnotherUser => e - flash[:error] = e.message - if request.referer - redirect_to request.referer || node_path(@node) - else - redirect_to node_path(@node) - end + @node.lock_for_editing!( current_user ) + @page = @node.autosave || @node.draft || @node.head + + 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." end + rescue LockedByAnotherUser => e + flash[:error] = e.message + redirect_to(request.referer || node_path(@node)) end def update @@ -104,6 +106,18 @@ class NodesController < ApplicationController render plain: "Autosave failed", status: :internal_server_error end + def revert + @node.revert!(current_user) + if @node.draft + redirect_to edit_node_path(@node) + else + redirect_to node_path(@node) + end + rescue LockedByAnotherUser => e + flash[:error] = e.message + redirect_to node_path(@node) + end + def destroy @node.destroy end diff --git a/config/routes.rb b/config/routes.rb index 1569a15..da46e5c 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -41,6 +41,7 @@ Cccms::Application.routes.draw do put :generate_shared_preview put :revoke_shared_preview put :autosave + put :revert end resources :revisions do diff --git a/test/models/node_test.rb b/test/models/node_test.rb index bdf556d..2138c19 100644 --- a/test/models/node_test.rb +++ b/test/models/node_test.rb @@ -379,6 +379,93 @@ class NodeTest < ActiveSupport::TestCase assert_nil node.autosave assert_nil node.lock_owner end + + test "revert! is a safe no-op on a fresh node with only a draft" do + node = create_node_with_draft + node.lock_for_editing!(@user1) + + node.revert!(@user1) + node.reload + + assert_not_nil node.draft + assert_equal @user1, node.lock_owner + end + + test "revert! discards an autosave on a fresh node without touching its only draft" do + node = create_node_with_draft + node.lock_for_editing!(@user1) + node.autosave!({ :title => "typing" }, @user1) + + node.revert!(@user1) + node.reload + + assert_nil node.autosave + assert_not_nil node.draft + assert_equal @user1, node.lock_owner + end + + test "revert! does nothing when a published node has no draft or autosave" do + node = create_node_with_published_page + node.lock_for_editing!(@user1) + + node.revert!(@user1) + node.reload + + assert_not_nil node.head + assert_nil node.draft + end + + test "revert! discards a fresh autosave and releases the lock when no draft exists" do + node = create_node_with_published_page + node.lock_for_editing!(@user1) + node.autosave!({ :title => "in progress" }, @user1) + + node.revert!(@user1) + node.reload + + assert_nil node.autosave + assert_nil node.draft + assert_nil node.lock_owner + end + + test "revert! destroys an existing draft and releases the lock" do + node = create_node_with_published_page + head_title = node.head.title + node.lock_for_editing!(@user1) + node.autosave!({ :title => "second version" }, @user1) + node.save_draft!(@user1) + + node.revert!(@user1) + node.reload + + assert_nil node.draft + assert_equal head_title, node.head.title + assert_nil node.lock_owner + end + + test "revert! discards only the autosave when a draft survives beneath it" do + node = create_node_with_published_page + node.lock_for_editing!(@user1) + node.autosave!({ :title => "second version" }, @user1) + node.save_draft!(@user1) + node.autosave!({ :title => "third version, still typing" }, @user1) + + node.revert!(@user1) + node.reload + + assert_nil node.autosave + assert_not_nil node.draft + assert_equal "second version", node.draft.title + assert_equal @user1, node.lock_owner + end + + test "revert! raises LockedByAnotherUser for a non-owner" do + node = create_node_with_published_page + node.lock_for_editing!(@user1) + + assert_raise(LockedByAnotherUser) { node.revert!(@user2) } + assert_equal @user1, node.reload.lock_owner + end def create_revisions node, count count.times 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/models') 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 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/models')

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 fd98a2d3053279bd4a848faa52a3ac676db10cbb Mon Sep 17 00:00:00 2001 From: erdgeist Date: Fri, 10 Jul 2026 03:59:34 +0200 Subject: Stop diff output from corrupting the document on structural tag changes sdiff aligned an inserted paragraph break correctly, but the renderer wrapped the raw

/

tokens in regardless -- invalid nesting that browsers silently repair by dropping the unmatched closing tag, leaving everything downstream rendered inside an with nothing left to close it. Same failure mode as the retired cacycle_diff.js, different cause. Tag tokens now render as a plain-text pilcrow with a tooltip naming the actual tag, never as literal markup inside /. Applies to both inline and side-by-side. Side-by-side's new panel flattening a real paragraph break into a marker, rather than showing its own true structure, is a known, accepted trade-off for now -- not the same problem, and not fixed here. --- lib/html_word_diff.rb | 36 +++++++++++++++++++++++++++++------- test/models/page_test.rb | 18 ++++++++++++++++++ 2 files changed, 47 insertions(+), 7 deletions(-) (limited to 'test/models') diff --git a/lib/html_word_diff.rb b/lib/html_word_diff.rb index 1f28cf5..aa0cb86 100644 --- a/lib/html_word_diff.rb +++ b/lib/html_word_diff.rb @@ -1,4 +1,5 @@ require 'diff/lcs' +require 'cgi' module HtmlWordDiff TOKEN_REGEXP = /<[^>]*>|[[:space:]]+|[[:alnum:]]+|[^[:space:][:alnum:]<>]/ @@ -10,11 +11,12 @@ module HtmlWordDiff when '=' html << new_token when '-' - html << "#{old_token}" + html << wrap_removed(old_token) when '+' - html << "#{new_token}" + html << wrap_inserted(new_token) when '!' - html << "#{old_token}#{new_token}" + html << wrap_removed(old_token) + html << wrap_inserted(new_token) end end html @@ -29,12 +31,12 @@ module HtmlWordDiff old_out << old_token new_out << new_token when '-' - old_out << "#{old_token}" + old_out << wrap_removed(old_token) when '+' - new_out << "#{new_token}" + new_out << wrap_inserted(new_token) when '!' - old_out << "#{old_token}" - new_out << "#{new_token}" + old_out << wrap_removed(old_token) + new_out << wrap_inserted(new_token) end end [old_out, new_out] @@ -49,4 +51,24 @@ module HtmlWordDiff def self.tokenize(html) html.to_s.scan(TOKEN_REGEXP) end + + def self.tag_token?(token) + token.to_s.match?(/\A<[^>]*>\z/) + end + + def self.wrap_removed(token) + if tag_token?(token) + %(\u00b6) + else + "#{token}" + end + end + + def self.wrap_inserted(token) + if tag_token?(token) + %(\u00b6) + else + "#{token}" + end + end end diff --git a/test/models/page_test.rb b/test/models/page_test.rb index ac5691a..3868a53 100644 --- a/test/models/page_test.rb +++ b/test/models/page_test.rb @@ -210,4 +210,22 @@ class PageTest < ActiveSupport::TestCase assert_match "Old", old_html assert_match "New", new_html end + + test "diff_against handles an inserted paragraph split without corrupting the document" do + n = Node.root.children.create! :slug => "paragraph_split_test" + d = n.find_or_create_draft @user1 + d.body = "

Der Vortragsraum ist ab 19 Uhr geöffnet, der Zugang erfolgt über den Hinterhof.

" + d.save! + n.publish_draft! + + d2 = n.find_or_create_draft @user1 + d2.body = "

Der Vortragsraum ist ab 19 Uhr geöffnet,

\n

der Zugang erfolgt über den Hinterhof.

" + d2.save! + + diff = d2.diff_against(n.head) + fragment = Nokogiri::HTML::DocumentFragment.parse(diff[:body]) + + assert_equal 2, fragment.css('ins.diff_structural').length + assert_match "der Zugang erfolgt über den Hinterhof.", fragment.text + 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/models') 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 c9401e45433ea45b46f9a8faf1e7e537e4683244 Mon Sep 17 00:00:00 2001 From: erdgeist Date: Fri, 10 Jul 2026 18:07:15 +0200 Subject: Fix rrule/url column overflow on events#index Long RRULEs previously overflowed their column with no wrap point; now escaped and rendered with a after each semicolon, so a long rule wraps at a clause boundary instead of running off the table. Deliberately not truncated -- a cut-off RRULE's trailing clause (BYDAY, BYMONTH, etc.) is usually the most specific part. The url column is now a real link, truncated with an ellipsis at a fixed width -- deliberately no tooltip, since hovering a real link already shows the full address in the browser's own status bar. rrule_with_break_opportunities splits on the raw string's own semicolons before escaping each piece, not after -- escaping first and searching the result for semicolons also matches the ones inside </> entities, corrupting anything containing a literal < or >. --- app/helpers/events_helper.rb | 11 +++++++++++ app/views/events/index.html.erb | 8 ++++++-- public/stylesheets/admin.css | 15 +++++++++++++++ test/models/helpers/events_helper_test.rb | 21 +++++++++++++++++++++ 4 files changed, 53 insertions(+), 2 deletions(-) (limited to 'test/models') diff --git a/app/helpers/events_helper.rb b/app/helpers/events_helper.rb index 8a9a878..5e84f53 100644 --- a/app/helpers/events_helper.rb +++ b/app/helpers/events_helper.rb @@ -1,2 +1,13 @@ +require 'cgi' + module EventsHelper + # Insert a zero-width break opportunity after each semicolon, so a long + # RRULE can wrap at a clause boundary instead of overflowing its column. + # Deliberately , not ellipsis -- unlike a URL, an RRULE's trailing + # characters (BYDAY, BYMONTH, etc.) are usually the most specific part, + # and truncating them would hide exactly the wrong end of the string. + def rrule_with_break_opportunities(rrule) + return "" if rrule.blank? + raw(rrule.split(';', -1).map { |part| CGI.escapeHTML(part) }.join(';')) + end end diff --git a/app/views/events/index.html.erb b/app/views/events/index.html.erb index 8127e3a..3d953d5 100644 --- a/app/views/events/index.html.erb +++ b/app/views/events/index.html.erb @@ -22,9 +22,13 @@ <%= link_to event.display_title, event %> <%=h event.start_time %> <%=h event.end_time %> - <%=h event.rrule %> + <%= rrule_with_break_opportunities(event.rrule) %> <%=h event.allday %> - <%=h event.url %> + + <% if event.url.present? %> + <%= link_to event.url, event.url %> + <% end %> + <%= event.node ? link_to(event.node_id, node_path(event.node)) : '' %> <%= link_to 'edit', edit_event_path(event) %> diff --git a/public/stylesheets/admin.css b/public/stylesheets/admin.css index 1a81728..4723f50 100644 --- a/public/stylesheets/admin.css +++ b/public/stylesheets/admin.css @@ -519,6 +519,21 @@ table.revisions_table tr:hover { background-color: #f1f1f1; } +.events_table .rrule_text { + display: inline-block; + max-width: 300px; + overflow-wrap: break-word; +} + +.events_table .truncate { + display: inline-block; + max-width: 200px; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; + vertical-align: bottom; +} + #diffview del { background: #ffd7d5; color: #82071e; diff --git a/test/models/helpers/events_helper_test.rb b/test/models/helpers/events_helper_test.rb index 2e7567e..0486b3e 100644 --- a/test/models/helpers/events_helper_test.rb +++ b/test/models/helpers/events_helper_test.rb @@ -1,4 +1,25 @@ require 'test_helper' class EventsHelperTest < ActionView::TestCase + test "rrule_with_break_opportunities inserts a break opportunity after each semicolon" do + result = rrule_with_break_opportunities("FREQ=MONTHLY;BYMONTH=1,2,3;BYDAY=-1TH") + assert_equal "FREQ=MONTHLY;BYMONTH=1,2,3;BYDAY=-1TH", result + assert result.html_safe? + end + + test "rrule_with_break_opportunities escapes HTML-significant characters" do + result = rrule_with_break_opportunities("FREQ=WEEKLY;BYDAY= + - - + + + +