feat: smart day string, terse refactor and use types properly, work on unimplemented commands lightly, util modules,

src/bot/commands/gcal.rs

@@ -1,7 +1,7 @@
 //! Google Calendar command implementation.
 
-use crate::banner::{Course, DayOfWeek, MeetingScheduleInfo, Term};
-use crate::bot::{Context, Error};
+use crate::banner::{Course, DayOfWeek, MeetingScheduleInfo};
+use crate::bot::{Context, Error, utils};
 use chrono::NaiveDate;
 use std::collections::HashMap;
 use tracing::{error, info};
@@ -18,34 +18,21 @@ pub async fn gcal(
 
     ctx.defer().await?;
 
-    let app_state = &ctx.data().app_state;
-    let banner_api = &app_state.banner_api;
-
-    // Get current term dynamically
-    let current_term_status = Term::get_current();
-    let term = current_term_status.inner();
-
-    // Fetch live course data from Redis cache via AppState
-    let course = match app_state
-        .get_course_or_fetch(&term.to_string(), &crn.to_string())
-        .await
-    {
-        Ok(course) => course,
-        Err(e) => {
-            error!(%e, crn, "failed to fetch course data");
-            return Err(Error::from(e));
-        }
-    };
+    let course = utils::get_course_by_crn(&ctx, crn).await?;
+    let term = course.term.clone();
 
     // Get meeting times
-    let meeting_times = match banner_api
-        .get_course_meeting_time(&term.to_string(), crn)
+    let meeting_times = match ctx
+        .data()
+        .app_state
+        .banner_api
+        .get_course_meeting_time(&term, &crn.to_string())
         .await
     {
         Ok(meeting_time) => meeting_time,
         Err(e) => {
             error!("failed to get meeting times: {}", e);
-            return Err(Error::from(e));
+            return Err(e);
         }
     };
 
@@ -74,8 +61,10 @@ pub async fn gcal(
                 .map(|m| {
                     let link = generate_gcal_url(&course, m)?;
                     let detail = match &m.time_range {
-                        Some(range) => format!("{} {}", m.days_string(), range.format_12hr()),
-                        None => m.days_string(),
+                        Some(range) => {
+                            format!("{} {}", m.days_string().unwrap(), range.format_12hr())
+                        }
+                        None => m.days_string().unwrap(),
                     };
                     Ok(LinkDetail { link, detail })
                 })
@@ -104,10 +93,7 @@ fn generate_gcal_url(
     course: &Course,
     meeting_time: &MeetingScheduleInfo,
 ) -> Result<String, anyhow::Error> {
-    let course_text = format!(
-        "{} {} - {}",
-        course.subject, course.course_number, course.course_title
-    );
+    let course_text = course.display_title();
 
     let dates_text = {
         let (start, end) = meeting_time.datetime_range();
@@ -119,18 +105,14 @@ fn generate_gcal_url(
     };
 
     // Get instructor name
-    let instructor_name = if !course.faculty.is_empty() {
-        &course.faculty[0].display_name
-    } else {
-        "Unknown"
-    };
+    let instructor_name = course.primary_instructor_name();
 
     // The event description
     let details_text = format!(
         "CRN: {}\nInstructor: {}\nDays: {}",
         course.course_reference_number,
         instructor_name,
-        meeting_time.days_string()
+        meeting_time.days_string().unwrap()
     );
 
     // The event location

src/bot/commands/ics.rs

@@ -1,6 +1,7 @@
 //! ICS command implementation for generating calendar files.
 
-use crate::bot::{Context, Error};
+use crate::bot::{Context, Error, utils};
+use tracing::info;
 
 /// Generate an ICS file for a course
 #[poise::command(slash_command, prefix_command)]
@@ -10,16 +11,15 @@ pub async fn ics(
 ) -> Result<(), Error> {
     ctx.defer().await?;
 
-    // TODO: Get BannerApi from context or global state
-    // TODO: Get current term dynamically
-    let term = 202510; // Hardcoded for now
+    let course = utils::get_course_by_crn(&ctx, crn).await?;
 
     // TODO: Implement actual ICS file generation
     ctx.say(format!(
-        "ICS command not yet implemented - BannerApi integration needed\nCRN: {}, Term: {}",
-        crn, term
+        "ICS generation for '{}' is not yet implemented.",
+        course.display_title()
     ))
     .await?;
 
+    info!("ics command completed for CRN: {}", crn);
     Ok(())
 }

src/bot/commands/search.rs

@@ -1,8 +1,10 @@
 //! Course search command implementation.
 
-use crate::banner::SearchQuery;
+use crate::banner::{SearchQuery, Term};
 use crate::bot::{Context, Error};
+use anyhow::anyhow;
 use regex::Regex;
+use tracing::info;
 
 /// Search for courses with various filters
 #[poise::command(slash_command, prefix_command)]
@@ -40,12 +42,37 @@ pub async fn search(
         query = query.max_results(max_results.min(25)); // Cap at 25
     }
 
-    // TODO: Get current term dynamically
-    // TODO: Get BannerApi from context or global state
-    // For now, we'll return an error
-    ctx.say("Search functionality not yet implemented - BannerApi integration needed")
+    let term = Term::get_current().inner().to_string();
+    let search_result = ctx
+        .data()
+        .app_state
+        .banner_api
+        .search(&term, &query, "subjectDescription", false)
         .await?;
 
+    let response = if let Some(courses) = search_result.data {
+        if courses.is_empty() {
+            "No courses found with the specified criteria.".to_string()
+        } else {
+            courses
+                .iter()
+                .map(|course| {
+                    format!(
+                        "**{}**: {} ({})",
+                        course.display_title(),
+                        course.primary_instructor_name(),
+                        course.course_reference_number
+                    )
+                })
+                .collect::<Vec<_>>()
+                .join("\n")
+        }
+    } else {
+        "No courses found with the specified criteria.".to_string()
+    };
+
+    ctx.say(response).await?;
+    info!("search command completed");
     Ok(())
 }
 
@@ -65,22 +92,24 @@ fn parse_course_code(input: &str) -> Result<(i32, i32), Error> {
             };
 
             if low > high {
-                return Err("Invalid range: low value greater than high value".into());
+                return Err(anyhow!(
+                    "Invalid range: low value greater than high value"
+                ));
             }
 
             if low < 1000 || high > 9999 {
-                return Err("Course codes must be between 1000 and 9999".into());
+                return Err(anyhow!("Course codes must be between 1000 and 9999"));
             }
 
             return Ok((low, high));
         }
-        return Err("Invalid range format".into());
+        return Err(anyhow!("Invalid range format"));
     }
 
     // Handle wildcard format (e.g, "34xx")
     if input.contains('x') {
         if input.len() != 4 {
-            return Err("Wildcard format must be exactly 4 characters".into());
+            return Err(anyhow!("Wildcard format must be exactly 4 characters"));
         }
 
         let re = Regex::new(r"(\d+)(x+)").unwrap();
@@ -92,22 +121,22 @@ fn parse_course_code(input: &str) -> Result<(i32, i32), Error> {
             let high = low + 10_i32.pow(x_count as u32) - 1;
 
             if low < 1000 || high > 9999 {
-                return Err("Course codes must be between 1000 and 9999".into());
+                return Err(anyhow!("Course codes must be between 1000 and 9999"));
             }
 
             return Ok((low, high));
         }
-        return Err("Invalid wildcard format".into());
+        return Err(anyhow!("Invalid wildcard format"));
     }
 
     // Handle single course code
     if input.len() == 4 {
         let code: i32 = input.parse()?;
         if !(1000..=9999).contains(&code) {
-            return Err("Course codes must be between 1000 and 9999".into());
+            return Err(anyhow!("Course codes must be between 1000 and 9999"));
         }
         return Ok((code, code));
     }
 
-    Err("Invalid course code format".into())
+    Err(anyhow!("Invalid course code format"))
 }

