Refactoring report table withSelect to fix issues with the table data populating correctly (https://github.com/woocommerce/woocommerce-admin/pull/7355)

This commit is contained in:
Joel Thiessen 2021-07-26 12:11:44 -07:00 committed by GitHub
parent 069537100e
commit 31804aeba3
9 changed files with 78 additions and 46 deletions

View File

@ -0,0 +1,4 @@
Significance: patch
Type: Fix
Fixing issues with ReportTable component data not populating correctly #7355

View File

@ -13,6 +13,7 @@ import {
getReportChartData, getReportChartData,
getTooltipValueFormat, getTooltipValueFormat,
SETTINGS_STORE_NAME, SETTINGS_STORE_NAME,
REPORTS_STORE_NAME,
} from '@woocommerce/data'; } from '@woocommerce/data';
import { import {
getAllowedIntervalsForQuery, getAllowedIntervalsForQuery,
@ -360,6 +361,9 @@ export default compose(
SETTINGS_STORE_NAME SETTINGS_STORE_NAME
).getSetting( 'wc_admin', 'wcAdminSettings' ); ).getSetting( 'wc_admin', 'wcAdminSettings' );
/* eslint @wordpress/no-unused-vars-before-return: "off" */
const reportStoreSelector = select( REPORTS_STORE_NAME );
const newProps = { const newProps = {
mode: chartMode, mode: chartMode,
filterParam, filterParam,
@ -387,7 +391,7 @@ export default compose(
endpoint, endpoint,
dataType: 'primary', dataType: 'primary',
query, query,
select, selector: reportStoreSelector,
limitBy, limitBy,
filters, filters,
advancedFilters, advancedFilters,
@ -406,7 +410,7 @@ export default compose(
endpoint, endpoint,
dataType: 'secondary', dataType: 'secondary',
query, query,
select, selector: reportStoreSelector,
limitBy, limitBy,
filters, filters,
advancedFilters, advancedFilters,

View File

@ -27,6 +27,7 @@ import {
getReportTableData, getReportTableData,
EXPORT_STORE_NAME, EXPORT_STORE_NAME,
SETTINGS_STORE_NAME, SETTINGS_STORE_NAME,
REPORTS_STORE_NAME,
useUserPreferences, useUserPreferences,
QUERY_DEFAULTS, QUERY_DEFAULTS,
} from '@woocommerce/data'; } from '@woocommerce/data';
@ -583,27 +584,32 @@ export default compose(
filters, filters,
advancedFilters, advancedFilters,
summaryFields, summaryFields,
extendedItemsStoreName,
} = props; } = props;
if ( /* eslint @wordpress/no-unused-vars-before-return: "off" */
isRequesting || const reportStoreSelector = select( REPORTS_STORE_NAME );
( query.search &&
! ( query[ endpoint ] && query[ endpoint ].length ) ) const extendedStoreSelector = extendedItemsStoreName
) { ? select( extendedItemsStoreName )
return EMPTY_OBJECT; : null;
}
const { woocommerce_default_date_range: defaultDateRange } = select( const { woocommerce_default_date_range: defaultDateRange } = select(
SETTINGS_STORE_NAME SETTINGS_STORE_NAME
).getSetting( 'wc_admin', 'wcAdminSettings' ); ).getSetting( 'wc_admin', 'wcAdminSettings' );
if ( isRequesting ) {
return EMPTY_OBJECT;
}
// Category charts are powered by the /reports/products/stats endpoint. // Category charts are powered by the /reports/products/stats endpoint.
const chartEndpoint = endpoint === 'categories' ? 'products' : endpoint; const chartEndpoint = endpoint === 'categories' ? 'products' : endpoint;
const primaryData = getSummary const primaryData = getSummary
? getReportChartData( { ? getReportChartData( {
endpoint: chartEndpoint, endpoint: chartEndpoint,
selector: reportStoreSelector,
dataType: 'primary', dataType: 'primary',
query, query,
select,
filters, filters,
advancedFilters, advancedFilters,
defaultDateRange, defaultDateRange,
@ -615,17 +621,16 @@ export default compose(
getReportTableData( { getReportTableData( {
endpoint, endpoint,
query, query,
select, selector: reportStoreSelector,
tableQuery, tableQuery,
filters, filters,
advancedFilters, advancedFilters,
defaultDateRange, defaultDateRange,
} ); } );
const extendedTableData = extendTableData(
select, const extendedTableData = extendedStoreSelector
props, ? extendTableData( extendedStoreSelector, props, queriedTableData )
queriedTableData : queriedTableData;
);
return { return {
primaryData, primaryData,

View File

@ -3,12 +3,12 @@
*/ */
import { first } from 'lodash'; import { first } from 'lodash';
export function extendTableData( select, props, queriedTableData ) { export function extendTableData(
const { extendedStoreSelector,
extendItemsMethodNames, props,
extendedItemsStoreName, queriedTableData
itemIdField, ) {
} = props; const { extendItemsMethodNames, itemIdField } = props;
const itemsData = queriedTableData.items.data; const itemsData = queriedTableData.items.data;
if ( if (
! Array.isArray( itemsData ) || ! Array.isArray( itemsData ) ||
@ -23,7 +23,7 @@ export function extendTableData( select, props, queriedTableData ) {
[ extendItemsMethodNames.getError ]: getErrorMethod, [ extendItemsMethodNames.getError ]: getErrorMethod,
[ extendItemsMethodNames.isRequesting ]: isRequestingMethod, [ extendItemsMethodNames.isRequesting ]: isRequestingMethod,
[ extendItemsMethodNames.load ]: loadMethod, [ extendItemsMethodNames.load ]: loadMethod,
} = select( extendedItemsStoreName ); } = extendedStoreSelector;
const extendQuery = { const extendQuery = {
include: itemsData.map( ( item ) => item[ itemIdField ] ).join( ',' ), include: itemsData.map( ( item ) => item[ itemIdField ] ).join( ',' ),
per_page: itemsData.length, per_page: itemsData.length,

View File

@ -7,7 +7,7 @@ import { withSelect } from '@wordpress/data';
import PropTypes from 'prop-types'; import PropTypes from 'prop-types';
import { find } from 'lodash'; import { find } from 'lodash';
import { getQuery, getSearchWords } from '@woocommerce/navigation'; import { getQuery, getSearchWords } from '@woocommerce/navigation';
import { searchItemsByString } from '@woocommerce/data'; import { searchItemsByString, ITEMS_STORE_NAME } from '@woocommerce/data';
/** /**
* Internal dependencies * Internal dependencies
@ -89,6 +89,9 @@ export default compose(
const query = getQuery(); const query = getQuery();
const { search } = query; const { search } = query;
/* eslint @wordpress/no-unused-vars-before-return: "off" */
const itemsSelector = select( ITEMS_STORE_NAME );
if ( ! search ) { if ( ! search ) {
return {}; return {};
} }
@ -101,7 +104,7 @@ export default compose(
? 'products' ? 'products'
: report; : report;
const itemsResult = searchItemsByString( const itemsResult = searchItemsByString(
select, itemsSelector,
mappedReport, mappedReport,
searchWords searchWords
); );

View File

@ -142,6 +142,11 @@ export default compose(
! query.search && ! query.search &&
query.products && query.products &&
query.products.split( ',' ).length === 1; query.products.split( ',' ).length === 1;
const { getItems, isResolving, getItemsError } = select(
ITEMS_STORE_NAME
);
if ( isRequesting ) { if ( isRequesting ) {
return { return {
query: { query: {
@ -152,9 +157,6 @@ export default compose(
}; };
} }
const { getItems, isResolving, getItemsError } = select(
ITEMS_STORE_NAME
);
if ( isSingleProductView ) { if ( isSingleProductView ) {
const productId = parseInt( query.products, 10 ); const productId = parseInt( query.products, 10 );
const includeArgs = { include: productId }; const includeArgs = { include: productId };

View File

@ -145,6 +145,7 @@ class ProductsReportTable extends Component {
const productCategories = const productCategories =
( categoryIds && ( categoryIds &&
categories &&
categoryIds categoryIds
.map( ( categoryId ) => categories.get( categoryId ) ) .map( ( categoryId ) => categories.get( categoryId ) )
.filter( Boolean ) ) || .filter( Boolean ) ) ||
@ -375,6 +376,11 @@ ProductsReportTable.contextType = CurrencyContext;
export default compose( export default compose(
withSelect( ( select, props ) => { withSelect( ( select, props ) => {
const { query, isRequesting } = props; const { query, isRequesting } = props;
const { getItems, getItemsError, isResolving } = select(
ITEMS_STORE_NAME
);
if ( if (
isRequesting || isRequesting ||
( query.search && ! ( query.products && query.products.length ) ) ( query.search && ! ( query.products && query.products.length ) )
@ -382,9 +388,6 @@ export default compose(
return {}; return {};
} }
const { getItems, getItemsError, isResolving } = select(
ITEMS_STORE_NAME
);
const tableQuery = { const tableQuery = {
per_page: -1, per_page: -1,
}; };

View File

@ -63,13 +63,13 @@ export function getLeaderboard( options ) {
/** /**
* Returns items based on a search query. * Returns items based on a search query.
* *
* @param {Object} select Instance of @wordpress/select * @param {Object} selector Instance of @wordpress/select response
* @param {string} endpoint Report API Endpoint * @param {string} endpoint Report API Endpoint
* @param {string[]} search Array of search strings. * @param {string[]} search Array of search strings.
* @return {Object} Object containing API request information and the matching items. * @return {Object} Object containing API request information and the matching items.
*/ */
export function searchItemsByString( select, endpoint, search ) { export function searchItemsByString( selector, endpoint, search ) {
const { getItems, getItemsError, isResolving } = select( STORE_NAME ); const { getItems, getItemsError, isResolving } = selector;
const items = {}; const items = {};
let isRequesting = false; let isRequesting = false;

View File

@ -342,16 +342,18 @@ const getReportChartDataResponse = memoize(
* @param {string} options.endpoint Report API Endpoint * @param {string} options.endpoint Report API Endpoint
* @param {string} options.dataType 'primary' or 'secondary' * @param {string} options.dataType 'primary' or 'secondary'
* @param {Object} options.query Query parameters in the url * @param {Object} options.query Query parameters in the url
* @param {Object} options.select Instance of @wordpress/select * @param {Object} options.selector Instance of @wordpress/select response
* @param {Array} options.limitBy Properties used to limit the results. It will be used in the API call to send the IDs. * @param {Array} options.limitBy Properties used to limit the results. It will be used in the API call to send the IDs.
* @param {string} options.defaultDateRange User specified default date range. * @param {string} options.defaultDateRange User specified default date range.
* @return {Object} Object containing API request information (response, fetching, and error details) * @return {Object} Object containing API request information (response, fetching, and error details)
*/ */
export function getReportChartData( options ) { export function getReportChartData( options ) {
const { endpoint, select } = options; const { endpoint } = options;
const { getReportStats, getReportStatsError, isResolving } = select( const {
STORE_NAME getReportStats,
); getReportStatsError,
isResolving,
} = options.selector;
const requestQuery = getRequestQuery( options ); const requestQuery = getRequestQuery( options );
// Disable eslint rule requiring `stats` to be defined below because the next two if statements // Disable eslint rule requiring `stats` to be defined below because the next two if statements
@ -489,16 +491,18 @@ export function getReportTableQuery( options ) {
* @param {Object} options arguments * @param {Object} options arguments
* @param {string} options.endpoint Report API Endpoint * @param {string} options.endpoint Report API Endpoint
* @param {Object} options.query Query parameters in the url * @param {Object} options.query Query parameters in the url
* @param {Object} options.select Instance of @wordpress/select * @param {Object} options.selector Instance of @wordpress/select response
* @param {Object} options.tableQuery Query parameters specific for that endpoint * @param {Object} options.tableQuery Query parameters specific for that endpoint
* @param {string} options.defaultDateRange User specified default date range. * @param {string} options.defaultDateRange User specified default date range.
* @return {Object} Object Table data response * @return {Object} Object Table data response
*/ */
export function getReportTableData( options ) { export function getReportTableData( options ) {
const { endpoint, select } = options; const { endpoint } = options;
const { getReportItems, getReportItemsError, isResolving } = select( const {
STORE_NAME getReportItems,
); getReportItemsError,
hasFinishedResolution,
} = options.selector;
const tableQuery = reportsUtils.getReportTableQuery( options ); const tableQuery = reportsUtils.getReportTableQuery( options );
const response = { const response = {
@ -516,9 +520,16 @@ export function getReportTableData( options ) {
// eslint-disable-next-line @wordpress/no-unused-vars-before-return // eslint-disable-next-line @wordpress/no-unused-vars-before-return
const items = getReportItems( endpoint, tableQuery ); const items = getReportItems( endpoint, tableQuery );
if ( isResolving( 'getReportItems', [ endpoint, tableQuery ] ) ) { const queryResolved = hasFinishedResolution( 'getReportItems', [
endpoint,
tableQuery,
] );
if ( ! queryResolved ) {
return { ...response, isRequesting: true }; return { ...response, isRequesting: true };
} else if ( getReportItemsError( endpoint, tableQuery ) ) { }
if ( getReportItemsError( endpoint, tableQuery ) ) {
return { ...response, isError: true }; return { ...response, isError: true };
} }