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

Add UTM parameters to Dashboard login buttons #7639

Merged
merged 12 commits into from
Jun 19, 2020
Merged

Add UTM parameters to Dashboard login buttons #7639

merged 12 commits into from
Jun 19, 2020

Conversation

panzarino
Copy link
Contributor

@panzarino panzarino commented Jun 9, 2020

Additional details

Adds tracking to dashboard login links

PR Tasks

  • Have tests been added/updated?
  • Has the original issue been tagged with a release in ZenHub?

@cypress-bot
Copy link
Contributor

cypress-bot bot commented Jun 9, 2020

Thanks for taking the time to open a PR!

@cypress
Copy link

cypress bot commented Jun 9, 2020



Test summary

7655 0 119 0


Run details

Project cypress
Status Passed
Commit 6f44bce
Started Jun 19, 2020 8:09 PM
Ended Jun 19, 2020 8:16 PM
Duration 07:29 💡
OS Linux Debian - 10.1
Browser Multiple

View run in Cypress Dashboard ➡️


This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

Copy link
Member

@jennifer-shehane jennifer-shehane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably add a test here, since we're not technically testing that this link in the project setup modal is correctly routing to the dashboard route.

https://github.com/cypress-io/cypress/blob/develop/packages/desktop-gui/cypress/integration/setup_project_modal_spec.js#L522:L522

@panzarino
Copy link
Contributor Author

@jennifer-shehane Turns out I was putting the UTM parameters on the wrong links, I've updated it now.

I'm fairly unfamiliar as to how to write a full e2e test that will check for full functionality here as these changes span both desktop-gui and server. I'm also unsure of whether Google Analytics will actually pick up on the UTM parameters when sent through this method, so essentially there needs to be additional testing all around.

Copy link
Contributor

@flotwig flotwig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fairly unfamiliar as to how to write a full e2e test that will check for full functionality here as these changes span both desktop-gui and server. I'm also unsure of whether Google Analytics will actually pick up on the UTM parameters when sent through this method, so essentially there needs to be additional testing all around.

@panzarino I don't think you need a full e2e test for this. Probably just a Cypress test in desktop-gui that verifies that the UTM code is sent to the IPC channel, and then a unit test in the server auth_spec that verifies that the expected URL is generated. You could also add a test in events_spec that the IPC event calls beginAuth properly.

packages/server/lib/gui/auth.js Outdated Show resolved Hide resolved
packages/server/lib/gui/auth.js Outdated Show resolved Hide resolved
@panzarino panzarino requested a review from flotwig June 16, 2020 17:08
Copy link
Contributor

@flotwig flotwig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add UTM parameters to Dashboard login buttons [TR-37]
3 participants