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

Run search term standardise function on GA4 search event #3229

Merged
merged 1 commit into from
Dec 8, 2023

Conversation

AshGDS
Copy link
Contributor

@AshGDS AshGDS commented Dec 6, 2023

⚠️ This repo is Continuously Deployed: make sure you follow the guidance ⚠️

Follow these steps if you are doing a Rails upgrade.

What

Run standardiseSearchTerm on the GA4 search event, so that it looks the same as all the other search terms that come through e.g. in the ecommerce and pageview events.

Why

https://trello.com/c/J5XcLlvh/743-collect-undefined-instead-of-when-removing-search-terms-on-finders

@AshGDS AshGDS added the do-not-merge Indicates that a PR should not be merged into master / release branches label Dec 6, 2023
@govuk-ci govuk-ci temporarily deployed to finder-frontend-pr-3229 December 6, 2023 15:12 Inactive
@AshGDS AshGDS force-pushed the ga4-standardise-search-term branch from a680c1b to c58fe8a Compare December 6, 2023 15:18
@govuk-ci govuk-ci temporarily deployed to finder-frontend-pr-3229 December 6, 2023 15:19 Inactive
@AshGDS AshGDS force-pushed the ga4-standardise-search-term branch from c58fe8a to 102e5ff Compare December 8, 2023 10:09
@govuk-ci govuk-ci temporarily deployed to finder-frontend-pr-3229 December 8, 2023 10:09 Inactive
@AshGDS AshGDS changed the title [DO NOT MERGE] Run search term standardise function on GA4 search event Run search term standardise function on GA4 search event Dec 8, 2023
@AshGDS AshGDS removed the do-not-merge Indicates that a PR should not be merged into master / release branches label Dec 8, 2023
@AshGDS AshGDS force-pushed the ga4-standardise-search-term branch from 102e5ff to 47084e6 Compare December 8, 2023 10:16
@govuk-ci govuk-ci temporarily deployed to finder-frontend-pr-3229 December 8, 2023 10:17 Inactive
@AshGDS AshGDS requested a review from andysellick December 8, 2023 10:18
Copy link
Contributor

@andysellick andysellick left a comment

Choose a reason for hiding this comment

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

Nice. Does this need documenting anywhere, or did you already do enough in the gem?

@govuk-ci govuk-ci temporarily deployed to finder-frontend-pr-3229 December 8, 2023 11:16 Inactive
@govuk-ci govuk-ci temporarily deployed to finder-frontend-pr-3229 December 8, 2023 11:20 Inactive
@AshGDS
Copy link
Contributor Author

AshGDS commented Dec 8, 2023

Thanks @andysellick - I have added a couple commits to this branch as I realised I can add another test, as well as updating the documentation.

I have created a PR for govuk-developer-docs: alphagov/govuk-developer-docs#4335

Would you be able to review the extra commits I've added?

Thanks 👍

@AshGDS AshGDS force-pushed the ga4-standardise-search-term branch from d5984e0 to 0107e65 Compare December 8, 2023 14:48
@govuk-ci govuk-ci temporarily deployed to finder-frontend-pr-3229 December 8, 2023 14:49 Inactive
@AshGDS AshGDS merged commit 98f0152 into main Dec 8, 2023
12 checks passed
@AshGDS AshGDS deleted the ga4-standardise-search-term branch December 8, 2023 14:55
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.

3 participants