diff options
| author | erdgeist <erdgeist@erdgeist.org> | 2026-07-31 18:55:35 +0200 |
|---|---|---|
| committer | erdgeist <erdgeist@erdgeist.org> | 2026-07-31 18:55:35 +0200 |
| commit | 683526b38442a9873d83a542b5cba1d15efc14d5 (patch) | |
| tree | 5cdf7d762d829d328e3c6437318d20af72bc65bf | |
| parent | 464af1625349d557f688da9f845471ef8b80a5f9 (diff) | |
Bind aggregates over scoped tags to their subtree
CccConventions::TAG_SCOPES maps "update" and "pressemitteilung" to /updates
and "disclosure" to /disclosure. Page.aggregate applies the mapping, so an
aggregate over one of those tags is bounded by its subtree regardless of what
the shortcode says. Tags stay unrestricted; positions are publish-gated.
Both RSS actions call Page.aggregate rather than repeating its tag join.
Empty feeds no longer crash: Atom falls back to the current time for the
required <updated>, and the optional dc:date is omitted.
| -rw-r--r-- | app/controllers/rss_controller.rb | 34 | ||||
| -rw-r--r-- | app/helpers/content_helper.rb | 2 | ||||
| -rw-r--r-- | app/models/page.rb | 7 | ||||
| -rw-r--r-- | app/views/rss/updates.rdf.builder | 2 | ||||
| -rw-r--r-- | app/views/rss/updates.xml.builder | 2 | ||||
| -rw-r--r-- | lib/ccc_conventions.rb | 10 | ||||
| -rw-r--r-- | test/controllers/content_controller_test.rb | 21 | ||||
| -rw-r--r-- | test/controllers/rss_controller_test.rb | 20 | ||||
| -rw-r--r-- | test/models/page_test.rb | 20 |
9 files changed, 80 insertions, 38 deletions
diff --git a/app/controllers/rss_controller.rb b/app/controllers/rss_controller.rb index 655ccba4..c2149d90 100644 --- a/app/controllers/rss_controller.rb +++ b/app/controllers/rss_controller.rb | |||
| @@ -1,19 +1,12 @@ | |||
| 1 | class RssController < ApplicationController | 1 | class RssController < ApplicationController |
| 2 | 2 | ||
| 3 | before_action :get_host | 3 | before_action :get_host |
| 4 | 4 | ||
| 5 | def updates | 5 | def updates |
| 6 | expires_in 31.minutes, :public => true | 6 | expires_in 31.minutes, :public => true |
| 7 | |||
| 8 | I18n.locale = I18n.default_locale | 7 | I18n.locale = I18n.default_locale |
| 9 | 8 | ||
| 10 | @items = Page.heads | 9 | @items = feed_items("update") |
| 11 | .joins("JOIN taggings ON taggings.taggable_id = pages.id | ||
| 12 | AND taggings.taggable_type = 'Page' | ||
| 13 | AND taggings.context = 'tags'") | ||
| 14 | .joins("JOIN tags ON tags.id = taggings.tag_id") | ||
| 15 | .where("LOWER(tags.name) = ?", "update") | ||
| 16 | .order("published_at DESC").limit(20) | ||
| 17 | 10 | ||
| 18 | respond_to do |format| | 11 | respond_to do |format| |
| 19 | format.xml {} | 12 | format.xml {} |
| @@ -23,16 +16,10 @@ class RssController < ApplicationController | |||
| 23 | 16 | ||
| 24 | def tag_updates | 17 | def tag_updates |
| 25 | expires_in 31.minutes, :public => true | 18 | expires_in 31.minutes, :public => true |
| 26 | |||
| 27 | I18n.locale = I18n.default_locale | 19 | I18n.locale = I18n.default_locale |
| 28 | @tag = params[:tag] | 20 | |
| 29 | @items = Page.heads | 21 | @tag = params[:tag] |
| 30 | .joins("JOIN taggings ON taggings.taggable_id = pages.id | 22 | @items = feed_items(@tag) |
| 31 | AND taggings.taggable_type = 'Page' | ||
| 32 | AND taggings.context = 'tags'") | ||
| 33 | .joins("JOIN tags ON tags.id = taggings.tag_id") | ||
| 34 | .where("LOWER(tags.name) = ?", @tag.downcase) | ||
| 35 | .order("published_at DESC").limit(20) | ||
| 36 | 23 | ||
| 37 | respond_to do |format| | 24 | respond_to do |format| |
| 38 | format.xml {} | 25 | format.xml {} |
| @@ -40,7 +27,14 @@ class RssController < ApplicationController | |||
| 40 | end | 27 | end |
| 41 | 28 | ||
| 42 | protected | 29 | protected |
| 43 | 30 | ||
| 31 | def feed_items tag | ||
| 32 | Page.aggregate(:tags => tag.to_s.downcase, | ||
| 33 | :limit => 20, | ||
| 34 | :order_by => "published_at", | ||
| 35 | :order_direction => "DESC") | ||
| 36 | end | ||
| 37 | |||
| 44 | def get_host | 38 | def get_host |
| 45 | @host = request.protocol + request.host_with_port | 39 | @host = request.protocol + request.host_with_port |
| 46 | end | 40 | end |
diff --git a/app/helpers/content_helper.rb b/app/helpers/content_helper.rb index 57f8c960..9d52c110 100644 --- a/app/helpers/content_helper.rb +++ b/app/helpers/content_helper.rb | |||
| @@ -123,7 +123,7 @@ module ContentHelper | |||
| 123 | sanitize(content, :attributes => cccms_attributes) | 123 | sanitize(content, :attributes => cccms_attributes) |
| 124 | end | 124 | end |
| 125 | 125 | ||
| 126 | rescue | 126 | rescue => e |
| 127 | Rails.logger.error("aggregate shortcode failed on page #{@page&.id}: #{e.class}: #{e.message}") | 127 | Rails.logger.error("aggregate shortcode failed on page #{@page&.id}: #{e.class}: #{e.message}") |
| 128 | fallback = content.sub(/\[aggregate[^\]]*\]/, "") | 128 | fallback = content.sub(/\[aggregate[^\]]*\]/, "") |
| 129 | fallback = sanitize(fallback, :attributes => cccms_attributes) | 129 | fallback = sanitize(fallback, :attributes => cccms_attributes) |
diff --git a/app/models/page.rb b/app/models/page.rb index cf003f74..a66527da 100644 --- a/app/models/page.rb +++ b/app/models/page.rb | |||
| @@ -61,7 +61,7 @@ class Page < ApplicationRecord | |||
| 61 | unless options[:tags].blank? | 61 | unless options[:tags].blank? |
| 62 | tag_names = options[:tags].gsub(/\s/, ",").split(",").map(&:strip).map(&:downcase).uniq.reject(&:blank?) | 62 | tag_names = options[:tags].gsub(/\s/, ",").split(",").map(&:strip).map(&:downcase).uniq.reject(&:blank?) |
| 63 | 63 | ||
| 64 | unless tag_names.empty? | 64 | unless tag_names.empty? |
| 65 | scope = scope | 65 | scope = scope |
| 66 | .joins("JOIN taggings ON taggings.taggable_id = pages.id | 66 | .joins("JOIN taggings ON taggings.taggable_id = pages.id |
| 67 | AND taggings.taggable_type = 'Page' | 67 | AND taggings.taggable_type = 'Page' |
| @@ -71,6 +71,11 @@ class Page < ApplicationRecord | |||
| 71 | .group("pages.id") | 71 | .group("pages.id") |
| 72 | .having("COUNT(DISTINCT tags.id) = ?", tag_names.length) | 72 | .having("COUNT(DISTINCT tags.id) = ?", tag_names.length) |
| 73 | end | 73 | end |
| 74 | |||
| 75 | CccConventions::TAG_SCOPES.values_at(*tag_names).compact.uniq.each do |root| | ||
| 76 | scope = scope.where("nodes.unique_name = ? OR nodes.unique_name LIKE ?", | ||
| 77 | root, "#{root}/%") | ||
| 78 | end | ||
| 74 | end | 79 | end |
| 75 | 80 | ||
| 76 | if options[:node] && options[:children] == "direct" | 81 | if options[:node] && options[:children] == "direct" |
diff --git a/app/views/rss/updates.rdf.builder b/app/views/rss/updates.rdf.builder index b02d34ff..b4fecdb0 100644 --- a/app/views/rss/updates.rdf.builder +++ b/app/views/rss/updates.rdf.builder | |||
| @@ -6,7 +6,7 @@ xml.tag!("rdf:RDF", "xmlns:rdf" => "http://www.w3.org/1999/02/22-rdf-syntax-ns#" | |||
| 6 | xml.title("Chaos Computer Club: Updates") | 6 | xml.title("Chaos Computer Club: Updates") |
| 7 | xml.link("https://www.ccc.de") | 7 | xml.link("https://www.ccc.de") |
| 8 | xml.description("Kabelsalat ist gesund.") | 8 | xml.description("Kabelsalat ist gesund.") |
| 9 | xml.tag!("dc:date", @items.first.published_at.xmlschema) | 9 | xml.tag!("dc:date", @items.first&.published_at&.xmlschema) if @items.any? |
| 10 | end | 10 | end |
| 11 | 11 | ||
| 12 | xml.image( "rdf:about" => "https://www.ccc.de/images/chaosknoten.gif") do | 12 | xml.image( "rdf:about" => "https://www.ccc.de/images/chaosknoten.gif") do |
diff --git a/app/views/rss/updates.xml.builder b/app/views/rss/updates.xml.builder index 27845c4c..f261de89 100644 --- a/app/views/rss/updates.xml.builder +++ b/app/views/rss/updates.xml.builder | |||
| @@ -4,7 +4,7 @@ xml.feed(:xmlns => "http://www.w3.org/2005/Atom", "xml:base" => @host) do | |||
| 4 | xml.title("Chaos Computer Club Updates") | 4 | xml.title("Chaos Computer Club Updates") |
| 5 | xml.link(:href => "https://www.ccc.de/") | 5 | xml.link(:href => "https://www.ccc.de/") |
| 6 | xml.link(:rel => "self", :href => "#{@host}/rss/updates.xml") | 6 | xml.link(:rel => "self", :href => "#{@host}/rss/updates.xml") |
| 7 | xml.updated(@items.first.published_at.xmlschema) | 7 | xml.updated((@items.first&.published_at || Time.now).xmlschema) |
| 8 | xml.author do | 8 | xml.author do |
| 9 | xml.name("Chaos Computer Club e.V.") | 9 | xml.name("Chaos Computer Club e.V.") |
| 10 | end | 10 | end |
diff --git a/lib/ccc_conventions.rb b/lib/ccc_conventions.rb index b840dee2..52549c9e 100644 --- a/lib/ccc_conventions.rb +++ b/lib/ccc_conventions.rb | |||
| @@ -52,4 +52,14 @@ module CccConventions | |||
| 52 | hint: "Automatically created under the Chaostreffs overview page, gets tag \"chaostreff-detail\", and uses the chapter detail template." | 52 | hint: "Automatically created under the Chaostreffs overview page, gets tag \"chaostreff-detail\", and uses the chapter detail template." |
| 53 | } | 53 | } |
| 54 | }.freeze | 54 | }.freeze |
| 55 | |||
| 56 | # Tags whose aggregates are forced into a subtree. Editors may apply any | ||
| 57 | # tag anywhere, but a page tagged "update" outside /updates will never | ||
| 58 | # appear in the front-page widget or the RSS feed, because every aggregate | ||
| 59 | # over that tag is scoped here rather than in the shortcode. | ||
| 60 | TAG_SCOPES = { | ||
| 61 | "update" => "updates", | ||
| 62 | "pressemitteilung" => "updates", | ||
| 63 | "disclosure" => "disclosure" | ||
| 64 | }.freeze | ||
| 55 | end | 65 | end |
diff --git a/test/controllers/content_controller_test.rb b/test/controllers/content_controller_test.rb index 5bb02f3d..39fe276b 100644 --- a/test/controllers/content_controller_test.rb +++ b/test/controllers/content_controller_test.rb | |||
| @@ -239,17 +239,16 @@ class ContentControllerTest < ActionController::TestCase | |||
| 239 | end | 239 | end |
| 240 | 240 | ||
| 241 | def fill_pages_with_content | 241 | def fill_pages_with_content |
| 242 | d1 = find_or_create_draft(@first_child, @user1) | 242 | updates = Node.root.children.find_by(:slug => "updates") || |
| 243 | d1.title = "one" | 243 | Node.root.children.create!(:slug => "updates") |
| 244 | d1.tag_list = "update" | ||
| 245 | d1.save | ||
| 246 | @first_child.publish_draft! | ||
| 247 | 244 | ||
| 248 | d2 = find_or_create_draft(@second_child, @user1) | 245 | [["one", "aggregated_one"], ["two", "aggregated_two"]].each do |title, slug| |
| 249 | d2.title = "two" | 246 | node = updates.children.create!(:slug => slug) |
| 250 | d2.tag_list = "update" | 247 | draft = find_or_create_draft(node, @user1) |
| 251 | d2.save | 248 | draft.title = title |
| 252 | @second_child.publish_draft! | 249 | draft.tag_list = "update" |
| 250 | draft.save | ||
| 251 | node.publish_draft! | ||
| 252 | end | ||
| 253 | end | 253 | end |
| 254 | |||
| 255 | end | 254 | end |
diff --git a/test/controllers/rss_controller_test.rb b/test/controllers/rss_controller_test.rb index 3f4b4fbf..cf50903a 100644 --- a/test/controllers/rss_controller_test.rb +++ b/test/controllers/rss_controller_test.rb | |||
| @@ -5,7 +5,9 @@ class RssControllerTest < ActionController::TestCase | |||
| 5 | def setup | 5 | def setup |
| 6 | @user = User.create :login => 'rsstest', :email => 'rsstest@example.com', | 6 | @user = User.create :login => 'rsstest', :email => 'rsstest@example.com', |
| 7 | :password => 'foobar', :password_confirmation => 'foobar' | 7 | :password => 'foobar', :password_confirmation => 'foobar' |
| 8 | @node = Node.root.children.create! :slug => 'rss_test_node' | 8 | updates = Node.root.children.find_by(:slug => "updates") || |
| 9 | Node.root.children.create!(:slug => "updates") | ||
| 10 | @node = updates.children.create! :slug => 'rss_test_node' | ||
| 9 | draft = find_or_create_draft(@node, @user) | 11 | draft = find_or_create_draft(@node, @user) |
| 10 | draft.title = "RSS Update Article" | 12 | draft.title = "RSS Update Article" |
| 11 | draft.tag_list = "update" | 13 | draft.tag_list = "update" |
| @@ -31,4 +33,20 @@ class RssControllerTest < ActionController::TestCase | |||
| 31 | assert assigns(:items).length <= 20 | 33 | assert assigns(:items).length <= 20 |
| 32 | end | 34 | end |
| 33 | 35 | ||
| 36 | test "the update feed excludes a page tagged update outside /updates" do | ||
| 37 | updates = Node.root.children.find_by(:slug => "updates") | ||
| 38 | inside = updates.children.create!(:slug => "feed-inside") | ||
| 39 | outside = Node.root.children.create!(:slug => "feed-outside") | ||
| 40 | |||
| 41 | [inside, outside].each do |node| | ||
| 42 | node.reload.draft.update!(:title => node.slug, :tag_list => "update") | ||
| 43 | node.publish_draft! | ||
| 44 | end | ||
| 45 | |||
| 46 | get :updates, params: { :format => :xml } | ||
| 47 | |||
| 48 | assert_response :success | ||
| 49 | assert_includes @response.body, "feed-inside" | ||
| 50 | assert_not_includes @response.body, "feed-outside" | ||
| 51 | end | ||
| 34 | end | 52 | end |
diff --git a/test/models/page_test.rb b/test/models/page_test.rb index 98a00d21..395b6315 100644 --- a/test/models/page_test.rb +++ b/test/models/page_test.rb | |||
| @@ -9,8 +9,9 @@ class PageTest < ActiveSupport::TestCase | |||
| 9 | 9 | ||
| 10 | def test_aggregation | 10 | def test_aggregation |
| 11 | # Create two nodes and move them beneath the root node | 11 | # Create two nodes and move them beneath the root node |
| 12 | n1 = Node.root.children.create! :slug => "one" | 12 | updates = Node.root.children.create! :slug => "updates" |
| 13 | n2 = Node.root.children.create! :slug => "two" | 13 | n1 = updates.children.create! :slug => "one" |
| 14 | n2 = updates.children.create! :slug => "two" | ||
| 14 | 15 | ||
| 15 | # get the drafts and assign a user to it | 16 | # get the drafts and assign a user to it |
| 16 | assert_not_nil d1 = find_or_create_draft(n1, @user1) | 17 | assert_not_nil d1 = find_or_create_draft(n1, @user1) |
| @@ -409,4 +410,19 @@ class PageTest < ActiveSupport::TestCase | |||
| 409 | page.reload | 410 | page.reload |
| 410 | assert page.update(:abstract => "still saveable") | 411 | assert page.update(:abstract => "still saveable") |
| 411 | end | 412 | end |
| 413 | |||
| 414 | test "an aggregate over a scoped tag ignores pages outside that subtree" do | ||
| 415 | updates = Node.root.children.create!(:slug => "updates") | ||
| 416 | inside = updates.children.create!(:slug => "inside-post") | ||
| 417 | outside = Node.root.children.create!(:slug => "outside-post") | ||
| 418 | |||
| 419 | [inside, outside].each do |node| | ||
| 420 | node.reload.draft.update!(:title => node.slug, :tag_list => "update") | ||
| 421 | node.publish_draft! | ||
| 422 | end | ||
| 423 | |||
| 424 | names = Page.aggregate({ :tags => "update" }).map { |p| p.node.unique_name } | ||
| 425 | assert_includes names, "updates/inside-post" | ||
| 426 | assert_not_includes names, "outside-post" | ||
| 427 | end | ||
| 412 | end | 428 | end |
