[SIP-133] Update "Time Range" filter in dashboard about superset HOT 9 CLOSED

I agree that this is inconsistent and that we should move the current "last day" to "previous day" when adding the new option. If a migration could do that automatically for existing charts, that would be fantastic.

Regarding naming, I think it's okay that "last 24 hours" and "previous day" don't have the same terminology because the definitions are different, if I understand. "Last 24 hours" is just like what it sounds like, "previous day" is the the most recent completed calendar day from midnight to midnight. Is that right?

I agree this should not be controversial or have enough different options to require the conversation of a SIP.

from superset.

@rusackas Thank you for your comment.

Yes, we can get from "Custom", but it's not a single click. We have to select "custom" and need tp select "Now" for END and modify days to 1 in START.

We can add new option like "Last 24hrs" where time range will be last 24 hours. It won't require more changes I guess.

For my usecase I added new option "Last 24hrs" and in time_range fetch api I just sending the time_range for last 24hrs in place of query params.

http://localhost:8088/api/v1/time_range/

query params:
q: '2024-05-14T13:14:18 : 2024-05-15T13:14:18'


response:
{
   "result": [
       {
           "since": "2024-05-14T13:14:18",
           "until": "2024-05-15T13:14:18",
           "timeRange": "2024-05-14T13:14:18 : 2024-05-15T13:14:18"
       }
   ]
}

from superset.

This doesn't seem like a SIP... and if it is, you'd need to fill out the rest of the template for it to be considered.

That said, I kind of agree here, and feel this could be interpreted as a bug. Superset offers both "last" and "previous" options, and we have a bit of an inconsistency here.

In "last" all the choices are essentially "last {granularity} until now" where "previous" is effectively "previous complete {granularity}. For example, "last month" is 2024-04-14 to 2024-05-14, whereas "previous month" is 2024-04-01 to 2024-05-01. To me, these seem sensible.

When it comes to the day option(s) however, we're a bit inconsistent. The "last day" option is really what should be called "previous calendar day" under the "previous" menu. For "last day" it seems like it should be "24-hours-before now"
to "now".

As a workaround, and potential inroads to a PR, you can currently do what you're seeking under the "Custom" tab, like so:

But I think I agree with you that this should be what "last day" actually does, and the current "last day" range should be moved over to a (new) "previous calendar day" option under "Previous".

Curious what others (@kasiazjc @villebro @michael-s-molina ) think about this. It might not be a hard feature to add, but could be a tricky migration for existing filters/charts/etc.

from superset.

I think adding that option would be a fantastic PR, and not be considered a "breaking change" or need a migration. Once we have "Last 24 Hours" in place, I think it would make sense to move the current "Last Day" implementation over to the "Previous" tab as "Previous calendar day" and assess what migrations (if any) need to be done for existing charts using that option.

from superset.

I'm not sure this requires a SIP... I think you can just go straight to a PR with the above idea, but I'll ping @michael-s-molina @kasiazjc @eschutho for a few more opinions on the matter. If you do want to carry this forward as a SIP, you should open a "Discuss" thread on the dev@ mailing list... say the word if you need help pursuing that.

from superset.

@rusackas
I think it would be better if you include "This Month", "This Year", "This Week", "Today".

Today- Should consider today morning 00:00 to present time.
This Week- This week starting to present day and time.
This Month- This month 1st to present day and time.
Similarly for "This Year"

from superset.

I think the "Current" option being proposed in the PR linked right above this comment seems to scratch that itch.

from superset.

I think the "Current" PR merged... can we call this one closed, or is there more that needs to be discussed/implemented?

from superset.

Closing for now since it hasn't been brought up as a [DISCUSS] thread on the mailing list, and the "Current" range PR has already merged, which I believe scratches this itch. Correct me if I'm wrong and this needs to be reopened.

from superset.