feat(analytics): Placements — player block_source emit + per-video section - #2041
Conversation
🔍 WordPress Plugin Check Report
📊 Report
|
| 📍 Line | 🔖 Check | 💬 Message |
|---|---|---|
0 |
missing_composer_json_file | The "/vendor" directory using composer exists, but "composer.json" file is missing. |
📁 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. |
📁 tests/bootstrap.php (7 warnings)
| 📍 Line | 🔖 Check | 💬 Message |
|---|---|---|
28 |
WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedFunctionFound | Functions declared in the global namespace by a theme/plugin should start with the theme/plugin prefix. Found: "wp_strip_all_tags". |
48 |
WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedFunctionFound | Functions declared in the global namespace by a theme/plugin should start with the theme/plugin prefix. Found: "wp_prepare_attachment_for_js". |
63 |
WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedFunctionFound | Functions declared in the global namespace by a theme/plugin should start with the theme/plugin prefix. Found: "get_post_meta". |
77 |
WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedFunctionFound | Functions declared in the global namespace by a theme/plugin should start with the theme/plugin prefix. Found: "get_post_mime_type". |
88 |
WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedFunctionFound | Functions declared in the global namespace by a theme/plugin should start with the theme/plugin prefix. Found: "wp_get_attachment_url". |
99 |
WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedFunctionFound | Functions declared in the global namespace by a theme/plugin should start with the theme/plugin prefix. Found: "get_the_title". |
111 |
WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedFunctionFound | Functions declared in the global namespace by a theme/plugin should start with the theme/plugin prefix. Found: "date_i18n". |
📁 assets/build/blocks/godam-gallery-v2/render.php (2 warnings)
| 📍 Line | 🔖 Check | 💬 Message |
|---|---|---|
15 |
WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedVariableFound | Global variables defined by a theme/plugin should start with the theme/plugin prefix. Found: "$inner_block_video_ids". |
23 |
WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedVariableFound | Global variables defined by a theme/plugin should start with the theme/plugin prefix. Found: "$inner_block_video_ids". |
📁 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/07/27/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/07/27/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
There was a problem hiding this comment.
Pull request overview
Note
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.
Adds analytics placement attribution end-to-end (player emits block_source / embed attribution) and surfaces it in the per-video Analytics UI via a new Placements section, plus a Placements count column in Top Videos.
Changes:
- Emit
block_source/host_post_idfrom the player/embed surfaces and thread these into analytics requests (including batched page_load). - Add per-video “Placements” analytics card with date-range scoping, grouping, and derived metrics.
- Enrich placement rows server-side with WP page metadata and expose a “Placements” column in Top Videos.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| pages/dashboard/components/TopVideosTable.js | Adds Placements column and updates empty/loading row colSpans. |
| pages/analytics/Analytics.js | Mounts the new Placements section on the per-video analytics page. |
| pages/analytics/components/Placements/index.js | Implements Placements UI, grouping helpers, derived metrics, and range-scoped query. |
| pages/analytics/components/Placements/placements.test.js | Adds unit coverage for Placements helper functions. |
| pages/analytics/components/DateRangePicker/index.js | Adds shared analytics DateRangePicker component + helper exports. |
| pages/analytics/components/DateRangePicker/index.test.js | Adds unit coverage for DateRangePicker helper functions. |
| pages/analytics/components/DateRangePicker/style.scss | Adds styling for the analytics date-range picker popover/calendar. |
| inc/helpers/custom-functions.php | Adds rtgodam_get_block_source_from_context() and threads new embed params into shortcode generation. |
| inc/templates/godam-player.php | Stamps data-block-source and optional data-host-post-id on the <video> element. |
| inc/templates/video-embed.php | Accepts host_post_id/block_source from iframe URL and passes into embed content generation. |
| inc/classes/shortcodes/class-godam-player.php | Adds shortcode attrs block_source and host_post_id. |
| inc/classes/rest-api/class-analytics.php | Enriches microservice placement rows with WP title/permalink/edit_url and deletion status. |
| assets/src/js/godam-player/analytics.js | Threads block_source/hostPostId through type=1/2/3 analytics paths (including batch/flush). |
| assets/src/js/godam-player/analytics-helpers.js | Adds block_source + host_post_id override logic to request body builder. |
| assets/src/js/godam-player/video-analytics-plugin.js | Passes new attribution fields into buildAnalyticsRequestBody. |
| assets/src/blocks/godam-gallery-v2/view.js | Appends host_post_id and block_source=video-gallery to embed iframe URL for attribution. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…acements section godam side of Placements (godam-plugin-wp#22), consuming godam-analytics#237. Player emit: - rtgodam_get_block_source_from_context() maps godam_context to the placement slug (shoppable-video / wc-product-gallery / product-reels / reel-pop; empty = video-block; unknown passes through). The player template stamps data-block-source (always) and data-host-post-id (embeds only) on the <video>; an explicit block_source shortcode att overrides. - analytics.js sends [videoId, jobId, blockSource] triples in the type=1 batch and block_source on every type=2 path (track + keepalive teardown); type=3 layer flushes carry it best-effort. - Embed-iframe attribution: gallery view.js appends host_post_id + block_source=video-gallery to the video-embed iframe URL; video-embed.php sanitizes and threads them as shortcode atts; buildAnalyticsRequestBody overrides body.post_id with hostPostId (>0), so gallery plays attribute to the hosting page, not the embed page. Placements UI (per-video analytics): - New Placements card: block-source sections (auto-select first) with per-page rows - "Displayed on" title/permalink, Edit Page (hidden without edit cap; "Deleted page" fallback), Views / Plays / Play Rate / Avg Watch Time (derived client-side from raw primitives). Own DateRangePicker (dedup via conditional-spread args). Loading / error / collecting-data states; godam-placements-* test ids. Mounted below "Views across the video". - Proxy: fetch_analytics_data enriches microservice placements rows with title/permalink/edit_url/is_deleted (capped 100 lookups); leaves the key absent for old microservices. - Top Videos: "Placements" column (placements_count). Tests: Placements helpers (17) + vendored picker suite (8) - 25 passed. Lint + PHPCS clean. Depends on #2028 for live date-range params (until it merges, non-All-Time picks refetch all-time data); the vendored picker commit is dropped on rebase after #2028 lands.
4977663 to
82502b5
Compare
…attribution
- Untrusted block_source as an object key (security): a value like 'toString',
'valueOf' or 'constructor' resolved to an inherited Object.prototype member,
making the label lookup truthy and the bucket a function, so
groupPlacementsByBlockSource threw a TypeError inside the card's useMemo and
white-screened the whole per-video Analytics page. The value is reachable by
an anonymous caller (the embed page takes block_source from a query arg and
the microservice normalizes but never rejects it). Buckets are now a
null-prototype map and both the grouping and the label lookup use an
own-property check. +13 jest cases covering those exact key names.
- Trashed pages: WordPress maps edit_post on a trashed post to its pre-trash
status, so current_user_can passed and a trashed page rendered as a live row
with an Edit link that wp-admin refuses to open (HTTP 409). Trashed posts now
route to the unavailable state regardless of the edit capability.
- Unavailable rows keep their ID ("Post #12 (unavailable)") so several of them
stay distinguishable instead of repeating one constant string.
- Host-post attribution: hostPostId was read once per page and applied to every
batched type=1 entry, so a single host-stamped player re-attributed every
other video's page_load on that page. It is now carried per queue entry and
the flush groups by it (post_id is one top-level field per request); the
type=3 flush reads it from that video's own element.
- video-embed.php uses mb_substr for the block_source cap: a byte-wise cut
could split a multibyte character, after which esc_attr() dropped the value
entirely.
- enrich_placements primes the post cache in one query instead of issuing up to
100 uncached get_post() calls per request on a public endpoint.
- Empty state distinguishes "no placements in this date range" (with a "View all
time" reset) from "collecting placement data", instead of telling the user the
feature has not started collecting whenever they narrow the range.
Tests: 57 passed (Placements helpers now 33). ESLint clean, PHPCS exit 0.
Known gap: the emit path (buildAnalyticsRequestBody / queue grouping) still has
no unit test; tracked for a follow-up.
Closes the test gap the re-review flagged: the attribution logic had no unit
coverage even though the UI helpers were fully covered.
- Moved the pure `groupBatchByHostPostId` from analytics.js into
analytics-helpers.js (the shared, importable module — analytics.js has
top-level DOM/library side effects, so it is not directly testable).
- New analytics-helpers.test.js (19 cases): block_source defaults to '' and
passes through verbatim; post_id is overridden only when hostPostId parses
> 0 and is left untouched for 0 / null / undefined / negative / non-numeric;
numeric strings coerce; type=1 video_ids triples pass through unchanged while
other types send none; both bail conditions return { endpoint: null,
body: null }.
- Grouping is pinned by the regression that motivated it: entries with
different host attributions land in separate groups, so one host-stamped
player can no longer drag the rest of the page's videos into its post_id
override. Also covers wire-triple shape and non-positive/unparseable ids.
77 tests passing across the analytics areas. ESLint clean.
…note The card now shows only real, labelled placements. Anything it cannot attribute to a surface — an empty block_source (unattributed) or a slug with no label — is dropped rather than collected into an "Other" section: those plays are already reported in the video's overall metrics above the card, so repeating them here would double-report them. - groupPlacementsByBlockSource drops unlabelled rows; the 'other' key is gone from SECTION_ORDER and getBlockSourceLabel no longer has an "Other" fallback (it returns '' for anything unknown). - Removed the "Placement analytics starts collecting after this update. Data appears as new plays come in." note; the no-data state is now just "No placement data for this video yet". The range-scoped empty state (with its "View all time" reset) is unchanged. Hostile block_source values are still safe: the null-prototype bucket map and own-property label check remain, and those rows are now dropped instead of bucketed. Tests updated accordingly (they assert dropping, and still assert no throw for Object.prototype key names). The microservice also excludes unattributed rows at the read layer, so the per-video sections and the dashboard's Placements count agree. 76 jest tests passing. ESLint clean.
…slugs
Round-3 review (MEDIUM, confirmed): the previous fix conflated two different
things under "don't show unattributed" -- the truly empty block_source
(unattributed, no surface at all) and a non-empty slug this build simply has
no friendly label for yet. Only the former should be hidden (it's already
reported in the video's overall metrics above the card); the latter is a
real, attributed placement, and dropping it made the card disagree with the
dashboard's placements_count, which only excludes the empty case -- a real
placement became invisible with no UI trace.
groupPlacementsByBlockSource now drops only rows whose block_source is empty.
A non-empty unmapped slug gets its own section, labelled with the raw slug,
sunk after the known SECTION_ORDER sections (sorted by row count). The
null-prototype bucket map keeps this safe for hostile keys ('toString' etc.)
-- they're now kept as ordinary sections instead of dropped, since they are
just as real (or fake) as any other unmapped slug and are only ever rendered
as text.
Tests updated to match: unmapped-but-real slugs are asserted present with the
slug as their label; hostile-key tests now assert they're kept, not dropped.
33 jest passing. ESLint clean; build green.
formatWatchTime floors to whole seconds, so a real but sub-second Avg Watch Time (now correctly computed after the analytics#237 page-load-scoping fix) rendered as a bare "0s" -- indistinguishable from no watch time at all. Show "<1s" instead when the average is positive but under a second.
Placement attribution is forward-only (block_source is captured at ingestion, no historical backfill), so the per-placement breakdown can legitimately show less than the video's all-time totals in the Insights above it. Adds a small footnote clarifying that, shown whenever the card has resolved (data or the empty state) -- not during loading/error, where it would just add noise.
…ider, less below the text
…zation - Placements row key drops `index`: (post_id, block_source) is already unique per row (the microservice aggregates by that exact pair), so including index bought nothing and could cause needless remounts when a range change re-sorts rows by plays desc. - Auto-select effect reads the previous selectedKey via the functional setSelectedKey(prev => ...) updater instead of closing over the outer variable, so it only needs `sections` as a dependency -- removes the eslint-disable-next-line react-hooks/exhaustive-deps with no behavior change (verified live: section switching still works). - godam-player.php now caps block_source the same way video-embed.php already does (mb_substr(...,100,'UTF-8') after sanitize_text_field) instead of only sanitizing with no length cap -- block_source is ultimately user-controlled (threaded from a public GET param on the embed page into this shortcode attribute), so both paths should bound it identically. Note: 4 additional Copilot comments on this PR (DateRangePicker DST/off-by-one and month-grid issues) are about code from already-merged PR #2028, inherited via the rebase onto develop -- confirmed 0 lines of DateRangePicker/index.js are in this PR's diff (git diff origin/develop...HEAD). Left untouched; belongs to a follow-up against develop, not this PR. 33 jest passing, PHPCS clean, verified live in Chrome (section switching intact).
Code review — Placements (#2041 + godam-for-woo#184, reviewed together)Reviewed both halves of the feature end-to-end (emit → embed threading → proxy enrichment → render). Overall it's carefully built and well-tested — the null-prototype bucket map, per-entry Security / info-disclosure
Correctness
Cleanup / altitude
Deploy-order note (not a code bug)Both PRs convert all |
…cements
Human review (both PRs reviewed together): enrich_placements gated title/
permalink for private/draft/pending/trashed pages, but the row's raw post_id
and engagement metrics (views/plays/page_load/play_time) still passed through
unchanged on the public `/analytics/fetch` route (permission_callback =>
__return_true). An anonymous caller could enumerate the private/draft page
IDs a video is placed on, plus their play counts.
Split the "post exists but this caller can't see it" case from "post_id
doesn't resolve to anything real":
- Real-but-inaccessible post: post_id -> 0, title -> generic "Unavailable"
(no ID), views/plays/page_load/play_time -> 0. Every such row is now
intentionally identical, so nothing distinguishes one hidden page from
another.
- Truly nonexistent post_id: unchanged behavior (numbered "Post #%d (deleted)"
label + real metrics) -- there's no real, currently-existing resource being
described, so nothing sensitive is exposed.
FE: since redacted rows now share post_id=0, the row key falls back to
`redacted-${block_source}-${index}` for that case only (avoids a duplicate-key
collision when a section has more than one hidden page); the normal case is
unchanged.
Also, from the same review: godam-player.php now caps block_source length
(mb_substr, 100 chars) instead of only sanitizing with no length cap, matching
video-embed.php's existing cap for the same user-controlled value.
Verified live: flipped a real placement's host post to private and confirmed
an anonymous /analytics/fetch call now returns post_id 0 / all-zero metrics
for it, while unrelated public placements are untouched; restored the post
afterward. 77 jest passing, PHPCS clean.
Noted but out of scope (flagged separately): the same review found the
sibling post_details/post_views enrichment block in this file leaks real
titles+permalinks for ALL posts with no viewability/capability gate at all --
confirmed pre-existing (git blame predates this branch entirely), not part of
this PR's diff. Also noted: reusing the existing calculatePlayRate() helper
instead of a parallel getPlayRate() with swapped arg order, and consolidating
4 copy-pasted host-post-id parsing call sites -- both low-risk cleanups,
deferred to keep this fix focused on the security issue.
What
godam side of Placements (rtCamp/godam-plugin-wp#22): the player emits
block_sourceon analytics events, and the per-video Analytics page gains a Placements section. Consumes rtCamp/godam-analytics#237.Player emit
rtgodam_get_block_source_from_context()maps the existinggodam_contextto the placement slug (shoppable-video/wc-product-gallery/product-reels/reel-pop; empty →video-block; unknown passes through). One mapper, no per-surface instrumentation.data-block-source(always) +data-host-post-id(embeds only) on the<video>; explicitblock_source/host_post_idshortcode atts override (used by the embed page).analytics.js: type=1 batch entries are now[videoId, jobId, blockSource]triples; every type=2 path (track + keepalive teardown) carriesblock_source; type=3 flushes best-effort.view.jsappendshost_post_id+block_source=video-galleryto thevideo-embediframe URL;video-embed.phpsanitizes + threads them;buildAnalyticsRequestBodyoverridespost_idwithhostPostId(>0) so lightbox plays attribute to the hosting page.Placements section (per-video analytics)
DateRangePicker(cache-key dedup via conditional spread), loading/error/empty states.fetch_analytics_dataaddstitle/permalink/edit_url/is_deletedper row (≤100 lookups); key stays absent on old microservices.placements_count).Testing
/process-analytics, open the video's analytics → sections group by surface, gallery plays attribute to the host page; check Top Videos "Placements" column.Automation impact
New test ids:
godam-placements-section,godam-placements-list-item-<block_source>,godam-placements-row,godam-placements-daterange-toggle.Deploy order
godam-analytics#237 first (store) → this PR (emit + consume) → godam-for-woo#184 (Reel Pop tuple).
Rebased onto
developafter #2028 merged, so the date-range plumbing is inherited (no vendored copy) and the card's picker is live end to end:startDate/endDate→useFetchAnalyticsDataQuery→start_date/end_date→ microservice range-sums the placements. Verified against the dev site: a past window returns 0 placements, today returns 4.Review fixes
Two adversarial review rounds (8 dimensions, 3-lens refutation per finding).
Round 1
enrich_placementsgates onis_post_publicly_viewable()+current_user_can('edit_post'), so the public/analytics/fetchroute no longer leaks private/draft/pending titles or permalinks to anonymous callers. Verified live: a private page returns "unavailable"/no permalink for anon, and the real title + Edit link for a capable admin.Post #<id>fallback (PHP) plus agetPlacementTitle()guard (JS), so no row renders nameless.Round 2
block_sourceas an object key (MED, security). Values liketoString,valueOforconstructorresolved to inheritedObject.prototypemembers, making the label lookup truthy and the bucket a function — sogroupPlacementsByBlockSourcethrew aTypeErrorinside the card'suseMemoand white-screened the whole per-video Analytics page. Reachable by an anonymous caller (the embed page takesblock_sourcefrom a query arg; the microservice normalizes but never rejects it). Buckets are now a null-prototype map and both the grouping and the label lookup use an own-property check.edit_poston a trashed post to its pre-trash status, socurrent_user_canpassed and a trashed page rendered as a live row with an Edit link thatwp-admin/post.phprefuses to open (HTTP 409). Trashed posts now route to the unavailable state regardless of the edit capability.hostPostIdwas read once per page and applied to every batched type=1 entry, so a single host-stamped player re-attributed every other video's page_load on that page. It is carried per queue entry now and the flush groups by it (post_idis one top-level field per request); the type=3 flush reads it from that video's own element.Post #12 (unavailable)) so several of them stay distinguishable.video-embed.phpusesmb_substrfor theblock_sourcecap — a byte-wise cut could split a multibyte character, after whichesc_attr()dropped the value entirely.enrich_placementsprimes the post cache in one query instead of up to 100 uncachedget_post()calls per request on a public endpoint.Tests: 76 passed (Placements helpers 33 incl. 13 hostile-key cases; emit-path helpers 19). ESLint clean; PHPCS exit 0; husky
lint:php+lint:jspass on the staged diff.Emit path coverage:
groupBatchByHostPostIdmoved intoanalytics-helpers.js(analytics.js has top-level side effects, so it is not importable in jest) and covered by a new 19-case spec —block_sourcedefaulting/passthrough,post_idoverridden only whenhostPostIdparses > 0 (untouched for 0/null/undefined/negative/non-numeric), triple passthrough per event type, both bail conditions, and the grouping regression itself: entries with different host attributions land in separate requests.Automation impact
Test ids:
godam-placements-section,godam-placements-list-item-<block_source>,godam-placements-row,godam-placements-daterange-toggle,godam-placements-reset-range.Deploy order
godam-analytics#237 first (store; run its alembic migration before the app restarts) → this PR (emit + consume) → godam-for-woo#184 (Reel Pop tuple + WC gallery attribution).
No catch-all section, no historical-data note
The card shows only real, labelled placements. Anything it cannot attribute to a surface — an empty
block_source(unattributed) or a slug with no label — is dropped rather than collected into an "Other" section: those plays are already reported in the video's overall metrics above the card, so repeating them here would double-report them.groupPlacementsByBlockSourcedrops unlabelled rows; the'other'key is gone fromSECTION_ORDER, andgetBlockSourceLabelreturns''for anything unknown instead of falling back to "Other".block_sourcevalues stay safe: the null-prototype bucket map and own-property label check remain, and such rows are now dropped instead of bucketed (tests still assert no throw forObject.prototypekey names).The microservice excludes unattributed rows at the read layer too, so the per-video sections and the dashboard's Placements count agree.
Demo Recording
https://app.godam.io/web/video/6821bb3iu9
Added a note at the bottom: