Skip to content

feat(api-web): add "All" pagination option and per-table search filter #3493

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.

Sign up for GitHub

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

Open
kdhulipala-wq wants to merge 2 commits into
base: main
Choose a base branch
from
Open

feat(api-web): add "All" pagination option and per-table search filter #3493

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2460d17
feat(api-web): add "All" pagination option and per-table search filter
kdhulipala-wq Jul 14, 2026
bd97d38
fix(api-web): skip empty-ID page lookups and revert pxe bump
kdhulipala-wq Jul 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 26 additions & 19 deletions crates/api-web/src/compute_allocation.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,10 +134,13 @@ pub async fn show(
}
};

let tmpl = ComputeAllocationShow {
allocations,
page: PageContext::from_page_count(current_page, limit, pages, path.path()),
// `limit == 0` is the "All" pagination option: everything on one page.
let page = if limit == 0 {
PageContext::all(allocations.len(), path.path())
} else {
PageContext::from_page_count(current_page, limit, pages, path.path())
};
let tmpl = ComputeAllocationShow { allocations, page };
(StatusCode::OK, Html(tmpl.render().unwrap())).into_response()
}

Expand All @@ -161,28 +164,32 @@ async fn fetch_compute_allocations(
.map(|response| response.into_inner())?
.ids;

let limit = if limit == 0 {
DEFAULT_PAGE_RECORD_LIMIT
} else {
limit
};

if all_ids.is_empty() {
return Ok((0, vec![]));
}

let pages = all_ids.len().div_ceil(limit);
let current_record_cnt_seen = current_page.saturating_mul(limit);
// `limit == 0` means "show all" on a single page.
let (pages, ids_for_page): (usize, Vec<_>) = if limit == 0 {
(1, all_ids)
} else {
let pages = all_ids.len().div_ceil(limit);
let current_record_cnt_seen = current_page.saturating_mul(limit);

if current_record_cnt_seen > all_ids.len() {
return Ok((pages, vec![]));
}
// `>=` (not `>`) so the first out-of-range page returns early instead of
// issuing a lookup with no IDs when the count is an exact multiple of limit.
if current_record_cnt_seen >= all_ids.len() {
return Ok((pages, vec![]));
}

let ids_for_page = all_ids
.into_iter()
.skip(current_record_cnt_seen)
.take(limit)
.collect();
(
pages,
all_ids
.into_iter()
.skip(current_record_cnt_seen)
.take(limit)
.collect(),
)
};
Comment on lines +171 to +192

@coderabbitai coderabbitai Bot Jul 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win

Avoid empty-ID RPCs for the first out-of-range page. When the record count is an exact multiple of limit, current_page == pages makes the offset equal to the ID count. The > check falls through and sends an unnecessary lookup with no IDs; use >=.

  • crates/api-web/src/compute_allocation.rs#L171-L190: change the offset guard to >=.
  • crates/api-web/src/instance_type.rs#L227-L250: change the offset guard to >=.
  • crates/api-web/src/network_security_group.rs#L178-L201: change the offset guard to >=.
📍 Affects 3 files
  • crates/api-web/src/compute_allocation.rs#L171-L190 (this comment)
  • crates/api-web/src/instance_type.rs#L227-L250
  • crates/api-web/src/network_security_group.rs#L178-L201
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/api-web/src/compute_allocation.rs` around lines 171 - 190, Update the
pagination offset guards to use >= instead of >, so an offset equal to the total
ID count returns an empty result without issuing an empty-ID lookup. Apply this
in the pagination logic of crates/api-web/src/compute_allocation.rs (lines
171-190), crates/api-web/src/instance_type.rs (lines 227-250), and
crates/api-web/src/network_security_group.rs (lines 178-201), preserving the
existing behavior for genuinely in-range pages.


let allocations = api
.find_compute_allocations_by_ids(tonic::Request::new(
Expand Down
51 changes: 30 additions & 21 deletions crates/api-web/src/instance_type.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,9 +191,15 @@ pub async fn show(
}
};

// `limit == 0` is the "All" pagination option: everything on one page.
let page = if limit == 0 {
PageContext::all(instance_types.len(), path.path())
} else {
PageContext::from_page_count(current_page, limit, pages, path.path())
};
let tmpl = InstanceTypeShow {
instance_types,
page: PageContext::from_page_count(current_page, limit, pages, path.path()),
page,
};
(StatusCode::OK, Html(tmpl.render().unwrap())).into_response()
}
Expand All @@ -214,33 +220,36 @@ async fn fetch_instance_types(
.map(|response| response.into_inner())?
.instance_type_ids;

// Handling the case of getting a nonsensical limit.
let limit = if limit == 0 {
DEFAULT_PAGE_RECORD_LIMIT
} else {
limit
};

if all_ids.is_empty() {
return Ok((0, vec![]));
}

let pages = all_ids.len().div_ceil(limit);
// `limit == 0` means "show all" on a single page.
let (pages, ids_for_page): (usize, Vec<_>) = if limit == 0 {
(1, all_ids)
} else {
let pages = all_ids.len().div_ceil(limit);

let current_record_cnt_seen = current_page.saturating_mul(limit);
let current_record_cnt_seen = current_page.saturating_mul(limit);

// Just handles the other case of someone messing around with the
// query params and suddenly setting a limit that makes
// current_record_cnt_seen no longer make sense.
if current_record_cnt_seen > all_ids.len() {
return Ok((pages, vec![]));
}
// Just handles the other case of someone messing around with the
// query params and suddenly setting a limit that makes
// current_record_cnt_seen no longer make sense. `>=` (not `>`) so the
// first out-of-range page returns early instead of issuing a lookup with
// no IDs when the count is an exact multiple of limit.
if current_record_cnt_seen >= all_ids.len() {
return Ok((pages, vec![]));
}

let ids_for_page = all_ids
.into_iter()
.skip(current_record_cnt_seen)
.take(limit)
.collect();
(
pages,
all_ids
.into_iter()
.skip(current_record_cnt_seen)
.take(limit)
.collect(),
)
};

let itypes = api
.find_instance_types_by_ids(tonic::Request::new(
Expand Down
7 changes: 7 additions & 0 deletions crates/api-web/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,7 @@ const SORTABLE_JS: &str = include_str!("../templates/static/sortable.min.js");
const SORTABLE_CSS: &str = include_str!("../templates/static/sortable.min.css");
const CARBIDE_CSS: &str = include_str!("../templates/static/carbide.css");
const TABS_JS: &str = include_str!("../templates/static/tabs.js");
const TABLE_FILTER_JS: &str = include_str!("../templates/static/table_filter.js");

// It would appear the oauth2 author read about the typestate pattern and decided making
// everyone declare 10 type parameters when storing a Client sounds like a great idea.
Expand Down Expand Up @@ -1049,6 +1050,12 @@ pub async fn static_data(
(StatusCode::OK, [(CONTENT_TYPE, "text/css")], CARBIDE_CSS).into_response()
}
"tabs.js" => (StatusCode::OK, [(CONTENT_TYPE, "text/javascript")], TABS_JS).into_response(),
"table_filter.js" => (
StatusCode::OK,
[(CONTENT_TYPE, "text/javascript")],
TABLE_FILTER_JS,
)
.into_response(),
_ => (StatusCode::NOT_FOUND, "No such file").into_response(),
}
}
Expand Down
51 changes: 30 additions & 21 deletions crates/api-web/src/network_security_group.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,9 +139,15 @@ pub async fn show(
}
};

// `limit == 0` is the "All" pagination option: everything on one page.
let page = if limit == 0 {
PageContext::all(network_security_groups.len(), path.path())
} else {
PageContext::from_page_count(current_page, limit, pages, path.path())
};
let tmpl = NetworkSecurityGroupShow {
network_security_groups,
page: PageContext::from_page_count(current_page, limit, pages, path.path()),
page,
};
(StatusCode::OK, Html(tmpl.render().unwrap())).into_response()
}
Expand All @@ -165,33 +171,36 @@ async fn fetch_network_security_groups(
.map(|response| response.into_inner())?
.network_security_group_ids;

// Handling the case of getting a nonsensical limit.
let limit = if limit == 0 {
DEFAULT_PAGE_RECORD_LIMIT
} else {
limit
};

if all_ids.is_empty() {
return Ok((0, vec![]));
}

let pages = all_ids.len().div_ceil(limit);
// `limit == 0` means "show all" on a single page.
let (pages, ids_for_page): (usize, Vec<_>) = if limit == 0 {
(1, all_ids)
} else {
let pages = all_ids.len().div_ceil(limit);

let current_record_cnt_seen = current_page.saturating_mul(limit);
let current_record_cnt_seen = current_page.saturating_mul(limit);

// Just handles the other case of someone messing around with the
// query params and suddenly setting a limit that makes
// current_record_cnt_seen no longer make sense.
if current_record_cnt_seen > all_ids.len() {
return Ok((pages, vec![]));
}
// Just handles the other case of someone messing around with the
// query params and suddenly setting a limit that makes
// current_record_cnt_seen no longer make sense. `>=` (not `>`) so the
// first out-of-range page returns early instead of issuing a lookup with
// no IDs when the count is an exact multiple of limit.
if current_record_cnt_seen >= all_ids.len() {
return Ok((pages, vec![]));
}

let ids_for_page = all_ids
.into_iter()
.skip(current_record_cnt_seen)
.take(limit)
.collect();
(
pages,
all_ids
.into_iter()
.skip(current_record_cnt_seen)
.take(limit)
.collect(),
)
};

let nsgs = api
.find_network_security_groups_by_ids(tonic::Request::new(
Expand Down
15 changes: 15 additions & 0 deletions crates/api-web/src/pagination.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,21 @@ impl PageContext {
}
}

/// Create a PageContext representing the "show all" view: a single page
/// holding every item. Used by handlers that opt into the `limit=0` "All"
/// pagination option and already know the total item count.
pub fn all(total_items: usize, path: impl Into<String>) -> Self {
Self {
info: PaginationInfo {
current_page: 0,
limit: 0,
total_items,
},
path: path.into(),
extra_query_params: String::new(),
}
}

pub fn with_extra_params(mut self, extra: String) -> Self {
self.extra_query_params = extra;
self
Expand Down
1 change: 1 addition & 0 deletions crates/api-web/templates/base.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ <h3>Tools</h3>
</script>
<script src="/admin/static/sortable.js"></script>
<script src="/admin/static/tabs.js"></script>
<script src="/admin/static/table_filter.js"></script>
<script>
const toastContainer = document.getElementById('toast-container');
const errorWrapper = document.getElementById('error-modal-wrapper');
Expand Down
1 change: 1 addition & 0 deletions crates/api-web/templates/pages_footer.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
{% if page.limit() == 25 %}<span class="page-link active">25</span>{% else %}<a class="page-link" href="{{ page.path }}?current_page=0&amp;limit=25{{page.extra_query_params}}">25</a>{% endif %}
{% if page.limit() == 50 %}<span class="page-link active">50</span>{% else %}<a class="page-link" href="{{ page.path }}?current_page=0&amp;limit=50{{page.extra_query_params}}">50</a>{% endif %}
{% if page.limit() == 100 %}<span class="page-link active">100</span>{% else %}<a class="page-link" href="{{ page.path }}?current_page=0&amp;limit=100{{page.extra_query_params}}">100</a>{% endif %}
{% if page.limit() == 0 %}<span class="page-link active" title="Show all items on one page">All</span>{% else %}<a class="page-link" href="{{ page.path }}?current_page=0&amp;limit=0{{page.extra_query_params}}" title="Show all items on one page">All</a>{% endif %}
{% endif %}
</div>
</div>
Expand Down
45 changes: 45 additions & 0 deletions crates/api-web/templates/static/carbide.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1941,6 +1941,51 @@ span.page-link.disabled {
cursor: default;
}

/* ------------------------------------------------------------------------
Per-table client-side filter (search box injected by table_filter.js)
------------------------------------------------------------------------ */
.table-filter {
display: flex;
align-items: center;
gap: 0.75rem;
margin: 0 0 0.75rem;
}

.table-filter-input {
flex: 0 1 22rem;
background-color: var(--bg-tertiary);
border: 1px solid var(--border-color);
color: var(--text-primary);
padding: 0.5rem 1rem;
border-radius: 8px;
font-size: 0.9rem;
transition: all 0.2s ease;
box-shadow: var(--shadow-sm);
height: 40px;
box-sizing: border-box;
}

.table-filter-input:focus {
outline: none;
border-color: var(--accent-green);
background-color: var(--bg-primary);
box-shadow: 0 0 0 3px rgba(118, 185, 0, 0.1);
}

.table-filter-input::placeholder {
color: var(--text-muted);
}

.table-filter-count {
font-size: 0.85rem;
color: var(--text-muted);
}

.table-filter-count.table-filter-empty {
color: var(--accent-red, #e5484d);
font-weight: 600;
}

/* ------------------------------------------------------------------------
CONFIGURATION PAGE (/admin)

Expand Down
Loading
Loading