feat: custom refresh frequency

[Abhishek-kumar-samsung]

Earlier at dashboard level we had very limited options in 'Set auto-refresh interval' as shown in image below

But as per our organisation level we wanted the frequency to be something else that was not in dropdown, so we implemented that.

As part of this PR, we can add custom frequency as required by user, user can select any frequency in hour, minute and seconds and set it as auto refresh interval frequency (in seconds).

[Abhishek-kumar-samsung]

@rusackas @justinpark @mistercrunch @villebro @geido @eschutho @betodealmeida @nytai @craig-rueda @kgabryje @dpgaspar

Please review.

[john-bodley]

@kasiazjc would you mind looking at the design? Personally I think that custom (and how it is defined/configured) should be a secondary option (hidden by default)—possibly under a collapsed menu, button, tab, etc.

[craig-rueda]

From the screenshots, it looks a little confusing to show the dropdown alongside the text fields. Perhaps a new "virtual" option (CUSTOM) should be added, which would then hide/show the text boxes when selected? Also, having two buttons when choosing a custom refresh interval looks a little confusing as well.

[codecov]

Codecov Report

Attention: Patch coverage is 40.00000% with 24 lines in your changes are missing coverage. Please review.

Project coverage is 67.41%. Comparing base (9ced255) to head (4bd1582).
Report is 54 commits behind head on master.

Files	Patch %	Lines
.../src/dashboard/components/RefreshIntervalModal.tsx	40.00%	23 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #24449      +/-   ##
==========================================
+ Coverage   67.34%   67.41%   +0.07%     
==========================================
  Files        1909     1910       +1     
  Lines       74623    74741     +118     
  Branches     8324     8356      +32     
==========================================
+ Hits        50256    50388     +132     
+ Misses      22314    22300      -14     
  Partials     2053     2053              

Flag	Coverage Δ
javascript	57.37% <40.00%> (+0.16%)	⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Abhishek-kumar-samsung]

Okay sure @john-bodley @craig-rueda
I will try to hide custom view and show custom view when required.

[Abhishek-kumar-samsung]

I have made changes as asked
When user will click CUSTOM button then box changes so that one can provide custom frequency, and if user clicks CUSTOM button again then original window will appear again.

[Abhishek-kumar-samsung]

@john-bodley @craig-rueda Please check and review.

I have shown only one refresh change option at a time as requested by @john-bodley and removed extra submit button as requested by @craig-rueda

[rusackas]

Sorry for the churn and delays on this, but I think the main issue is UI design, not the technical implementation.

My own opinion:
• It would be nice if it had the Select menu (including the Custom option, rather than a button for Custom)
• If (and only if) the Custom option is selected from the Select menu, would the additional fields appear (or disappear)
• The Select menu would remain present... if you select something other than Custom, the form elements are removed, and the values ignored.

As a side note, I feel like the Submit button should be on the right, and the cancel button on the left. I'm not sure if we have a solid documented pattern around this, though (cc. @kasiazjc)

[Shisir99]

@Abhishek-kumar-samsung is this frequency only for a session or persistent?

[Abhishek-kumar-samsung]

Sorry for the churn and delays on this, but I think the main issue is UI design, not the technical implementation.

My own opinion: • It would be nice if it had the Select menu (including the Custom option, rather than a button for Custom) • If (and only if) the Custom option is selected from the Select menu, would the additional fields appear (or disappear) • The Select menu would remain present... if you select something other than Custom, the form elements are removed, and the values ignored.

As a side note, I feel like the Submit button should be on the right, and the cancel button on the left. I'm not sure if we have a solid documented pattern around this, though (cc. @kasiazjc)

Okay i will do some modification with UI.

[Abhishek-kumar-samsung]

@Abhishek-kumar-samsung is this frequency only for a session or persistent?

It is persistent.

[Shisir99]

@Abhishek-kumar-samsung my dashboard frequency is saving only for a particular session , how can i make it persistent?

[kasiazjc]

Sorry for the churn and delays on this, but I think the main issue is UI design, not the technical implementation.
My own opinion: • It would be nice if it had the Select menu (including the Custom option, rather than a button for Custom) • If (and only if) the Custom option is selected from the Select menu, would the additional fields appear (or disappear) • The Select menu would remain present... if you select something other than Custom, the form elements are removed, and the values ignored.
As a side note, I feel like the Submit button should be on the right, and the cancel button on the left. I'm not sure if we have a solid documented pattern around this, though (cc. @kasiazjc)

Okay i will do some modification with UI.

Thanks and sorry for the long wait! What I'm thinking in terms of UI improvements is this:

-> instead of button/switcher use single select, for selecting the type of interval, the options are:

• don’t refresh

• custom interval (triggers the additional fields as shown below)

• all of the preset options that are available today (in the example dropdown I listed only two, but should be all of them)

-> set fixed height for the modal to accommodate for disappearing custom fields, so that the height does not jump

I think this way setting up the custom interval will be more intuitive, as we are using common patterns. What do you think and could you possibly take this up @Abhishek-kumar-samsung ?

[Abhishek-kumar-samsung]

@kasiazjc

Yes i will take this, i will try to make as you have suggested.
I was busy in last few days, so i will work on this from now.

[kasiazjc]

@kasiazjc

Yes i will take this, i will try to make as you have suggested. I was busy in last few days, so i will work on this from now.

Great, thank you! If you need some more design specs let me know. I was using default font sizes and components that we use in Superset :)

[Abhishek-kumar-samsung]

@Abhishek-kumar-samsung my dashboard frequency is saving only for a particular session , how can i make it persistent?

