Add GA4 to form validation errors #9148
Conversation
lauraghiorghisor-tw
force-pushed
the
add-ga4-for-form-validation
branch
5 times, most recently
from
9f00e04
to
454952d
Compare
| @@ -1,13 +1,4 @@ | |||
| <% if flash["alert"].present? %> | |||
There was a problem hiding this comment.
@minhngocd Not super sure what the logic here was meant to achieve.
There was a problem hiding this comment.
this is to track flash alerts back in the days i guess - if you check the commit message it says
Ensure flash-danger Google Analytics event is retained
We've moved away from using flash[:alert] in favour of the error summary component when there are errors on an object.
However, we want to retain the GA event that is triggered by the flash. To get around this I've ensured a flash["alert"] is only rendered on the page when the error summary block is empty.
This will ensure we only ever get 1 banner on the page. To ensure we still get the GA event i've wrapped the error summary component in a div which tracks the flash["alert"].
There was a problem hiding this comment.
I dug it up too. I just find it a bit confusing/convoluted. We're still firing
data_attributes: track_analytics_data("form-error", analytics_action, error.full_message) from within the error_summary_component.rb so it feels a bit duplicated.
I can leave it in and we can make this call when we implement whatever schema we want for flash, as part of the migration to GA4.
There was a problem hiding this comment.
update on conversations - we've discussed with Carmen, and believe if the user doesn't see the flash as a separate thing, then we should not need to see it as a separate event. we can remove this logic to reduce the duplication
| @@ -69,7 +69,7 @@ class Admin::StatisticsAnnouncementPublicationsControllerTest < ActionController | |||
|
|
|||
| assert_response :success | |||
| has_search_results_table | |||
| assert_select "ul.govuk-error-summary__list a[data-track-action='statistics announcement-error'][data-track-label=\"Publication type does not match: must be statistics\"]", text: "Publication type does not match: must be statistics" | |||
There was a problem hiding this comment.
@minhngocd not sure why this was ever needed
There was a problem hiding this comment.
probably to test that we are sending that tracking data by setting those attributes
There was a problem hiding this comment.
It is tested elsewhere, the test is not about tracking, the test does not require tracking selector to pass. 🤷🏻♀️
lauraghiorghisor-tw
force-pushed
the
add-ga4-for-form-validation
branch
2 times, most recently
from
d4805a3
to
c8beb7f
Compare
lauraghiorghisor-tw
force-pushed
the
add-ga4-for-form-validation
branch
from
c8beb7f
to
1017629
Compare
| def ga4_title | ||
| return object.class.name.humanize if [ActiveModel::Errors, Array].include?(object.class) | ||
|
|
||
| "#{object.try(:new_record?) ? 'New' : 'Editing'} #{object.model_name.human.downcase.titleize}" |
There was a problem hiding this comment.
we do this so many times you it warrants a helper method 👀 but also happy to refactor as a separate piece of work
We want to be able to track validation errors on the page. This is in the context of the visual editor tool, albeit validation errors are not very relevant in that case.
We are looking to also migrate UA to GA4 so these changes also remove UA tracking for the
ErrorSummarycomponent.Trello card