From 40f97f18a98eed185cbd4877af58527d64edd6d2 Mon Sep 17 00:00:00 2001 From: philipp Date: Wed, 22 May 2024 00:13:23 +0200 Subject: [PATCH] clean code with clippy --- src/model/logbook.rs | 4 +- src/model/planned_event.rs | 44 ++++++++-------- src/model/trip.rs | 93 +++++++++++++++++++++------------ src/model/user.rs | 4 +- src/model/waterlevel.rs | 25 ++++----- src/scheduled/waterlevel.rs | 36 +++++++------ src/scheduled/weather.rs | 14 +++-- src/tera/admin/planned_event.rs | 22 ++++---- src/tera/auth.rs | 5 +- src/tera/cox.rs | 24 ++++----- src/tera/log.rs | 4 +- 11 files changed, 151 insertions(+), 124 deletions(-) diff --git a/src/model/logbook.rs b/src/model/logbook.rs index 09713f2..797a2fc 100644 --- a/src/model/logbook.rs +++ b/src/model/logbook.rs @@ -118,7 +118,7 @@ pub enum LogbookCreateError { BoatLocked, BoatNotFound, TooManyRowers(usize, usize), - RowerAlreadyOnWater(User), + RowerAlreadyOnWater(Box), RowerCreateError(i64, String), ArrivalNotAfterDeparture, SteeringPersonNotInRowers, @@ -386,7 +386,7 @@ ORDER BY departure DESC let user = User::find_by_id(db, *rower as i32).await.unwrap(); if user.on_water(db).await { - return Err(LogbookCreateError::RowerAlreadyOnWater(user)); + return Err(LogbookCreateError::RowerAlreadyOnWater(Box::new(user))); } } diff --git a/src/model/planned_event.rs b/src/model/planned_event.rs index 2db5692..3a7b4bc 100644 --- a/src/model/planned_event.rs +++ b/src/model/planned_event.rs @@ -98,6 +98,15 @@ FROM trip WHERE planned_event_id = ? } } +pub struct EventUpdate<'a> { + pub name: &'a str, + pub planned_amount_cox: i32, + pub max_people: i32, + pub notes: Option<&'a str>, + pub always_show: bool, + pub is_locked: bool, +} + impl PlannedEvent { pub async fn find_by_id(db: &SqlitePool, id: i64) -> Option { sqlx::query_as!( @@ -207,20 +216,11 @@ INNER JOIN trip_details ON planned_event.trip_details_id = trip_details.id", } //TODO: create unit test - pub async fn update( - &self, - db: &SqlitePool, - name: &str, - planned_amount_cox: i32, - max_people: i32, - notes: Option<&str>, - always_show: bool, - is_locked: bool, - ) { + pub async fn update(&self, db: &SqlitePool, update: &EventUpdate<'_>) { sqlx::query!( "UPDATE planned_event SET name = ?, planned_amount_cox = ? WHERE id = ?", - name, - planned_amount_cox, + update.name, + update.planned_amount_cox, self.id ) .execute(db) @@ -229,10 +229,10 @@ INNER JOIN trip_details ON planned_event.trip_details_id = trip_details.id", sqlx::query!( "UPDATE trip_details SET max_people = ?, notes = ?, always_show = ?, is_locked = ? WHERE id = ?", - max_people, - notes, - always_show, - is_locked, + update.max_people, + update.notes, + update.always_show, + update.is_locked, self.trip_details_id ) .execute(db) @@ -241,16 +241,18 @@ INNER JOIN trip_details ON planned_event.trip_details_id = trip_details.id", } pub async fn delete(&self, db: &SqlitePool) -> Result<(), String> { - if Registration::all_rower(db, self.trip_details_id) + if !Registration::all_rower(db, self.trip_details_id) .await - .len() - > 0 + .is_empty() { return Err( "Event kann nicht gelöscht werden, weil mind. 1 Ruderer angemeldet ist.".into(), ); } - if Registration::all_cox(db, self.trip_details_id).await.len() > 0 { + if !Registration::all_cox(db, self.trip_details_id) + .await + .is_empty() + { return Err( "Event kann nicht gelöscht werden, weil mind. 1 Steuerperson angemeldet ist." .into(), @@ -326,7 +328,7 @@ mod test { let pool = testdb!(); let planned_event = PlannedEvent::find_by_id(&pool, 1).await.unwrap(); - planned_event.delete(&pool).await; + planned_event.delete(&pool).await.unwrap(); let res = PlannedEvent::get_for_day(&pool, NaiveDate::from_ymd_opt(1970, 1, 1).unwrap()).await; diff --git a/src/model/trip.rs b/src/model/trip.rs index 704c4b9..0c0f2ef 100644 --- a/src/model/trip.rs +++ b/src/model/trip.rs @@ -34,6 +34,16 @@ pub struct TripWithUserAndType { trip_type: Option, } +pub struct TripUpdate<'a> { + pub cox: &'a CoxUser, + pub trip: &'a Trip, + pub max_people: i32, + pub notes: Option<&'a str>, + pub trip_type: Option, //TODO: Move to `TripType` + pub always_show: bool, + pub is_locked: bool, +} + impl TripWithUserAndType { pub async fn from(db: &SqlitePool, trip: Trip) -> Self { let mut trip_type = None; @@ -177,40 +187,36 @@ WHERE day=? /// Cox decides to update own trip. pub async fn update_own( db: &SqlitePool, - cox: &CoxUser, - trip: &Trip, - max_people: i32, - notes: Option<&str>, - trip_type: Option, //TODO: Move to `TripType` - always_show: bool, - is_locked: bool, + update: &TripUpdate<'_>, ) -> Result<(), TripUpdateError> { - if !trip.is_trip_from_user(cox.id) { + if !update.trip.is_trip_from_user(update.cox.id) { return Err(TripUpdateError::NotYourTrip); } - let Some(trip_details_id) = trip.trip_details_id else { + let Some(trip_details_id) = update.trip.trip_details_id else { return Err(TripUpdateError::TripDetailsDoesNotExist); //TODO: Remove? }; sqlx::query!( "UPDATE trip_details SET max_people = ?, notes = ?, trip_type_id = ?, always_show = ?, is_locked = ? WHERE id = ?", - max_people, - notes, - trip_type, - always_show, - is_locked, + update.max_people, + update.notes, + update.trip_type, + update.always_show, + update.is_locked, trip_details_id ) .execute(db) .await .unwrap(); //Okay, as trip_details can only be created with proper DB backing - if max_people == 0 { - let rowers = TripWithUserAndType::from(db, trip.clone()).await.rower; + if update.max_people == 0 { + let rowers = TripWithUserAndType::from(db, update.trip.clone()) + .await + .rower; for user in rowers { if let Some(user) = User::find_by_name(db, &user.name).await { - let notes = match notes { + let notes = match update.notes { Some(n) if !n.is_empty() => n, _ => ".", }; @@ -220,7 +226,10 @@ WHERE day=? &user, &format!( "Die Ausfahrt von {} am {} um {} wurde abgesagt{}", - cox.user.name, trip.day, trip.planned_starting_time, notes + update.cox.user.name, + update.trip.day, + update.trip.planned_starting_time, + notes ), "Absage Ausfahrt", None, @@ -349,7 +358,7 @@ mod test { use crate::{ model::{ planned_event::PlannedEvent, - trip::TripDeleteError, + trip::{self, TripDeleteError}, tripdetails::TripDetails, user::{CoxUser, User}, usertrip::UserTrip, @@ -434,11 +443,17 @@ mod test { let trip = Trip::find_by_id(&pool, 1).await.unwrap(); - assert!( - Trip::update_own(&pool, &cox, &trip, 10, None, None, false, false) - .await - .is_ok() - ); + let update = trip::TripUpdate { + cox: &cox, + trip: &trip, + max_people: 10, + notes: None, + trip_type: None, + always_show: false, + is_locked: false, + }; + + assert!(Trip::update_own(&pool, &update).await.is_ok()); let trip = Trip::find_by_id(&pool, 1).await.unwrap(); assert_eq!(trip.max_people, 10); @@ -457,11 +472,16 @@ mod test { let trip = Trip::find_by_id(&pool, 1).await.unwrap(); - assert!( - Trip::update_own(&pool, &cox, &trip, 10, None, Some(1), false, false) - .await - .is_ok() - ); + let update = trip::TripUpdate { + cox: &cox, + trip: &trip, + max_people: 10, + notes: None, + trip_type: Some(1), + always_show: false, + is_locked: false, + }; + assert!(Trip::update_own(&pool, &update).await.is_ok()); let trip = Trip::find_by_id(&pool, 1).await.unwrap(); assert_eq!(trip.max_people, 10); @@ -481,11 +501,16 @@ mod test { let trip = Trip::find_by_id(&pool, 1).await.unwrap(); - assert!( - Trip::update_own(&pool, &cox, &trip, 10, None, None, false, false) - .await - .is_err() - ); + let update = trip::TripUpdate { + cox: &cox, + trip: &trip, + max_people: 10, + notes: None, + trip_type: None, + always_show: false, + is_locked: false, + }; + assert!(Trip::update_own(&pool, &update).await.is_err()); assert_eq!(trip.max_people, 1); } diff --git a/src/model/user.rs b/src/model/user.rs index a0a1c1b..799e16a 100644 --- a/src/model/user.rs +++ b/src/model/user.rs @@ -80,7 +80,7 @@ pub enum LoginError { NotACox, NotATech, GuestNotAllowed, - NoPasswordSet(User), + NoPasswordSet(Box), DeserializationError, } @@ -706,7 +706,7 @@ ORDER BY last_access DESC Err(LoginError::InvalidAuthenticationCombo) } else { info!("User {name} has no PW set"); - Err(LoginError::NoPasswordSet(user)) + Err(LoginError::NoPasswordSet(Box::new(user))) } } diff --git a/src/model/waterlevel.rs b/src/model/waterlevel.rs index a064a21..351b36a 100644 --- a/src/model/waterlevel.rs +++ b/src/model/waterlevel.rs @@ -17,6 +17,17 @@ pub struct Waterlevel { pub tumittel: i64, } +pub struct Create { + pub day: NaiveDate, + pub time: String, + pub max: i64, + pub min: i64, + pub mittel: i64, + pub tumax: i64, + pub tumin: i64, + pub tumittel: i64, +} + impl Waterlevel { pub async fn find_by_id(db: &SqlitePool, id: i32) -> Option { sqlx::query_as!(Self, "SELECT * FROM waterlevel WHERE id like ?", id) @@ -31,20 +42,10 @@ impl Waterlevel { .ok() } - pub async fn create( - db: &mut Transaction<'_, Sqlite>, - day: NaiveDate, - time: String, - max: i64, - min: i64, - mittel: i64, - tumax: i64, - tumin: i64, - tumittel: i64, - ) -> Result<(), String> { + pub async fn create(db: &mut Transaction<'_, Sqlite>, create: &Create) -> Result<(), String> { sqlx::query!( "INSERT INTO waterlevel(day, time, max, min, mittel, tumax, tumin, tumittel) VALUES (?,?,?,?,?,?,?,?)", - day, time, max, min, mittel, tumax, tumin, tumittel + create.day, create.time, create.max, create.min, create.mittel, create.tumax, create.tumin, create.tumittel ) .execute(db.deref_mut()) .await diff --git a/src/scheduled/waterlevel.rs b/src/scheduled/waterlevel.rs index e90080f..3fa30c1 100644 --- a/src/scheduled/waterlevel.rs +++ b/src/scheduled/waterlevel.rs @@ -2,7 +2,7 @@ use chrono::{DateTime, FixedOffset, NaiveDate, NaiveTime}; use serde::{Deserialize, Serialize}; use sqlx::SqlitePool; -use crate::model::waterlevel::Waterlevel; +use crate::model::waterlevel::{self, Waterlevel}; pub async fn update(db: &SqlitePool) -> Result<(), String> { let mut tx = db.begin().await.unwrap(); @@ -29,10 +29,18 @@ pub async fn update(db: &SqlitePool) -> Result<(), String> { let time: NaiveTime = datetime.naive_utc().time(); let time_str = time.format("%H:%M").to_string(); - Waterlevel::create( - &mut tx, date, time_str, max, min, mittel, tumax, tumin, tumittel, - ) - .await? + let create = waterlevel::Create { + day: date, + time: time_str, + max, + min, + mittel, + tumax, + tumin, + tumittel, + }; + + Waterlevel::create(&mut tx, &create).await? } // 3. Save in DB @@ -77,25 +85,23 @@ fn fetch() -> Result { if let Ok(data) = forecast { if data.len() == 1 { - return Ok(data[0].clone()); + Ok(data[0].clone()) } else { - return Err(format!( + Err(format!( "Expected 1 station (Linz); got {} while fetching from {url}. Maybe the hydro data format changed?", data.len() - )); + )) } } else { - return Err(format!( + Err(format!( "Failed to parse the json received by {url}: {}", forecast.err().unwrap() - )); + )) } } - Err(_) => { - return Err(format!( - "Could not fetch {url}, do you have internet? Maybe their server is down?" - )); - } + Err(_) => Err(format!( + "Could not fetch {url}, do you have internet? Maybe their server is down?" + )), } } diff --git a/src/scheduled/weather.rs b/src/scheduled/weather.rs index 113f8ab..a8d0555 100644 --- a/src/scheduled/weather.rs +++ b/src/scheduled/weather.rs @@ -103,18 +103,16 @@ fn fetch(api_key: &str) -> Result { let data: Result = response.into_json(); if let Ok(data) = data { - return Ok(data); + Ok(data) } else { - return Err(format!( + Err(format!( "Failed to parse the json received by {url}: {}", data.err().unwrap() - )); + )) } } - Err(_) => { - return Err(format!( - "Could not fetch {url}, do you have internet? Maybe their server is down?" - )); - } + Err(_) => Err(format!( + "Could not fetch {url}, do you have internet? Maybe their server is down?" + )), } } diff --git a/src/tera/admin/planned_event.rs b/src/tera/admin/planned_event.rs index 18a04e9..f98b8c9 100644 --- a/src/tera/admin/planned_event.rs +++ b/src/tera/admin/planned_event.rs @@ -8,7 +8,7 @@ use serde::Serialize; use sqlx::SqlitePool; use crate::model::{ - planned_event::PlannedEvent, + planned_event::{self, PlannedEvent}, tripdetails::{TripDetails, TripDetailsToAdd}, user::PlannedEventUser, }; @@ -57,19 +57,17 @@ async fn update( data: Form>, _admin: PlannedEventUser, ) -> Flash { + let update = planned_event::EventUpdate { + name: data.name, + planned_amount_cox: data.planned_amount_cox, + max_people: data.max_people, + notes: data.notes, + always_show: data.always_show, + is_locked: data.is_locked, + }; match PlannedEvent::find_by_id(db, data.id).await { Some(planned_event) => { - planned_event - .update( - db, - data.name, - data.planned_amount_cox, - data.max_people, - data.notes, - data.always_show, - data.is_locked, - ) - .await; + planned_event.update(db, &update).await; Flash::success(Redirect::to("/planned"), "Event erfolgreich bearbeitet") } None => Flash::error(Redirect::to("/planned"), "Planned event id not found"), diff --git a/src/tera/auth.rs b/src/tera/auth.rs index be48ea2..108612e 100644 --- a/src/tera/auth.rs +++ b/src/tera/auth.rs @@ -39,7 +39,6 @@ struct LoginForm<'r> { password: &'r str, } -#[derive(Debug)] pub struct UserAgent(String); #[rocket::async_trait] @@ -83,8 +82,8 @@ async fn login( Log::create( db, format!( - "Succ login of {} with this useragent: {:?}", - login.name, agent + "Succ login of {} with this useragent: {}", + login.name, agent.0 ), ) .await; diff --git a/src/tera/cox.rs b/src/tera/cox.rs index cc31580..bb7fde9 100644 --- a/src/tera/cox.rs +++ b/src/tera/cox.rs @@ -9,7 +9,7 @@ use sqlx::SqlitePool; use crate::model::{ log::Log, planned_event::PlannedEvent, - trip::{CoxHelpError, Trip, TripDeleteError, TripHelpDeleteError, TripUpdateError}, + trip::{self, CoxHelpError, Trip, TripDeleteError, TripHelpDeleteError, TripUpdateError}, tripdetails::{TripDetails, TripDetailsToAdd}, user::CoxUser, }; @@ -54,18 +54,16 @@ async fn update( cox: CoxUser, ) -> Flash { if let Some(trip) = Trip::find_by_id(db, trip_id).await { - match Trip::update_own( - db, - &cox, - &trip, - data.max_people, - data.notes, - data.trip_type, - data.always_show, - data.is_locked, - ) - .await - { + let update = trip::TripUpdate { + cox: &cox, + trip: &trip, + max_people: data.max_people, + notes: data.notes, + trip_type: data.trip_type, + always_show: data.always_show, + is_locked: data.is_locked, + }; + match Trip::update_own(db, &update).await { Ok(_) => Flash::success( Redirect::to("/planned"), "Ausfahrt erfolgreich aktualisiert.", diff --git a/src/tera/log.rs b/src/tera/log.rs index 05fff3d..9b42eba 100644 --- a/src/tera/log.rs +++ b/src/tera/log.rs @@ -27,7 +27,7 @@ use crate::model::{ user::{AdminUser, DonauLinzUser, User, UserWithDetails}, }; -pub struct KioskCookie(String); +pub struct KioskCookie(()); #[rocket::async_trait] impl<'r> FromRequest<'r> for KioskCookie { @@ -35,7 +35,7 @@ impl<'r> FromRequest<'r> for KioskCookie { async fn from_request(request: &'r Request<'_>) -> request::Outcome { match request.cookies().get_private("kiosk") { - Some(cookie) => request::Outcome::Success(KioskCookie(cookie.value().to_string())), + Some(_) => request::Outcome::Success(KioskCookie(())), None => request::Outcome::Forward(rocket::http::Status::SeeOther), } }