[subodhr258]

Fixes #2220

Review packet

In one line: audio that GoDAM Central converts to MP3 is now saved with WordPress's MP3 type, audio/mpeg, instead of audio/mp3, so the block editor's Inserter > Media > Audio list and other REST API readers find it; a one-time update step fixes files converted before. Fixes no issue: this is the "Transcoded MP3 mime type" known issue from the 2.4.0 QA sign-off.

Why it happened: when Central's "MP3 ready" callback arrives, add_transcoded_files() in admin/class-rtgodam-transcoder-handler.php saves the attachment as audio/mp3. WordPress registers MP3 as audio/mpeg and does not know audio/mp3, so anything that reads WordPress's list of types misses these files. The Media Library's Audio filter still showed them, because it matches audio/ as a prefix.

The fix:

• New conversions are saved as audio/mpeg (video still saves video/mp4).
• A one-time migration, Transcoded_Mp3_Mime_Type, in the existing migrations runner under a 2.5.0 batch: one UPDATE changes every audio/mp3 attachment to audio/mpeg, then clears each one's cache. It runs on the first request after the update, once per site (each site on a multisite), and retries if the database fails.

1. Criteria and proof

Acceptance criterion	Proof
After GoDAM Central's "MP3 ready" callback the attachment is audio/mpeg (2.4.0: audio/mp3)	V2 step 2 vs V1 step 2; TranscodedAudioMimeTypeTest
The converted file is in the block editor's Inserter > Media > Audio, which asks REST for media_type=audio (2.4.0: missing)	V2 step 3: 2 players (the 0:03 WAV and the 0:07 note); V1 step 3: only the WAV
A file converted by 2.4.0 (still audio/mp3) becomes audio/mpeg on the first request after the update and shows in the list	V2 steps 5 and 6: 3 players; V1: still audio/mp3, still missing; TranscodedMp3MimeTypeTest
A site that already updated to 2.4.1 (which ships without this PR) still gets the fix when it updates to 2.5.0	V4: the older file goes from audio/mp3 to audio/mpeg and appears in the list; V3 shows the same update skipping it while the fix was filed under 2.4.1; test_the_migration_runner_runs_it
Video conversions still save video/mp4	test_converted_video_is_still_saved_as_video_mp4
The migration runs once, does nothing on a site with no audio/mp3 files, and is retried (not marked done) when the SELECT or the UPDATE fails	TranscodedMp3MimeTypeTest (6 tests)

2. Before and after

Two panes, recorded in Chrome on godam-dev.local with the same steps on 2.4.0 and on this branch. Left (CAUSE): the recording's details in the Media Library. Right (EFFECT): the block editor's Inserter > Media > Audio list, which asks the REST API for media_type=audio. GoDAM Central's "MP3 ready" callback is its real request, replayed on the local site with an MP3 served from the same machine. Each caption is checked before the next step. The videos are in the rtCamp Test GoDAM library (log in to watch).

V1, before (GoDAM 2.4.0): after the callback the 0:07 voice note reads audio/mp3 and is missing from the Audio list (only the existing 0:03 WAV). A 0:05 note converted earlier stays audio/mp3 after the update step.

V2, after (this PR): the same steps. The voice note reads audio/mpeg and appears in the list; after the update step the older note is audio/mpeg too, and the list shows all three.

Update from 2.4.1 to 2.5.0. 2.4.1 ships without this PR but still stores 2.4.1 as the site's update version, and the runner only runs batches above that. Both videos start on godam-dev with the stored version at 2.4.1 and an older voice note saved as audio/mp3, then run the update step with the version set to 2.5.0 (one WP-CLI request).

V3, fix filed under 2.4.1 (4db9e5e): the stored version moves to 2.5.0, but the file stays audio/mp3 and missing from the Audio list: the update skipped it.

V4, fix filed under 2.5.0 (4b6b0a5): the same steps. The file becomes audio/mpeg and appears in the Audio list.

