Merge branch 'master' into add/410-order-summary-numbers

This commit is contained in:
Jonathan Belcher 2018-10-11 14:45:35 -04:00
commit e3224a5abb
10 changed files with 340 additions and 103 deletions

View File

@ -1,24 +1,19 @@
/** @format */
{ {
"root": true, "root": true,
"parser": "babel-eslint", "parser": "babel-eslint",
"extends": [ "extends": [ "wpcalypso/react", "plugin:jsx-a11y/recommended" ],
"wpcalypso/react", "plugins": [ "jsx-a11y", "jest" ],
"plugin:jsx-a11y/recommended"
],
"plugins": [
"jsx-a11y",
"jest"
],
"env": { "env": {
"browser": true, "browser": true,
"jest/globals": true, "jest/globals": true,
"node": true "node": true,
}, },
"globals": { "globals": {
"jQuery": true,
"wp": true, "wp": true,
"wpApiSettings": true, "wpApiSettings": true,
"wcSettings": true "wcSettings": true,
}, },
"rules": { "rules": {
"camelcase": 0, "camelcase": 0,
@ -29,6 +24,6 @@
"react/react-in-jsx-scope": 0, "react/react-in-jsx-scope": 0,
"wpcalypso/import-no-redux-combine-reducers": 0, "wpcalypso/import-no-redux-combine-reducers": 0,
"wpcalypso/jsx-classname-namespace": 0, "wpcalypso/jsx-classname-namespace": 0,
"wpcalypso/redux-no-bound-selectors": 1 "wpcalypso/redux-no-bound-selectors": 1,
} },
} }

View File

@ -0,0 +1,41 @@
/\*_ @format _/
---
name: Bug report
about: Create a report to help us improve
---
**Describe the bug**
A clear and concise description of what the bug is.
**To Reproduce**
Steps to reproduce the behavior:
1. Go to '...'
2. Click on '....'
3. Scroll down to '....'
4. See error
**Expected behavior**
A clear and concise description of what you expected to happen.
**Screenshots**
If applicable, add screenshots to help explain your problem.
**Desktop (please complete the following information):**
* OS: [e.g. iOS]
* Browser [e.g. chrome, safari]
* Version [e.g. 22]
**Smartphone (please complete the following information):**
* Device: [e.g. iPhone6]
* OS: [e.g. iOS8.1]
* Browser [e.g. stock browser, safari]
* Version [e.g. 22]
**Additional context**
Add any other context about the problem here.

View File

@ -0,0 +1,22 @@
/\*_ @format _/
---
name: Feature request
about: Suggest an idea for this project
---
**Is your feature request related to a problem? Please describe.**
A clear and concise description of what the problem is. Ex. I'm always frustrated when [...]
**Describe the solution you'd like**
A clear and concise description of what you want to happen.
**Describe alternatives you've considered**
A clear and concise description of any alternative solutions or features you've considered.
**Should this be prioritized? Why?**
**Additional context**
Add any other context or screenshots about the feature request here.

View File

@ -0,0 +1,12 @@
/\*_ @format _/
Fixes #
_Replace this with a good description of your changes & reasoning._
### Screenshots
### Detailed test instructions:
- Ex: Open page `url`
- Click XYZ…

View File