src/bot/commands/terms.rs

@@ -1,6 +1,8 @@
 //! Terms command implementation.
 
+use crate::banner::{BannerTerm, Term};
 use crate::bot::{Context, Error};
+use tracing::info;
 
 /// List available terms or search for a specific term
 #[poise::command(slash_command, prefix_command)]
@@ -13,14 +15,40 @@ pub async fn terms(
 
     let search_term = search.unwrap_or_default();
     let page_number = page.unwrap_or(1).max(1);
+    let max_results = 10;
 
-    // TODO: Get BannerApi from context or global state
-    // For now, we'll return a placeholder response
-    ctx.say(format!(
-        "Terms command not yet implemented - BannerApi integration needed\nSearch: '{}', Page: {}",
-        search_term, page_number
-    ))
-    .await?;
+    let terms = ctx
+        .data()
+        .app_state
+        .banner_api
+        .get_terms(&search_term, page_number, max_results)
+        .await?;
 
+    let response = if terms.is_empty() {
+        "No terms found.".to_string()
+    } else {
+        let current_term_code = Term::get_current().inner().to_string();
+        terms
+            .iter()
+            .map(|term| format_term(term, &current_term_code))
+            .collect::<Vec<_>>()
+            .join("\n")
+    };
+
+    ctx.say(response).await?;
+    info!("terms command completed");
     Ok(())
 }
