Skip to content

Name every screen PostHog runs on in the readme's External Services entry - #2212

Open
subodhr258 wants to merge 3 commits into
developfrom
fix/readme-posthog-screens
Open

subodhr258 wants to merge 3 commits into
developfrom
fix/readme-posthog-screens

Conversation

@subodhr258

@subodhr258 subodhr258 commented Oct 19, 2026 •

Copy link
Copy Markdown
Collaborator

Review packet

In one line: Updates the PostHog entry in the readme's External Services section so it names every screen PostHog runs on once an admin allows it, not only GoDAM's own screens. Fixes no issue.

1. Criteria and proof

Acceptance criterion Proof
The entry names every screen where PostHog loads New Purpose line names GoDAM's own screens, the Media Library and Edit Media screens, the editors for posts, pages and other post types, the site editor and the widgets screen. In code: godam_is_media_library_screen() (inc/helpers/custom-functions.php) lists the screen bases upload, post, page, attachment, widgets and site-editor, and admin_enqueue_scripts() in inc/classes/class-pages.php gives the media-library-react bundle posthogConfig on exactly those screens
The entry names no screen where PostHog does not load The WordPress Dashboard (where the Allow notice shows), the posts list and the front end are not named. None of them loads the bundle
"When data is sent" no longer says GoDAM's screens only Item 2 now reads "Only while someone is using one of these screens in the WordPress admin. It is never sent for visitors to your site." posthogConfig is only set inside admin_enqueue_scripts
Every other statement in the entry is still true Checked against the code, see "Statements checked" below
It says the same as the godam.io privacy policy Matches the screen list in rtCamp/godam.io#1033 (the PR that adds the PostHog section to the privacy policy), and is slightly wider where the code is wider (see section 6)

2. Before and after

Nothing changes on screen in the plugin. The listing changes when the plugin release deploys readme.txt to wordpress.org.

Before After
Purpose "...product analytics for GoDAM's own screens in the WordPress admin." "...product analytics for GoDAM's own screens in the WordPress admin, and for the screens where GoDAM adds its media tools: the Media Library and Edit Media screens, the editors for posts, pages and other post types, the site editor, and the widgets screen."
Data sent, item 2 How GoDAM's admin screens are used: page views, clicks, and errors How these screens are used: page views, clicks, and errors
Data sent, item 3 Session replays and heatmaps of GoDAM's admin screens Session replays and heatmaps of these screens. They cover the whole screen, not only GoDAM's parts of it
Data sent, item 5 The IP address and browser details that every web request carries The IP address, and details about the browser and device, such as the browser, operating system, screen size, time zone, and the page the admin came from
When data is sent, item 2 Only while someone is using GoDAM's screens in the WordPress admin. It is never sent for visitors to your site. Only while someone is using one of these screens in the WordPress admin. It is never sent for visitors to your site.

Unchanged: the list of site details (item 1), the address and browser-stored ID (item 4), the opt-in rule, how to turn it off and the note about us-assets.i.posthog.com.

3. Tests

  • Added: none. This is a text-only change.
  • Changed: none.
  • Revert check: not applicable to text. Before this change the entry named GoDAM's own screens only, while pages/utils/posthog.js starts PostHog as soon as the media-library-react bundle loads on any of the screens above.

4. Risk flags

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

5. Independent review

A separate review pass saw only the task and git diff origin/develop...HEAD, read the code and returned the findings below. I checked each against the code.