@ -6,7 +6,7 @@ import { __ } from '@wordpress/i18n';
import { Component, Fragment } from '@wordpress/element'; import { Component, Fragment } from '@wordpress/element';
import { compose } from '@wordpress/compose'; import { compose } from '@wordpress/compose';
import { format as formatDate } from '@wordpress/date'; import { format as formatDate } from '@wordpress/date';
import { map, find, isEqual } from 'lodash'; import { map, find } from 'lodash';
import PropTypes from 'prop-types'; import PropTypes from 'prop-types';
import { withSelect } from '@wordpress/data'; import { withSelect } from '@wordpress/data';
@ -28,53 +28,23 @@ import {
import { downloadCSVFile, generateCSVDataFromTable, generateCSVFileName } from 'lib/csv'; import { downloadCSVFile, generateCSVDataFromTable, generateCSVFileName } from 'lib/csv';
import { formatCurrency, getCurrencyFormatDecimal } from 'lib/currency'; import { formatCurrency, getCurrencyFormatDecimal } from 'lib/currency';
import { getAdminLink, getNewPath, onQueryChange } from 'lib/nav-utils'; import { getAdminLink, getNewPath, onQueryChange } from 'lib/nav-utils';
import { getReportChartData } from 'store/reports/utils'; import { getReportChartData, getSummaryNumbers } from 'store/reports/utils';
import { import {
getCurrentDates,
getPreviousDate,
getIntervalForQuery,
getAllowedIntervalsForQuery, getAllowedIntervalsForQuery,
getCurrentDates,
getDateFormatsForInterval, getDateFormatsForInterval,
getDateParamsFromQuery,
getIntervalForQuery,
getPreviousDate,
} from 'lib/date'; } from 'lib/date';
import { MAX_PER_PAGE } from 'store/constants'; import { MAX_PER_PAGE } from 'store/constants';
export class RevenueReport extends Component { export class RevenueReport extends Component {
constructor() { constructor() {
super(); super();
this.state = {
primaryTotals: null,
secondaryTotals: null,
};
this.onDownload = this.onDownload.bind( this ); this.onDownload = this.onDownload.bind( this );
} }
// Track primary and secondary 'totals' indepdent of query.
// We don't want each little query update (interval, sorting, etc)
componentDidUpdate( prevProps ) {
/* eslint-disable react/no-did-update-set-state */
if ( ! isEqual( prevProps.dates, this.props.dates ) ) {
this.setState( {
primaryTotals: null,
secondaryTotals: null,
} );
}
const { secondaryData, primaryData } = this.props;
if ( ! isEqual( prevProps.secondaryData, secondaryData ) ) {
if ( secondaryData && secondaryData.data && secondaryData.data.totals ) {
this.setState( { secondaryTotals: secondaryData.data.totals } );
}
}
if ( ! isEqual( prevProps.primaryData, primaryData ) ) {
if ( primaryData && primaryData.data && primaryData.data.totals ) {
this.setState( { primaryTotals: primaryData.data.totals } );
}
}
/* eslint-enable react/no-did-update-set-state */
}
onDownload( headers, rows, query ) { onDownload( headers, rows, query ) {
// @TODO The current implementation only downloads the contents displayed in the table. // @TODO The current implementation only downloads the contents displayed in the table.
// Another solution is required when the data set is larger (see #311). // Another solution is required when the data set is larger (see #311).
@ -263,12 +233,13 @@ export class RevenueReport extends Component {
renderChartSummaryNumbers() { renderChartSummaryNumbers() {
const selectedChart = this.getSelectedChart(); const selectedChart = this.getSelectedChart();
const charts = this.getCharts(); const charts = this.getCharts();
if ( ! this.state.primaryTotals || ! this.state.secondaryTotals ) { if ( this.props.summaryNumbers.isRequesting ) {
return <SummaryListPlaceholder numberOfItems={ charts.length } />; return <SummaryListPlaceholder numberOfItems={ charts.length } />;
} }
const totals = this.state.primaryTotals || {}; const totals = this.props.summaryNumbers.totals.primary || {};
const secondaryTotals = this.state.secondaryTotals || {}; const secondaryTotals = this.props.summaryNumbers.totals.secondary || {};
const { compare } = getDateParamsFromQuery( this.props.query );
const summaryNumbers = map( this.getCharts(), chart => { const summaryNumbers = map( this.getCharts(), chart => {
const { key, label, type } = chart; const { key, label, type } = chart;
@ -300,6 +271,11 @@ export class RevenueReport extends Component {
label={ label } label={ label }
selected={ isSelected } selected={ isSelected }
prevValue={ secondaryValue } prevValue={ secondaryValue }
prevLabel={
'previous_period' === compare
? __( 'Previous Period:', 'wc-admin' )
: __( 'Previous Year:', 'wc-admin' )
}
delta={ delta } delta={ delta }
href={ href } href={ href }
/> />
@ -413,9 +389,21 @@ export class RevenueReport extends Component {
} }
render() { render() {
const { path, query, primaryData, secondaryData, isTableDataError } = this.props; const {
path,
query,
primaryData,
secondaryData,
summaryNumbers,
isTableDataError,
} = this.props;
if ( primaryData.isError || secondaryData.isError || isTableDataError ) { if (
primaryData.isError ||
secondaryData.isError ||
isTableDataError ||
summaryNumbers.isError
) {
let title, actionLabel, actionURL, actionCallback; let title, actionLabel, actionURL, actionCallback;
if ( primaryData.isError || secondaryData.isError ) { if ( primaryData.isError || secondaryData.isError ) {
title = __( 'There was an error getting your stats. Please try again.', 'wc-admin' ); title = __( 'There was an error getting your stats. Please try again.', 'wc-admin' );
@ -472,6 +460,15 @@ export default compose(
per_page: MAX_PER_PAGE, per_page: MAX_PER_PAGE,
}; };
const summaryNumbers = getSummaryNumbers(
'revenue',
{
primary: datesFromQuery.primary,
secondary: datesFromQuery.secondary,
},
select
);
const primaryData = getReportChartData( const primaryData = getReportChartData(
'revenue', 'revenue',
{ {
@ -506,22 +503,8 @@ export default compose(
const isTableDataError = isReportStatsError( 'revenue', tableQuery ); const isTableDataError = isReportStatsError( 'revenue', tableQuery );
const isTableDataRequesting = isReportStatsRequesting( 'revenue', tableQuery ); const isTableDataRequesting = isReportStatsRequesting( 'revenue', tableQuery );
const primaryDates = {
after: datesFromQuery.primary.after,
before: datesFromQuery.primary.before,
};
const secondaryDates = {
after: datesFromQuery.secondary.after,
before: datesFromQuery.secondary.before,
};
const dates = {
primaryDates,
secondaryDates,
};
return { return {
dates, summaryNumbers,
primaryData, primaryData,
secondaryData, secondaryData,
tableQuery, tableQuery,

View File

@ -33,11 +33,19 @@ describe( 'RevenueReport', () => {
totalPages: 1, totalPages: 1,
}; };
const summaryNumbers = {
totals: {
primary: {},
secondary: {},
},
};
const revenueReport = shallow( const revenueReport = shallow(
<RevenueReport <RevenueReport
params={ { report: 'revenue' } } params={ { report: 'revenue' } }
path="/analytics/revenue" path="/analytics/revenue"
query={ {} } query={ {} }
summaryNumbers={ summaryNumbers }
primaryData={ primaryData } primaryData={ primaryData }
tableData={ primaryData } tableData={ primaryData }
secondaryData={ primaryData } secondaryData={ primaryData }

View File

@ -61,20 +61,28 @@ class Controller extends Component {
// When the route changes, we need to update wp-admin's menu with the correct section & current link // When the route changes, we need to update wp-admin's menu with the correct section & current link
window.wpNavMenuClassChange = function( menuClass, pathname ) { window.wpNavMenuClassChange = function( menuClass, pathname ) {
const path = '/' === pathname ? '' : '#' + pathname; const path = '/' === pathname ? '' : '#' + pathname;
jQuery( '.current' ).each( function( i, obj ) { Array.from( document.getElementsByClassName( 'current' ) ).forEach( function( item ) {
jQuery( obj ).removeClass( 'current' ); item.classList.remove( 'current' );
} ); } );
jQuery( '.wp-has-current-submenu' )
.removeClass( 'wp-has-current-submenu' ) const submenu = document.querySelector( '.wp-has-current-submenu' );
.removeClass( 'wp-menu-open' ) submenu.classList.remove( 'wp-has-current-submenu' );
.removeClass( 'selected' ) submenu.classList.remove( 'wp-menu-open' );
.addClass( 'wp-not-current-submenu menu-top' ); submenu.classList.remove( 'selected' );
jQuery( 'li > a[href$="admin.php?page=wc-admin' + path + '"]' ) submenu.classList.add( 'wp-not-current-submenu' );
.parent() submenu.classList.add( 'menu-top' );
.addClass( 'current' );
jQuery( '#' + menuClass ) Array.from(
.removeClass( 'wp-not-current-submenu' ) document.querySelectorAll( `li > a[href$="admin.php?page=wc-admin${ path }"]` )
.addClass( 'wp-has-current-submenu wp-menu-open current' ); ).forEach( function( item ) {
item.parentElement.classList.add( 'current' );
} );
const currentMenu = document.querySelector( '#' + menuClass );
currentMenu.classList.remove( 'wp-not-current-submenu' );
currentMenu.classList.add( 'wp-has-current-submenu' );
currentMenu.classList.add( 'wp-menu-open' );
currentMenu.classList.add( 'current' );
}; };
export { Controller, getPages }; export { Controller, getPages };

View File

@ -5,7 +5,7 @@
/** /**
* Internal dependencies * Internal dependencies
*/ */
import { isReportDataEmpty, getReportChartData } from '../utils'; import { isReportDataEmpty, getReportChartData, getSummaryNumbers } from '../utils';
describe( 'isReportDataEmpty()', () => { describe( 'isReportDataEmpty()', () => {
it( 'returns false if report is valid', () => { it( 'returns false if report is valid', () => {
@ -227,3 +227,116 @@ describe( 'getReportChartData()', () => {
expect( result ).toEqual( { ...response, isEmpty: true } ); expect( result ).toEqual( { ...response, isEmpty: true } );
} ); } );
} ); } );
describe( 'getSummaryNumbers()', () => {
const select = jest.fn().mockReturnValue( {} );
const response = {
isError: false,
isRequesting: false,
totals: {
primary: null,
secondary: null,
},
};
const dates = {
primary: {
after: '2018-10-10',
before: '2018-10-10',
},
secondary: {
after: '2018-10-09',
before: '2018-10-09',
},
};
beforeAll( () => {
select( 'wc-admin' ).getReportStats = jest.fn().mockReturnValue( {} );
select( 'wc-admin' ).isReportStatsRequesting = jest.fn().mockReturnValue( false );
select( 'wc-admin' ).isReportStatsError = jest.fn().mockReturnValue( false );
} );
afterAll( () => {
select( 'wc-admin' ).getReportStats.mockRestore();
select( 'wc-admin' ).isReportStatsRequesting.mockRestore();
select( 'wc-admin' ).isReportStatsError.mockRestore();
} );
function setGetReportStats( func ) {
select( 'wc-admin' ).getReportStats.mockImplementation( ( ...args ) => func( ...args ) );
}
function setIsReportStatsRequesting( func ) {
select( 'wc-admin' ).isReportStatsRequesting.mockImplementation( ( ...args ) =>
func( ...args )
);
}
function setIsReportStatsError( func ) {
select( 'wc-admin' ).isReportStatsError.mockImplementation( ( ...args ) => func( ...args ) );
}
it( 'returns isRequesting if a request is in progress', () => {
setIsReportStatsRequesting( () => {
return true;
} );
const result = getSummaryNumbers( 'revenue', dates, select );
expect( result ).toEqual( { ...response, isRequesting: true } );
} );
it( 'returns isError if request errors', () => {
setIsReportStatsRequesting( () => {
return false;
} );
setIsReportStatsError( () => {
return true;
} );
const result = getSummaryNumbers( 'revenue', dates, select );
expect( result ).toEqual( { ...response, isError: true } );
} );
it( 'returns results after queries finish', () => {
const totals = {
primary: {
orders_count: 115,
gross_revenue: 13966.92,
},
secondary: {
orders_count: 85,
gross_revenue: 10406.1,
},
};
setIsReportStatsRequesting( () => {
return false;
} );
setIsReportStatsError( () => {
return false;
} );
setGetReportStats( () => {
return {
totals,
};
} );
setGetReportStats( ( endpoint, query ) => {
if ( '2018-10-10T00:00:00+00:00' === query.after ) {
return {
data: {
totals: totals.primary,
intervals: [],
},
};
}
return {
data: {
totals: totals.secondary,
intervals: [],
},
};
} );
const result = getSummaryNumbers( 'revenue', dates, select );
expect( result ).toEqual( { ...response, totals } );
} );
} );

View File

@ -32,6 +32,62 @@ export function isReportDataEmpty( report ) {
return false; return false;
} }
/**
* Returns summary number totals needed to render a report page.
*
* @param {String} endpoint Report API Endpoint
* @param {Object} dates Primary and secondary dates.
* @param {object} select Instance of @wordpress/select
* @return {Object} Object containing summary number responses.
*/
export function getSummaryNumbers( endpoint, dates, select ) {
const { getReportStats, isReportStatsRequesting, isReportStatsError } = select( 'wc-admin' );
const response = {
isRequesting: false,
isError: false,
totals: {
primary: null,
secondary: null,
},
};
const baseQuery = {
interval: 'day',
per_page: 1, // We only need the `totals` part of the response.
};
const primaryQuery = {
...baseQuery,
after: dates.primary.after + 'T00:00:00+00:00',
before: dates.primary.before + 'T23:59:59+00:00',
};
const primary = getReportStats( endpoint, primaryQuery );
if ( isReportStatsRequesting( endpoint, primaryQuery ) ) {
return { ...response, isRequesting: true };
} else if ( isReportStatsError( endpoint, primaryQuery ) ) {
return { ...response, isError: true };
}
const primaryTotals = ( primary && primary.data && primary.data.totals ) || null;
const secondaryQuery = {
...baseQuery,
per_page: 1,
after: dates.secondary.after + 'T00:00:00+00:00',
before: dates.secondary.before + 'T23:59:59+00:00',
};
const secondary = getReportStats( endpoint, secondaryQuery );
if ( isReportStatsRequesting( endpoint, secondaryQuery ) ) {
return { ...response, isRequesting: true };
} else if ( isReportStatsError( endpoint, secondaryQuery ) ) {
return { ...response, isError: true };
}
const secondaryTotals = ( secondary && secondary.data && secondary.data.totals ) || null;
return { ...response, totals: { primary: primaryTotals, secondary: secondaryTotals } };
}
/** /**
* Returns all of the data needed to render a chart with summary numbers on a report page. * Returns all of the data needed to render a chart with summary numbers on a report page.
* *

View File

@ -16,7 +16,6 @@ const externals = {
'@wordpress/html-entities': { this: [ 'wp', 'htmlEntities' ] }, '@wordpress/html-entities': { this: [ 'wp', 'htmlEntities' ] },
'@wordpress/i18n': { this: [ 'wp', 'i18n' ] }, '@wordpress/i18n': { this: [ 'wp', 'i18n' ] },
'@wordpress/keycodes': { this: [ 'wp', 'keycodes' ] }, '@wordpress/keycodes': { this: [ 'wp', 'keycodes' ] },
jquery: 'jQuery',
tinymce: 'tinymce', tinymce: 'tinymce',
moment: 'moment', moment: 'moment',
react: 'React', react: 'React',