Allow multiple reminder dates for a single chat group

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"

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"

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;

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;

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<Bot>, msg: Message, db: Database) -> Result<
 	})?;
 
 	let mut chat_info = ChatInfo::<Utc> {
+		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::<DbChat>(db)
 	})?;
 
-	let chat = ChatInfo::<Utc>::from(chat);
+	let chat = ChatInfo::<Utc>::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::<Result<Vec<i64>, _>>()
+		{
+			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::<Vec<_>>();
+		diesel::insert_into(reminder).values(&values).execute(db)
 	})?;
 
 	Ok(())
@@ -220,7 +234,7 @@ pub async fn fetch_and_announce_appointment(
 			.first::<DbChat>(db)
 	})?;
 
-	let entry = ChatInfo::from(entry);
+	let entry = ChatInfo::from_db(entry, vec![]);
 
 	let is_new_appointment = entry
 		.next_appointment

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<i64>,
 	next_appointment_end: Option<i64>,
 	last_reminder: Option<i64>,
-	pinned_message: Option<i32>,
+	pinned_message_id: Option<i32>,
 	locale: Option<String>,
-	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<Tz: TimeZone> {
+	pub db_id: i32,
 	pub id: i64,
 	pub calendar: String,
 	pub next_appointment: Option<Appointment<Tz>>,
 	pub last_reminder: Option<DateTime<Tz>>,
 	pub pinned_message_id: Option<i32>,
 	pub locale: String,
-	pub remind_days_ahead: u64,
+	pub remind_days_ahead: Vec<u64>,
 }
 
-impl From<DbChat> for ChatInfo<Utc> {
-	fn from(db_chat: DbChat) -> Self {
+impl ChatInfo<Utc> {
+	pub fn from_db(db_chat: DbChat, db_reminders: Vec<DbReminder>) -> Self {
 		let next_appointment = db_chat
 			.next_appointment_start
 			// Join appointments into single option
@@ -43,14 +57,18 @@ impl From<DbChat> for ChatInfo<Utc> {
 
 		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,
 		}

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<NaiveTime, D::Error> {
 	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<Tz: TimeZone> {
 // 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<Bot>, 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::<DbChat>(db)
+		use schema::reminder::dsl::*;
+		let chats = chat.load::<DbChat>(db)?;
+
+		let reminders: Vec<DbReminder> = 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::<Vec<_>>();
+
+		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<Bot>, 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<Bot>, config: &Config, db: &Database) -> Resu
 				reminder = Some(Reminder {
 					time: reminder_date_time,
 					text: reminder_text,
-				})
+				});
 			}
 		}
 

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<BigInt>,
-		next_appointment_end -> Nullable<BigInt>,
-		last_reminder -> Nullable<BigInt>,
-		pinned_message_id -> Nullable<Integer>,
-		locale -> Nullable<Text>,
-		remind_days_ahead -> BigInt,
-	}
+    chat (id) {
+        id -> Integer,
+        telegram_id -> BigInt,
+        calendar -> Text,
+        next_appointment_start -> Nullable<BigInt>,
+        next_appointment_end -> Nullable<BigInt>,
+        last_reminder -> Nullable<BigInt>,
+        pinned_message_id -> Nullable<Integer>,
+        locale -> Nullable<Text>,
+    }
 }
+
+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,
+);