feat: store bag finished_at as full datetime for correct timeline ordering

The finished_at field previously stored only a date, causing bag "finished"
timeline events to sort before same-day brews. Now stores a full datetime
and uses the actual close time for timeline ordering.
This commit is contained in:
Jon Seager 2026-02-24 21:38:39 +00:00
parent 6a2bd5e563
commit 937ee4c5f2
No known key found for this signature in database
11 changed files with 129 additions and 26 deletions

View file

@ -0,0 +1,34 @@
-- Convert date-only finished_at values to full ISO 8601 datetimes.
-- Going forward, finished_at stores full datetimes for correct timeline
-- ordering (bag "finished" events must sort after same-day brews).
--
-- Strategy: set finished_at to 1 minute after the last brew from that bag.
-- If no brews exist for the bag, fall back to end-of-day (23:59:59Z).
UPDATE bags
SET finished_at = COALESCE(
(
SELECT strftime('%Y-%m-%dT%H:%M:%fZ', brews.created_at, '+1 minute')
FROM brews
WHERE brews.bag_id = bags.id
ORDER BY brews.created_at DESC
LIMIT 1
),
finished_at || 'T23:59:59Z'
)
WHERE finished_at IS NOT NULL
AND LENGTH(finished_at) = 10;
-- Fix existing timeline events so bag "finished" uses the actual finished_at.
UPDATE timeline_events
SET occurred_at = (
SELECT bags.finished_at
FROM bags
WHERE bags.id = timeline_events.entity_id
)
WHERE entity_type = 'bag'
AND action = 'finished'
AND EXISTS (
SELECT 1 FROM bags
WHERE bags.id = timeline_events.entity_id
AND bags.finished_at IS NOT NULL
);

View file

@ -136,8 +136,11 @@ pub(crate) struct UpdateBagSubmission {
remaining: Option<f64>, remaining: Option<f64>,
#[serde(default)] #[serde(default)]
closed: Option<bool>, closed: Option<bool>,
#[serde(default)] #[serde(
finished_at: Option<chrono::NaiveDate>, default,
deserialize_with = "crate::domain::bags::deserialize_flexible_finished_at"
)]
finished_at: Option<DateTime<Utc>>,
#[serde(default)] #[serde(default)]
created_at: Option<DateTime<Utc>>, created_at: Option<DateTime<Utc>>,
#[serde(default)] #[serde(default)]

View file

