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

Use ATI View Tracking Public API in ATIAnalytics Container #5304

Closed
1 task
andrewscfc opened this issue Jan 29, 2020 · 1 comment
Closed
1 task

Use ATI View Tracking Public API in ATIAnalytics Container #5304

andrewscfc opened this issue Jan 29, 2020 · 1 comment
Labels
investigation Refinement Needed This is ready for refinement. It shouldn't be worked on until it has been refined by Dev & Test. technical-work Technical debt, support work and building new technical tools and features ws-articles Tasks for the WS Articles Team

Comments

@andrewscfc
Copy link
Contributor

Is your feature request related to a problem? Please describe.
This issue intends to resolve a confusing and duplicated set of logic that made #5233 take longer to implement and opened us up to a buggy implementation going live.

  1. As it stands in the ATIAnalytics container, we build the params to send to ATI using the respective 'param builder' for each asset type:
    https://github.com/bbc/simorgh/blob/807a9816fdb8568ba28c29065ac9b7906b616967/src/app/containers/ATIAnalytics/index.jsx#L6..L10
  2. We switch to each 'param builder' based on asset type:
    https://github.com/bbc/simorgh/blob/807a9816fdb8568ba28c29065ac9b7906b616967/src/app/containers/ATIAnalytics/index.jsx#L17..L46
  3. Then dispatch our page view to ATI:
    https://github.com/bbc/simorgh/blob/807a9816fdb8568ba28c29065ac9b7906b616967/src/app/containers/ATIAnalytics/index.jsx#L48..L53

An API already exists that does much of 1 and 2 above but we have not used it in ATIAnalytics:

export default buildATIUrl;

You can see the duplicated logic here https://github.com/bbc/simorgh/blob/807a9816fdb8568ba28c29065ac9b7906b616967/src/app/containers/ATIAnalytics/params/index.js#L38..L56

Describe the solution you'd like
Ideally we would like to use the API rather than duplicate logic, we did quickly try this but got quite a few errors, there may be some work getting the API fit for use.

Describe alternatives you've considered
Potentially the the API could be deleted instead, resolving the duplication. If we were to do this, we would have to consider where the currently unused 'click tracking' logic would go:

export const buildATIClickParams = (data, requestContext, serviceContext) => {

Testing notes
[Tester to complete]

Dev insight: Will Cypress tests be required or are unit tests sufficient? Will there be any potential regression? etc

  • This feature is expected to need manual testing.

Additional context
Add any other context or screenshots about the feature request here.

@andrewscfc andrewscfc added the Refinement Needed This is ready for refinement. It shouldn't be worked on until it has been refined by Dev & Test. label Jan 29, 2020
@andrewscfc andrewscfc changed the title Use ATI View Tracking Public Interface in ATIAnalytics Container Use ATI View Tracking Public API in ATIAnalytics Container Jan 29, 2020
@andrewscfc andrewscfc added the ws-articles Tasks for the WS Articles Team label Jan 29, 2020
@karinathomasbbc karinathomasbbc added technical-work Technical debt, support work and building new technical tools and features and removed tech-debt labels Jul 21, 2020
@andrewscfc andrewscfc removed their assignment Aug 26, 2020
@joshcoventry
Copy link
Contributor

Tackle as part of #5743

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
investigation Refinement Needed This is ready for refinement. It shouldn't be worked on until it has been refined by Dev & Test. technical-work Technical debt, support work and building new technical tools and features ws-articles Tasks for the WS Articles Team
Projects
None yet
Development

No branches or pull requests

3 participants