Do not retain values when switching between rules in the DateFilter component (https://github.com/woocommerce/woocommerce-admin/pull/7423)

* Do not retain values when switching between rules in the DateFilter component

* Add changelog entry

* Remove unnecessary use of the ternary operator

* Fix the changelog entry

* Add tests

* Add changelog entry

Co-authored-by: Chris <chris.aprea@automattic.comchris.aprea@automattic.comchris.aprea@automattic.com>
This commit is contained in:
Chris Aprea 2021-08-06 15:39:16 +10:00 committed by GitHub
parent b5e973ed18
commit 71aa954577
7 changed files with 332 additions and 16 deletions

View File

@ -0,0 +1,4 @@
Significance: patch
Type: Fix
No changelog necessary as it is in the component package. #7423

View File

@ -1,6 +1,7 @@
# Unreleased
- Fix a bug in the deprecated callback handlers of Form component. #7356
- Fix a bug in the `<DateFilter>` component where values were retained when switching between rules #7423
- Add `hidden` legend position to `Chart`. #7378
# 8.0.0

View File

@ -74,6 +74,7 @@
"@storybook/addon-console": "1.2.3",
"@storybook/react": "6.2.9",
"@testing-library/react": "11.2.7",
"@testing-library/user-event": "^13.2.1",
"@woocommerce/wc-admin-settings": "file:../wc-admin-settings",
"@wordpress/scripts": "12.6.1",
"concurrently": "5.3.0"

View File

@ -36,10 +36,12 @@ class DateFilter extends Component {
after,
afterText: after ? after.format( dateFormat ) : '',
afterError: null,
rule: filter.rule,
};
this.onSingleDateChange = this.onSingleDateChange.bind( this );
this.onRangeDateChange = this.onRangeDateChange.bind( this );
this.onRuleChange = this.onRuleChange.bind( this );
}
getBetweenString() {
@ -50,8 +52,8 @@ class DateFilter extends Component {
);
}
getScreenReaderText( filter, config ) {
const rule = find( config.rules, { value: filter.rule } ) || {};
getScreenReaderText( filterRule, config ) {
const rule = find( config.rules, { value: filterRule } ) || {};
const { before, after } = this.state;
@ -132,6 +134,34 @@ class DateFilter extends Component {
}
}
onRuleChange( newRule ) {
const { onFilterChange } = this.props;
const { rule } = this.state;
let newDateState = null;
let shouldResetValue = false;
if ( [ rule, newRule ].includes( 'between' ) ) {
newDateState = {
before: null,
beforeText: '',
beforeError: null,
after: null,
afterText: '',
afterError: null,
};
shouldResetValue = true;
}
this.setState( {
rule: newRule,
...newDateState,
} );
onFilterChange( 'rule', newRule, shouldResetValue );
}
isFutureDate( dateString ) {
return moment().isBefore( moment( dateString ), 'day' );
}
@ -179,10 +209,9 @@ class DateFilter extends Component {
}
getFilterInputs() {
const { filter } = this.props;
const { before, beforeText, beforeError } = this.state;
const { before, beforeText, beforeError, rule } = this.state;
if ( filter.rule === 'between' ) {
if ( rule === 'between' ) {
return this.getRangeInput();
}
@ -195,16 +224,10 @@ class DateFilter extends Component {
}
render() {
const {
className,
config,
filter,
isEnglish,
onFilterChange,
} = this.props;
const { rule } = filter;
const { className, config, isEnglish } = this.props;
const { rule } = this.state;
const { labels, rules } = config;
const screenReaderText = this.getScreenReaderText( filter, config );
const screenReaderText = this.getScreenReaderText( rule, config );
const children = interpolateComponents( {
mixedString: labels.title,
components: {
@ -217,7 +240,7 @@ class DateFilter extends Component {
) }
options={ rules }
value={ rule }
onChange={ partial( onFilterChange, 'rule' ) }
onChange={ this.onRuleChange }
aria-label={ labels.rule }
/>
),

View File

@ -117,11 +117,12 @@ class AdvancedFilters extends Component {
onAdvancedFilterAction( 'match', { match } );
}
onFilterChange( index, property, value ) {
onFilterChange( index, property, value, shouldResetValue = false ) {
const newActiveFilters = [ ...this.state.activeFilters ];
newActiveFilters[ index ] = {
...newActiveFilters[ index ],
[ property ]: value,
...( shouldResetValue === true ? { value: null } : {} ),
};
this.setState( { activeFilters: newActiveFilters } );

View File

@ -145,6 +145,32 @@ const advancedFilters = {
type: 'currency',
},
},
date: {
labels: {
add: 'Date',
remove: 'Remove date filter',
rule: 'Select a date filter match',
title: '{{title}}Date{{/title}} {{rule /}} {{filter /}}',
filter: 'Select a transaction date',
},
rules: [
{
value: 'before',
label: 'Before',
},
{
value: 'after',
label: 'After',
},
{
value: 'between',
label: 'Between',
},
],
input: {
component: 'Date',
},
},
},
};

View File

@ -0,0 +1,260 @@
/**
* @jest-environment jsdom
*/
/**
* External dependencies
*/
import { render, screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import { AdvancedFilters } from '@woocommerce/components';
import { CURRENCY } from '@woocommerce/wc-admin-settings';
import { createElement } from '@wordpress/element';
const ORDER_STATUSES = {
cancelled: 'Cancelled',
completed: 'Completed',
failed: 'Failed',
'on-hold': 'On hold',
pending: 'Pending payment',
processing: 'Processing',
refunded: 'Refunded',
};
const advancedFiltersConfig = {
title: 'Orders Match {{select /}} Filters',
filters: {
status: {
labels: {
add: 'Order Status',
remove: 'Remove order status filter',
rule: 'Select an order status filter match',
title:
'{{title}}Order Status{{/title}} {{rule /}} {{filter /}}',
filter: 'Select an order status',
},
rules: [
{
value: 'is',
label: 'Is',
},
{
value: 'is_not',
label: 'Is Not',
},
],
input: {
component: 'SelectControl',
options: Object.keys( ORDER_STATUSES ).map( ( key ) => ( {
value: key,
label: ORDER_STATUSES[ key ],
} ) ),
},
},
product: {
labels: {
add: 'Products',
placeholder: 'Search products',
remove: 'Remove products filter',
rule: 'Select a product filter match',
title: '{{title}}Product{{/title}} {{rule /}} {{filter /}}',
filter: 'Select products',
},
rules: [
{
value: 'includes',
label: 'Includes',
},
{
value: 'excludes',
label: 'Excludes',
},
],
input: {
component: 'Search',
type: 'products',
getLabels: () => Promise.resolve( [] ),
},
},
customer: {
labels: {
add: 'Customer Type',
remove: 'Remove customer filter',
rule: 'Select a customer filter match',
title: '{{title}}Customer is{{/title}} {{filter /}}',
filter: 'Select a customer type',
},
input: {
component: 'SelectControl',
options: [
{ value: 'new', label: 'New' },
{ value: 'returning', label: 'Returning' },
],
defaultOption: 'new',
},
},
quantity: {
labels: {
add: 'Item Quantity',
remove: 'Remove item quantity filter',
rule: 'Select an item quantity filter match',
title:
'{{title}}Item Quantity is{{/title}} {{rule /}} {{filter /}}',
},
rules: [
{
value: 'lessthan',
label: 'Less Than',
},
{
value: 'morethan',
label: 'More Than',
},
{
value: 'between',
label: 'Between',
},
],
input: {
component: 'Number',
},
},
subtotal: {
labels: {
add: 'Subtotal',
remove: 'Remove subtotal filter',
rule: 'Select a subtotal filter match',
title: '{{title}}Subtotal is{{/title}} {{rule /}} {{filter /}}',
},
rules: [
{
value: 'lessthan',
label: 'Less Than',
},
{
value: 'morethan',
label: 'More Than',
},
{
value: 'between',
label: 'Between',
},
],
input: {
component: 'Number',
type: 'currency',
},
},
date: {
labels: {
add: 'Date',
remove: 'Remove date filter',
rule: 'Select a date filter match',
title: '{{title}}Date{{/title}} {{rule /}} {{filter /}}',
filter: 'Select a transaction date',
},
rules: [
{
value: 'before',
label: 'Before',
},
{
value: 'after',
label: 'After',
},
{
value: 'between',
label: 'Between',
},
],
input: {
component: 'Date',
},
},
},
};
const AdvancedFiltersComponent = ( props = null ) => (
<AdvancedFilters
siteLocale="en_US"
path=""
query={ { component: 'advanced-filters' } }
filterTitle="Orders"
config={ advancedFiltersConfig }
currency={ CURRENCY }
{ ...props }
/>
);
describe( 'AdvancedFilters', () => {
test( 'should render', () => {
const { getByRole } = render( <AdvancedFiltersComponent /> );
expect(
getByRole( 'button', { name: 'Add a Filter' } )
).toBeInTheDocument();
} );
test( 'should retain value/state when the rule is switched from "Before" to "After" in <DateFilter />', () => {
render( <AdvancedFiltersComponent /> );
// Add a new filter.
userEvent.click(
screen.getByRole( 'button', { name: 'Add a Filter' } )
);
// Add a "Before" date filter.
userEvent.click( screen.getByRole( 'button', { name: 'Date' } ) );
// Add a date in mm/dd/yyyy format.
userEvent.type(
screen.getByRole( 'textbox', { name: 'Choose a date' } ),
'01/01/2020'
);
// Switch the date filter from "Before" to "After".
userEvent.selectOptions(
screen.getByRole( 'combobox', {
name: 'Select a date filter match',
} ),
'after'
);
// The previous value should be retained when switching between the "Before" and "After" rules.
expect(
screen.getByRole( 'textbox', { name: 'Choose a date' } )
).toHaveValue( '01/01/2020' );
} );
test( 'should reset value/state when the rule is switched to/from "Between" in <DateFilter />', () => {
render( <AdvancedFiltersComponent /> );
// Add a new filter.
userEvent.click(
screen.getByRole( 'button', { name: 'Add a Filter' } )
);
// Add a "Before" date filter.
userEvent.click( screen.getByRole( 'button', { name: 'Date' } ) );
// Add a date in mm/dd/yyyy format.
userEvent.type(
screen.getByRole( 'textbox', { name: 'Choose a date' } ),
'01/01/2020'
);
// Switch the date filter from "Before" to "Between".
userEvent.selectOptions(
screen.getByRole( 'combobox', {
name: 'Select a date filter match',
} ),
'between'
);
const dateFields = screen.getAllByRole( 'textbox', {
name: 'Choose a date',
} );
// The previous value should be reset when switching to/from the "Between" rule.
expect( dateFields[ 0 ] ).not.toHaveValue();
expect( dateFields[ 1 ] ).not.toHaveValue();
} );
} );