Sync image removal to Castos and add featured image filters#901
Merged
Conversation
Castos API now distinguishes absent vs empty featured_image_url. Send empty value on updates so stale images get cleared. Add four filters for developer control over featured image fallback and removal. Co-Authored-By: Claude via AIContext
📝 WalkthroughWalkthroughThis PR adds filter-driven control over featured-image fallback behavior across three image contexts: feed URLs, player album art, and Castos sync. Each context introduces a corresponding filter ( ChangesFeatured-Image Fallback Filters Across Feed, Player, and Castos Sync
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/WPUnit/CastosHandlerImageTest.php (1)
24-25: ⚡ Quick winScope
pre_http_requestcleanup to test-owned callbacks only.
remove_all_filters('pre_http_request')can wipe unrelated callbacks and create cross-test coupling. Prefer storing the callback and removing that specific callback.Suggested change
class CastosHandlerImageTest extends \Codeception\TestCase\WPTestCase { + /** `@var` callable|null */ + protected $pre_http_request_callback = null; + protected function tearDown(): void { remove_all_filters('ssp_use_featured_image_for_castos'); remove_all_filters('ssp_allow_castos_image_removal'); - remove_all_filters('pre_http_request'); + if ($this->pre_http_request_callback) { + remove_filter('pre_http_request', $this->pre_http_request_callback, 10); + $this->pre_http_request_callback = null; + } parent::tearDown(); }🤖 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/WPUnit/CastosHandlerImageTest.php` around lines 24 - 25, The test currently calls remove_all_filters('pre_http_request') which can remove other tests' callbacks; instead, store the specific callback when adding the filter (e.g., assign the callable to a property like $this->preHttpRequestCallback in setUp or where add_filter('pre_http_request', ...) is called) and in tearDown call remove_filter('pre_http_request', $this->preHttpRequestCallback, $priority) (matching the original priority) rather than remove_all_filters; update the test class (CastosHandlerImageTest) to keep the callback reference and use remove_filter in tearDown to only remove the test-owned callback.
🤖 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.
Nitpick comments:
In `@tests/WPUnit/CastosHandlerImageTest.php`:
- Around line 24-25: The test currently calls
remove_all_filters('pre_http_request') which can remove other tests' callbacks;
instead, store the specific callback when adding the filter (e.g., assign the
callable to a property like $this->preHttpRequestCallback in setUp or where
add_filter('pre_http_request', ...) is called) and in tearDown call
remove_filter('pre_http_request', $this->preHttpRequestCallback, $priority)
(matching the original priority) rather than remove_all_filters; update the test
class (CastosHandlerImageTest) to keep the callback reference and use
remove_filter in tearDown to only remove the test-owned callback.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ec106371-5d32-4f93-a980-6544c07b00e9
📒 Files selected for processing (6)
php/classes/controllers/class-frontend-controller.phpphp/classes/handlers/class-castos-handler.phpphp/classes/repositories/class-episode-repository.phptests/WPUnit/CastosHandlerImageTest.phptests/WPUnit/EpisodeRepositoryAlbumArtTest.phptests/WPUnit/FrontendControllerTest.php
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
1 participant
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
featured_image_urlwas omitted from the payload, so Castos kept the old imagessp_use_featured_image_for_castos,ssp_use_featured_image_for_player, andssp_use_featured_image_for_feedfilters (all defaulttrue) to control featured image fallback per pathssp_allow_castos_image_removalfilter (defaulttrue) to control whether empty values clear images on Castos$episode_idas second argument for per-episode controlCastos_Handler::get_image_for_castos()for clarityTest plan
add_filter('ssp_use_featured_image_for_castos', '__return_false')— verify featured image is not syncedadd_filter('ssp_allow_castos_image_removal', '__return_false')— verify removing image does not clear on Castosadd_filter('ssp_use_featured_image_for_player', '__return_false')— verify player shows placeholder instead of featured imageadd_filter('ssp_use_featured_image_for_feed', '__return_false')— verify feed item omits featured imageCloses CastosHQ/ssp-issues#849
Summary by CodeRabbit
New Features
Tests