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

Track stats for automatic block conversion #2205

Merged
merged 14 commits into from
Aug 4, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions blocks/api/parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,11 @@
import { parse as hpqParse } from 'hpq';
import { pickBy } from 'lodash';

/**
* WordPress dependencies
*/
import { bumpStat } from '@wordpress/utils';

/**
* Internal dependencies
*/
Expand Down Expand Up @@ -75,6 +80,7 @@ export function createBlockWithFallback( name, rawContent, attributes ) {
// Convert 'core/text' blocks in existing content to the new
// 'core/paragraph'.
if ( name === 'core/text' ) {
bumpStat( 'block_auto_convert', 'core-text-to-paragraph' );
name = 'core/paragraph';
}

Expand Down
2 changes: 1 addition & 1 deletion components/button/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class Button extends Component {
isToggled,
className,
disabled,
...additionalProps, // eslint-disable-line comma-dangle
...additionalProps
} = this.props;
const classes = classnames( 'components-button', className, {
button: ( isPrimary || isSecondary || isLarge ),
Expand Down
2 changes: 1 addition & 1 deletion editor/enable-tracking-prompt/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@ import clickOutside from 'react-click-outside';
import { Component } from '@wordpress/element';
import { __ } from '@wordpress/i18n';
import { Button, Popover } from '@wordpress/components';
import { bumpStat } from '@wordpress/utils';

/**
* Internal dependencies
*/
import './style.scss';
import { bumpStat } from '../utils/tracking';
import { removeNotice } from '../actions';

export const TRACKING_PROMPT_NOTICE_ID = 'notice:enable-tracking-prompt';
Expand Down
70 changes: 55 additions & 15 deletions editor/enable-tracking-prompt/test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
* External dependencies
*/
import { mount } from 'enzyme';
import clickOutside from 'react-click-outside';

/**
* Internal dependencies
Expand All @@ -12,35 +13,37 @@ import {
} from '../';

describe( 'EnableTrackingPrompt', () => {
const tracking = require( '../../utils/tracking' ); // no default export
const originalSetUserSetting = window.setUserSetting;
const originalBumpStat = tracking.bumpStat;
const originalDocumentAddEventListener = document.addEventListener;
let eventMap = {};
let removeNotice;

beforeEach( () => {
window.setUserSetting = jest.fn();
tracking.bumpStat = jest.fn();
document.addEventListener = jest.fn( ( event, cb ) => {
eventMap[ event ] = cb;
} );
Copy link
Member Author

Choose a reason for hiding this comment

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

This approach is from enzymejs/enzyme#426 (comment).

removeNotice = jest.fn();
} );

afterEach( () => {
window.setUserSetting = originalSetUserSetting;
tracking.bumpStat = originalBumpStat;
document.addEventListener = originalDocumentAddEventListener;
eventMap = {};
} );

it( 'should render a prompt with Yes and No buttons', () => {
it( 'should render a prompt with Yes, No, and More info buttons', () => {
const prompt = mount(
<EnableTrackingPrompt />
);
const buttons = prompt.find( '.button' );
expect( buttons.length ).toBe( 2 );
const buttons = prompt.find( 'Button' );
expect( buttons.length ).toBe( 3 );
expect( buttons.at( 0 ).text() ).toBe( 'Yes' );
expect( buttons.at( 1 ).text() ).toBe( 'No' );
expect( buttons.at( 2 ).text() ).toBe( 'More info' );

expect( window.setUserSetting )
.not.toHaveBeenCalled();
expect( tracking.bumpStat )
.not.toHaveBeenCalled();
expect( removeNotice )
.not.toHaveBeenCalled();
} );
Expand All @@ -49,14 +52,12 @@ describe( 'EnableTrackingPrompt', () => {
const prompt = mount(
<EnableTrackingPrompt removeNotice={ removeNotice } />
);
const buttonYes = prompt.find( '.button' )
const buttonYes = prompt.find( 'Button' )
.filterWhere( node => node.text() === 'Yes' );
buttonYes.simulate( 'click' );

expect( window.setUserSetting )
.toHaveBeenCalledWith( 'gutenberg_tracking', 'on' );
expect( tracking.bumpStat )
.toHaveBeenCalledWith( 'tracking', 'opt-in' );
expect( removeNotice )
.toHaveBeenCalledWith( TRACKING_PROMPT_NOTICE_ID );
} );
Expand All @@ -65,15 +66,54 @@ describe( 'EnableTrackingPrompt', () => {
const prompt = mount(
<EnableTrackingPrompt removeNotice={ removeNotice } />
);
const buttonNo = prompt.find( '.button' )
const buttonNo = prompt.find( 'Button' )
.filterWhere( node => node.text() === 'No' );
buttonNo.simulate( 'click' );

expect( window.setUserSetting )
.toHaveBeenCalledWith( 'gutenberg_tracking', 'off' );
expect( tracking.bumpStat )
.not.toHaveBeenCalled();
expect( removeNotice )
.toHaveBeenCalledWith( TRACKING_PROMPT_NOTICE_ID );
} );

it( 'should show and hide a popover when clicking More info', () => {
const EnableTrackingPromptWrapped = clickOutside( EnableTrackingPrompt );
const prompt = mount(
<EnableTrackingPromptWrapped removeNotice={ removeNotice } />
);

expect( prompt.find( 'Popover' ).length ).toBe( 0 );

const buttonMoreInfo = prompt.find( 'Button' )
.filterWhere( node => node.text() === 'More info' );

// Click the "More info" button to show the info popover
buttonMoreInfo.simulate( 'click' );
expect( prompt.find( 'Popover' ).length ).toBe( 1 );

// Click the "More info" button to hide the info popover
buttonMoreInfo.simulate( 'click' );
expect( prompt.find( 'Popover' ).length ).toBe( 0 );

// Click the "More info" button to show the info popover
buttonMoreInfo.simulate( 'click' );
expect( prompt.find( 'Popover' ).length ).toBe( 1 );

// Click inside the prompt to hide the info popover
prompt.simulate( 'click' );
expect( prompt.find( 'Popover' ).length ).toBe( 0 );

// Click the "More info" button to show the info popover
buttonMoreInfo.simulate( 'click' );
expect( prompt.find( 'Popover' ).length ).toBe( 1 );

// Click outside the prompt to hide the info popover
eventMap.click( { target: document.body } );
expect( prompt.find( 'Popover' ).length ).toBe( 0 );

expect( window.setUserSetting )
.not.toHaveBeenCalled();
expect( removeNotice )
.not.toHaveBeenCalled();
} );
} );
2 changes: 1 addition & 1 deletion editor/inserter/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,14 @@ import { __ } from '@wordpress/i18n';
import { Component } from '@wordpress/element';
import { IconButton } from '@wordpress/components';
import { createBlock } from '@wordpress/blocks';
import { bumpStat } from '@wordpress/utils';

/**
* Internal dependencies
*/
import InserterMenu from './menu';
import { getBlockInsertionPoint, getEditorMode } from '../selectors';
import { insertBlock, hideInsertionPoint } from '../actions';
import { bumpStat } from '../utils/tracking';

class Inserter extends Component {
constructor() {
Expand Down
3 changes: 1 addition & 2 deletions editor/modes/visual-editor/block-list.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { __ } from '@wordpress/i18n';
import { Component } from '@wordpress/element';
import { serialize, getDefaultBlock, createBlock } from '@wordpress/blocks';
import { IconButton } from '@wordpress/components';
import { keycodes } from '@wordpress/utils';
import { keycodes, bumpStat } from '@wordpress/utils';

/**
* Internal dependencies
Expand All @@ -29,7 +29,6 @@ import {
getMultiSelectedBlockUids,
} from '../../selectors';
import { insertBlock, multiSelect } from '../../actions';
import { bumpStat } from '../../utils/tracking';

const INSERTION_POINT_PLACEHOLDER = '[[insertion-point]]';
const { ENTER } = keycodes;
Expand Down
11 changes: 11 additions & 0 deletions test/setup-globals.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,3 +51,14 @@ global.wp = global.wp || {};
global.wp.a11y = {
speak: () => {},
};

global.window.getUserSetting = settingName => {
switch ( settingName ) {
case 'gutenberg_tracking':
return 'on';
default:
throw new Error(
'Unrecognized user setting requested: ' + settingName
);
}
};
2 changes: 2 additions & 0 deletions utils/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,5 @@ import * as nodetypes from './nodetypes';

export { keycodes };
export { nodetypes };

export * from './tracking';
27 changes: 15 additions & 12 deletions editor/utils/test/tracking.js → utils/test/tracking.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
/* eslint-disable no-console */

/**
* External dependencies
*/
import url from 'url';

/**
* Internal dependencies
*/
Expand All @@ -11,7 +16,6 @@ describe( 'bumpStat', () => {

beforeEach( () => {
console.error = jest.fn();
window.getUserSetting = () => 'off';
} );

afterEach( () => {
Expand Down Expand Up @@ -62,22 +66,21 @@ describe( 'bumpStat', () => {
} );

it( 'should do nothing if the user has not opted in', () => {
window.getUserSetting = () => 'off';
expect( bumpStat( 'valid_group', 'valid-name' ) ).toBeUndefined();
expect( console.error ).not.toHaveBeenCalled();
} );

it( 'should bump valid stats', () => {
window.getUserSetting = () => 'on';
const url = bumpStat( 'valid_group', 'valid-name' );
// There are a couple of pieces of the URL where we don't care about
// testing the specific value. Replace them with placeholders.
const urlMatch = url
.replace( /^[a-z]+:/, 'PROTOCOL:' )
.replace( /t=[0-9.]+$/, 't=NUMBER' );
expect( urlMatch ).toBe(
'PROTOCOL://pixel.wp.com/g.gif?v=wpcom-no-pv'
+ '&x_gutenberg_valid_group=valid-name'
+ '&t=NUMBER'
// Testing the URL protocol and the randomized `?t=` cache-buster is
// difficult, so only test the pieces we're actually interested in.
const statUrlPieces = url.parse(
bumpStat( 'valid_group', 'valid-name' ),
true
);
expect( statUrlPieces.hostname ).toBe( 'pixel.wp.com' );
expect( statUrlPieces.pathname ).toBe( '/g.gif' );
expect( statUrlPieces.query.v ).toBe( 'wpcom-no-pv' );
expect( statUrlPieces.query.x_gutenberg_valid_group ).toBe( 'valid-name' );
} );
} );
File renamed without changes.
8 changes: 4 additions & 4 deletions webpack.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,13 @@ const extractConfig = {
};

const entryPointNames = [
'element',
'i18n',
'components',
'utils',
'blocks',
'components',
'date',
'editor',
'element',
'i18n',
'utils',
];

const externals = {
Expand Down