From cbb0a51bca9e4e0d6a8fcee90465c93943f2a30e Mon Sep 17 00:00:00 2001 From: Xevion Date: Sat, 31 Jan 2026 00:26:41 -0600 Subject: [PATCH] refactor(terms): move term formatting from frontend to backend --- src/banner/models/terms.rs | 125 +++++++++++++++++++++ src/web/routes.rs | 71 ++++++++---- web/src/lib/api.ts | 10 +- web/src/lib/bindings/TermResponse.ts | 3 + web/src/lib/bindings/index.ts | 3 + web/src/lib/components/TermCombobox.svelte | 23 ++-- web/src/lib/term-format.test.ts | 64 ----------- web/src/lib/term-format.ts | 48 -------- web/src/routes/+page.svelte | 21 ++-- 9 files changed, 210 insertions(+), 158 deletions(-) create mode 100644 web/src/lib/bindings/TermResponse.ts delete mode 100644 web/src/lib/term-format.test.ts delete mode 100644 web/src/lib/term-format.ts diff --git a/src/banner/models/terms.rs b/src/banner/models/terms.rs index b83759e..fe21628 100644 --- a/src/banner/models/terms.rs +++ b/src/banner/models/terms.rs @@ -147,6 +147,37 @@ impl Term { }, } } + + /// URL-friendly slug, e.g. "spring-2026" + pub fn slug(&self) -> String { + format!("{}-{}", self.season.slug(), self.year) + } + + /// Parse a slug like "spring-2026" into a Term + pub fn from_slug(s: &str) -> Option { + let (season_str, year_str) = s.rsplit_once('-')?; + let season = Season::from_slug(season_str)?; + let year = year_str.parse::().ok()?; + if !VALID_YEARS.contains(&year) { + return None; + } + Some(Term { year, season }) + } + + /// Human-readable description, e.g. "Spring 2026" + pub fn description(&self) -> String { + format!("{} {}", self.season, self.year) + } + + /// Resolve a string that is either a term code ("202620") or a slug ("spring-2026") to a term code. + pub fn resolve_to_code(s: &str) -> Option { + // Try parsing as a 6-digit code first + if let Ok(term) = s.parse::() { + return Some(term.to_string()); + } + // Try parsing as a slug + Term::from_slug(s).map(|t| t.to_string()) + } } impl TermPoint { @@ -195,6 +226,25 @@ impl Season { Season::Summer => "30", } } + + /// Returns the lowercase slug for URL-friendly representation + pub fn slug(self) -> &'static str { + match self { + Season::Fall => "fall", + Season::Spring => "spring", + Season::Summer => "summer", + } + } + + /// Parse a slug like "spring", "summer", "fall" into a Season + pub fn from_slug(s: &str) -> Option { + match s { + "fall" => Some(Season::Fall), + "spring" => Some(Season::Spring), + "summer" => Some(Season::Summer), + _ => None, + } + } } impl std::fmt::Display for Season { @@ -445,4 +495,79 @@ mod tests { } ); } + + // --- Season::slug / from_slug --- + + #[test] + fn test_season_slug_roundtrip() { + for season in [Season::Fall, Season::Spring, Season::Summer] { + assert_eq!(Season::from_slug(season.slug()), Some(season)); + } + } + + #[test] + fn test_season_from_slug_invalid() { + assert_eq!(Season::from_slug("winter"), None); + assert_eq!(Season::from_slug(""), None); + assert_eq!(Season::from_slug("Spring"), None); // case-sensitive + } + + // --- Term::slug / from_slug --- + + #[test] + fn test_term_slug() { + let term = Term { + year: 2026, + season: Season::Spring, + }; + assert_eq!(term.slug(), "spring-2026"); + } + + #[test] + fn test_term_from_slug_roundtrip() { + for code in ["202510", "202520", "202530"] { + let term = Term::from_str(code).unwrap(); + let slug = term.slug(); + let parsed = Term::from_slug(&slug).unwrap(); + assert_eq!(parsed, term); + } + } + + #[test] + fn test_term_from_slug_invalid() { + assert_eq!(Term::from_slug("winter-2026"), None); + assert_eq!(Term::from_slug("spring"), None); + assert_eq!(Term::from_slug(""), None); + } + + // --- Term::description --- + + #[test] + fn test_term_description() { + let term = Term { + year: 2026, + season: Season::Spring, + }; + assert_eq!(term.description(), "Spring 2026"); + } + + // --- Term::resolve_to_code --- + + #[test] + fn test_resolve_to_code_from_code() { + assert_eq!(Term::resolve_to_code("202620"), Some("202620".to_string())); + } + + #[test] + fn test_resolve_to_code_from_slug() { + assert_eq!( + Term::resolve_to_code("spring-2026"), + Some("202620".to_string()) + ); + } + + #[test] + fn test_resolve_to_code_invalid() { + assert_eq!(Term::resolve_to_code("garbage"), None); + } } diff --git a/src/web/routes.rs b/src/web/routes.rs index 7b8a56c..3eb1e81 100644 --- a/src/web/routes.rs +++ b/src/web/routes.rs @@ -9,13 +9,13 @@ use axum::{ routing::{get, post, put}, }; -use crate::web::admin; -use crate::web::admin_rmp; use crate::web::admin_scraper; use crate::web::auth::{self, AuthConfig}; use crate::web::calendar; use crate::web::timeline; use crate::web::ws; +use crate::{data, web::admin}; +use crate::{data::models, web::admin_rmp}; #[cfg(feature = "embed-assets")] use axum::{ http::{HeaderMap, StatusCode, Uri}, @@ -468,7 +468,7 @@ pub struct CourseResponse { link_identifier: Option, is_section_linked: Option, part_of_term: Option, - meeting_times: Vec, + meeting_times: Vec, attributes: Vec, instructors: Vec, } @@ -505,10 +505,19 @@ pub struct CodeDescription { description: String, } +#[derive(Serialize, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export)] +pub struct TermResponse { + code: String, + slug: String, + description: String, +} + /// Build a `CourseResponse` from a DB course with pre-fetched instructor details. fn build_course_response( - course: &crate::data::models::Course, - instructors: Vec, + course: &models::Course, + instructors: Vec, ) -> CourseResponse { let instructors = instructors .into_iter() @@ -557,12 +566,20 @@ async fn search_courses( State(state): State, axum_extra::extract::Query(params): axum_extra::extract::Query, ) -> Result, (AxumStatusCode, String)> { + use crate::banner::models::terms::Term; + + let term_code = Term::resolve_to_code(¶ms.term).ok_or_else(|| { + ( + AxumStatusCode::BAD_REQUEST, + format!("Invalid term: {}", params.term), + ) + })?; let limit = params.limit.clamp(1, 100); let offset = params.offset.max(0); - let (courses, total_count) = crate::data::courses::search_courses( + let (courses, total_count) = data::courses::search_courses( &state.db_pool, - ¶ms.term, + &term_code, if params.subject.is_empty() { None } else { @@ -591,7 +608,7 @@ async fn search_courses( // Batch-fetch all instructors in a single query instead of N+1 let course_ids: Vec = courses.iter().map(|c| c.id).collect(); let mut instructor_map = - crate::data::courses::get_instructors_for_courses(&state.db_pool, &course_ids) + data::courses::get_instructors_for_courses(&state.db_pool, &course_ids) .await .unwrap_or_default(); @@ -616,7 +633,7 @@ async fn get_course( State(state): State, Path((term, crn)): Path<(String, String)>, ) -> Result, (AxumStatusCode, String)> { - let course = crate::data::courses::get_course_by_crn(&state.db_pool, &crn, &term) + let course = data::courses::get_course_by_crn(&state.db_pool, &crn, &term) .await .map_err(|e| { tracing::error!(error = %e, "Course lookup failed"); @@ -627,7 +644,7 @@ async fn get_course( })? .ok_or_else(|| (AxumStatusCode::NOT_FOUND, "Course not found".to_string()))?; - let instructors = crate::data::courses::get_course_instructors(&state.db_pool, course.id) + let instructors = data::courses::get_course_instructors(&state.db_pool, course.id) .await .unwrap_or_default(); Ok(Json(build_course_response(&course, instructors))) @@ -636,9 +653,10 @@ async fn get_course( /// `GET /api/terms` async fn get_terms( State(state): State, -) -> Result>, (AxumStatusCode, String)> { - let cache = state.reference_cache.read().await; - let term_codes = crate::data::courses::get_available_terms(&state.db_pool) +) -> Result>, (AxumStatusCode, String)> { + use crate::banner::models::terms::Term; + + let term_codes = data::courses::get_available_terms(&state.db_pool) .await .map_err(|e| { tracing::error!(error = %e, "Failed to get terms"); @@ -648,14 +666,15 @@ async fn get_terms( ) })?; - let terms: Vec = term_codes + let terms: Vec = term_codes .into_iter() - .map(|code| { - let description = cache - .lookup("term", &code) - .unwrap_or("Unknown Term") - .to_string(); - CodeDescription { code, description } + .filter_map(|code| { + let term: Term = code.parse().ok()?; + Some(TermResponse { + code, + slug: term.slug(), + description: term.description(), + }) }) .collect(); @@ -667,7 +686,15 @@ async fn get_subjects( State(state): State, Query(params): Query, ) -> Result>, (AxumStatusCode, String)> { - let rows = crate::data::courses::get_subjects_by_enrollment(&state.db_pool, ¶ms.term) + use crate::banner::models::terms::Term; + + let term_code = Term::resolve_to_code(¶ms.term).ok_or_else(|| { + ( + AxumStatusCode::BAD_REQUEST, + format!("Invalid term: {}", params.term), + ) + })?; + let rows = data::courses::get_subjects_by_enrollment(&state.db_pool, &term_code) .await .map_err(|e| { tracing::error!(error = %e, "Failed to get subjects"); @@ -696,7 +723,7 @@ async fn get_reference( if entries.is_empty() { // Fall back to DB query in case cache doesn't have this category drop(cache); - let rows = crate::data::reference::get_by_category(&category, &state.db_pool) + let rows = data::reference::get_by_category(&category, &state.db_pool) .await .map_err(|e| { tracing::error!(error = %e, category = %category, "Reference lookup failed"); diff --git a/web/src/lib/api.ts b/web/src/lib/api.ts index 7a2e223..8d2f05b 100644 --- a/web/src/lib/api.ts +++ b/web/src/lib/api.ts @@ -21,6 +21,7 @@ import type { SubjectResultEntry, SubjectSummary, SubjectsResponse, + TermResponse, TimeseriesPoint, TimeseriesResponse, TopCandidateResponse, @@ -51,13 +52,14 @@ export type { SubjectResultEntry, SubjectSummary, SubjectsResponse, + TermResponse, TimeseriesPoint, TimeseriesResponse, TopCandidateResponse, }; -// Semantic aliases — these all share the CodeDescription shape -export type Term = CodeDescription; +// Semantic aliases +export type Term = TermResponse; export type Subject = CodeDescription; export type ReferenceEntry = CodeDescription; @@ -268,8 +270,8 @@ export class BannerApiClient { return this.request("/terms"); } - async getSubjects(termCode: string): Promise { - return this.request(`/subjects?term=${encodeURIComponent(termCode)}`); + async getSubjects(term: string): Promise { + return this.request(`/subjects?term=${encodeURIComponent(term)}`); } async getReference(category: string): Promise { diff --git a/web/src/lib/bindings/TermResponse.ts b/web/src/lib/bindings/TermResponse.ts new file mode 100644 index 0000000..8748f94 --- /dev/null +++ b/web/src/lib/bindings/TermResponse.ts @@ -0,0 +1,3 @@ +// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. + +export type TermResponse = { code: string, slug: string, description: string, }; diff --git a/web/src/lib/bindings/index.ts b/web/src/lib/bindings/index.ts index 70f48bb..1de67e1 100644 --- a/web/src/lib/bindings/index.ts +++ b/web/src/lib/bindings/index.ts @@ -20,6 +20,9 @@ export type { SubjectDetailResponse } from "./SubjectDetailResponse"; export type { SubjectResultEntry } from "./SubjectResultEntry"; export type { SubjectSummary } from "./SubjectSummary"; export type { SubjectsResponse } from "./SubjectsResponse"; +export type { TermResponse } from "./TermResponse"; +export type { TimelineResponse } from "./TimelineResponse"; +export type { TimelineSlot } from "./TimelineSlot"; export type { TimeseriesPoint } from "./TimeseriesPoint"; export type { TimeseriesResponse } from "./TimeseriesResponse"; export type { TopCandidateResponse } from "./TopCandidateResponse"; diff --git a/web/src/lib/components/TermCombobox.svelte b/web/src/lib/components/TermCombobox.svelte index 3ffd676..4d5a8ee 100644 --- a/web/src/lib/components/TermCombobox.svelte +++ b/web/src/lib/components/TermCombobox.svelte @@ -16,12 +16,11 @@ let open = $state(false); let searchValue = $state(""); let containerEl = $state(null!); -const currentTermCode = $derived( - terms.find((t) => !t.description.includes("(View Only)"))?.code ?? "" -); +// The first term from the backend is the most current +const currentTermSlug = $derived(terms[0]?.slug ?? ""); const selectedLabel = $derived( - terms.find((t) => t.code === value)?.description ?? "Select term..." + terms.find((t) => t.slug === value)?.description ?? "Select term..." ); const filteredTerms = $derived.by(() => { @@ -29,8 +28,8 @@ const filteredTerms = $derived.by(() => { const matched = query === "" ? terms : terms.filter((t) => t.description.toLowerCase().includes(query)); - const current = matched.find((t) => t.code === currentTermCode); - const rest = matched.filter((t) => t.code !== currentTermCode); + const current = matched.find((t) => t.slug === currentTermSlug); + const rest = matched.filter((t) => t.slug !== currentTermSlug); return current ? [current, ...rest] : rest; }); @@ -100,22 +99,22 @@ $effect(() => {
- {#each filteredTerms as term, i (term.code)} - {#if i === 1 && term.code !== currentTermCode && filteredTerms[0]?.code === currentTermCode} + {#each filteredTerms as term, i (term.slug)} + {#if i === 1 && term.slug !== currentTermSlug && filteredTerms[0]?.slug === currentTermSlug}
{/if} {#snippet children({ selected })} {term.description} - {#if term.code === currentTermCode} + {#if term.slug === currentTermSlug} current {/if} diff --git a/web/src/lib/term-format.test.ts b/web/src/lib/term-format.test.ts deleted file mode 100644 index a080955..0000000 --- a/web/src/lib/term-format.test.ts +++ /dev/null @@ -1,64 +0,0 @@ -import { describe, expect, it } from "vitest"; -import { termToBanner, termToFriendly } from "./term-format"; - -describe("termToFriendly", () => { - it("converts spring term correctly", () => { - expect(termToFriendly("202610")).toBe("spring-26"); - expect(termToFriendly("202510")).toBe("spring-25"); - }); - - it("converts summer term correctly", () => { - expect(termToFriendly("202620")).toBe("summer-26"); - expect(termToFriendly("202520")).toBe("summer-25"); - }); - - it("converts fall term correctly", () => { - expect(termToFriendly("202630")).toBe("fall-26"); - expect(termToFriendly("202530")).toBe("fall-25"); - }); - - it("returns null for invalid codes", () => { - expect(termToFriendly("20261")).toBe(null); - expect(termToFriendly("2026100")).toBe(null); - expect(termToFriendly("202640")).toBe(null); // Invalid semester code - expect(termToFriendly("")).toBe(null); - }); -}); - -describe("termToBanner", () => { - it("converts spring term correctly", () => { - expect(termToBanner("spring-26")).toBe("202610"); - expect(termToBanner("spring-25")).toBe("202510"); - }); - - it("converts summer term correctly", () => { - expect(termToBanner("summer-26")).toBe("202620"); - expect(termToBanner("summer-25")).toBe("202520"); - }); - - it("converts fall term correctly", () => { - expect(termToBanner("fall-26")).toBe("202630"); - expect(termToBanner("fall-25")).toBe("202530"); - }); - - it("returns null for invalid formats", () => { - expect(termToBanner("winter-26")).toBe(null); - expect(termToBanner("spring26")).toBe(null); - expect(termToBanner("spring-2026")).toBe(null); - expect(termToBanner("26-spring")).toBe(null); - expect(termToBanner("")).toBe(null); - }); -}); - -describe("round-trip conversion", () => { - it("converts back and forth correctly", () => { - const bannerCodes = ["202610", "202620", "202630", "202510"]; - - for (const code of bannerCodes) { - const friendly = termToFriendly(code); - expect(friendly).not.toBeNull(); - const backToBanner = termToBanner(friendly!); - expect(backToBanner).toBe(code); - } - }); -}); diff --git a/web/src/lib/term-format.ts b/web/src/lib/term-format.ts deleted file mode 100644 index ae31e7d..0000000 --- a/web/src/lib/term-format.ts +++ /dev/null @@ -1,48 +0,0 @@ -/** - * Convert between Banner's internal term codes (e.g., "202620") and human-friendly format (e.g., "summer-26") - */ - -export type SemesterName = "spring" | "summer" | "fall"; - -const SEMESTER_CODES: Record = { - "10": "spring", - "20": "summer", - "30": "fall", -}; - -const SEMESTER_TO_CODE: Record = { - spring: "10", - summer: "20", - fall: "30", -}; - -/** - * Convert Banner term code (e.g., "202620") to friendly format (e.g., "summer-26") - */ -export function termToFriendly(bannerCode: string): string | null { - if (bannerCode.length !== 6) return null; - - const year = bannerCode.substring(0, 4); - const semesterCode = bannerCode.substring(4, 6); - const semester = SEMESTER_CODES[semesterCode]; - - if (!semester) return null; - - const shortYear = year.substring(2, 4); - return `${semester}-${shortYear}`; -} - -/** - * Convert friendly format (e.g., "summer-26") to Banner term code (e.g., "202620") - */ -export function termToBanner(friendly: string): string | null { - const match = friendly.match(/^(spring|summer|fall)-(\d{2})$/); - if (!match) return null; - - const [, semester, shortYear] = match; - const semesterCode = SEMESTER_TO_CODE[semester as SemesterName]; - if (!semesterCode) return null; - - const fullYear = `20${shortYear}`; - return `${fullYear}${semesterCode}`; -} diff --git a/web/src/routes/+page.svelte b/web/src/routes/+page.svelte index ea0601a..5185413 100644 --- a/web/src/routes/+page.svelte +++ b/web/src/routes/+page.svelte @@ -12,7 +12,6 @@ import Footer from "$lib/components/Footer.svelte"; import Pagination from "$lib/components/Pagination.svelte"; import SearchFilters from "$lib/components/SearchFilters.svelte"; import SearchStatus, { type SearchMeta } from "$lib/components/SearchStatus.svelte"; -import { termToBanner, termToFriendly } from "$lib/term-format"; import type { SortingState } from "@tanstack/table-core"; import { untrack } from "svelte"; @@ -21,10 +20,14 @@ let { data } = $props(); // Read initial state from URL params (intentionally captured once) const initialParams = untrack(() => new URLSearchParams(data.url.search)); -// Filter state - only set term from URL if present (no auto-default) +// The default term is the first one returned by the backend (most current) +const defaultTermSlug = data.terms[0]?.slug ?? ""; + +// Default to the first term when no URL param is present const urlTerm = initialParams.get("term"); -const bannerTerm = urlTerm ? (termToBanner(urlTerm) ?? "") : ""; -let selectedTerm = $state(bannerTerm); +let selectedTerm = $state( + urlTerm && data.terms.some((t) => t.slug === urlTerm) ? urlTerm : defaultTermSlug +); let selectedSubjects: string[] = $state(untrack(() => initialParams.getAll("subject"))); let query = $state(initialParams.get("q") ?? ""); let openOnly = $state(initialParams.get("open") === "true"); @@ -163,10 +166,6 @@ async function performSearch( sort.length > 0 ? (sort[0].desc ? "desc" : "asc") : undefined; const params = new URLSearchParams(); - const friendlyTerm = termToFriendly(term); - if (friendlyTerm) { - params.set("term", friendlyTerm); - } for (const s of subjects) { params.append("subject", s); } @@ -175,6 +174,12 @@ async function performSearch( if (off > 0) params.set("offset", String(off)); if (sortBy) params.set("sort_by", sortBy); if (sortDir && sortBy) params.set("sort_dir", sortDir); + + // Include term in URL only when it differs from the default or other params are active + const hasOtherParams = params.size > 0; + if (term !== defaultTermSlug || hasOtherParams) { + params.set("term", term); + } goto(`?${params.toString()}`, { replaceState: true, noScroll: true, keepFocus: true }); const t0 = performance.now();