diff --git a/crates/shell/src/approvals/mod.rs b/crates/shell/src/approvals/mod.rs index 9c914e2e..33168e40 100644 --- a/crates/shell/src/approvals/mod.rs +++ b/crates/shell/src/approvals/mod.rs @@ -21,7 +21,8 @@ //! [`contacts::CONTACTS_FILE`] and [`audit::AUDIT_FILE`], all owner-only. //! //! The shell calls [`init`] at startup, [`tick`] once a second (and shows -//! [`take_notices`] as notifications), and gives pointer events to +//! [`take_notices`] as notifications, withdrawing a request's own once it +//! no longer waits: [`RequestNotices`]), and gives pointer events to //! [`pointer`] before anything else while a sheet or the Settings page is //! up. The relay calls [`approval_requested`] and installs itself with //! [`set_relay`]; an app module registers its own confirmation sheet with @@ -296,6 +297,49 @@ pub fn dismiss_expired(id: &RequestId) { pub fn take_notices() -> Vec { with(|a| a.router.take_notices()).unwrap_or_default() } + +/// Whether `id` still waits for its answer (on its sheet, or on the +/// owning app's). +pub fn is_pending(id: &RequestId) -> bool { + with(|a| a.router.is_pending(id)).unwrap_or(false) +} + +/// Where the shell showed one notice: its desktop toast and its phone shade +/// notification. +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +pub struct Shown { + pub toast: Option, + pub shade: Option, +} + +/// The notifications of the notices that ask the person to answer a request +/// ([`Notice::request`]: "Needs you: … Open the sheet to approve or +/// deny."), by that request. The shell withdraws each once its request is +/// no longer pending, together with its sheet line: after Stop withdrew +/// the sheet, its toast still said to open it. +#[derive(Debug, Default)] +pub struct RequestNotices(Vec<(RequestId, Shown)>); + +impl RequestNotices { + pub fn record(&mut self, request: RequestId, shown: Shown) { + self.0.push((request, shown)); + } + + /// The notifications of the requests `pending` no longer holds, to + /// withdraw now; forgotten here. + pub fn withdrawn(&mut self, pending: impl Fn(&RequestId) -> bool) -> Vec<(RequestId, Shown)> { + let mut gone = Vec::new(); + self.0.retain(|(id, shown)| { + let keep = pending(id); + if !keep { + gone.push((id.clone(), *shown)); + } + keep + }); + gone + } +} + pub fn generation() -> u64 { with(|a| a.generation()).unwrap_or(0) } diff --git a/crates/shell/src/approvals/router.rs b/crates/shell/src/approvals/router.rs index bb92b1d5..a5bbf518 100644 --- a/crates/shell/src/approvals/router.rs +++ b/crates/shell/src/approvals/router.rs @@ -121,6 +121,11 @@ const EXPIRED_KEPT: usize = 16; pub struct Notice { pub title: String, pub body: String, + /// The request this notice asks the person to answer ("Needs you: … + /// Open the sheet"): its notification goes once the request is no + /// longer pending, with its sheet line (answered, stopped, withdrawn, + /// expired). `None`: the notice stands on its own. + pub request: Option, } pub struct Router { @@ -276,7 +281,7 @@ impl Router { std::mem::take(&mut self.notices) } fn notice(&mut self, title: impl Into, body: impl Into) { - self.notices.push(Notice { title: title.into(), body: body.into() }); + self.notices.push(Notice { title: title.into(), body: body.into(), request: None }); } /// Route one request. The decision may be given before this returns. @@ -361,7 +366,12 @@ impl Router { fn surface(&mut self, req: &Request, surfaced: Surfaced, now: u64) -> u64 { let line = Line::for_request(req, surfaced, &self.contacts); let batch = req.context.batch.clone(); - self.notice(format!("Needs you: {}", line.heading()), format!("{} asks. Open the sheet to approve or deny.", line.caller)); + // Withdrawn with the sheet line: it points at the sheet. + self.notices.push(Notice { + title: format!("Needs you: {}", line.heading()), + body: format!("{} asks. Open the sheet to approve or deny.", line.caller), + request: Some(req.id.clone()), + }); self.changed(); if let Some(b) = &batch { if let Some(sheet) = self.sheets.iter_mut().find(|s| s.batch_id() == Some(&b.id) && !s.done()) { diff --git a/crates/shell/src/approvals/tests.rs b/crates/shell/src/approvals/tests.rs index 56aa4b66..6b41ace1 100644 --- a/crates/shell/src/approvals/tests.rs +++ b/crates/shell/src/approvals/tests.rs @@ -719,6 +719,52 @@ fn stop_denies_what_the_stopped_agent_asks() { assert_eq!(r.sheets().len(), 1); } +/// The sheet's "Needs you … Open the sheet" notice names its request, and +/// the shell withdraws that notification once the request no longer waits, +/// with its sheet line: after the person's Stop (an instrument run found +/// the toast still up, pointing at a sheet that was gone), an answer on +/// the sheet, the turn's own end, or the deadline. Other notices stand on +/// their own. +#[test] +fn a_sheets_notification_is_withdrawn_with_its_line() { + let (mut r, _) = router(); + r.sheet_expiry_s = 600; + let own = |id: &str| { + let mut q = send(id, json!({"to": "eve@example.org"})); + q.caller = Caller::OwnAgent { client: None }; + q + }; + let Route::Sheet(_) = r.request(send("stopped", json!({"to": "eve@example.org"})), T0) else { panic!() }; + let Route::Sheet(answered_on) = r.request(own("answered"), T0) else { panic!() }; + let Route::Sheet(_) = r.request(own("withdrawn"), T0) else { panic!() }; + let mut late = own("expired"); + late.received = T0 + 60; + let Route::Sheet(_) = r.request(late, T0 + 60) else { panic!() }; + let mut shown = super::RequestNotices::default(); + let notices = r.take_notices(); + assert_eq!(notices.len(), 4, "{notices:?}"); + for (n, notice) in notices.iter().enumerate() { + assert!(notice.title.starts_with("Needs you: ") && notice.body.ends_with("Open the sheet to approve or deny."), "{notice:?}"); + let request = notice.request.clone().expect("a sheet's notice names its request"); + shown.record(request, super::Shown { toast: Some(n as u64 + 1), shade: Some(n as u64 + 11) }); + } + let mut withdrawn = |r: &Router| -> Vec { shown.withdrawn(|id| r.is_pending(id)).into_iter().map(|(id, _)| id.0).collect() }; + assert!(withdrawn(&r).is_empty(), "all four still wait on their sheets"); + r.stop_agent("calendar", T0 + 1); + assert_eq!(withdrawn(&r), ["stopped"], "Stop withdrew the line: its toast goes too"); + r.answer(answered_on, &RequestId("answered".into()), Answer::Deny, &ApprovalGesture::sheet_tap(), T0 + 2).unwrap(); + assert_eq!(withdrawn(&r), ["answered"]); + assert!(r.withdraw(&RequestId("withdrawn".into()), "the turn ended", T0 + 3)); + assert_eq!(withdrawn(&r), ["withdrawn"]); + r.tick(T0 + 600); + assert!(withdrawn(&r).is_empty(), "the last one has a minute left"); + r.tick(T0 + 660); + assert_eq!(withdrawn(&r), ["expired"]); + let expired = r.take_notices(); + assert!(expired.iter().any(|n| n.title.starts_with("Expired: ") && n.request.is_none()), "the expiry notice stands on its own: {expired:?}"); + assert!(withdrawn(&r).is_empty(), "each is withdrawn once"); +} + // ---------------------------------------------------------------- confirm: app #[derive(Clone, Default)] diff --git a/crates/shell/src/lib.rs b/crates/shell/src/lib.rs index de004007..70d02d62 100644 --- a/crates/shell/src/lib.rs +++ b/crates/shell/src/lib.rs @@ -79,6 +79,8 @@ pub mod glance_notice; pub mod glance_panel; pub mod glance_sheet; use glance::NoteTargets as GlanceNoteTargets; +// The App derive takes a plain type name for a field. +use approvals::RequestNotices as ApprovalNotices; pub mod system_chat; pub use octosense_ai_host as ai_host; @@ -437,6 +439,10 @@ pub struct App { /// The approvals' generation last drawn (approvals/). #[rust] pub approvals_generation: u64, + /// The notifications of "Needs you" notices, withdrawn with their + /// requests' sheet lines (approvals/). + #[rust] + pub approval_notices: ApprovalNotices, /// The system chat's generation the surfaces last drew (system_chat/). #[rust] pub system_chat_generation: u64, @@ -4134,7 +4140,16 @@ impl App { self.on_bus_route(cx, route); } for n in approvals::take_notices() { - self.notify(cx, &n.title, &n.body); + let shown = self.notify_shown(cx, &n.title, &n.body); + if let Some(request) = n.request { + self.approval_notices.record(request, shown); + } + } + // "Needs you … Open the sheet" goes with its sheet line (answered, + // stopped, withdrawn, expired): it pointed at a sheet that was gone. + for (request, shown) in self.approval_notices.withdrawn(approvals::is_pending) { + log!("approvals: {request} no longer waits; its notification is withdrawn"); + self.withdraw_notification(cx, shown); } let generation = approvals::generation(); if generation != self.approvals_generation { @@ -4146,15 +4161,32 @@ impl App { /// The in-process notification API — `WmRequest::Notify{title, body}` /// lands here. pub fn notify(&mut self, cx: &mut Cx, title: &str, body: &str) { + self.notify_shown(cx, title, body); + } + + /// [`Self::notify`], saying where it was shown (the desktop toast, the + /// phone shade's notification), so it can be withdrawn. + fn notify_shown(&mut self, cx: &mut Cx, title: &str, body: &str) -> approvals::Shown { let notes = self.ui.widget(cx, ids!(shell_notes)); - { + let toast = notes.borrow_mut::().map(|mut n| n.notify(cx, title, body)); + let now = cx.seconds_since_app_start(); + let shade = self.state.as_mut().map(|state| state.phone.shade.post("wm", title, body, now, Vec::new())); + self.redraw_all(cx); + approvals::Shown { toast, shade } + } + + /// Take back what [`Self::notify_shown`] showed (gone already: nothing). + fn withdraw_notification(&mut self, cx: &mut Cx, shown: approvals::Shown) { + if let Some(id) = shown.toast { + let notes = self.ui.widget(cx, ids!(shell_notes)); let mut borrowed = notes.borrow_mut::(); if let Some(n) = borrowed.as_mut() { - n.notify(cx, title, body); + n.dismiss(cx, id); } } - let now = cx.seconds_since_app_start(); - if let Some(state) = self.state.as_mut() { state.phone.shade.post("wm", title, body, now, Vec::new()); } + if let (Some(id), Some(state)) = (shown.shade, self.state.as_mut()) { + state.phone.shade.dismiss(id); + } self.redraw_all(cx); }