What REST returns for a converted file on godam-dev.local (Central's callback replayed locally):

Request	2.4.0	This branch
/wp/v2/media?media_type=audio	the converted file is left out (only the WAV)	listed
/wp/v2/media?mime_type=audio/mp3	the filter is ignored: all 64 media items come back	no audio/mp3 items left
/wp/v2/media?mime_type=audio/mpeg	0 items	the converted files

3. Tests

• Added: tests/php/TranscodedAudioMimeTypeTest.php (4 tests). It calls add_transcoded_files() with stubbed WordPress functions: converted .webm, Safari .mp4 and WAV audio is saved as audio/mpeg and _wp_attached_file points at the MP3; a converted video is still saved as video/mp4.
• Added: tests/php/TranscodedMp3MimeTypeTest.php (6 tests). audio/mp3 attachments become audio/mpeg and only they change (not other audio, videos, or non-attachment rows), and their caches are cleared; a site with none runs no UPDATE; a failed SELECT or UPDATE is not marked done; it runs only once; the runner has it in the 2.5.0 batch and the batches stay in version order.
• Changed: none.
• Revert check: with audio/mp3 put back in the handler, 3 of 4 TranscodedAudioMimeTypeTest tests fail ('post_mime_type' => 'audio/mp3' instead of 'audio/mpeg'). Removing the runner entry fails test_the_migration_runner_runs_it; writing another type fails test_audio_mp3_attachments_become_audio_mpeg.
• Full PHP suite: 434 tests OK. PHPCS clean; hook-integrity checks (coverage and balance) pass.

4. Risk flags

2 spots in 2 files, raised by a script, not by judgement.

Database migrations (2)

• inc/classes/migrations/class-runner.php, inc/classes/migrations/class-transcoded-mp3-mime-type.php: a new 2.5.0 batch that changes post_mime_type from audio/mp3 to audio/mpeg on attachments. It cannot be undone by the plugin, but it only moves files to the type WordPress already uses for MP3. It does not run on develop today (RTGODAM_VERSION is 2.4.0); it runs when 2.5.0 or later reaches a site, whether that site comes from 2.4.0 or from 2.4.1.

5. Independent review

A separate review pass saw only the problem and the diff, ran the PHP suite, PHPCS and both hook checks, and mutated the code to see which tests fail.

Finding	Grade	What I did
The hook-integrity check (coverage.php) flags the migration's two direct queries, so CI would go red	Valid	Fixed in 4db9e5e: each query carries a godam-coverage-ignore note (the runner already runs it once per site); the check passes
The handler change had no test: reverting it left every test green	Valid	Added TranscodedAudioMimeTypeTest in 4db9e5e. Writing it showed add_transcoded_files() started $transcoded_files as false and then used it as an array, which PHP 8.1+ reports as deprecated on every conversion; it now starts as array()
A failed SELECT returns an empty list, so the migration was marked done without running	Valid, low	Fixed in 4db9e5e: it checks $wpdb->last_error and returns false; test_a_failed_select_is_retried
The runner test asserted 2.4.1 is the last batch, which breaks when a later batch is added	Valid, nit	It now checks that batches are in version order
The 2.4.1 key must be the version this ships in	Valid	Fixed in 4b6b0a5: this ships in 2.5.0, so the batch is keyed 2.5.0. 2.4.1 ships without it but stores 2.4.1 as the site's version, and the runner skips batches at or below that, so a 2.4.1 key would never run on sites that update through 2.4.1
clean_post_cache() per file on a site with thousands of converted MP3s, not batched	Valid, low	Not changed: converted audio is usually a few dozen files per site
The raw UPDATE skips edit_attachment hooks	Overstated	GoDAM's listeners there are for video; the audio block already writes type="audio/mpeg"
No other code writes or compares audio/mp3	Checked	All type checks in PHP and JS use the audio/ prefix
GoDAM-tab virtual media take Central's type as sent; if Central ever sends audio/mp3 they would have the same problem	Unverified	Not changed; listed in section 6

6. Not verified

• A real GoDAM Central conversion: the callback was replayed on godam-dev.local with Central's parameters and an MP3 served from the same machine; nothing was sent to Central.
• A real plugin update: the update step was run through WP-CLI with the version constant set (2.4.1 for V2; 2.5.0 for V3 and V4, starting from a site whose stored version is 2.4.1).
• Multisite: covered by reading the runner code (it switches to each site), not run.
• What Central sends as the type for GoDAM-tab virtual media.
• audio/webm and audio/mp4 (stored since 2.4.0 for audio-only uploads) are not registered WordPress types either, so a file is missing from media_type=audio while it is being converted, or if the conversion fails. Not changed here.

7. Scope

• add_transcoded_files() starts $transcoded_files as array() instead of false (PHP 8.1+ deprecation, found by the new test).
• The migration changes every audio/mp3 attachment, including any another plugin created; all of them are MP3 files, so audio/mpeg is right for all.

[github-actions]

🔍 WordPress Plugin Check Report

⚠️ Status: Passed with warnings

📊 Report

🎯 Total Issues	❌ Errors	⚠️ Warnings
13	0	13

---

⚠️ Warnings (13)

📁 readme.txt (2 warnings)

📍 Line	🔖 Check	💬 Message
0	mismatched_plugin_name	Plugin name "GoDAM - Organize WordPress Media Library & File Manager with Unlimited Folders for Images, Videos & more" is different from the name declared in plugin header "GoDAM".
0	trademarked_term	The plugin name includes a restricted term. Your chosen plugin name - "GoDAM - Organize WordPress Media Library & File Manager with Unlimited Folders for Images, Videos & more" - contains the restricted term "wordpress" which cannot be used at all in your plugin name.

📁 assets/build/css/main.css (1 warning)

📍 Line	🔖 Check	💬 Message
0	EnqueuedStylesScope	This style is being loaded in all contexts.

📁 assets/src/libs/analytics.min.js (5 warnings)

📍 Line	🔖 Check	💬 Message
0	EnqueuedScriptsScope	This script is being loaded in all frontend contexts.
0	NonBlockingScripts.NoStrategy	This script on http://localhost:8880 (with handle analytics-library) is loaded in the footer. Consider a defer or async script loading strategy instead.
0	NonBlockingScripts.NoStrategy	This script on http://localhost:8880/2026/10/20/hello-world/ (with handle analytics-library) is loaded in the footer. Consider a defer or async script loading strategy instead.
0	NonBlockingScripts.NoStrategy	This script on http://localhost:8880/sample-page/ (with handle analytics-library) is loaded in the footer. Consider a defer or async script loading strategy instead.
0	NonBlockingScripts.NoStrategy	This script on http://localhost:8880/demo-attachment-post/ (with handle analytics-library) is loaded in the footer. Consider a defer or async script loading strategy instead.

📁 assets/build/js/main.min.js (5 warnings)

📍 Line	🔖 Check	💬 Message
0	EnqueuedScriptsScope	This script is being loaded in all frontend contexts.
0	NonBlockingScripts.NoStrategy	This script on http://localhost:8880 (with handle rtgodam-script) is loaded in the footer. Consider a defer or async script loading strategy instead.
0	NonBlockingScripts.NoStrategy	This script on http://localhost:8880/2026/10/20/hello-world/ (with handle rtgodam-script) is loaded in the footer. Consider a defer or async script loading strategy instead.
0	NonBlockingScripts.NoStrategy	This script on http://localhost:8880/sample-page/ (with handle rtgodam-script) is loaded in the footer. Consider a defer or async script loading strategy instead.
0	NonBlockingScripts.NoStrategy	This script on http://localhost:8880/demo-attachment-post/ (with handle rtgodam-script) is loaded in the footer. Consider a defer or async script loading strategy instead.

---

🤖 Generated by WordPress Plugin Check Action • Learn more about Plugin Check

[KMchaudhary]

Code review

Verdict: LGTM — correct fix, idempotent migration, and genuinely thorough tests. I traced both halves:

• Handler: the real fix is the wp_update_post literal at :961 (audio/mp3 → audio/mpeg), gated by strpos( $mime_type, 'audio' ). The false → array() change fixes a real PHP 8.1 Automatic conversion of false to array deprecation at the $transcoded_files[$key][] = append, and both usage sites behave identically (! empty() treats false and [] the same), so no behavioural change there. Good catch via the new test.
• Migration: the retry-on-failure design is right — last_error after the SELECT and false === after the UPDATE both return false so the per-site OPTION_KEY is not set and the runner retries. It's also idempotent (a re-run finds no audio/mp3 rows) and safe on multisite (per-site option, switch_to_blog). Caches are cleared for the selected IDs.

Three low-severity notes, none blocking:

1. The migration normalizes all audio/mp3 attachments, not only GoDAM's — class-transcoded-mp3-mime-type.php

The UPDATE … WHERE post_mime_type = 'audio/mp3' has no GoDAM-specific predicate (e.g. a rtgodam_transcoded_url meta), so any audio/mp3 row from another plugin or a manual import is also rewritten. This is almost certainly correct — WordPress never assigns audio/mp3, and audio/mpeg is the right type for an MP3 — so it's corrective, not harmful. Just worth one line in the release notes that the step normalizes every audio/mp3 attachment, since the scope is a touch wider than "files GoDAM converted."

2. The :902 ternary edit is cosmetic, not the fix — class-rtgodam-transcoder-handler.php:902

$post_mime_type from that ternary is consumed only by the $timeout branch (if ( 'video/mp4' === $post_mime_type )); the stored type comes from wp_update_post at :961. Changing the audio branch from audio/mp3 to audio/mpeg has no functional effect. Fine to keep for consistency — flagging only so a future reader doesn't think this line is where the mime type is set.

3. @since n.e.x.t — class-transcoded-mp3-mime-type.php

Placeholder on the class and maybe_run(); resolve to 2.5.0 before tagging (your usual convention).

Minor

• Tiny SELECT→UPDATE race: an audio/mp3 attachment created between the two queries gets its type changed but its cache not cleared (it isn't in $ids). For a one-time migration this is negligible.
• Worth confirming the RTGODAM_VERSION bump to 2.5.0 ships with this release, since the 2.5.0 batch only runs once the constant reaches that version (you've noted it won't run on develop at 2.4.0).

Really nice test coverage — the in-memory $wpdb fake exercising the SELECT/UPDATE failure paths and the once-only guard is exactly right for this.

_{🤖 Automated review via Claude Code (/code-review, high effort).}

[subodhr258]

@KMchaudhary on your comment:

Thanks Kuldip.

1. Every audio/mp3 attachment: yes, intended, since WordPress never assigns audio/mp3 and audio/mpeg is right for any MP3. It's in the PR's scope section, and the 2.5.0 release notes will say it.
2. The :902 ternary: agreed, consistency only. The stored type comes from wp_update_post at :961.
3. @since n.e.x.t: resolved to 2.5.0 at release prep.

Minor: agreed the race is negligible. The batch is keyed 2.5.0 since 4b6b0a5 and the version bump ships with the release, so it runs on update from 2.4.0 and from 2.4.1 (V3 and V4 in the description).