fix(dashboard filters): add "previous quarter" as option #30506
Conversation
dosubot
bot
added
change:frontend
|
@sfirke as per Slack discussion, thank you! |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
personofnorank
changed the title
superset-frontend/src/explore/components/controls/DateFilterControl/types.ts
Outdated
Show resolved
Hide resolved
superset-frontend/packages/superset-ui-core/src/time-comparison/getTimeOffset.ts
Outdated
Show resolved
Hide resolved
| @@ -78,6 +78,11 @@ export const parseDttmToDate = ( | |||
| now.setMonth(now.getMonth() - 1); | |||
| } | |||
| return now; | |||
| case 'previous calendar quarter': { | |||
There was a problem hiding this comment.
@personofnorank You can add tests to the getTimeOffset.test.ts file.
There was a problem hiding this comment.
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'));
});
There was a problem hiding this comment.
It should work because Date allows a negative month.
new Date(2016, -1) gives you December 2015.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #30506 +/- ##
===========================================
+ Coverage 60.48% 83.92% +23.43%
===========================================
Files 1931 533 -1398
Lines 76236 38612 -37624
Branches 8568 0 -8568
===========================================
- Hits 46114 32404 -13710
+ Misses 28017 6208 -21809
+ Partials 2105 0 -2105
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
Looks like there is a pre-commit linting check failing, it says to delete two spaces I think. And then a test failing. Can you investigate the CI failures when you get a chance, @personofnorank ? |
|
Re-running CI :D |
|
@justinpark I think this is pending your review - please let me know if anything needed from my side, with apologies for any oversight as I'm new to the process. |
|
Looks like there is a failing Python test: https://github.com/apache/superset/actions/runs/11278958608/job/31368926066?pr=30506 |
|
Fixed a typo in the test script |
|
Looks like tests are failing with a function getting too many arguments |
|
In the last run the code seemed to be working but test was expecting wrong thing... maybe this time :) |
|
It's getting closer to passing tests 🙏 |
SUMMARY
The custom date range filter should include previous quarter along with previous calendar week, month or year
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION