Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 fix double-logging data catalog searches #4553

Merged
merged 1 commit into from
Feb 13, 2025

Conversation

ikesau
Copy link
Member

@ikesau ikesau commented Feb 11, 2025

Fixes #4316

Context

The data catalog was logging searches to google analytics twice. You can see this on prod by going to https://ourworldindata.org/data, entering a query, and then opening the dev console and typing dataLayer.at(-1) and dataLayer.at(-2)

Problem

The data catalog ran all of its business logic through a single useEffect, which meant it called anytime any of its dependencies changed. Analytics was one of the last things I added and I didn't stop to think that this callback would fire anytime any of the dependencies updated, including updates to the cache or even no-op calls to update the state.

The fix

Extracts the analytics logic into an independent effect, which only runs when stateAsUrl changes (which always corresponds to a search running, even when the results are restored from the cache)

How to test

Add a console.log to logDataCatalogSearch and run some searches locally, noting that a log call only happens when a new query is run.

logDataCatalogSearch(state: DataCatalogState) {
        console.log("logging search", state);
        this.logToGA({
            event: EventCategory.DataCatalogSearch,
            eventAction: "search",
            eventContext: JSON.stringify({
                ...state,
                topics: Array.from(state.topics),
                selectedCountryNames: Array.from(state.selectedCountryNames),
            }),
        })
    }

@owidbot
Copy link
Contributor

owidbot commented Feb 11, 2025

Quick links (staging server):

Site Dev Site Preview Admin Wizard Docs

Login: ssh owid@staging-site-double-counting-data-catalog

SVG tester:

Number of differences (default views): 0 ✅
Number of differences (all views): 0 ✅

Edited: 2025-02-11 23:05:46 UTC
Execution time: 1.18 seconds

@ikesau ikesau force-pushed the double-counting-data-catalog-events branch from 6263eac to 65c42cb Compare February 12, 2025 22:25
@ikesau ikesau force-pushed the double-counting-data-catalog-events branch from 65c42cb to 1bbde92 Compare February 12, 2025 22:48
@ikesau ikesau requested a review from rakyi February 12, 2025 22:49
@ikesau ikesau marked this pull request as ready for review February 12, 2025 22:49
Copy link
Contributor

@rakyi rakyi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for the very clear description!

@ikesau ikesau merged commit 52a0c1a into master Feb 13, 2025
30 checks passed
@ikesau ikesau deleted the double-counting-data-catalog-events branch February 13, 2025 16:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Data catalog double counts Google Analytics search events
3 participants