[subodhr258]

Review packet

In one line: The plugin no longer puts the signed-in viewer's email address into the front-end page (in the videoAnalyticsParams object and in the player's godamData.currentLoggedInUserData) or into the analytics events the player sends. No issue; planned in rtCamp/godam-analytics#364 (item 4).

1. Criteria and proof

Acceptance criterion	Proof
A signed-in viewer's page source has no emailId (before: it carries the viewer's address)	PHP test test_signed_in_viewers_email_is_not_in_the_page_params; capture S01, block 1
The player's page data has no email (before: godamData.currentLoggedInUserData.email holds the address)	PHP test test_player_page_data_for_a_signed_in_viewer_has_no_email; capture S01, block 2
Across 12 front-end pages, the viewer's address appears 0 times in the page source from GoDAM (before: 2 times on each page)	capture S01, block 4
The analytics request body has no email key (before: email holds the viewer's address)	JS test does not send the viewer email, even when the page still carries one; capture S01, block 3
A page cached by an older plugin version, which still carries emailId, does not make the player send it	same JS test (its fixture keeps emailId: 'a@b.test' on purpose)
The comments routes still get the viewer's email on the server	PHP test test_the_server_side_user_data_still_has_the_email
The page and the request keep the user id, site id, token and post details	PHP test test_page_params_keep_what_analytics_still_needs; JS test still sends the WordPress user id
Plays are still recorded and the country still resolves	capture S02

2. Before and after

A signed-in subscriber opens a page with videos on godam-dev in Chrome and plays one for 6 seconds. Before is develop (2.4.0); after is this branch. Captures are verbatim from the browser and from the analytics service's database.

What was read	Before	After
emailId in videoAnalyticsParams	the viewer's address	not present
email in godamData.currentLoggedInUserData	the viewer's address	not present (name and type stay)
email in the play request	the viewer's address	not a key
email stored for the 6 events of the run	the viewer's address, 6 of 6	empty, 6 of 6
The viewer's address in the page source, 12 pages	2 each (3 on the 2 pages with WooCommerce blocks)	0 each (1 on those 2 pages: WooCommerce's own cart data)
Video 203 plays, page loads, country	1, 1, USA 1	1, 1, USA 1

3. Tests

• Added: tests/php/ViewerEmailNotInPageTest.php. It builds the page params with a signed-in user and asserts there is no emailId key and the address appears nowhere in the JSON; that the player's page data has no email but keeps name and type; that the server-side user data still has the email; and that userId, siteId, endpoint, token and postId are still in the page params. Each test runs in its own process, and loads its stubs there, because loading godam-transcoder-functions.php registers hooks and EarlyTranslationHooksTest checks exactly that on first load.
• Added: two tests in assets/src/js/godam-player/analytics-helpers.test.js (no email key even when the page still carries emailId; wp_user_id still sent).
• Added: tests/stubs/template-tags.php, the page-tag stubs the PHP test needs (is_single, get_the_title and similar).
• Changed: tests/stubs/engagement-functions.php. The current-user stub now also returns ID and data->user_email (an empty data when nobody is signed in), as WordPress does. The Engagement tests that use it still pass: 428 PHP tests, 1414 assertions, in normal, reversed and random order.
• Changed: assets/src/js/godam-player/engagement.test.js. Its page-data fixture no longer has an email, matching what the page now prints.
• Revert check: with the fix reverted, test_signed_in_viewers_email_is_not_in_the_page_params fails with "Failed asserting that an array does not have the key 'emailId'", test_player_page_data_for_a_signed_in_viewer_has_no_email fails with "Failed asserting that an array does not have the key 'email'", and the JS test fails with Expected path: not "email", Received value: "a@b.test".

4. Risk flags

No risk flags fired on #2213. Nothing in the diff touched the watched areas.

5. Independent review

A separate review pass saw only the task and the diff, first on the analytics change and then again after the player-data and readme commits.

Finding	Grade	What I did
The stub file was loaded at the top of the test file, so its stand-in functions and the RTGODAM_ANALYTICS_BASE constant reached every other test	Valid	Fixed in 2b88478: loaded inside setUp(), in the separate process. The suite passes in normal, reversed and random order.
Pages already stored by a page cache before the update still contain the email, and browsers or CDNs that cached the old script keep sending email, until those caches expire (raised twice)	Valid, outside this diff	No code change. Listed in section 6. Flushing caches on update is new behaviour and not part of this fix.
Other code that reads or prints the address	Valid question, checked	Found one: godamData.currentLoggedInUserData.email in the player script's page data, seen on godam-dev. Fixed in 5732543. Static search of the source: emailId is read only by the analytics sender and the godam-for-woo add-on; rtgodam_get_current_logged_in_user_data() is called by the page data (now filtered) and by the comments routes, which run on the server. assets/build is not committed. A scan of 12 front-end pages as a signed-in viewer finds 0 copies from GoDAM.
The stub returned a non-empty data even when nobody was signed in, and its header comment overstated what it guards	Valid	Fixed in 2b88478: an empty object for visitors, comment reworded.
The request body now has no email key where it had ''; a required column could reject it	Valid question, checked	Capture S02: the service stored all 6 events with an empty email. The service tested is develop, which per rtCamp/godam-analytics#364 holds the same code as production.
readme.txt lists "Email" among the data sent to analytics	Valid	Fixed in b2281c2: the entry is removed from the External services list for analytics.godam.io. README.md has the same table row; that file is changed by the readme PR of this release.
No test covers the call in class-godam-player.php that builds godamData, and removing one field is a blocklist	Valid, partly	The helper now returns only name and type (240c9ba), so a later field is not printed by accident. No unit test for the call itself: the method registers about 20 scripts and styles and needs the built files, so the stubs would outweigh the check. The scan of 12 pages in S01 covers it on a real site.
Process isolation relies on PHPUnit annotations; the tests assert the stubs' own fixed values	Overstated	composer.json pins PHPUnit to ^9.6, which honours them. The fixed values are the stub's, not the plugin's.

