Skip to content

Commit

Permalink
Checklist: Untangle server and client list merging (#25956)
Browse files Browse the repository at this point in the history
Note that both `onboardingChecklist.js` and `jetpack-checklist.js` (yes, we should harmonize filenames, too) had a function duplicated (named `onboardingTasks` and `jetpackTasks()`, respectively). At closer look, that function was essentially merging two objects, and sorting them by key according to an array. I've reshuffled that functionality so that it makes more sense now IMO.
  • Loading branch information
ockham authored Jul 12, 2018
1 parent a782afd commit 0e615b5
Show file tree
Hide file tree
Showing 6 changed files with 184 additions and 107 deletions.
13 changes: 8 additions & 5 deletions client/my-sites/checklist/checklist-show/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,20 @@
import React, { Fragment, PureComponent } from 'react';
import { connect } from 'react-redux';
import { localize } from 'i18n-calypso';
import { find } from 'lodash';
import { find, get } from 'lodash';

/**
* Internal dependencies
*/
import Checklist from 'blocks/checklist';
import { jetpackTasks } from '../jetpack-checklist';
import { tasks as jetpackTasks } from '../jetpack-checklist';
import { requestSiteChecklistTaskUpdate } from 'state/checklist/actions';
import { getSelectedSiteId } from 'state/ui/selectors';
import getSiteChecklist from 'state/selectors/get-site-checklist';
import { isJetpackSite, getSiteSlug } from 'state/sites/selectors';
import QuerySiteChecklist from 'components/data/query-site-checklist';
import { launchTask, onboardingTasks } from '../onboardingChecklist';
import { launchTask, tasks as wpcomTasks } from '../onboardingChecklist';
import { mergeObjectIntoArrayById } from '../util';
import { recordTracksEvent } from 'state/analytics/actions';
import { createNotice } from 'state/notices/actions';
import { requestGuidedTour } from 'state/ui/guided-tours/actions';
Expand Down Expand Up @@ -69,11 +70,13 @@ const mapStateToProps = state => {
const siteSlug = getSiteSlug( state, siteId );
const siteChecklist = getSiteChecklist( state, siteId );
const isJetpack = isJetpackSite( state, siteId );
const tasks = isJetpack ? jetpackTasks( siteChecklist ) : onboardingTasks( siteChecklist );
const tasks = isJetpack ? jetpackTasks : wpcomTasks;
const tasksFromServer = get( siteChecklist, [ 'tasks' ] );

return {
siteId,
siteSlug,
tasks,
tasks: tasksFromServer ? mergeObjectIntoArrayById( tasks, tasksFromServer ) : null,
};
};

Expand Down
50 changes: 17 additions & 33 deletions client/my-sites/checklist/jetpack-checklist.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,80 +4,64 @@
*/
import { translate } from 'i18n-calypso';

const tasks = {
jetpack_brute_force: {
export const tasks = [
{
id: 'jetpack_brute_force',
completedTitle: translate(
"We've automatically protected you from brute force login attacks."
),
completed: true,
},
jetpack_spam_filtering: {
{
id: 'jetpack_spam_filtering',
completedTitle: translate( "We've automatically turned on spam filtering." ),
completed: true,
},
jetpack_backups: {
{
id: 'jetpack_backups',
title: translate( 'Backups & Scanning' ),
description: translate(
"Connect your site's server to Jetpack to perform backups, rewinds, and security scans."
),
completed: true,
completedTitle: translate( 'You turned on backups and scanning.' ),
completedButtonText: 'Change',
completedButtonText: translate( 'Change' ),
duration: translate( '2 min' ),
url: '/stats/activity/$siteSlug',
},
jetpack_monitor: {
{
id: 'jetpack_monitor',
title: translate( 'Jetpack Monitor' ),
description: translate(
"Monitor your site's uptime and alert you the moment downtime is detected with instant notifications."
),
completedTitle: translate( 'You turned on Jetpack Monitor.' ),
completedButtonText: 'Change',
completedButtonText: translate( 'Change' ),
duration: translate( '3 min' ),
tour: 'jetpackMonitoring',
url: '/settings/security/$siteSlug',
},
jetpack_plugin_updates: {
{
id: 'jetpack_plugin_updates',
title: translate( 'Automatic Plugin Updates' ),
description: translate(
'Choose which WordPress plugins you want to keep automatically updated.'
),
completedTitle: translate( 'You turned on automatic plugin updates.' ),
completedButtonText: 'Change',
completedButtonText: translate( 'Change' ),
duration: translate( '3 min' ),
url: '/plugins/manage/$siteSlug',
},
jetpack_sign_in: {
{
id: 'jetpack_sign_in',
title: translate( 'WordPress.com sign in' ),
description: translate(
'Manage your log in preferences and two-factor authentication settings.'
),
completedTitle: translate( 'You completed your sign in preferences.' ),
completedButtonText: 'Change',
completedButtonText: translate( 'Change' ),
duration: translate( '3 min' ),
tour: 'jetpackSignIn',
url: '/settings/security/$siteSlug',
},
};

const sequence = [
'jetpack_brute_force',
'jetpack_spam_filtering',
'jetpack_backups',
'jetpack_monitor',
'jetpack_plugin_updates',
'jetpack_sign_in',
];

export function jetpackTasks( checklist ) {
if ( ! checklist || ! checklist.tasks ) {
return null;
}

return sequence.map( id => {
const task = tasks[ id ];
const taskFromServer = checklist.tasks[ id ];

return { id, ...task, ...taskFromServer };
} );
}
114 changes: 47 additions & 67 deletions client/my-sites/checklist/onboardingChecklist.js
Original file line number Diff line number Diff line change
@@ -1,27 +1,28 @@
/** @format */
/**
* External dependencies
*
* @format
*/
import page from 'page';
import { isDesktop } from 'lib/viewport';

/**
* Internal dependencies
*/
const tasks = {
avatar_uploaded: {
title: 'Upload your profile picture',
description:
'Who’s the person behind the site? Personalize your posts and comments with a custom profile picture.',
duration: '2 mins',
completedTitle: 'You uploaded a profile picture',
completedButtonText: 'Change',
url: '/me',
image: '/calypso/images/stats/tasks/upload-profile-picture.svg',
tour: 'checklistUserAvatar',
export const tasks = [
{
id: 'site_created',
title: 'Create your site',
description: 'This is where your adventure begins.',
completedTitle: 'You created your site',
completed: true,
},
{
id: 'domain_selected',
title: 'Pick a website address',
description: 'Choose an address so people can find you on the internet.',
completedTitle: 'You picked a website address',
completed: true,
image: '/calypso/images/stats/tasks/domains.svg',
},
blogname_set: {
{
id: 'blogname_set',
title: 'Give your site a name',
description: 'Give your site a descriptive name to entice visitors.',
duration: '1 min',
Expand All @@ -31,7 +32,19 @@ const tasks = {
image: '/calypso/images/stats/tasks/personalize-your-site.svg',
tour: 'checklistSiteTitle',
},
blogdescription_set: {
{
id: 'site_icon_set',
title: 'Upload a site icon',
description: 'Help people recognize your site in browser tabs — just like the WordPress.com W!',
duration: '1 min',
completedTitle: 'You uploaded a site icon',
completedButtonText: 'Change',
url: '/settings/general/$siteSlug',
image: '/calypso/images/stats/tasks/upload-icon.svg',
tour: 'checklistSiteIcon',
},
{
id: 'blogdescription_set',
title: 'Create a tagline',
description: 'Pique readers’ interest with a little more detail about your site.',
duration: '2 mins',
Expand All @@ -41,7 +54,20 @@ const tasks = {
image: '/calypso/images/stats/tasks/create-tagline.svg',
tour: 'checklistSiteTagline',
},
contact_page_updated: {
{
id: 'avatar_uploaded',
title: 'Upload your profile picture',
description:
'Who’s the person behind the site? Personalize your posts and comments with a custom profile picture.',
duration: '2 mins',
completedTitle: 'You uploaded a profile picture',
completedButtonText: 'Change',
url: '/me',
image: '/calypso/images/stats/tasks/upload-profile-picture.svg',
tour: 'checklistUserAvatar',
},
{
id: 'contact_page_updated',
title: 'Personalize your Contact page',
description: 'Encourage visitors to get in touch — a website is for connecting with people.',
duration: '2 mins',
Expand All @@ -51,14 +77,8 @@ const tasks = {
url: '/post/$siteSlug/2',
tour: 'checklistContactPage',
},
domain_selected: {
title: 'Pick a website address',
description: 'Choose an address so people can find you on the internet.',
completedTitle: 'You picked a website address',
completed: true,
image: '/calypso/images/stats/tasks/domains.svg',
},
post_published: {
{
id: 'post_published',
title: 'Publish your first blog post',
description: 'Introduce yourself to the world! That’s why you’re here.',
duration: '10 mins',
Expand All @@ -68,33 +88,6 @@ const tasks = {
image: '/calypso/images/stats/tasks/first-post.svg',
tour: 'checklistPublishPost',
},
site_created: {
title: 'Create your site',
description: 'This is where your adventure begins.',
completedTitle: 'You created your site',
completed: true,
},
site_icon_set: {
title: 'Upload a site icon',
description: 'Help people recognize your site in browser tabs — just like the WordPress.com W!',
duration: '1 min',
completedTitle: 'You uploaded a site icon',
completedButtonText: 'Change',
url: '/settings/general/$siteSlug',
image: '/calypso/images/stats/tasks/upload-icon.svg',
tour: 'checklistSiteIcon',
},
};

const sequence = [
'site_created',
'domain_selected',
'blogname_set',
'site_icon_set',
'blogdescription_set',
'avatar_uploaded',
'contact_page_updated',
'post_published',
];

export function launchTask( { task, location, requestTour, siteSlug, track } ) {
Expand Down Expand Up @@ -127,16 +120,3 @@ export function launchTask( { task, location, requestTour, siteSlug, track } ) {
requestTour( tour );
}
}

export function onboardingTasks( checklist ) {
if ( ! checklist || ! checklist.tasks ) {
return null;
}

return sequence.map( id => {
const task = tasks[ id ];
const taskFromServer = checklist.tasks[ id ];

return { id, ...task, ...taskFromServer };
} );
}
80 changes: 80 additions & 0 deletions client/my-sites/checklist/test/util.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
/** @format */

/**
* Internal dependencies
*/
import { mergeObjectIntoArrayById } from '../util';

describe( 'mergeObjectIntoArrayById', () => {
test( 'should produce a new array', () => {
const arr = [ { id: 'a', prop: 'prop' } ];
const obj = { a: { newProp: 'newProp' } };
expect( mergeObjectIntoArrayById( arr, obj ) ).not.toBe( arr );
} );

test( 'should produce a new object when merging', () => {
const arr = [ { id: 'a', prop: 'prop' } ];
const obj = { a: { newProp: 'newProp' } };

const result = mergeObjectIntoArrayById( arr, obj );
expect( result[ 0 ] ).not.toBe( arr[ 0 ] );
expect( result[ 0 ] ).not.toBe( obj );
} );

test( 'should not replace unchanged objects', () => {
const arr = [ { id: 'a', prop: 'prop' }, { id: 'b' } ];
const obj = { a: { newProp: 'newProp' } };

expect( mergeObjectIntoArrayById( arr, obj )[ 1 ] ).toBe( arr[ 1 ] );
} );

test( 'should overwrite existing props', () => {
const arr = [ { id: 'a', prop: 'prop' } ];
const obj = { a: { prop: 'updated' } };

expect( mergeObjectIntoArrayById( arr, obj ) ).toEqual( [
{
id: 'a',
prop: 'updated',
},
] );
} );

test( 'should keep existing props', () => {
const arr = [ { id: 'a', prop: 'prop', untouched: 'stay-the-same' } ];
const obj = { a: { prop: 'updated' } };

expect( mergeObjectIntoArrayById( arr, obj ) ).toEqual( [
{
id: 'a',
prop: 'updated',
untouched: 'stay-the-same',
},
] );
} );

test( 'should add new props', () => {
const arr = [ { id: 'a', prop: 'prop' } ];
const obj = { a: { newProp: 'add me' } };

expect( mergeObjectIntoArrayById( arr, obj ) ).toEqual( [
{
id: 'a',
prop: 'prop',
newProp: 'add me',
},
] );
} );

test( 'should ignore object keys not present in the array', () => {
const arr = [ { id: 'a', prop: 'prop' } ];
const obj = { c: { ignore: 'me' } };

expect( mergeObjectIntoArrayById( arr, obj ) ).toEqual( [
{
id: 'a',
prop: 'prop',
},
] );
} );
} );
Loading

0 comments on commit 0e615b5

Please sign in to comment.