fix: include feed source URL and import ID in log context for easier troubleshooting#1284
Open
lucadobrescu wants to merge 5 commits into
Open
fix: include feed source URL and import ID in log context for easier troubleshooting#1284lucadobrescu wants to merge 5 commits into
lucadobrescu wants to merge 5 commits into
Conversation
…troubleshooting Scope the logger context to each import run so every entry written while a job runs carries import_id, import_title and source; log single-feed parse failures that were previously swallowed; surface the import and feed URL as badges in the logs viewer with fallbacks for legacy job_id/source entries. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
|
Plugin build for 69fc9f7 is ready 🛎️!
Note You can preview the changes in the Playground |
The Translations Diff CI job fails on any POT change; render the import ID as a plain (#ID) suffix and drop the badge tooltips so the badges add no new strings. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add direct-access guards to 26 PHP files; replace unlink() with wp_delete_file(); annotate the deliberate direct-filesystem fast paths; rename chosen-sprite@2x.png (special character) and the SimplePie util file that false-matched the core-library check (class name unchanged, resolved via an autoloader class map). Also raise the run-now wait timeout in the e2e helper for slow import runs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment on lines
+62
to
+71
| // Class files named differently from the autoloader convention | ||
| // (e.g. to avoid matching core-library filename patterns). | ||
| $class_map = array( | ||
| 'Feedzy_Rss_Feeds_Util_SimplePie' => 'includes/util/feedzy-rss-feeds-util-feed.php', | ||
| ); | ||
| if ( isset( $class_map[ $class_to_load ] ) ) { | ||
| require_once plugin_dir_path( __FILE__ ) . $class_map[ $class_to_load ]; | ||
|
|
||
| return true; | ||
| } |
Contributor
There was a problem hiding this comment.
I think we can remove this.
Soare-Robert-Daniel
requested changes
Jul 20, 2026
…Util_Feed Match the class name to its file (feedzy-rss-feeds-util-feed.php) so the autoloader's naming convention resolves it, and drop the special-case class map from feedzy_rss_feeds_autoload(). Addresses review feedback: #1284 (review) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds import/feed attribution to logs and improves troubleshooting, alongside Plugin Check cleanup.
Changes:
- Adds scoped import context and feed parsing error attribution.
- Displays import/source badges with legacy fallbacks and E2E coverage.
- Adds access guards, safer file deletion, and asset/class renames.
Reviewed changes
Copilot reviewed 35 out of 36 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
tests/e2e/utils.js |
Adds log-test helpers and import utilities. |
tests/e2e/specs/log-attribution.spec.js |
Tests log attribution flows. |
phpstan-baseline.neon |
Updates renamed utility references. |
includes/views/wordai-view.php |
Adds direct-access guard. |
includes/views/spinnerchief-view.php |
Adds direct-access guard. |
includes/views/openrouter-view.php |
Adds direct-access guard. |
includes/views/openai-view.php |
Adds direct-access guard. |
includes/views/misc-view.php |
Adds direct-access guard. |
includes/views/import-metabox-edit.php |
Adds direct-access guard. |
includes/views/css/chosen.css |
References renamed sprite. |
includes/views/css/chosen-sprite-2x.png |
Renames high-resolution sprite. |
includes/views/amazon-product-advertising-view.php |
Adds direct-access guard. |
includes/views/ai-quota-view.php |
Adds direct-access guard. |
includes/util/feedzy-rss-feeds-util-feed.php |
Renames the SimplePie utility class/file. |
includes/layouts/setup-wizard.php |
Adds direct-access guard. |
includes/layouts/settings.php |
Adds direct-access guard. |
includes/layouts/integration.php |
Adds direct-access guard. |
includes/layouts/header.php |
Adds direct-access guard. |
includes/layouts/feedzy-tutorial.php |
Adds direct-access guard. |
includes/layouts/feedzy-support.php |
Adds direct-access guard. |
includes/layouts/feedzy-subscribe-notice.php |
Adds direct-access guard. |
includes/layouts/feedzy-schedules.php |
Adds direct-access guard. |
includes/layouts/feedzy-pro.php |
Adds direct-access guard. |
includes/layouts/feedzy-logs-viewer.php |
Renders attribution badges and fallbacks. |
includes/layouts/feedzy-license.php |
Adds direct-access guard. |
includes/layouts/feedzy-improve.php |
Adds direct-access guard. |
includes/layouts/feedzy-documentation.php |
Adds direct-access guard. |
includes/feedzy-rss-feeds-feed-tweaks.php |
Adds direct-access guard. |
includes/admin/feedzy-rss-feeds-upgrader.php |
Adds direct-access guard. |
includes/admin/feedzy-rss-feeds-options.php |
Adds direct-access guard. |
includes/admin/feedzy-rss-feeds-log.php |
Documents direct filesystem operations. |
includes/admin/feedzy-rss-feeds-import.php |
Scopes log context and modernizes file deletion. |
includes/admin/feedzy-rss-feeds-actions.php |
Adds direct-access guard. |
includes/abstract/feedzy-rss-feeds-admin-abstract.php |
Logs single-feed failures with URLs. |
form/form.php |
Adds direct-access guard. |
css/settings.css |
Styles attribution badges. |
Comments suppressed due to low confidence (1)
includes/util/feedzy-rss-feeds-util-feed.php:31
- Renaming this public class removes the autoload target for
Feedzy_Rss_Feeds_Util_SimplePie. Any extension that still instantiates the old class will now fail before this new file is loaded, and there is no compatibility alias or autoloader mapping. Please keep the old class name resolvable while avoiding the flagged filename, for example via a bootstrap/autoloader alias to this class.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+294
to
+309
| await page | ||
| .locator('tr', { hasText: importName }) | ||
| .getByRole('button', { name: 'Run Now' }) | ||
| .click(); | ||
|
|
||
| const runNowResponse = await page.waitForResponse( | ||
| (response) => | ||
| response.url().includes('/wp-admin/admin-ajax.php') && | ||
| response.request().method() === 'POST' && | ||
| response | ||
| .request() | ||
| .postData() | ||
| .includes('action=feedzy&_action=run_now'), | ||
| // Importing a feed can take a while (remote fetch + image sideloads). | ||
| { timeout: 60000 } | ||
| ); |
Comment on lines
+168
to
+170
| // Backward compatibility: entries whose context has no attribution keys | ||
| // (this one only has a `feed_urls` array) render fine, without a badge row. | ||
| const keylessEntry = page.locator('.fz-log-container', { |
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.
Feedzy logs couldn't tell you which import produced an entry, so users with multiple feeds couldn't trace an error back to its source (#1278). This PR tags every log entry written during an import run with the import's ID, title and source, and surfaces them as badges in the log viewer.
What changed
run_job()is now a thin wrapper that sets persistent logger context (import_id,import_title,source) via the existingFeedzy_Rss_Feeds_Log::set_context()and clears it infinally; the original body moved unchanged torun_job_logic(). Every entry written during a run — including fetch-layer errors — is attributed automatically.init_single_feed()returned early on unparseable feeds); it now logs the SimplePie error withfeed_url, matching the multi-feed path.Title (#ID)) and a feed-source badge per entry, falling back to the legacyjob_id/sourcecontext keys; entries without attribution keys render as before, and the raw JSON context stays.Also in this PR
WordPress.org Plugin Check errors (pre-existing, plugin-wide — the check fails every PR until resolved):
ABSPATHguards added to 26 PHP files.unlink()→wp_delete_file()(5 call sites); the deliberate direct-filesystem fast paths (rename()on temp files,fclose()/readfile()in the guarded log tail/download) carry the matchingphpcs:ignoreannotations the file already used forfopen().chosen-sprite@2x.png→chosen-sprite-2x.png(special character in filename), CSS reference updated.feedzy-rss-feeds-util-simplepie.php→feedzy-rss-feeds-util-feed.php— the filename false-matched the bundled-core-library check. The class is renamed to match (Feedzy_Rss_Feeds_Util_SimplePie→Feedzy_Rss_Feeds_Util_Feed), so the autoloader's naming convention resolves it with no special-casing; Pro has no references to the old name.Where to start reviewing
includes/admin/feedzy-rss-feeds-import.php— therun_job()wrapper is the behavior change;run_job_logic()is a pure rename.includes/abstract/feedzy-rss-feeds-admin-abstract.php— the previously silent single-feed error path.includes/layouts/feedzy-logs-viewer.php— badge rendering and legacy-key fallbacks.css/settings.cssare mechanical.flowchart TD A["run_now() / run_cron()"] --> B["run_job(): set_context(import_id, import_title, source), clear in finally"]:::added B --> C["run_job_logic() — former run_job body"] C --> D["init_single_feed(): parse failure now logged with feed_url"]:::changed D --> E["Feedzy_Rss_Feeds_Log: persistent + per-call context merged"] E --> F["Logs viewer: import + feed badges, legacy job_id/source fallback"]:::changed classDef added fill:#1a7f37,color:#fff,stroke:#1a7f37 classDef changed fill:#9a6700,color:#fff,stroke:#9a6700🟩 green = added by this PR, 🟨 amber = behavior changed, gray = untouched.
Data transformation
New optional keys inside each JSONL log entry's
contextobject (file:wp-content/uploads/feedzy-logs/feedzy.jsonl; written only while an import job runs; old entries are never rewritten):import_idrun_job()wrapperimport_titlerun_job()wrappersourcesourcemeta (URL list or category slug)run_job()wrapper; per-callfeed_urloverrides it for URL-specific entriesQA instructions
https://example.com/nonexistent-feed.xml), Run Now on both, open Feedzy → Settings → Logs, filter by Error: each error entry names the failing import and its feed URL — the healthy import is never blamed.[feedzy-rss feeds="…dead-url…"]shortcode, reload the logs: the fetch error shows a feed badge but no import badge.tests/e2e/specs/log-attribution.spec.jscovers all three flows (written failing-first against the unpatched code to confirm the bug, green after the fix).Check before Pull Request is ready:
Closes #1278.
🤖 Generated with Claude Code