refactor(ga): decouple Express.Request from analytics

[LoneRifle]

Problem

Closes #254

Solution

• Move biz logic for creating GA page view requests into its own func,
  so that we can more easily decouple Express.Request from analytics

  • Declare and implement createPageViewHit(), call that from
    sendPageViewHit()
  • Augment tests to expect mockAnalyticsLogger to record and compare
    page view hits, to facilitate later refactoring which changes
    func arguments for logRedirectAnalytics()

• Change analytics logging such that responsibility of creating
  view page requests to GA falls on the controller by invoking an
  utility function, which is then passed to analyticsLogger

  • Call createPageViewHit from controller, pass result to
    analyticsLogger
  • Change args to logRedirectAnalytics() such that it receives
    an object of generic type T, usually the type of the page view hit
    payload. Change GaLogger so that it implements
    AnalyticsLogger
  • Rework tests to account for changes in signature

• Re-org naming

Tests

To be done in staging, exercising GA

[liangyuanruo]

Can we go a bit further for this refactor and

1. remove the need to create a logRedirect function each time we have to make a redirect
2. remove the need to pass req to createPageViewHit? in fact, we could refactor to make createPageViewHit an internal function within logRedirectAnalytics and call that at the appropriate junction, instead of exposing it to RedirectController?

Main idea being that controllers parse req and delegate work to services, without needing to know the internals of how other functions work.

[LoneRifle]

2. remove the need to pass req to createPageViewHit? in fact, we could refactor to make createPageViewHit an internal function within logRedirectAnalytics and call that at the appropriate junction, instead of exposing it to RedirectController?

Wouldn't that defeat the purpose of this PR? Isn't the intent to ensure that only the controller deals with the handling of Express.Request before it gets passed to the Service?

Main idea being that controllers parse req and delegate work to services, without needing to know the internals of how other functions work.

I would contend that this is what the PR is doing - it's delegating work to other modules and doesn't actually know how a req is transformed into a pageViewHit

[liangyuanruo]

I would contend that this is what the PR is doing - it's delegating work to other modules and doesn't actually know how a req is transformed into a pageViewHit

True, but I still feel that the level of abstraction may be incomplete . If pageViewHit is only used by logRedirectAnalytics, then the logic could also be encapsulated as an internal function.

[LoneRifle]

While true, shouldn't the need to prevent Express.Request leaking into analyticsLogger take precedence?

[LoneRifle]

Given you've made the approval I will merge this in for now. We can discuss what further changes - if any - should be made at a later point