@ -43,7 +43,7 @@ impl BagService {
/// repository, and records a "finished" timeline event. /// repository, and records a "finished" timeline event.
pub async fn finish(&self, id: BagId, mut update: UpdateBag) -> Result<Bag, RepositoryError> { pub async fn finish(&self, id: BagId, mut update: UpdateBag) -> Result<Bag, RepositoryError> {
if update.finished_at.is_none() { if update.finished_at.is_none() {
update.finished_at = Some(chrono::Utc::now().date_naive()); update.finished_at = Some(chrono::Utc::now());
} }
let bag = self.bag_repo.update(id, update).await?; let bag = self.bag_repo.update(id, update).await?;
self.record_timeline_event(&bag, "finished").await; self.record_timeline_event(&bag, "finished").await;

View file

@ -294,15 +294,29 @@ async fn refresh_entity_event(
roast_timeline_event(&rwr.roast, &roaster) roast_timeline_event(&rwr.roast, &roaster)
} }
EntityType::Bag => { EntityType::Bag => {
// Bags have two timeline events ("added" + optional "finished"),
// so delete-and-reinsert to keep occurred_at in sync with finished_at.
let bwr = rebuilder let bwr = rebuilder
.bag_repo .bag_repo
.get_with_roast(BagId::new(entity_id)) .get_with_roast(BagId::new(entity_id))
.await?; .await?;
let roast = rebuilder.roast_repo.get(bwr.bag.roast_id).await?; let roast = rebuilder.roast_repo.get(bwr.bag.roast_id).await?;
let roaster = rebuilder.roaster_repo.get(roast.roaster_id).await?; let roaster = rebuilder.roaster_repo.get(roast.roaster_id).await?;
// update_by_entity updates all events for this entity, rebuilder
// preserving each event's original action ("added" or "finished") .timeline_repo
bag_timeline_event(&bwr.bag, "added", &roast, &roaster) .delete_by_entity(entity_type, entity_id)
.await?;
rebuilder
.timeline_repo
.insert(bag_timeline_event(&bwr.bag, "added", &roast, &roaster))
.await?;
if bwr.bag.closed {
rebuilder
.timeline_repo
.insert(bag_timeline_event(&bwr.bag, "finished", &roast, &roaster))
.await?;
}
return Ok(());
} }
EntityType::Brew => { EntityType::Brew => {
let enriched = rebuilder let enriched = rebuilder
@ -458,10 +472,7 @@ async fn rebuild_bag_events(
warn!(error = %err, id = %bwr.bag.id, "failed to rebuild bag 'added' timeline event"); warn!(error = %err, id = %bwr.bag.id, "failed to rebuild bag 'added' timeline event");
} }
if bwr.bag.closed { if bwr.bag.closed {
let mut finished_event = bag_timeline_event(&bwr.bag, "finished", &roast, &roaster); let finished_event = bag_timeline_event(&bwr.bag, "finished", &roast, &roaster);
if let Some(finished_at) = bwr.bag.finished_at {
finished_event.occurred_at = finished_at.and_time(chrono::NaiveTime::MIN).and_utc();
}
if let Err(err) = rebuilder.timeline_repo.insert(finished_event).await { if let Err(err) = rebuilder.timeline_repo.insert(finished_event).await {
warn!(error = %err, id = %bwr.bag.id, "failed to rebuild bag 'finished' timeline event"); warn!(error = %err, id = %bwr.bag.id, "failed to rebuild bag 'finished' timeline event");
} }

View file

@ -1,4 +1,4 @@
use chrono::{DateTime, NaiveDate, Utc}; use chrono::{DateTime, NaiveDate, NaiveTime, Utc};
use serde::{Deserialize, Serialize}; use serde::{Deserialize, Serialize};
use crate::define_sort_key; use crate::define_sort_key;
@ -8,6 +8,40 @@ use crate::domain::roasters::Roaster;
use crate::domain::roasts::Roast; use crate::domain::roasts::Roast;
use crate::domain::timeline::{NewTimelineEvent, TimelineEventDetail}; use crate::domain::timeline::{NewTimelineEvent, TimelineEventDetail};
/// 23:59:59 — used when converting a date-only `finished_at` to a datetime
/// so that bag "finished" events sort after same-day brews.
pub const END_OF_DAY: NaiveTime = match NaiveTime::from_hms_opt(23, 59, 59) {
Some(t) => t,
None => unreachable!(),
};
/// Deserializes a datetime that accepts both RFC 3339 (`2025-02-24T15:30:00Z`)
/// and date-only (`2025-02-24`) formats. Date-only values become 23:59:59 UTC
/// so bag "finished" events sort after same-day brews.
pub(crate) fn deserialize_flexible_finished_at<'de, D>(
deserializer: D,
) -> Result<Option<DateTime<Utc>>, D::Error>
where
D: serde::Deserializer<'de>,
{
let opt: Option<String> = Option::deserialize(deserializer)?;
match opt {
None => Ok(None),
Some(s) if s.is_empty() => Ok(None),
Some(s) => {
if let Ok(dt) = DateTime::parse_from_rfc3339(&s) {
Ok(Some(dt.with_timezone(&Utc)))
} else if let Ok(date) = NaiveDate::parse_from_str(&s, "%Y-%m-%d") {
Ok(Some(date.and_time(END_OF_DAY).and_utc()))
} else {
Err(serde::de::Error::custom(format!(
"invalid finished_at: expected RFC 3339 or YYYY-MM-DD, got: {s}"
)))
}
}
}
}
#[derive(Debug, Clone, Serialize, Deserialize)] #[derive(Debug, Clone, Serialize, Deserialize)]
pub struct Bag { pub struct Bag {
pub id: BagId, pub id: BagId,
@ -16,7 +50,8 @@ pub struct Bag {
pub amount: f64, pub amount: f64,
pub remaining: f64, pub remaining: f64,
pub closed: bool, pub closed: bool,
pub finished_at: Option<NaiveDate>, #[serde(default, deserialize_with = "deserialize_flexible_finished_at")]
pub finished_at: Option<DateTime<Utc>>,
pub created_at: DateTime<Utc>, pub created_at: DateTime<Utc>,
pub updated_at: DateTime<Utc>, pub updated_at: DateTime<Utc>,
} }
@ -50,7 +85,8 @@ pub struct UpdateBag {
pub amount: Option<f64>, pub amount: Option<f64>,
pub remaining: Option<f64>, pub remaining: Option<f64>,
pub closed: Option<bool>, pub closed: Option<bool>,
pub finished_at: Option<NaiveDate>, #[serde(default, deserialize_with = "deserialize_flexible_finished_at")]
pub finished_at: Option<DateTime<Utc>>,
#[serde(default, skip_serializing_if = "Option::is_none")] #[serde(default, skip_serializing_if = "Option::is_none")]
pub created_at: Option<DateTime<Utc>>, pub created_at: Option<DateTime<Utc>>,
} }
@ -110,11 +146,16 @@ pub fn bag_timeline_event(
roast: &Roast, roast: &Roast,
roaster: &Roaster, roaster: &Roaster,
) -> NewTimelineEvent { ) -> NewTimelineEvent {
let occurred_at = if action == "finished" {
bag.finished_at.unwrap_or(bag.created_at)
} else {
bag.created_at
};
NewTimelineEvent { NewTimelineEvent {
entity_type: EntityType::Bag, entity_type: EntityType::Bag,
entity_id: bag.id.into_inner(), entity_id: bag.id.into_inner(),
action: action.to_string(), action: action.to_string(),
occurred_at: bag.created_at, occurred_at,
title: roast.name.clone(), title: roast.name.clone(),
details: vec![ details: vec![
TimelineEventDetail { TimelineEventDetail {

View file

@ -685,7 +685,7 @@ struct BagRecord {
amount: f64, amount: f64,
remaining: f64, remaining: f64,
closed: bool, closed: bool,
finished_at: Option<NaiveDate>, finished_at: Option<DateTime<Utc>>,
created_at: DateTime<Utc>, created_at: DateTime<Utc>,
updated_at: DateTime<Utc>, updated_at: DateTime<Utc>,
} }

View file

@ -77,7 +77,7 @@ impl<'a> BagsClient<'a> {
id: BagId, id: BagId,
remaining: Option<f64>, remaining: Option<f64>,
closed: Option<bool>, closed: Option<bool>,
finished_at: Option<NaiveDate>, finished_at: Option<DateTime<Utc>>,
created_at: Option<DateTime<Utc>>, created_at: Option<DateTime<Utc>>,
) -> Result<BagWithRoast> { ) -> Result<BagWithRoast> {
let url = self.inner.endpoint(&format!("api/v1/bags/{id}"))?; let url = self.inner.endpoint(&format!("api/v1/bags/{id}"))?;

View file

@ -227,7 +227,7 @@ struct BagRecord {
amount: f64, amount: f64,
remaining: f64, remaining: f64,
closed: bool, closed: bool,
finished_at: Option<NaiveDate>, finished_at: Option<DateTime<Utc>>,
created_at: DateTime<Utc>, created_at: DateTime<Utc>,
updated_at: DateTime<Utc>, updated_at: DateTime<Utc>,
} }
@ -256,7 +256,7 @@ struct BagWithRoastRecord {
amount: f64, amount: f64,
remaining: f64, remaining: f64,
closed: bool, closed: bool,
finished_at: Option<NaiveDate>, finished_at: Option<DateTime<Utc>>,
created_at: DateTime<Utc>, created_at: DateTime<Utc>,
updated_at: DateTime<Utc>, updated_at: DateTime<Utc>,
roast_name: String, roast_name: String,

View file

@ -2,8 +2,8 @@ use anyhow::Result;
use clap::{Args, Subcommand}; use clap::{Args, Subcommand};
use super::macros::{define_delete_command, define_get_command}; use super::macros::{define_delete_command, define_get_command};
use super::parse_created_at;
use super::print_json; use super::print_json;
use super::{parse_created_at, parse_finished_at};
use crate::domain::ids::{BagId, RoastId}; use crate::domain::ids::{BagId, RoastId};
use crate::infrastructure::client::BrewlogClient; use crate::infrastructure::client::BrewlogClient;
@ -99,7 +99,7 @@ pub struct UpdateBagCommand {
pub async fn update_bag(client: &BrewlogClient, command: UpdateBagCommand) -> Result<()> { pub async fn update_bag(client: &BrewlogClient, command: UpdateBagCommand) -> Result<()> {
let finished_at = command let finished_at = command
.finished_at .finished_at
.map(|d| chrono::NaiveDate::parse_from_str(&d, "%Y-%m-%d")) .map(|d| parse_finished_at(&d))
.transpose()?; .transpose()?;
let created_at = command let created_at = command
.created_at .created_at

View file

@ -158,6 +158,20 @@ pub fn parse_created_at(value: &str) -> anyhow::Result<DateTime<Utc>> {
) )
} }
/// Like `parse_created_at` but date-only values become 23:59:59 UTC so
/// bag "finished" events sort after same-day brews.
pub fn parse_finished_at(value: &str) -> anyhow::Result<DateTime<Utc>> {
if let Ok(dt) = DateTime::parse_from_rfc3339(value) {
return Ok(dt.with_timezone(&Utc));
}
if let Ok(date) = NaiveDate::parse_from_str(value, "%Y-%m-%d") {
return Ok(date.and_time(crate::domain::bags::END_OF_DAY).and_utc());
}
anyhow::bail!(
"invalid date format: expected RFC 3339 (e.g. 2025-08-05T10:00:00Z) or YYYY-MM-DD"
)
}
pub(crate) fn print_json<T>(value: &T) -> anyhow::Result<()> pub(crate) fn print_json<T>(value: &T) -> anyhow::Result<()>
where where
T: serde::Serialize, T: serde::Serialize,

View file

@ -4,7 +4,7 @@ use crate::helpers::{
}; };
use crate::test_macros::define_crud_tests; use crate::test_macros::define_crud_tests;
use brewlog::domain::bags::{Bag, BagWithRoast, NewBag, UpdateBag}; use brewlog::domain::bags::{Bag, BagWithRoast, NewBag, UpdateBag};
use chrono::NaiveDate; use chrono::{NaiveDate, TimeZone, Utc};
define_crud_tests!( define_crud_tests!(
entity: bag, entity: bag,
@ -186,7 +186,7 @@ async fn updating_a_bag_returns_200_and_updates_data() {
let update_payload = UpdateBag { let update_payload = UpdateBag {
remaining: Some(100.0), remaining: Some(100.0),
closed: Some(true), closed: Some(true),
finished_at: Some(NaiveDate::from_ymd_opt(2023, 2, 1).unwrap()), finished_at: Some(Utc.with_ymd_and_hms(2023, 2, 1, 23, 59, 59).unwrap()),
..Default::default() ..Default::default()
}; };
@ -206,7 +206,7 @@ async fn updating_a_bag_returns_200_and_updates_data() {
assert!(updated_bag.closed); assert!(updated_bag.closed);
assert_eq!( assert_eq!(
updated_bag.finished_at, updated_bag.finished_at,
Some(NaiveDate::from_ymd_opt(2023, 2, 1).unwrap()) Some(Utc.with_ymd_and_hms(2023, 2, 1, 23, 59, 59).unwrap())
); );
} }
@ -308,10 +308,10 @@ async fn closing_a_bag_automatically_sets_finished_at() {
// Accept today or yesterday to avoid midnight-boundary flakiness // Accept today or yesterday to avoid midnight-boundary flakiness
let today = chrono::Utc::now().date_naive(); let today = chrono::Utc::now().date_naive();
let yesterday = today - chrono::Duration::days(1); let yesterday = today - chrono::Duration::days(1);
let finished = updated_bag.finished_at.unwrap(); let finished_date = updated_bag.finished_at.unwrap().date_naive();
assert!( assert!(
finished == today || finished == yesterday, finished_date == today || finished_date == yesterday,
"expected finished_at to be today or yesterday, got {finished}" "expected finished_at date to be today or yesterday, got {finished_date}"
); );
} }