Refactor HeyaNotFound error to simplify handlers

src/data/heya.rs

@@ -59,11 +59,11 @@ impl Heya {
         .collect()
     }
 
-    pub fn with_id(db: &Connection, id: HeyaId, include_members: bool) -> SqlResult<Option<Self>> {
+    pub fn with_id(db: &Connection, id: HeyaId, include_members: bool) -> Result<Self> {
         Self::query_one(&db, Some(id), None, include_members)
     }
 
-    pub fn with_slug(db: &Connection, slug: &str, include_members: bool) -> SqlResult<Option<Self>> {
+    pub fn with_slug(db: &Connection, slug: &str, include_members: bool) -> Result<Self> {
         Self::query_one(&db, None, Some(slug), include_members)
     }
 
@@ -72,7 +72,7 @@ impl Heya {
         id: Option<HeyaId>,
         slug: Option<&str>,
         include_members: bool,
-    ) -> SqlResult<Option<Self>> {
+    ) -> Result<Self> {
         let rank_for_basho = BashoInfo::current_or_next_basho_id(&db)?;
         let (where_clause, params) = if id.is_some() {
             ("heya.id = ?", params![id])
@@ -81,9 +81,9 @@ impl Heya {
         } else {
             panic!("must specify id or slug for Heya::query_one");
         };
-        match db
-            .query_row_and_then(
-                &format!("
+        match db.query_row_and_then(
+            &format!(
+                "
                     SELECT
                         heya.id AS heya_id,
                         heya.name AS heya_name,
@@ -93,31 +93,30 @@ impl Heya {
                     FROM heya
                     JOIN player_info AS oyakata ON oyakata.id = heya.oyakata_player_id
                     WHERE {where_clause}
-                "),
-                params,
-                |row| {
-                    Ok(Self {
-                        id: row.get("heya_id")?,
-                        name: row.get("heya_name")?,
-                        slug: row.get("heya_slug")?,
-                        create_date: row.get("create_date")?,
-                        oyakata: Player::from_row(row)?,
-                        members: None,
-                        member_count: 0,
-                    })
-                },
-            )
-            .optional()?
-        {
+                "
+            ),
+            params,
+            |row| -> SqlResult<Heya> {
+                Ok(Self {
+                    id: row.get("heya_id")?,
+                    name: row.get("heya_name")?,
+                    slug: row.get("heya_slug")?,
+                    create_date: row.get("create_date")?,
+                    oyakata: Player::from_row(row)?,
+                    members: None,
+                    member_count: 0,
+                })
+            },
+        ).optional()? {
             Some(mut heya) => {
                 if include_members {
                     let members = Member::in_heya(&db, heya.id, rank_for_basho)?;
                     heya.member_count = members.len();
                     heya.members = Some(members);
                 }
-                Ok(Some(heya))
+                Ok(heya)
             }
-            None => Ok(None),
+            None => Err(DataError::HeyaNotFound { slug: slug.map(|s| s.to_string()), id })
         }
     }
 
@@ -180,9 +179,7 @@ impl Heya {
         .execute(params![heya_id, oyakata, now])?;
 
         Self::validate_quota(&txn, oyakata)?;
-        let heya = Self::with_slug(&txn, &slug, false)?.ok_or(DataError::HeyaIntegrity {
-            what: "heya failed to insert".to_string(),
-        })?;
+        let heya = Self::with_slug(&txn, &slug, false)?;
         txn.commit()?;
 
         Ok(heya)

src/data/mod.rs

@@ -1,3 +1,4 @@
+use heya::HeyaId;
 use rusqlite::config::DbConfig::SQLITE_DBCONFIG_ENABLE_FKEY;
 use rusqlite::{Connection, OpenFlags};
 use std::path::Path;
@@ -63,9 +64,19 @@ type Result<T> = std::result::Result<T, DataError>;
 pub enum DataError {
     BashoHasStarted,
     InvalidPicks,
-    HeyaIntegrity { what: String },
-    RikishiNotFound { family_name: String },
-    AmbiguousShikona { family_names: Vec<String> },
+    HeyaIntegrity {
+        what: String,
+    },
+    RikishiNotFound {
+        family_name: String,
+    },
+    AmbiguousShikona {
+        family_names: Vec<String>,
+    },
+    HeyaNotFound {
+        slug: Option<String>,
+        id: Option<HeyaId>,
+    },
     DatabaseError(rusqlite::Error),
     WebPushError(web_push::WebPushError),
     JsonError(serde_json::Error),
@@ -104,6 +115,9 @@ impl fmt::Display for DataError {
             DataError::AmbiguousShikona { family_names } => {
                 write!(f, "Multiple rikishi with shikona: {:?}", family_names)
             }
+            DataError::HeyaNotFound { slug, id } => {
+                write!(f, "Heya not found for slug {slug:?} or id {id:?}")
+            }
             DataError::DatabaseError(e) => write!(f, "Database error: {}", e),
             DataError::UnknownLoginProvider => write!(f, "Unknown login provider"),
             DataError::WebPushError(e) => write!(f, "Web Push error: {}", e),

src/handlers/basho.rs

@@ -104,7 +104,7 @@ pub async fn basho(
     let heya = query
         .0
         .heya
-        .and_then(|heya_id| Heya::with_id(&db, heya_id, false).transpose())
+        .map(|heya_id| Heya::with_id(&db, heya_id, false))
         .transpose()?;
     let leaders = BashoPlayerResults::fetch(
         &db,

src/handlers/heya.rs

@@ -3,8 +3,8 @@ use actix_web::{get, http, post, web, HttpResponse, Responder};
 use askama::Template;
 use rusqlite::Connection;
 
-use crate::data::{Heya, PlayerId};
 use crate::data::heya::{HOST_MAX, JOIN_MAX};
+use crate::data::{Heya, PlayerId};
 use crate::handlers::{HandlerError, IdentityExt};
 use crate::AppState;
 
@@ -27,14 +27,12 @@ pub async fn page(
     let db = state.db.lock().unwrap();
     let base = BaseTemplate::new(&db, identity.as_ref(), &state)?;
     let player_id = identity.and_then(|i| i.player_id().ok());
-    match Heya::with_slug(&db, &path, true)? {
-        Some(heya) => Ok(HeyaTemplate {
-            is_oyakata: player_id.map_or(false, |pid| pid == heya.oyakata.id),
-            base,
-            heya,
-        }),
-        None => Err(HandlerError::NotFound("heya".to_string())),
-    }
+    let heya = Heya::with_slug(&db, &path, true)?;
+    Ok(HeyaTemplate {
+        is_oyakata: player_id.map_or(false, |pid| pid == heya.oyakata.id),
+        base,
+        heya,
+    })
 }
 
 #[derive(Debug, Deserialize)]
@@ -52,14 +50,11 @@ pub async fn edit(
     identity: Identity,
 ) -> Result<impl Responder> {
     let mut db = state.db.lock().unwrap();
-    if let Some(mut heya) = Heya::with_slug(&db, &path, false)? {
-        apply_edit_actions(&mut heya, &mut db, data.0, identity.player_id()?)?;
-        Ok(HttpResponse::SeeOther()
-            .insert_header((http::header::LOCATION, heya.url_path()))
-            .finish())
-    } else {
-        Err(HandlerError::NotFound("heya".to_string()))
-    }
+    let mut heya = Heya::with_slug(&db, &path, false)?;
+    apply_edit_actions(&mut heya, &mut db, data.0, identity.player_id()?)?;
+    Ok(HttpResponse::SeeOther()
+        .insert_header((http::header::LOCATION, heya.url_path()))
+        .finish())
 }
 
 fn apply_edit_actions(
@@ -108,8 +103,11 @@ pub async fn list(
     let player_id = identity.as_ref().and_then(|i| i.player_id().ok());
     Ok(HeyaListTemplate {
         base: BaseTemplate::new(&db, identity.as_ref(), &state)?,
-        hosted: heyas.iter().filter(|h| h.oyakata.id == player_id.unwrap_or(-1)).count(),
-        heyas
+        hosted: heyas
+            .iter()
+            .filter(|h| h.oyakata.id == player_id.unwrap_or(-1))
+            .count(),
+        heyas,
     })
 }
 

src/handlers/mod.rs

@@ -73,7 +73,13 @@ impl error::ResponseError for HandlerError {
 
 impl From<DataError> for HandlerError {
     fn from(err: DataError) -> Self {
-        Self::DatabaseError(err)
+        match err {
+            DataError::HeyaNotFound { slug: _, id: _ } => {
+                debug!("{err:?}");
+                HandlerError::NotFound("heya".to_string())
+            }
+            _ => Self::DatabaseError(err),
+        }
     }
 }
 

templates/basho.html

@@ -147,7 +147,7 @@ <h2>Basho Leaderboard</h2>
             {% if let Some(heya_options) = self.heya_options() %}
               {% if heya_options.len() > 0 %}
                 <form method="GET">
-                  <label for="heya-select">Filter by Heya:</label>
+                  <label for="heya-select">Filter by heya:</label>
                   <select name="heya" id="heya-select">
                     <option {% if heya.is_none() %} selected {% endif %}}>
                       Everybody
@@ -244,6 +244,8 @@ <h2>Basho Leaderboard</h2>
                 {% when None %}
                 {% endmatch %}
             </div>
+
+
             {% if basho.player_count > leaders.len() - 2 %}
                 <p>
                     Showing top {{leaders.len() - 2}} players out of {{basho.player_count}} total.