Tables for export-, refresh- and forwarding configs to support pagination and automatic refresh.#195
Tables for export-, refresh- and forwarding configs to support pagination and automatic refresh.#195kaapstorm wants to merge 29 commits into
Conversation
export_config lived inline in test_list_view.py; relocate it to the shared exports fixtures module so other test modules (and the multi-project / run fixtures added next) can reuse it via import.
The last_run property was duplicated, byte-for-byte except the Run class named in the status check, across ExportConfigBase, RefreshConfig and ForwardingConfig. All three inherit ScheduleMixin, and QUEUED is shared via RunBaseModel.Status, so the method belongs on the mixin alongside has_queued_runs(). No behaviour change.
When a caller has prefetched a config's runs into _all_runs (via Prefetch(..., to_attr='_all_runs')), pick the latest non-queued run from that list in Python instead of issuing a per-config query. Falls back to the DB query when _all_runs is absent, so behaviour is unchanged until callers opt in.
The legacy exports/multi_project_exports template variables were kept for exports_home.html compatibility; that template now uses the merged config_table partial, so the variables and their queries are dead. Remove them.
| return rf.get(f'/?{params}') | ||
|
|
||
| def test_default_is_10(self): | ||
| assert get_config_page_size(self._req()) == 10 |
There was a problem hiding this comment.
what do you think of replace 10 with VALID_PAGE_SIZES[0] ?
There was a problem hiding this comment.
Yes, I don't think the test should be checking the value of the constant. It should be checking the behavior of the function. 3cfaab5
Mypy doesn't pick up the "last_run_at" attribute, which is added by `.annotate()` in the queries above.
kaapstorm
force-pushed
the
nh/config-tables
branch
from
7a23745 to
8719d9f
Compare
| hx-get="{% url 'refreshes:config_table' %}" | ||
| hx-trigger="every 60s" | ||
| hx-swap="outerHTML" | ||
| hx-vals="js:{page: '{{ configs.number }}', page_size: '{{ page_size }}', etag: document.getElementById('refreshes-config-table').dataset.etag}" |
There was a problem hiding this comment.
It seems slightly funny that we're getting the etag based on a value on the element that this attribute itself is defined on... I assume it's because if we used {{ etag }} it would always use the initial value, rather than being updated if/when the hx-swap takes place?
| {% for size in page_sizes %} | ||
| <a | ||
| href="?page=1&page_size={{ size }}" | ||
| class="btn btn-sm {% if page_size == size %}btn-secondary{% else %}btn-outline-secondary{% endif %} py-0" |
There was a problem hiding this comment.
I think toggling active could be slightly nicer here than swapping btn-secondary with btn-outline-secondary? Noticing that's what we do above with the logOpen button. The appearance would be a little bit different (hover effects don't apply if it's already active, I think).
| {% endfor %} | ||
| </table> | ||
|
|
||
| {# Pagination footer #} |
There was a problem hiding this comment.
Is it possible to turn this into a shared partial for at least the non-HTMX Paginator footers? They seem nearly identical between templates.
| if 'has_status_filter' not in request.GET: | ||
| return None |
There was a problem hiding this comment.
Is there a case where this would be true? It seems like we include the run_history.html partial that has a hard-coded has_status_filter with value="1" on all the views that use get_run_statuses_from_request.
| class="icon-link btn btn-outline-danger btn-sm" | ||
| href="{% url 'exports:delete_export_config' export.id %}" | ||
| > | ||
| <i class="fa-solid fa-xmark"></i> |
There was a problem hiding this comment.
I think we normally use fa-trash for delete actions, possible this (and other similar uses in this commit) are intentionally different though.
| </thead> | ||
| <tbody x-data="{ expandedLogs: {}, expandedViews: {} }"> | ||
| {% for refresh_run in runs %} | ||
| <table class="table w-100 my-2"> |
There was a problem hiding this comment.
nit: a bootstrap table probably doesn't need w-100
| </tbody> | ||
| </table> | ||
|
|
||
| {# Pagination footer #} |
There was a problem hiding this comment.
It seems like the htmx pagination could maybe be a shared partial as well, it looks like just the -table id changes between them (but it's entirely possible I'm missing some nuance here).
| assert response.status_code == 200 | ||
|
|
||
| @use(authed_client, export_config_db_fixture) | ||
| def test_no_details_suffix_in_heading(self): |
There was a problem hiding this comment.
I'm not sure I understand what this test is for as written, we're testing the heading is no longer what it used to be? Could be more useful to actually see what the heading is now.
3 participants
Summary
This pull request adds support for pagination and automatic refresh to the tables that list export configurations, refresh configurations and forwarding configurations. It also combines the tables for export configurations and multi-project export configurations.
AI Disclosure
Most of this code was generated using Claude (Opus 4.6). It was then reviewed and edited by me.
BEFORE
AFTER
BEFORE
AFTER