diff --git a/crates/api-web/src/compute_allocation.rs b/crates/api-web/src/compute_allocation.rs index 1dfd7343fb..38b20c6d34 100644 --- a/crates/api-web/src/compute_allocation.rs +++ b/crates/api-web/src/compute_allocation.rs @@ -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() } @@ -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(), + ) + }; let allocations = api .find_compute_allocations_by_ids(tonic::Request::new( diff --git a/crates/api-web/src/instance_type.rs b/crates/api-web/src/instance_type.rs index aeafde518e..35b825eae8 100644 --- a/crates/api-web/src/instance_type.rs +++ b/crates/api-web/src/instance_type.rs @@ -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() } @@ -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( diff --git a/crates/api-web/src/lib.rs b/crates/api-web/src/lib.rs index a902f69b6c..5087985f03 100644 --- a/crates/api-web/src/lib.rs +++ b/crates/api-web/src/lib.rs @@ -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. @@ -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(), } } diff --git a/crates/api-web/src/network_security_group.rs b/crates/api-web/src/network_security_group.rs index 37e93127bd..96e11b3cf9 100644 --- a/crates/api-web/src/network_security_group.rs +++ b/crates/api-web/src/network_security_group.rs @@ -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() } @@ -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( diff --git a/crates/api-web/src/pagination.rs b/crates/api-web/src/pagination.rs index 2af02b2b4b..86afe1560c 100644 --- a/crates/api-web/src/pagination.rs +++ b/crates/api-web/src/pagination.rs @@ -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) -> 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 diff --git a/crates/api-web/templates/base.html b/crates/api-web/templates/base.html index 541e714d56..96d2a1cf2b 100644 --- a/crates/api-web/templates/base.html +++ b/crates/api-web/templates/base.html @@ -159,6 +159,7 @@

Tools

+