fix(email-service): Upsert thread history when upserting draft#958
fix(email-service): Upsert thread history when upserting draft#958evanhutnik merged 2 commits intomainfrom
Conversation
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
| #[tracing::instrument(skip(pool), level = "info")] | ||
| pub async fn upsert_user_history( | ||
| pool: &PgPool, | ||
| #[tracing::instrument(skip(executor), level = "info")] |
There was a problem hiding this comment.
remove level = "info". I'd also recommend adding err here to automatically log the specific error
| .execute(pool) | ||
| .execute(executor) | ||
| .await | ||
| .map_err(|_| anyhow!("Failed to upsert user history"))?; |
There was a problem hiding this comment.
instead of mapping to anyhow error that loses all the context of the actual error you can just do .await?
combined with my recommendation above this could help with giving us better logs to see what actually errors
| .context("unable to fetch from email id")?; | ||
|
|
||
| insert_message_to_send(tx, draft, thread_db_id.unwrap(), from_email_id, true) | ||
| insert_message_to_send(tx, draft, thread_db_id, from_email_id, true) |
There was a problem hiding this comment.
Instead of creating this thread_db_id couldn't you just use draft.thread_db_id since it's explicitly set in the else if it's not already present.
There was a problem hiding this comment.
don't know which approach is cleaner, just giving alternative
There was a problem hiding this comment.
draft.thread_db_id is optional so that would require us to call unwrap on it, which isn't great imo - i think this is the cleaner way
2 participants
Summary
Title
Screenshots, GIFs, and Videos