Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(time-series table): Can't compare from the beginning of the time range #26814

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,19 @@ test('triggers onChange when time lag changes', () => {
expect(onChange).toHaveBeenCalledWith(expect.objectContaining({ timeLag }));
});

test('time lag allows negative values', () => {
const timeLag = '-1';
const onChange = jest.fn();
render(<TimeSeriesColumnControl colType="time" onChange={onChange} />);
userEvent.click(screen.getByRole('button'));
const timeLagInput = screen.getByPlaceholderText('Time Lag');
userEvent.clear(timeLagInput);
userEvent.type(timeLagInput, timeLag);
expect(onChange).not.toHaveBeenCalled();
userEvent.click(screen.getByRole('button', { name: 'Save' }));
expect(onChange).toHaveBeenCalledWith(expect.objectContaining({ timeLag }));
});

test('triggers onChange when color bounds changes', () => {
const min = 1;
const max = 5;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,9 @@ export default class TimeSeriesColumnControl extends React.Component {
{['time', 'avg'].indexOf(this.state.colType) >= 0 &&
this.formRow(
t('Time lag'),
t('Number of periods to compare against'),
t(
'Number of periods to compare against. You can use negative numbers to compare from the beginning of the time range.',
),
'time-lag',
<Input
value={this.state.timeLag}
Expand Down
6 changes: 4 additions & 2 deletions superset-frontend/src/visualizations/TimeTable/TimeTable.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -181,11 +181,13 @@
let v;
let errorMsg;
if (column.colType === 'time') {
// Time lag ratio
// If time lag is negative, we compare from the beginning of the data
const timeLag = column.timeLag || 0;
const totalLag = Object.keys(reversedEntries).length;
if (timeLag >= totalLag) {
if (Math.abs(timeLag) >= totalLag) {
errorMsg = `The time lag set at ${timeLag} is too large for the length of data at ${reversedEntries.length}. No data available.`;
} else if (timeLag < 0) {
v = reversedEntries[totalLag + timeLag][valueField];

Check warning on line 190 in superset-frontend/src/visualizations/TimeTable/TimeTable.jsx

View check run for this annotation

Codecov / codecov/patch

superset-frontend/src/visualizations/TimeTable/TimeTable.jsx#L190

Added line #L190 was not covered by tests
} else {
v = reversedEntries[timeLag][valueField];
}
Expand Down
Loading