feat(date-filter): add sub-hour time range presets (needs review)#41527
feat(date-filter): add sub-hour time range presets (needs review)#41527imddevaraj wants to merge 14 commits into
Conversation
… Last 1 hour) Add four new preset options to the DateFilterControl Common (Last...) frame: - Last 5 minutes - Last 15 minutes - Last 30 minutes - Last 1 hour These presets appear in both the Explore chart builder and the Dashboard native Time filter, grouped above the existing day/week/month presets. Backend: fix a latent bug where 'hour' was missing from the time-unit regex in get_since_until(), causing 'Last 1 hour' to fail to parse despite being documented as supported. The regex now covers second|minute|hour|day|week|month|quarter|year. Frontend: - Extend CommonRangeType union with the four new string literals - Add entries to COMMON_RANGE_OPTIONS and COMMON_RANGE_SET in constants.ts - COMMON_RANGE_VALUES_SET and guessFrame() update automatically Tests: - Frontend: two new test() blocks asserting guessFrame() returns 'Common' for all four new presets and that existing presets are unaffected - Backend: test_get_since_until_sub_hour_presets covers all four presets with relative_end='now' and asserts correct datetime pairs
dosubot
Bot
added
change:backend
![codeant-ai-for-open-source[bot]](https://avatars.githubusercontent.com/u/151810221?s=60&v=4){kind=small}
|
The suggestion to add a type annotation to the local variable is valid, as it improves code readability and maintains consistency with the existing codebase. To resolve this, you should explicitly declare the type of the variable when it is initialized. Since the provided context does not include the specific lines in Regarding other comments on this PR, the current review data is empty, so there are no additional comments to address at this time. |
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![bito-code-review[bot]](https://avatars.githubusercontent.com/in/1061978?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Agent Run #f83bec
Actionable Suggestions - 1
-
superset-frontend/src/explore/components/controls/DateFilterControl/tests/utils.test.ts - 1
- Incomplete test coverage · Line 192-197
Additional Suggestions - 1
-
superset-frontend/src/explore/components/controls/DateFilterControl/tests/utils.test.ts - 1
-
Misleading test name · Line 199-205Test name uses 'still returns' implying a regression scenario, but no prior behavior is documented in the diff. If these test existing behavior (not a regression), consider a more accurate name like 'guessFrame returns Common for day/week/month presets'.
-
Filtered by Review Rules
Bito filtered these suggestions based on rules created automatically for your feedback. Manage rules.
-
superset-frontend/src/explore/components/controls/DateFilterControl/types.ts - 1
- Missing COMMON_RANGE_SET entries · Line 75-78
Review Details
-
Files reviewed - 6 · Commit Range:
50873ec..50873ec- superset-frontend/.gitignore
- superset-frontend/src/explore/components/controls/DateFilterControl/tests/utils.test.ts
- superset-frontend/src/explore/components/controls/DateFilterControl/types.ts
- superset-frontend/src/explore/components/controls/DateFilterControl/utils/constants.ts
- superset/utils/date_parser.py
- tests/unit_tests/utils/date_parser_tests.py
-
Files skipped - 0
-
Tools
- MyPy (Static Code Analysis) - ✔︎ Successful
- Astral Ruff (Static Code Analysis) - ✔︎ Successful
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
- Eslint (Linter) - ✔︎ Successful
Bito Usage Guide
Commands
Type the following command in the pull request comment and save the comment.
-
/review- Manually triggers a full AI review. -
/pause- Pauses automatic reviews on this pull request. -
/resume- Resumes automatic reviews. -
/resolve- Marks all Bito-posted review comments as resolved. -
/abort- Cancels all in-progress reviews.
Refer to the documentation for additional commands.
Configuration
This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.
Documentation & Help
AI Code Review powered by
|
@yousoph @kasiazjc thoughts on this feature? @imddevaraj wondering if we can make this more generic? Like right now we are adding some defaults but the list gets bigger as others may want more like, what if i wanted last 4 hours, 24 hours, 8 hours, etc? |
|
/review |
Code Review Agent Run #6a52b5Actionable Suggestions - 0Filtered by Review RulesBito filtered these suggestions based on rules created automatically for your feedback. Manage rules.
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
…icker - Backend: Last/Next N second/minute/hour now appends 'now' as the until/since bound instead of 'today' (midnight), preventing the 'From date cannot be larger than to date' error that fired at any time of day when DEFAULT_RELATIVE_END_TIME='today'. - Frontend: Replace the fixed radio list in CommonFrame with an 'Other' row that exposes a number input and unit select, enabling arbitrary ranges like 'Last 4 hours' or 'Last 24 hours' without hard-coding every possible preset. - Tests: new Python test test_get_since_until_granular_units_use_now_by_default and new Jest suite CommonFrame.test.tsx (10 tests).
|
/review |
@sadpandajoe |
There was a problem hiding this comment.
Code Review Agent Run #14bd76
Actionable Suggestions - 2
-
superset-frontend/src/explore/components/controls/DateFilterControl/components/CommonFrame.tsx - 1
- Missing useEffect for side-effect in render · Line 77-79
-
superset-frontend/src/explore/components/controls/DateFilterControl/utils/constants.ts - 1
- Type incompatibility in deprecated export · Line 122-125
Additional Suggestions - 3
-
superset-frontend/src/explore/components/controls/DateFilterControl/tests/CommonFrame.test.tsx - 2
-
Flaky onChange assertion · Line 50-54The test 'defaults to Last week when value does not match any known range' asserts `onChange` was called with 'Last week' but doesn't verify call count. Since `CommonFrame` calls `props.onChange('Last week')` during render (line 77-79), React StrictMode or future concurrent-render changes could cause multiple calls, making this assertion pass incorrectly. Add `expect(onChange).toHaveBeenCalledTimes(1)` to enforce exactly one call.
-
Flaky onChange assertion · Line 92-103The test 'changing the number input emits an updated Last N unit string' extracts the last onChange call without verifying call count. If React StrictMode causes an extra render-time call, the last call might not be the expected input-change emission. Add `expect(onChange).toHaveBeenCalledTimes(1)` to guarantee the test captures only the user-interaction emission.
-
-
superset-frontend/src/explore/components/controls/DateFilterControl/types.ts - 1
-
Type narrowing lost on union-to-string · Line 80-80The change from a literal union type to `string` loses compile-time type safety. While the JSDoc explains the runtime validation via `COMMON_RANGE_REGEX`, the type cannot guide callers toward valid values at compile time. A branded type would restore some safety while supporting the flexible pattern.
-
Filtered by Review Rules
Bito filtered these suggestions based on rules created automatically for your feedback. Manage rules.
-
superset-frontend/src/explore/components/controls/DateFilterControl/components/CommonFrame.tsx - 1
- Duplicate regex not using imported constant · Line 51-56
Review Details
-
Files reviewed - 9 · Commit Range:
50873ec..41c45d5- superset-frontend/.gitignore
- superset-frontend/src/explore/components/controls/DateFilterControl/components/CommonFrame.tsx
- superset-frontend/src/explore/components/controls/DateFilterControl/tests/CommonFrame.test.tsx
- superset-frontend/src/explore/components/controls/DateFilterControl/tests/utils.test.ts
- superset-frontend/src/explore/components/controls/DateFilterControl/types.ts
- superset-frontend/src/explore/components/controls/DateFilterControl/utils/constants.ts
- superset-frontend/src/explore/components/controls/DateFilterControl/utils/dateFilterUtils.ts
- superset/utils/date_parser.py
- tests/unit_tests/utils/date_parser_tests.py
-
Files skipped - 0
-
Tools
- MyPy (Static Code Analysis) - ✔︎ Successful
- Astral Ruff (Static Code Analysis) - ✔︎ Successful
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
- Eslint (Linter) - ✔︎ Successful
Bito Usage Guide
Commands
Type the following command in the pull request comment and save the comment.
-
/review- Manually triggers a full AI review. -
/pause- Pauses automatic reviews on this pull request. -
/resume- Resumes automatic reviews. -
/resolve- Marks all Bito-posted review comments as resolved. -
/abort- Cancels all in-progress reviews.
Refer to the documentation for additional commands.
Configuration
This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.
Documentation & Help
AI Code Review powered by
Code Review Agent Run #6dc29aActionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
…assertions, simplify onChange effect
|
/review |
Code Review Agent Run #4a179eActionable Suggestions - 0Additional Suggestions - 2
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
Code Review Agent Run #8efc47Actionable Suggestions - 0Additional Suggestions - 2
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #41527 +/- ##
==========================================
- Coverage 64.61% 64.61% -0.01%
==========================================
Files 2684 2684
Lines 148514 148550 +36
Branches 34263 34274 +11
==========================================
+ Hits 95969 95989 +20
- Misses 50786 50799 +13
- Partials 1759 1762 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
- Replace any with RadioChangeEvent in CommonFrame handleRadioChange - Add str annotations to granular regex pattern variables in date_parser - Make numeric count optional in granular patterns to match Last/Next hour forms - Fix customN/customUnit init to use defaults when current value is a preset, preventing Other click from emitting a preset string and snapping radio back - Add explicit type annotations to expected, expected_until, expected_since in date_parser unit tests
|
/resolve |
There was a problem hiding this comment.
Code Review Agent Run #4b128a
Actionable Suggestions - 2
-
superset/utils/date_parser.py - 2
- CWE-20: Regex quantifier precedence bug · Line 434-434
- CWE-20: Identical regex bug in Next pattern · Line 444-444
Review Details
-
Files reviewed - 3 · Commit Range:
a148e80..77b3792- superset-frontend/src/explore/components/controls/DateFilterControl/components/CommonFrame.tsx
- superset/utils/date_parser.py
- tests/unit_tests/utils/date_parser_tests.py
-
Files skipped - 0
-
Tools
- MyPy (Static Code Analysis) - ✔︎ Successful
- Astral Ruff (Static Code Analysis) - ✔︎ Successful
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
Bito Usage Guide
Commands
Type the following command in the pull request comment and save the comment.
-
/review- Manually triggers a full AI review. -
/pause- Pauses automatic reviews on this pull request. -
/resume- Resumes automatic reviews. -
/resolve- Marks all Bito-posted review comments as resolved. -
/abort- Cancels all in-progress reviews.
Refer to the documentation for additional commands.
Configuration
This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.
Documentation & Help
AI Code Review powered by
…erns Replace (second|minute|hour)s? with (?:seconds?|minutes?|hours?) in both _granular_last_pattern and _granular_next_pattern so each unit owns its own optional plural suffix, removes the unused capturing group, and eliminates any alternation-precedence ambiguity (CWE-20).
Code Review Agent Run #c7ceaeActionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
|
/resolve |
| if (!m || m[1] === undefined) return null; | ||
| return { n: parseInt(m[1], 10), unit: m[2].toLowerCase() }; |
There was a problem hiding this comment.
Suggestion: The custom parser rejects valid shorthand ranges like Last hour/Last minute because it requires the numeric capture group to be present, and those values then get treated as invalid and reset to Last week. Treat missing numbers for non-preset units as 1 (or tighten the regex so such forms are never considered valid) to avoid silently overwriting valid user ranges. [incorrect condition logic]
Severity Level: Major ⚠️
- ❌ "Last hour" defaults revert to "Last week" silently.
- ⚠️ Custom granular Last ranges misparsed and overwritten in CommonFrame.Steps of Reproduction ✅
1. In backend config at `superset/config.py:211`, set `DEFAULT_TIME_FILTER = "Last hour"`
so new charts and filters default to the shorthand granular range instead of the shipped
`NO_TIME_RANGE`.
2. Load an Explore view or dashboard with a native time filter; the frontend hydrates
`time_range` from `DEFAULT_TIME_FILTER` via `hydrateExplore.ts:93`, and `DateFilterLabel`
initializes `value` to `"Last hour"` at `DateFilterLabel.tsx:14`.
3. `DateFilterLabel` calls `guessFrame(value)` at `DateFilterLabel.tsx:18`, which uses
`COMMON_RANGE_REGEX.test(timeRange)` in `dateFilterUtils.ts:32-33`; `"Last hour"` matches
the regex, so `guessFrame` returns `'Common'` and the Common frame is rendered at
`DateFilterLabel.tsx:35-37` with `props.value === "Last hour"`.
4. Inside `CommonFrame` (`CommonFrame.tsx:65`), `isPreset` is computed from
`PRESET_VALUES` (built from `COMMON_RANGE_OPTIONS` at `constants.ts:46-55`), which does
not include `"Last hour"` (only `"Last 1 hour"`), so `isPreset` is false;
`parseLastN(props.value)` at `CommonFrame.tsx:52-57` matches `COMMON_RANGE_REGEX` but has
`m[1] === undefined` for `"Last hour"`, causing `parseLastN` to return `null`. As a result
`isCustom = !isPreset && parsedCustom !== null` evaluates to false, and the `useEffect` at
`CommonFrame.tsx:78-84` treats the value as invalid and calls `props.onChange('Last
week')`, silently overwriting the user’s intended `"Last hour"` range with `"Last week"`
whenever the Common frame is mounted.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** superset-frontend/src/explore/components/controls/DateFilterControl/components/CommonFrame.tsx
**Line:** 56:57
**Comment:**
*Incorrect Condition Logic: The custom parser rejects valid shorthand ranges like `Last hour`/`Last minute` because it requires the numeric capture group to be present, and those values then get treated as invalid and reset to `Last week`. Treat missing numbers for non-preset units as `1` (or tighten the regex so such forms are never considered valid) to avoid silently overwriting valid user ranges.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| export const COMMON_RANGE_REGEX = | ||
| /^[Ll]ast\s+(\d+\s+)?(second|minute|hour|day|week|month|quarter|year)s?$/ |
There was a problem hiding this comment.
Suggestion: This regex now accepts lowercase last ..., but the backend shorthand handling is case-sensitive on startswith("Last"), so values like last 15 minutes can be classified as Common in the UI but fail to resolve correctly server-side. Align casing expectations (either require Last here or make backend prefix checks case-insensitive) to prevent frontend/backend contract mismatch. [api mismatch]
Severity Level: Major ⚠️
- ❌ Lowercase Common ranges ignored by backend time_range shorthand.
- ⚠️ Frontend shows valid label while backend uses midnight.Steps of Reproduction ✅
1. Configure a lowercase shorthand Common range by setting `DEFAULT_TIME_FILTER = "last 15
minutes"` in `superset/config.py:211`, which is allowed by the config comment (“values may
be "Last day", "Last week", <ISO date> : now, etc.”) and will be propagated to the
frontend as the default time range.
2. Open any chart or dashboard that uses the time range control; `DateFilterLabel` reads
`defaultTimeFilter` from Redux via `useDefaultTimeFilter()` (`dateFilterUtils.ts:51-56`)
and initializes its local `value` to `"last 15 minutes"` at `DateFilterLabel.tsx:14`.
3. `DateFilterLabel` computes `guessedFrame = guessFrame(value)` at
`DateFilterLabel.tsx:18`; `guessFrame` in `dateFilterUtils.ts:32-35` calls
`COMMON_RANGE_REGEX.test(timeRange)`, and because the regex explicitly allows both `Last`
and `last` (`constants.ts:63-64`), `"last 15 minutes"` is classified as `'Common'`. The
overlay renders `CommonFrame` at `DateFilterLabel.tsx:35-37` with `timeRangeValue ===
"last 15 minutes"`, and clicking Apply calls `onSave()` at `DateFilterLabel.tsx:100-104`,
forwarding the unchanged lowercase string to `onChange(timeRangeValue)` and the backend.
4. On the server side, chart queries and time range evaluation call
`get_since_until(time_range=value, ...)` in `superset/utils/date_parser.py:388-396`. The
granular-unit shorthand logic at `date_parser.py:49-58` checks `if time_range and
time_range.startswith("Last") and separator not in time_range:`; for `"last 15 minutes"`
this prefix check fails because it is case-sensitive, so `_granular_last_pattern` is never
applied and `time_range` is not rewritten to `"last 15 minutes : now"`. Because the string
lacks the `" : "` separator, the later branch at `date_parser.py:8-79` is skipped and the
function falls back to the else clause at `date_parser.py:80-89`, which ignores
`time_range` entirely and returns `_since = None` and `_until =
parse_human_datetime(_relative_end)` (defaulting to `"today"`). The backend thus executes
queries using a generic “until today midnight” window instead of the intended “last 15
minutes” range, creating a frontend/backend contract mismatch for lowercase Common ranges.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** superset-frontend/src/explore/components/controls/DateFilterControl/utils/constants.ts
**Line:** 63:64
**Comment:**
*Api Mismatch: This regex now accepts lowercase `last ...`, but the backend shorthand handling is case-sensitive on `startswith("Last")`, so values like `last 15 minutes` can be classified as Common in the UI but fail to resolve correctly server-side. Align casing expectations (either require `Last` here or make backend prefix checks case-insensitive) to prevent frontend/backend contract mismatch.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
imddevaraj
changed the title
|
| Language | Fuzzy before | Fuzzy after | New |
|---|---|---|---|
ar |
1130 | 1133 | +3 |
it |
1660 | 1662 | +2 |
ko |
1514 | 1516 | +2 |
pt |
1835 | 1837 | +2 |
How to fix
1. Install dependencies (if not already set up):
pip install -r superset/translations/requirements.txt
sudo apt-get install gettext # or: brew install gettext2. Re-extract strings and sync .po files:
./scripts/translations/babel_update.shThis rewrites superset/translations/messages.pot from the current source files and merges the changes into every .po file. Strings whose msgid changed will be marked #, fuzzy.
3. Resolve the fuzzy entries in the affected language files (ar, it, ko, pt):
grep -n '#, fuzzy' superset/translations/<lang>/LC_MESSAGES/messages.poFor each fuzzy entry, either rewrite the msgstr to match the new string and remove the #, fuzzy line, or clear the msgstr to "" if you cannot provide a translation.
4. Commit your changes to the .po files.
| /** | ||
| * Matches any "Last <N> <unit>(s)" time range string. | ||
| * Mirrors the backend regex in superset/utils/date_parser.py, so arbitrary | ||
| * values like "Last 4 hours" or "Last 24 hours" are recognised without | ||
| * needing to enumerate every possibility in a hardcoded list. | ||
| */ | ||
| export const COMMON_RANGE_REGEX = | ||
| /^[Ll]ast\s+(\d+\s+)?(second|minute|hour|day|week|month|quarter|year)s?$/ |
There was a problem hiding this comment.
Suggestion: The comment states this regex matches Last <N> <unit>, but the implementation makes the number optional, so it also matches forms without N (for example Last hour). Update the comment (or the regex) so documentation and behavior are aligned; the current mismatch is misleading for future maintenance. [comment mismatch]
Severity Level: Minor 🧹
⚠️ Comment slightly misleading about optional numeric component.
⚠️ Future maintainers may misinterpret regex numeric requirement.Steps of Reproduction ✅
1. Inspect `COMMON_RANGE_REGEX` in
`superset-frontend/src/explore/components/controls/DateFilterControl/utils/constants.ts`
at lines 57–64; the comment states it "Matches any 'Last <N> <unit>(s)' time range string"
while the regex implementation uses an optional numeric group `(\d+\s+)?`.
2. Observe that `COMMON_RANGE_REGEX` is used by `guessFrame()` in
`superset-frontend/src/explore/components/controls/DateFilterControl/utils/dateFilterUtils.ts:32-49`,
which calls `COMMON_RANGE_REGEX.test(timeRange)`; this successfully classifies numberless
forms like "Last hour" as "Common", matching backend support shown in `get_since_until()`
(date_parser.py lines 169–181) and its tests.
3. Note that type-level documentation in `types.ts:75-82` also describes "Any 'Last <N>
<unit>(s)' time range string" even though both frontend and backend explicitly support
numberless single-unit forms (e.g., "Last hour"), meaning the comments slightly
under-document the actual behavior but do not cause runtime misclassification.
4. Because the code paths (guessFrame, CommonFrame, backend parsing) already rely on the
optional numeric group and tests cover numberless variants, this suggestion concerns
documentation clarity only; existing behavior is intentional and correct, so updating the
comment would be a minor maintenance improvement rather than a functional bug fix.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** superset-frontend/src/explore/components/controls/DateFilterControl/utils/constants.ts
**Line:** 57:64
**Comment:**
*Comment Mismatch: The comment states this regex matches `Last <N> <unit>`, but the implementation makes the number optional, so it also matches forms without `N` (for example `Last hour`). Update the comment (or the regex) so documentation and behavior are aligned; the current mismatch is misleading for future maintenance.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
| if re.search(_granular_last_pattern, time_range, re.IGNORECASE): | ||
| time_range = time_range + separator + "now" |
There was a problem hiding this comment.
Suggestion: The new granular Last branch always appends now for seconds/minutes/hours, which overrides a caller-provided relative_end (for example relative_end="today" from extras/config). That breaks the existing contract where relative_end controls the end anchor; only default/unset cases should be forced to now. [api mismatch]
Severity Level: Major ⚠️
- ⚠️ Caller-provided relative_end ignored for sub-day “Last” ranges.
- ⚠️ Chart extras.relative_end no longer affects granular “Last”.
- ⚠️ /v1/time_range endpoint can’t honor end anchor overrides.Steps of Reproduction ✅
1. Note that callers can explicitly control the end anchor via the `relative_end`
argument. For chart requests, `BaseViz.query_obj` in `superset/viz.py:394–401` calls
`get_since_until(relative_end=current_app.config["DEFAULT_RELATIVE_END_TIME"], ...)`, and
`get_since_until_from_time_range` in `superset/common/utils/time_range_utils.py:29–46`
passes `relative_end=(extras or {}).get("relative_end",
current_app.config["DEFAULT_RELATIVE_END_TIME"])` into `get_since_until()`.
2. Configure or use a chart that supplies a specific `relative_end` (for example via the
`relative_end` field in `ChartDataExtrasSchema` at `superset/charts/schemas.py:14–20`,
which documents this as “End time for relative time deltas” and allows values `"today"` or
`"now"`). This value flows into `extras["relative_end"]`, then into `get_since_until()`
through `get_since_until_from_time_range()`.
3. From the Explore UI or a dashboard native time filter, choose a granular “Last” range
such as “Last 5 minutes”. This sets `time_range="Last 5 minutes"` on the backend, which
reaches `get_since_until()` in `superset/utils/date_parser.py:9–17` with the
caller-provided `relative_end` already resolved into `_relative_end` at lines 42–44.
4. Inside `get_since_until()`, the new block at `superset/utils/date_parser.py:49–60`
matches `"Last 5 minutes"` against `_granular_last_pattern` and executes:
`if re.search(_granular_last_pattern, time_range, re.IGNORECASE): time_range =
time_range + separator + "now"`.
This unconditionally appends `"now"` (lines 55–58), bypassing `_relative_end` (line
44). As a result, any explicit `relative_end` supplied by the caller or via config is
ignored for granular “Last” ranges, changing the previous contract where `relative_end`
determined the end anchor.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** superset/utils/date_parser.py
**Line:** 436:437
**Comment:**
*Api Mismatch: The new granular `Last` branch always appends `now` for seconds/minutes/hours, which overrides a caller-provided `relative_end` (for example `relative_end="today"` from extras/config). That breaks the existing contract where `relative_end` controls the end anchor; only default/unset cases should be forced to `now`.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| time_range = "now" + separator + time_range | ||
| else: |
There was a problem hiding this comment.
Suggestion: The new granular Next branch always prepends now, so an explicit relative_start from callers is ignored for seconds/minutes/hours. This changes behavior for clients that intentionally set relative_start and should only happen when no explicit start anchor is provided. [api mismatch]
Severity Level: Major ⚠️
- ⚠️ Caller-provided relative_start ignored for sub-day “Next”.
- ⚠️ Chart extras.relative_start ineffective for granular future windows.
- ⚠️ /v1/time_range future ranges can’t respect start anchor.Steps of Reproduction ✅
1. Observe that callers can control the start anchor via the `relative_start` argument.
`BaseViz.query_obj` in `superset/viz.py:394–401` passes
`relative_start=current_app.config["DEFAULT_RELATIVE_START_TIME"]`, and
`get_since_until_from_time_range` in `superset/common/utils/time_range_utils.py:29–46`
passes `relative_start=(extras or {}).get("relative_start",
current_app.config["DEFAULT_RELATIVE_START_TIME"])` into `get_since_until()`.
2. Configure or use a chart that sets `relative_start` (either via global config
`DEFAULT_RELATIVE_START_TIME` in `superset/config.py:2328–2337` or per-chart
`relative_start` in `ChartDataExtrasSchema` at `superset/charts/schemas.py:6–13`). This
value is propagated into `get_since_until()` as the `relative_start` parameter.
3. Supply a granular “Next” time range, such as `"Next 5 minutes"`, either by calling the
`/v1/time_range/` endpoint (`superset/views/api.py:105–132`) with `timeRange="Next 5
minutes"` or by setting the chart’s `time_range` to that string. This reaches
`get_since_until()` in `superset/utils/date_parser.py:9–17`, where `_relative_start` is
derived from `relative_start` at line 43.
4. In `get_since_until()`, the new “Next” block at `superset/utils/date_parser.py:62–70`
builds `_granular_next_pattern` and, for `"Next 5 minutes"`, executes:
`if re.search(_granular_next_pattern, time_range, re.IGNORECASE): time_range = "now" +
separator + time_range`.
This prepends `"now"` as the start expression (lines 65–68), instead of using
`_relative_start` (line 70). Thus, for granular “Next” ranges, any caller-specified
`relative_start` is ignored and the start anchor is forced to `"now"`, diverging from
the documented contract that `relative_start` controls the base for relative time
deltas.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** superset/utils/date_parser.py
**Line:** 447:448
**Comment:**
*Api Mismatch: The new granular `Next` branch always prepends `now`, so an explicit `relative_start` from callers is ignored for seconds/minutes/hours. This changes behavior for clients that intentionally set `relative_start` and should only happen when no explicit start anchor is provided.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix3 participants
SUMMARY
Add four quick-select presets to the Last… (Common) frame of the date range filter — both in the Explore chart builder and in Dashboard native Time filters:
The new options appear at the top of the radio group, grouped above the existing day/week/month presets, giving users a fast path for recent/real-time monitoring use cases.
Backend fix included:
get_since_until()had a latent bug wherehourwas absent from the last/next/this time-unit regex despite being documented as supported. This PR addshourto the pattern soLast 1 hourresolves correctly. Theget_relative_base()function already classifiedhouras a granular unit (→ 'now'base), so no additional changes were needed there.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before: Common frame shows Last day / Last week / Last month / Last quarter / Last year
After: Common frame shows Last 5 minutes / Last 15 minutes / Last 30 minutes / Last 1 hour / Last day / Last week / Last month / Last quarter / Last year
TESTING INSTRUCTIONS
Last dayUnit tests:
ADDITIONAL INFORMATION