Pass _ga client id to external signon services #1483
Conversation
test/controllers/service_sign_in_content_item_controller_test.rb
Outdated
Show resolved
Hide resolved
|
I'm going to do some reading to find out whether the cookie will work, or if we need to pass a value generated by the analytics.js library instead. |
bilbof
force-pushed
the
include-ga-param-in-signins
branch
from
2ed6c11 to
2431c7a
Compare
|
@andysellick I need to get the client id from the analytics.js library, rather than simply using the cookie, so I've added a bit of JS to do that magic. Please could I get some more 👀 on this? |
On pages such as https://www.gov.uk/log-in-file-self-assessment-tax-return/sign-in/prove-identity we redirect users to services such as tax.service.gov.uk to complete their sign in. This will pass the client id as a _ga param as part of cross-domain tracking, so that we can measure user journeys across domains. This will only pass the _ga param to other .gov.uk services. Currently these are: tax.service.gov.uk www.signin.service.gov.uk www.ruralpayments.service.gov.uk Further work is required on the signin.service.gov.uk to make use of the _ga param and ensure it is included in redirects. Trello: https://trello.com/c/ORdXWurC/101
bilbof
force-pushed
the
include-ga-param-in-signins
branch
from
2431c7a to
a7f69c5
Compare
| window.ga(function(tracker) { | ||
| var clientId = tracker.get('clientId') | ||
| var action = form.attr('action') | ||
| form.attr('action', action + "?_ga=" + clientId) |
There was a problem hiding this comment.
I guess this won't work as intended if the action URL already includes query parameters. Do you know if any action URLs include query parameters?
There was a problem hiding this comment.
The form currently submits without query parameters for all URLs (apart from the _ga param).
I checked the service sign in pages published by publisher from https://github.com/alphagov/publisher/tree/master/lib/service_sign_in and they seem to work (some of those rendered by the Heroku app are linked to in the description).
I considered using url = new URL(action) and then do something like url.searchParams.set('_ga', 'clientId') which would handle the case of multiple query params. But the constructor isn't supported by Internet Explorer (https://developer.mozilla.org/en-US/docs/Web/API/URL#Browser_compatibility).
| if ($form.length) { | ||
| new GOVUK.SetGaClientIdOnForm({ $form: $form }) | ||
| } | ||
| }) |
There was a problem hiding this comment.
It looks like the module system in the govuk_frontend_toolkit would be able to handle this. https://github.com/alphagov/govuk_frontend_toolkit/blob/master/docs/javascript.md#modules
That's used elsewhere in this app, in the track-radio-group.js file. Is that something you considered?
There was a problem hiding this comment.
Yes I can do, I'd need to roll the behaviour into the track-radio-group module since that module is already applied to the form at https://github.com/alphagov/government-frontend/pull/1483/files#diff-1e002ab60ce763d5443df324a62b2bcfR17, unless there's a way to apply two modules to the same element?
There was a problem hiding this comment.
Hmm, I didn't anticipate a one module per element restriction, but it looks from the code like that's the case [1].
In which case, I'd just stick with your current approach.
On pages such as https://www.gov.uk/log-in-file-self-assessment-tax-return/sign-in/prove-identity
we redirect users to services such as tax.service.gov.uk to complete their sign in.
This will pass the
_gacookie as the_gaparam as part of cross-domain tracking, so that we can measure user journeys across domains.This will only pass the _ga param to other .gov.uk services. Currently these are:
Pages to check:
The urls we redirect to are defined in service_sign_on content item
details.choose_sign_in.options(example).Further work is required on the signin.service.gov.uk to make use of the _ga param and ensure it is included in further redirects.
Trello: https://trello.com/c/ORdXWurC/101