From 13b1565ceaceb6941abfc0e48d03a00670f6e0d2 Mon Sep 17 00:00:00 2001 From: ponbac Date: Fri, 6 Mar 2026 10:59:39 +0100 Subject: [PATCH] Handle locked-period timer saves and surface errors in web and TUI --- app/src/components/cmd-k.tsx | 8 ++ app/src/components/floating-timer.tsx | 11 +++ app/src/lib/api/error.ts | 46 +++++++++++ app/src/lib/api/mutations/time-tracking.ts | 2 + milltime/src/client.rs | 25 ++++-- .../src/adapters/outbound/milltime/mod.rs | 78 ++++++++++++++++++- toki-api/src/domain/error.rs | 4 +- .../src/domain/ports/inbound/time_tracking.rs | 1 + toki-api/src/domain/services/time_tracking.rs | 70 ++++++++++++++--- toki-api/src/routes/error.rs | 19 ++++- toki-api/src/routes/time_tracking/timer.rs | 5 +- toki-tui/src/api/client.rs | 31 +++++++- toki-tui/src/api/dto.rs | 1 + toki-tui/src/main.rs | 12 ++- toki-tui/src/runtime/actions.rs | 18 ++++- toki-tui/src/ui/statistics_view.rs | 5 +- 16 files changed, 303 insertions(+), 33 deletions(-) create mode 100644 app/src/lib/api/error.ts diff --git a/app/src/components/cmd-k.tsx b/app/src/components/cmd-k.tsx index 107c5894..07d94abb 100644 --- a/app/src/components/cmd-k.tsx +++ b/app/src/components/cmd-k.tsx @@ -35,6 +35,8 @@ import { rememberLastProjectAtom, buildRememberedTimerParams, } from "@/lib/time-tracking-preferences"; +import dayjs from "dayjs"; +import { getSaveTimerErrorDescription } from "@/lib/api/error"; export function CmdK() { const [open, setOpen] = React.useState(false); @@ -131,6 +133,11 @@ function ActionsCommandGroup(props: { close: () => void }) { }), }); }, + onError: (error) => { + void getSaveTimerErrorDescription(error).then((description) => { + toast.error("Could not save timer", { description }); + }); + }, }); const saveTimerDisabled = !timer?.activityName || !timer?.projectName; @@ -165,6 +172,7 @@ function ActionsCommandGroup(props: { close: () => void }) { onSelect={() => { saveTimer({ userNote: timer?.note ?? "", + regDay: dayjs().format("YYYY-MM-DD"), }); props.close(); }} diff --git a/app/src/components/floating-timer.tsx b/app/src/components/floating-timer.tsx index 01d40c60..df0b3bed 100644 --- a/app/src/components/floating-timer.tsx +++ b/app/src/components/floating-timer.tsx @@ -29,6 +29,7 @@ import { lastProjectAtom, rememberLastProjectAtom, } from "@/lib/time-tracking-preferences"; +import { getSaveTimerErrorDescription } from "@/lib/api/error"; export const FloatingTimer = () => { const queryClient = useQueryClient(); @@ -199,6 +200,7 @@ export const FloatingTimer = () => { saveTimer( { userNote: userNote ?? "", + regDay: dayjs().format("YYYY-MM-DD"), }, { onSuccess: () => { @@ -241,6 +243,15 @@ export const FloatingTimer = () => { }); } }, + onError: (error) => { + void getSaveTimerErrorDescription(error).then( + (description) => { + toast.error("Could not save timer", { + description, + }); + }, + ); + }, }, ); }} diff --git a/app/src/lib/api/error.ts b/app/src/lib/api/error.ts new file mode 100644 index 00000000..1c0503d7 --- /dev/null +++ b/app/src/lib/api/error.ts @@ -0,0 +1,46 @@ +import { HTTPError } from "ky"; + +type ApiErrorBody = { + error?: string; + code?: string; +}; + +export const TIME_TRACKING_PERIOD_LOCKED_CODE = "TIME_TRACKING_PERIOD_LOCKED"; +const LOCKED_PERIOD_SAVE_TIMER_DESCRIPTION = + "The selected day is in a locked period in Milltime. Unlock it or save to a different date."; + +export async function getApiErrorDetails(error: unknown): Promise<{ + message: string; + code?: string; +}> { + if (error instanceof HTTPError) { + const body = await error.response + .clone() + .json() + .then((value) => value as ApiErrorBody) + .catch(() => null); + + if (body?.error) { + return { message: body.error, code: body.code }; + } + + return { + message: `${error.response.status} ${error.response.statusText}`.trim(), + }; + } + + if (error instanceof Error) { + return { message: error.message }; + } + + return { message: "Unknown error" }; +} + +export async function getSaveTimerErrorDescription(error: unknown): Promise { + const { message, code } = await getApiErrorDetails(error); + if (code === TIME_TRACKING_PERIOD_LOCKED_CODE) { + return LOCKED_PERIOD_SAVE_TIMER_DESCRIPTION; + } + + return message; +} diff --git a/app/src/lib/api/mutations/time-tracking.ts b/app/src/lib/api/mutations/time-tracking.ts index 7b358b93..38c378f5 100644 --- a/app/src/lib/api/mutations/time-tracking.ts +++ b/app/src/lib/api/mutations/time-tracking.ts @@ -100,6 +100,7 @@ function useSaveTimer(options?: DefaultMutationOptions) { api.put("time-tracking/timer", { json: { userNote: body.userNote, + regDay: body.regDay, }, }), ...options, @@ -237,6 +238,7 @@ export type StartTimerMutationAsync = MutationFnAsync; export type SaveTimerPayload = { userNote?: string; + regDay?: string; }; export type EditTimerPayload = { diff --git a/milltime/src/client.rs b/milltime/src/client.rs index ccef14f1..c92b5967 100644 --- a/milltime/src/client.rs +++ b/milltime/src/client.rs @@ -47,12 +47,19 @@ impl MilltimeClient { &self, resp: reqwest::Response, ) -> Result { - if resp.status().is_client_error() { - return match resp.status() { + let status = resp.status(); + if status.is_client_error() { + return match status { reqwest::StatusCode::UNAUTHORIZED | reqwest::StatusCode::FORBIDDEN => { Err(MilltimeFetchError::Unauthorized) } - _ => Err(MilltimeFetchError::ResponseError(resp.status().to_string())), + _ => { + let body = resp.text().await.unwrap_or_default(); + Err(MilltimeFetchError::ResponseError { + status: Some(status), + body, + }) + } }; } @@ -378,8 +385,11 @@ impl MilltimeClient { pub enum MilltimeFetchError { #[error("Unauthorized")] Unauthorized, - #[error("ResponseError: {0}")] - ResponseError(String), + #[error("ResponseError: status={status:?}, body={body}")] + ResponseError { + status: Option, + body: String, + }, #[error("ParsingError: {0}")] ParsingError(String), #[error("Other: {0}")] @@ -388,7 +398,10 @@ pub enum MilltimeFetchError { impl From for MilltimeFetchError { fn from(e: reqwest::Error) -> Self { - Self::ResponseError(e.to_string()) + Self::ResponseError { + status: e.status(), + body: e.to_string(), + } } } diff --git a/toki-api/src/adapters/outbound/milltime/mod.rs b/toki-api/src/adapters/outbound/milltime/mod.rs index 6cdfe5fc..96523792 100644 --- a/toki-api/src/adapters/outbound/milltime/mod.rs +++ b/toki-api/src/adapters/outbound/milltime/mod.rs @@ -156,7 +156,7 @@ impl TimeTrackingClient for MilltimeAdapter { .client .new_project_registration(&payload) .await - .map_err(map_milltime_error)?; + .map_err(map_milltime_registration_write_error)?; Ok(TimerId::new(response.project_registration_id)) } @@ -207,7 +207,7 @@ impl TimeTrackingClient for MilltimeAdapter { .client .new_project_registration(&new_payload) .await - .map_err(map_milltime_error)?; + .map_err(map_milltime_registration_write_error)?; // Delete old registration self.client @@ -234,7 +234,7 @@ impl TimeTrackingClient for MilltimeAdapter { self.client .edit_project_registration(&payload) .await - .map_err(map_milltime_error)?; + .map_err(map_milltime_registration_write_error)?; Ok(TimerId::new(request.registration_id.clone())) } @@ -256,8 +256,78 @@ fn map_milltime_error(e: milltime::MilltimeFetchError) -> TimeTrackingError { { TimeTrackingError::TimerNotFound } - milltime::MilltimeFetchError::ResponseError(msg) => TimeTrackingError::unknown(msg), + milltime::MilltimeFetchError::ResponseError { status, body } => { + TimeTrackingError::unknown(format!("status={status:?}, body={body}")) + } milltime::MilltimeFetchError::ParsingError(msg) => TimeTrackingError::unknown(msg), milltime::MilltimeFetchError::Other(msg) => TimeTrackingError::unknown(msg), } } + +fn map_milltime_registration_write_error(e: milltime::MilltimeFetchError) -> TimeTrackingError { + if is_locked_period_error(&e) { + TimeTrackingError::PeriodLocked + } else { + map_milltime_error(e) + } +} + +fn is_locked_period_error(e: &milltime::MilltimeFetchError) -> bool { + match e { + milltime::MilltimeFetchError::ResponseError { + status: Some(reqwest::StatusCode::NOT_FOUND), + body, + } => { + let body_lower = body.to_lowercase(); + body_lower.contains("timer not found") + || body_lower.contains("locked") + || body_lower.contains("attest") + || body_lower.contains("attester") + } + _ => false, + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn registration_write_maps_locked_period_error() { + let err = milltime::MilltimeFetchError::ResponseError { + status: Some(reqwest::StatusCode::NOT_FOUND), + body: "timer not found".to_string(), + }; + + assert!(matches!( + map_milltime_registration_write_error(err), + TimeTrackingError::PeriodLocked + )); + } + + #[test] + fn registration_write_keeps_non_lock_errors_as_unknown() { + let err = milltime::MilltimeFetchError::ResponseError { + status: Some(reqwest::StatusCode::BAD_REQUEST), + body: "validation failed".to_string(), + }; + + assert!(matches!( + map_milltime_registration_write_error(err), + TimeTrackingError::Unknown(_) + )); + } + + #[test] + fn registration_write_keeps_blank_404_errors_as_unknown() { + let err = milltime::MilltimeFetchError::ResponseError { + status: Some(reqwest::StatusCode::NOT_FOUND), + body: String::new(), + }; + + assert!(matches!( + map_milltime_registration_write_error(err), + TimeTrackingError::Unknown(_) + )); + } +} diff --git a/toki-api/src/domain/error.rs b/toki-api/src/domain/error.rs index 680a1d7b..89a0befc 100644 --- a/toki-api/src/domain/error.rs +++ b/toki-api/src/domain/error.rs @@ -1,7 +1,7 @@ use thiserror::Error; /// Errors that can occur during time tracking operations. -#[derive(Debug, Error)] +#[derive(Debug, Error, PartialEq, Eq)] pub enum TimeTrackingError { #[error("timer not found")] TimerNotFound, @@ -19,6 +19,8 @@ pub enum TimeTrackingError { ActivityNotFound(String), #[error("invalid date range")] InvalidDateRange, + #[error("time period is locked")] + PeriodLocked, #[error("{0}")] Unknown(String), } diff --git a/toki-api/src/domain/ports/inbound/time_tracking.rs b/toki-api/src/domain/ports/inbound/time_tracking.rs index a1bee810..400c13a4 100644 --- a/toki-api/src/domain/ports/inbound/time_tracking.rs +++ b/toki-api/src/domain/ports/inbound/time_tracking.rs @@ -45,6 +45,7 @@ pub trait TimeTrackingService: Send + Sync + 'static { &self, user_id: &UserId, note: Option, + reg_day: Option, ) -> Result; /// Edit the active timer for a user. diff --git a/toki-api/src/domain/services/time_tracking.rs b/toki-api/src/domain/services/time_tracking.rs index c3128258..a75f2219 100644 --- a/toki-api/src/domain/services/time_tracking.rs +++ b/toki-api/src/domain/services/time_tracking.rs @@ -33,6 +33,28 @@ impl TimeTrackingServiceImpl { } } +fn resolve_save_day_and_week( + now: OffsetDateTime, + reg_day: Option<&str>, +) -> Result<(String, i32), TimeTrackingError> { + let format = + time::format_description::parse("[year]-[month]-[day]").expect("valid format description"); + match reg_day { + Some(day) => { + let date = + Date::parse(day, &format).map_err(|_| TimeTrackingError::InvalidDateRange)?; + Ok((day.to_string(), date.iso_week() as i32)) + } + None => { + let day = now + .date() + .format(&format) + .expect("failed to format current day"); + Ok((day, now.iso_week() as i32)) + } + } +} + #[async_trait] impl TimeTrackingService for TimeTrackingServiceImpl @@ -69,6 +91,7 @@ impl TimeTrackingService &self, user_id: &UserId, note: Option, + reg_day: Option, ) -> Result { // Get the active timer let active_timer = self @@ -81,15 +104,7 @@ impl TimeTrackingService const BONUS_TIME_MINUTES: i64 = 1; let now = OffsetDateTime::now_utc(); let end_time = now + time::Duration::minutes(BONUS_TIME_MINUTES); - - let current_day = now - .date() - .format( - &time::format_description::parse("[year]-[month]-[day]") - .expect("valid format description"), - ) - .expect("failed to format current day"); - let week_number = now.iso_week() as i32; + let (save_day, week_number) = resolve_save_day_and_week(now, reg_day.as_deref())?; // Build the create request let req = CreateTimeEntryRequest { @@ -111,7 +126,7 @@ impl TimeTrackingService .ok_or_else(|| TimeTrackingError::unknown("activity name not set on timer"))?, start_time: active_timer.started_at, end_time, - reg_day: current_day, + reg_day: save_day, week_number, note: note.unwrap_or_else(|| active_timer.note.clone()), }; @@ -312,3 +327,38 @@ impl TimeTrackingService self.timer_repo.get_history(user_id).await } } + +#[cfg(test)] +mod tests { + use time::macros::datetime; + + use super::*; + + #[test] + fn resolve_save_day_and_week_uses_provided_reg_day() { + let now = datetime!(2026-03-06 10:00:00 UTC); + let (reg_day, week_number) = + resolve_save_day_and_week(now, Some("2026-03-01")).expect("expected valid reg day"); + + assert_eq!(reg_day, "2026-03-01"); + assert_eq!(week_number, 9); + } + + #[test] + fn resolve_save_day_and_week_falls_back_to_now() { + let now = datetime!(2026-03-06 10:00:00 UTC); + let (reg_day, week_number) = + resolve_save_day_and_week(now, None).expect("expected fallback to now"); + + assert_eq!(reg_day, "2026-03-06"); + assert_eq!(week_number, 10); + } + + #[test] + fn resolve_save_day_and_week_rejects_invalid_date() { + let now = datetime!(2026-03-06 10:00:00 UTC); + let result = resolve_save_day_and_week(now, Some("2026-99-99")); + + assert!(matches!(result, Err(TimeTrackingError::InvalidDateRange))); + } +} diff --git a/toki-api/src/routes/error.rs b/toki-api/src/routes/error.rs index 7e3a1201..2d3a3faf 100644 --- a/toki-api/src/routes/error.rs +++ b/toki-api/src/routes/error.rs @@ -6,10 +6,11 @@ use axum::{ use serde::Serialize; use std::fmt; -#[derive(Debug, Clone, Copy, Serialize)] +#[derive(Debug, Clone, Copy, Serialize, PartialEq, Eq)] #[serde(rename_all = "SCREAMING_SNAKE_CASE")] pub enum ErrorCode { TimeTrackingAuthenticationFailed, + TimeTrackingPeriodLocked, } #[derive(Serialize)] @@ -121,6 +122,8 @@ impl From for ApiError { } TimeTrackingError::TimerAlreadyRunning => Self::conflict(err.to_string()), TimeTrackingError::InvalidDateRange => Self::bad_request(err.to_string()), + TimeTrackingError::PeriodLocked => Self::new(StatusCode::LOCKED, err.to_string()) + .with_code(ErrorCode::TimeTrackingPeriodLocked), _ => Self::internal(err.to_string()), } } @@ -174,3 +177,17 @@ impl From for ApiError { Self::new(err.status, err.message) } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn period_locked_maps_to_locked_status_and_code() { + let api_error = ApiError::from(TimeTrackingError::PeriodLocked); + + assert_eq!(api_error.status, StatusCode::LOCKED); + assert_eq!(api_error.code, Some(ErrorCode::TimeTrackingPeriodLocked)); + assert_eq!(api_error.message, "time period is locked"); + } +} diff --git a/toki-api/src/routes/time_tracking/timer.rs b/toki-api/src/routes/time_tracking/timer.rs index 7389498e..ffaa642f 100644 --- a/toki-api/src/routes/time_tracking/timer.rs +++ b/toki-api/src/routes/time_tracking/timer.rs @@ -153,6 +153,7 @@ pub struct SaveTimerPayload { project_name: Option, activity_id: Option, activity_name: Option, + reg_day: Option, } #[instrument(name = "save_timer", skip(jar))] @@ -169,6 +170,7 @@ pub async fn save_timer( let parsed_update = parse_save_timer_project_activity_update(&body)?; let user_note = body.user_note; + let reg_day = body.reg_day; // Allow clients to include latest project/activity values in the save call. // This makes save robust if a prior timer-edit sync call was missed. @@ -191,7 +193,7 @@ pub async fn save_timer( service.edit_timer(&user.id, &updated_timer).await?; } - service.save_timer(&user.id, user_note).await?; + service.save_timer(&user.id, user_note, reg_day).await?; Ok((jar, StatusCode::OK)) } @@ -314,6 +316,7 @@ mod tests { project_name: project_name.map(ToString::to_string), activity_id: activity_id.map(ToString::to_string), activity_name: activity_name.map(ToString::to_string), + reg_day: None, } } diff --git a/toki-tui/src/api/client.rs b/toki-tui/src/api/client.rs index 8418b27c..14f8c18e 100644 --- a/toki-tui/src/api/client.rs +++ b/toki-tui/src/api/client.rs @@ -4,6 +4,7 @@ use reqwest::{ Client, RequestBuilder, Response, StatusCode, Url, }; use serde::de::DeserializeOwned; +use serde::Deserialize; use std::sync::Arc; use crate::api::dev_backend::DevBackend; @@ -23,6 +24,12 @@ const UNAUTH_INVALID_SESSION: &str = const UNAUTH_RELOGIN: &str = "Session expired. Run `toki-tui login` to re-authenticate."; const UNAUTH_INVALID_MILLTIME_CREDENTIALS: &str = "Invalid Milltime credentials."; +#[derive(Debug, Deserialize)] +struct ApiErrorBody { + error: String, + code: Option, +} + #[derive(Debug, Clone)] pub struct ApiClient { client: Client, @@ -143,9 +150,27 @@ impl ApiClient { anyhow::bail!("{unauthorized_message}"); } - response - .error_for_status_ref() - .with_context(|| format!("{} returned error", call_name))?; + if !response.status().is_success() { + let status = response.status(); + let body_text = response.text().await.unwrap_or_default(); + + if let Ok(api_error) = serde_json::from_str::(&body_text) { + let message = match api_error.code { + Some(code) => format!("{} ({})", api_error.error, code), + None => api_error.error, + }; + anyhow::bail!("{message}"); + } + + if body_text.trim().is_empty() { + anyhow::bail!("{call_name} returned error ({status})"); + } + + anyhow::bail!( + "{call_name} returned error ({status}): {}", + body_text.trim() + ); + } self.sync_mt_cookies_from_jar()?; Ok(response) diff --git a/toki-tui/src/api/dto.rs b/toki-tui/src/api/dto.rs index 00f50a24..c07a828d 100644 --- a/toki-tui/src/api/dto.rs +++ b/toki-tui/src/api/dto.rs @@ -32,6 +32,7 @@ pub struct SaveTimerRequest { pub project_name: Option, pub activity_id: Option, pub activity_name: Option, + pub reg_day: Option, } #[derive(Serialize)] diff --git a/toki-tui/src/main.rs b/toki-tui/src/main.rs index fca00b82..2ba588f0 100644 --- a/toki-tui/src/main.rs +++ b/toki-tui/src/main.rs @@ -34,8 +34,16 @@ async fn main() -> Result<()> { Commands::Status => { let session = session_store::load_session()?; let mt_cookies = session_store::load_mt_cookies()?; - let session_status = if session.is_some() { "logged in" } else { "not logged in" }; - let mt_status = if !mt_cookies.is_empty() { "authenticated" } else { "no cookies" }; + let session_status = if session.is_some() { + "logged in" + } else { + "not logged in" + }; + let mt_status = if !mt_cookies.is_empty() { + "authenticated" + } else { + "no cookies" + }; println!("Azure AD: {}", session_status); println!("Milltime: {}", mt_status); } diff --git a/toki-tui/src/runtime/actions.rs b/toki-tui/src/runtime/actions.rs index 88bc7190..b2c57388 100644 --- a/toki-tui/src/runtime/actions.rs +++ b/toki-tui/src/runtime/actions.rs @@ -362,7 +362,10 @@ async fn yank_entry_to_timer(entry: types::TimeEntry, app: &mut App, client: &mu ) .await { - app.set_status(format!("Warning: Could not sync copied entry to server: {}", e)); + app.set_status(format!( + "Warning: Could not sync copied entry to server: {}", + e + )); } } } @@ -404,6 +407,18 @@ async fn resume_entry(entry: types::TimeEntry, app: &mut App, client: &mut ApiCl } } +fn local_reg_day() -> String { + let local_offset = time::UtcOffset::current_local_offset().unwrap_or(time::UtcOffset::UTC); + let now_local = time::OffsetDateTime::now_utc().to_offset(local_offset); + + format!( + "{:04}-{:02}-{:02}", + now_local.year(), + now_local.month() as u8, + now_local.day() + ) +} + pub(super) async fn handle_save_timer_with_action( app: &mut App, client: &mut ApiClient, @@ -429,6 +444,7 @@ pub(super) async fn handle_save_timer_with_action( project_name: app.selected_project.as_ref().map(|p| p.name.clone()), activity_id: app.selected_activity.as_ref().map(|a| a.id.clone()), activity_name: app.selected_activity.as_ref().map(|a| a.name.clone()), + reg_day: Some(local_reg_day()), }; // Save the active timer to Milltime diff --git a/toki-tui/src/ui/statistics_view.rs b/toki-tui/src/ui/statistics_view.rs index 60a6a04b..0f035102 100644 --- a/toki-tui/src/ui/statistics_view.rs +++ b/toki-tui/src/ui/statistics_view.rs @@ -29,10 +29,7 @@ pub fn render_statistics_view(frame: &mut Frame, app: &App, body: Rect) { let stats_block = Block::default() .borders(Borders::ALL) .border_style(Style::default().fg(Color::White)) - .title(Span::styled( - " Stats ", - Style::default().fg(Color::White), - )); + .title(Span::styled(" Stats ", Style::default().fg(Color::White))); let stats_inner = stats_block.inner(outer[0]); frame.render_widget(stats_block, outer[0]);