max / makenotwork
5 files changed,
+240 insertions,
-87 deletions
| @@ -2,7 +2,7 @@ | |||
| 2 | 2 | //! | |
| 3 | 3 | //! Uses `pg_trgm` trigram indexes for fuzzy text matching. | |
| 4 | 4 | ||
| 5 | - | use sqlx::{FromRow, PgPool}; | |
| 5 | + | use sqlx::{FromRow, PgPool, Row}; | |
| 6 | 6 | ||
| 7 | 7 | use super::enums::{AiTierFilter, DiscoverSort, ItemType}; | |
| 8 | 8 | use super::models::*; | |
| @@ -632,6 +632,30 @@ pub async fn get_item_type_counts( | |||
| 632 | 632 | Ok(counts) | |
| 633 | 633 | } | |
| 634 | 634 | ||
| 635 | + | /// The discover price buckets: label, inclusive lower bound, inclusive upper | |
| 636 | + | /// bound (`None` = unbounded). | |
| 637 | + | /// | |
| 638 | + | /// The single source of truth for both the `COUNT(*) FILTER` clauses that | |
| 639 | + | /// produce the sidebar numbers and the `min_price`/`max_price` links those | |
| 640 | + | /// numbers are clickable through. They were duplicated across two files and | |
| 641 | + | /// kept in step only by a comment at each site; a bucket whose bounds drifted | |
| 642 | + | /// from its own count would display a number its link could not reproduce. | |
| 643 | + | pub const PRICE_BUCKETS: [(&str, i32, Option<i32>); 5] = [ | |
| 644 | + | ("Free", 0, Some(0)), | |
| 645 | + | ("Under $25", 1, Some(2499)), | |
| 646 | + | ("$25-50", 2500, Some(4999)), | |
| 647 | + | ("$50-100", 5000, Some(9999)), | |
| 648 | + | ("$100+", 10000, None), | |
| 649 | + | ]; | |
| 650 | + | ||
| 651 | + | /// The `FILTER (WHERE ...)` predicate for one bucket. | |
| 652 | + | fn price_bucket_predicate(min: i32, max: Option<i32>) -> String { | |
| 653 | + | match max { | |
| 654 | + | Some(max) => format!("i.price_cents >= {min} AND i.price_cents <= {max}"), | |
| 655 | + | None => format!("i.price_cents >= {min}"), | |
| 656 | + | } | |
| 657 | + | } | |
| 658 | + | ||
| 635 | 659 | /// Get price range counts for the discover page sidebar (items mode only). | |
| 636 | 660 | /// | |
| 637 | 661 | /// Buckets are in cents: free (0), under $25 (1..2499), $25-$50 (2500..4999), | |
| @@ -641,23 +665,34 @@ pub async fn get_item_type_counts( | |||
| 641 | 665 | pub async fn get_price_range_counts( | |
| 642 | 666 | pool: &PgPool, | |
| 643 | 667 | filters: &DiscoverFilters<'_>, | |
| 644 | - | ) -> Result<DbPriceRangeCounts> { | |
| 668 | + | ) -> Result<Vec<i64>> { | |
| 645 | 669 | let search_term = normalize_search(filters.search); | |
| 646 | 670 | let has_search = search_term.is_some(); | |
| 647 | 671 | let short_query = search_term.as_deref().is_some_and(is_short_query); | |
| 648 | 672 | ||
| 649 | - | let mut query = String::from( | |
| 673 | + | // One aggregate per bucket, generated from PRICE_BUCKETS so the counts and | |
| 674 | + | // the bucket links can never describe different ranges. Bounds are integer | |
| 675 | + | // literals from a const, not user input. | |
| 676 | + | let selects: Vec<String> = PRICE_BUCKETS | |
| 677 | + | .iter() | |
| 678 | + | .enumerate() | |
| 679 | + | .map(|(i, (_, min, max))| { | |
| 680 | + | format!( | |
| 681 | + | "COUNT(*) FILTER (WHERE {}) as bucket_{i}", | |
| 682 | + | price_bucket_predicate(*min, *max) | |
| 683 | + | ) | |
| 684 | + | }) | |
| 685 | + | .collect(); | |
| 686 | + | ||
| 687 | + | let mut query = format!( | |
| 650 | 688 | r#" | |
| 651 | 689 | SELECT | |
| 652 | - | COUNT(*) FILTER (WHERE i.price_cents = 0) as free, | |
| 653 | - | COUNT(*) FILTER (WHERE i.price_cents > 0 AND i.price_cents < 2500) as under_25, | |
| 654 | - | COUNT(*) FILTER (WHERE i.price_cents >= 2500 AND i.price_cents < 5000) as range_25_50, | |
| 655 | - | COUNT(*) FILTER (WHERE i.price_cents >= 5000 AND i.price_cents < 10000) as range_50_100, | |
| 656 | - | COUNT(*) FILTER (WHERE i.price_cents >= 10000) as over_100 | |
| 690 | + | {} | |
| 657 | 691 | FROM items i | |
| 658 | 692 | JOIN projects p ON i.project_id = p.id | |
| 659 | 693 | JOIN users u ON p.user_id = u.id | |
| 660 | 694 | "#, | |
| 695 | + | selects.join(",\n ") | |
| 661 | 696 | ); | |
| 662 | 697 | query.push_str(ITEM_VISIBILITY_WHERE); | |
| 663 | 698 | ||
| @@ -669,30 +704,20 @@ pub async fn get_price_range_counts( | |||
| 669 | 704 | short_query, | |
| 670 | 705 | ); | |
| 671 | 706 | ||
| 672 | - | #[derive(FromRow)] | |
| 673 | - | struct PriceRow { | |
| 674 | - | free: Option<i64>, | |
| 675 | - | under_25: Option<i64>, | |
| 676 | - | range_25_50: Option<i64>, | |
| 677 | - | range_50_100: Option<i64>, | |
| 678 | - | over_100: Option<i64>, | |
| 679 | - | } | |
| 680 | - | ||
| 681 | 707 | let row = bind_item_discover_filters!( | |
| 682 | - | sqlx::query_as::<_, PriceRow>(&query), | |
| 708 | + | sqlx::query(&query), | |
| 683 | 709 | filters, | |
| 684 | 710 | search_term.as_deref() | |
| 685 | 711 | ) | |
| 686 | 712 | .fetch_one(pool) | |
| 687 | 713 | .await?; | |
| 688 | 714 | ||
| 689 | - | Ok(DbPriceRangeCounts { | |
| 690 | - | free: row.free.unwrap_or(0), | |
| 691 | - | under_25: row.under_25.unwrap_or(0), | |
| 692 | - | range_25_50: row.range_25_50.unwrap_or(0), | |
| 693 | - | range_50_100: row.range_50_100.unwrap_or(0), | |
| 694 | - | over_100: row.over_100.unwrap_or(0), | |
| 695 | - | }) | |
| 715 | + | // Positional read, in PRICE_BUCKETS order. | |
| 716 | + | let counts = (0..PRICE_BUCKETS.len()) | |
| 717 | + | .map(|i| row.try_get::<Option<i64>, _>(i).ok().flatten().unwrap_or(0)) | |
| 718 | + | .collect(); | |
| 719 | + | ||
| 720 | + | Ok(counts) | |
| 696 | 721 | } | |
| 697 | 722 | ||
| 698 | 723 | /// Get AI tier counts for the discover page sidebar (items mode only). |
| @@ -76,20 +76,6 @@ pub struct DbItemTypeCount { | |||
| 76 | 76 | pub count: i64, | |
| 77 | 77 | } | |
| 78 | 78 | ||
| 79 | - | /// Bucketed item counts by price range for the discover page sidebar. | |
| 80 | - | #[derive(Debug, Clone, Default)] | |
| 81 | - | pub struct DbPriceRangeCounts { | |
| 82 | - | /// Items priced at $0. | |
| 83 | - | pub free: i64, | |
| 84 | - | /// Items priced $0.01 -- $24.99. | |
| 85 | - | pub under_25: i64, | |
| 86 | - | /// Items priced $25.00 -- $49.99. | |
| 87 | - | pub range_25_50: i64, | |
| 88 | - | /// Items priced $50.00 -- $99.99. | |
| 89 | - | pub range_50_100: i64, | |
| 90 | - | /// Items priced $100.00 and above. | |
| 91 | - | pub over_100: i64, | |
| 92 | - | } | |
| 93 | 79 | ||
| 94 | 80 | /// A user-defined external link shown on their profile. | |
| 95 | 81 | #[derive(Debug, Clone, FromRow, Serialize)] |
| @@ -260,50 +260,30 @@ async fn build_sidebar( | |||
| 260 | 260 | ||
| 261 | 261 | (type_filters, tag_filters, ai_tier_filters, price_counts) | |
| 262 | 262 | } else { | |
| 263 | - | (vec![], vec![], vec![], db::DbPriceRangeCounts::default()) | |
| 263 | + | (vec![], vec![], vec![], Vec::<i64>::new()) | |
| 264 | 264 | }; | |
| 265 | 265 | ||
| 266 | - | let price_filters = vec![ | |
| 267 | - | PriceFilter { label: "Free".to_string(), count: price_counts.free as u32 }, | |
| 268 | - | PriceFilter { label: "Under $25".to_string(), count: price_counts.under_25 as u32 }, | |
| 269 | - | PriceFilter { label: "$25-50".to_string(), count: price_counts.range_25_50 as u32 }, | |
| 270 | - | PriceFilter { label: "$50-100".to_string(), count: price_counts.range_50_100 as u32 }, | |
| 271 | - | PriceFilter { label: "$100+".to_string(), count: price_counts.over_100 as u32 }, | |
| 272 | - | ]; | |
| 273 | - | ||
| 274 | - | // The same five buckets as clickable ranges. Bounds mirror the FILTER | |
| 275 | - | // clauses in db::discover::get_price_range_counts exactly; if one moves the | |
| 276 | - | // other must, or a bucket will report a count its own link cannot reproduce. | |
| 266 | + | // Both the counts and the clickable ranges come from db::discover::PRICE_BUCKETS, | |
| 267 | + | // so a bucket can never display a number its own link fails to reproduce. | |
| 277 | 268 | let price_base_params = query.params_without_price(); | |
| 278 | 269 | let (applied_min, applied_max) = sanitize_price_range(query.min_price, query.max_price); | |
| 279 | - | let price_buckets: Vec<PriceBucket> = [ | |
| 280 | - | ("Free", price_counts.free, Some(0), Some(0)), | |
| 281 | - | ("Under $25", price_counts.under_25, Some(1), Some(2499)), | |
| 282 | - | ("$25-50", price_counts.range_25_50, Some(2500), Some(4999)), | |
| 283 | - | ("$50-100", price_counts.range_50_100, Some(5000), Some(9999)), | |
| 284 | - | ("$100+", price_counts.over_100, Some(10000), None), | |
| 285 | - | ] | |
| 286 | - | .into_iter() | |
| 287 | - | .map(|(label, count, min, max)| { | |
| 288 | - | let mut parts = price_base_params.clone(); | |
| 289 | - | if let Some(v) = min { | |
| 290 | - | parts.push(format!("min_price={v}")); | |
| 291 | - | } | |
| 292 | - | if let Some(v) = max { | |
| 293 | - | parts.push(format!("max_price={v}")); | |
| 294 | - | } | |
| 295 | - | PriceBucket { | |
| 296 | - | label: label.to_string(), | |
| 297 | - | count: count as u32, | |
| 298 | - | url: if parts.is_empty() { | |
| 299 | - | "/discover".to_string() | |
| 300 | - | } else { | |
| 301 | - | format!("/discover?{}", parts.join("&")) | |
| 302 | - | }, | |
| 303 | - | active: applied_min == min && applied_max == max, | |
| 304 | - | } | |
| 305 | - | }) | |
| 306 | - | .collect(); | |
| 270 | + | let price_buckets: Vec<PriceBucket> = db::discover::PRICE_BUCKETS | |
| 271 | + | .iter() | |
| 272 | + | .zip(price_counts.iter()) | |
| 273 | + | .map(|((label, min, max), count)| { | |
| 274 | + | let mut parts = price_base_params.clone(); | |
| 275 | + | parts.push(format!("min_price={min}")); | |
| 276 | + | if let Some(v) = *max { | |
| 277 | + | parts.push(format!("max_price={v}")); | |
| 278 | + | } | |
| 279 | + | PriceBucket { | |
| 280 | + | label: label.to_string(), | |
| 281 | + | count: *count as u32, | |
| 282 | + | url: format!("/discover?{}", parts.join("&")), | |
| 283 | + | active: applied_min == Some(*min) && applied_max == *max, | |
| 284 | + | } | |
| 285 | + | }) | |
| 286 | + | .collect(); | |
| 307 | 287 | ||
| 308 | 288 | // The mobile filter badge counts *groups* with an active selection, not | |
| 309 | 289 | // individual values: picking three tags is still one filter in use. | |
| @@ -350,7 +330,6 @@ async fn build_sidebar( | |||
| 350 | 330 | type_filters, | |
| 351 | 331 | tag_filters, | |
| 352 | 332 | category_filters, | |
| 353 | - | price_filters, | |
| 354 | 333 | price_buckets, | |
| 355 | 334 | ai_tier_filters, | |
| 356 | 335 | tag_chips, | |
| @@ -378,7 +357,8 @@ type FacetBundle = ( | |||
| 378 | 357 | Vec<db::DbItemTypeCount>, | |
| 379 | 358 | Vec<db::DbTagCount>, | |
| 380 | 359 | Vec<db::DbItemTypeCount>, | |
| 381 | - | db::DbPriceRangeCounts, | |
| 360 | + | // Bucket counts, positional and in `db::discover::PRICE_BUCKETS` order. | |
| 361 | + | Vec<i64>, | |
| 382 | 362 | ); | |
| 383 | 363 | ||
| 384 | 364 | /// Cache key: the filter inputs every facet query keys off. Same filters -> | |
| @@ -739,6 +719,97 @@ mod price_range_tests { | |||
| 739 | 719 | } | |
| 740 | 720 | } | |
| 741 | 721 | ||
| 722 | + | /// Every `DiscoverQuery` field, paired with whether the rendered page must | |
| 723 | + | /// carry a form control submitting under that name. | |
| 724 | + | /// | |
| 725 | + | /// This exists because a form's control names are a contract with its handler | |
| 726 | + | /// and nothing checks it: a rename pass once rewrote `name="has_source"` to | |
| 727 | + | /// `name="sidebar.has_source"` and the entire suite still passed, with the | |
| 728 | + | /// filter silently inert. | |
| 729 | + | /// | |
| 730 | + | /// The exhaustive destructure is the point. Adding a field to `DiscoverQuery` | |
| 731 | + | /// stops this compiling, which forces a decision about whether the new | |
| 732 | + | /// parameter needs a control rather than letting it be silently unreachable. | |
| 733 | + | #[cfg(test)] | |
| 734 | + | fn query_param_contract() -> Vec<(&'static str, bool)> { | |
| 735 | + | let DiscoverQuery { | |
| 736 | + | q: _, | |
| 737 | + | item_type: _, | |
| 738 | + | tag: _, | |
| 739 | + | category: _, | |
| 740 | + | min_price: _, | |
| 741 | + | max_price: _, | |
| 742 | + | sort: _, | |
| 743 | + | page: _, | |
| 744 | + | mode: _, | |
| 745 | + | ai_tier: _, | |
| 746 | + | has_source: _, | |
| 747 | + | browse: _, | |
| 748 | + | } = DiscoverQuery { | |
| 749 | + | q: None, | |
| 750 | + | item_type: Vec::new(), | |
| 751 | + | tag: Vec::new(), | |
| 752 | + | category: None, | |
| 753 | + | min_price: None, | |
| 754 | + | max_price: None, | |
| 755 | + | sort: None, | |
| 756 | + | page: None, | |
| 757 | + | mode: None, | |
| 758 | + | ai_tier: None, | |
| 759 | + | has_source: None, | |
| 760 | + | browse: None, | |
| 761 | + | }; | |
| 762 | + | ||
| 763 | + | vec![ | |
| 764 | + | ("q", true), | |
| 765 | + | ("item_type", true), | |
| 766 | + | ("tag", true), | |
| 767 | + | ("category", true), | |
| 768 | + | ("min_price", true), | |
| 769 | + | ("max_price", true), | |
| 770 | + | ("sort", true), | |
| 771 | + | // Pagination is rendered as links carrying hx-vals, not a control. | |
| 772 | + | ("page", false), | |
| 773 | + | ("mode", true), | |
| 774 | + | ("ai_tier", true), | |
| 775 | + | ("has_source", true), | |
| 776 | + | // The drill-down cursor moves by link, never by form submission. | |
| 777 | + | ("browse", false), | |
| 778 | + | ] | |
| 779 | + | } | |
| 780 | + | ||
| 781 | + | /// The names the rendered page must expose as form controls, for the | |
| 782 | + | /// integration test that checks the markup. | |
| 783 | + | #[cfg(test)] | |
| 784 | + | pub(crate) fn required_control_names() -> Vec<&'static str> { | |
| 785 | + | query_param_contract() | |
| 786 | + | .into_iter() | |
| 787 | + | .filter(|(_, required)| *required) | |
| 788 | + | .map(|(name, _)| name) | |
| 789 | + | .collect() | |
| 790 | + | } | |
| 791 | + | ||
| 792 | + | #[cfg(test)] | |
| 793 | + | mod query_contract_tests { | |
| 794 | + | use super::*; | |
| 795 | + | ||
| 796 | + | #[test] | |
| 797 | + | fn every_query_param_is_accounted_for() { | |
| 798 | + | let contract = query_param_contract(); | |
| 799 | + | assert_eq!( | |
| 800 | + | contract.len(), | |
| 801 | + | 12, | |
| 802 | + | "DiscoverQuery gained or lost a field; decide whether it needs a control" | |
| 803 | + | ); | |
| 804 | + | // Names must be unique, or a duplicate would mask a missing one. | |
| 805 | + | let mut names: Vec<_> = contract.iter().map(|(n, _)| *n).collect(); | |
| 806 | + | names.sort_unstable(); | |
| 807 | + | let before = names.len(); | |
| 808 | + | names.dedup(); | |
| 809 | + | assert_eq!(before, names.len(), "duplicate param name in the contract"); | |
| 810 | + | } | |
| 811 | + | } | |
| 812 | + | ||
| 742 | 813 | #[cfg(test)] | |
| 743 | 814 | mod page_url_tests { | |
| 744 | 815 | use super::DiscoverQuery; |
| @@ -114,11 +114,6 @@ pub struct PriceBucket { | |||
| 114 | 114 | pub active: bool, | |
| 115 | 115 | } | |
| 116 | 116 | ||
| 117 | - | pub struct PriceFilter { | |
| 118 | - | pub label: String, | |
| 119 | - | pub count: u32, | |
| 120 | - | } | |
| 121 | - | ||
| 122 | 117 | /// Everything the discover sidebar renders. | |
| 123 | 118 | /// | |
| 124 | 119 | /// Grouped rather than flattened onto the templates because both the full page | |
| @@ -129,7 +124,6 @@ pub struct SidebarView { | |||
| 129 | 124 | pub type_filters: Vec<FilterCategory>, | |
| 130 | 125 | pub tag_filters: Vec<FilterCategory>, | |
| 131 | 126 | pub category_filters: Vec<FilterCategory>, | |
| 132 | - | pub price_filters: Vec<PriceFilter>, | |
| 133 | 127 | pub price_buckets: Vec<PriceBucket>, | |
| 134 | 128 | pub ai_tier_filters: Vec<FilterCategory>, | |
| 135 | 129 | pub tag_chips: Vec<TagChip>, |
| @@ -442,6 +442,83 @@ async fn price_filter_round_trips_into_its_inputs() { | |||
| 442 | 442 | } | |
| 443 | 443 | ||
| 444 | 444 | // --------------------------------------------------------------------------- | |
| 445 | + | // Form/handler name contract. | |
| 446 | + | // --------------------------------------------------------------------------- | |
| 447 | + | ||
| 448 | + | /// Every filter parameter the handler reads is reachable from the markup. | |
| 449 | + | /// | |
| 450 | + | /// A form's control names are an untested contract with its handler. A rename | |
| 451 | + | /// pass once rewrote `name="has_source"` to `name="sidebar.has_source"` and the | |
| 452 | + | /// whole suite passed with the filter inert. The list is derived from an | |
| 453 | + | /// exhaustive destructure of `DiscoverQuery` (see `query_param_contract`), so a | |
| 454 | + | /// new query parameter cannot be added without deciding whether it needs a | |
| 455 | + | /// control here. | |
| 456 | + | #[tokio::test] | |
| 457 | + | async fn every_filter_param_has_a_control_in_the_markup() { | |
| 458 | + | let mut h = TestHarness::new().await; | |
| 459 | + | // Seed both modes so every facet renders something. | |
| 460 | + | let setup = h.create_creator_with_item("namecontract", "audio", 1000).await; | |
| 461 | + | sqlx::query( | |
| 462 | + | "UPDATE items SET is_public = true, listed = true, scan_status = 'clean', \ | |
| 463 | + | deleted_at = NULL WHERE id = $1::uuid", | |
| 464 | + | ) | |
| 465 | + | .bind(&setup.item_id) | |
| 466 | + | .execute(&h.db) | |
| 467 | + | .await | |
| 468 | + | .expect("publish item"); | |
| 469 | + | sqlx::query("UPDATE projects SET is_public = true WHERE id = $1::uuid") | |
| 470 | + | .bind(&setup.project_id) | |
| 471 | + | .execute(&h.db) | |
| 472 | + | .await | |
| 473 | + | .expect("publish project"); | |
| 474 | + | ||
| 475 | + | // Items mode carries q, item_type, min_price, max_price, sort, mode, ai_tier. | |
| 476 | + | let items = h.client.get("/discover?mode=items").await; | |
| 477 | + | assert!(items.status.is_success(), "{}", items.status); | |
| 478 | + | for name in ["q", "item_type", "min_price", "max_price", "sort", "mode", "ai_tier"] { | |
| 479 | + | assert!( | |
| 480 | + | items.text.contains(&format!(r#"name="{name}""#)), | |
| 481 | + | "items mode must expose a control named {name}" | |
| 482 | + | ); | |
| 483 | + | } | |
| 484 | + | ||
| 485 | + | // `tag` is deliberately absent at the root cursor: the rungs there are | |
| 486 | + | // depth-1 type roots, which cannot be assigned to an item and so render as | |
| 487 | + | // navigation rather than checkboxes. It appears once the cursor reaches a | |
| 488 | + | // level with assignable leaves (or via the typeahead, or as a hidden input | |
| 489 | + | // once something is selected). | |
| 490 | + | assert!( | |
| 491 | + | !items.text.contains(r#"name="tag""#), | |
| 492 | + | "the root rung has no assignable tags, so it should offer no tag control" | |
| 493 | + | ); | |
| 494 | + | let drilled = h.client.get("/discover?mode=items&browse=audio.genre").await; | |
| 495 | + | assert!(drilled.status.is_success(), "{}", drilled.status); | |
| 496 | + | assert!( | |
| 497 | + | drilled.text.contains(r#"name="tag""#), | |
| 498 | + | "drilling to a leaf level must expose a control named tag" | |
| 499 | + | ); | |
| 500 | + | ||
| 501 | + | // Projects mode carries category and has_source instead of the item facets. | |
| 502 | + | let projects = h.client.get("/discover?mode=projects").await; | |
| 503 | + | assert!(projects.status.is_success(), "{}", projects.status); | |
| 504 | + | for name in ["q", "category", "has_source", "sort", "mode"] { | |
| 505 | + | assert!( | |
| 506 | + | projects.text.contains(&format!(r#"name="{name}""#)), | |
| 507 | + | "projects mode must expose a control named {name}" | |
| 508 | + | ); | |
| 509 | + | } | |
| 510 | + | ||
| 511 | + | // No control may submit under a template field path, which is what the | |
| 512 | + | // sidebar-extraction rename produced. | |
| 513 | + | for body in [&items.text, &projects.text] { | |
| 514 | + | assert!( | |
| 515 | + | !body.contains(r#"name="sidebar."#), | |
| 516 | + | "a form control is submitting under a template field path" | |
| 517 | + | ); | |
| 518 | + | } | |
| 519 | + | } | |
| 520 | + | ||
| 521 | + | // --------------------------------------------------------------------------- | |
| 445 | 522 | // Projects mode facets. | |
| 446 | 523 | // --------------------------------------------------------------------------- | |
| 447 | 524 |