Skip to content

/ ghostery-extension

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

Jump to bottom

GH-1947 Plus checkout UTM params #499

Merged
merged 4 commits into from Feb 21, 2020
+28 −17
Merged

GH-1947 Plus checkout UTM params #499

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
da2976a
Add UTM params to Plus checkout from all locations
benstrumeyer Feb 18, 2020
190c99a
Update tests
benstrumeyer Feb 19, 2020
e06e516
Remove / from links with utm params
benstrumeyer Feb 19, 2020
1f5c6c8
Refactor messaging passing
benstrumeyer Feb 21, 2020
File filter
Filter file types
Jump to
Jump to file
Failed to load files.

Always

Just for now

Next
Loading status checks…
Add UTM params to Plus checkout from all locations
  • Loading branch information
@benstrumeyer
benstrumeyer committed Feb 18, 2020
commit da2976a1f45379abbd061996bc6f0982cbb29f14
2 _locales/en/messages.json
View file
Open in desktop
@@ -1936,7 +1936,7 @@
"content": "</a>"
},
"link_start": {
"content": "<br><a target=\"_blank\" rel=\"noopener noreferrer\" href=\"https://www.ghostery.com/products/plus/\">"
"content": "<br><a target=\"_blank\" rel=\"noopener noreferrer\" href=\"https://www.ghostery.com/products/plus/?utm_source=gbe&utm_campaign=in_app_hstats_lm\">"
}
This conversation was marked as resolved by Eden12345
Comment on lines 1938 to 1940

This comment has been minimized.

Sign in to view
@Eden12345

Eden12345 Feb 19, 2020
Contributor

The changed content in this string makes sense, but I'm just confused as to where it is being used as I was unable to find anywhere that it is referenced in the project.
}
},
2 app/hub/Views/PlusView/PlusView.jsx
View file
Open in desktop
@@ -29,7 +29,7 @@ class PlusView extends Component {
*/
_renderButton = (additionalClasses) => {
const { isPlus, onPlusClick } = this.props;
const buttonHref = `https://checkout.${globals.GHOSTERY_DOMAIN}.com/plus`;
const buttonHref = `https://checkout.${globals.GHOSTERY_DOMAIN}.com/plus/?utm_source=gbe&utm_campaign=intro_hub_plus`;
const buttonClassNames = ClassNames('PlusView__button', 'button', additionalClasses, {
disabled: isPlus,
});
2 app/panel/components/Stats.jsx
View file
Open in desktop
@@ -277,7 +277,7 @@ class Stats extends React.Component {
*/
subscribe = () => {
sendMessage('ping', 'hist_plus_cta');
openCheckoutPage();
openCheckoutPage('historical_stats');
}

/**
4 app/panel/components/Subscribe.jsx
View file
Open in desktop
@@ -21,7 +21,7 @@ import { sendMessage, openCheckoutPage } from '../utils/msg';
*/
function _handleBecomeClick() {
sendMessage('ping', 'plus_cta_extension');
openCheckoutPage();
openCheckoutPage('plus_panel');
}

/**
@@ -36,7 +36,7 @@ const Subscribe = (props) => {
<div className="pitch-container">
<span className="pitch-text" dangerouslySetInnerHTML={{ __html: t('subscribe_pitch') }} />
</div>
<PanelToTabLink href="https://www.ghostery.com/products/plus/">
<PanelToTabLink href="https://www.ghostery.com/products/plus/?utm_source=gbe&utm_campaign=in_app_plus_lm">
<span className="pitch-learn-more">{t('subscribe_pitch_learn_more')}</span>
</PanelToTabLink>
<div>
6 app/panel/utils/msg.js
View file
Open in desktop
@@ -124,8 +124,10 @@ export function openSubscriptionPage() {
* This should be used for messages that don't require a callback.
* @memberOf PanelUtils
*/
export function openCheckoutPage() {
sendMessage('account.openCheckoutPage');
export function openCheckoutPage(from) {
sendMessage('account.openCheckoutPage', {
from

This comment has been minimized.

Sign in to view
@christophertino

christophertino Feb 21, 2020
Member

Change this property to utms

This comment has been minimized.

Sign in to view
@benstrumeyer

benstrumeyer Feb 21, 2020
Author Contributor

Good name!
});
window.close();
}

9 src/background.js
View file
Open in desktop
@@ -849,7 +849,14 @@ function onMessageHandler(request, sender, callback) {
return false;
}
if (name === 'account.openCheckoutPage') {
utils.openNewTab({ url: `https://checkout.${globals.GHOSTERY_DOMAIN}.com/plus`, become_active: true });
let url = `https://checkout.${globals.GHOSTERY_DOMAIN}.com/plus`;
const { from } = message;

This comment has been minimized.

Sign in to view
@christophertino

christophertino Feb 21, 2020
Member

Rather than checking for the source, let's just check to see if any utms are passed along with the message and append to the url string

This comment has been minimized.

Sign in to view
@benstrumeyer

benstrumeyer Feb 21, 2020
Author Contributor

I like that this removes the if statement on a magic string
if (from === 'historical_stats') {
url += '?utm_source=gbe&utm_campaign=in_app_hstats';
} else if (from === 'plus_panel') {
url += '?utm_source=gbe&utm_campaign=in_app_plus';
}
utils.openNewTab({ url, become_active: true });
This conversation was marked as resolved by Eden12345
Comment on lines +852 to +859

This comment has been minimized.

Sign in to view
@Eden12345

Eden12345 Feb 19, 2020 •
edited
Contributor

In this function, you aren't appending a / to the end of the URL before the UTM parameters, where as in the rest of the URLs in this PR you are. Both work fine, but we should do it the same way across the entire project if possible.

This comment has been minimized.

Sign in to view
@benstrumeyer

benstrumeyer Feb 19, 2020
Author Contributor

Conventions matter! I've removed the / before the ? from all of the links with utm parameters throughout the project..
return false;
}
if (name === 'account.openSupportPage') {
ProTip! Use n and p to navigate between commits in a pull request.