refactor(search): simplify order by + wire reverse
This commit is contained in:
@@ -27,8 +27,8 @@ additionally restricted to administrators — see [Result Caching](#result-cachi
|
|||||||
| `modified_after` / `modified_before` | Filter by modification time |
|
| `modified_after` / `modified_before` | Filter by modification time |
|
||||||
| `min_size` / `max_size` | Filter by file size in bytes |
|
| `min_size` / `max_size` | Filter by file size in bytes |
|
||||||
| `resource_types` | Comma-separated: `file`, `folder` (both by default) |
|
| `resource_types` | Comma-separated: `file`, `folder` (both by default) |
|
||||||
| `order_by` | `relevance` (default), `name`, `name_desc`, `date`, `date_desc`, `size`, `size_desc` |
|
| `order_by` | Sort dimension: `relevance` (default), `name`, `size`, `updated_at`, `created_at` |
|
||||||
| `reverse` | Reverse the sort order |
|
| `reverse` | Reverse sort direction (no-op for `relevance`) |
|
||||||
| `limit` | Page size (1–200, default 50) |
|
| `limit` | Page size (1–200, default 50) |
|
||||||
| `cursor` | Opaque cursor returned by the previous page |
|
| `cursor` | Opaque cursor returned by the previous page |
|
||||||
|
|
||||||
|
|||||||
@@ -16,12 +16,12 @@ it('builds search requests including filters', async () => {
|
|||||||
fileTypes: ['mp3', 'wav'],
|
fileTypes: ['mp3', 'wav'],
|
||||||
minSize: 1,
|
minSize: 1,
|
||||||
maxSize: 9,
|
maxSize: 9,
|
||||||
sortBy: 'date'
|
sortBy: 'updated_at'
|
||||||
}).catch(() => {});
|
}).catch(() => {});
|
||||||
expect(j).toHaveBeenCalledWith(expect.stringContaining('type=mp3%2Cwav'), expect.anything());
|
expect(j).toHaveBeenCalledWith(expect.stringContaining('type=mp3%2Cwav'), expect.anything());
|
||||||
// Sort dimension is sent on the wire as `order_by`, matching the
|
// Sort dimension is sent on the wire as `order_by`, matching the
|
||||||
// backend's `SearchResourcesQuery` (post-normalization).
|
// backend's `SearchResourcesQuery` (post-normalization).
|
||||||
expect(j).toHaveBeenCalledWith(expect.stringContaining('order_by=date'), expect.anything());
|
expect(j).toHaveBeenCalledWith(expect.stringContaining('order_by=updated_at'), expect.anything());
|
||||||
await searchSuggest('q').catch(() => {});
|
await searchSuggest('q').catch(() => {});
|
||||||
await clearSearchCache().catch(() => {});
|
await clearSearchCache().catch(() => {});
|
||||||
expect(f.mock.calls.length + j.mock.calls.length).toBeGreaterThan(1);
|
expect(f.mock.calls.length + j.mock.calls.length).toBeGreaterThan(1);
|
||||||
|
|||||||
@@ -248,14 +248,14 @@ export interface AuthResponse {
|
|||||||
expires_in: number;
|
expires_in: number;
|
||||||
}
|
}
|
||||||
|
|
||||||
export type SortBy =
|
/**
|
||||||
| 'relevance'
|
* Sort dimension for `GET /api/search`. Wire-matches the backend's
|
||||||
| 'name'
|
* `SearchResourcesQuery.order_by` — 5 canonical values, direction is
|
||||||
| 'name_desc'
|
* a separate `reverse` boolean (the `_desc` suffix pattern was
|
||||||
| 'date'
|
* retired 2026-07-26; `date` was renamed to the more explicit
|
||||||
| 'date_desc'
|
* `updated_at` alongside the new `created_at`).
|
||||||
| 'size'
|
*/
|
||||||
| 'size_desc';
|
export type SortBy = 'relevance' | 'name' | 'size' | 'updated_at' | 'created_at';
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Per-item search metadata inline on every hit in the normalized
|
* Per-item search metadata inline on every hit in the normalized
|
||||||
|
|||||||
@@ -235,8 +235,14 @@
|
|||||||
{
|
{
|
||||||
key: 'modifiedAt',
|
key: 'modifiedAt',
|
||||||
label: t('groupby.modifiedAt', 'Modified date'),
|
label: t('groupby.modifiedAt', 'Modified date'),
|
||||||
orderBy: 'date',
|
orderBy: 'updated_at',
|
||||||
bucketOf: (item) => dateBucket(item.modified_at)
|
bucketOf: (item) => dateBucket(item.modified_at)
|
||||||
|
},
|
||||||
|
{
|
||||||
|
key: 'createdAt',
|
||||||
|
label: t('groupby.createdAt', 'Created date'),
|
||||||
|
orderBy: 'created_at',
|
||||||
|
bucketOf: (item) => dateBucket(item.created_at)
|
||||||
}
|
}
|
||||||
];
|
];
|
||||||
|
|
||||||
|
|||||||
@@ -64,9 +64,18 @@ pub struct SearchCriteriaDto {
|
|||||||
#[serde(default)]
|
#[serde(default)]
|
||||||
pub offset: usize,
|
pub offset: usize,
|
||||||
|
|
||||||
/// Sort order for results: "relevance", "name", "name_desc", "date", "date_desc", "size", "size_desc"
|
/// Sort dimension. Canonical set: `"relevance"` | `"name"` | `"size"`
|
||||||
|
/// | `"updated_at"` | `"created_at"`. Direction is `reverse` below —
|
||||||
|
/// the `_desc` suffix pattern was retired 2026-07-26 in favour of
|
||||||
|
/// a single boolean, so every consumer treats "which column" and
|
||||||
|
/// "which direction" as orthogonal concerns.
|
||||||
#[serde(default = "default_sort_by")]
|
#[serde(default = "default_sort_by")]
|
||||||
pub sort_by: String,
|
pub sort_by: String,
|
||||||
|
|
||||||
|
/// Reverse the sort direction — descending for name/size/date,
|
||||||
|
/// no-op for `relevance` (a descending relevance sort is meaningless).
|
||||||
|
#[serde(default)]
|
||||||
|
pub reverse: bool,
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Default value for recursive search (true)
|
/// Default value for recursive search (true)
|
||||||
@@ -100,6 +109,7 @@ impl Default for SearchCriteriaDto {
|
|||||||
limit: default_limit(),
|
limit: default_limit(),
|
||||||
offset: 0,
|
offset: 0,
|
||||||
sort_by: default_sort_by(),
|
sort_by: default_sort_by(),
|
||||||
|
reverse: false,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -345,10 +355,11 @@ pub struct SearchResourcesQuery {
|
|||||||
pub cursor: Option<String>,
|
pub cursor: Option<String>,
|
||||||
|
|
||||||
/// Sort dimension. Supported: `"relevance"` (default), `"name"`,
|
/// Sort dimension. Supported: `"relevance"` (default), `"name"`,
|
||||||
/// `"name_desc"`, `"date"` (= `modified_at`), `"date_desc"`,
|
/// `"size"`, `"updated_at"`, `"created_at"`. Direction is the
|
||||||
/// `"size"`, `"size_desc"`. Names match the pre-normalisation
|
/// separate `reverse` flag — the historical `_desc` suffix pattern
|
||||||
/// values `SearchCriteriaDto.sort_by` accepted so cached results
|
/// (`name_desc`, `date_desc`, `size_desc`) was retired 2026-07-26
|
||||||
/// remain reachable.
|
/// in favour of a single boolean, and `"date"` was renamed to the
|
||||||
|
/// more explicit `"updated_at"` alongside the new `"created_at"`.
|
||||||
pub order_by: Option<String>,
|
pub order_by: Option<String>,
|
||||||
|
|
||||||
/// Comma-separated resource types to include, e.g. `"file,folder"`.
|
/// Comma-separated resource types to include, e.g. `"file,folder"`.
|
||||||
@@ -399,10 +410,11 @@ impl SearchResourcesQuery {
|
|||||||
.as_deref()
|
.as_deref()
|
||||||
.and_then(SearchResourceCursor::decode)
|
.and_then(SearchResourceCursor::decode)
|
||||||
}
|
}
|
||||||
/// Convert to the internal `SearchCriteriaDto` the service still
|
/// Convert to the internal `SearchCriteriaDto` the service consumes.
|
||||||
/// consumes. `limit` / `offset` come from the decoded cursor (or the
|
/// `limit` / `offset` come from the decoded cursor (or the query's
|
||||||
/// query's `limit` on the first page). Sort names pass through
|
/// `limit` on the first page). `sort_by` + `reverse` pass through as
|
||||||
/// verbatim — the service's `sort_by` matcher accepts the same set.
|
/// two orthogonal fields — every downstream consumer (SQL builders,
|
||||||
|
/// in-memory folder sort) reads both.
|
||||||
pub fn to_criteria(&self) -> SearchCriteriaDto {
|
pub fn to_criteria(&self) -> SearchCriteriaDto {
|
||||||
let offset = self.decode_cursor().map(|c| c.offset).unwrap_or(0);
|
let offset = self.decode_cursor().map(|c| c.offset).unwrap_or(0);
|
||||||
let file_types = self.type_filter.as_deref().map(|s| {
|
let file_types = self.type_filter.as_deref().map(|s| {
|
||||||
@@ -425,6 +437,7 @@ impl SearchResourcesQuery {
|
|||||||
limit: self.limit_clamped(),
|
limit: self.limit_clamped(),
|
||||||
offset,
|
offset,
|
||||||
sort_by: self.order_by.clone().unwrap_or_else(default_sort_by),
|
sort_by: self.order_by.clone().unwrap_or_else(default_sort_by),
|
||||||
|
reverse: self.reverse,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
/// Which resource kinds to include. `None` = both. Anything else
|
/// Which resource kinds to include. `None` = both. Anything else
|
||||||
|
|||||||
@@ -201,14 +201,23 @@ fn content_relevance(score: f32, max_score: f32) -> u32 {
|
|||||||
|
|
||||||
/// Re-sort the merged file list with the same semantics the folder list
|
/// Re-sort the merged file list with the same semantics the folder list
|
||||||
/// uses. Only invoked when content hits were merged into a SQL-ordered page.
|
/// uses. Only invoked when content hits were merged into a SQL-ordered page.
|
||||||
fn sort_enriched_files(files: &mut [SearchFileResultDto], sort_by: &str) {
|
///
|
||||||
|
/// Sort dimension + direction are orthogonal (matches the wire
|
||||||
|
/// `SearchResourcesQuery` / internal `SearchCriteriaDto` split): 5
|
||||||
|
/// canonical `sort_by` values (`relevance | name | size | updated_at |
|
||||||
|
/// created_at`) × the boolean `reverse`.
|
||||||
|
fn sort_enriched_files(files: &mut [SearchFileResultDto], sort_by: &str, reverse: bool) {
|
||||||
match sort_by {
|
match sort_by {
|
||||||
|
"name" if reverse => files.sort_by_cached_key(|f| Reverse(f.name.to_lowercase())),
|
||||||
"name" => files.sort_by_cached_key(|f| f.name.to_lowercase()),
|
"name" => files.sort_by_cached_key(|f| f.name.to_lowercase()),
|
||||||
"name_desc" => files.sort_by_cached_key(|f| Reverse(f.name.to_lowercase())),
|
"updated_at" if reverse => files.sort_by_key(|f| Reverse(f.modified_at)),
|
||||||
"date" => files.sort_by_key(|f| f.modified_at),
|
"updated_at" => files.sort_by_key(|f| f.modified_at),
|
||||||
"date_desc" => files.sort_by_key(|f| Reverse(f.modified_at)),
|
"created_at" if reverse => files.sort_by_key(|f| Reverse(f.created_at)),
|
||||||
|
"created_at" => files.sort_by_key(|f| f.created_at),
|
||||||
|
"size" if reverse => files.sort_by_key(|f| Reverse(f.size)),
|
||||||
"size" => files.sort_by_key(|f| f.size),
|
"size" => files.sort_by_key(|f| f.size),
|
||||||
"size_desc" => files.sort_by_key(|f| Reverse(f.size)),
|
// `relevance` (default) — reverse is a no-op; descending
|
||||||
|
// relevance would be "least-relevant first," meaningless.
|
||||||
_ => files.sort_by_key(|f| Reverse(f.relevance_score)),
|
_ => files.sort_by_key(|f| Reverse(f.relevance_score)),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -519,7 +528,7 @@ impl SearchService {
|
|||||||
added += 1;
|
added += 1;
|
||||||
}
|
}
|
||||||
if added > 0 {
|
if added > 0 {
|
||||||
sort_enriched_files(enriched_files, &criteria.sort_by);
|
sort_enriched_files(enriched_files, &criteria.sort_by, criteria.reverse);
|
||||||
}
|
}
|
||||||
Ok(added)
|
Ok(added)
|
||||||
}
|
}
|
||||||
@@ -734,20 +743,33 @@ impl SearchUseCase for SearchService {
|
|||||||
})
|
})
|
||||||
.collect();
|
.collect();
|
||||||
|
|
||||||
// Sort folders (cached_key avoids O(N log N) temporary String allocations)
|
// Sort folders (cached_key avoids O(N log N) temporary String allocations).
|
||||||
match criteria.sort_by.as_str() {
|
// 5-dimension model (`relevance | name | size | updated_at | created_at`)
|
||||||
"name" => {
|
// × the boolean `reverse` — matches the file-side `sort_enriched_files`
|
||||||
|
// + the SQL match in `file_blob_read_repository`. `size` isn't
|
||||||
|
// meaningful for folders (no size column), so it falls through
|
||||||
|
// to the relevance default.
|
||||||
|
match (criteria.sort_by.as_str(), criteria.reverse) {
|
||||||
|
("name", false) => {
|
||||||
enriched_folders.sort_by_cached_key(|f| f.name.to_lowercase());
|
enriched_folders.sort_by_cached_key(|f| f.name.to_lowercase());
|
||||||
}
|
}
|
||||||
"name_desc" => {
|
("name", true) => {
|
||||||
enriched_folders.sort_by_cached_key(|f| Reverse(f.name.to_lowercase()));
|
enriched_folders.sort_by_cached_key(|f| Reverse(f.name.to_lowercase()));
|
||||||
}
|
}
|
||||||
"date" => {
|
("updated_at", false) => {
|
||||||
enriched_folders.sort_by_key(|f| f.modified_at);
|
enriched_folders.sort_by_key(|f| f.modified_at);
|
||||||
}
|
}
|
||||||
"date_desc" => {
|
("updated_at", true) => {
|
||||||
enriched_folders.sort_by_key(|f| Reverse(f.modified_at));
|
enriched_folders.sort_by_key(|f| Reverse(f.modified_at));
|
||||||
}
|
}
|
||||||
|
("created_at", false) => {
|
||||||
|
enriched_folders.sort_by_key(|f| f.created_at);
|
||||||
|
}
|
||||||
|
("created_at", true) => {
|
||||||
|
enriched_folders.sort_by_key(|f| Reverse(f.created_at));
|
||||||
|
}
|
||||||
|
// `relevance` (default) + `size` (N/A for folders) +
|
||||||
|
// anything unrecognised — all land on relevance-desc.
|
||||||
_ => {
|
_ => {
|
||||||
enriched_folders.sort_by_key(|f| Reverse(f.relevance_score));
|
enriched_folders.sort_by_key(|f| Reverse(f.relevance_score));
|
||||||
}
|
}
|
||||||
@@ -1147,17 +1169,20 @@ mod tests {
|
|||||||
dto("b-content.txt", 30, 10, 200),
|
dto("b-content.txt", 30, 10, 200),
|
||||||
dto("a-name.txt", 80, 99, 100),
|
dto("a-name.txt", 80, 99, 100),
|
||||||
];
|
];
|
||||||
sort_enriched_files(&mut files, "relevance");
|
sort_enriched_files(&mut files, "relevance", false);
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
files[0].name, "a-name.txt",
|
files[0].name, "a-name.txt",
|
||||||
"name match must outrank content match"
|
"name match must outrank content match"
|
||||||
);
|
);
|
||||||
|
|
||||||
sort_enriched_files(&mut files, "size_desc");
|
// 5-dimension sort model (2026-07-26): direction is a separate
|
||||||
|
// boolean, `_desc` suffixes retired. `size + reverse=true` = old
|
||||||
|
// `size_desc`, `updated_at + reverse=false` = old `date`, etc.
|
||||||
|
sort_enriched_files(&mut files, "size", true);
|
||||||
assert_eq!(files[0].name, "a-name.txt");
|
assert_eq!(files[0].name, "a-name.txt");
|
||||||
sort_enriched_files(&mut files, "date");
|
sort_enriched_files(&mut files, "updated_at", false);
|
||||||
assert_eq!(files[0].name, "a-name.txt");
|
assert_eq!(files[0].name, "a-name.txt");
|
||||||
sort_enriched_files(&mut files, "name_desc");
|
sort_enriched_files(&mut files, "name", true);
|
||||||
assert_eq!(files[0].name, "b-content.txt");
|
assert_eq!(files[0].name, "b-content.txt");
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1216,16 +1216,23 @@ impl FileReadPort for FileBlobReadRepository {
|
|||||||
let offset = criteria.offset as i64;
|
let offset = criteria.offset as i64;
|
||||||
let limit = criteria.limit as i64;
|
let limit = criteria.limit as i64;
|
||||||
|
|
||||||
// Determine sort order
|
// Determine sort order. Canonical `sort_by` set (matches the wire
|
||||||
let (order_column, order_dir) = match criteria.sort_by.as_str() {
|
// `SearchResourcesQuery.order_by` 1:1 — no rename at any layer):
|
||||||
"name" => ("fi.name", "ASC"),
|
// `relevance | name | size | updated_at | created_at`. Direction
|
||||||
"name_desc" => ("fi.name", "DESC"),
|
// comes from `criteria.reverse`; the old `_desc`-suffix pattern
|
||||||
"date" => ("fi.updated_at", "ASC"),
|
// was retired 2026-07-26.
|
||||||
"date_desc" => ("fi.updated_at", "DESC"),
|
let order_column = match criteria.sort_by.as_str() {
|
||||||
"size" => ("fi.size", "ASC"),
|
"name" => "fi.name",
|
||||||
"size_desc" => ("fi.size", "DESC"),
|
"updated_at" => "fi.updated_at",
|
||||||
_ => ("fi.name", "ASC"),
|
"created_at" => "fi.created_at",
|
||||||
|
"size" => "fi.size",
|
||||||
|
// `relevance` (or anything unrecognised) has no dedicated
|
||||||
|
// column here — the recursive/non-recursive services blend
|
||||||
|
// in content-index hits and re-sort in memory. Falling back
|
||||||
|
// to name keeps the SQL page stable.
|
||||||
|
_ => "fi.name",
|
||||||
};
|
};
|
||||||
|
let order_dir = if criteria.reverse { "DESC" } else { "ASC" };
|
||||||
|
|
||||||
// ── Build dynamic WHERE + bind indices ───────────────────────────
|
// ── Build dynamic WHERE + bind indices ───────────────────────────
|
||||||
let mut conditions: Vec<String> = vec![
|
let mut conditions: Vec<String> = vec![
|
||||||
@@ -1376,16 +1383,23 @@ impl FileReadPort for FileBlobReadRepository {
|
|||||||
let offset = criteria.offset as i64;
|
let offset = criteria.offset as i64;
|
||||||
let limit = criteria.limit as i64;
|
let limit = criteria.limit as i64;
|
||||||
|
|
||||||
// Determine sort order
|
// Determine sort order. Canonical `sort_by` set (matches the wire
|
||||||
let (order_column, order_dir) = match criteria.sort_by.as_str() {
|
// `SearchResourcesQuery.order_by` 1:1 — no rename at any layer):
|
||||||
"name" => ("fi.name", "ASC"),
|
// `relevance | name | size | updated_at | created_at`. Direction
|
||||||
"name_desc" => ("fi.name", "DESC"),
|
// comes from `criteria.reverse`; the old `_desc`-suffix pattern
|
||||||
"date" => ("fi.updated_at", "ASC"),
|
// was retired 2026-07-26.
|
||||||
"date_desc" => ("fi.updated_at", "DESC"),
|
let order_column = match criteria.sort_by.as_str() {
|
||||||
"size" => ("fi.size", "ASC"),
|
"name" => "fi.name",
|
||||||
"size_desc" => ("fi.size", "DESC"),
|
"updated_at" => "fi.updated_at",
|
||||||
_ => ("fi.name", "ASC"),
|
"created_at" => "fi.created_at",
|
||||||
|
"size" => "fi.size",
|
||||||
|
// `relevance` (or anything unrecognised) has no dedicated
|
||||||
|
// column here — the recursive/non-recursive services blend
|
||||||
|
// in content-index hits and re-sort in memory. Falling back
|
||||||
|
// to name keeps the SQL page stable.
|
||||||
|
_ => "fi.name",
|
||||||
};
|
};
|
||||||
|
let order_dir = if criteria.reverse { "DESC" } else { "ASC" };
|
||||||
|
|
||||||
// ── Build dynamic WHERE clauses ──
|
// ── Build dynamic WHERE clauses ──
|
||||||
let mut conditions = Vec::new();
|
let mut conditions = Vec::new();
|
||||||
|
|||||||
@@ -257,7 +257,7 @@ pub struct SuggestParams {
|
|||||||
("query" = Option<String>, Query, description = "Text to search in names / content"),
|
("query" = Option<String>, Query, description = "Text to search in names / content"),
|
||||||
("limit" = Option<u32>, Query, description = "Max items per page (1–200, default 50)"),
|
("limit" = Option<u32>, Query, description = "Max items per page (1–200, default 50)"),
|
||||||
("cursor" = Option<String>, Query, description = "Opaque cursor from a previous response"),
|
("cursor" = Option<String>, Query, description = "Opaque cursor from a previous response"),
|
||||||
("order_by" = Option<String>, Query, description = "Sort dimension: relevance (default) | name | name_desc | date | date_desc | size | size_desc"),
|
("order_by" = Option<String>, Query, description = "Sort dimension: relevance (default) | name | size | updated_at | created_at"),
|
||||||
("resource_types" = Option<String>, Query, description = "Comma-separated: file, folder (both by default)"),
|
("resource_types" = Option<String>, Query, description = "Comma-separated: file, folder (both by default)"),
|
||||||
("reverse" = Option<bool>, Query, description = "Reverse the sort order"),
|
("reverse" = Option<bool>, Query, description = "Reverse the sort order"),
|
||||||
("type" = Option<String>, Query, description = "Filter by file extensions (comma-separated)"),
|
("type" = Option<String>, Query, description = "Filter by file extensions (comma-separated)"),
|
||||||
|
|||||||
Reference in New Issue
Block a user