Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
169 changes: 168 additions & 1 deletion toki-api/src/routes/time_tracking/timer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,46 @@ use tracing::instrument;

use super::CookieJarResult;

const SAVE_TIMER_PARTIAL_UPDATE_ERROR: &str = "Project/activity update must be atomic: provide projectId, projectName, activityId, and activityName together.";

#[derive(Debug, PartialEq, Eq)]
struct AtomicSaveTimerProjectActivityUpdate {
project_id: String,
project_name: String,
activity_id: String,
activity_name: String,
}

fn parse_save_timer_project_activity_update(
body: &SaveTimerPayload,
) -> Result<Option<AtomicSaveTimerProjectActivityUpdate>, ApiError> {
let has_any_update = body.project_id.is_some()
|| body.project_name.is_some()
|| body.activity_id.is_some()
|| body.activity_name.is_some();

if !has_any_update {
return Ok(None);
}

match (
body.project_id.clone(),
body.project_name.clone(),
body.activity_id.clone(),
body.activity_name.clone(),
) {
(Some(project_id), Some(project_name), Some(activity_id), Some(activity_name)) => {
Ok(Some(AtomicSaveTimerProjectActivityUpdate {
project_id,
project_name,
activity_id,
activity_name,
}))
}
_ => Err(ApiError::bad_request(SAVE_TIMER_PARTIAL_UPDATE_ERROR)),
}
}

