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

[personofnorank]

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

• [x] Has associated issue: https://github.com/apache/superset/issues/26651
• [ ] Required feature flags:
• [x] Changes UI
• [ ] Includes DB Migration (follow approval process in SIP-59)
  • [ ] Migration is atomic, supports rollback & is backwards-compatible
  • [ ] Confirm DB migration upgrade and downgrade tested
  • [ ] Runtime estimates and downtime expectations provided
• [ ] Introduces new feature or API
• [ ] Removes existing feature or API

[personofnorank]

@sfirke as per Slack discussion, thank you!

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 83.45%. Comparing base (76d897e) to head (cc5d5fc). Report is 1561 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff             @@
##           master   #30506       +/-   ##
===========================================
+ Coverage   60.48%   83.45%   +22.96%     
===========================================
  Files        1931      546     -1385     
  Lines       76236    39112    -37124     
  Branches     8568        0     -8568     
===========================================
- Hits        46114    32642    -13472     
+ Misses      28017     6470    -21547     
+ Partials     2105        0     -2105     

Flag	Coverage Δ
hive	48.51% <ø> (-0.65%)	:arrow_down:
javascript	?
mysql	75.87% <ø> (?)
postgres	75.93% <ø> (?)
presto	53.05% <ø> (-0.76%)	:arrow_down:
python	83.45% <ø> (+19.96%)	:arrow_up:
sqlite	75.43% <ø> (?)
unit	61.11% <ø> (+3.49%)	:arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[sfirke]

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 ?

[rusackas]

Re-running CI :D

[personofnorank]

@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.

[sfirke]

Looks like there is a failing Python test: https://github.com/apache/superset/actions/runs/11278958608/job/31368926066?pr=30506

[personofnorank]

Fixed a typo in the test script

[sfirke]

Looks like tests are failing with a function getting too many arguments

[personofnorank]

In the last run the code seemed to be working but test was expecting wrong thing... maybe this time :)

[sfirke]

It's getting closer to passing tests 🙏

[sfirke]

@personofnorank I forgot this is still open, ack! Can we try to get it done for 5.0.0?

[personofnorank]

@sfirke sorry for slow response, just did a commit, is it too late for the release (assuming it works this time)?

[personofnorank]

Seems to be failing because of "E501 Line too long" in superset/utils/date_parser.py, although that file is full of # pylint: disable=line-too-long,useless-suppression There was an actual code error which I've fixed.

[sfirke]

Thanks for picking this back up! I just re-ran those failing tests in case some were just flaky and failing for unrelated reasons. I agree that adding the pylint: disable is appropriate, it matches what has been done above, and I don't see why it's not working.

[sfirke]

This one at least looks like a real failing test:

 FAIL packages/superset-ui-core/test/time-comparison/getTimeOffset.test.ts (16.271 s)
  ● should handle custom range with previous calendar quarter

    expect(received).toEqual(expected) // deep equality

    - Expected  - 1
    + Received  + 1

      Array [
    -   "5 days ago",
    +   "86 days after",
      ]

      48 |     includeFutureOffsets,
      49 |   });
    > 50 |   expect(result).toEqual(expected_result);
         |                  ^
      51 |   timezoneMock.unregister();
      52 | };
      53 |

      at toEqual (packages/superset-ui-core/test/time-comparison/getTimeOffset.test.ts:50:18)
      at Object.runTimezoneTest (packages/superset-ui-core/test/time-comparison/getTimeOffset.test.ts:537:3)

From https://github.com/apache/superset/actions/runs/13585311794/job/38000863290?pr=30506#step:4:22. Can you investigate?

[personofnorank]

Yes, I had a syntax error (== instead of ===) which would have meant the code didn't execute causing any functional test to fail. I've fixed that now. If it fails again it won't be for that reason!

[sfirke]

Good catch! Did you push that change? I'll approve re-running the tests but don't see the new commit

[personofnorank]

Oops, thought I'd done a push - just done now.

[sfirke]

For the line length, I wonder if adding # noqa: E501 to the end of the line would help? Maybe that comment string needs to occur before the line length hits its limit and that's why it's not working right now? I'm looking at https://docs.astral.sh/ruff/rules/line-too-long/#error-suppression and https://stackoverflow.com/a/77396292

[personofnorank]

Thanks, did a new push.

[personofnorank]

I'm a bit stumped. It's failing on the tests in superset-frontend/packages/superset-ui-core/test/time-comparison/getTimeOffset.test.ts which I added for the "previous calendar quarter" use case as they exist for all the other use cases.

I basically cloned the other tests but I'm not sure what exactly they're doing or whether it matters what value of startDate you set. Setting different values leads to different values in the error message, which implies I could hack it to get the right value but that doesn't feel right without understanding what it's doing.

[LevisNgigi]

Hey, was wondering if this is a duplicate of this? https://github.com/apache/superset/issues/31870

[sfirke]

Hey, was wondering if this is a duplicate of this? #31870

Great catch @LevisNgigi, I think yes that issue and specifically the PR https://github.com/apache/superset/pull/31889 made this one obsolete.

I'm glad to see the functionality will be in Superset 5.0.0 but I'm sorry @personofnorank that your hard work did not make it in. A lesson to project committers to try to get PRs like this across the finish line faster so they don't get superseded, I'll try to do more in the future in cases like this.

[personofnorank]

Well, an earlier issue can't be a duplicate of a later one :) but I'm glad this is getting fixed, and it was all good experience so thank you to everyone.