diff --git a/frontend/src/components/TimePicker.vue b/frontend/src/components/TimePicker.vue index 6aa9b47..ba2f390 100644 --- a/frontend/src/components/TimePicker.vue +++ b/frontend/src/components/TimePicker.vue @@ -1,32 +1,40 @@ diff --git a/frontend/src/components/admin/ReservationArchiveDialog.vue b/frontend/src/components/admin/ReservationArchiveDialog.vue new file mode 100644 index 0000000..d631142 --- /dev/null +++ b/frontend/src/components/admin/ReservationArchiveDialog.vue @@ -0,0 +1,140 @@ + + + diff --git a/frontend/src/components/admin/ReservationCard.vue b/frontend/src/components/admin/ReservationCard.vue index 25e714e..08ba5e9 100644 --- a/frontend/src/components/admin/ReservationCard.vue +++ b/frontend/src/components/admin/ReservationCard.vue @@ -66,7 +66,7 @@ function move(status: ReservationStatus) { {{ $t(`reservation.status.${reservation.status}`) }} - {{ unitLabel(reservation.unit) }} + {{ unitLabel(reservation.unit) }}
@@ -87,7 +87,7 @@ function move(status: ReservationStatus) {
- + ('weekStart', { default: () => startOfWeek(new Date()) }) const selectedBike = ref('all') -function startOfWeek(date: Date) { - const start = new Date(date) - start.setHours(0, 0, 0, 0) - start.setDate(start.getDate() - ((start.getDay() + 6) % 7)) - return start -} - function shiftWeek(weeks: number) { - const next = new Date(weekStart.value) - next.setDate(next.getDate() + weeks * 7) - weekStart.value = next + weekStart.value = addDays(weekStart.value, weeks * 7) } const days = computed(() => Array.from({ length: 7 }, (_, i) => { - const day = new Date(weekStart.value) - day.setDate(day.getDate() + i) - return day + return addDays(weekStart.value, i) }), ) @@ -77,11 +69,10 @@ const rangeFormatter = computed( }), ) -const range = computed(() => { - const end = new Date(weekStart.value) - end.setDate(end.getDate() + 6) - return `${rangeFormatter.value.format(weekStart.value)} – ${rangeFormatter.value.format(end)}` -}) +const range = computed( + () => + `${rangeFormatter.value.format(weekStart.value)} – ${rangeFormatter.value.format(addDays(weekStart.value, 6))}`, +) const hours = computed(() => Array.from({ length: LAST_HOUR - FIRST_HOUR }, (_, i) => FIRST_HOUR + i), @@ -145,7 +136,6 @@ function blocksFor(day: Date, bike: Bike): Block[] {
{{ $t('calendar.title') }} - {{ $t('calendar.intro') }}
@@ -267,7 +257,9 @@ function blocksFor(day: Date, bike: Bike): Block[] { {{ $t(`reservation.status.${openedReservation.status}`) }} - {{ unitLabel(openedReservation.unit) }} + + {{ unitLabel(openedReservation.unit) }} + diff --git a/frontend/src/components/reservation/ReservationDetails.vue b/frontend/src/components/reservation/ReservationDetails.vue index b50d143..3337da9 100644 --- a/frontend/src/components/reservation/ReservationDetails.vue +++ b/frontend/src/components/reservation/ReservationDetails.vue @@ -2,14 +2,21 @@ /** * The read-only body of a reservation, shared by the admin card and the two * dialogs so the three never drift apart. + * + * Each field is a stacked block — caption above, value below — and the blocks + * flow in two even columns: side-by-side label/value pairs wrapped badly on a + * phone, and a single column made the cards far taller than they needed to be. */ -import { computed } from 'vue' +import { computed, ref } from 'vue' import { useI18n } from 'vue-i18n' import type { Bike, ReservationLike } from '@/utils/types' type Field = 'period' | 'bikes' | 'telegram' | 'people' | 'linka' | 'reason' +/** Past this many Linka Go accounts the list is folded behind a button */ +const LINKA_SHOWN = 5 + const props = withDefaults( defineProps<{ reservation: ReservationLike @@ -26,52 +33,107 @@ function shows(field: Field) { const { locale } = useI18n() +/** Names when we know the bike, `#id` when the list has not loaded yet */ const bikeNames = computed(() => - props.reservation.bikes - .map((id) => props.bikes.find((bike) => bike.id === id)?.name ?? `#${id}`) - .join(', '), + props.reservation.bikes.map((id) => props.bikes.find((bike) => bike.id === id)?.name ?? `#${id}`), ) -const formatter = computed( +// The e-mail address is already on the reservation as a Linka Go account or in +// the admin's own directory: the name is what tells the people apart here. +const peopleNames = computed(() => + (props.reservation.users ?? []).map((user) => `${user.firstname} ${user.name}`), +) + +const linkaEmails = computed(() => props.reservation.linka_emails ?? []) +const linkaExpanded = ref(false) +const linkaShown = computed(() => + linkaExpanded.value ? linkaEmails.value : linkaEmails.value.slice(0, LINKA_SHOWN), +) +const linkaHidden = computed(() => Math.max(0, linkaEmails.value.length - LINKA_SHOWN)) + +const dateFormatter = computed( () => new Intl.DateTimeFormat(locale.value === 'fr' ? 'fr-CH' : 'en-GB', { - dateStyle: 'short', + dateStyle: 'medium', timeStyle: 'short', }), ) function format(iso: string) { - return formatter.value.format(new Date(iso)) + return dateFormatter.value.format(new Date(iso)) } diff --git a/frontend/src/components/ui/dialog/DialogContent.vue b/frontend/src/components/ui/dialog/DialogContent.vue index b2bf24a..6d7cab6 100644 --- a/frontend/src/components/ui/dialog/DialogContent.vue +++ b/frontend/src/components/ui/dialog/DialogContent.vue @@ -1,4 +1,9 @@ @@ -39,7 +69,7 @@ const updatedAt = computed(() => {{ $t('admin.intro') }} - @@ -49,16 +79,22 @@ const updatedAt = computed(() => - + - +
diff --git a/frontend/src/views/CalendarView.vue b/frontend/src/views/CalendarView.vue index 1f6074f..1dfa6c3 100644 --- a/frontend/src/views/CalendarView.vue +++ b/frontend/src/views/CalendarView.vue @@ -4,12 +4,22 @@ * endpoint that carries no personal field. Anybody can look up when the bikes * are taken without being signed in. */ +import { ref } from 'vue' + import BikeCalendar from '@/components/reservation/BikeCalendar.vue' import { Skeleton } from '@/components/ui/skeleton' import { useBikes } from '@/services/api/bikes' import { useCalendarReservations } from '@/services/api/reservations' +import { addDays, startOfWeek } from '@/utils/week' -const { data: reservations, isPending, isError } = useCalendarReservations() +// The week on screen is the week fetched: changing week refetches that one +// rather than shipping every reservation to the browser. +const weekStart = ref(startOfWeek(new Date())) +const { + data: reservations, + isPending, + isError, +} = useCalendarReservations(() => ({ from: weekStart.value, to: addDays(weekStart.value, 7) })) const { data: bikes, isPending: bikesPending } = useBikes() @@ -24,6 +34,11 @@ const { data: bikes, isPending: bikesPending } = useBikes() {{ $t('calendar.load-error') }}

- +
diff --git a/frontend/src/views/MyReservationsView.vue b/frontend/src/views/MyReservationsView.vue index 99fed25..1041173 100644 --- a/frontend/src/views/MyReservationsView.vue +++ b/frontend/src/views/MyReservationsView.vue @@ -6,8 +6,11 @@ * What can still be changed depends on the status, and the backend applies the * same rule: a request is fully editable, an approved or ongoing reservation * only accepts a change of Linka Go accounts, and a final one is read-only. + * + * Three queries rather than one: the two live sections are short by nature, and + * the history — the part that keeps growing — is paged by the backend. */ -import { computed, ref } from 'vue' +import { computed, ref, watch } from 'vue' import ReservationDetails from '@/components/reservation/ReservationDetails.vue' import ReservationEditDialog from '@/components/reservation/ReservationEditDialog.vue' @@ -20,7 +23,24 @@ import { useBikes } from '@/services/api/bikes' import { useMyReservations } from '@/services/api/reservations' import { unitLabel, type Reservation, type ReservationStatus } from '@/utils/types' -const { data: reservations, isPending, isError } = useMyReservations() +/** How many past reservations one page of the history holds */ +const PAGE_SIZE = 5 +/** What is worth showing at once in the two live sections */ +const SECTION_LIMIT = 50 + +const requested = useMyReservations(() => ({ status: ['requested'], limit: SECTION_LIMIT })) +const active = useMyReservations(() => ({ + status: ['approved', 'ongoing'], + limit: SECTION_LIMIT, +})) + +const pastPage = ref(0) +const past = useMyReservations(() => ({ + status: ['refused', 'cancelled', 'archived'], + limit: PAGE_SIZE, + offset: pastPage.value * PAGE_SIZE, +})) + const { data: bikes } = useBikes() const BADGE_CLASS: Record = { @@ -32,25 +52,33 @@ const BADGE_CLASS: Record = { archived: 'bg-muted text-muted-foreground', } -const SECTIONS = [ - { key: 'requested', statuses: ['requested'] }, - { key: 'active', statuses: ['approved', 'ongoing'] }, - { key: 'past', statuses: ['refused', 'cancelled', 'archived'] }, -] as const +const sections = computed(() => [ + { key: 'requested', reservations: requested.data.value?.items ?? [] }, + { key: 'active', reservations: active.data.value?.items ?? [] }, + { key: 'past', reservations: past.data.value?.items ?? [] }, +]) -const sections = computed(() => - SECTIONS.map((section) => ({ - key: section.key, - reservations: (reservations.value ?? []).filter((r) => - (section.statuses as readonly string[]).includes(r.status), - ), - })), +const isPending = computed( + () => requested.isPending.value || active.isPending.value || past.isPending.value, ) +const isError = computed( + () => requested.isError.value || active.isError.value || past.isError.value, +) +const empty = computed( + () => !pastPage.value && sections.value.every((section) => !section.reservations.length), +) + +const pastTotal = computed(() => past.data.value?.total ?? 0) +const pastPages = computed(() => Math.max(1, Math.ceil(pastTotal.value / PAGE_SIZE))) +// A page that no longer exists — the last one emptied by a status change, say +watch(pastPages, (pages) => { + if (pastPage.value >= pages) pastPage.value = pages - 1 +}) /** Which reservation the edit dialog is on, by id */ const editing = ref(null) const editingReservation = computed(() => - (reservations.value ?? []).find((r) => r.id === editing.value), + sections.value.flatMap((section) => section.reservations).find((r) => r.id === editing.value), ) const dialogOpen = computed({ get: () => editing.value !== null, @@ -85,7 +113,7 @@ function editableEmails(reservation: Reservation) { {{ $t('my-reservations.load-error') }}

-

+

{{ $t('my-reservations.empty') }}

@@ -94,7 +122,7 @@ function editableEmails(reservation: Reservation) {

{{ $t(`my-reservations.${section.key}`) }} - ({{ section.reservations.length }}) + ({{ section.key === 'past' ? pastTotal : section.reservations.length }})

@@ -113,9 +141,7 @@ function editableEmails(reservation: Reservation) { {{ $t(`reservation.status.${reservation.status}`) }} - - {{ unitLabel(reservation.unit) }} - + {{ unitLabel(reservation.unit) }} + + {{ $t('pagination.page', { page: pastPage + 1, pages: pastPages }) }} + + diff --git a/src/api/reservations.rs b/src/api/reservations.rs index 2010023..aa48163 100644 --- a/src/api/reservations.rs +++ b/src/api/reservations.rs @@ -13,7 +13,11 @@ use aide::{ }, transform::TransformOperation, }; -use axum::{Json, extract::Path, http::StatusCode}; +use axum::{ + Json, + extract::{Path, Query}, + http::StatusCode, +}; use chrono::{DateTime, Utc}; use schemars::JsonSchema; use serde::Deserialize; @@ -28,8 +32,8 @@ use crate::{ models::{ bike::BikeId, reservation::{ - CalendarReservation, NewReservation, Reservation, ReservationEdit, - ReservationStatus, + CalendarReservation, NewReservation, Reservation, ReservationEdit, ReservationPage, + ReservationQuery, ReservationStatus, }, }, }, @@ -58,19 +62,73 @@ pub fn routes() -> ApiRouter { .api_route("/{id}/status", put_with(set_status, set_status_docs)) } +/// The listing filters, the way a query string carries them. +/// +/// Everything is optional and everything narrows, so the same route serves the +/// three sections of the admin page and the archive browser. The alternative — +/// handing the whole table to the browser and filtering it there — stops +/// working the day the archive is a few thousand rows long. +#[derive(Debug, Deserialize, JsonSchema)] +struct ReservationSearchParams { + /// Comma-separated statuses (`requested,approved`); left out means any + status: Option, + /// Free text: the unit, the telegram handle, the people, the Linka Go + /// addresses, the reason, or the number of a reservation + q: Option, + /// Keeps only what overlaps the window + from: Option>, + to: Option>, + limit: Option, + offset: Option, +} + +impl TryFrom for ReservationQuery { + type Error = String; + + fn try_from(params: ReservationSearchParams) -> Result { + let statuses = params + .status + .as_deref() + .unwrap_or_default() + .split(',') + .map(str::trim) + .filter(|status| !status.is_empty()) + .map(|status| status.parse::()) + .collect::, _>>() + .map_err(|err| err.to_string())?; + + Ok(ReservationQuery { + statuses, + // Set by the handler that needs it, from the session + involving: None, + search: params.q, + from: params.from, + to: params.to, + limit: params.limit.unwrap_or(ReservationQuery::DEFAULT_LIMIT), + offset: params.offset.unwrap_or(0), + }) + } +} + #[axum::debug_handler] async fn get_reservations( ac: AppController, -) -> Result>, (StatusCode, String)> { - match admin(ac)?.get_reservations().await { - Ok(reservations) => Ok(Json(reservations)), + Query(params): Query, +) -> Result, (StatusCode, String)> { + let query = ReservationQuery::try_from(params).map_err(|err| (StatusCode::BAD_REQUEST, err))?; + match admin(ac)?.search_reservations(query).await { + Ok(page) => Ok(Json(page)), Err(err) => unexpected_error("get_reservations", err), } } fn get_reservations_docs(op: TransformOperation) -> TransformOperation { op.tag("Reservations") - .summary("Get every reservation") + .summary("Get a page of the reservations, filtered") + .description( + "The filtering, the text search and the paging all happen in the database: no caller ever receives the whole table.", + ) + .response_with::<400, (), _>(desc("A status in the filter is not a known one")) .response_with::<403, (), _>(admin_desc) } @@ -117,11 +175,19 @@ fn create_reservation_docs(op: TransformOperation) -> TransformOperation { /// The availability calendar, open to everybody: when the bikes are taken and by /// which association, with nothing personal attached. +/// The window a calendar draws, so only that window is read. +#[derive(Debug, Deserialize, JsonSchema)] +struct CalendarWindow { + from: Option>, + to: Option>, +} + #[axum::debug_handler] async fn get_calendar_reservations( aac: AnonAppController, + Query(window): Query, ) -> Result>, (StatusCode, String)> { - match aac.get_calendar_reservations().await { + match aac.get_calendar_reservations(window.from, window.to).await { Ok(reservations) => Ok(Json(reservations)), Err(err) => unexpected_error("get_calendar_reservations", err), } @@ -131,7 +197,8 @@ fn get_calendar_reservations_docs(op: TransformOperation) -> TransformOperation op.tag("Reservations") .summary("Get the approved and ongoing reservations, for the public calendar") .description( - "No session needed, and no personal field travels: the telegram handle, \ + "Give `from` and `to` to read one week rather than the whole table. No session \ + needed, and no personal field travels: the telegram handle, \ the people, the Linka Go addresses and the reason are left out.", ) } @@ -141,21 +208,25 @@ fn get_calendar_reservations_docs(op: TransformOperation) -> TransformOperation #[axum::debug_handler] async fn get_my_reservations( ac: AppController, -) -> Result>, (StatusCode, String)> { - match ac.get_my_reservations().await { - Ok(reservations) => Ok(Json(reservations)), + Query(params): Query, +) -> Result, (StatusCode, String)> { + let query = ReservationQuery::try_from(params).map_err(|err| (StatusCode::BAD_REQUEST, err))?; + match ac.get_my_reservations(query).await { + Ok(page) => Ok(Json(page)), Err(err) => unexpected_error("get_my_reservations", err), } } fn get_my_reservations_docs(op: TransformOperation) -> TransformOperation { op.tag("Reservations") - .summary("Get the reservations the session user is part of") + .summary("Get a page of the reservations the session user is part of") .description( - "An address listed among the Linka Go accounts is enough, which is how \ - somebody added before they ever logged in finds the reservation waiting \ - for them.", + "Takes the same filters as the listing, and answers the same page. An \ + address listed among the Linka Go accounts is enough to be part of a \ + reservation, which is how somebody added before they ever logged in \ + finds the one waiting for them.", ) + .response_with::<400, (), _>(desc("A status in the filter is not a known one")) } /// Only what the admin page lets somebody change. The unit, the requester, the diff --git a/src/core/controller/reservations.rs b/src/core/controller/reservations.rs index 09a8016..fa42189 100644 --- a/src/core/controller/reservations.rs +++ b/src/core/controller/reservations.rs @@ -1,3 +1,4 @@ +use chrono::{DateTime, Utc}; use thiserror::Error; use crate::core::{ @@ -5,8 +6,8 @@ use crate::core::{ models::{ bike::BikeStatus, reservation::{ - CalendarReservation, NewReservation, NewReservationUnit, Reservation, ReservationEdit, - ReservationId, ReservationStatus, + CalendarReservation, Involvement, NewReservation, NewReservationUnit, Reservation, + ReservationEdit, ReservationId, ReservationPage, ReservationQuery, ReservationStatus, }, unit::UnitId, }, @@ -15,8 +16,13 @@ use crate::core::{ /// Reading the reservations needs no session: the calendar is public. impl AnonAppController { - pub async fn get_reservations(&self) -> Result, ControllerError> { - self.db.get_reservations().await.map_err(Into::into) + /// One page of the reservations matching `query`. Reading the whole list is + /// an admin action, which the API layer is what enforces. + pub async fn search_reservations( + &self, + query: ReservationQuery, + ) -> Result { + self.db.search_reservations(query).await.map_err(Into::into) } pub async fn get_unit_reservations( @@ -33,35 +39,50 @@ impl AnonAppController { self.db.get_reservation(id).await.map_err(Into::into) } - /// What the availability calendar shows: the reservations that actually hold - /// a bike, stripped of everything personal. Reading it needs no session. + /// What the availability calendar shows: the reservations that actually + /// hold a bike over the window asked for, stripped of everything personal. + /// Reading it needs no session. + /// + /// The window is what keeps this cheap: a calendar draws one week, so one + /// week is what the database returns. pub async fn get_calendar_reservations( &self, + from: Option>, + to: Option>, ) -> Result, ControllerError> { - Ok(self + let page = self .db - .get_reservations() - .await? - .into_iter() - .filter(|reservation| { - matches!( - reservation.status, - ReservationStatus::Approved | ReservationStatus::Ongoing - ) + .search_reservations(ReservationQuery { + statuses: vec![ReservationStatus::Approved, ReservationStatus::Ongoing], + from, + to, + limit: ReservationQuery::MAX_LIMIT, + ..Default::default() }) - .map(Into::into) - .collect()) + .await?; + Ok(page.items.into_iter().map(Into::into).collect()) } } /// Filing a request is done in one's own name: the requester is the session /// user, never something the client gets to choose. impl AppController { - /// Everything the session user is part of, whichever way. - pub async fn get_my_reservations(&self) -> Result, ControllerError> { + /// One page of what the session user is part of, whichever way. The + /// involvement is taken from the session, never from the query: this is the + /// route on which somebody could otherwise read another person's list. + pub async fn get_my_reservations( + &self, + query: ReservationQuery, + ) -> Result { let user = self.user(); self.db - .get_involved_reservations(user.id, &user.email) + .search_reservations(ReservationQuery { + involving: Some(Involvement { + user: user.id, + email: user.email.clone(), + }), + ..query + }) .await .map_err(Into::into) } diff --git a/src/core/models/reservation.rs b/src/core/models/reservation.rs index 201e250..ee77935 100644 --- a/src/core/models/reservation.rs +++ b/src/core/models/reservation.rs @@ -37,12 +37,48 @@ impl ReservationStatus { ) } + /// The spelling shared by the JSON, the Postgres enum and the query + /// parameters — one name, so no mapping table has to be kept in step. + pub fn as_str(self) -> &'static str { + use ReservationStatus::*; + match self { + Requested => "requested", + Refused => "refused", + Approved => "approved", + Cancelled => "cancelled", + Ongoing => "ongoing", + Archived => "archived", + } + } + pub fn is_final(self) -> bool { use ReservationStatus::*; matches!(self, Refused | Cancelled | Archived) } } +impl std::str::FromStr for ReservationStatus { + type Err = UnknownStatus; + + fn from_str(value: &str) -> Result { + use ReservationStatus::*; + match value { + "requested" => Ok(Requested), + "refused" => Ok(Refused), + "approved" => Ok(Approved), + "cancelled" => Ok(Cancelled), + "ongoing" => Ok(Ongoing), + "archived" => Ok(Archived), + other => Err(UnknownStatus(other.to_owned())), + } + } +} + +/// A status spelling that is not one of the six +#[derive(Debug, thiserror::Error)] +#[error("unknown reservation status: {0}")] +pub struct UnknownStatus(pub String); + /// The unit a reservation is filed for. /// /// Either one we know — a Whiskey group, with its row — or a plain name the @@ -148,6 +184,83 @@ impl From for CalendarReservation { } } +/// The three ways of being part of a reservation: having filed it, being named +/// on it, or having one's address among the Linka Go accounts. +/// +/// The address is what binds a reservation to somebody who had never logged in +/// when it was filed — nothing is written at login time, the match is made when +/// the list is read. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Involvement { + pub user: UserId, + pub email: String, +} + +/// What a listing asks the database for. +/// +/// Everything here narrows: an empty `statuses` means any status, `search` left +/// out means no text filter. It exists so that a page showing a handful of rows +/// never receives the whole table — the archive alone will outgrow that — and so +/// that the narrowing happens once, in the index, instead of in every browser. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ReservationQuery { + pub statuses: Vec, + /// Keeps only the reservations one person is part of. `None` is "anybody's", + /// which is what the admin listing asks for. + pub involving: Option, + /// Free text, matched against the unit, the telegram handle, the people on + /// the reservation, the Linka Go addresses, the reason and the id + pub search: Option, + /// Keeps only the reservations overlapping the window + pub from: Option>, + pub to: Option>, + pub limit: i64, + pub offset: i64, +} + +impl ReservationQuery { + pub const DEFAULT_LIMIT: i64 = 20; + /// A page nobody can read past. It caps what one request may cost, whatever + /// the client asks for. + pub const MAX_LIMIT: i64 = 200; + + /// The same query with the parts a client controls brought back into range: + /// a limit outside `1..=MAX_LIMIT` is clamped, a negative offset is zero, + /// and a blank search is no search at all. + pub fn sanitised(mut self) -> Self { + self.limit = self.limit.clamp(1, Self::MAX_LIMIT); + self.offset = self.offset.max(0); + self.search = self + .search + .map(|search| search.trim().to_owned()) + .filter(|search| !search.is_empty()); + self + } +} + +impl Default for ReservationQuery { + fn default() -> Self { + ReservationQuery { + statuses: Vec::new(), + involving: None, + search: None, + from: None, + to: None, + limit: Self::DEFAULT_LIMIT, + offset: 0, + } + } +} + +/// One page of a listing, with the size of the whole so the caller can page +/// through it without asking twice. +#[derive(Debug, Serialize, Deserialize, Clone, JsonSchema)] +pub struct ReservationPage { + pub items: Vec, + /// How many reservations match the query, ignoring `limit` and `offset` + pub total: i64, +} + #[derive(Debug, Serialize, Deserialize, Clone, JsonSchema, PartialEq, Eq)] pub struct NewReservation { pub unit: NewReservationUnit, @@ -218,6 +331,18 @@ impl ReservationEdit { mod tests { use super::*; + #[test] + fn every_status_survives_a_round_trip_through_its_name() { + use ReservationStatus::*; + for status in [Requested, Refused, Approved, Cancelled, Ongoing, Archived] { + assert_eq!( + status.as_str().parse::().unwrap(), + status + ); + } + assert!("started".parse::().is_err()); + } + #[test] fn telegram_handles_match_the_database_constraint() { assert!(telegram_valid("@milan_hyenne")); diff --git a/src/core/repositories/reservations_repository.rs b/src/core/repositories/reservations_repository.rs index 875fd0a..b8e3641 100644 --- a/src/core/repositories/reservations_repository.rs +++ b/src/core/repositories/reservations_repository.rs @@ -3,7 +3,8 @@ use async_trait::async_trait; use crate::core::{ models::{ reservation::{ - NewReservation, Reservation, ReservationEdit, ReservationId, ReservationStatus, + NewReservation, Reservation, ReservationEdit, ReservationId, ReservationPage, + ReservationQuery, ReservationStatus, }, unit::UnitId, user::UserId, @@ -13,23 +14,22 @@ use crate::core::{ #[async_trait] pub trait ReservationsRepository { - async fn get_reservations(&self) -> Result, RepositoryError>; + /// One page of the reservations matching `query`, most recent first, with + /// the size of the whole match. + /// + /// There is deliberately no "give me every reservation": the table only + /// grows, and the narrowing belongs here, where the indexes are, rather + /// than in a browser that would have to download the archive to hide it. + async fn search_reservations( + &self, + query: ReservationQuery, + ) -> Result; async fn get_unit_reservations( &self, unit: UnitId, ) -> Result, RepositoryError>; async fn get_reservation(&self, id: ReservationId) -> Result; - /// Everything the user is part of: what they filed, what they were added - /// to, and what lists their address among the Linka Go accounts. The email - /// is what binds a reservation to somebody who had never logged in when it - /// was filed. - async fn get_involved_reservations( - &self, - user: UserId, - email: &str, - ) -> Result, RepositoryError>; - /// Always stored as `Requested`: the state machine starts here. The /// requester comes from the session, not from the request body. async fn create_reservation( diff --git a/src/services/database/reservations.rs b/src/services/database/reservations.rs index 7f08af2..e121c47 100644 --- a/src/services/database/reservations.rs +++ b/src/services/database/reservations.rs @@ -13,7 +13,7 @@ use crate::{ models::{ reservation::{ NewReservation, NewReservationUnit, Reservation, ReservationEdit, ReservationId, - ReservationStatus, ReservationUnit, + ReservationPage, ReservationQuery, ReservationStatus, ReservationUnit, }, unit::{Unit, UnitId}, user::UserId, @@ -107,6 +107,46 @@ impl TryFrom for Reservation { } } +/// The page query carries one extra column — how many rows the filter matches +/// in all, from a window function — so it needs its own row type. Everything +/// else is the shared one, which the conversion below hands back. +struct ReservationPageRowDB { + pub id: i32, + pub unit_id: Option, + pub unit_name: Option, + pub unit_label: Option, + pub start_time: DateTime, + pub end_time: DateTime, + pub requester_id: i32, + pub telegram: String, + pub description: String, + pub status: ReservationStatusDB, + pub linka_emails: Vec, + pub users: Value, + pub bikes: Vec, + pub total: i64, +} + +impl From for ReservationDB { + fn from(value: ReservationPageRowDB) -> Self { + ReservationDB { + id: value.id, + unit_id: value.unit_id, + unit_name: value.unit_name, + unit_label: value.unit_label, + start_time: value.start_time, + end_time: value.end_time, + requester_id: value.requester_id, + telegram: value.telegram, + description: value.description, + status: value.status, + linka_emails: value.linka_emails, + users: value.users, + bikes: value.bikes, + } + } +} + impl SqlxDatabase { async fn set_reservation_links( tx: &mut sqlx::Transaction<'_, sqlx::Postgres>, @@ -170,11 +210,37 @@ fn trim_emails(emails: &[String]) -> Vec { .collect() } +/// Wraps the search text for `ILIKE`, escaping what the pattern language would +/// otherwise read as a wildcard — an address contains `_` often enough that +/// leaving it alone would quietly widen the search. +fn like_pattern(search: &str) -> String { + let escaped = search + .replace('\\', "\\\\") + .replace('%', "\\%") + .replace('_', "\\_"); + format!("%{escaped}%") +} + #[async_trait] impl ReservationsRepository for SqlxDatabase { - async fn get_reservations(&self) -> Result, RepositoryError> { - Ok(query_as!( - ReservationDB, + async fn search_reservations( + &self, + query: ReservationQuery, + ) -> Result { + let query = query.sanitised(); + // Every filter is "the parameter is NULL, or it matches": one prepared + // statement serves every combination the admin page can ask for. + let statuses = (!query.statuses.is_empty()).then(|| { + query + .statuses + .iter() + .map(|status| status.as_str().to_owned()) + .collect::>() + }); + let pattern = query.search.as_deref().map(like_pattern); + + let rows = query_as!( + ReservationPageRowDB, r#"SELECT r.id, r.unit_id, @@ -203,16 +269,69 @@ impl ReservationsRepository for SqlxDatabase { ARRAY( SELECT bike_id FROM reservations_bikes WHERE reservation_id = r.id ORDER BY bike_id - ) AS "bikes!" + ) AS "bikes!", + -- The size of the whole match, alongside the page itself: the + -- caller can show "42 results" without a second round trip + COUNT(*) OVER () AS "total!" FROM reservations r LEFT JOIN units un ON un.id = r.unit_id - ORDER BY r.start_time DESC"# + WHERE ($1::text[] IS NULL OR r.status::text = ANY($1)) + -- The three ways of being part of a reservation. The last one is + -- what lets somebody added by address see it the first time they + -- log in, without anything having to be written at login time. + AND ($8::integer IS NULL + OR r.requester_id = $8 + OR EXISTS ( + SELECT 1 FROM reservations_users ru + WHERE ru.reservation_id = r.id AND ru.user_id = $8 + ) + OR EXISTS ( + SELECT 1 FROM unnest(r.linka_emails) AS listed + WHERE lower(listed) = lower($9) + )) + -- Overlap with the window, not containment: a reservation running + -- across the displayed week belongs to it + AND ($2::timestamptz IS NULL OR r.end_time > $2) + AND ($3::timestamptz IS NULL OR r.start_time < $3) + AND ($4::text IS NULL + OR COALESCE(un."name", r.unit_label, '') ILIKE $4 + OR r.telegram ILIKE $4 + OR r."description" ILIKE $4 + OR array_to_string(r.linka_emails, ' ') ILIKE $4 + -- Typing the number of a reservation is how an admin looks + -- one up from a mail or a chat + OR r.id::text = $5 + OR EXISTS ( + SELECT 1 FROM reservations_users ru + JOIN users u ON u.id = ru.user_id + WHERE ru.reservation_id = r.id + AND u.firstname || ' ' || u."name" || ' ' || u.email ILIKE $4 + )) + ORDER BY r.start_time DESC, r.id DESC + LIMIT $6 OFFSET $7"#, + statuses.as_deref(), + query.from, + query.to, + pattern, + query.search, + query.limit, + query.offset, + query.involving.as_ref().map(|who| who.user), + query.involving.as_ref().map(|who| who.email.as_str()), ) .fetch_all(&self.pool) - .await? - .into_iter() - .map(TryInto::try_into) - .collect::, _>>()?) + .await?; + + // The window function only rides along on the rows; an empty page is + // an empty match, or a page past the end of one. + let total = rows.first().map_or(0, |row| row.total); + Ok(ReservationPage { + items: rows + .into_iter() + .map(|row| ReservationDB::from(row).try_into()) + .collect::, _>>()?, + total, + }) } async fn get_unit_reservations( @@ -263,67 +382,6 @@ impl ReservationsRepository for SqlxDatabase { .collect::, _>>()?) } - async fn get_involved_reservations( - &self, - user: UserId, - email: &str, - ) -> Result, RepositoryError> { - Ok(query_as!( - ReservationDB, - r#"SELECT - r.id, - r.unit_id, - -- `?` forces the nullability sqlx cannot infer: `units.name` is - -- NOT NULL, but the LEFT JOIN makes it null for a free label - un."name" AS "unit_name?", - r.unit_label, - r.start_time, - r.end_time, - r.requester_id, - r.telegram, - r."description", - r.linka_emails, - r.status AS "status: ReservationStatusDB", - COALESCE(( - SELECT json_agg(json_build_object( - 'id', u.id, - 'firstname', u.firstname, - 'name', u."name", - 'email', u.email - ) ORDER BY u."name", u.firstname) - FROM reservations_users ru - JOIN users u ON u.id = ru.user_id - WHERE ru.reservation_id = r.id - ), '[]'::json) AS "users!", - ARRAY( - SELECT bike_id FROM reservations_bikes - WHERE reservation_id = r.id ORDER BY bike_id - ) AS "bikes!" - FROM reservations r - LEFT JOIN units un ON un.id = r.unit_id - -- Three ways to be part of a reservation. The last one is what - -- lets somebody added by address see it the first time they log in, - -- without anything having to be written at login time. - WHERE r.requester_id = $1 - OR EXISTS ( - SELECT 1 FROM reservations_users ru - WHERE ru.reservation_id = r.id AND ru.user_id = $1 - ) - OR EXISTS ( - SELECT 1 FROM unnest(r.linka_emails) AS listed - WHERE lower(listed) = lower($2) - ) - ORDER BY r.start_time DESC"#, - user, - email - ) - .fetch_all(&self.pool) - .await? - .into_iter() - .map(TryInto::try_into) - .collect::, _>>()?) - } - async fn get_reservation(&self, id: ReservationId) -> Result { Ok(query_as!( ReservationDB, @@ -484,3 +542,17 @@ impl ReservationsRepository for SqlxDatabase { Ok(()) } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn like_patterns_escape_the_wildcards() { + assert_eq!(like_pattern("milan"), "%milan%"); + // Underscores are ordinary in an address; they must not match anything + assert_eq!(like_pattern("a_b"), "%a\\_b%"); + assert_eq!(like_pattern("100%"), "%100\\%%"); + assert_eq!(like_pattern("a\\b"), "%a\\\\b%"); + } +}