+
+fn format_term(term: &BannerTerm, current_term_code: &str) -> String {
+    let is_current = if term.code == current_term_code {
+        " (current)"
+    } else {
+        ""
+    };
+    let is_archived = if term.is_archived() { " (archived)" } else { "" };
+    format!(
+        "- `{}`: {}{}{}",
+        term.code, term.description, is_current, is_archived
+    )
+}

src/bot/commands/time.rs

@@ -1,6 +1,7 @@
 //! Time command implementation for course meeting times.
 
-use crate::bot::{Context, Error};
+use crate::bot::{utils, Context, Error};
+use tracing::info;
 
 /// Get meeting times for a specific course
 #[poise::command(slash_command, prefix_command)]
@@ -10,16 +11,15 @@ pub async fn time(
 ) -> Result<(), Error> {
     ctx.defer().await?;
 
-    // TODO: Get BannerApi from context or global state
-    // TODO: Get current term dynamically
-    let term = 202510; // Hardcoded for now
+    let course = utils::get_course_by_crn(&ctx, crn).await?;
 
-    // TODO: Implement actual meeting time retrieval
+    // TODO: Implement actual meeting time retrieval and display
     ctx.say(format!(
-        "Time command not yet implemented - BannerApi integration needed\nCRN: {}, Term: {}",
-        crn, term
+        "Meeting time display for '{}' is not yet implemented.",
+        course.display_title()
     ))
     .await?;
 
+    info!("time command completed for CRN: {}", crn);
     Ok(())
 }

src/bot/mod.rs

@@ -1,12 +1,13 @@
 use crate::app_state::AppState;
+use crate::error::Error;
 
 pub mod commands;
+pub mod utils;
 
 #[derive(Debug)]
 pub struct Data {
     pub app_state: AppState,
 } // User data, which is stored and accessible in all command invocations
-pub type Error = Box<dyn std::error::Error + Send + Sync>;
 pub type Context<'a> = poise::Context<'a, Data, Error>;
 
 /// Get all available commands

src/bot/utils.rs

@@ -0,0 +1,24 @@
+//! Bot command utilities.
+
+use crate::banner::{Course, Term};
+use crate::bot::Context;
+use crate::error::Result;
+use tracing::error;
+
+/// Gets a course by its CRN for the current term.
+pub async fn get_course_by_crn(ctx: &Context<'_>, crn: i32) -> Result<Course> {
+    let app_state = &ctx.data().app_state;
+
+    // Get current term dynamically
+    let current_term_status = Term::get_current();
+    let term = current_term_status.inner();
+
+    // Fetch live course data from Redis cache via AppState
+    app_state
+        .get_course_or_fetch(&term.to_string(), &crn.to_string())
+        .await
+        .map_err(|e| {
+            error!(%e, crn, "failed to fetch course data");
+            e
+        })
+}