feat(commands): general refactor and cleanup

- Update commands to use new helper macros
- Add tracing and inspection to alias models
- Remove dead code
This commit is contained in:
Suya1671 2025-06-19 16:19:31 +02:00
parent 4bd019c825
commit 4be14128f5
No known key found for this signature in database
10 changed files with 91 additions and 201 deletions

View file

@ -37,8 +37,6 @@ pub enum Alias {
#[derive(thiserror::Error, displaydoc::Display, Debug)]
pub enum CommandError {
/// Error while calling the Slack API
Slack,
/// Error while calling the database
Sqlx,
}
@ -67,6 +65,7 @@ impl Alias {
member: MemberRef,
alias: String,
) -> Result<SlackCommandEventResponse, CommandError> {
debug!("Creating alias");
let states = state.read().await;
let user_state = states.get_user_state::<user::State>().unwrap();
@ -74,7 +73,7 @@ impl Alias {
fetch_member!(member, user_state, system_id => member_id);
if alias.parse::<i64>().is_err() {
if alias.parse::<i64>().is_ok() {
return Ok(SlackCommandEventResponse::new(
SlackMessageContent::new().with_text(
"Alias cannot be a valid integer, as it could be mistaken for a member ID."
@ -83,7 +82,7 @@ impl Alias {
));
}
models::Alias::insert(&user_state.db, member_id, system_id, alias)
models::Alias::insert(member_id, system_id, alias, &user_state.db)
.await
.change_context(CommandError::Sqlx)?;
@ -98,6 +97,7 @@ impl Alias {
state: &SlackClientEventsUserState,
alias: alias::Id<Untrusted>,
) -> Result<SlackCommandEventResponse, CommandError> {
debug!("Deleting alias");
let states = state.read().await;
let user_state = states.get_user_state::<user::State>().unwrap();
@ -129,6 +129,7 @@ impl Alias {
state: &SlackClientEventsUserState,
member: Option<MemberRef>,
) -> Result<SlackCommandEventResponse, CommandError> {
debug!("Listing aliases");
let states = state.read().await;
let user_state = states.get_user_state::<user::State>().unwrap();
@ -138,11 +139,11 @@ impl Alias {
debug!("Fetching aliases by member");
fetch_member!(member, user_state, system_id => member_id);
models::Alias::fetch_by_member_id(&user_state.db, member_id)
models::Alias::fetch_by_member_id(member_id, &user_state.db)
.await
.change_context(CommandError::Sqlx)?
} else {
models::Alias::fetch_by_system_id(&user_state.db, system_id)
models::Alias::fetch_by_system_id(system_id, &user_state.db)
.await
.change_context(CommandError::Sqlx)?
};
@ -165,7 +166,7 @@ impl Alias {
];
SlackSectionBlock::new()
.with_text(md!("**Alias {}**", alias.id))
.with_text(md!("*Alias {}*", alias.id))
.with_fields(fields)
})
.map(Into::into)
@ -183,6 +184,7 @@ impl Alias {
alias: alias::Id<Untrusted>,
new_alias: String,
) -> Result<SlackCommandEventResponse, CommandError> {
debug!("Editing alias");
let states = state.read().await;
let user_state = states.get_user_state::<user::State>().unwrap();
@ -199,7 +201,7 @@ impl Alias {
};
alias
.change_alias(&user_state.db, new_alias)
.change_alias(new_alias, &user_state.db)
.await
.change_context(CommandError::Sqlx)?;

View file

@ -101,7 +101,6 @@ impl Member {
let user_state = states.get_user_state::<user::State>().unwrap();
fetch_system!(event, user_state => system_id);
debug!("Found user system");
let new_active_member_id = if base {
None
@ -189,8 +188,8 @@ impl Member {
.into_iter()
.map(|member| {
let fields = [
Some(md!("Display Name: {}", member.display_name)),
Some(md!("Member ID: {}", member.id)),
Some(md!("Display Name: {}", member.display_name)),
member.title.as_ref().map(|title| md!("Title: {}", title)),
member
.pronouns
@ -239,12 +238,11 @@ impl Member {
.await
.change_context(CommandError::Sqlx)?;
fields!(member_id = %member.id);
debug!("Member found");
let fields = [
Some(md!("Display Name: {}", member.display_name)),
Some(md!("Member ID: {}", member.id)),
Some(md!("Display Name: {}", member.display_name)),
member.title.as_ref().map(|title| md!("Title: {}", title)),
member
.pronouns

View file

@ -5,31 +5,31 @@ use slack_morphism::prelude::*;
use tracing::debug;
use crate::{
BOT_TOKEN, fields,
models::{self, member, trigger, user},
BOT_TOKEN, fetch_member, fetch_system, fields,
models::{self, Untrusted, member::MemberRef, trigger, user},
};
#[derive(clap::Subcommand, Debug)]
pub enum Trigger {
/// Adds a new trigger for a member. Expect a popup to fill in the info!
Add {
/// The member to add the trigger for. Use the member id from /member list
member: i64,
/// The member to add the trigger for.
member: MemberRef,
},
/// Deletes a trigger
Delete {
/// The trigger to delete. Use the trigger id from /trigger list
id: i64,
/// The trigger to delete.
id: trigger::Id<Untrusted>,
},
/// Lists all of your triggers
List {
/// If specified, lists the triggers for the given member. Use the member id from /member list
member: Option<i64>,
/// If specified, lists the triggers for the given member.
member: Option<MemberRef>,
},
/// Edit a trigger
Edit {
/// The trigger to edit. Use the trigger id from /trigger list
id: i64,
id: trigger::Id<Untrusted>,
},
}
@ -70,33 +70,17 @@ impl Trigger {
event: SlackCommandEvent,
state: &SlackClientEventsUserState,
session: SlackClientSession<'_, SlackClientHyperHttpsConnector>,
member_id: i64,
member_id: MemberRef,
) -> Result<SlackCommandEventResponse, CommandError> {
let states = state.read().await;
let user_state = states.get_user_state::<user::State>().unwrap();
let member_id = member::Id::new(member_id);
let Some(system_id) =
models::System::fetch_by_user_id(&user_state.db, &user::Id::new(event.user_id))
.await
.change_context(CommandError::Sqlx)?
.map(|system| system.id)
else {
debug!("User does not have a system");
return Ok(SlackCommandEventResponse::new(
SlackMessageContent::new().with_text(
"You don't have a system yet! Make one with `/system create <name>`".into(),
),
));
};
fetch_system!(event, user_state => system_id);
fields!(system_id = %system_id);
let Some(member_id) =
models::Member::fetch_by_and_trust_id(system_id, member_id, &user_state.db)
.await
.change_context(CommandError::Sqlx)?
.map(|member| member.id)
let Some(member_id) = member_id
.validate_by_system(system_id, &user_state.db)
.await
.change_context(CommandError::Sqlx)?
else {
debug!("Member not found");
return Ok(SlackCommandEventResponse::new(
@ -126,11 +110,10 @@ impl Trigger {
pub async fn delete_trigger(
event: SlackCommandEvent,
state: &SlackClientEventsUserState,
trigger_id: i64,
trigger_id: trigger::Id<Untrusted>,
) -> Result<SlackCommandEventResponse, CommandError> {
let states = state.read().await;
let user_state = states.get_user_state::<user::State>().unwrap();
let trigger_id = trigger::Id::new(trigger_id);
let Some(system_id) =
models::System::fetch_by_user_id(&user_state.db, &user::Id::new(event.user_id))
@ -174,44 +157,15 @@ impl Trigger {
pub async fn list_triggers(
event: SlackCommandEvent,
state: &SlackClientEventsUserState,
member_id: Option<i64>,
member_ref: Option<MemberRef>,
) -> Result<SlackCommandEventResponse, CommandError> {
let states = state.read().await;
let user_state = states.get_user_state::<user::State>().unwrap();
let Some(system_id) =
models::System::fetch_by_user_id(&user_state.db, &user::Id::new(event.user_id))
.await
.change_context(CommandError::Sqlx)?
.map(|system| system.id)
else {
debug!("User doesn't have a system");
return Ok(SlackCommandEventResponse::new(
SlackMessageContent::new().with_text(
"You don't have a system yet! Make one with `/system create <name>`".into(),
),
));
};
fetch_system!(event, user_state => system_id);
fields!(system_id = %system_id);
let triggers = if let Some(member_id) = member_id {
let member_id = member::Id::new(member_id);
// Validate the member belongs to the user's system
let Some(member_id) = member_id
.validate_by_system(system_id, &user_state.db)
.await
.change_context(CommandError::Sqlx)?
else {
debug!("Member not found");
return Ok(SlackCommandEventResponse::new(
SlackMessageContent::new()
.with_text("Member not found. Make sure you used the correct ID".into()),
));
};
fields!(member_id = %member_id);
let triggers = if let Some(member_ref) = member_ref {
fetch_member!(member_ref, user_state, system_id => member_id);
member_id
.fetch_triggers(&user_state.db)
@ -259,27 +213,12 @@ impl Trigger {
event: SlackCommandEvent,
state: &SlackClientEventsUserState,
session: SlackClientSession<'_, SlackClientHyperHttpsConnector>,
trigger_id: i64,
trigger_id: trigger::Id<Untrusted>,
) -> Result<SlackCommandEventResponse, CommandError> {
let states = state.read().await;
let user_state = states.get_user_state::<user::State>().unwrap();
let trigger_id = trigger::Id::new(trigger_id);
let Some(system_id) =
models::System::fetch_by_user_id(&user_state.db, &user::Id::new(event.user_id))
.await
.change_context(CommandError::Sqlx)?
.map(|system| system.id)
else {
debug!("User does not have a system");
return Ok(SlackCommandEventResponse::new(
SlackMessageContent::new().with_text(
"You don't have a system yet! Make one with `/system create <name>`".into(),
),
));
};
fields!(system_id = %system_id);
fetch_system!(event, user_state => system_id);
// Validate the trigger belongs to the user's system
let Ok(trigger_id) = trigger_id

View file

@ -52,7 +52,9 @@ pub async fn process_push_event(
SlackPushEvent::EventCallback(event) => {
let client = environment.client.clone();
let state = environment.user_state.clone();
if let Err(e) = push_event_callback(event, client, state).await {
// https://rust-lang.github.io/rust-clippy/master/index.html#large_futures
// Into the box you go
if let Err(e) = Box::pin(push_event_callback(event, client, state)).await {
error!("Error processing push event: {:#?}", e);
}

View file

@ -4,7 +4,6 @@ use std::error::Error;
use std::sync::Arc;
use axum::Extension;
use error_stack::ResultExt;
use member::{create_member, edit_member};
use slack_morphism::prelude::*;
use tracing::{debug, error};

View file

@ -1,10 +1,8 @@
use crate::id;
use super::{Trustability, Trusted, Untrusted, member, system};
use super::{Trusted, Untrusted, member, system};
use error_stack::{Result, ResultExt};
use slack_morphism::prelude::*;
use sqlx::{SqlitePool, prelude::*, sqlite::SqliteQueryResult};
use tracing::{debug, warn};
id!(
/// For an ID to be trusted, it must
@ -15,13 +13,7 @@ id!(
);
impl Id<Untrusted> {
pub const fn new(id: i64) -> Self {
Self {
id,
trusted: std::marker::PhantomData,
}
}
#[tracing::instrument(skip(db))]
pub async fn validate_by_system(
self,
system_id: system::Id<Trusted>,
@ -43,7 +35,8 @@ impl Id<Untrusted> {
}
impl Id<Trusted> {
pub async fn delete(self, db_pool: &SqlitePool) -> Result<SqliteQueryResult, sqlx::Error> {
#[tracing::instrument(skip(db))]
pub async fn delete(self, db: &SqlitePool) -> Result<SqliteQueryResult, sqlx::Error> {
sqlx::query!(
r#"
DELETE FROM aliases
@ -51,15 +44,16 @@ impl Id<Trusted> {
"#,
self.id
)
.execute(db_pool)
.execute(db)
.await
.attach_printable("Failed to delete alias from database")
}
#[tracing::instrument(skip(db))]
pub async fn change_alias(
self,
db_pool: &SqlitePool,
new_alias: String,
db: &SqlitePool,
) -> Result<SqliteQueryResult, sqlx::Error> {
sqlx::query!(
r#"
@ -70,54 +64,27 @@ impl Id<Trusted> {
self.id,
new_alias
)
.execute(db_pool)
.execute(db)
.await
.attach_printable("Failed to change alias in database")
}
}
#[derive(thiserror::Error, displaydoc::Display, Debug)]
pub enum Error {
/// Error while calling the database
Sqlx,
}
#[derive(FromRow, Debug)]
#[allow(dead_code)]
pub struct Alias {
pub id: Id<Trusted>,
pub member_id: member::Id<Trusted>,
pub system_id: system::Id<Trusted>,
#[allow(clippy::struct_field_names)]
pub alias: String,
}
impl Alias {
pub async fn fetch_by_id<T>(id: Id<T>, db: &SqlitePool) -> Result<Option<Self>, sqlx::Error>
where
T: Trustability,
{
sqlx::query_as!(
Alias,
r#"
SELECT
id as "id: Id<Trusted>",
member_id as "member_id: member::Id<Trusted>",
system_id as "system_id: system::Id<Trusted>",
alias
FROM
aliases
WHERE id = $1
"#,
id.id,
)
.fetch_optional(db)
.await
.attach_printable("Failed to fetch alias from database")
}
#[tracing::instrument(skip(db))]
pub async fn fetch_by_system_id(
db: &SqlitePool,
system_id: system::Id<Trusted>,
db: &SqlitePool,
) -> Result<Vec<Self>, sqlx::Error> {
sqlx::query_as!(
Alias,
@ -139,9 +106,10 @@ impl Alias {
.attach_printable("Failed to fetch aliases from database")
}
#[tracing::instrument(skip(db))]
pub async fn fetch_by_member_id(
db: &SqlitePool,
member_id: member::Id<Trusted>,
db: &SqlitePool,
) -> error_stack::Result<Vec<Self>, sqlx::Error> {
sqlx::query_as!(
Self,
@ -162,11 +130,12 @@ impl Alias {
.attach_printable("Failed to fetch aliases from database")
}
#[tracing::instrument(skip(db))]
pub async fn insert(
db: &SqlitePool,
member_id: member::Id<Trusted>,
system_id: system::Id<Trusted>,
alias: String,
db: &SqlitePool,
) -> error_stack::Result<Self, sqlx::Error> {
sqlx::query_as!(
Self,

View file

@ -13,14 +13,6 @@ use super::{
user,
};
#[derive(thiserror::Error, displaydoc::Display, Debug)]
pub enum Error {
/// Error while calling the database
Database,
/// A field was missing from the view
MissingField(String),
}
id!(
/// For an ID to be trusted, it must
///
@ -37,6 +29,7 @@ impl Id<Untrusted> {
}
}
#[tracing::instrument(skip(db))]
pub async fn validate_by_system(
self,
system_id: system::Id<Trusted>,
@ -56,6 +49,7 @@ impl Id<Untrusted> {
.map(|res| res.map(|res| res.id))
}
#[tracing::instrument(skip(db))]
pub async fn validate_by_user(
self,
user_id: &user::Id<Trusted>,
@ -78,6 +72,7 @@ impl Id<Untrusted> {
.map(|res| res.map(|res| res.id))
}
#[tracing::instrument(skip(db))]
pub async fn fetch_by_alias(
alias: &str,
system_id: system::Id<Trusted>,
@ -99,12 +94,13 @@ impl Id<Untrusted> {
}
impl Id<Trusted> {
#[tracing::instrument(skip(db))]
pub async fn fetch_triggers(self, db: &SqlitePool) -> Result<Vec<Trigger>, sqlx::Error> {
Trigger::fetch_by_member_id(db, self).await
Trigger::fetch_by_member_id(self, db).await
}
}
#[derive(Debug, Clone, derive_more::From)]
#[derive(Debug, Clone)]
/// An untrusted member reference from an external source
pub enum MemberRef {
Id(Id<Untrusted>),
@ -124,17 +120,18 @@ impl FromStr for MemberRef {
}
impl MemberRef {
#[tracing::instrument(skip(db))]
pub async fn validate_by_system(
&self,
system_id: system::Id<Trusted>,
db: &SqlitePool,
) -> Result<Option<Id<Trusted>>, sqlx::Error> {
match self {
MemberRef::Id(id) => id
Self::Id(id) => id
.validate_by_system(system_id, db)
.await
.attach_printable("Failed to validate member reference via id and system"),
MemberRef::Alias(alias) => Id::fetch_by_alias(alias, system_id, db)
Self::Alias(alias) => Id::fetch_by_alias(alias, system_id, db)
.await
.attach_printable("Failed to validate member reference via alias and system"),
}
@ -162,38 +159,8 @@ pub struct Member {
}
impl Member {
pub async fn fetch_by_and_trust_id(
system_id: system::Id<Trusted>,
member_id: Id<Untrusted>,
db: &SqlitePool,
) -> Result<Option<Self>, sqlx::Error> {
sqlx::query_as!(
Member,
r#"
SELECT
id as "id: Id<Trusted>",
system_id as "system_id: system::Id<Trusted>",
full_name,
display_name,
profile_picture_url,
title,
pronouns,
name_pronunciation,
name_recording_url,
created_at as "created_at: time::PrimitiveDateTime"
FROM members
WHERE system_id = $1 AND id = $2
"#,
system_id,
// Safe because this query also checks if the ID is trusted
member_id.id
)
.fetch_optional(db)
.await
.attach_printable("Failed to fetch member by id and trust by system")
}
/// Fetch a member by their id
#[tracing::instrument(skip(db))]
pub async fn fetch_by_id(member_id: Id<Trusted>, db: &SqlitePool) -> Result<Self, sqlx::Error> {
sqlx::query_as!(
Member,
@ -210,7 +177,7 @@ impl Member {
name_recording_url,
created_at as "created_at: time::PrimitiveDateTime"
FROM members
WHERE id = $2
WHERE id = $1
"#,
member_id
)
@ -247,7 +214,7 @@ impl From<Member> for TriggeredMember {
}
}
#[derive(Default, Clone)]
#[derive(Debug, Default, Clone)]
pub struct View {
pub full_name: String,
pub display_name: String,
@ -351,6 +318,7 @@ impl View {
/// Add a member to the database
///
/// Returns the id of the new member
#[tracing::instrument(skip(db))]
pub async fn add(
&self,
system_id: system::Id<Trusted>,
@ -380,6 +348,7 @@ impl View {
/// Update a member in the database to match this view
///
/// Returns None if the member does not exist
#[tracing::instrument(skip(db))]
pub async fn update(
&self,
member_id: Id<Trusted>,
@ -403,8 +372,12 @@ impl View {
}
}
#[derive(thiserror::Error, displaydoc::Display, Debug)]
/// A field was missing from the view
pub struct MissingFieldError(String);
impl TryFrom<SlackViewState> for View {
type Error = Error;
type Error = MissingFieldError;
fn try_from(value: SlackViewState) -> std::result::Result<Self, Self::Error> {
let mut view = Self::default();
@ -414,12 +387,12 @@ impl TryFrom<SlackViewState> for View {
"full_name" => {
view.full_name = content
.value
.ok_or_else(|| Error::MissingField("display_name".to_string()))?;
.ok_or_else(|| MissingFieldError("display_name".to_string()))?;
}
"display_name" => {
view.display_name = content
.value
.ok_or_else(|| Error::MissingField("display_name".to_string()))?;
.ok_or_else(|| MissingFieldError("display_name".to_string()))?;
}
"profile_picture_url" => view.profile_picture_url = content.value,
"title" => view.title = content.value,
@ -434,11 +407,11 @@ impl TryFrom<SlackViewState> for View {
}
if view.full_name.is_empty() {
return Err(Error::MissingField("full_name".to_string()));
return Err(MissingFieldError("full_name".to_string()));
}
if view.display_name.is_empty() {
return Err(Error::MissingField("display_name".to_string()));
return Err(MissingFieldError("display_name".to_string()));
}
Ok(view)

View file

@ -69,8 +69,8 @@ impl MessageLog {
/// Fetches all message logs by the member ID.
#[tracing::instrument(skip(db))]
pub async fn fetch_all_by_member_id(
db: &SqlitePool,
member_id: member::Id<Trusted>,
db: &SqlitePool,
) -> Result<Vec<Self>, sqlx::Error> {
sqlx::query_as!(
MessageLog,
@ -91,6 +91,7 @@ impl MessageLog {
.attach_printable("Failed to fetch message logs")
}
#[tracing::instrument(skip(db))]
pub async fn insert(
member_id: member::Id<Trusted>,
message_id: SlackTs,

View file

@ -25,7 +25,7 @@ id!(
impl Id<Trusted> {
#[tracing::instrument(skip(db))]
pub async fn list_triggers(self, db: &SqlitePool) -> Result<Vec<Trigger>, sqlx::Error> {
Trigger::fetch_by_system_id(db, self).await
Trigger::fetch_by_system_id(self, db).await
}
#[tracing::instrument(skip(db))]
@ -142,6 +142,7 @@ impl System {
.attach_printable("Error fetching system")
}
#[tracing::instrument(skip(db))]
pub async fn active_member(&self, db: &SqlitePool) -> Result<Option<Member>, sqlx::Error> {
match self.active_member_id {
Some(id) => Ok(Some(Member::fetch_by_id(id, db).await?)),

View file

@ -24,6 +24,7 @@ impl Id<Untrusted> {
}
}
#[tracing::instrument(skip(db))]
pub async fn validate_by_system(
self,
system_id: system::Id<Trusted>,
@ -45,7 +46,8 @@ impl Id<Untrusted> {
}
impl Id<Trusted> {
pub async fn delete(self, db_pool: &SqlitePool) -> Result<SqliteQueryResult, sqlx::Error> {
#[tracing::instrument(skip(db))]
pub async fn delete(self, db: &SqlitePool) -> Result<SqliteQueryResult, sqlx::Error> {
sqlx::query!(
r#"
DELETE FROM triggers
@ -53,7 +55,7 @@ impl Id<Trusted> {
"#,
self.id
)
.execute(db_pool)
.execute(db)
.await
.attach_printable("Error deleting trigger")
}
@ -137,9 +139,10 @@ impl Trigger {
.attach_printable("Error fetching trigger")
}
#[tracing::instrument(skip(db))]
pub async fn fetch_by_system_id(
db: &SqlitePool,
system_id: system::Id<Trusted>,
db: &SqlitePool,
) -> Result<Vec<Self>, sqlx::Error> {
sqlx::query_as!(
Trigger,
@ -162,9 +165,10 @@ impl Trigger {
.attach_printable("Error fetching triggers")
}
#[tracing::instrument(skip(db))]
pub async fn fetch_by_member_id(
db: &SqlitePool,
member_id: member::Id<Trusted>,
db: &SqlitePool,
) -> error_stack::Result<Vec<Self>, sqlx::Error> {
sqlx::query_as!(
Trigger,
@ -248,11 +252,12 @@ impl View {
/// Add a trigger to the database
///
/// Returns the id of the new trigger
#[tracing::instrument(skip(db))]
pub async fn add(
&self,
system_id: system::Id<Trusted>,
member_id: member::Id<Trusted>,
db_pool: &SqlitePool,
db: &SqlitePool,
) -> error_stack::Result<Id<Trusted>, Error> {
debug!(
"Adding trigger for {} (Member ID {}) to database",
@ -270,7 +275,7 @@ impl View {
self.text,
self.typ
)
.fetch_one(db_pool)
.fetch_one(db)
.await
.attach_printable("Error adding trigger to database")
.change_context(Error::Sqlx)
@ -281,6 +286,7 @@ impl View {
}
/// Update a trigger in the database to match this view
#[tracing::instrument(skip(db))]
pub async fn update(
&self,
trigger_id: Id<Trusted>,