Skip to content

Commit

Permalink
fix: SQL Lab sorting of non-numbers (#18006)
Browse files Browse the repository at this point in the history
  • Loading branch information
Erik Ritter authored and eschutho committed Jan 27, 2022
1 parent ab32149 commit 5f2553d
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -271,4 +271,63 @@ describe('FilterableTable sorting - RTL', () => {
expect(gridCells[9]).toHaveTextContent('3439718.0300000007');
expect(gridCells[10]).toHaveTextContent('4528047.219999993');
});

it('sorts YYYY-MM-DD properly', () => {
const dsProps = {
orderedColumnKeys: ['ds'],
data: [
{ ds: '2021-01-01' },
{ ds: '2022-01-01' },
{ ds: '2021-01-02' },
{ ds: '2021-01-03' },
{ ds: '2021-12-01' },
{ ds: '2021-10-01' },
{ ds: '2022-01-02' },
],
height: 500,
};
render(<FilterableTable {...dsProps} />);

const dsColumn = screen.getByRole('columnheader', { name: 'ds' });
const gridCells = screen.getAllByRole('gridcell');

// Original order
expect(gridCells[0]).toHaveTextContent('2021-01-01');
expect(gridCells[1]).toHaveTextContent('2022-01-01');
expect(gridCells[2]).toHaveTextContent('2021-01-02');
expect(gridCells[3]).toHaveTextContent('2021-01-03');
expect(gridCells[4]).toHaveTextContent('2021-12-01');
expect(gridCells[5]).toHaveTextContent('2021-10-01');
expect(gridCells[6]).toHaveTextContent('2022-01-02');

// First click to sort ascending
userEvent.click(dsColumn);
expect(gridCells[0]).toHaveTextContent('2021-01-01');
expect(gridCells[1]).toHaveTextContent('2021-01-02');
expect(gridCells[2]).toHaveTextContent('2021-01-03');
expect(gridCells[3]).toHaveTextContent('2021-10-01');
expect(gridCells[4]).toHaveTextContent('2021-12-01');
expect(gridCells[5]).toHaveTextContent('2022-01-01');
expect(gridCells[6]).toHaveTextContent('2022-01-02');

// Second click to sort descending
userEvent.click(dsColumn);
expect(gridCells[0]).toHaveTextContent('2022-01-02');
expect(gridCells[1]).toHaveTextContent('2022-01-01');
expect(gridCells[2]).toHaveTextContent('2021-12-01');
expect(gridCells[3]).toHaveTextContent('2021-10-01');
expect(gridCells[4]).toHaveTextContent('2021-01-03');
expect(gridCells[5]).toHaveTextContent('2021-01-02');
expect(gridCells[6]).toHaveTextContent('2021-01-01');

// Third click to sort ascending again
userEvent.click(dsColumn);
expect(gridCells[0]).toHaveTextContent('2021-01-01');
expect(gridCells[1]).toHaveTextContent('2021-01-02');
expect(gridCells[2]).toHaveTextContent('2021-01-03');
expect(gridCells[3]).toHaveTextContent('2021-10-01');
expect(gridCells[4]).toHaveTextContent('2021-12-01');
expect(gridCells[5]).toHaveTextContent('2022-01-01');
expect(gridCells[6]).toHaveTextContent('2022-01-02');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,10 @@ const JSON_TREE_THEME = {
base0E: '#ae81ff',
base0F: '#cc6633',
};
// This regex handles all possible number formats in javascript, including ints, floats,
// exponential notation, NaN, and Infinity.
// See https://stackoverflow.com/a/30987109 for more details
const ONLY_NUMBER_REGEX = /^(NaN|-?((\d*\.\d+|\d+)([Ee][+-]?\d+)?|Infinity))$/;

const StyledFilterableTable = styled.div`
height: 100%;
Expand Down Expand Up @@ -321,16 +325,21 @@ export default class FilterableTable extends PureComponent<
);
}

// Parse any floating numbers so they'll sort correctly
parseFloatingNums = (value: any) => {
const floatValue = parseFloat(value);
return Number.isNaN(floatValue) ? value : floatValue;
// Parse any numbers from strings so they'll sort correctly
parseNumberFromString = (value: string | number | null) => {
if (typeof value === 'string') {
if (ONLY_NUMBER_REGEX.test(value)) {
return parseFloat(value);
}
}

return value;
};

sortResults(sortBy: string, descending: boolean) {
return (a: Datum, b: Datum) => {
const aValue = this.parseFloatingNums(a[sortBy]);
const bValue = this.parseFloatingNums(b[sortBy]);
const aValue = this.parseNumberFromString(a[sortBy]);
const bValue = this.parseNumberFromString(b[sortBy]);

// equal items sort equally
if (aValue === bValue) {
Expand Down

0 comments on commit 5f2553d

Please sign in to comment.