// ============================================================================
// Get Timer
// ============================================================================
Expand Down Expand Up @@ -109,6 +149,10 @@ pub async fn stop_timer(
#[serde(rename_all = "camelCase")]
pub struct SaveTimerPayload {
user_note: Option<String>,
project_id: Option<String>,
project_name: Option<String>,
activity_id: Option<String>,
activity_name: Option<String>,
}

#[instrument(name = "save_timer", skip(jar))]
Expand All @@ -123,7 +167,31 @@ pub async fn save_timer(
.create_service(jar, &app_state.cookie_domain)
.await?;

service.save_timer(&user.id, body.user_note).await?;
let parsed_update = parse_save_timer_project_activity_update(&body)?;
let user_note = body.user_note;

// 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.
if let Some(AtomicSaveTimerProjectActivityUpdate {
project_id,
project_name,
activity_id,
activity_name,
}) = parsed_update
{
let current_timer = service
.get_active_timer(&user.id)
.await?
.ok_or_else(|| ApiError::not_found("no active timer found"))?;

let updated_timer = ActiveTimer::new(current_timer.started_at)
.with_project(project_id, project_name)
.with_activity(activity_id, activity_name)
.with_note(current_timer.note);
service.edit_timer(&user.id, &updated_timer).await?;
Comment on lines +187 to +191

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve server timer state when saving from stale clients

This update unconditionally rewrites the active timer with project/activity values from the save payload before persisting the entry, so a stale client can overwrite newer server-side changes and save to the wrong project/activity. In toki-tui, active timer state is only restored during startup (initialize_app_state in toki-tui/src/bootstrap.rs) and is not periodically refreshed, so editing the same timer from another client (web UI/second TUI session) is enough to trigger this. Before this commit, save_timer used the server’s current timer state directly and did not have this overwrite path.

Useful? React with 👍 / 👎.

}

service.save_timer(&user.id, user_note).await?;

Ok((jar, StatusCode::OK))
}
Expand Down Expand Up @@ -227,3 +295,102 @@ pub async fn get_timer_history(

Ok((jar, Json(response)))
}

#[cfg(test)]
mod tests {
use axum::response::IntoResponse;

use super::*;

fn save_payload(
project_id: Option<&str>,
project_name: Option<&str>,
activity_id: Option<&str>,
activity_name: Option<&str>,
) -> SaveTimerPayload {
SaveTimerPayload {
user_note: None,
project_id: project_id.map(ToString::to_string),
project_name: project_name.map(ToString::to_string),
activity_id: activity_id.map(ToString::to_string),
activity_name: activity_name.map(ToString::to_string),
}
}

#[test]
fn parse_save_timer_update_accepts_no_project_or_activity_fields() {
let body = save_payload(None, None, None, None);
let parsed = if let Ok(parsed) = parse_save_timer_project_activity_update(&body) {
parsed
} else {
panic!("expected parse success");
};
assert_eq!(parsed, None);
}

#[test]
fn parse_save_timer_update_accepts_atomic_project_and_activity_fields() {
let body = save_payload(Some("p1"), Some("Project"), Some("a1"), Some("Activity"));
let parsed = if let Ok(parsed) = parse_save_timer_project_activity_update(&body) {
parsed
} else {
panic!("expected parse success");
};
assert_eq!(
parsed,
Some(AtomicSaveTimerProjectActivityUpdate {
project_id: "p1".to_string(),
project_name: "Project".to_string(),
activity_id: "a1".to_string(),
activity_name: "Activity".to_string()
})
);
}

fn assert_bad_request_for_partial_update(body: SaveTimerPayload) {
let err =
parse_save_timer_project_activity_update(&body).expect_err("expected bad request");
let response = err.into_response();
assert_eq!(response.status(), StatusCode::BAD_REQUEST);
}

#[test]
fn parse_save_timer_update_rejects_project_only_pair() {
let body = save_payload(Some("p1"), Some("Project"), None, None);
assert_bad_request_for_partial_update(body);
}

#[test]
fn parse_save_timer_update_rejects_activity_only_pair() {
let body = save_payload(None, None, Some("a1"), Some("Activity"));
assert_bad_request_for_partial_update(body);
}

#[test]
fn parse_save_timer_update_rejects_single_field_partials() {
let cases = [
save_payload(Some("p1"), None, None, None),
save_payload(None, Some("Project"), None, None),
save_payload(None, None, Some("a1"), None),
save_payload(None, None, None, Some("Activity")),
];

for body in cases {
assert_bad_request_for_partial_update(body);
}
}

#[test]
fn parse_save_timer_update_rejects_mixed_partial_pairs() {
let cases = [
save_payload(Some("p1"), Some("Project"), Some("a1"), None),
save_payload(Some("p1"), Some("Project"), None, Some("Activity")),
save_payload(Some("p1"), None, Some("a1"), Some("Activity")),
save_payload(None, Some("Project"), Some("a1"), Some("Activity")),
];

for body in cases {
assert_bad_request_for_partial_update(body);
}
}
}
7 changes: 4 additions & 3 deletions toki-tui/src/api/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@ use std::sync::Arc;
use crate::api::dev_backend::DevBackend;
use crate::api::dto::{
ActivityDto, AuthenticateRequest, DeleteEntryRequest, EditEntryRequest, ProjectDto,
SaveTimerRequest, StartTimerRequest, UpdateActiveTimerRequest,
StartTimerRequest, UpdateActiveTimerRequest,
};
use crate::api::SaveTimerRequest;
use crate::session_store;
use crate::types::{
ActiveTimerState, Activity, GetTimerResponse, Me, Project, TimeEntry, TimeInfo,
Expand Down Expand Up @@ -288,15 +289,15 @@ impl ApiClient {
.await
}

pub async fn save_timer(&mut self, note: Option<String>) -> Result<()> {
pub async fn save_timer(&mut self, request: SaveTimerRequest) -> Result<()> {
if self.dev_backend.is_some() {
return Ok(());
}

self.send_without_body(
self.client
.put(self.endpoint("/time-tracking/timer")?)
.json(&SaveTimerRequest { user_note: note }),
.json(&request),
"PUT /time-tracking/timer",
UNAUTH_RELOGIN,
)
Expand Down
4 changes: 4 additions & 0 deletions toki-tui/src/api/dto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ pub struct StartTimerRequest {
#[serde(rename_all = "camelCase")]
pub struct SaveTimerRequest {
pub user_note: Option<String>,
pub project_id: Option<String>,
pub project_name: Option<String>,
pub activity_id: Option<String>,
pub activity_name: Option<String>,
}

#[derive(Serialize)]
Expand Down
1 change: 1 addition & 0 deletions toki-tui/src/api/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ mod dev_backend;
mod dto;

pub use client::ApiClient;
pub(crate) use dto::SaveTimerRequest;
11 changes: 9 additions & 2 deletions toki-tui/src/runtime/actions.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::api::ApiClient;
use crate::api::{ApiClient, SaveTimerRequest};
use crate::app::{self, App, TextInput};
use crate::types;
use anyhow::{Context, Result};
Expand Down Expand Up @@ -317,9 +317,16 @@ pub(super) async fn handle_save_timer_with_action(

let project_display = app.current_project_name();
let activity_display = app.current_activity_name();
let save_request = SaveTimerRequest {
user_note: note,
project_id: app.selected_project.as_ref().map(|p| p.id.clone()),
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()),
};

// Save the active timer to Milltime
match client.save_timer(note.clone()).await {
match client.save_timer(save_request).await {
Ok(()) => {
let hours = duration.as_secs() / 3600;
let minutes = (duration.as_secs() % 3600) / 60;
Expand Down
Loading