From a4479e6a9dfcb0698f4bbb6e9ab0b05d9f74b5fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20V=C3=B6gele?= Date: Wed, 17 Jan 2024 17:02:20 +0100 Subject: [PATCH] Allow multiple reminder dates for a single chat group --- locales/de.yml | 2 +- locales/en.yml | 2 +- .../down.sql | 5 ++ .../up.sql | 13 ++++ src/bot.rs | 72 +++++++++++-------- src/db.rs | 38 +++++++--- src/main.rs | 65 +++++++++++------ src/schema.rs | 36 +++++++--- 8 files changed, 160 insertions(+), 73 deletions(-) create mode 100644 migrations/2024-01-17-093811_multi_ahead_reminders/down.sql create mode 100644 migrations/2024-01-17-093811_multi_ahead_reminders/up.sql diff --git a/locales/de.yml b/locales/de.yml index fad3ad4..01ba34b 100644 --- a/locales/de.yml +++ b/locales/de.yml @@ -6,5 +6,5 @@ messages: starting_now: "Jetzt geht's weiter" errors: - param_no_days: "Bitte gib die Anzahl Tage an, die du vorab erinnert werden möchtest, als Parameter an" + invalid_number: "Ungültige Zahl: %{number}" param_no_days_nonnegative: "Die Anzahl der Tage darf nicht negativ sein" \ No newline at end of file diff --git a/locales/en.yml b/locales/en.yml index b96ee7e..c97a049 100644 --- a/locales/en.yml +++ b/locales/en.yml @@ -6,5 +6,5 @@ messages: starting_now: "The meeting starts now" errors: - param_no_days: "Please specify the number of days that you want to be reminded in advance as parameter" + invalid_number: "Invalid number: %{number}" param_no_days_nonnegative: "The number of days may not be negative" \ No newline at end of file diff --git a/migrations/2024-01-17-093811_multi_ahead_reminders/down.sql b/migrations/2024-01-17-093811_multi_ahead_reminders/down.sql new file mode 100644 index 0000000..7c452e0 --- /dev/null +++ b/migrations/2024-01-17-093811_multi_ahead_reminders/down.sql @@ -0,0 +1,5 @@ +ALTER TABLE chat ADD remind_days_ahead BIGINT NOT NULL DEFAULT 0; + +UPDATE chat SET remind_days_ahead = (SELECT days_ahead FROM reminder WHERE chat_id = chat.id ORDER BY days_ahead ASC LIMIT 1); + +DROP TABLE reminder; \ No newline at end of file diff --git a/migrations/2024-01-17-093811_multi_ahead_reminders/up.sql b/migrations/2024-01-17-093811_multi_ahead_reminders/up.sql new file mode 100644 index 0000000..510fef3 --- /dev/null +++ b/migrations/2024-01-17-093811_multi_ahead_reminders/up.sql @@ -0,0 +1,13 @@ +-- This migration extracts "remind days ahead" into a separate table so that a chat can configure mutiple reminders + +CREATE TABLE reminder ( + id INTEGER PRIMARY KEY NOT NULL, + chat_id INTEGER NOT NULL REFERENCES chat (id) ON DELETE CASCADE, + days_ahead BIGINT NOT NULL +); + +CREATE INDEX reminders_chat_id ON reminder (chat_id, days_ahead); -- Having days_ahead as index saves us from taking extra effort for sorted data + +INSERT INTO reminder (chat_id, days_ahead) SELECT id, remind_days_ahead FROM chat; + +ALTER TABLE chat DROP remind_days_ahead; \ No newline at end of file diff --git a/src/bot.rs b/src/bot.rs index 1fd0ef0..935d4c3 100644 --- a/src/bot.rs +++ b/src/bot.rs @@ -1,3 +1,4 @@ +use anyhow::anyhow; use anyhow::{Error, Result}; use chrono::Datelike; use chrono::Utc; @@ -114,13 +115,14 @@ async fn set_calendar(bot: Throttle, msg: Message, db: Database) -> Result< })?; let mut chat_info = ChatInfo:: { + db_id: -1, id: msg.chat.id.0, calendar: url.to_owned(), next_appointment: None, last_reminder: None, pinned_message_id: None, locale: "de".into(), - remind_days_ahead: 0, + remind_days_ahead: vec![], }; fetch_and_announce_appointment(&bot, &mut chat_info, &db).await?; @@ -159,28 +161,39 @@ async fn set_remind_days_ahead( .first::(db) })?; - let chat = ChatInfo::::from(chat); + let chat = ChatInfo::::from_db(chat, vec![]); - let days_ahead = msg - .text() - .map(|text| text.splitn(2, " ").nth(1).map(|param| param.parse().ok())) - .flatten() - .flatten(); + let days = + match msg + .text() + .ok_or_else(|| { + anyhow!("Set remind days ahead command didn't receive any text (this should never happen)") + })? + .split(" ") + .skip(1) + .map(|day| day.trim()) + .filter(|day| !day.is_empty()) + .map(|day| day.parse().map_err(|err| (day, err))) + .collect::, _>>() + { + Ok(days_ahead) => days_ahead, + Err((invalid_str, _)) => { + bot.send_message( + msg.chat.id, + t!( + "errors.invalid_number", + locale = &chat.locale, + number = invalid_str + ), + ) + .reply_to_message_id(msg.id) + .send() + .await?; + return Ok(()); + } + }; - if days_ahead.is_none() { - bot.send_message( - msg.chat.id, - t!("errors.param_no_days", locale = &chat.locale), - ) - .reply_to_message_id(msg.id) - .send() - .await?; - return Ok(()); - } - - let days_ahead: i64 = days_ahead.unwrap(); - - if days_ahead < 0 { + if days.iter().any(|day| *day < 0) { bot.send_message( msg.chat.id, t!("errors.param_no_days_nonnegative", locale = &chat.locale), @@ -191,13 +204,14 @@ async fn set_remind_days_ahead( return Ok(()); } - db.lock().await.transaction::<_, Error, _>(|db| { - use schema::chat::dsl::*; - diesel::update(chat) - .filter(telegram_id.eq(msg.chat.id.0)) - .set(remind_days_ahead.eq(days_ahead)) - .execute(db)?; - Ok(()) + db.lock().await.transaction(|db| { + use schema::reminder::dsl::*; + diesel::delete(reminder.filter(chat_id.eq(chat.db_id))).execute(db)?; + let values = days + .iter() + .map(|days| (chat_id.eq(chat.db_id), days_ahead.eq(days))) + .collect::>(); + diesel::insert_into(reminder).values(&values).execute(db) })?; Ok(()) @@ -220,7 +234,7 @@ pub async fn fetch_and_announce_appointment( .first::(db) })?; - let entry = ChatInfo::from(entry); + let entry = ChatInfo::from_db(entry, vec![]); let is_new_appointment = entry .next_appointment diff --git a/src/db.rs b/src/db.rs index e4cd4ba..06515f4 100644 --- a/src/db.rs +++ b/src/db.rs @@ -1,33 +1,47 @@ use chrono::{DateTime, TimeZone, Utc}; -use diesel::Queryable; +use diesel::{ + associations::{Associations, Identifiable}, + Queryable, Selectable, +}; use crate::appointment::Appointment; +use crate::schema::{chat, reminder}; -#[derive(Queryable)] +#[derive(Queryable, Selectable, Identifiable)] +#[diesel(table_name = chat)] pub struct DbChat { - _id: i32, + id: i32, telegram_id: i64, calendar: String, next_appointment_start: Option, next_appointment_end: Option, last_reminder: Option, - pinned_message: Option, + pinned_message_id: Option, locale: Option, - remind_days_ahead: i64, +} + +#[derive(Queryable, Selectable, Identifiable, Associations)] +#[diesel(belongs_to(DbChat, foreign_key=chat_id))] +#[diesel(table_name = reminder)] +pub struct DbReminder { + id: i32, + chat_id: i32, + days_ahead: i64, } pub struct ChatInfo { + pub db_id: i32, pub id: i64, pub calendar: String, pub next_appointment: Option>, pub last_reminder: Option>, pub pinned_message_id: Option, pub locale: String, - pub remind_days_ahead: u64, + pub remind_days_ahead: Vec, } -impl From for ChatInfo { - fn from(db_chat: DbChat) -> Self { +impl ChatInfo { + pub fn from_db(db_chat: DbChat, db_reminders: Vec) -> Self { let next_appointment = db_chat .next_appointment_start // Join appointments into single option @@ -43,14 +57,18 @@ impl From for ChatInfo { let locale = db_chat.locale.unwrap_or("de".into()); - let remind_days_ahead = db_chat.remind_days_ahead.try_into().unwrap_or(0); + let remind_days_ahead = db_reminders + .into_iter() + .map(|reminder| reminder.days_ahead.try_into().unwrap_or(0)) + .collect(); ChatInfo { + db_id: db_chat.id, id: db_chat.telegram_id, calendar: db_chat.calendar, next_appointment: next_appointment, last_reminder, - pinned_message_id: db_chat.pinned_message, + pinned_message_id: db_chat.pinned_message_id, locale, remind_days_ahead, } diff --git a/src/main.rs b/src/main.rs index 310bc44..4c7ba9c 100644 --- a/src/main.rs +++ b/src/main.rs @@ -14,13 +14,15 @@ use bot::fetch_and_announce_appointment; use chrono::{DateTime, Days, NaiveTime, TimeZone, Utc}; use chrono_tz::Europe; use db::ChatInfo; -use diesel::result::Error::NotFound; -use diesel::{Connection, RunQueryDsl, SqliteConnection}; +use diesel::result::Error::{self, NotFound}; +use diesel::{ + BelongingToDsl, Connection, GroupedBy, RunQueryDsl, SelectableHelper, SqliteConnection, +}; use diesel::{ExpressionMethods, QueryDsl}; use diesel_migrations::{embed_migrations, EmbeddedMigrations, MigrationHarness}; use error::ConfigLoadError; use log::*; -use serde::{de::Error, Deserialize, Deserializer}; +use serde::{Deserialize, Deserializer}; use teloxide::adaptors::Throttle; use teloxide::prelude::RequesterExt; use teloxide::requests::Requester; @@ -29,7 +31,7 @@ use teloxide::{adaptors::throttle::Limits, Bot}; use tokio::time::sleep; use crate::bot::Command; -use crate::db::DbChat; +use crate::db::{DbChat, DbReminder}; #[macro_use] extern crate rust_i18n; @@ -49,7 +51,7 @@ pub struct Config { fn deserialize_time<'de, D: Deserializer<'de>>(deserializer: D) -> Result { let s: String = Deserialize::deserialize(deserializer)?; - NaiveTime::parse_from_str(&s, "%H:%M").map_err(D::Error::custom) + NaiveTime::parse_from_str(&s, "%H:%M").map_err(serde::de::Error::custom) } impl Config { @@ -146,15 +148,29 @@ struct Reminder { // Checks if the date of the next appointment has changed (and announces if so) // Additionally, checks if it is time for a reminder and sends that reminder if necessary async fn check_task(bot: &Throttle, config: &Config, db: &Database) -> Result<()> { - let chats = db.lock().await.transaction(|db| { + let chats = db.lock().await.transaction::<_, Error, _>(|db| { use schema::chat::dsl::*; - chat.load::(db) + use schema::reminder::dsl::*; + let chats = chat.load::(db)?; + + let reminders: Vec = DbReminder::belonging_to(&chats) + .select(DbReminder::as_select()) + .order(days_ahead.asc()) + .load(db)?; + + let reminders_per_chat = reminders + .grouped_by(&chats) + .into_iter() + .zip(chats) + .collect::>(); + + Ok(reminders_per_chat) })?; let now = Utc::now().with_timezone(&Europe::Berlin); - for chat in chats { - let mut chat_info = ChatInfo::from(chat); + for (reminders, chat) in chats { + let mut chat_info = ChatInfo::from_db(chat, reminders); fetch_and_announce_appointment(bot, &mut chat_info, db).await?; let appointment = match chat_info.next_appointment { @@ -170,17 +186,24 @@ async fn check_task(bot: &Throttle, config: &Config, db: &Database) -> Resu text: t!("messages.starting_now", locale = &chat_info.locale), }); } else { - let reminder_day = - appointment.start.date_naive() - Days::new(chat_info.remind_days_ahead); - let reminder_date_time = if chat_info.remind_days_ahead == 0 { - reminder_day.and_time(config.reminder_time) - } else { - reminder_day.and_time(config.preceeding_day_reminder_time) - }; - let reminder_date_time = reminder_date_time - .and_local_timezone(now.timezone()) - .unwrap(); - if now >= reminder_date_time { + // This assumes that remind_days_ahead is sorted in ascending order + let most_recent_active_reminder = chat_info + .remind_days_ahead + .iter() + .map(|days_ahead| { + let reminder_day = appointment.start.date_naive() - Days::new(*days_ahead); + let reminder_date_time = if *days_ahead == 0 { + reminder_day.and_time(config.reminder_time) + } else { + reminder_day.and_time(config.preceeding_day_reminder_time) + }; + reminder_date_time + .and_local_timezone(now.timezone()) + .unwrap() + }) + .find(|reminder_datetime| now >= *reminder_datetime); + + if let Some(reminder_date_time) = most_recent_active_reminder { // TODO This can have weird effects if it's happenig around midnight, since it's not timezone aware (and may even mix multiple timezones) let remaining_time = appointment.start.date_naive() - now.date_naive(); let remaining_days = remaining_time.num_days(); @@ -203,7 +226,7 @@ async fn check_task(bot: &Throttle, config: &Config, db: &Database) -> Resu reminder = Some(Reminder { time: reminder_date_time, text: reminder_text, - }) + }); } } diff --git a/src/schema.rs b/src/schema.rs index aa5af55..d86c0b7 100644 --- a/src/schema.rs +++ b/src/schema.rs @@ -1,15 +1,29 @@ // @generated automatically by Diesel CLI. diesel::table! { - chat (id) { - id -> Integer, - telegram_id -> BigInt, - calendar -> Text, - next_appointment_start -> Nullable, - next_appointment_end -> Nullable, - last_reminder -> Nullable, - pinned_message_id -> Nullable, - locale -> Nullable, - remind_days_ahead -> BigInt, - } + chat (id) { + id -> Integer, + telegram_id -> BigInt, + calendar -> Text, + next_appointment_start -> Nullable, + next_appointment_end -> Nullable, + last_reminder -> Nullable, + pinned_message_id -> Nullable, + locale -> Nullable, + } } + +diesel::table! { + reminder (id) { + id -> Integer, + chat_id -> Integer, + days_ahead -> BigInt, + } +} + +diesel::joinable!(reminder -> chat (chat_id)); + +diesel::allow_tables_to_appear_in_same_query!( + chat, + reminder, +);