Finding Grade What I did
"The post and page editors" names less than the code covers. The gate is the screen base post, which is every post type (products and other custom post types), and attachment (Edit Media) is in the list too Valid Reworded to "the Media Library and Edit Media screens, the editors for posts, pages and other post types"
"Session replays and heatmaps of these screens" is literally true but could be read as GoDAM's parts only. PostHog is started for the whole page, so it covers post text and file names on the editor and Media Library screens Valid Added "They cover the whole screen, not only GoDAM's parts of it". I did not say anything about masking: the plugin sets no recording options, so that depends on the PostHog project settings, which the code does not show
"The IP address and browser details that every web request carries" understates what the PostHog script adds in the browser (screen size, time zone, referrer) Valid Reworded. Checked in the posthog-js 1.360.0 source: the default properties include $browser, $os, $screen_width, $timezone and $referrer
The us-assets.i.posthog.com sentence cannot be checked from plugin code Valid, library behaviour Checked in the posthog-js 1.360.0 source: for a us.i.posthog.com api host it loads its remote config (/array/<key>/config) and scripts from us-assets.i.posthog.com. Left unchanged
No contradiction found in: the Allow notice and the Help toggle (both need manage_options and set both flags), the opt-in default off, "connecting an API key does not switch it on" (only the notice handler, the settings REST route and the 2.4.0 consent-reset migration write the setting), "never sent for visitors", "stops from the next page load" Verified No change

Also found: the reviewer grepped the Elementor editor and found no PostHog load there; I left Elementor out of the entry (see section 6).

6. Not verified

  • I did not run the plugin or a browser, so I did not watch PostHog load on each screen. The screen list comes from reading the enqueue code. The PHP tests for the opt-in (tests/php/PostHog*Test.php) were not run (no code changed).
  • Whether session replay is on, and what it masks, is a PostHog project setting. The code does not show it, so the entry says what is sent and does not say what is hidden.
  • The Elementor editor: read from the source of Elementor 4.2.1 (the version on the test site), not run. PostHog's setup is added only in admin_enqueue_scripts (inc/classes/class-pages.php). Elementor's editor builds its own page from wp_head and wp_footer and never fires admin_enqueue_scripts, and GoDAM's elementor/editor/before_enqueue_scripts hook enqueues only the widget scripts. So PostHog does not load there, and the entry does not name Elementor. WPBakery's backend editor runs inside the normal post screen, so PostHog does load there, and "the editors for posts, pages and other post types" covers it. The comment above godam_is_media_library_screen() says base post/page also covers the Elementor editor; that is not right for Elementor and is not in a file this PR owns.
  • The godam_should_load_media_library_assets filter lets site code add screens. Nothing in this repo uses it, so the entry does not mention it.
  • I did not run the wordpress.org readme validator. The change is wording inside an existing entry.
  • rtCamp/godam.io#1033 (the privacy policy change) says "the post and page editors" and does not name Edit Media. The code covers every post type and Edit Media, so that line should be widened to match this readme before it is published.

7. Scope

readme.txt: the PostHog entry of == External Services ==, and nothing else in that file. No changelog line and no version change (the version-bump PR writes the changelog). README.md: one new row in its External Services table that mirrors the PostHog entry (the table had no PostHog row). The row that listed Email as sent to analytics.godam.io was fixed separately in 44b75e9, which is also on this branch.

After an admin allows it, PostHog also loads on the Media Library and Edit Media screens, the editors for every post type, the site editor and the widgets screen, not only on GoDAM's own admin screens. The entry now says so, says the replays cover the whole screen, and lists the browser and device details PostHog's code collects.
@github-actions

github-actions Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

🔍 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/21/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/21/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

Copy link
Copy Markdown
Collaborator

Code review

Verdict: LGTM — the reworded PostHog entry is accurate. I checked the screen list against the code and it maps exactly to the enqueue gate:

readme says $media_bases in godam_is_media_library_screen()
Media Library upload
Edit Media attachment
editors for posts, pages and other post types post, page
site editor site-editor
widgets screen widgets
GoDAM's own screens the rtgodam menu screen ids

Good calls on widening "post and page editors" to "other post types" (base post is every post type) and on adding "they cover the whole screen, not only GoDAM's parts" to item 3 — that's the honest disclosure given PostHog is started for the whole page, and the item-5 expansion (OS, screen size, time zone, referrer) matches posthog-js's default properties.

