-
Notifications
You must be signed in to change notification settings - Fork 17
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 GA4 event data on taxon pages #3896
Conversation
app/views/taxons/show.html.erb
Outdated
@@ -62,7 +62,7 @@ | |||
} | |||
%> | |||
<%= content_tag(:div, data: ga4_data) do %> | |||
<%= render(partial: section[:partial_template], locals: { section: section }) %> | |||
<%= render(partial: section[:partial_template], locals: { section: section, disable_ga4: true }) %> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Putting a false
default into the partials seems sensible, but is there a situation where that isn't overridden by this line?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could not see any other uses of the section partials outside of the taxon page where the change is required.
My initial option was to hardcode disable_ga4: true
in each section partial, happy to go with this approach if preferred
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that might be simpler - otherwise it implies that this is necessary because some other part of the code doesn't pass true
, which isn't correct. We can always extend it later if needed 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, thanks Andy, I've made this change
Set `disable_ga4` to true for the document-list component used to render each section of the taxon page. This fixes an issue on taxon pages (/education for example), where link clicks would fire 2 events instead of 1, both of which were missing some event data.
9c01f80
to
3a7aec6
Compare
What
Set
disable_ga4
to true for the document-list component used to render each section on a taxon page.Why
This fixes an issue on taxon pages (/education for example), where link clicks would fire 2 events instead of 1, both of which were missing some data.
Event data changes
The data below is sent to GA after clicking the "Student finance for undergraduates" link on this page - https://www.gov.uk/education/funding-and-finance-for-students#/education/student-grants-bursaries-scholarships
Before
After
Only 1 event is fired and information for "index_section", "index_section_count" and "section" is now included.