Skip to content

refactor(rss): declare the comments feed from rss_menu's own e_rss.php - #6458

Open
e107help[bot] wants to merge 3 commits into
masterfrom
e107help/5880
Open

e107help[bot] wants to merge 3 commits into
masterfrom
e107help/5880

Conversation

@e107help

@e107help e107help Bot commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Why

Fixes #5880.

Every RSS feed a plugin serves is resolved from what that plugin declares: its
e_rss.php returns the feed in config(), builds it in data(), and names any
pre-v0.7.6 numeric key it still answers to in legacy(). The comments feed was
the exception. rss_menu served it inline from a
case 'comments': arm
in rssCreate, held its legacy key 5 in
rss_resolver.php,
and seeded its row from a
hardcoded block in the admin importer.
Three hardcodes for one feed.

Commenting is core rather than a plugin, so there is no e107_plugins/comments
to put the addon in. Deltik settled the location: it lives in rss_menu, which
already serves every other feed, is installed wherever this feed is reachable,
and needs no new folder and no new plugin registration.

What Changed

  • New e107_plugins/rss_menu/e_rss.php, class rss_menu_rss. legacy() returns
    array(5 => 'comments'), config() returns the feed the importer used to
    hardcode, and data() is the old commentItems() with the five private
    methods behind it moved across.
  • rss.php loses the case 'comments': arm and those five methods. The switch
    that remains carries only the three 0.7 content types that never became
    plugins.
  • rss_addons scans this plugin's own folder as well as the plugins the
    e_rss_list pref names. Only
    e_plugin::buildAddonPrefLists()
    writes that pref, and nothing runs it on a core upgrade, so the feed has to
    resolve before an admin has run the plugin update that would. legacyKeys()
    loops the same list instead of e107::getAddonConfig(), and the include both
    need is one private method, because a v1 addon declares its feeds by assigning
    to $eplug_rss_feed at include time.
  • rss_feed_resolver declares nothing itself. Ownership of a legacy numeric key
    now holds when the row does not name a different plugin, which covers a row
    whose rss_path is the feed's own key and a row older than that column.
  • The importer's duplicate check drops rss_path. rss.php serves a feed by url
    and topic id alone, so a row already holding those is this feed however its
    path is spelled, and a site that has the comments feed is not offered it again
    after the move.
  • Item links come from SITEURL rather than from
    $_SERVER['HTTP_HOST'],
    so the feed honours the site's URL configuration instead of whichever name the
    request arrived under.
  • rss_menu_setup::upgrade_post() points a legacy comments row at the plugin
    that now serves it, so the row stops naming a folder that does not exist and
    the feed stops paying for the legacy lookup on every request. Resolution does
    not depend on it; it retires a fallback rather than enabling anything.
  • rss_menu goes to 1.4 in plugin.xml, and default_install.xml gains
    rss_menu in e_rss_list so a fresh install matches what a plugin scan would
    write.

