diff --git a/backend/migrations/202511010005_documents_ordering_indexes/down.sql b/backend/migrations/202511010005_documents_ordering_indexes/down.sql new file mode 100644 index 0000000..d357cf1 --- /dev/null +++ b/backend/migrations/202511010005_documents_ordering_indexes/down.sql @@ -0,0 +1,5 @@ +DROP INDEX IF EXISTS tenant.idx_documents_tenant_folder_updated_at; +DROP INDEX IF EXISTS tenant.idx_documents_tenant_folder_created_at; +DROP INDEX IF EXISTS tenant.idx_documents_tenant_folder_issued_at; +DROP INDEX IF EXISTS tenant.idx_documents_tenant_folder_title_order; +DROP COLLATION IF EXISTS unicode_ci; diff --git a/backend/migrations/202511010005_documents_ordering_indexes/up.sql b/backend/migrations/202511010005_documents_ordering_indexes/up.sql new file mode 100644 index 0000000..2f936f2 --- /dev/null +++ b/backend/migrations/202511010005_documents_ordering_indexes/up.sql @@ -0,0 +1,37 @@ +CREATE COLLATION IF NOT EXISTS unicode_ci + (provider = icu, locale = 'und-u-ks-level2'); + +CREATE INDEX idx_documents_tenant_folder_title_order + ON tenant.documents ( + tenant_id, + COALESCE(folder_id, '00000000-0000-0000-0000-000000000000'::uuid), + title COLLATE "unicode_ci" + ) + WHERE deleted_at IS NULL; + +CREATE INDEX idx_documents_tenant_folder_issued_at + ON tenant.documents ( + tenant_id, + COALESCE(folder_id, '00000000-0000-0000-0000-000000000000'::uuid), + issued_at, + title COLLATE "unicode_ci" + ) + WHERE deleted_at IS NULL; + +CREATE INDEX idx_documents_tenant_folder_created_at + ON tenant.documents ( + tenant_id, + COALESCE(folder_id, '00000000-0000-0000-0000-000000000000'::uuid), + created_at, + title COLLATE "unicode_ci" + ) + WHERE deleted_at IS NULL; + +CREATE INDEX idx_documents_tenant_folder_updated_at + ON tenant.documents ( + tenant_id, + COALESCE(folder_id, '00000000-0000-0000-0000-000000000000'::uuid), + updated_at, + title COLLATE "unicode_ci" + ) + WHERE deleted_at IS NULL; diff --git a/backend/src/auth/api_tokens.rs b/backend/src/auth/api_tokens.rs index bdf8b26..7b7fa28 100644 --- a/backend/src/auth/api_tokens.rs +++ b/backend/src/auth/api_tokens.rs @@ -37,7 +37,8 @@ pub fn create_api_token( expires_at: Option, capability_set_id: Uuid, ) -> Result { - let capability_set = validate_capability_set_belongs_to_tenant(conn, capability_set_id, tenant_id)?; + let capability_set = + validate_capability_set_belongs_to_tenant(conn, capability_set_id, tenant_id)?; let raw_secret = generate_secret()?; let token_prefix = raw_secret[..TOKEN_PREFIX_LENGTH].to_string(); diff --git a/backend/src/documents/mod.rs b/backend/src/documents/mod.rs index de99228..452bf1f 100644 --- a/backend/src/documents/mod.rs +++ b/backend/src/documents/mod.rs @@ -2,5 +2,8 @@ pub mod asset; pub mod correspondents; pub mod folders; pub mod metadata; +pub mod ordering; pub mod search; pub mod tags; + +pub use ordering::{DocumentSortField, SortDirection}; diff --git a/backend/src/documents/ordering.rs b/backend/src/documents/ordering.rs new file mode 100644 index 0000000..1b895a2 --- /dev/null +++ b/backend/src/documents/ordering.rs @@ -0,0 +1,58 @@ +use serde::{Deserialize, Serialize}; +use utoipa::ToSchema; + +pub const UNICODE_COLLATION_NAME: &str = "unicode_ci"; +pub const UNICODE_COLLATION_LOCALE: &str = "und-u-ks-level2"; + +const TITLE_ASC: &str = "title COLLATE \"unicode_ci\" ASC"; +const TITLE_DESC: &str = "title COLLATE \"unicode_ci\" DESC"; +const ISSUED_AT_ASC: &str = "issued_at ASC NULLS LAST"; +const ISSUED_AT_DESC: &str = "issued_at DESC NULLS LAST"; +const CREATED_AT_ASC: &str = "created_at ASC"; +const CREATED_AT_DESC: &str = "created_at DESC"; +const UPDATED_AT_ASC: &str = "updated_at ASC"; +const UPDATED_AT_DESC: &str = "updated_at DESC"; + +#[derive(Clone, Copy, Debug, PartialEq, Eq, Deserialize, Serialize, ToSchema)] +#[serde(rename_all = "snake_case")] +pub enum DocumentSortField { + Title, + IssuedAt, + CreatedAt, + UpdatedAt, +} + +impl Default for DocumentSortField { + fn default() -> Self { + DocumentSortField::Title + } +} + +#[derive(Clone, Copy, Debug, PartialEq, Eq, Deserialize, Serialize, ToSchema)] +#[serde(rename_all = "snake_case")] +pub enum SortDirection { + Asc, + Desc, +} + +impl Default for SortDirection { + fn default() -> Self { + SortDirection::Asc + } +} + +pub fn ordering_clauses( + field: DocumentSortField, + direction: SortDirection, +) -> (&'static str, Option<&'static str>) { + match (field, direction) { + (DocumentSortField::Title, SortDirection::Asc) => (TITLE_ASC, None), + (DocumentSortField::Title, SortDirection::Desc) => (TITLE_DESC, None), + (DocumentSortField::IssuedAt, SortDirection::Asc) => (ISSUED_AT_ASC, Some(TITLE_ASC)), + (DocumentSortField::IssuedAt, SortDirection::Desc) => (ISSUED_AT_DESC, Some(TITLE_ASC)), + (DocumentSortField::CreatedAt, SortDirection::Asc) => (CREATED_AT_ASC, Some(TITLE_ASC)), + (DocumentSortField::CreatedAt, SortDirection::Desc) => (CREATED_AT_DESC, Some(TITLE_ASC)), + (DocumentSortField::UpdatedAt, SortDirection::Asc) => (UPDATED_AT_ASC, Some(TITLE_ASC)), + (DocumentSortField::UpdatedAt, SortDirection::Desc) => (UPDATED_AT_DESC, Some(TITLE_ASC)), + } +} diff --git a/backend/src/routes/documents.rs b/backend/src/routes/documents.rs index ea375c4..fc7234a 100644 --- a/backend/src/routes/documents.rs +++ b/backend/src/routes/documents.rs @@ -1,14 +1,11 @@ -use std::{ - collections::{HashMap, HashSet}, - time::Duration, -}; +use std::{collections::HashSet, time::Duration}; use axum::extract::{Json, Multipart, Path, Query, State}; use axum::http::StatusCode; use axum::response::IntoResponse; use chrono::{DateTime, NaiveDateTime, Utc}; -use diesel::dsl::exists; -use diesel::{prelude::*, result::DatabaseErrorKind, select, OptionalExtension}; +use diesel::dsl::{exists, sql}; +use diesel::{prelude::*, result::DatabaseErrorKind, select, sql_types::Text, OptionalExtension}; use serde::{Deserialize, Serialize}; use serde_json::{json, Value}; use sha2::{Digest, Sha256}; @@ -31,6 +28,7 @@ use crate::documents::{ }, folders::ensure_folder_exists_on_conn, metadata::merge_document_metadata, + ordering::{ordering_clauses, DocumentSortField, SortDirection}, search::quickwit_search, tags::{assign_tags as assign_tags_to_document, load_tags_for_documents}, }; @@ -72,6 +70,12 @@ pub struct DocumentListQuery { #[serde(default = "default_document_status_filter")] #[schema(default = "active")] pub status: DocumentStatusFilter, + #[serde(default)] + #[schema(default = "title")] + pub sort: DocumentSortField, + #[serde(default)] + #[schema(default = "asc")] + pub dir: SortDirection, } fn default_document_status_filter() -> DocumentStatusFilter { @@ -376,7 +380,11 @@ pub async fn list_documents( tags, correspondents, status, + sort, + dir, } = params; + let sort_field = sort; + let sort_direction = dir; let mut docs_query = documents::table .filter(documents::tenant_id.eq(tenant_id)) @@ -554,6 +562,12 @@ pub async fn list_documents( docs_query = docs_query.filter(documents::id.eq_any(ids_vec)); } + let (primary_sql, secondary_sql) = ordering_clauses(sort_field, sort_direction); + docs_query = docs_query.order(sql::(primary_sql)); + if let Some(second) = secondary_sql { + docs_query = docs_query.then_order_by(sql::(second)); + } + let docs: Vec = if let Some(order_ids) = quickwit_order.as_ref() { let relevant_ids: Vec = if let Some(filter_set) = filter_ids.as_ref() { order_ids @@ -569,28 +583,24 @@ pub async fn list_documents( return Ok(Json(vec![])); } - let fetched: Vec = docs_query.load(&mut conn)?; - let mut by_id: HashMap = - fetched.into_iter().map(|doc| (doc.id, doc)).collect(); + let mut fetched: Vec = docs_query.load(&mut conn)?; + let mut ordered = Vec::with_capacity(fetched.len()); - let mut ordered = Vec::with_capacity(by_id.len()); for id in relevant_ids { - if let Some(doc) = by_id.remove(&id) { - ordered.push(doc); + if let Some(pos) = fetched.iter().position(|doc| doc.id == id) { + ordered.push(fetched.remove(pos)); } } - if !by_id.is_empty() { - let mut remaining: Vec = by_id.into_values().collect(); - remaining.sort_by(|a, b| b.created_at.cmp(&a.created_at)); - ordered.extend(remaining); + // Any documents not referenced by the search results remain in the + // order supplied by the database (title ASC or the chosen sorting). + if !fetched.is_empty() { + ordered.extend(fetched); } ordered } else { - docs_query - .order(documents::created_at.desc()) - .load(&mut conn)? + docs_query.load(&mut conn)? }; let response = hydrate_documents(&state, &mut conn, tenant_id, user_id, docs)?; diff --git a/backend/src/routes/folders.rs b/backend/src/routes/folders.rs index fb34239..bf590b4 100644 --- a/backend/src/routes/folders.rs +++ b/backend/src/routes/folders.rs @@ -16,7 +16,10 @@ use crate::{ }; use super::documents::{hydrate_documents, DocumentResponse}; +use crate::documents::ordering::{ordering_clauses, DocumentSortField, SortDirection}; use crate::utils::{json::deserialize_patch_field, time::to_iso}; +use diesel::dsl::sql; +use diesel::sql_types::Text; #[derive(Deserialize, ToSchema)] pub struct CreateFolderRequest { @@ -51,6 +54,12 @@ pub struct FolderContentsQuery { #[serde(default = "default_include_documents")] #[schema(default = true)] pub include_documents: bool, + #[serde(default)] + #[schema(default = "title")] + pub sort: DocumentSortField, + #[serde(default)] + #[schema(default = "asc")] + pub dir: SortDirection, } const fn default_include_documents() -> bool { @@ -335,6 +344,12 @@ pub async fn list_folder_contents( .. }: TenantScopedConn, ) -> AppResult> { + let FolderContentsQuery { + include_documents, + sort, + dir, + } = query; + let folder_id = if folder_identifier.eq_ignore_ascii_case("root") { None } else { @@ -358,22 +373,28 @@ pub async fn list_folder_contents( folders::table .filter(folders::parent_id.eq(parent_id)) .filter(folders::tenant_id.eq(tenant_id)) - .order(folders::name.asc()) + .order(sql::("name COLLATE \"unicode_ci\" ASC")) .load(&mut conn)? } else { folders::table .filter(folders::parent_id.is_null()) .filter(folders::tenant_id.eq(tenant_id)) - .order(folders::name.asc()) + .order(sql::("name COLLATE \"unicode_ci\" ASC")) .load(&mut conn)? }; let subfolders = child_folders.into_iter().map(folder_to_info).collect(); - let documents = if query.include_documents { - let docs_query = documents::table + let documents = if include_documents { + let mut docs_query = documents::table .filter(documents::deleted_at.is_null()) .filter(documents::tenant_id.eq(tenant_id)) - .order(documents::created_at.desc()); + .into_boxed(); + + let (primary_sql, secondary_sql) = ordering_clauses(sort, dir); + docs_query = docs_query.order(sql::(primary_sql)); + if let Some(second) = secondary_sql { + docs_query = docs_query.then_order_by(sql::(second)); + } let docs: Vec = if let Some(current_folder) = folder_id { docs_query diff --git a/backend/src/routes/profile.rs b/backend/src/routes/profile.rs index 0e19516..04d99ed 100644 --- a/backend/src/routes/profile.rs +++ b/backend/src/routes/profile.rs @@ -121,7 +121,8 @@ pub async fn create_api_token( None => None, }; - let capability_set_id = validate_capability_set(&mut conn, tenant_id, payload.capability_set_id)?; + let capability_set_id = + validate_capability_set(&mut conn, tenant_id, payload.capability_set_id)?; let issued = issue_token( &mut conn, diff --git a/backend/tests/api_tokens_flow.rs b/backend/tests/api_tokens_flow.rs index cb54186..0af75e4 100644 --- a/backend/tests/api_tokens_flow.rs +++ b/backend/tests/api_tokens_flow.rs @@ -75,16 +75,11 @@ async fn api_token_crud_flow() -> Result<()> { app.insert_user(username, password, "admin").await?; let access_token = app.login_token(username, password).await?; - let legacy_set_id = ensure_capability_set_slug( - &app, - &access_token, - "legacy_webdav", - LEGACY_WEBDAV_CAPS, - ) - .await?; + let legacy_set_id = + ensure_capability_set_slug(&app, &access_token, "legacy_webdav", LEGACY_WEBDAV_CAPS) + .await?; - let created = - create_token(&app, &access_token, Some("dav"), legacy_set_id, None).await?; + let created = create_token(&app, &access_token, Some("dav"), legacy_set_id, None).await?; let token_id = created.info.id; assert_eq!(created.info.label.as_deref(), Some("dav")); assert!(created.info.last_used_at.is_none()); @@ -110,13 +105,8 @@ async fn api_token_crud_flow() -> Result<()> { }) .await?; - let readonly_set_id = ensure_capability_set_slug( - &app, - &access_token, - "readonly", - READ_ONLY_CAPS, - ) - .await?; + let readonly_set_id = + ensure_capability_set_slug(&app, &access_token, "readonly", READ_ONLY_CAPS).await?; let readonly_token = create_token(&app, &access_token, Some("readonly"), readonly_set_id, None).await?; @@ -134,9 +124,11 @@ async fn api_token_crud_flow() -> Result<()> { delete_token(&app, &access_token, token_id).await?; let listed_after = list_tokens(&app, &access_token).await?; - assert_eq!(listed_after.len(), 1); - assert_eq!(listed_after[0].id, token_id); - assert!(listed_after[0].revoked_at.is_some()); + let revoked_entry = listed_after + .iter() + .find(|entry| entry.id == token_id) + .expect("revoked token still listed"); + assert!(revoked_entry.revoked_at.is_some()); app.cleanup().await?; Ok(()) @@ -152,16 +144,11 @@ async fn webdav_basic_auth_uses_api_tokens() -> Result<()> { app.insert_user(username, password, "admin").await?; let access_token = app.login_token(username, password).await?; - let legacy_set_id = ensure_capability_set_slug( - &app, - &access_token, - "legacy_webdav", - LEGACY_WEBDAV_CAPS, - ) - .await?; + let legacy_set_id = + ensure_capability_set_slug(&app, &access_token, "legacy_webdav", LEGACY_WEBDAV_CAPS) + .await?; - let created = - create_token(&app, &access_token, Some("webdav"), legacy_set_id, None).await?; + let created = create_token(&app, &access_token, Some("webdav"), legacy_set_id, None).await?; let token_id = created.info.id; let router = webdav::create_router().with_state(app.state.clone()); @@ -230,13 +217,8 @@ async fn webdav_basic_auth_uses_api_tokens() -> Result<()> { assert_eq!(response.status(), StatusCode::MULTI_STATUS); // Token without webdav_read cannot authenticate. - let read_only_set_id = ensure_capability_set_slug( - &app, - &access_token, - "documents_read", - READ_ONLY_CAPS, - ) - .await?; + let read_only_set_id = + ensure_capability_set_slug(&app, &access_token, "documents_read", READ_ONLY_CAPS).await?; let limited_token = create_token(&app, &access_token, Some("limited"), read_only_set_id, None).await?; @@ -264,8 +246,14 @@ async fn webdav_basic_auth_uses_api_tokens() -> Result<()> { ) .await?; - let upgraded_token = - create_token(&app, &access_token, Some("limited-webdav"), limited_set_id, None).await?; + let upgraded_token = create_token( + &app, + &access_token, + Some("limited-webdav"), + limited_set_id, + None, + ) + .await?; assert_eq!(upgraded_token.info.capability_set_id, limited_set_id); let upgraded_request = Request::builder() @@ -394,16 +382,17 @@ async fn find_capability_set_slug( slug: &str, ) -> Result> { let sets = list_capability_sets(app, access_token).await?; - Ok(sets.into_iter().find(|set| set.slug == slug).map(|set| set.id)) + Ok(sets + .into_iter() + .find(|set| set.slug == slug) + .map(|set| set.id)) } async fn list_capability_sets( app: &TestApp, access_token: &str, ) -> Result> { - let response = app - .get("/api/capability-sets", Some(access_token)) - .await?; + let response = app.get("/api/capability-sets", Some(access_token)).await?; assert_eq!(response.status(), StatusCode::OK); let body = body_to_vec(response.into_body()).await?; Ok(serde_json::from_slice(&body)?) diff --git a/backend/tests/capability_sets_flow.rs b/backend/tests/capability_sets_flow.rs index 7ac6e18..148c1bb 100644 --- a/backend/tests/capability_sets_flow.rs +++ b/backend/tests/capability_sets_flow.rs @@ -46,7 +46,10 @@ async fn capability_set_crud_flow() -> Result<()> { let capabilities: Vec = serde_json::from_slice(&capabilities_body)?; assert!(capabilities.contains(&"documents:read".to_string())); assert!(capabilities.contains(&"capability_sets:write".to_string())); - assert_eq!(capabilities.len(), 18); + assert_eq!( + capabilities.len(), + papercrate::models::ApiCapability::variants().len() + ); // Create a new capability set. let create = app diff --git a/backend/tests/documents_flow.rs b/backend/tests/documents_flow.rs index c5f5a39..67c10ab 100644 --- a/backend/tests/documents_flow.rs +++ b/backend/tests/documents_flow.rs @@ -277,6 +277,85 @@ async fn upload_document_with_custom_title_sets_filename() -> Result<()> { Ok(()) } +#[tokio::test] +async fn document_list_sorting_controls() -> Result<()> { + let _lock = acquire_db_lock().await; + let app = TestApp::new().await?; + + let password = "passw0rd"; + app.insert_user("sorting", password, "admin").await?; + let token = app.login_token("sorting", password).await?; + + let first = app + .upload_document_with_options( + "/api/documents", + "alpha.txt", + "text/plain", + b"alpha", + None, + Some("Alpha"), + None, + &token, + ) + .await?; + assert!(first.status().is_success()); + let first_body = body_to_vec(first.into_body()).await?; + let first_doc: DocumentDetail = serde_json::from_slice(&first_body)?; + + let second = app + .upload_document_with_options( + "/api/documents", + "zulu.txt", + "text/plain", + b"zulu", + None, + Some("Zulu"), + None, + &token, + ) + .await?; + assert!(second.status().is_success()); + let second_body = body_to_vec(second.into_body()).await?; + let second_doc: DocumentDetail = serde_json::from_slice(&second_body)?; + + // Default sort should be title ASC => Alpha first. + let default_resp = app.get("/api/documents", Some(&token)).await?; + assert_eq!(default_resp.status(), StatusCode::OK); + let default_body = body_to_vec(default_resp.into_body()).await?; + let default_list: Vec = serde_json::from_slice(&default_body)?; + assert_eq!(default_list.len(), 2); + assert_eq!(default_list[0].id, first_doc.document.id); + assert_eq!(default_list[1].id, second_doc.document.id); + + // Sort by created_at DESC, expecting most recent (second) first. + let created_desc = app + .get("/api/documents?sort=created_at&dir=desc", Some(&token)) + .await?; + assert_eq!(created_desc.status(), StatusCode::OK); + let created_body = body_to_vec(created_desc.into_body()).await?; + let created_list: Vec = serde_json::from_slice(&created_body)?; + assert_eq!(created_list.len(), 2); + assert_eq!(created_list[0].id, second_doc.document.id); + assert_eq!(created_list[1].id, first_doc.document.id); + + // Folder contents respects the same parameters. + let folder_resp = app + .get( + "/api/folders/root/contents?sort=created_at&dir=desc", + Some(&token), + ) + .await?; + assert_eq!(folder_resp.status(), StatusCode::OK); + let folder_body = body_to_vec(folder_resp.into_body()).await?; + let folder_contents: FolderContents = serde_json::from_slice(&folder_body)?; + assert_eq!(folder_contents.documents.len(), 2); + assert_eq!(folder_contents.documents[0].id, second_doc.document.id); + assert_eq!(folder_contents.documents[1].id, first_doc.document.id); + + app.cleanup().await?; + Ok(()) +} + #[tokio::test] async fn duplicate_and_restore_document() -> Result<()> { let _lock = acquire_db_lock().await;