Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create incident list update interval preference #1174

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

elfjes
Copy link
Collaborator

@elfjes elfjes commented Feb 14, 2025

This PR creates a preference argus_htmx.update_interval. The preference can be managed through the settings:

ARGUS_INCIDENTS_UPDATE_INTERVALS
ARGUS_INCIDENTS_UPDATE_INTERVAL_DEFAULT

The default values are ["never", 5, 30, 60] with values in seconds. The default default is 30. "never" is a special value that disables auto refresh

In the UI, a user can change their preference at the bottom of the incident list table in the refresh info block. There already was a dropdown for the page size. The behaviour of the update interval dropdown is however, different. Where page_size adds the page_size as a query param and does a partial page reload. Due to the internals of how htmx handles "every xs" triggers, it is not feasible to do a partial reload when changing the update_interval. See also bigskysoftware/htmx#3170. We therefore need to do a full page refresh when changing the update interval. This also means that we cannot send the update_interval as a query param like we do with page_size. When triggering an HX-Refresh, the client sends refreshes the current page (with current query params). The update_interval is not yet part of the query params, so now it won't pick up the change... I therefore opted to make it a non-param setting, like with date_format.

Other things:

  • I have removed the load trigger from filterbox. On page load, the incidents should already be correctly filtered, so this resulted in an additional, unnecessary request.
  • updated the hx-include for refreshing the incident list to .incident-list-param so that we have a single way to include certain parameters in the refresh. Just add that class to any form or input that needs to be included

@codecov-commenter
Copy link

codecov-commenter commented Feb 14, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.52%. Comparing base (f6ae0b5) to head (51b524d).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1174      +/-   ##
==========================================
+ Coverage   78.48%   78.52%   +0.03%     
==========================================
  Files         141      141              
  Lines        5442     5452      +10     
==========================================
+ Hits         4271     4281      +10     
  Misses       1171     1171              

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

@hmpf
Copy link
Contributor

hmpf commented Feb 19, 2025

The update_interval is not yet part of the query params, so now it won't pick up the change... I therefore opted to make it a non-param setting, like with date_format.

This makes sense, being able to bookmark an update-interval is a lot less useful then being able to bookmark a filter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants