From 4397ffb71bf1bc0b91dba759cd6ca31d31719bad Mon Sep 17 00:00:00 2001
From: Olga Bulat <obulat@gmail.com>
Date: Fri, 13 Dec 2024 07:31:05 +0300
Subject: [PATCH] Make analytics tests stricter; replace collectionValue with
 query (#5267)

---
 frontend/shared/types/analytics.ts            | 68 +++++++++----------
 .../src/components/VImageCell/VImageCell.vue  |  7 +-
 .../VMediaInfo/VLicenseTabPanel.vue           |  4 +-
 .../VSearchResultsGrid/VAudioResult.vue       |  7 +-
 frontend/src/stores/search.ts                 |  6 +-
 .../e2e/all-results-analytics.spec.ts         | 57 +++++++++-------
 .../test/playwright/e2e/audio-detail.spec.ts  | 25 ++++---
 .../test/playwright/e2e/image-detail.spec.ts  | 20 +++---
 .../test/playwright/e2e/load-more.spec.ts     | 43 ++++++------
 frontend/test/playwright/utils/analytics.ts   | 10 ++-
 10 files changed, 136 insertions(+), 111 deletions(-)

diff --git a/frontend/shared/types/analytics.ts b/frontend/shared/types/analytics.ts
index 3d10fed25a9..bf4340938b1 100644
--- a/frontend/shared/types/analytics.ts
+++ b/frontend/shared/types/analytics.ts
@@ -28,17 +28,21 @@ export type AudioComponent =
  */
 export type SearchParamsForEvent = {
   kind: "search" | "collection"
+  /** The search type of the current results page */
   searchType: SupportedSearchType
+  /** The search term, or a string representing a unique identifier for the collection */
   query: string
-
-  /**
-   * Common properties related to searches on collection pages, added in the
-   * "Additional Search Views" project.
-   */
   /** If a collection page, the type of collection */
   collectionType: Collection | "null"
-  /** A string representing a unique identifier for the collection */
-  collectionValue: string | "null"
+}
+
+/**
+ * Common properties for search result events, used to link the result to
+ * assess relevancy.
+ */
+export type SearchResultParams = {
+  /** The unique ID of the media */
+  id: string
 }
 
 /**
@@ -75,7 +79,7 @@ export type Events = {
    *   - How many results do most searches yield?
    */
   GET_SEARCH_RESULTS: {
-    /** the media type being searched */
+    /** the media type of the results. Is different from `searchType` when searchType is "all media" */
     mediaType: SupportedMediaType
     /** The search term */
     query: string
@@ -108,9 +112,7 @@ export type Events = {
    *   - Do users right-click images often? Does this suggest downloading them directly,
    *     when not paired with a `GET_MEDIA` event?
    */
-  RIGHT_CLICK_IMAGE: {
-    id: string
-  }
+  RIGHT_CLICK_IMAGE: SearchResultParams
   /**
    * Click on the 'back to search' link on a single result
    *
@@ -155,9 +157,7 @@ export type Events = {
    * Questions:
    *   - How often do users go to the source after viewing a result?
    */
-  GET_MEDIA: {
-    /** the unique ID of the media */
-    id: string
+  GET_MEDIA: SearchResultParams & {
     /** The slug (not the prettified name) of the provider */
     provider: string
     /** The media type being searched */
@@ -169,9 +169,7 @@ export type Events = {
    *   - How often do users use our attribution tool?
    *   - Which format is the most popular?
    */
-  COPY_ATTRIBUTION: {
-    /** The unique ID of the media */
-    id: string
+  COPY_ATTRIBUTION: SearchResultParams & {
     /** The format of the copied attribution */
     format: "plain" | "rich" | "html" | "xml"
     /** The media type being searched */
@@ -296,22 +294,23 @@ export type Events = {
    *   - How often do searches lead to clicking a result?
    *   - Are there popular searches that do not result in result selection?
    */
-  SELECT_SEARCH_RESULT: Omit<SearchParamsForEvent, "searchType" | "kind"> & {
-    /** The unique ID of the media */
-    id: string
-    /** If the result is a related result, provide the ID of the 'original' result */
-    relatedTo: string | "null"
-    /** Kind of the result selected: search/related/collection */
-    kind: ResultKind
-    /** The media type being searched */
-    mediaType: SearchType
-    /** The slug (not the prettified name) of the provider */
-    provider: string
-    /** the reasons for why this result is considered sensitive */
-    sensitivities: string
-    /** whether the result was blurred or visible when selected by the user */
-    isBlurred: boolean | "null"
-  }
+  SELECT_SEARCH_RESULT: SearchResultParams &
+    Pick<SearchParamsForEvent, "collectionType"> & {
+      /** If the result is a related result, provide the ID of the 'original' result */
+      relatedTo: string | "null"
+      /** Kind of the result selected: search/related/collection */
+      kind: ResultKind
+      /** The media type of the selected result */
+      mediaType: SupportedMediaType
+      /** The search type of the current results page */
+      searchType: SupportedSearchType
+      /** The slug (not the prettified name) of the provider */
+      provider: string
+      /** the reasons for why this result is considered sensitive */
+      sensitivities: string
+      /** whether the result was blurred or visible when selected by the user */
+      isBlurred: boolean | "null"
+    }
   /**
    * Description: When a user opens the external sources popover.
    * Questions:
@@ -337,8 +336,7 @@ export type Events = {
    *     from anyway?
    */
   LOAD_MORE_RESULTS: {
-    /** The current page of results the user is on,
-     * *before* loading more results.. */
+    /** The current page of results the user is on, *before* loading more results. */
     resultPage: number
   } & SearchParamsForEvent
   /**
diff --git a/frontend/src/components/VImageCell/VImageCell.vue b/frontend/src/components/VImageCell/VImageCell.vue
index a80d9a47db3..b69f9ea4905 100644
--- a/frontend/src/components/VImageCell/VImageCell.vue
+++ b/frontend/src/components/VImageCell/VImageCell.vue
@@ -111,18 +111,17 @@ const sendSelectSearchResultEvent = (event: MouseEvent) => {
     return
   }
 
+  const { searchType, collectionType } = searchStore.searchParamsForEvent
   $sendCustomEvent("SELECT_SEARCH_RESULT", {
+    searchType,
+    collectionType,
     id: props.image.id,
     kind: props.kind,
     mediaType: IMAGE,
     provider: props.image.provider,
-    query: props.searchTerm || "",
     relatedTo: props.relatedTo ?? "null",
     sensitivities: props.image.sensitivity?.join(",") ?? "",
     isBlurred: shouldBlur.value ?? "null",
-    collectionType:
-      searchStore.strategy !== "default" ? searchStore.strategy : "null",
-    collectionValue: searchStore.collectionValue ?? "null",
   })
 }
 
diff --git a/frontend/src/components/VMediaInfo/VLicenseTabPanel.vue b/frontend/src/components/VMediaInfo/VLicenseTabPanel.vue
index 02ef9df5036..1ecc8b4d7b1 100644
--- a/frontend/src/components/VMediaInfo/VLicenseTabPanel.vue
+++ b/frontend/src/components/VMediaInfo/VLicenseTabPanel.vue
@@ -1,7 +1,7 @@
 <script setup lang="ts">
 import { useNuxtApp } from "#imports"
 
-import type { MediaType } from "#shared/constants/media"
+import type { SupportedMediaType } from "#shared/constants/media"
 
 import VCopyButton from "~/components/VCopyButton.vue"
 import VTabPanel from "~/components/VTabs/VTabPanel.vue"
@@ -20,7 +20,7 @@ const props = defineProps<{
    * The media type of the media for which the attribution is generated.
    * Used for analytics.
    */
-  mediaType: MediaType
+  mediaType: SupportedMediaType
 }>()
 
 const { $sendCustomEvent } = useNuxtApp()
diff --git a/frontend/src/components/VSearchResultsGrid/VAudioResult.vue b/frontend/src/components/VSearchResultsGrid/VAudioResult.vue
index fab6e6f861d..44f0ce24b4c 100644
--- a/frontend/src/components/VSearchResultsGrid/VAudioResult.vue
+++ b/frontend/src/components/VSearchResultsGrid/VAudioResult.vue
@@ -43,18 +43,17 @@ const sendSelectSearchResultEvent = (
     return
   }
   useAudioSnackbar().hide()
+  const { searchType, collectionType } = searchStore.searchParamsForEvent
   $sendCustomEvent("SELECT_SEARCH_RESULT", {
+    searchType,
+    collectionType,
     id: audio.id,
     kind: props.kind,
     mediaType: AUDIO,
-    query: props.searchTerm,
     provider: audio.provider,
     relatedTo: props.relatedTo ?? "null",
     sensitivities: audio.sensitivity?.join(",") ?? "",
     isBlurred: shouldBlur.value ?? "null",
-    collectionType:
-      searchStore.strategy !== "default" ? searchStore.strategy : "null",
-    collectionValue: searchStore.collectionValue ?? "null",
   })
 }
 const sendInteractionEvent = (
diff --git a/frontend/src/stores/search.ts b/frontend/src/stores/search.ts
index 23c6ca4d93f..9d600d7a203 100644
--- a/frontend/src/stores/search.ts
+++ b/frontend/src/stores/search.ts
@@ -239,12 +239,14 @@ export const useSearchStore = defineStore("search", {
     searchParamsForEvent(): SearchParamsForEvent {
       return {
         kind: this.strategy === "default" ? "search" : "collection",
-        query: this.searchTerm,
+        query:
+          this.strategy === "default"
+            ? this.searchTerm
+            : (this.collectionValue ?? ""),
         searchType: isSearchTypeSupported(this.searchType)
           ? this.searchType
           : ALL_MEDIA,
         collectionType: this.collectionParams?.collection ?? "null",
-        collectionValue: this.collectionValue ?? "null",
       }
     },
   },
diff --git a/frontend/test/playwright/e2e/all-results-analytics.spec.ts b/frontend/test/playwright/e2e/all-results-analytics.spec.ts
index 4fce11e7ed4..1a82a563c63 100644
--- a/frontend/test/playwright/e2e/all-results-analytics.spec.ts
+++ b/frontend/test/playwright/e2e/all-results-analytics.spec.ts
@@ -12,18 +12,28 @@ import {
 } from "~~/test/playwright/utils/analytics"
 import { t } from "~~/test/playwright/utils/i18n"
 
-import { AUDIO, IMAGE } from "#shared/constants/media"
+import { ALL_MEDIA, AUDIO, IMAGE } from "#shared/constants/media"
+import type { Events } from "#shared/types/analytics"
 
 test.describe.configure({ mode: "parallel" })
 
-const defaultPayload = {
-  query: "birds",
+const searchResultPayload = {
   kind: "search",
-  relatedTo: "null",
   collectionType: "null",
-  collectionValue: "null",
-  sensitivities: "",
+  searchType: ALL_MEDIA,
+} as const
+
+const defaultPayload = {
+  ...searchResultPayload,
   isBlurred: false,
+  relatedTo: "null",
+  sensitivities: "",
+} as const
+
+const audioResult = {
+  mediaType: AUDIO,
+  id: "2e38ac1e-830c-4e9c-b13d-2c9a1ad53f95",
+  provider: "jamendo",
 } as const
 
 test.describe("all results grid analytics test", () => {
@@ -42,15 +52,14 @@ test.describe("all results grid analytics test", () => {
     const selectSearchResultEvent = analyticsEvents.find(
       (event) => event.n === "SELECT_SEARCH_RESULT"
     )
-    expectEventPayloadToMatch(selectSearchResultEvent, {
+    const expectedPayload: Events["SELECT_SEARCH_RESULT"] = {
       ...defaultPayload,
-      mediaType: AUDIO,
-      id: "2e38ac1e-830c-4e9c-b13d-2c9a1ad53f95",
-      provider: "jamendo",
-    })
+      ...audioResult,
+    }
+    expectEventPayloadToMatch(selectSearchResultEvent, expectedPayload)
   })
 
-  test("should send SELECT_SEARCH_RESULT event when image result is selected", async ({
+  test("sends SELECT_SEARCH_RESULT event when image result is selected", async ({
     context,
     page,
   }) => {
@@ -60,12 +69,14 @@ test.describe("all results grid analytics test", () => {
     const selectSearchResultEvent = analyticsEvents.find(
       (event) => event.n === "SELECT_SEARCH_RESULT"
     )
-
-    expectEventPayloadToMatch(selectSearchResultEvent, {
+    const expectedPayload: Events["SELECT_SEARCH_RESULT"] = {
+      ...defaultPayload,
       id: "da5cb478-c093-4d62-b721-cda18797e3fb",
       mediaType: IMAGE,
       provider: "flickr",
-    })
+    }
+
+    expectEventPayloadToMatch(selectSearchResultEvent, expectedPayload)
   })
 
   test("sends AUDIO_INTERACTION event when audio is interacted", async ({
@@ -83,13 +94,13 @@ test.describe("all results grid analytics test", () => {
     const audioInteractionEvent = analyticsEvents.find(
       (event) => event.n === "AUDIO_INTERACTION"
     )
-
-    expectEventPayloadToMatch(audioInteractionEvent, {
-      id: "2e38ac1e-830c-4e9c-b13d-2c9a1ad53f95",
+    const expectedPayload: Events["AUDIO_INTERACTION"] = {
+      id: audioResult.id,
+      provider: audioResult.provider,
       event: "play",
-      provider: "jamendo",
       component: "VAllResultsGrid",
-    })
+    }
+    expectEventPayloadToMatch(audioInteractionEvent, expectedPayload)
   })
 
   test("sends CHANGE_CONTENT_TYPE event when content type is changed", async ({
@@ -109,11 +120,11 @@ test.describe("all results grid analytics test", () => {
     const changeContentTypeEvent = events.find(
       (event) => event.n === "CHANGE_CONTENT_TYPE"
     )
-
-    expectEventPayloadToMatch(changeContentTypeEvent, {
+    const expectedPayload: Events["CHANGE_CONTENT_TYPE"] = {
       component: "VContentLink",
       next: "image",
       previous: "all",
-    })
+    }
+    expectEventPayloadToMatch(changeContentTypeEvent, expectedPayload)
   })
 })
diff --git a/frontend/test/playwright/e2e/audio-detail.spec.ts b/frontend/test/playwright/e2e/audio-detail.spec.ts
index 912bf5699a7..b04ec41fe1e 100644
--- a/frontend/test/playwright/e2e/audio-detail.spec.ts
+++ b/frontend/test/playwright/e2e/audio-detail.spec.ts
@@ -11,6 +11,8 @@ import {
 import { t } from "~~/test/playwright/utils/i18n"
 import { getH1 } from "~~/test/playwright/utils/components"
 
+import type { Events } from "#shared/types/analytics"
+
 const audioObject = {
   id: "1cb1af19-7232-40c2-b9ea-8d6c47e677f9",
   provider: "wikimedia_audio",
@@ -51,10 +53,11 @@ test("sends GET_MEDIA event on CTA button click", async ({ context, page }) => {
 
   const getMediaEvent = analyticsEvents.find((event) => event.n === "GET_MEDIA")
 
-  expectEventPayloadToMatch(getMediaEvent, {
+  const expectedPayload: Events["GET_MEDIA"] = {
     ...audioObject,
     mediaType: "audio",
-  })
+  }
+  expectEventPayloadToMatch(getMediaEvent, expectedPayload)
 })
 
 test("sends AUDIO_INTERACTION event on play", async ({ page, context }) => {
@@ -68,11 +71,12 @@ test("sends AUDIO_INTERACTION event on play", async ({ page, context }) => {
     (event) => event.n === "AUDIO_INTERACTION"
   )
 
-  expectEventPayloadToMatch(audioInteractionEvent, {
+  const expectedPayload: Events["AUDIO_INTERACTION"] = {
     ...audioObject,
     event: "play",
     component: "AudioDetailPage",
-  })
+  }
+  expectEventPayloadToMatch(audioInteractionEvent, expectedPayload)
 })
 
 test("sends AUDIO_INTERACTION event on seek", async ({ page, context }) => {
@@ -86,11 +90,12 @@ test("sends AUDIO_INTERACTION event on seek", async ({ page, context }) => {
     (event) => event.n === "AUDIO_INTERACTION"
   )
 
-  expectEventPayloadToMatch(audioInteractionEvent, {
+  const expectedPayload: Events["AUDIO_INTERACTION"] = {
     ...audioObject,
     event: "seek",
     component: "AudioDetailPage",
-  })
+  }
+  expectEventPayloadToMatch(audioInteractionEvent, expectedPayload)
 })
 
 test("shows the 404 error page when no valid id", async ({ page }) => {
@@ -126,16 +131,16 @@ test("sends SELECT_SEARCH_RESULT event on related audio click", async ({
     (event) => event.n === "SELECT_SEARCH_RESULT"
   )
 
-  expectEventPayloadToMatch(selectSearchResultEvent, {
+  const expectedPayload: Events["SELECT_SEARCH_RESULT"] = {
     id: "0b94484c-d7d1-43f2-8710-69399b6a0310",
     relatedTo: audioObject.id,
     kind: "related",
     mediaType: "audio",
     provider: "wikimedia_audio",
-    query: "",
+    searchType: "all",
     sensitivities: "",
     isBlurred: false,
     collectionType: "null",
-    collectionValue: "null",
-  })
+  }
+  expectEventPayloadToMatch(selectSearchResultEvent, expectedPayload)
 })
diff --git a/frontend/test/playwright/e2e/image-detail.spec.ts b/frontend/test/playwright/e2e/image-detail.spec.ts
index 4392dc3821a..cebddd73a9e 100644
--- a/frontend/test/playwright/e2e/image-detail.spec.ts
+++ b/frontend/test/playwright/e2e/image-detail.spec.ts
@@ -12,6 +12,8 @@ import {
 } from "~~/test/playwright/utils/analytics"
 import { getBackToSearchLink } from "~~/test/playwright/utils/components"
 
+import type { Events } from "#shared/types/analytics"
+
 const imageObject = {
   id: "e9d97a98-621b-4ec2-bf70-f47a74380452",
   provider: "flickr",
@@ -80,11 +82,12 @@ test("sends GET_MEDIA event on CTA button click", async ({ context, page }) => {
 
   const getMediaEvent = analyticsEvents.find((event) => event.n === "GET_MEDIA")
 
-  expectEventPayloadToMatch(getMediaEvent, {
+  const expectedPayload: Events["GET_MEDIA"] = {
     id: imageObject.id,
     provider: imageObject.provider,
     mediaType: "image",
-  })
+  }
+  expectEventPayloadToMatch(getMediaEvent, expectedPayload)
 })
 
 test("sends RIGHT_CLICK_IMAGE event on right-click", async ({
@@ -102,9 +105,10 @@ test("sends RIGHT_CLICK_IMAGE event on right-click", async ({
     (event) => event.n === "RIGHT_CLICK_IMAGE"
   )
 
-  expectEventPayloadToMatch(rightClickImageEvent, {
+  const expectedPayload: Events["RIGHT_CLICK_IMAGE"] = {
     id: imageObject.id,
-  })
+  }
+  expectEventPayloadToMatch(rightClickImageEvent, expectedPayload)
 })
 
 test("sends SELECT_SEARCH_RESULT event on related image click", async ({
@@ -126,16 +130,16 @@ test("sends SELECT_SEARCH_RESULT event on related image click", async ({
     (event) => event.n === "SELECT_SEARCH_RESULT"
   )
 
-  expectEventPayloadToMatch(selectSearchResultEvent, {
+  const expectedPayload: Events["SELECT_SEARCH_RESULT"] = {
     id: "1c57f839-6be5-449a-b41a-b1c7de819182",
     relatedTo: imageObject.id,
     kind: "related",
     mediaType: "image",
     provider: "flickr",
-    query: "",
+    searchType: "all",
     sensitivities: "",
     isBlurred: false,
     collectionType: "null",
-    collectionValue: "null",
-  })
+  }
+  expectEventPayloadToMatch(selectSearchResultEvent, expectedPayload)
 })
diff --git a/frontend/test/playwright/e2e/load-more.spec.ts b/frontend/test/playwright/e2e/load-more.spec.ts
index 7c2b7221a71..c033d0dc16c 100644
--- a/frontend/test/playwright/e2e/load-more.spec.ts
+++ b/frontend/test/playwright/e2e/load-more.spec.ts
@@ -12,6 +12,7 @@ import {
 import { getLoadMoreButton } from "~~/test/playwright/utils/components"
 
 import { AUDIO, IMAGE, type SupportedMediaType } from "#shared/constants/media"
+import type { Events } from "#shared/types/analytics"
 
 test.describe.configure({ mode: "parallel" })
 
@@ -24,6 +25,14 @@ const openSingleMediaView = async (
     .click()
   await page.waitForURL(/search\/(audio|image)/)
 }
+
+const defaultPayload = {
+  query: "cat",
+  searchType: "all",
+  collectionType: "null",
+  kind: "search",
+} as const
+
 /**
  * Cases, check both SSR and CSR:
  * 1. All content view with more than 1 page of results for each media type:
@@ -151,13 +160,11 @@ test.describe("Load more button", () => {
       (event) => event.n === "REACH_RESULT_END"
     )
 
-    expectEventPayloadToMatch(reachResultEndEvent, {
-      query: "cat",
-      searchType: "all",
+    const expectedPayload: Events["REACH_RESULT_END"] = {
+      ...defaultPayload,
       resultPage: 1,
-      collectionType: "null",
-      collectionValue: "null",
-    })
+    }
+    expectEventPayloadToMatch(reachResultEndEvent, expectedPayload)
   })
 
   test(`is sent when loading one page of results.`, async ({
@@ -173,13 +180,11 @@ test.describe("Load more button", () => {
       (event) => event.n === "LOAD_MORE_RESULTS"
     )
 
-    expectEventPayloadToMatch(loadMoreEvent, {
-      query: "cat",
-      searchType: "all",
+    const expectedPayload: Events["LOAD_MORE_RESULTS"] = {
+      ...defaultPayload,
       resultPage: 1,
-      collectionType: "null",
-      collectionValue: "null",
-    })
+    }
+    expectEventPayloadToMatch(loadMoreEvent, expectedPayload)
   })
 
   test(`is sent when loading two pages of results.`, async ({
@@ -199,15 +204,13 @@ test.describe("Load more button", () => {
     )
 
     expect(loadMoreEvents.length).toBe(2)
-    loadMoreEvents.every((event, index) =>
-      expectEventPayloadToMatch(event, {
-        query: "cat",
-        searchType: "all",
+    loadMoreEvents.every((event, index) => {
+      const expectedPayload: Events["LOAD_MORE_RESULTS"] = {
+        ...defaultPayload,
         resultPage: index + 1,
-        collectionType: "null",
-        collectionValue: "null",
-      })
-    )
+      }
+      return expectEventPayloadToMatch(event, expectedPayload)
+    })
   })
 
   test(`is not sent when more results are not loaded.`, async ({
diff --git a/frontend/test/playwright/utils/analytics.ts b/frontend/test/playwright/utils/analytics.ts
index 93497986391..65ca5a7e296 100644
--- a/frontend/test/playwright/utils/analytics.ts
+++ b/frontend/test/playwright/utils/analytics.ts
@@ -8,21 +8,25 @@ import type { EventName, Events } from "#shared/types/analytics"
 
 export type EventResponse<T extends EventName> = {
   n: T
-  p: Events[T]
+  p: Events[T] & { width: number; height: number; breakpoint: string }
 }
 
 export type AnalyticEventResponses = Array<EventResponse<EventName>>
 
 export function expectEventPayloadToMatch<T extends EventName>(
   event: EventResponse<T> | undefined,
-  expectedPayload: Events[T]
+  expectedPayload: Omit<Events[T], "width" | "height" | "breakpoint">
 ): void {
   expect(
     event,
     `Event not captured; expected payload of ${JSON.stringify(expectedPayload)}`
   ).toBeDefined()
   // Safe to cast as previous line ensures it is defined
-  expect((event as EventResponse<T>).p).toMatchObject(expectedPayload)
+  const p = (event as EventResponse<T>).p
+  // eslint-disable-next-line @typescript-eslint/no-unused-vars
+  const { width, height, breakpoint, ...rest } = p
+
+  expect(rest as Record<string, string>).toEqual(expectedPayload)
 }
 
 export const collectAnalyticsEvents = (context: BrowserContext) => {