6. Not verified

• Page caches and CDNs: pages cached by the previous version keep the old params, and cached copies of the old script keep sending email, until they expire. Site owners can purge their page cache after updating. Not tested.
• The godam-for-woo add-on reads emailId and sends it as email too. After this PR it reads nothing there, so its Reel Pop events carry an empty email (seen in capture S02, type 4, stored empty). No change is needed in that repo for this PR.
• Plugin versions already installed keep sending email until they update. The analytics service already treats the field as optional; dropping it on the service side is item 4 of rtCamp/godam-analytics#364.
• The captures were taken at 5732543. The later commits change test files and make the helper list its fields (same output, covered by the PHP tests); the site was not re-captured for them.
• Tested as a signed-in subscriber, with Chrome driven by a script. Not tested: a logged-out visitor (the plugin never had an address for them), other browsers, or a site behind a page cache.

7. Scope

Beyond the analytics code: the player's page data lost its email (found when the page source was checked on godam-dev; no script reads it), and one line of readme.txt changed, because the External services text for analytics.godam.io no longer lists Email as data sent (the later items are renumbered). languages/godam.pot is regenerated (5 source line numbers moved, because the new helper sits above those strings in custom-functions.php). wp_user_id is kept for now: replacing it with an opaque id is a larger change and is not part of this PR.

What changed

• rtgodam_get_localize_array() no longer sets emailId from the current user.
• buildAnalyticsRequestBody() no longer reads emailId or sends email.
• New rtgodam_get_current_user_data_for_page() returns the current user's data without the email; the player script's godamData.currentLoggedInUserData uses it. The comments routes keep calling rtgodam_get_current_logged_in_user_data(), which still has the email.

How to check by hand

1. Sign in, open a page that has a GoDAM video, and view the page source. Search for your email address: GoDAM prints it nowhere. videoAnalyticsParams has userId and no emailId.
2. Open DevTools, Network, filter on analytics, play the video. The request payload has no email key.

[github-actions]

🔍 WordPress Plugin Check Report

⚠️ Status: Passed with warnings

📊 Report

🎯 Total Issues	❌ Errors	⚠️ Warnings
15	0	15

---

⚠️ Warnings (15)

📁 inc/classes/class-video-sitemap.php (1 warning)

📍 Line	🔖 Check	💬 Message
620	WordPress.DB.SlowDBQuery.slow_db_query_meta_key	Detected usage of meta_key, possible slow query.

📁 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.

📁 inc/classes/class-video-sitemap-core-provider.php (1 warning)

📍 Line	🔖 Check	💬 Message
73	WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedHooknameFound	Hook names invoked by a theme/plugin should start with the theme/plugin prefix. Found: "wp_sitemaps_index_entry".

📁 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/demo-post-post/ (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-page-post/ (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/demo-post-post/ (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-page-post/ (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 — the privacy change is complete and verified at both ends (page params and request body), and the server-side comments path correctly keeps the email.

I traced every consumer of the removed data:

• emailId is read only by rtgodam_get_localize_array() (removed here) and analytics-helpers.js (removed here) within this repo — no other reader, so the key can be dropped cleanly. The godam-for-woo add-on reads it in its own repo and degrades to an empty email, as the packet notes.
• currentLoggedInUserData.email: engagement.js only destructures currentLoggedInUserData and forwards it as userData; there is no .email access anywhere in assets/src (non-test). So narrowing it to name/type via rtgodam_get_current_user_data_for_page() breaks no UI.
• rtgodam_get_current_logged_in_user_data() still returns email for the REST/comments routes.

Using an allowlist (array_intersect_key on name/type) rather than unsetting email is the right call — a field added later to the base helper won't leak into the page by accident. Good test coverage with the revert check.

Two documentation-level notes, neither blocking:

1. README.md still lists "Email" for analytics — README.md:284

You removed "Email" from the analytics.godam.io data-sent list in readme.txt, but README.md:284 still reads IP, Email, Author, Browser info, OS, User Agent. The packet says README.md is handled by this release's readme PR (#2212) — just flagging the coupling so both land in 2.5.0 and the external-services disclosure stays consistent. (Lines 264/274 legitimately keep Email — those are server-side comment/activation flows.)

2. @since n.e.x.t placeholder — inc/helpers/custom-functions.php

rtgodam_get_current_user_data_for_page() is tagged @since n.e.x.t. That's your existing placeholder convention (also in class-pages.php), so this is just a reminder to resolve it to 2.5.0 before tagging.

Minor

• The class-godam-player.php call site (passing the page helper into godamData) has no unit test; you cover it with the 12-page scan and argue the ~20-script stubbing cost outweighs it. Reasonable — a one-line assertion that the method calls the page helper (not the full one) would cheaply guard against a future revert, if you want belt-and-suspenders.

Thorough packet, especially calling out the page-cache / CDN-cached-script window explicitly.

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

[subodhr258]

@KMchaudhary on your comment:

Thanks Kuldip.

1. README.md:284: fixed in 44b75e9 on Name every screen PostHog runs on in the readme's External Services entry #2212, so README.md drops Email too.
2. @since n.e.x.t: resolved to 2.5.0 at release prep.

Minor: leaving the call-site test out; the 12-page scan covers it.