sadpandajoe commented on code in PR #45039:
URL: https://github.com/apache/superset/pull/45039#discussion_r4217271760
##########
superset-frontend/src/dashboard/components/RefreshFrequency/RefreshFrequencySelect.tsx:
##########
@@ -120,11 +220,13 @@ export const RefreshFrequencySelect = ({
if (selectedValue === -1) {
// Custom selected - use current custom value or minimum
const numValue = parseInt(customValue, 10) || MINIMUM_REFRESH_INTERVAL;
Review Comment:
If someone types an invalid draft such as `-5` into Custom, picks a preset,
and then picks Custom again, this line re-emits `-5`: `parseInt('-5', 10)` is
truthy so the `|| MINIMUM_REFRESH_INTERVAL` fallback never applies. The typing
handler rejects values below the minimum, but this path doesn't, and
`validateRefreshFrequency` only checks positive frequencies, so Save accepts a
negative interval and auto-refresh quietly stops while Custom stays selected.
Before this change the draft was cleared when a preset was chosen, so the path
wasn't reachable.
Could this clamp the value (e.g. only reuse the draft when it parses to at
least `MINIMUM_REFRESH_INTERVAL`), and could a controlled test cover the type
`-5` → preset → Custom sequence?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
