feat(dashboard): Top Products + Video-to-Cart Woo analytics (Phase 1) - #2074
feat(dashboard): Top Products + Video-to-Cart Woo analytics (Phase 1)#2074subodhr258 wants to merge 72 commits into
Conversation
Adds the /analytics/top-products route + fetch_top_products handler, mirroring fetch_top_videos: forwards page/limit/site_url/sort_by/order (+ date range) to the microservice /dashboard/top-products/, POSTing a product_ids include-filter resolved from a product-name search (resolve_top_products_id_filter, WP-side) or a plain GET otherwise. Each returned row is hydrated with the product's current name, permalink and thumbnail from product_id via wc_get_product (display fields are not stored in the service, so renames/reprices reflect immediately); missing products come back exists:false. Registered in the range-routes list so start_date/end_date apply. Gated to upload_files like top-videos. PHPCS clean.
fetchTopProducts query (siteUrl/page/limit/search/sort/range) added to the dashboard RTK api, and a TopProductsTable component cloned from TopVideosTable: one row per product with the WooCommerce name/image/link, a Source-chips column, Product Views with its click-through rate, the direct/assisted Add-to-Cart split (count + rate, with the split as a sub-line and a tooltip), and a Revenue column marked Phase 2. Reuses the shared search (debounced), date-range picker, pagination, skeleton, empty-state, and CSV export. data-test-ids throughout. ESLint clean.
Adds a segmented Top Videos / Top Products switcher above the table, shown only when
WooCommerce is active (new videoData.isWoo flag = class_exists('WooCommerce')). Non-Woo
sites keep the plain Top Videos table with no switcher. ESLint + PHPCS clean; the JS
bundle builds without errors.
Minimal styling for the new .godam-top-tabs switcher (segmented control, active tab uses the WP admin accent) and the .godam-source-chips / .godam-chip pills in Top Products, plus a muted pill variant. Reuses --wp-admin-theme-color. Stylelint clean; JS+SCSS bundle builds without errors.
There was a problem hiding this comment.
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.
Adds a WooCommerce-gated Top Products view to the dashboard analytics, backed by a new REST proxy endpoint that forwards queries to the analytics microservice and hydrates rows with WooCommerce product display data.
Changes:
- Added RTK Query endpoint/hooks and a new
TopProductsTablecomponent with search, date range, pagination, and CSV export. - Introduced a Woo-only segmented switcher between Top Videos and Top Products on the dashboard.
- Implemented a new WP REST proxy route (
/analytics/top-products) that forwards requests to the microservice and hydrates products viawc_get_product.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| pages/dashboard/redux/api/dashboardAnalyticsApi.js | Adds fetchTopProducts RTK Query endpoint and hooks used by the dashboard UI. |
| pages/dashboard/index.scss | Adds styling for the Top Videos/Top Products switcher and source “chip” UI. |
| pages/dashboard/components/TopProductsTable.js | New table component for Top Products including search, date range filtering, pagination, and CSV export. |
| pages/dashboard/Dashboard.js | Adds WooCommerce-gated segmented tab switcher and renders TopProductsTable. |
| inc/classes/rest-api/class-analytics.php | Adds /top-products REST route and proxy handler that hydrates rows with WooCommerce product data. |
| inc/classes/class-pages.php | Exposes window.videoData.isWoo to gate the tab UI to WooCommerce sites. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| const handleExportCSV = async () => { | ||
| setIsExporting( true ); | ||
|
|
||
| const pageCount = Math.max( 1, Math.ceil( ( totalItems || products.length ) / EXPORT_PAGE_SIZE ) ); | ||
| const results = await Promise.all( | ||
| Array.from( { length: pageCount }, ( _, i ) => | ||
| fetchForExport( { | ||
| siteUrl, | ||
| page: i + 1, | ||
| limit: EXPORT_PAGE_SIZE, | ||
| search, | ||
| startDate: dateRange.startDate, | ||
| endDate: dateRange.endDate, | ||
| } ).unwrap().catch( () => ( { products: [] } ) ), | ||
| ), | ||
| ); | ||
| const fetched = results.flatMap( ( result ) => result?.products || [] ); | ||
| const exportProducts = fetched.length ? fetched : products; |
There was a problem hiding this comment.
Already handled at the current head, no change needed. handleExportCSV pages with a fixed pool of 4 workers (CONCURRENCY = 4) pulling from a shared cursor, so at most 4 /top-products requests are in flight regardless of catalog size, and failed pages are counted and surfaced to the user.
| const handleExportCSV = async () => { | ||
| setIsExporting( true ); | ||
|
|
||
| const pageCount = Math.max( 1, Math.ceil( ( totalItems || products.length ) / EXPORT_PAGE_SIZE ) ); | ||
| const results = await Promise.all( | ||
| Array.from( { length: pageCount }, ( _, i ) => | ||
| fetchForExport( { | ||
| siteUrl, | ||
| page: i + 1, | ||
| limit: EXPORT_PAGE_SIZE, | ||
| search, | ||
| startDate: dateRange.startDate, | ||
| endDate: dateRange.endDate, | ||
| } ).unwrap().catch( () => ( { products: [] } ) ), | ||
| ), | ||
| ); |
There was a problem hiding this comment.
Already handled, no change needed. handleExportCSV wraps the whole flow (fetches, Blob/URL creation, DOM ops) in try/finally with setIsExporting(false) in the finally, so the button recovers on any throw.
| <div className="godam-top-tabs__nav" role="tablist" aria-label={ __( 'Top content', 'godam' ) }> | ||
| <button | ||
| type="button" | ||
| role="tab" | ||
| aria-selected={ topTab === 'videos' } | ||
| className={ `godam-top-tabs__tab${ topTab === 'videos' ? ' is-active' : '' }` } | ||
| data-test-id="godam-top-tab-videos" | ||
| onClick={ () => setTopTab( 'videos' ) } | ||
| > | ||
| { __( 'Top Videos', 'godam' ) } | ||
| </button> | ||
| <button | ||
| type="button" | ||
| role="tab" | ||
| aria-selected={ topTab === 'products' } | ||
| className={ `godam-top-tabs__tab${ topTab === 'products' ? ' is-active' : '' }` } | ||
| data-test-id="godam-top-tab-products" | ||
| onClick={ () => setTopTab( 'products' ) } | ||
| > | ||
| { __( 'Top Products', 'godam' ) } | ||
| </button> | ||
| </div> | ||
| { topTab === 'videos' | ||
| ? <TopVideosTable siteUrl={ siteUrl } skip={ shouldSkipSecondaryQueries } /> | ||
| : <TopProductsTable siteUrl={ siteUrl } skip={ shouldSkipSecondaryQueries } /> } |
There was a problem hiding this comment.
Fixed in b7aec61 (same change as the line-190 thread). The switcher is now plain buttons with aria-pressed instead of a tablist/tab with no tabpanel.
| if ( is_wp_error( $response ) ) { | ||
| return new WP_REST_Response( | ||
| array( | ||
| 'status' => 'error', | ||
| 'message' => $response->get_error_message(), | ||
| ), | ||
| 500 | ||
| ); | ||
| } | ||
|
|
||
| $body = json_decode( wp_remote_retrieve_body( $response ), true ); | ||
| $top_products = $body['top_products'] ?? array(); |
There was a problem hiding this comment.
Fixed in b4122ad. The JSON-validity half is now handled in both fetch_top_videos and fetch_top_products: the guard is 200 !== $http_code || ! is_array( $body ), matching fetch_placement_funnels, so a 200 with an HTML or unparseable body returns status:error instead of an empty success. See the two line-specific threads.
| if ( ! empty( $sort_by ) ) { | ||
| $query['sort_by'] = $sort_by; | ||
| } | ||
| if ( ! empty( $order ) ) { | ||
| $query['order'] = $order; | ||
| } |
There was a problem hiding this comment.
Already handled, no change needed. The top-products route registers sort_by and order with an enum plus rest_validate_request_arg (sort_by in product_views|add_to_cart|impressions|ctr, order in asc|desc), so an out-of-set value 400s at the proxy before the callback runs.
…Revenue/Phase-2 column Visual review of the rendered dashboard found three issues, all fixed: - The tab switcher now renders inside the active table's head (passed as tabSwitcher), so there is no longer a second 'Top Products' heading below the tabs. - Removed the Revenue column and its 'Phase 2' badge from the UI entirely; 'Phase 2' is a planning label and must never appear on screen. Revenue lands when Phase 2 does. - Non-Woo sites still fall back to the plain Top Videos heading (tabSwitcher null). ESLint + Stylelint clean; bundle builds.
…ent blue) Active tab is now a white pill with dark #1e1e1e text (was the admin accent, which rendered blue/pink), inactive is grey; larger padding/size to match the design.
🔍 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. |
📁 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/09/09/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/09/09/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
Per review: keep both Add to Cart and Revenue columns. Add to Cart shows the in-video (Direct) count now from backfill, with the Assisted split filling in when Mayank's type=6 capture ships. Revenue is a placeholder column until Phase 2, with no 'Phase 2' label in the UI (that is a planning term, not for the screen).
The product-name cell fell back to a hardcoded English `Product ID: N` when a product had no resolved title. Wrap it in sprintf( __() ) with a translator comment, matching every other string in the component.
… added in-video Per review: variable, grouped and external products can't be added to cart from inside a video (Store API only supports simple products), so their in-video (Direct) count is structurally 0. Rather than a bare 0 that reads as 'no adds', the proxy now returns supports_direct_add_to_cart and the table greys the in-video figure with a hover helper explaining it converts on the product page (Assisted). Nothing is greyed for simple or deleted products.
Adds a Video to Cart KPI card to the dashboard Insights row: the count of distinct
people who played a video then added a product leads, the rate (share of viewers)
and the Direct/Assisted split ('N in-video · M via product page') support it,
mirroring the Top Products Add-to-Cart column. Reads dashboard_metrics.video_to_cart
(godam-analytics#248), which passes through the WP metrics proxy unchanged.
Verified on the dev site: shows 1 cart, 25.0% of viewers, 1 in-video / 0 via product
page (1 of 4 players added in-video).
Adds the Video to Cart card to the single-video Insights row (Analytics.js), reading rangedAnalyticsData.video_to_cart from the per-video analytics endpoint (godam-analytics#248), which the WP proxy passes through unchanged. Moves VideoToCartCard into pages/analytics/ so the dashboard and per-video surfaces share one component, matching how SingleMetrics is shared. Verified live on the dev site (video id 327): card shows 1 cart, 25.0% of viewers, 1 in-video / 0 via product page, matching the endpoint and RTK store.
Adds godam-video-to-cart-card / -label / -value hooks so end-to-end tests can locate the card and read its value, matching the data-test-id convention used across the dashboard analytics tables.
…available Review follow-up (#2074): the Video-to-Cart card was unconditional and collapsed an absent payload to zeros, so a non-WooCommerce site (and a Woo site pointed at an analytics service that predates the video_to_cart roll-up) showed a permanent, misleading 'Video to Cart: 0'. - Gate the dashboard card on hasWooProducts (window.videoData.isWoo), matching the Top Products tab, and the per-video card on the same flag (now localized for the analytics page too). - VideoToCartCard renders nothing when videoToCart is null/undefined, so it never asserts '0 carts' for 'metric unavailable'. A present payload with zero carts is a real value and still renders. Verified on the dev site (Woo, data present): card renders 1 / 25.0% of viewers / 1 in-video / 0 via product page; hidden when the payload is absent.
…t a fixed 7-day trend
On the per-video Insights row the metric cards showed a fixed 'vs prev 7 days'
trend badge that ignored the range picker, while the Video-to-Cart card showed the
active range ('All time'), so the row disagreed with itself and with the picker.
Drop the client-side 7-day trend badge from the per-video SingleMetrics and
PlaysVsViewers and show the active range label instead, matching the Video-to-Cart
card and the range picker. Dashboard mode is unchanged: it still shows the
range-aware server period-over-period delta for a bounded range and the range
label at All Time. Removes the now-unused 7-day-history trend helpers from both
components.
…ISO-date validation - TopProductsTable.test.js: escapeCsvCell quotes a leading =,+,-,@ or tab/CR/newline so product names cannot execute as spreadsheet formulas; ordinary values unchanged. Exports escapeCsvCell for the test (no behavior change; default export intact). - VideoToCartCard.test.js: the M1 guard - renders nothing for a null/undefined video_to_cart payload (metric unavailable), still renders for a real zero-carts payload (0 is a measured value). Rendered via renderToString, no new dependency. - AnalyticsValidateIsoDateTest.php: validate_iso_date accepts strict Y-m-d (incl. valid leap day), rejects non-calendar/malformed dates; empty is allowed (the date params are optional). Wires class-analytics.php into the existing PHP test bootstrap. JS: 19 passed. PHP: 75 passed / 151 assertions.
…two lines The combined 'N in-video · M via product page' line was whitespace-nowrap, so in the narrow dashboard Insights card (5 cards on a laptop-width row) it overflowed and clipped at the right edge. Split it into two short stacked lines that fit at any card width.
Design feedback (Figma): - Drop Engagement Rate from the dashboard Insights row so it is four cards on WooCommerce sites (three on non-Woo, where Video-to-Cart is hidden), matching the design and giving each card room. Average Engagement stays on the per-video page. - Delta badge uses diagonal arrows (up-right / down-right) and keeps the arrow with its percentage on one line instead of wrapping the arrow onto its own row. - Video-to-Cart card gets a 'Woo' badge on its title, so it reads as a WooCommerce metric. - The rightmost card's info tooltip grew rightward and ran off the screen edge; it now anchors to the icon's right and grows inward, so it stays on-screen.
…e line With the dashboard row down to four cards there is width for it, and the card's extra height was leaving the sibling cards with dead space. Put '% of viewers' beside the count and restore the in-video/via-product-page split to a single line, so the card is about as short as the others. It wraps rather than clips if the window gets very narrow.
Mirror the thumbnail link: the product name now links to the product permalink (new tab), and stays plain text when a product has no permalink (missing product), exactly as the thumbnail already behaves.
Adds a unit test for sourceLabel: woo-layer/shoppable-video (+ the placeholder reel-pop / wc-product-gallery / godam-image labels) map to their display strings, and an unknown block_source falls back to the raw value. Exports sourceLabel for the test (no behavior change).
…split The card counts distinct people (deduped per person), so a buyer who added both in-video and via the product page would make the split read higher than the headline count and confuse the reader. The split lives in the Top Products table, where the numbers sum cleanly. Also drop the em-dash from the grey-out title in Top Products.
…oCommerce
Both are paid GoDAM for Woo add-on features, but the dashboard/analytics
videoData localized isWoo on class_exists('WooCommerce'), so any WooCommerce
store saw the Top Products tab and the Video to Cart card even without the paid
add-on. Gate them on the add-on's own premium check,
godam_woo_should_show_premium_blocks() (guarded by function_exists so it is
false when the add-on is inactive). This is the exact gate the add-on uses for
its own Pro blocks (Elementor/WPBakery Shoppable Video), so these features now
appear under identical conditions: add-on active and a valid GoDAM license.
* feat(top-products): render real Revenue once the analytics service reports it TopProductsTable's Revenue column was shipped as an empty placeholder (no 'Phase 2' label). The microservice's /dashboard/top-products/ response can now include revenue_minor/orders/currency per product; the REST proxy (fetch_top_products) and the RTK Query transformResponse already forward unknown fields through untouched, so no PHP or redux change was needed. Format revenue_minor/currency with Intl.NumberFormat and show the orders count as secondary text, matching the product_views/CTR pattern. Rows from an older service build that omit revenue_minor keep the existing "-" placeholder instead of showing a misleading value. Also carries the new columns into the CSV export. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Format Top Products revenue by currency fraction digits; make CSV numeric Scale revenue_minor by the currency's own ISO 4217 fraction digits (from Intl) instead of a hardcoded /100 and toFixed(2), so JPY (0 digits) and KWD (3 digits) render correctly. Export revenue in the CSV as a plain dot-decimal number in major units with the currency code in its own column, rather than a locale currency string. --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: Subodh Rajpopat <subodh.rajpopat@rtcamp.com>
GoDAM commits to a single store currency (no cross-currency conversion).
- WP proxy (class-analytics.php): pass base_currency = get_option('woocommerce_currency')
to /dashboard/metrics/fetch/ and /dashboard/top-products/, so the service returns
base-currency revenue and a count of orders in other currencies. The `revenue`
object already forwards through the dashboard-metrics array_merge.
- RevenueCard.js: a WooCommerce-only Insights KPI card showing total video-attributed
revenue via the existing formatRevenue(minor, currency), with an
"excluding N orders in other currencies" sub-line when that count is > 0 (and
nothing when 0, i.e. a single-currency store). Renders nothing if the payload is
absent, so it never shows a misleading 0.
- Dashboard.js: render RevenueCard in the Insights row, gated on hasWooProducts.
Base branch: feat/top-products (where #2086 shipped the Top Products revenue UI +
formatRevenue). Pairs with godam-analytics#257.
Sub-task C (core). Consumes the per-hotspot revenue the layer-analytics endpoint
now returns.
- useVideoLayerData: map revenue_minor/orders/currency onto each sub-hotspot; the
parent carries the sum of its hotspots' revenue (currency from any child).
- SubHotspotRail: a small secondary line per hotspot, "<amount> · N orders",
shown only when that hotspot has orders (Woo layers only).
- LayerDetailPanel: a headline "This hotspot/layer drove <amount> across N orders"
above the funnel, with an InfoTooltip that it is the Direct in-video contribution
only, not the product's total (Assisted purchases are excluded). Reuses the
shipped formatRevenue (correct ISO fraction digits), full numbers.
- WP proxy: pass base_currency = get_option('woocommerce_currency') to
/processed-layer-analytics/.
Base branch feat/top-products (#2086). Pairs with godam-analytics#259.
Each Woo composite row carries revenue_minor/orders/currency from the endpoint; assert groupRows maps them onto the sub, sums them onto the parent (currency from a base-currency child), and a non-base order that the server excluded (revenue_minor 0) is never blended in. Also assert the fields default to 0/empty when the endpoint sends none.
…fixes From the Phase 2 review: - Add the missing unit tests for the standalone layer-analytics runtime (findVideoElementById's image fallback that carries block_source, idempotent registration, single flush-listener bind). Correct the runtime doc comment: on a mixed video+image page analytics.js also flushes the buffer, harmless because flushLayerInteractions clears it after dispatch. - MetricTrend: render an exact 0% change as a neutral grey flat badge, not a green up arrow (0% is not growth). - PurchaseFunnelCard: clamp the cart-advanced annotation to 100% like buyAdvanced, so a skewed payload (carts > played) cannot print above 100%. - PlacementFunnelCard: surface a service error with a message instead of rendering it identically to a store with no placements. All prove-failed where a defect existed; 25 tests across the four files pass.
Product decision: the Export CSV should carry every column and sub-line shown on screen, so a merchant reconciling in a spreadsheet sees the same numbers as the table. Previously the CSV omitted the Influenced tier and the per-placement breakdown and the reach counts. Adds columns: Layers, Videos (reach); Influenced Revenue / Currency / Orders / Provisional; and Revenue by Placement (shown only when 2+ surfaces drove revenue, matching the on-screen sub-line). Extracts buildCsvRow and CSV_HEADERS to module level so the row-vs-header parity and the new fields are unit-tested.
… split is present Product decision: the total shown above the split bar should equal the parts below it. The headline was the service's revenue_minor (from the rollup) while the bar is drawn from the Direct/Assisted split (from raw type=7); the two are built to match but could diverge on an edge case, showing a total that does not equal its parts (worst case a nonzero total over an empty bar). When the split is present, headline = direct + assisted. Every paid order line carries a tier, so this can never hide revenue; without a split, the service total is used. Test prove-fails (headline 80.00 from the split, not the diverging service total 100.00).
…r a later interactive-sort feature
The revenue figures use grouped digits (₹1,234.00) but the order and reach counts beside them used %d, so a high-volume product read "1234 orders" next to "₹1,234.00". Switch the order and reach labels to toLocaleString grouping via %s + _n (plural rules still key off the raw count). Test covers the 1,000+ case and the singular.
…s endpoints Phase 2 added WooCommerce revenue / order-value to /analytics/fetch, /dashboard-metrics, /layer-analytics and the new /placement-funnels, all of which were permission_callback __return_true (public). Anyone with the public site_url could read a store's revenue. Gate the four on current_user_can( 'upload_files' ), matching /top-products and /top-videos; only the admin dashboard/editor/analytics React apps call them, never the front-end player, so this is safe. /history and /dashboard-history (no revenue) stay public.
Match the Top Products table: each Top Videos column header now carries an 'i' tooltip explaining the metric, so users can read the table without guessing -- in particular Conversion Rate, which counts layer actions (clicks, form submits, poll votes, add-to-cart), not the commerce-specific meaning.
The workflow gated on [develop, main], but the Phase 2 stack targets feature branches (feat/top-products, feat/phase2-woo-analytics-ui), so the JS unit tests, including the fourteen new .test.js files in this PR, never ran during review. Add feat/** so the stack verifies itself before merge.
feat(analytics): Woo Phase 2 revenue UI (single-currency card, per-hotspot, influenced)
* feat(dashboard): GA4 connection widget for add_to_cart/purchase counts Adds a slim dashboard widget confirming whether GoDAM is pushing GA4 add_to_cart/purchase events (via the godam-for-woo add-on) into the store's window.dataLayer. Not-connected state links to the existing General Settings enable_gtm_tracking toggle; connected state fetches running counts from the add-on's godam-for-woo/v1/ga4-counts REST route via a new RTK Query endpoint. Gated on the same hasWooProducts (add-on active + license) flag as Top Products / Video-to-Cart. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(ga4-widget): add standing-down, loading/error states, all-time label, and de-cramp Insights row Four PR #2097 review findings on GA4ConnectionWidget: - isConnected only reflected the enable_gtm_tracking toggle, so a store also running a competing GA4 plugin (source_active: true from the ga4-counts REST response) showed a green "Sending to GA4" while GoDAM was actually standing down and pushing nothing. Add a distinct standing-down state that names the other source_type and relabels the counts as "prepared" rather than sent. - The widget only read `data` from useFetchGa4CountsQuery, so a missing endpoint, 403, or network error rendered as "connected, 0/0" instead of an error state. Read isLoading/isError and show distinct loading and "status unavailable" states; only render counts on confirmed success. - The counts are lifetime totals with no date filtering, but sit in the same Insights row as range-scoped KPI cards with no indication they're all-time. Add an explicit "All time" caption. - lg:flex-nowrap was sized for four cards; GA4ConnectionWidget makes a fifth, squeezing every card near the 1024px breakpoint. Push the nowrap breakpoint to xl so the row wraps between lg and xl instead. Builds against the frozen ga4-counts REST shape being added in parallel in godam-for-woo (source_active/source_type), via dashboardAnalyticsApi's transformResponse. * chore: bump Tested up to 7.1 Plugin Check flags readme.txt as declaring compatibility only through WordPress 7.0, one behind the current release, which excludes the plugin from search results until updated. Pre-existing, unrelated to the GA4 feature in this PR — fixing it here since it's the only failing CI check. * fix: simplify GA4 widget copy for non-technical merchants, surface the All-time badge consistently Drop GA4's raw event-name jargon (add_to_cart, purchase) from the merchant- facing labels in favor of plain "Add to Cart"/"Purchase", and shorten the header and error copy. Also drop the standing-down paragraph now that the same explanation already lives in that state's status tooltip, and move the "All time" badge out of the metrics row into its own row alongside the Manage/Enable link so it renders consistently across all three connected states. * fix: correct GA4 widget layout and give it real status/action styling The widget wasn't wrapped in the analytics-info-container its own CSS requires, so .analytics-single-info rendered completely unstyled — number/ label pairs looked disconnected and the card never filled its row at large widths. An ID-scoped rule (analytics/index.scss, under #root-video-dashboard) was also unconditionally forcing column layout, which no plain Tailwind utility class can outrank regardless of breakpoint. Also: added a colored status dot for every state (off/checking/error/ standing-down/connected) alongside the existing text label, and replaced the unstyled "Enable"/"Manage" links with a real button in the site's own admin theme color (not the fixed brand red the shared .godam-button.is-secondary uses) with a trailing arrow. Refactored the four near-duplicate per-state JSX blocks into shared StatusLine/MetricsRow/ActionButton components. --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
Error handling, validation, revenue fallback, analytics accuracy, CSV safety, accessibility, and requirement mismatches remain unresolved.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
inc/classes/rest-api/class-analytics.php:1130
- Returning this new error envelope still does not surface the failure in Top Videos:
fetchTopVideos.transformResponsethrows forstatus:error, whileTopVideosTableignores the hook'sisErrorand renders its normal “no activity” state. Complete the change by adding an explicit query-error state to that table.
return new WP_REST_Response(
array(
'status' => 'error',
'message' => $detail,
'errorType' => 400 === $http_code ? 'bad_request' : 'microservice_error',
),
200
- Files reviewed: 43/43 changed files
- Comments generated: 13
- Review effort level: Balanced
| if ( /^[=+\-@\t\r]/.test( str ) ) { | ||
| str = `'${ str }`; | ||
| } | ||
| return /["\n,]/.test( str ) ? `"${ str.replace( /"/g, '""' ) }"` : str; |
There was a problem hiding this comment.
Fixed in 9495fbd (Top Products) and cc0fbe9 (Top Videos). The prefix test is now /^[=+-@\t\r\n]/ and the quoting test /["\n\r,]/, so a leading LF or CR is both prefixed and quote-wrapped and an interior CR quotes the field (RFC 4180). Tests added for a leading \n, a leading \r, and an interior \r; the sibling escapeCsvCell in TopVideosTable got the identical fix.
| // A still image has no play/visibility transition, so fire the | ||
| // parent-layer 'viewed' impression beacon once, now, on render. | ||
| // This is the impressions signal the product rollup reads; without | ||
| // it image Woo hotspots would always show 0 impressions. Guarded | ||
| // no-op until the analytics runtime is loaded (it is a dependency | ||
| // of this script, so it is present by now). | ||
| if ( typeof wooManager.emitLayerVisible === 'function' ) { | ||
| wooManager.emitLayerVisible( layerObj.layer ); |
There was a problem hiding this comment.
Keeping the on-render emit for this release. It is deliberate: a still image has no play or visibility transition to hang the impression on. Your point stands that an image below the fold or in a hidden tab still counts an impression, which loosens the CTR and add-to-cart denominator versus the video path (which emits on playback reaching the layer). Gating the emit behind an IntersectionObserver (emit once on first intersection, disconnect on teardown) is a tracked follow-up, not this release.
| // Top Products and Video to Cart are paid GoDAM for Woo add-on | ||
| // features. Gate them on the same check the add-on uses for its | ||
| // own Pro blocks: the add-on active (function_exists) plus a | ||
| // valid GoDAM license (godam_woo_should_show_premium_blocks). | ||
| // A plain WooCommerce store without the paid add-on does not | ||
| // see them. | ||
| 'isWoo' => function_exists( 'godam_woo_should_show_premium_blocks' ) && godam_woo_should_show_premium_blocks(), |
There was a problem hiding this comment.
Working as intended in code: the Top Products / Video-to-Cart tab is gated on the paid add-on plus a valid license (godam_woo_should_show_premium_blocks()), not plain WooCommerce presence. The PR description was wrong to say class_exists('WooCommerce'); I am correcting the description and acceptance criteria to state premium-only. No code change.
| $body = json_decode( wp_remote_retrieve_body( $response ), true ); | ||
|
|
||
| $top_videos = $body['top_videos'] ?? array(); | ||
| if ( 200 !== $http_code ) { |
There was a problem hiding this comment.
Fixed in b4122ad. Guard at fetch_top_videos is now 200 !== $http_code || ! is_array( $body ), so a 200 with a non-JSON body returns status:error. A PHPUnit case (200 + non-JSON body asserts status:error) covers it.
| { /* Count + rate only. The in-video vs via-product-page split is | ||
| intentionally not shown here: the count is deduped per person, so | ||
| a buyer who added both ways would make the split read higher than | ||
| the count. That split lives in the Top Products table, where the | ||
| numbers sum. */ } |
There was a problem hiding this comment.
By design, no change. This card's value is distinct people counted once, while direct/assisted are per-event counts, so a person who added both ways would make the split sum exceed the headline. The in-video vs product-page split lives in the Top Products table, where the counts add up. If the PR text says this shared card carries the split, that text should point at Top Products instead; the component is correct.
| <span className="text-xs text-zinc-500"> | ||
| { sprintf( | ||
| /* translators: %s: percentage of viewers who played and then purchased. */ | ||
| __( 'orders from video, %s%% of viewers who played', 'godam' ), |
There was a problem hiding this comment.
Fixed in 0f426c5. The subtitle now reads 'purchasers from video' to match the value, which is distinct people counted once, not orders. Worth a quick designer confirm on the exact wording, but 'orders' was factually wrong for a deduped-person count.
| <div className="godam-top-tabs__nav" role="tablist" aria-label={ __( 'Top content', 'godam' ) }> | ||
| <button | ||
| type="button" | ||
| role="tab" | ||
| aria-selected={ topTab === 'videos' } |
There was a problem hiding this comment.
Fixed in b7aec61. The Top-tab switcher no longer uses role=tablist / role=tab / aria-selected; the tabs are plain buttons carrying aria-pressed for the active tab. Because the content swaps (one table at a time) there is no persistent tabpanel to wire, so plain toggle buttons are the correct pattern here.
| { /* Engagement Rate is intentionally not shown on the dashboard; | ||
| Average Engagement still appears on each video's own | ||
| analytics page. */ } |
There was a problem hiding this comment.
Restored in b7aec61. The Engagement Rate card is back in the first Insights row, alongside Active Videos, Avg. Play Rate and Watch Time, per the decision to keep it. Per-video Average Engagement is unchanged.
| const { data, isFetching } = useFetchTopProductsQuery( | ||
| { | ||
| siteUrl, | ||
| page, | ||
| limit: PER_PAGE, | ||
| search, | ||
| startDate: dateRange.startDate, | ||
| endDate: dateRange.endDate, | ||
| }, | ||
| { skip }, | ||
| ); |
There was a problem hiding this comment.
Fixed in 9495fbd. The component now reads isError/error and renders a distinct error row before the empty-state branch, and the empty state is gated on ! isError, so a failed fetch (which transformResponse throws on status:error) no longer shows 'No product activity yet'. This is what makes the new proxy status:error handling visible.
…e card Replace the incomplete ARIA tab widget (role=tablist/role=tab/aria-selected with no tabpanel, aria-controls, roving tabIndex or arrow-key handling) with plain toggle buttons carrying aria-pressed, grouped by role=group + aria-label. Only one table renders at a time, so a full tab pattern was misleading to AT. Also restore the Engagement Rate insight card into the first Insights row (alongside Active Videos, Avg. Play Rate and Watch Time), matching develop. Addresses review comments 3763844514 / 3955271127 (ARIA) and 3955271165 (Engagement Rate card).
CSV: extend the formula-prefix test to a leading LF and the field-quoting test to a carriage return, so a value that leads with \n is neutralised and any interior \r is quote-wrapped inside its cell. Update/extend the unit tests for leading \n, leading \r, and an interior \r. Error vs empty: pull isError/error from useFetchTopProductsQuery and render a distinct error row before the empty-state branch, so a failed request (the transformResponse throws on status:error) no longer renders as 'No product activity yet'. Addresses review comments 3955270812 (CSV) and 3955271203 (error vs empty).
Mirror the Top Products CSV guard: extend the formula-prefix test to a leading LF and the field-quoting test to a carriage return. Addresses review comment 3955270812.
Read isError from useFetchPlacementFunnelsQuery and OR it with the existing soft
{ error } check, so a transport/HTTP failure shows the error state instead of
the 'no activity' empty state.
Addresses review comment 3955270996.
Without a tier split there are no Direct/Assisted amounts, so the previously ungated bar and labels rendered an empty bar and two misleading currency-zero rows. Wrap both in the hasSplit guard. Addresses review comment 3955271022.
The value is distinct people counted once, not orders, so change the subtitle from 'orders from video' to 'purchasers from video'. Update the test copy. Addresses review comment 3955271075.
…-bad-body - Add /analytics/placement-funnels to the range-args route list so its start_date/end_date get the shared validate_iso_date guard (a malformed date now 400s at the proxy instead of being forwarded). - In fetch_top_videos and fetch_top_products, treat a 200 whose body is not a JSON array (decodes to null, e.g. an HTML error page) as an error, mirroring fetch_placement_funnels, so it is not rendered as an empty 'no data' table. Add PHPUnit coverage: a 200-with-non-JSON-body asserts status:error for both methods, plus a valid-body guard rail. New pure-suite WP stubs (WP_REST_Request/Response, wp_remote_*, get_option) support the tests. Addresses review comments 3955270922 (placement range) and 3763844534 / 3955270950 / 3955270971 (body guard).
…fixes # Conflicts: # pages/analytics/hooks/useVideoLayerData.test.js
…vs empty render PHP (AnalyticsProxyBodyGuardTest): for fetch_top_videos, fetch_top_products and fetch_placement_funnels, assert a non-2xx upstream (503) returns status:error rather than an empty success; assert a 400 with a JSON detail is classified errorType='bad_request' on the two proxies that set it; and add the first error test for fetch_placement_funnels' 200-with-non-JSON-body guard. JS (TopProductsTable.test.js): render the component (client-side, react-dom since the WP serializer has no hook dispatcher) and assert the distinct godam-top-products-error row shows on isError (not the empty-state copy), and that the 'No product activity yet' empty state shows for an empty product set.
…ing the cards A range-scoped dashboard-metrics error (e.g. a heavy live scan timing out on a large account) only fed the range queries, but the top-level analyticsUnreachable notice watches ONLY the all-time primary query (and only microservice_error). So a ranged error silently zeroed the Insights KPIs and made the Video-to-Cart / Video-to-Purchase / Revenue / purchase-funnel cards vanish, with the gauge still showing all-time numbers and no notice anywhere. Surface it in-place, the way PlacementFunnelCard already does, with a new scoped AnalyticsSectionError (plain markup, actionable Try again): - full error (status:error or a hard RTK error) -> the Insights section and the gauge each show the scoped error instead of the cards. - partial degradation -> the service now returns dashboard_metrics.unavailable_ sections; the core KPIs still render and only the Woo cards show the scoped error. Revenue/purchase-funnel are hidden in both cases rather than reading a fake 0 or vanishing unexplained. Pairs with the service-side change that stops one failed heavy section 500-ing the whole endpoint.
… and range labels Top Videos CSV export (mirrors the already-hardened Top Products): - wrap in try/finally so the button never sticks on "Exporting…" if a fetch/ Blob/DOM op throws - page through with bounded concurrency (4) instead of one unbounded Promise.all that could fire hundreds of simultaneous requests on a large library - count failed pages and warn that the CSV is incomplete, instead of silently dropping them Top Videos table: show a dedicated error state on a backend error instead of the "No video plays yet" empty state, which misread an error as a real "no data" result (matches Top Products). Playback Performance chart: - remove any prior #chart-tooltip before appending; the ResizeObserver re-ran renderChart without cleanup, leaking a duplicate-id tooltip node per resize - read the event from the mousemove handler arg, not the deprecated global window.event (undefined in Firefox -> d3.pointer threw, breaking the follow-line) Insights labels: derive the range label from the range whose data is actually on screen (RTK currentData), not the picker state, so a slow range fetch no longer shows the new range's label over the previous range's retained numbers.
… error-scoping Restores #2090 (each commerce card its own date-range picker), which was committed to feat/phase2-woo-analytics-ui AFTER PR #2090 had merged an earlier state, so it never actually landed in this PR. RevenueCard / PurchaseFunnelCard / PlacementFunnelCard now each carry a DateRangePicker (rangeControl prop) and fetch their own data via useFetchRevenueSummaryQuery / useFetchVideoFunnelQuery + two proxy routes forwarding to the microservice /dashboard/revenue-summary/ and /dashboard/video-funnel/ endpoints. Reconciled with the empty-window / error-scoping fix so restoring the pickers does NOT reopen the vanish: - the cards are decoupled from the Insights query (no longer gated on insightsFullError); - an empty range returns a zeroed payload from the standalone endpoints, so each card renders 0, not a vanish; - a per-card query error ({ error: true }) now surfaces INSIDE the card, keeping its picker, instead of vanishing (RevenueCard/PurchaseFunnelCard) - PlacementFunnelCard already did this. Also covers the pre-existing Top Products thumbnail loop with the centralized rtgodam_before/after_attachment_lookup pair (the hook-integrity gate flagged it once this file was touched; product images are attachments and need the offload context). Tests: rangeControl slot + the new in-place error state for all three cards. Pairs with godam-analytics#248 (the two standalone endpoints).
…-currency note
Two follow-ups from an adversarial review of the per-card restore:
1. No-vanish gap (should-fix). RevenueCard/PurchaseFunnelCard handled only the SOFT
{ error: true } marker (a 2xx status:error the WP proxy normalises). On a HARD
route failure (stale-nonce 401/403, a PHP fatal / 5xx, unparseable JSON, or a
version-skew 404) RTK sets isError and leaves data undefined, so the cards hit
their null guard and silently vanished — while PlacementFunnelCard, which folds
its own isError in, did not. Capture isError from both standalone hooks and
synthesize the marker (revenue = isError ? { error: true } : data), so both cards
show their inline error and keep the picker on hard errors too.
2. Top Products base-currency note. The revenue column is single-currency; add a
help note under the table head naming the store base currency code (from the
rows), and fold the code into the Revenue column tooltip, so a multi-currency
store owner is not confused when a product that sold in another currency reads 0.
Pairs with godam-analytics#248 (revenue currency = base). Tests: TopProductsTable
base-currency note present with the code / omitted when no row carries a currency.
elifvish
left a comment
There was a problem hiding this comment.
Approving
Third review pass, at 71a3d16e, covering the full Phase 1 + Phase 2 UI now that #2090 and #2086 are merged in.
Verified in this pass
Route permissions are now correct and the pattern held. Ten analytics routes: eight gate on current_user_can( 'upload_files' ), two stay public — /history and /dashboard-history — and I confirmed both carry zero revenue references. The two routes added since my last look, /revenue-summary and /video-funnel, were gated from the start rather than needing a follow-up fix.
I re-checked the regression risk rather than assuming: every caller of the gated routes lives under pages/ (dashboard, analytics, video-editor). Nothing under assets/src — the front-end player — touches any of them. upload_files matches the capability on the Analytics and Media Editor submenus exactly; the only edit_posts submenus are Help and What's New, which call nothing. No role loses access.
No XSS surface in the new UI. No dangerouslySetInnerHTML in any new component; product name, image and permalink are resolved server-side through wc_get_product() / get_permalink() and React escapes the rest.
JS unit tests, PHP unit tests, PHPCS and the hook-integrity check are all green.
Outstanding checks
The POT check is failing — mechanical staleness, npm run build:prod && composer pot and commit. Approving on the code, but this still needs clearing before merge. Plugin Check was pending when I looked.
Follow-ups, not blocking
- The ISO 4217 exponent table is still duplicated across repos:
TopProductsTable.js:129andgodam-for-woo'sOrder_Revenue_Emission::currency_minor_unit_exponent, held in sync only by a comment. Both copies are correct today — I checked them entry for entry — so this is about the next edit, where a one-sided addition renders that currency's revenue 100× out and it will be believed. Carryingminor_unit_exponenton the event and decoding with the value the money was actually encoded with would make drift impossible by construction. formatRevenue/formatRevenueNumeric/currencyFractionDigitsare the money formatters for the whole feature but live in, and are exported from, a table component —RevenueCard,RevenueTipsand the CSV export all reach intoTopProductsTable.jsfor them.- Only the JS workflow got the
feat/**branch filter;php_unit_tests_on_pull_request.ymlstill gates ondevelop/mainonly. Worth making that consistent so the next stacked PR tests itself both ways.
Good hardening work across the review-response commits — the CSV escaping for LF/CR, distinguishing load error from empty state, and scoping the per-card query errors were all real improvements.
Summary
Adds the dashboard UI for WooCommerce Phase 2 revenue analytics (issue #26): Video-Attributed Revenue, Top Products, and the shopping funnels. This is the frontend counterpart to the analytics-service work in rtCamp/godam-analytics#248; together they surface, per store, the order value traced back to the video that earned it.
What is included
{error:true}marker and hard RTK errors both handled) instead of collapsing the whole Insights section, matching the service'sunavailable_sectionsresponse.Paired PRs (ship together)
Revenue only works end to end with all three, so they release together; without the service this UI shows the commerce sections with no data (and degrades gracefully).
How to test
Automation impact
New data-test-ids on the added components (revenue card, funnels, Top Products table rows/notes/pagination/export) for E2E selection. No existing test-ids removed or renamed.
Notes
woocommerce_currency); other-currency orders are excluded and counted separately. Non-Woo installs omit the currency and the revenue sections stay empty by design./dashboard/metrics/fetch/, an empty date range now returns a zeroed metrics object instead ofnull, andunique_viewersdefaults to0(notnull) when a section is unavailable, with the failed section named inunavailable_sections.