From 560195f9f42011b7144e28c2b645a287b4815b3e Mon Sep 17 00:00:00 2001 From: Coding On Star <447357187@qq.com> Date: Fri, 17 Apr 2026 20:24:34 +0800 Subject: [PATCH] feat(explore): implement banner impression tracking and refactor tracking logic (#35369) Co-authored-by: CodingOnStar --- .../explore/banner/__tests__/banner.spec.tsx | 47 ++++++++++--- web/app/components/explore/banner/banner.tsx | 70 +++++++++++++------ 2 files changed, 83 insertions(+), 34 deletions(-) diff --git a/web/app/components/explore/banner/__tests__/banner.spec.tsx b/web/app/components/explore/banner/__tests__/banner.spec.tsx index 069aaf02dc..c95f2532e2 100644 --- a/web/app/components/explore/banner/__tests__/banner.spec.tsx +++ b/web/app/components/explore/banner/__tests__/banner.spec.tsx @@ -1,6 +1,6 @@ -import type * as React from 'react' import type { Banner as BannerType } from '@/models/app' import { cleanup, fireEvent, render, screen } from '@testing-library/react' +import * as React from 'react' import { act } from 'react' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import Banner from '../banner' @@ -8,6 +8,13 @@ import Banner from '../banner' const mockUseGetBanners = vi.fn() const mockUseSelector = vi.fn() const mockTrackEvent = vi.fn() +let mockSelectedIndex = 0 +const mockCarouselListeners = new Set<() => void>() + +const setMockSelectedIndex = (index: number) => { + mockSelectedIndex = index + mockCarouselListeners.forEach(listener => listener()) +} vi.mock('@/service/use-explore', () => ({ useGetBanners: (...args: unknown[]) => mockUseGetBanners(...args), @@ -54,13 +61,23 @@ vi.mock('@/app/components/base/carousel', () => ({ }, }, ), - useCarousel: () => ({ - api: { - scrollTo: vi.fn(), - slideNodes: () => [], - }, - selectedIndex: 0, - }), + useCarousel: () => { + const selectedIndex = React.useSyncExternalStore( + (listener) => { + mockCarouselListeners.add(listener) + return () => mockCarouselListeners.delete(listener) + }, + () => mockSelectedIndex, + ) + + return { + api: { + scrollTo: vi.fn(), + slideNodes: () => [], + }, + selectedIndex, + } + }, })) vi.mock('../banner-item', () => ({ @@ -102,7 +119,10 @@ const createMockBanner = (id: string, status: string = 'enabled', title: string describe('Banner', () => { beforeEach(() => { + vi.clearAllMocks() vi.useFakeTimers() + mockSelectedIndex = 0 + mockCarouselListeners.clear() mockUseSelector.mockImplementation(selector => selector({ userProfile: { id: 'account-123', @@ -112,7 +132,6 @@ describe('Banner', () => { afterEach(() => { cleanup() - vi.clearAllMocks() vi.useRealTimers() }) @@ -257,7 +276,7 @@ describe('Banner', () => { expect(screen.getByTestId('carousel')).toHaveClass('rounded-2xl') }) - it('tracks enabled banner impressions with expected payload', () => { + it('tracks only the current banner impression and reports the next one after slide changes', () => { mockUseGetBanners.mockReturnValue({ data: [ createMockBanner('1', 'enabled', 'Enabled Banner 1'), @@ -270,7 +289,7 @@ describe('Banner', () => { render() - expect(mockTrackEvent).toHaveBeenCalledTimes(2) + expect(mockTrackEvent).toHaveBeenCalledTimes(1) expect(mockTrackEvent).toHaveBeenNthCalledWith(1, 'explore_banner_impression', expect.objectContaining({ banner_id: '1', title: 'Enabled Banner 1', @@ -281,6 +300,12 @@ describe('Banner', () => { account_id: 'account-123', event_time: expect.any(Number), })) + + act(() => { + setMockSelectedIndex(1) + }) + + expect(mockTrackEvent).toHaveBeenCalledTimes(2) expect(mockTrackEvent).toHaveBeenNthCalledWith(2, 'explore_banner_impression', expect.objectContaining({ banner_id: '3', title: 'Enabled Banner 2', diff --git a/web/app/components/explore/banner/banner.tsx b/web/app/components/explore/banner/banner.tsx index a320bb1974..505902195b 100644 --- a/web/app/components/explore/banner/banner.tsx +++ b/web/app/components/explore/banner/banner.tsx @@ -1,8 +1,9 @@ import type { FC } from 'react' +import type { Banner as BannerType } from '@/models/app' import * as React from 'react' import { useEffect, useMemo, useRef, useState } from 'react' import { trackEvent } from '@/app/components/base/amplitude' -import { Carousel } from '@/app/components/base/carousel' +import { Carousel, useCarousel } from '@/app/components/base/carousel' import { useSelector } from '@/context/app-context' import { useLocale } from '@/context/i18n' import { useGetBanners } from '@/service/use-explore' @@ -22,6 +23,45 @@ const LoadingState: FC = () => ( ) +type BannerImpressionTrackerProps = { + banners: BannerType[] + accountId?: string + language: string + trackedBannerIdsRef: React.MutableRefObject> +} + +const BannerImpressionTracker: FC = ({ + banners, + accountId, + language, + trackedBannerIdsRef, +}) => { + const { selectedIndex } = useCarousel() + + useEffect(() => { + if (!accountId) + return + + const currentBanner = banners[selectedIndex] + if (!currentBanner || trackedBannerIdsRef.current.has(currentBanner.id)) + return + + trackEvent('explore_banner_impression', { + banner_id: currentBanner.id, + title: currentBanner.content.title, + sort: selectedIndex + 1, + link: currentBanner.link, + page: 'explore', + language, + account_id: accountId, + event_time: Date.now(), + }) + trackedBannerIdsRef.current.add(currentBanner.id) + }, [accountId, banners, language, selectedIndex, trackedBannerIdsRef]) + + return null +} + const Banner: FC = () => { const locale = useLocale() const { data: banners, isLoading, isError } = useGetBanners(locale) @@ -60,28 +100,6 @@ const Banner: FC = () => { } }, []) - useEffect(() => { - if (!accountId) - return - - enabledBanners.forEach((banner, index) => { - if (trackedBannerIdsRef.current.has(banner.id)) - return - - trackEvent('explore_banner_impression', { - banner_id: banner.id, - title: banner.content.title, - sort: index + 1, - link: banner.link, - page: 'explore', - language: locale, - account_id: accountId, - event_time: Date.now(), - }) - trackedBannerIdsRef.current.add(banner.id) - }) - }, [accountId, enabledBanners, locale]) - if (isLoading) return @@ -102,6 +120,12 @@ const Banner: FC = () => { onMouseEnter={() => setIsHovered(true)} onMouseLeave={() => setIsHovered(false)} > + {enabledBanners.map((banner, index) => (