Two things worth a look, neither blocking the wording:

1. Elementor/WPBakery: the code comment and your testing disagree

godam_is_media_library_screen() carries a comment that base post/page "also cover the Elementor and WPBakery editors, which run on post.php" — i.e. PostHog would load there and, per the new item 3, record full-screen replays/heatmaps of the page-builder canvas. Your review instead found no PostHog load on the installed Elementor and left it unnamed. The readme's generic "editors for posts, pages and other post types" covers it either way, so the text is safe — but since this is a privacy-disclosure point, it's worth confirming empirically which is true and fixing whichever of the comment or the behaviour is wrong.

2. Who owns the README.md analytics "Email" removal?

Separate from PostHog, but related: PR #2213 removes "Email" from readme.txt's analytics.godam.io list and says README.md "is changed by the readme PR of this release." This PR only touches the PostHog entry, so README.md:284 would still list IP, Email, Author, … for analytics.godam.io after both merge. Worth confirming which PR actually removes it so it doesn't fall through.

Note

  • Per your section 6: the godam.io privacy policy (rtCamp/godam.io#1033) says "the post and page editors" and omits Edit Media, while the code is wider. Widening that to match this readme before publishing keeps the two consistent.

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

The page no longer sends the signed-in viewer's email (#2213), and readme.txt
already dropped it; README.md still listed it.

Addresses review: #2213 (comment)
Addresses review: #2212 (comment)
@subodhr258

Copy link
Copy Markdown
Collaborator Author

@KMchaudhary on your comment:

Thanks Kuldip.

  1. Elementor and WPBakery: agreed the readme wording is safe either way. The code comment and the test result disagree, so I'll confirm on a test site whether PostHog loads in each builder's editor and fix whichever is wrong in a follow-up. No change in this PR.
  2. README.md:284: fixed in 44b75e9. Email is gone from the analytics.godam.io row, so README.md now matches readme.txt after Stop sending the signed-in viewer's email with video analytics #2213.
  3. Privacy policy: noted, rtCamp/godam.io#1033 will be widened to match this list.

README.md had no PostHog row. The table now lists it the way readme.txt does.
@subodhr258

Copy link
Copy Markdown
Collaborator Author

Thanks for checking both. I looked at each against the code.

1. Elementor and WPBakery. You are right that the comment above godam_is_media_library_screen() and my test result disagree, and the comment is the one that is wrong for Elementor. PostHog's setup (posthogConfig and the media-library-react bundle) is added only in admin_enqueue_scripts (inc/classes/class-pages.php). I read the source of Elementor 4.2.1, the version on the test site, and its editor builds its own page from wp_head and wp_footer: it removes the other hooks and never fires admin_enqueue_scripts. GoDAM's own elementor/editor/before_enqueue_scripts hook (inc/classes/class-elementor-widgets.php) enqueues only the widget scripts. So PostHog does not load in the Elementor editor, and the entry does not name it. WPBakery's backend editor runs inside the normal post screen, so PostHog does load there, and "the editors for posts, pages and other post types" covers it. I read the source and did not run Elementor, so a run on a throwaway site would settle it; I did not do that. I left the readme wording as it is. The wrong comment is in inc/helpers/custom-functions.php, which this PR does not touch, so it needs a one-line code fix in a separate PR.

2. README.md. That one was meant for the readme PRs: #2213 left the README.md table to them. The "Email" removal from the analytics.godam.io row is already on this branch (44b75e9). README.md had no PostHog row at all, so I added one that mirrors the readme.txt entry (e786ec0). If you would rather keep this PR to readme.txt, that commit can be dropped, but then the README.md PostHog row needs another owner.

Also noted in the description: rtCamp/godam.io#1033 says "the post and page editors" and does not name Edit Media, so it should be widened to match this readme before it is published.

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.

2 participants