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.

Sign up for GitHub

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

Jump to bottom

fix(dashboard filters): add "previous quarter" as option #30506

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
+56 −0
Open

fix(dashboard filters): add "previous quarter" as option #30506

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
001d61e
Added previous quarter to date filer
personofnorank Oct 3, 2024
1383614
Added previous quarter logic to custom frame
personofnorank Oct 3, 2024
06124a4
Added previous quarter logic to custom frame
personofnorank Oct 3, 2024
aa31f1a
Fixed bug in quarter variable declaration
personofnorank Oct 3, 2024
5246b5a
Fixed unexpected lexical declaration in case block
personofnorank Oct 3, 2024
8614cc7
Updated previous quarter language in response to feedback from @micha…
personofnorank Oct 3, 2024
13db748
Added unit test for previous calendar quarter filter
personofnorank Oct 3, 2024
d1f8921
Fixed syntax error
personofnorank Oct 4, 2024
962c51c
Added previous quarter coverage to getTimeOffset.test.ts
personofnorank Oct 4, 2024
392cd0c
Deleted two spaces as required by prettier
personofnorank Oct 4, 2024
13c8ffa
get_since_until should return two dates not one
personofnorank Oct 4, 2024
30e2e2d
get_since_until should return two dates not one
personofnorank Oct 4, 2024
c36bdb8
Prettier/lint fixes
personofnorank Oct 4, 2024
83e411e
Removed trailing whitespace
personofnorank Oct 7, 2024
7333ffe
Removed trailing whitespace
personofnorank Oct 7, 2024
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
14 changes: 14 additions & 0 deletions superset-frontend/packages/superset-ui-core/src/time-comparison/getTimeOffset.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,20 @@ export const parseDttmToDate = (
now.setMonth(now.getMonth() - 1);
}
return now;
case 'previous calendar quarter': {
Copy link
Member

@michael-s-molina michael-s-molina Oct 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@personofnorank You can add tests to the getTimeOffset.test.ts file.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @personofnorank. There are problems with this logic as it can result in an invalid month value (e.g., -3, -2, -1) when the current quarter is the first quarter of the year.

I suggest adding some tests like these:

test('should return the first day of the previous calendar quarter when current date is in Q4', () => {
  const result = getTimeOffset('previous calendar quarter', false, now);
  expect(result).toEqual(new Date('2023-07-01T00:00:00Z'));
});

test('should return the first day of the previous calendar quarter when current date is in Q3', () => {
  now = new Date('2023-08-15T00:00:00Z');
  const result = getTimeOffset('previous calendar quarter', false, now);
  expect(result).toEqual(new Date('2023-04-01T00:00:00Z'));
});

test('should return the first day of the previous calendar quarter when current date is in Q2', () => {
  now = new Date('2023-05-15T00:00:00Z');
  const result = getTimeOffset('previous calendar quarter', false, now);
  expect(result).toEqual(new Date('2023-01-01T00:00:00Z'));
});

test('should return the first day of the previous calendar quarter when current date is in Q1', () => {
  now = new Date('2023-02-15T00:00:00Z');
  const result = getTimeOffset('previous calendar quarter', false, now);
  expect(result).toEqual(new Date('2022-10-01T00:00:00Z'));
});

Copy link
Author

@personofnorank personofnorank Oct 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should work because Date allows a negative month.
new Date(2016, -1) gives you December 2015.

const quarter = Math.floor(now.getMonth() / 3);
const startquarter = new Date(now.getFullYear(), quarter * 3 - 3, 1);
const endquarter = new Date(
startquarter.getFullYear(),
startquarter.getMonth() + 3,
0
);
if (isEndDate) {
return endquarter;
} else {
return startquarter;
}
}
case 'previous calendar year':
if (isEndDate) {
now.setFullYear(now.getFullYear(), 0, 1); // end date is the last day of the previous year
Expand Down
33 changes: 33 additions & 0 deletions superset-frontend/packages/superset-ui-core/test/time-comparison/getTimeOffset.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -527,6 +527,39 @@ test('should handle custom range with previous calendar month', () => {
);
});

test('should handle custom range with previous calendar quarter', () => {
const timeRangeFilter = {
comparator: 'previous calendar quarter',
};
const shifts = ['custom'];
const startDate = '2024-04-26';
jest.useFakeTimers();
runTimezoneTest(
'2024-06-05T02:06:00+02:00',
'Etc/GMT-2',
timeRangeFilter,
shifts,
startDate,
['5 days ago'],
);
runTimezoneTest(
'2024-06-05T00:06:00Z',
'UTC',
timeRangeFilter,
shifts,
startDate,
['5 days ago'],
);
runTimezoneTest(
'2024-06-04T16:06:00-08:00',
'Etc/GMT+8',
timeRangeFilter,
shifts,
startDate,
['5 days ago'],
);
});

test('should handle custom range with previous calendar year', () => {
const timeRangeFilter = {
comparator: 'previous calendar year',
Expand Down
2 changes: 2 additions & 0 deletions superset-frontend/src/explore/components/controls/DateFilterControl/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,12 @@ export type CommonRangeType =

export const PreviousCalendarWeek = 'previous calendar week';
export const PreviousCalendarMonth = 'previous calendar month';
export const PreviousCalendarQuarter = 'previous calendar quarter';
export const PreviousCalendarYear = 'previous calendar year';
export type CalendarRangeType =
| typeof PreviousCalendarWeek
| typeof PreviousCalendarMonth
| typeof PreviousCalendarQuarter
| typeof PreviousCalendarYear;

export const CurrentDay = 'Current day';
Expand Down
3 changes: 3 additions & 0 deletions superset-frontend/src/explore/components/controls/DateFilterControl/utils/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
SelectOptionType,
PreviousCalendarWeek,
PreviousCalendarMonth,
PreviousCalendarQuarter,
PreviousCalendarYear,
CommonRangeType,
CalendarRangeType,
Expand Down Expand Up @@ -56,6 +57,7 @@ export const COMMON_RANGE_VALUES_SET = new Set(
export const CALENDAR_RANGE_OPTIONS: SelectOptionType[] = [
{ value: PreviousCalendarWeek, label: t('previous calendar week') },
{ value: PreviousCalendarMonth, label: t('previous calendar month') },
{ value: PreviousCalendarQuarter, label: t('previous calendar quarter') },
{ value: PreviousCalendarYear, label: t('previous calendar year') },
];
export const CALENDAR_RANGE_VALUES_SET = new Set(
Expand Down Expand Up @@ -119,6 +121,7 @@ export const COMMON_RANGE_SET: Set<CommonRangeType> = new Set([
export const CALENDAR_RANGE_SET: Set<CalendarRangeType> = new Set([
PreviousCalendarWeek,
PreviousCalendarMonth,
PreviousCalendarQuarter,
PreviousCalendarYear,
]);

Expand Down
4 changes: 4 additions & 0 deletions tests/unit_tests/utils/date_parser_tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,10 @@ def test_get_since_until() -> None:
expected = datetime(2016, 10, 1, 0, 0, 0), datetime(2016, 11, 1, 0, 0, 0)
assert result == expected

result = get_since_until("previous calendar quarter")
expected = datetime(2016, 7, 1, 0, 0, 0), datetime(2016, 9, 30, 0, 0, 0)
assert result == expected

result = get_since_until("previous calendar year")
expected = datetime(2015, 1, 1, 0, 0, 0), datetime(2016, 1, 1, 0, 0, 0)
assert result == expected
Expand Down