Skip to content

Fix page load warnings and dead GoDAM enqueue - #2359

Open
sanketio wants to merge 1 commit into
developfrom
fix/page-load-warnings-and-dead-godam-enqueue
Open

Fix page load warnings and dead GoDAM enqueue#2359
sanketio wants to merge 1 commit into
developfrom
fix/page-load-warnings-and-dead-godam-enqueue

Conversation

@sanketio

Copy link
Copy Markdown
Contributor

Part of #2358

Bucket-one fixes from the audit in #2358 — the subset that was reproduced on a running install rather than inferred from reading code. 5 lines added, 3 removed, no behaviour change beyond silencing two warnings and dropping one dead call.

What changed

app/main/RTMediaUploadTerms.php:100 — guard general_upload_terms_error_message with isset(). The key is never seeded by init_site_options(), so it only exists after an admin saves the Upload Terms settings. Every neighbouring line in the method already guards; this one did not. Fired a PHP Warning: Undefined array key on every front-end page load of a default install.

app/importers/RTMediaMigration.php:166 — initialise $count = 0 before the four accumulator branches. $count was first assigned inside if ( ! empty( $_SESSION['migration_user_album'] ) ), so on an install with no legacy bp-media data nothing assigned it and return $count; warned. Fired on every admin page load via add_migration_notice() on admin_init. Line 171 still uses = rather than +=, so behaviour is unchanged — this only removes the undefined-variable path.

templates/media/godam-integration.php:27 — drop wp_enqueue_style( 'godam-player-frontend-style' ). That handle exists nowhere in GoDAM 2.1.1; the other seven handles this function enqueues are all registered in inc/classes/shortcodes/class-godam-player.php:137-190. wp_enqueue_style() on an unregistered handle is a silent no-op, which is why it never surfaced.

templates/media/godam-integration.php:11,30 — correct two comments claiming the skin lookup is "multisite safe (uses get_site_option)". GoDAM reads and writes rtgodam-settings with get_option() / update_option() (inc/classes/rest-api/class-settings.php:348 is the writer, all 12 read sites match). The get_option() call is correct and unchanged — only the misleading comments are fixed, so nobody "corrects" this into a multisite regression later.

app/main/RTMediaUploadTerms.php:88load_plugin_textdomain( 'rtmedia', … )'buddypress-media'. 1,016 strings use buddypress-media, 5 use rtmedia; the call was loading a domain nothing reads.

Verification

Run against WP 7.0.3 / PHP 8.3.30 / BuddyPress 14.5.2 / MariaDB 11.8 on DDEV v1.25.2.

  • Warning counts in the PHP error log unchanged after loading /, /members/, /activity/, /wp-admin/, admin.php?page=rtmedia-settings, admin.php?page=rtmedia-migration-media-size-import — previously every front-end request incremented the first counter
  • No new rtMedia entries in the error log
  • All six URLs return HTTP 200
  • PHPCS clean on both changed PHP files
  • godam-integration.php still reports 4 pre-existing PHPCS errors (missing @package, 2x comment end-char, missing function doc) — untouched here, tracked in Fresh-install audit (4.7.11): PHP warnings on every page load, dead migration code, and asset build gaps #2358
  • Not verified with GoDAM actually installed and active — the removed handle was confirmed absent by reading the GoDAM 2.1.1 source, not by running it

Commands a reviewer can run:

# reproduce the two warnings on develop, then confirm they stop on this branch
ddev logs | grep -cE 'general_upload_terms_error_message|Undefined variable \$count'
curl -sk -o /dev/null -w '%{http_code}\n' https://<your-ddev-host>/
ddev logs | grep -cE 'general_upload_terms_error_message|Undefined variable \$count'   # unchanged

# confirm the removed handle does not exist in GoDAM
curl -sL -o godam.zip https://downloads.wordpress.org/plugin/godam.zip && unzip -q godam.zip
grep -rn 'godam-player-frontend-style' godam/    # no output
grep -rn "'godam-player-style'" godam/           # registered at class-godam-player.php:157

Durable how-to-test steps live in #2358.

Out of scope

Generated CSS is deliberately not included. grunt build only runs sass:minify, so the expanded admin.css / rtmedia.css had drifted from source (finding 3), and two SCSS files are never compiled at all (finding 4). Both need a diff of compiled-vs-committed output before landing, in case those CSS files were hand-edited while uncompiled. Separate PR.

Copilot AI balanced review requested due to automatic review settings August 10, 2026 07:06
@rtBot

rtBot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Unable to PHPCS or SVG scan one or more files due to error running PHPCS/SVG scanner:

  • app/importers/RTMediaMigration.php
  • app/main/RTMediaUploadTerms.php
  • templates/media/godam-integration.php

The error may be temporary. If the error persists, please contact a human (commit-ID: 14d3c90).

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Addresses reproducible runtime warnings from the audit in #2358 by adding missing guards/initialization, removing a dead GoDAM enqueue call, and correcting misleading comments/documentation.

Changes:

  • Prevent undefined array key/variable warnings by guarding option access and initializing a counter.
  • Remove a wp_enqueue_style() call for a GoDAM handle that is not registered in the referenced GoDAM version.
  • Update translation domain loading and correct misleading multisite-related comments.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
templates/media/godam-integration.php Removes dead style enqueue and updates comments about per-site GoDAM settings storage.
app/main/RTMediaUploadTerms.php Fixes option access warning and updates loaded textdomain to the actively used domain.
app/importers/RTMediaMigration.php Initializes $count to avoid undefined-variable warning when no legacy data exists.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

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.

3 participants