@Shisir99 i am sorey, i was not sure that time, yes it is not persistent for me too, that is for a particular session only.

@rusackas isn't that a problem?
Automatic refresh interval for just a single session does not makes much sense(according to me, or maybe i missed something), it should be persistent.

[coetzeevs]

Hi all - please direct me to an issue link if there's a separate place for questions. We recently adopted Superset and I'm looking into setting up refresh schedules for our dashboards. This PR comes close to introducing the feature I think we need, but really something with the flexibility like cron schedule expressions would be ideal. I haven't been able to understand when the refresh interval triggers (i.e. how it's determined when it'll happen) or to specify a time it should kick off.

Effectively I just want to specify when my refresh interval should start and (based off the time window I choose) when it should stop. Can any help me understand this, please?

[Abhishek-kumar-samsung]

Hi all - please direct me to an issue link if there's a separate place for questions. We recently adopted Superset and I'm looking into setting up refresh schedules for our dashboards. This PR comes close to introducing the feature I think we need, but really something with the flexibility like cron schedule expressions would be ideal. I haven't been able to understand when the refresh interval triggers (i.e. how it's determined when it'll happen) or to specify a time it should kick off.

Effectively I just want to specify when my refresh interval should start and (based off the time window I choose) when it should stop. Can any help me understand this, please?

Currently in this PR, i was implementing refresh intervals based on relative timing. But crons idea would be nice.

If we need to include crons then this PR will extend to some long time, maybe endlessly.

So maybe we can start an SIP with multiple stages of PR,
First stage maybe this one i.e relative time,
and Second stage can be crons.

What say? @rusackas

[rusackas]

I agree we don't want to grow the scope of this PR. We should merge this first, and then deal with the cron idea (which is a nice idea) as a subsequent PR.

For the cron feature, it seems like a reasonable plan to seek "lazy consensus" on the dev@ list. Then if that starts an argument, we can upgrade the conversation to a SIP and vote it.

[Abhishek-kumar-samsung]

I agree we don't want to grow the scope of this PR. We should merge this first, and then deal with the cron idea (which is a nice idea) as a subsequent PR.

For the cron feature, it seems like a reasonable plan to seek "lazy consensus" on the dev@ list. Then if that starts an argument, we can upgrade the conversation to a SIP and vote it.

Yes sure, i will quickly do the two changes that you had suggested and once merged, we will raise SIP for cron.

Then we can work on it, @coetzeevs you can also work on that if you had implemented something in it.

[Abhishek-kumar-samsung]

@geido @rusackas i resolved whatever issues pointed out, can you pls review.

[rusackas]

/testenv up

[github-actions]

@rusackas Ephemeral environment spinning up at http://54.245.47.10:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

[LeoDiep]

Hi all - please direct me to an issue link if there's a separate place for questions. We recently adopted Superset and I'm looking into setting up refresh schedules for our dashboards. This PR comes close to introducing the feature I think we need, but really something with the flexibility like cron schedule expressions would be ideal. I haven't been able to understand when the refresh interval triggers (i.e. how it's determined when it'll happen) or to specify a time it should kick off.

Effectively I just want to specify when my refresh interval should start and (based off the time window I choose) when it should stop. Can any help me understand this, please?

Hi there,
cron refresh would be an ideal solution to tell which dashboard should refresh all their pages at which time, based on the refresh schedule of underneath dataset.
Hi @rusackas I wonder is it ideal similar to the cache warm up feature, could you please take a look at this idea or fix the cache warm up feature, so that users will always be able to view the lastest update data without clicking into 'Force refresh' on charts or 'Refresh dashboard' on top.
Thank you

[rusackas]

After some quick sanity tests on the ephemeral, this is looking pretty good!

[eschutho]

LGTM! Thanks @Abhishek-kumar-samsung

[github-actions]

Ephemeral environment shutdown and build artifacts deleted.

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 40.00000% with 24 lines in your changes are missing coverage. Please review.

Project coverage is 67.41%. Comparing base (9ced255) to head (4bd1582).
Report is 226 commits behind head on master.

Files	Patch %	Lines
.../src/dashboard/components/RefreshIntervalModal.tsx	40.00%	23 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #24449      +/-   ##
==========================================
+ Coverage   67.34%   67.41%   +0.07%     
==========================================
  Files        1909     1910       +1     
  Lines       74623    74741     +118     
  Branches     8324     8356      +32     
==========================================
+ Hits        50256    50388     +132     
+ Misses      22314    22300      -14     
  Partials     2053     2053              

Flag	Coverage Δ
javascript	57.37% <40.00%> (+0.16%)	⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Abhishek-kumar-samsung]

I accidently closed the PR and I reopened it again.

[rusackas]

@geido can you check your requested changes to (potentially) unblock the merge when you have a chance, please?

[geido]

@Abhishek-kumar-samsung it seems that CI is stuck. Please pull latest master and push back to kick CI on a fresh master state. Thank you

[Abhishek-kumar-samsung]

@Abhishek-kumar-samsung it seems that CI is stuck. Please pull latest master and push back to kick CI on a fresh master state. Thank you

Hi @geido i synced my fork to latest main branch, i don't know why the pull_request check is failing.
From code side it did not gave any check failure.

Last time when it ran, the same code passed all checks.

[rusackas]

Thanks @Abhishek-kumar-samsung - I/we appreciate you sticking with this and getting it through!

[Abhishek-kumar-samsung]

Thanks @Abhishek-kumar-samsung - I/we appreciate you sticking with this and getting it through!

Thankyou @rusackas