Add site-wide RSS and Atom feeds#74
Open
samdark wants to merge 1 commit into
Open
Conversation
📝 WalkthroughWalkthroughThis PR adds site-wide Atom and RSS feed generation ( ChangesSite-wide Feed Generation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/Unit/Build/FeedGeneratorTest.php (1)
277-330: ⚡ Quick winAdd a focused unit test for
writeSiteAtomFile()/writeSiteRssFile()paths.This new test validates in-memory site feed generation well, but the new site-level file-writing methods would be safer with a direct unit test (mirroring
testFeedFilesCanBeWrittenDirectly()), so regressions are isolated without relying only on command-level integration.🧪 Suggested test addition
+ public function testSiteFeedFilesCanBeWrittenDirectly(): void + { + $generator = new FeedGenerator(new ContentProcessorPipeline(new MarkdownProcessor(new MarkdownRenderer()))); + $collections = ['blog' => $this->collection]; + $entries = $this->createEntries(); + $atomPath = sys_get_temp_dir() . '/yiipress-site-feed-atom-' . uniqid() . '.xml'; + $rssPath = sys_get_temp_dir() . '/yiipress-site-feed-rss-' . uniqid() . '.xml'; + + try { + $generator->writeSiteAtomFile($atomPath, $this->siteConfig, $collections, $entries); + $generator->writeSiteRssFile($rssPath, $this->siteConfig, $collections, $entries); + assertFileExists($atomPath); + assertFileExists($rssPath); + } finally { + if (is_file($atomPath)) { + unlink($atomPath); + } + if (is_file($rssPath)) { + unlink($rssPath); + } + } + }As per coding guidelines,
**/*Test.php: For each piece of code add a test using phpunit.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Unit/Build/FeedGeneratorTest.php` around lines 277 - 330, Add a focused unit test that exercises FeedGenerator::writeSiteAtomFile() and FeedGenerator::writeSiteRssFile() directly (similar to the existing testFeedFilesCanBeWrittenDirectly pattern) rather than only generating feeds in-memory: reuse the setup from testSiteFeedsUseSiteMetadataAndEntryCollections (create the same collections and entries and instantiate FeedGenerator with ContentProcessorPipeline/MarkdownProcessor), create two temporary file paths, call writeSiteAtomFile($siteConfig, $collections, $entries, $atomPath) and writeSiteRssFile($siteConfig, $collections, $entries, $rssPath), assert the files were created, and assert their contents contain the same key strings checked in testSiteFeedsUseSiteMetadataAndEntryCollections (site title, site links, entry links, and the self atom/rss link) to ensure the file-writing paths are covered.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/Build/FeedGenerator.php`:
- Around line 120-129: The call to $this->createFileWriter() in
YiiPress\Build\FeedGenerator::writeSiteAtomFile is reported as undefined; ensure
the private function createFileWriter(string $path): XMLWriter is present with
the exact name and signature in the same final class
YiiPress\Build\FeedGenerator (or change its visibility to match usage if
needed), then rebuild/refresh packaging and autoload so CI picks up the updated
class (run composer dump-autoload, clear any PHAR/classmap caches and OPcache,
and verify no duplicate/stale FeedGenerator class exists in vendor or packaged
artifacts); if you intentionally moved or renamed createFileWriter, update
writeSiteAtomFile to call the new method name instead.
---
Nitpick comments:
In `@tests/Unit/Build/FeedGeneratorTest.php`:
- Around line 277-330: Add a focused unit test that exercises
FeedGenerator::writeSiteAtomFile() and FeedGenerator::writeSiteRssFile()
directly (similar to the existing testFeedFilesCanBeWrittenDirectly pattern)
rather than only generating feeds in-memory: reuse the setup from
testSiteFeedsUseSiteMetadataAndEntryCollections (create the same collections and
entries and instantiate FeedGenerator with
ContentProcessorPipeline/MarkdownProcessor), create two temporary file paths,
call writeSiteAtomFile($siteConfig, $collections, $entries, $atomPath) and
writeSiteRssFile($siteConfig, $collections, $entries, $rssPath), assert the
files were created, and assert their contents contain the same key strings
checked in testSiteFeedsUseSiteMetadataAndEntryCollections (site title, site
links, entry links, and the self atom/rss link) to ensure the file-writing paths
are covered.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 085be0e3-c3c5-45cb-80ec-00c61924b606
📒 Files selected for processing (8)
docs/commands.mddocs/configuration.mddocs/content.mdroadmap.mdsrc/Build/FeedGenerator.phpsrc/Console/BuildCommand.phptests/Unit/Build/FeedGeneratorTest.phptests/Unit/Console/BuildCommandTest.php
Comment on lines
+120
to
+129
| public function writeSiteAtomFile( | ||
| string $path, | ||
| SiteConfig $siteConfig, | ||
| array $collections, | ||
| array $entries, | ||
| ): void { | ||
| $xml = $this->createFileWriter($path); | ||
| $collection = $this->siteFeedCollection($siteConfig); | ||
| $this->writeAtomDocument($xml, $siteConfig, $collection, $this->limitEntries($collection, $entries), $collections); | ||
| $xml->flush(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== FeedGenerator class definitions =="
rg -nP --type=php 'namespace\s+YiiPress\\Build;|class\s+FeedGenerator\b'
echo
echo "== FeedGenerator method declarations related to failing call =="
rg -nP --type=php -C2 'function\s+writeSiteAtomFile\s*\(|function\s+createFileWriter\s*\('
echo
echo "== Autoload/classmap references to FeedGenerator =="
rg -nP 'YiiPress\\\\Build\\\\FeedGenerator|FeedGenerator\.php'Repository: yiipress/engine
Length of output: 3226
CI fatal: createFileWriter() undefined in writeSiteAtomFile()—check for stale/packaged FeedGenerator
src/Build/FeedGenerator.phpdefines a singlefinal class YiiPress\Build\FeedGenerator.- That same class declares
private function createFileWriter(string $path): XMLWriter(around line ~391) andwriteSiteAtomFile()calls$this->createFileWriter($path)(around line ~122). - If CI still reports the method as undefined at the call site, the build is likely loading an older/stale
FeedGeneratorartifact (PHAR/vendor/autoload/classmap cache). Inspect the CI packaging/autoload inputs used duringbuild.
🧰 Tools
🪛 GitHub Actions: Run Tests / 0_test.txt
[error] 122-122: Console command "build" failed: PHP Fatal error — Call to undefined method YiiPress\Build\FeedGenerator::createFileWriter().
🪛 GitHub Actions: Run Tests / test
[error] 122-122: Error: Call to undefined method YiiPress\Build\FeedGenerator::createFileWriter() while running console command "build". Triggered from writeSiteAtomFile() via /app/src/Console/BuildCommand.php(759).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/Build/FeedGenerator.php` around lines 120 - 129, The call to
$this->createFileWriter() in YiiPress\Build\FeedGenerator::writeSiteAtomFile is
reported as undefined; ensure the private function createFileWriter(string
$path): XMLWriter is present with the exact name and signature in the same final
class YiiPress\Build\FeedGenerator (or change its visibility to match usage if
needed), then rebuild/refresh packaging and autoload so CI picks up the updated
class (run composer dump-autoload, clear any PHAR/classmap caches and OPcache,
and verify no duplicate/stale FeedGenerator class exists in vendor or packaged
artifacts); if you intentionally moved or renamed createFileWriter, update
writeSiteAtomFile to call the new method name instead.
Source: Pipeline failures
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
/feed.xmland/rss.xmlfrom all feed-enabled collectionsTests
make test -- --filter FeedGeneratorTestmake test -- --filter testBuildGeneratesFeedsForCollectionsWithFeedEnabledmake test -- --filter testDryRunListsFilesWithoutWritingmake psalmmake testSummary by CodeRabbit
New Features
/feed.xml) and RSS 2.0 (/rss.xml) feeds now aggregate entries from all feed-enabled collections, respecting thefeed_limitsetting.Documentation