Two defects the move surfaced are fixed in commits of their own, so they can be
read, or dropped, separately:

  • The feed read comment_author, a column the 2.0 schema
    split
    into comment_author_id and comment_author_name, so the author was always
    empty and no item ever carried a dc:creator.
  • An item's author was the one field the writer emitted raw, in all three output
    modes and in the Atom entry. Every other field goes through toRss(). That was
    dead for the comments feed while its author was empty, but the news feed has
    been handing user_name over unescaped
    (rss.php#L745),
    and one commenter called Tom & Jerry makes the whole document malformed XML,
    which a conforming reader rejects in one piece.

How It Was Tested

The whole unit suite in the project's Docker harness on PHP 8.5 with MariaDB
10.11: 2737 tests, green. New coverage:

  • e107_tests/tests/unit/plugins/rss_menu/e_rssTest.php seeds a published,
    unrestricted news item and a comment on it, then asserts that the item link
    comes from SITEURL with $_SERVER['HTTP_HOST'] set to a host the site is not
    configured for, that the row carries its date under datestamp (the key the
    feed mapper reads), and that the author is named.
  • RssAddonsTest::testLegacyKeysCarryTheCommentsFeed() holds the scan to
    covering this plugin's own folder, which is what an existing site depends on.
  • RssFeedResolverTest gains the two literal-numeric-row cases, rss_path = 'comments' and no path at all, plus a case proving the resolver resolves
    nothing when no addon declares a key. The first of those was verified red
    against the unmodified rss_resolver.php and green with it.
  • rss_setupTest seeds three rows and holds the upgrade routine to rewriting the
    two that are the comments feed and leaving the third alone.
  • RssCommentsFeedCest gains an author called Tom & Jerry, and asserts the
    feed carries the escaped form and not the raw one. Verified red against the
    previous commit's rss.php.

RssCommentsFeedCest and CommentFeedParentClassCest are the backwards
compatibility proof, and every assertion in them predates this branch. They seed
a legacy row with rss_path => 'comments' and fetch the feed over HTTP; all of
them pass. I then put a site's e_rss_list pref back to
array('news' => 'news') by hand, which is the shape an upgraded site has, and
ran them again: they pass there too. That is the case the folder scan exists for.

Backwards Compatibility

The rendered feed XML is unchanged apart from three deliberate differences: item
links now honour the site's URL configuration rather than the request host, each
item carries a dc:creator for the author that the broken column read had been
swallowing, and an author name containing &, < or > is escaped in every
feed rather than emitted raw.

No data migration is required for the feed to keep working. A legacy row keeps
resolving under both comments and the old numeric 5, whether its rss_path
says comments, says nothing, or names the plugin, and an existing site needs no
plugin rescan, because the scan covers this plugin's folder whatever the
e_rss_list pref holds. The upgrade routine rewrites rss_path on a comments
row that still says comments or nothing, which is tidying rather than repair.

One shape does change meaning: a row whose rss_path names a different
plugin now serves that plugin's feed under the comments row's name, where before
the inline arm overrode it. The field is
readonly in the admin UI
and no core version wrote that combination, so it takes a hand-edited row to
reach.

rssCreate, rss_addons and rss_feed_resolver gain no public method and lose
none. rss_menu_setup gains upgrade_post(), which the plugin upgrade path
calls by convention.

AI Model

Claude Opus 5 (claude-opus-5), as e107help.

Checklist

  • One issue per PR: the diff is scoped to this change only
  • Commit messages explain why, not just what
  • New or changed behavior has test coverage (or explain why not)
  • No unrelated reformatting, renames, or import reordering

The comments feed was the one feed rss_menu served inline, from a switch
arm in rss.php, while the resolver held its pre-0.7.6 numeric key 5 and
the admin importer seeded its row from a hardcoded block. Every other
feed is resolved from what the plugin serving it declares. Commenting is
core rather than a plugin, so the feed is declared by rss_menu, which
serves every other feed and is installed wherever this feed is reachable.

Two things had to widen for existing sites to keep working.

The addon scan now reads this plugin's own folder as well as the plugins
the e_rss_list pref names. Only e_plugin::buildAddonPrefLists() writes
that pref, and nothing runs it on a core upgrade, so a feed that ships
with core cannot wait for a plugin scan before it resolves.

Ownership of a legacy numeric key now also matches a row whose rss_path
is the feed's own key, which is what a row written before the feed became
an addon holds. Without that, a request for key 5 would find the row and
then hand rssCreate the number, which serves nothing.

The importer's duplicate check drops rss_path for the same reason: rss.php
serves a feed by url and topic id alone, so a row already holding those is
this feed however its path is spelled, and a site that has the feed is not
offered it again after the move.

Item links come from SITEURL rather than from $_SERVER['HTTP_HOST'], so
the feed honours the site's URL configuration instead of whichever name
the request happened to arrive under.

Refs #5880
The feed read comment_author, a column the 2.0 schema split into
comment_author_id and comment_author_name, and then stripped the "id."
prefix that column used to carry. On a v2 database that read has always
been empty, so every item's author fell away and buildRss emitted no
dc:creator element for the feed at all.

Read comment_author_name, which is what comment::enter_comment() writes.

Refs #5880
Every other item field in the three output modes goes through toRss().
The author did not, in any of them. The comments feed supplied an empty
author until the previous commit, so the sink was dead for it, but the
news feed has been handing user_name over raw since the mapper existed,
and the comment handler writes USERNAME verbatim while toDB leaves an
ampersand alone.

One commenter called Tom & Jerry therefore makes the whole document
malformed XML, and a conforming reader rejects a feed in one piece rather
than skipping the item it choked on.

Refs #5880
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Move the comments RSS feed into an e_rss.php addon

1 participant