Analytics event: BACK_TO_SEARCH #1118

masif2002 · 2023-04-03T01:20:09Z

Fixes

Description

Adds BACK_TO_SEARCH event to the Events type.
Fires the event from VBackToSearchResultsLink.vue using the useAnalytics composable.

sarayourfriend

This is on the right path, but needs at least one additional change to make it work. Please pass the new id and media type props to the component in the image/_id/index.vue and audio/_id/index.vue page components. Please also add a unit test to confirm the analytics event is called when the component is clicked.

If possible, please also test the change locally to verify that it is working:

Run just frontend/up && just frontend/init
Run the app locally: just frontend/run dev
Log into plausible by visiting http://0.0.0.0:50288 and using the default username and password [email protected] and deploy
Add the custom event as a goal in http://0.0.0.0:50288/localhost/settings/goals
Verify that clicking the back to search link populates the goal on the main Plausible page for the localhost site (http://0.0.0.0:50288/localhost) at the bottom under the Goal Conversions section

@dhruvkb I'm writing a page for the docs site with this basic setup information so we can link it on these issues for adding new events. Otherwise, anyone who didn't review the PRs adding Plausible would be a little lost on how to start testing it locally.

It would probably also be worthwhile to have a quick PR to set up testing utilities like an easy mock to use to test the function is called as expected, to make it easier for new contributors, as the issues are marked as "good first issue".

masif2002 · 2023-04-03T05:40:28Z

Will make the changes and test it locally @sarayourfriend

I have never written unit tests before. Is there something that can get me started on this?

sarayourfriend · 2023-04-03T06:28:38Z

Sure. Openverse uses jest, a popular JavaScript testing library: https://jestjs.io/

Their documentation is fairly comprehensive, and we have lots of examples in our repository for other tests. A good basic example for Openverse specifically is the test suite for VLicense: https://github.com/wordpress/openverse/blob/7c92ba5bdff84b2771dc25a946bb371600906e0e/frontend/test/unit/specs/components/v-license.spec.js#L1

We also use testing-library which you would need to use in the case of the tests for this change by causing a click event on the rendered component. The basic example for Vue testing includes an example for how to fire click events on a rendered component.

You'll need to "spy" on the sendCustomEvent function that is returned by useAnalytics. To do that, it will be necessary to mock useAnalytics, similar to this useI18n mock from our own test suite: https://github.com/wordpress/openverse/blob/7c92ba5bdff84b2771dc25a946bb371600906e0e/frontend/test/unit/specs/components/AudioTrack/v-audio-track.spec.js#L38-L43

Something like:

jest.mock("~/composables/use-analytics", () => ({
  useAnalytics: jest.fn(() => ({
    sendCustomEvent: jest.fn()
  })
})

Then you can import the mocked useAnalytics function to retrieve the mocked sendCustomEvent and follow jest's documentation for asserting the calls work as expected: https://jestjs.io/docs/mock-functions

The mock for useI18n is an instructive example.

dhruvkb

@masif2002 thanks for the PR, it's on the right track but needs a little more work before it can be merged.

dhruvkb · 2023-04-03T11:01:37Z

frontend/src/components/VBackToSearchResultsLink.vue

@@ -3,14 +3,18 @@
  <VLink
    class="time inline-flex flex-row items-center gap-2 rounded-sm p-2 text-xs font-semibold text-dark-charcoal-70 pe-3 hover:text-dark-charcoal"
    v-bind="$attrs"
+    @click="handleClick()"


When referring to an event handler function, the parenthesis are not used (because that would invoke the function).

Suggested change

@click="handleClick()"

@click="handleClick"

dhruvkb · 2023-04-03T11:03:01Z

frontend/src/components/VBackToSearchResultsLink.vue

@@ -26,9 +30,37 @@ export default defineComponent({
    VIcon,
    VLink,
  },
+  props: {


These must be added below the inheritAttrs field to match the recommended option order.

Also, if you add new required props to a component, you will need to update the usage of that component. This particular component VBackToSearchResultsLink.vue is used in the following locations, which need to be updated:

frontend/src/pages/audio/_id/index.vue

frontend/src/pages/image/_id/index.vue

frontend/src/components/meta/VBackToSearchResultsLink.stories.mdx

dhruvkb · 2023-04-03T11:05:53Z

frontend/src/types/analytics.ts

+   * - Are these links used much? Are they necessary?
+   */
+  BACK_TO_SEARCH: {
+    /** The unique ID of the media */


Small grammar nit.

Suggested change

/** The unique ID of the media */

/** the unique ID of the media */

dhruvkb · 2023-04-03T11:06:07Z

frontend/src/types/analytics.ts

+  BACK_TO_SEARCH: {
+    /** The unique ID of the media */
+    id: string
+    /** The media type being searched */


Small grammar nit.

Suggested change

/** The media type being searched */

/** the media type being searched */

masif2002 · 2023-04-04T05:51:43Z

Hey @sarayourfriend .. I tried testing the changes locally. I set the BACK_TO_SEARCH event as a goal, but it does not show up in the Goal Conversions section :/

sarayourfriend · 2023-04-04T06:23:00Z

I tried testing the changes locally. I set the BACK_TO_SEARCH event as a goal, but it does not show up in the Goal Conversions section :/

Have you confirmed that the sendCustomEvent function is being called as expected? I won't be able to take a closer look at this PR until next Tuesday, but maybe @dhruvkb can help debug if there's a deeper issue.

masif2002 · 2023-04-04T12:06:01Z

Have you confirmed that the sendCustomEvent function is being called as expected?

Nope, the function is not invoked on click.. I'm trying to figure out why

dhruvkb

@masif the code looks fine to me, but yes, the handler is not being fired on the click event. For some reason, attaching the @click handler to the VLink does not work here. I'll tag @obulat for help in debugging this issue.

dhruvkb · 2023-04-05T23:57:40Z

frontend/src/components/VBackToSearchResultsLink.vue

@@ -3,14 +3,18 @@
  <VLink
    class="time inline-flex flex-row items-center gap-2 rounded-sm p-2 text-xs font-semibold text-dark-charcoal-70 pe-3 hover:text-dark-charcoal"
    v-bind="$attrs"
+    @click="handleClick"


I checked out the PR locally and it seems like this handler is not being called, despite being perfectly ok from a code standpoint. In fact, the identical code from a different component VHomeGallery.vue works just as expected.

openverse/frontend/src/components/VHomeGallery/VHomeGallery.vue

Line 32 in 5106dc3

@click="handleClick(image.identifier)"

Yes Dhruv, the CLICK_HOME_GALLERY_IMAGE gets fired as expected, but BACK_TO_SEARCH event doesn't

We'll try to investigate, but in the meantime please feel free to work on another issue if you'd like.

We'll try to investigate, but in the meantime please feel free to work on another issue if you'd like.

Sure, thanks!

obulat

Thank you for your contribution, @masif2002 !

I've tried debugging the issue locally. The link's click event is fired after the page transition is complete and the button is already unmounted. That's why the analytics event is not fired.

I found a (somewhat hacky) solution: we can add a mousedown.native event to the VLink component, and use that for firing the analytics event.

Here are the changes that worked for me locally:

--- a/frontend/src/components/VBackToSearchResultsLink.vue
+++ b/frontend/src/components/VBackToSearchResultsLink.vue
-    @click="handleClick"
+    @mousedown="handleClick"
   >
     <VIcon :icon-path="chevronIcon" :rtl-flip="true" />
     {{ $t("single-result.back") }}
diff --git a/frontend/src/components/VLink.vue b/frontend/src/components/VLink.vue
--- a/frontend/src/components/VLink.vue
+++ b/frontend/src/components/VLink.vue
@@ -5,6 +5,7 @@
     :class="{ 'inline-flex flex-row items-center gap-2': showExternalIcon }"
     :to="linkTo"
     v-on="$listeners"
+    @mousedown.native="$emit('mousedown', $event)"
     @click.native="$emit('click', $event)"
   >

masif2002 · 2023-04-06T07:09:58Z

I've tried debugging the issue locally. The link's click event is fired after the page transition is complete and the button is already unmounted. That's why the analytics event is not fired.

Awesome, that's good to know. How did you find out that the event is fired only after the page transition?

masif2002 · 2023-04-06T07:11:44Z

I found a (somewhat hacky) solution: we can add a mousedown.native event to the VLink component, and use that for firing the analytics event.

Yea this one works, thank you!
@dhruvkb Should i commit the changes?

dhruvkb · 2023-04-06T08:21:00Z

@obulat how does this work in VHomeGallery.vue?

obulat · 2023-04-06T10:31:29Z

I've tried debugging the issue locally. The link's click event is fired after the page transition is complete and the button is already unmounted. That's why the analytics event is not fired.

Awesome, that's good to know. How did you find out that the event is fired only after the page transition?

In the VLink component, I added a route watcher that logged when the route changed, and an onUnmounted that logged when the link was unmounted, with its href. Not the most optimal way of debugging, but it worked :)

masif2002 · 2023-04-09T02:35:23Z

In the VLink component, I added a route watcher that logged when the route changed, and an onUnmounted that logged when the link was unmounted, with its href. Not the most optimal way of debugging, but it worked :)

Wow, that's great! But as @dhruvkb said, how does the analytics event work as is in VHomeGallery.vue because there is also a page transition happening on click

obulat · 2023-04-12T06:06:42Z

@obulat how does this work in VHomeGallery.vue?

I think I found the answer to this. When you click on a link on the home page, you navigate to a different page that has a different layout. The same is true when you click on a page link from a search page. However, here, we stay in the same layout. This is the only difference between the events.

sarayourfriend · 2023-04-12T08:00:12Z

@obulat Does that mean that when navigating between base layouts, Vue is, for some reason, keeping the previous layout mounted behind the scenes until it finishes triggering the event? It's interesting that that is the only difference, but I'm super curious why having different layouts between the navigated pages causes the difference in behaviour... 🤔

obulat · 2023-04-27T14:23:52Z

@obulat Does that mean that when navigating between base layouts, Vue is, for some reason, keeping the previous layout mounted behind the scenes until it finishes triggering the event? It's interesting that that is the only difference, but I'm super curious why having different layouts between the navigated pages causes the difference in behaviour... 🤔

Apparently, this is a regression that happened in Vue 2.7.x: nuxt/nuxt#10593 and vuejs/vue#12781. I think with a mousedown we have a good enough workaround and don't need to investigate further.

obulat

@masif2002, I rebased your PR onto main.
Because the CI did not start, I replaced the @click with @mousedown to trigger it. We might need to change it after testing.

This PR needs unit tests. Are you interested in writing them? You can find examples in #1180. If not, please let us know and we can help finish your work in this PR.

frontend/src/components/VBackToSearchResultsLink.vue

masif2002 · 2023-04-28T00:51:33Z

Hey @obulat ! I'm swamped right now and won't be able to write the unit tests for this PR. Thanks for offering to help out, though - please go ahead and take care of it.

obulat · 2023-04-28T11:09:58Z

Hey @obulat ! I'm swamped right now and won't be able to write the unit tests for this PR. Thanks for offering to help out, though - please go ahead and take care of it.

Thank you for quick response, @masif2002! I'll add tests, then. You are always welcome to contribute when you have more time 😃

frontend/src/components/meta/VBackToSearchResultsLink.stories.mdx

sarayourfriend

LGTM. Just small questions but no blockers. Questions are for @obulat, to be clear.

frontend/test/unit/setup.js

frontend/test/unit/specs/components/v-back-to-search-results-link.spec.js

masif2002 requested a review from a team as a code owner April 3, 2023 01:20

masif2002 requested review from zackkrida and dhruvkb April 3, 2023 01:20

sarayourfriend requested changes Apr 3, 2023

View reviewed changes

dhruvkb reviewed Apr 3, 2023

View reviewed changes

masif2002 requested a review from dhruvkb April 4, 2023 05:51

obulat removed the 🚦 status: awaiting triage Has not been triaged & therefore, not ready for work label Apr 4, 2023

dhruvkb reviewed Apr 5, 2023

View reviewed changes

obulat mentioned this pull request Apr 6, 2023

just frontend/init fails #1142

Closed

obulat reviewed Apr 6, 2023

View reviewed changes

obulat force-pushed the analytics-event-BACK_TO_SEARCH branch from dc1f52f to 347ccc2 Compare April 27, 2023 14:04

obulat requested a review from a team as a code owner April 27, 2023 14:04

obulat changed the base branch from main_old to main April 27, 2023 14:13

obulat reviewed Apr 27, 2023

View reviewed changes

frontend/src/components/VBackToSearchResultsLink.vue Outdated Show resolved Hide resolved

obulat changed the title ~~Add analytic event: BACK_TO_SEARCH~~ Analytics event: BACK_TO_SEARCH Apr 27, 2023

obulat reviewed Apr 28, 2023

View reviewed changes

frontend/src/components/meta/VBackToSearchResultsLink.stories.mdx Outdated Show resolved Hide resolved

obulat self-assigned this Apr 30, 2023

obulat approved these changes Apr 30, 2023

View reviewed changes

obulat requested a review from sarayourfriend May 1, 2023 12:13

obulat force-pushed the analytics-event-BACK_TO_SEARCH branch from e31bc27 to 19e4dfc Compare May 1, 2023 12:20

zackkrida requested a review from dhruvkb May 1, 2023 18:51

masif2002 and others added 7 commits May 2, 2023 07:39

Add analytic event: BACK_TO_SEARCH

5145046

Added required props to component

6992eb4

Lint

24c3f3f

Update frontend/src/components/VBackToSearchResultsLink.vue

93b40b9

Use searchType instead of mediaType; add mousedown; add tests

5037f77

Use UI to turn on Analytics

15c3c08

Update frontend/src/components/meta/VBackToSearchResultsLink.stories.mdx

8d85236

obulat force-pushed the analytics-event-BACK_TO_SEARCH branch from 19e4dfc to 8d85236 Compare May 2, 2023 04:39

sarayourfriend approved these changes May 2, 2023

View reviewed changes

frontend/test/unit/setup.js Outdated Show resolved Hide resolved

frontend/test/unit/specs/components/v-back-to-search-results-link.spec.js Show resolved Hide resolved

Apply changes from code review

5d03035

obulat force-pushed the analytics-event-BACK_TO_SEARCH branch from 73fe26c to 5d03035 Compare May 2, 2023 05:53

obulat merged commit 2a37692 into WordPress:main May 2, 2023

obulat mentioned this pull request May 2, 2023

Update Back to results button to use the new VButton variant and size #1141

Merged

7 tasks

dhruvkb mentioned this pull request Jun 16, 2023

Usage Analytics #394

Closed

26 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Analytics event: BACK_TO_SEARCH #1118

Analytics event: BACK_TO_SEARCH #1118

masif2002 commented Apr 3, 2023

sarayourfriend left a comment •

edited by zackkrida

Loading

masif2002 commented Apr 3, 2023

sarayourfriend commented Apr 3, 2023

dhruvkb left a comment

dhruvkb Apr 3, 2023

dhruvkb Apr 3, 2023

dhruvkb Apr 3, 2023

dhruvkb Apr 3, 2023

masif2002 commented Apr 4, 2023

sarayourfriend commented Apr 4, 2023

masif2002 commented Apr 4, 2023 •

edited

Loading

dhruvkb left a comment •

edited

Loading

dhruvkb Apr 5, 2023

masif2002 Apr 6, 2023

dhruvkb Apr 6, 2023

masif2002 Apr 6, 2023

obulat left a comment

masif2002 commented Apr 6, 2023

masif2002 commented Apr 6, 2023

dhruvkb commented Apr 6, 2023

obulat commented Apr 6, 2023

masif2002 commented Apr 9, 2023

obulat commented Apr 12, 2023

sarayourfriend commented Apr 12, 2023

obulat commented Apr 27, 2023

obulat left a comment

masif2002 commented Apr 28, 2023

obulat commented Apr 28, 2023

sarayourfriend left a comment •

edited

Loading

	/** The unique ID of the media */
	/** the unique ID of the media */

	/** The media type being searched */
	/** the media type being searched */

Analytics event: BACK_TO_SEARCH #1118

Analytics event: BACK_TO_SEARCH #1118

Conversation

masif2002 commented Apr 3, 2023

Fixes

Description

sarayourfriend left a comment • edited by zackkrida Loading

Choose a reason for hiding this comment

masif2002 commented Apr 3, 2023

sarayourfriend commented Apr 3, 2023

dhruvkb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

masif2002 commented Apr 4, 2023

sarayourfriend commented Apr 4, 2023

masif2002 commented Apr 4, 2023 • edited Loading

dhruvkb left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

obulat left a comment

Choose a reason for hiding this comment

masif2002 commented Apr 6, 2023

masif2002 commented Apr 6, 2023

dhruvkb commented Apr 6, 2023

obulat commented Apr 6, 2023

masif2002 commented Apr 9, 2023

obulat commented Apr 12, 2023

sarayourfriend commented Apr 12, 2023

obulat commented Apr 27, 2023

obulat left a comment

Choose a reason for hiding this comment

masif2002 commented Apr 28, 2023

obulat commented Apr 28, 2023

sarayourfriend left a comment • edited Loading

Choose a reason for hiding this comment

sarayourfriend left a comment •

edited by zackkrida

Loading

masif2002 commented Apr 4, 2023 •

edited

Loading

dhruvkb left a comment •

edited

Loading

sarayourfriend left a comment •

edited

Loading