feat: Limit dashboard search results (#5284)

This PR introduces the optional `page_size` query parameter to the
`ListDashboards` endpoint. When set this will limit the size of the
result set to the specified value. The client/frontend will be able to
set this query parameter in calls to `ListDashboards` to limit the
number of results returned - specifically in requests to search
dashboards across folders.

This PR only implements limiting the result set size and does not
actually expose pagination as functionality of the `ListDashboards`
endpoint. My hope however is that this implementation will put us in a
good place to easily expose pagination on this endpoint if that's
something we want to do in the future - though it would by no means be
necessary.

There are a couple of idiosyncrasies here.

1. The `ListDashboards` endpoint does not read or use any `page_idx` or
`page_number` query parameter, as one might expect from a paginated
endpoint. Therefore if the client sets the `page_size` parameter to some
number N then the `ListDashboards` endpoint will always return the top N
results. (Considered in a different way, the endpoint always acts as if
the requested page index is `0`.)
- In the future we might want to add an additional `page_idx` query
parameter as well to enable "true" pagination (rather than simply
limiting the result set to the first N results). However there are no
immediate plans for this.
2. The `page_size` query parameter is only read/used by the endpoint
when the `title` parameter is **also** set. This means that when the
`title` parameter is **not** set the endpoint will ignore the
`page_size` parameter and will always return all dashboards that match
the other query parameter, `folder`.
- In the future we could choose to use this parameter any time it is
provided, not just when `title` is also provided. To do that there would
need to first be an update to the UI to address the issue described
below.
- For the time being this is necessary in order to not break the UI.
This is because currently the UI always sends the query parameter
`page_size=1000` to the endpoint, even though before now that query
parameter was never recognized by the endpoint. Additionally, the UI
assumes that the calls it makes to `ListDashboards` in order to show the
dashboards in a selected folder always return **all** dashboards, and in
such requests the frontend sets the `folder` query parameter but not the
`title` query parameter. Therefore, if we actually to use the
`page_size` parameter in **those** requests, then in its current
implementation the UI would fail to show all dashboards when a folder
with over 1000 dashboards is selected.
This commit is contained in:
Patrick Sullivan
2024-12-06 10:07:23 -06:00
committed by GitHub
parent 1dfb8ce38b
commit c81610bc9f
3 changed files with 71 additions and 17 deletions
+10
View File
@@ -201,6 +201,9 @@ pub struct ListDashboardsParams {
/// The optional case-insensitive title substring with which to filter
/// dashboards.
pub title_pat: Option<String>,
/// The optional page size and page index of results to retrieve.
pub page_size_and_idx: Option<(u64, u64)>,
}
impl ListDashboardsParams {
@@ -211,6 +214,7 @@ impl ListDashboardsParams {
org_id: org_id.to_string(),
folder_id: None,
title_pat: None,
page_size_and_idx: None,
}
}
@@ -228,4 +232,10 @@ impl ListDashboardsParams {
self.title_pat = Some(title_pat.to_string());
self
}
/// Paginate the results by the given page size and page index.
pub fn paginate(mut self, page_size: u64, page_idx: u64) -> Self {
self.page_size_and_idx = Some((page_size, page_idx));
self
}
}
+31 -5
View File
@@ -30,6 +30,7 @@ pub struct UpdateDashboardResponseBody(DashboardDetails);
/// HTTP URL query component that contains parameters for listing dashboards.
#[derive(Debug, Deserialize, utoipa::IntoParams)]
#[into_params(style = Form, parameter_in = Query)]
#[serde(rename_all = "camelCase")]
pub struct ListDashboardsQuery {
/// Optional folder ID filter parameter
///
@@ -43,6 +44,13 @@ pub struct ListDashboardsQuery {
/// The optional case-insensitive title substring with which to filter
/// dashboards.
title: Option<String>,
/// The optional number of dashboards to retrieve. If not set then all
/// dashboards that match the query parameters will be returned.
///
/// Currently this parameter is only untilized by the API when the `title`
/// parameter is also set.
page_size: Option<u64>,
}
/// HTTP response body for `ListDashboards` endpoint.
@@ -137,33 +145,51 @@ impl From<MetaDashboard> for UpdateDashboardResponseBody {
impl ListDashboardsQuery {
pub fn into(self, org_id: &str) -> config::meta::dashboards::ListDashboardsParams {
match self {
let mut query = match &self {
Self {
folder: Some(f),
title: Some(t),
..
} => config::meta::dashboards::ListDashboardsParams::new(org_id)
.with_folder_id(&f)
.where_title_contains(&t),
.with_folder_id(f)
.where_title_contains(t),
Self {
folder: None,
title: Some(t),
..
} => {
config::meta::dashboards::ListDashboardsParams::new(org_id).where_title_contains(&t)
config::meta::dashboards::ListDashboardsParams::new(org_id).where_title_contains(t)
}
Self {
folder: Some(f),
title: None,
} => config::meta::dashboards::ListDashboardsParams::new(org_id).with_folder_id(&f),
..
} => config::meta::dashboards::ListDashboardsParams::new(org_id).with_folder_id(f),
Self {
folder: None,
title: None,
..
} => {
// To preserve backwards-compatability when no filter parameters
// are given we will list the contents of the default folder.
config::meta::dashboards::ListDashboardsParams::new(org_id)
.with_folder_id(config::meta::folder::DEFAULT_FOLDER)
}
};
// The API currently only supports using page_size to limit the output
// to the top results. And the page_size parameter is only used when the
// title parameter is provided to search dashboards by title pattern.
// When the title parameter is not set we simply want to return all
// dashboards that match the selected folder so we ignore the page_size
// parameter.
if self.title.is_some_and(|t| !t.is_empty()) {
if let Some(page_size) = self.page_size {
query = query.paginate(page_size, 0)
}
}
query
}
}
+30 -12
View File
@@ -23,7 +23,8 @@ use config::meta::{
};
use sea_orm::{
prelude::Expr, sea_query::Func, ActiveModelTrait, ActiveValue::NotSet, ColumnTrait,
DatabaseConnection, EntityTrait, ModelTrait, QueryFilter, QueryOrder, Set, TryIntoModel,
DatabaseConnection, EntityTrait, ModelTrait, PaginatorTrait, QueryFilter, QueryOrder, Set,
TryIntoModel,
};
use serde_json::Value as JsonValue;
@@ -277,7 +278,8 @@ async fn get_model(
Ok(Some((folder, maybe_dashboard)))
}
/// Lists all dashboard ORM models that belong to the given org and folder.
/// Lists dashboard ORM models using the given parameters. Returns each
/// dashboard and its parent folder.
async fn list_models(
db: &DatabaseConnection,
params: ListDashboardsParams,
@@ -310,13 +312,20 @@ async fn list_models(
.order_by_asc(dashboards::Column::Title)
.order_by_asc(folders::Column::Name);
let dashboards_and_folders = query
.all(db)
.await?
// Execute the query, either getting all results or a specific page of results.
let results = if let Some((page_size, page_idx)) = params.page_size_and_idx {
query.paginate(db, page_size).fetch_page(page_idx).await?
} else {
query.all(db).await?
};
// Flatten the results so that each dashboard is returned alongside its
// parent folder.
let folders_and_dashboards = results
.into_iter()
.filter_map(|(d, maybe_f)| maybe_f.map(|f| (f, d)))
.collect();
Ok(dashboards_and_folders)
Ok(folders_and_dashboards)
}
/// Lists all dashboard ORM models that belong to the given org and folder.
@@ -410,18 +419,21 @@ mod tests {
org_id: "orgId".to_owned(),
folder_id: Some("folderId".to_owned()),
title_pat: Some("tItLePat".to_owned()),
page_size_and_idx: Some((100, 2)),
};
list_models(&db, params).await?;
assert_eq!(
db.into_transaction_log(),
vec![Transaction::from_sql_and_values(
DatabaseBackend::Postgres,
r#"SELECT "dashboards"."id" AS "A_id", "dashboards"."dashboard_id" AS "A_dashboard_id", "dashboards"."folder_id" AS "A_folder_id", "dashboards"."owner" AS "A_owner", "dashboards"."role" AS "A_role", "dashboards"."title" AS "A_title", "dashboards"."description" AS "A_description", "dashboards"."data" AS "A_data", "dashboards"."version" AS "A_version", "dashboards"."created_at" AS "A_created_at", "folders"."id" AS "B_id", "folders"."org" AS "B_org", "folders"."folder_id" AS "B_folder_id", "folders"."name" AS "B_name", "folders"."description" AS "B_description", "folders"."type" AS "B_type" FROM "dashboards" LEFT JOIN "folders" ON "dashboards"."folder_id" = "folders"."id" WHERE "folders"."org" = $1 AND "folders"."type" = $2 AND "folders"."folder_id" = $3 AND LOWER("title") LIKE $4 ORDER BY "dashboards"."title" ASC, "folders"."name" ASC"#,
r#"SELECT "dashboards"."id" AS "A_id", "dashboards"."dashboard_id" AS "A_dashboard_id", "dashboards"."folder_id" AS "A_folder_id", "dashboards"."owner" AS "A_owner", "dashboards"."role" AS "A_role", "dashboards"."title" AS "A_title", "dashboards"."description" AS "A_description", "dashboards"."data" AS "A_data", "dashboards"."version" AS "A_version", "dashboards"."created_at" AS "A_created_at", "folders"."id" AS "B_id", "folders"."org" AS "B_org", "folders"."folder_id" AS "B_folder_id", "folders"."name" AS "B_name", "folders"."description" AS "B_description", "folders"."type" AS "B_type" FROM "dashboards" LEFT JOIN "folders" ON "dashboards"."folder_id" = "folders"."id" WHERE "folders"."org" = $1 AND "folders"."type" = $2 AND "folders"."folder_id" = $3 AND LOWER("title") LIKE $4 ORDER BY "dashboards"."title" ASC, "folders"."name" ASC LIMIT $5 OFFSET $6"#,
[
"orgId".into(),
0i16.into(),
"folderId".into(),
"%titlepat%".into()
"%titlepat%".into(),
100u64.into(),
200u64.into()
]
)]
);
@@ -437,18 +449,21 @@ mod tests {
org_id: "orgId".to_owned(),
folder_id: Some("folderId".to_owned()),
title_pat: Some("tItLePat".to_owned()),
page_size_and_idx: Some((100, 2)),
};
list_models(&db, params).await?;
assert_eq!(
db.into_transaction_log(),
vec![Transaction::from_sql_and_values(
DatabaseBackend::MySql,
r#"SELECT `dashboards`.`id` AS `A_id`, `dashboards`.`dashboard_id` AS `A_dashboard_id`, `dashboards`.`folder_id` AS `A_folder_id`, `dashboards`.`owner` AS `A_owner`, `dashboards`.`role` AS `A_role`, `dashboards`.`title` AS `A_title`, `dashboards`.`description` AS `A_description`, `dashboards`.`data` AS `A_data`, `dashboards`.`version` AS `A_version`, `dashboards`.`created_at` AS `A_created_at`, `folders`.`id` AS `B_id`, `folders`.`org` AS `B_org`, `folders`.`folder_id` AS `B_folder_id`, `folders`.`name` AS `B_name`, `folders`.`description` AS `B_description`, `folders`.`type` AS `B_type` FROM `dashboards` LEFT JOIN `folders` ON `dashboards`.`folder_id` = `folders`.`id` WHERE `folders`.`org` = ? AND `folders`.`type` = ? AND `folders`.`folder_id` = ? AND LOWER(`title`) LIKE ? ORDER BY `dashboards`.`title` ASC, `folders`.`name` ASC"#,
r#"SELECT `dashboards`.`id` AS `A_id`, `dashboards`.`dashboard_id` AS `A_dashboard_id`, `dashboards`.`folder_id` AS `A_folder_id`, `dashboards`.`owner` AS `A_owner`, `dashboards`.`role` AS `A_role`, `dashboards`.`title` AS `A_title`, `dashboards`.`description` AS `A_description`, `dashboards`.`data` AS `A_data`, `dashboards`.`version` AS `A_version`, `dashboards`.`created_at` AS `A_created_at`, `folders`.`id` AS `B_id`, `folders`.`org` AS `B_org`, `folders`.`folder_id` AS `B_folder_id`, `folders`.`name` AS `B_name`, `folders`.`description` AS `B_description`, `folders`.`type` AS `B_type` FROM `dashboards` LEFT JOIN `folders` ON `dashboards`.`folder_id` = `folders`.`id` WHERE `folders`.`org` = ? AND `folders`.`type` = ? AND `folders`.`folder_id` = ? AND LOWER(`title`) LIKE ? ORDER BY `dashboards`.`title` ASC, `folders`.`name` ASC LIMIT ? OFFSET ?"#,
[
"orgId".into(),
0i16.into(),
"folderId".into(),
"%titlepat%".into()
"%titlepat%".into(),
100u64.into(),
200u64.into()
]
)]
);
@@ -464,18 +479,21 @@ mod tests {
org_id: "orgId".to_owned(),
folder_id: Some("folderId".to_owned()),
title_pat: Some("tItLePat".to_owned()),
page_size_and_idx: Some((100, 2)),
};
list_models(&db, params).await?;
assert_eq!(
db.into_transaction_log(),
vec![Transaction::from_sql_and_values(
DatabaseBackend::Sqlite,
r#"SELECT "dashboards"."id" AS "A_id", "dashboards"."dashboard_id" AS "A_dashboard_id", "dashboards"."folder_id" AS "A_folder_id", "dashboards"."owner" AS "A_owner", "dashboards"."role" AS "A_role", "dashboards"."title" AS "A_title", "dashboards"."description" AS "A_description", "dashboards"."data" AS "A_data", "dashboards"."version" AS "A_version", "dashboards"."created_at" AS "A_created_at", "folders"."id" AS "B_id", "folders"."org" AS "B_org", "folders"."folder_id" AS "B_folder_id", "folders"."name" AS "B_name", "folders"."description" AS "B_description", "folders"."type" AS "B_type" FROM "dashboards" LEFT JOIN "folders" ON "dashboards"."folder_id" = "folders"."id" WHERE "folders"."org" = ? AND "folders"."type" = ? AND "folders"."folder_id" = ? AND LOWER("title") LIKE ? ORDER BY "dashboards"."title" ASC, "folders"."name" ASC"#,
r#"SELECT "dashboards"."id" AS "A_id", "dashboards"."dashboard_id" AS "A_dashboard_id", "dashboards"."folder_id" AS "A_folder_id", "dashboards"."owner" AS "A_owner", "dashboards"."role" AS "A_role", "dashboards"."title" AS "A_title", "dashboards"."description" AS "A_description", "dashboards"."data" AS "A_data", "dashboards"."version" AS "A_version", "dashboards"."created_at" AS "A_created_at", "folders"."id" AS "B_id", "folders"."org" AS "B_org", "folders"."folder_id" AS "B_folder_id", "folders"."name" AS "B_name", "folders"."description" AS "B_description", "folders"."type" AS "B_type" FROM "dashboards" LEFT JOIN "folders" ON "dashboards"."folder_id" = "folders"."id" WHERE "folders"."org" = ? AND "folders"."type" = ? AND "folders"."folder_id" = ? AND LOWER("title") LIKE ? ORDER BY "dashboards"."title" ASC, "folders"."name" ASC LIMIT ? OFFSET ?"#,
[
"orgId".into(),
0i16.into(),
"folderId".into(),
"%titlepat%".into()
"%titlepat%".into(),
100u64.into(),
200u64.into()
]
)]
);