Code audit: deduplication, security hardening, and CI improvements - #4
Conversation
- Extract breadcrumb helpers (link/current/primary-term) to remove ~12 duplicated escaping patterns in the breadcrumbs shortcode - Extract mrdemonwolf_mu_dir() to deduplicate WPMU_PLUGIN_DIR fallback - Move inline service metabox JS to assets/admin-service-metabox.js; enqueue via wp_localize_script scoped to service edit screen only - Guard file_put_contents() with error_log() on failure - Switch social share to rawurlencode() for correct URL encoding - Add :root CSS vars --mdw-bg / --mdw-border; replace 10 hardcoded hex values - Add phpcs WordPress Coding Standards step to CI - Create CHANGELOG.md seeded with v1.0.0 (fixes broken release workflow) - Document Magnific Popup v1.1.0 pin and upgrade path in README Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 44 minutes and 18 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThis PR introduces a 1.0.0 release with CI improvements (PHP CodeSniffer linting), documentation updates (CHANGELOG.md and README notes), new admin JavaScript for service metabox handling, refactored breadcrumb and helper functions, improved error handling for file operations, and CSS custom properties for neutral palette colors. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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.
🧹 Nitpick comments (4)
theme/assets/admin-service-metabox.js (1)
19-24: Guard against missing attachment URL.
selection.first().toJSON()is normally populated, but if for any reason the selection is empty or the attachment has nourl(e.g., permissions/filter side-effects),attachment.urlwill beundefinedand the previewsrcwill be set to the string"undefined". A tiny guard avoids a user-visible broken image.Proposed guard
frame.on('select', function () { - var attachment = frame.state().get('selection').first().toJSON(); - $('#mdw-service-image').val(attachment.url); - $('#mdw-service-image-preview').attr('src', attachment.url).show(); - $('#mdw-service-remove-btn').show(); + var selection = frame.state().get('selection').first(); + if (!selection) { return; } + var attachment = selection.toJSON(); + if (!attachment || !attachment.url) { return; } + $('#mdw-service-image').val(attachment.url); + $('#mdw-service-image-preview').attr('src', attachment.url).show(); + $('#mdw-service-remove-btn').show(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@theme/assets/admin-service-metabox.js` around lines 19 - 24, Guard against a missing selection or missing attachment.url inside the frame.on('select', ...) handler: check that frame.state().get('selection').first() exists and that its toJSON() result has a truthy url before setting $('#mdw-service-image').val(...), $('#mdw-service-image-preview').attr('src', ...).show(), and $('#mdw-service-remove-btn').show(); if url is absent, avoid updating the preview src and instead clear or hide the preview and remove button to prevent a broken "undefined" image.theme/style.css (1)
11-14: Consider extending the variable coverage (optional).Nice centralization. For full consistency you could also migrate the remaining
#EEF2F7occurrence embedded inside the SVG data URI on line 49 (fill=\"%23EEF2F7\") — though since it's inside a data URI that's harder to parameterize, a brief comment near the:rootdeclaration pointing at that spot would help future rebrands. Not blocking.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@theme/style.css` around lines 11 - 14, The CSS centralizes colors in :root with --mdw-bg and --mdw-border but there's an embedded literal "#EEF2F7" inside an SVG data URI (fill="%23EEF2F7") that won't pick up the variable during rebrands; add a brief comment next to the :root declaration (or next to the --mdw-bg line) pointing developers to the SVG data URI occurrence (fill="%23EEF2F7") so future changes know to update that data URI as well or to consider parameterizing it.theme/functions.php (2)
236-255: Helpers look good — minor note on primary term resolution.
mrdemonwolf_primary_term_link()usesreset( $terms )to pick the first returned term, which is not necessarily the "primary" one (Yoast/RankMath store a primary term in post meta, andget_the_terms()order is by term_id by default). This matches the pre-refactor behavior so it's not a regression, but if you later want the SEO-primary term, this helper is the place to add that lookup.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@theme/functions.php` around lines 236 - 255, mrdemonwolf_primary_term_link currently picks the first term from get_the_terms via reset($terms); update mrdemonwolf_primary_term_link to first check for an SEO-primary term stored in post meta (e.g. meta keys like _yoast_wpseo_primary_{taxonomy} and _rank_math_primary_{taxonomy}), retrieve that term ID and load the term (get_term/get_term_by) for the given $taxonomy, and only if no primary-meta term exists fall back to the existing get_the_terms() + reset($terms) behavior; ensure the function still returns '' on empty/error and continues to call mrdemonwolf_breadcrumb_link with the term link and name.
119-121: Replacefile_put_contents()anderror_log()withWP_Filesystemmethods.WordPress Coding Standards flags both functions by default in the WordPress standard:
file_put_contentstriggersWordPress.WP.AlternativeFunctions.file_system_operationsanderror_logtriggers development function warnings. Since this runs in theswitch_themehook (admin context), initializingWP_Filesystemis straightforward.Sketch
global $wp_filesystem; if ( ! function_exists( 'WP_Filesystem' ) ) { require_once ABSPATH . 'wp-admin/includes/file.php'; } WP_Filesystem(); if ( ! $wp_filesystem || ! $wp_filesystem->put_contents( $mu_file, $mu_code, FS_CHMOD_FILE ) ) { error_log( 'MrDemonWolf: failed to write cleanup mu-plugin to ' . $mu_file ); }Also, add a one-line comment at line 109 explaining the intentional duplication of the
WPMU_PLUGIN_DIRfallback logic inside the heredoc—the generated mu-plugin runs standalone and cannot call theme functions, so reusingmrdemonwolf_mu_dir()is not possible.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@theme/functions.php` around lines 119 - 121, Replace direct file operations and error logging: initialize WP_Filesystem (require_once ABSPATH . 'wp-admin/includes/file.php' if needed, call WP_Filesystem()) and use $wp_filesystem->put_contents( $mu_file, $mu_code, FS_CHMOD_FILE ) instead of file_put_contents(), and replace the error condition to log via the same fallback (keep the existing error_log message text but only call it if $wp_filesystem is not available or put_contents returns false). Also add a one-line comment above the heredoc explaining the intentional duplication of the WPMU_PLUGIN_DIR fallback because the generated mu-plugin runs standalone and cannot call mrdemonwolf_mu_dir().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@theme/assets/admin-service-metabox.js`:
- Around line 19-24: Guard against a missing selection or missing attachment.url
inside the frame.on('select', ...) handler: check that
frame.state().get('selection').first() exists and that its toJSON() result has a
truthy url before setting $('#mdw-service-image').val(...),
$('#mdw-service-image-preview').attr('src', ...).show(), and
$('#mdw-service-remove-btn').show(); if url is absent, avoid updating the
preview src and instead clear or hide the preview and remove button to prevent a
broken "undefined" image.
In `@theme/functions.php`:
- Around line 236-255: mrdemonwolf_primary_term_link currently picks the first
term from get_the_terms via reset($terms); update mrdemonwolf_primary_term_link
to first check for an SEO-primary term stored in post meta (e.g. meta keys like
_yoast_wpseo_primary_{taxonomy} and _rank_math_primary_{taxonomy}), retrieve
that term ID and load the term (get_term/get_term_by) for the given $taxonomy,
and only if no primary-meta term exists fall back to the existing
get_the_terms() + reset($terms) behavior; ensure the function still returns ''
on empty/error and continues to call mrdemonwolf_breadcrumb_link with the term
link and name.
- Around line 119-121: Replace direct file operations and error logging:
initialize WP_Filesystem (require_once ABSPATH . 'wp-admin/includes/file.php' if
needed, call WP_Filesystem()) and use $wp_filesystem->put_contents( $mu_file,
$mu_code, FS_CHMOD_FILE ) instead of file_put_contents(), and replace the error
condition to log via the same fallback (keep the existing error_log message text
but only call it if $wp_filesystem is not available or put_contents returns
false). Also add a one-line comment above the heredoc explaining the intentional
duplication of the WPMU_PLUGIN_DIR fallback because the generated mu-plugin runs
standalone and cannot call mrdemonwolf_mu_dir().
In `@theme/style.css`:
- Around line 11-14: The CSS centralizes colors in :root with --mdw-bg and
--mdw-border but there's an embedded literal "#EEF2F7" inside an SVG data URI
(fill="%23EEF2F7") that won't pick up the variable during rebrands; add a brief
comment next to the :root declaration (or next to the --mdw-bg line) pointing
developers to the SVG data URI occurrence (fill="%23EEF2F7") so future changes
know to update that data URI as well or to consider parameterizing it.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ab08ad63-9ce6-49f9-98e7-1e00f7d5dbe7
📒 Files selected for processing (6)
.github/workflows/ci.ymlCHANGELOG.mdREADME.mdtheme/assets/admin-service-metabox.jstheme/functions.phptheme/style.css
10up/wpcs-action requires a composer.json in the repo and does not accept an 'extensions' input. Install WPCS globally via composer and run phpcs directly instead. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ugin dealerdirect/phpcodesniffer-composer-installer is blocked by Composer's allow-plugins policy on the runner. Install phpcs + WPCS directly and set installed_paths via phpcs --config-set instead. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The runner has dealerdirect/phpcodesniffer-composer-installer pre-installed globally; Composer blocks it without explicit allow-plugins permission. Allow it first, then install WPCS. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Replace broken --config-set installed_paths (clobbered WPCS 3 deps) with a single composer require; the plugin sets paths correctly - Switch CI standard to WordPress-Core with -n (errors only); the full WordPress ruleset only flags doc-block/comment-style items that conflict with Divi-theme conventions - Auto-format theme/functions.php via phpcbf (spacing, tabs, short→ long array syntax, brace style, Yoda condition) — 231 errors → 0 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 participant
Summary
Full production code audit of the MrDemonWolf Divi child theme — no new features, only fixes and improvements identified during audit.
mrdemonwolf_breadcrumb_link(),mrdemonwolf_breadcrumb_current(), andmrdemonwolf_primary_term_link()helpers, removing ~12 repeated escaping patterns across WooCommerce, post, project, page, category, and archive branchesWPMU_PLUGIN_DIRfallback — extracted tomrdemonwolf_mu_dir()helper<script>block totheme/assets/admin-service-metabox.js, enqueued viawp_localize_scriptscoped to service edit screen only viaadmin_enqueue_scriptsfile_put_contents()error handling — mu-plugin write failure now reported viaerror_log()instead of silently failingrawurlencode()in social share — replacedurlencode()so spaces encode as%20not+in URL query strings#EEF2F7and#C8D3E0to--mdw-bg/--mdw-bordercustom properties at:root; 10 usage sites updated10up/wpcs-action@stableCHANGELOG.mdcreated — release workflow was referencing a missing file, causing broken release builds; seeded with v1.0.0 entryTest plan
php -l theme/functions.php— no errorsgrep -ri "nexus" theme/ supplementary/— 0 matches./build.sh→build/mrdemonwolf.zipproduced[mrdemonwolf_tags]and[mrdemonwolf_social_share]render🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Chores