diff --git a/README.md b/README.md index eabdebc9..3645fb87 100644 --- a/README.md +++ b/README.md @@ -81,7 +81,8 @@ not an exhaustive list; the in-app **What's New** is the complete, current recor then you can create events on its calendars (+ New event, or press c; Shift+C opens a quick box, and in the day, week, month and agenda views the new event shows as you type, except in the Terminal look), repeating ones too (Google's choices, or your own with Custom…), and change and - delete events you organise without guests, repeating ones included. Every change goes straight to + delete events you organise, repeating ones and ones with guests included (PM asks whether Google + should email the guests, as Google Calendar does). Every change goes straight to Google, a save never overwrites a field someone changed there in the meantime, and a new event whose save loses the connection is looked for in Google before it's ever sent again. - **Bring your own model.** Choose your model through OpenRouter — separate chat and diff --git a/package-lock.json b/package-lock.json index 91aef41e..6dfb8f00 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "pm", - "version": "3.154.2-alpha", + "version": "3.155.0-alpha", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "pm", - "version": "3.154.2-alpha", + "version": "3.155.0-alpha", "license": "AGPL-3.0-or-later", "dependencies": { "@fontsource/atkinson-hyperlegible": "^5.3.0", diff --git a/package.json b/package.json index 1a2283f2..f1f82182 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "pm", "private": true, - "version": "3.154.2-alpha", + "version": "3.155.0-alpha", "license": "AGPL-3.0-or-later", "type": "module", "engines": { diff --git a/src-tauri/Cargo.lock b/src-tauri/Cargo.lock index b7924acf..8602019a 100644 --- a/src-tauri/Cargo.lock +++ b/src-tauri/Cargo.lock @@ -3072,7 +3072,7 @@ dependencies = [ [[package]] name = "pm" -version = "3.154.2-alpha" +version = "3.155.0-alpha" dependencies = [ "argon2", "base64 0.23.1", diff --git a/src-tauri/Cargo.toml b/src-tauri/Cargo.toml index 275ead24..bae3a55e 100644 --- a/src-tauri/Cargo.toml +++ b/src-tauri/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "pm" -version = "3.154.2-alpha" +version = "3.155.0-alpha" description = "PM — a local-first personal manager (Archivist + Personal Assistant)" authors = ["Bobby Yu"] edition = "2021" diff --git a/src-tauri/src/calendar_write/attendees.rs b/src-tauri/src/calendar_write/attendees.rs index 8a394725..29cb1195 100644 --- a/src-tauri/src/calendar_write/attendees.rs +++ b/src-tauri/src/calendar_write/attendees.rs @@ -328,6 +328,9 @@ pub struct Reach { reached: BTreeSet, /// Who it takes off. removed: BTreeSet, + /// Who only a delete or a cancel reaches: no other write merged reaches them, so all they hear + /// is that an occurrence is cancelled, never an update. + only_cancelled: BTreeSet, /// Emails it would send: one per guest per event written. emails: u32, /// PM saw every guest list it counted. @@ -349,6 +352,7 @@ impl Reach { was: BTreeSet::new(), reached: BTreeSet::new(), removed: BTreeSet::new(), + only_cancelled: BTreeSet::new(), emails: 0, complete: true, cancelled: false, @@ -388,6 +392,7 @@ impl Reach { was: before, reached, removed, + only_cancelled: BTreeSet::new(), // A personal-only change emails nobody, however much of the list Google sent. complete: !cut_short || !shared, cancelled: false, @@ -401,6 +406,7 @@ impl Reach { Reach { emails: count(everyone.len()), reached: everyone.clone(), + only_cancelled: everyone.clone(), was: everyone, removed: BTreeSet::new(), complete: !omitted(event), @@ -417,6 +423,7 @@ impl Reach { reached: everyone, was: BTreeSet::new(), removed: BTreeSet::new(), + only_cancelled: BTreeSet::new(), complete: true, cancelled: false, shared: true, @@ -425,6 +432,15 @@ impl Reach { /// The two together, as one plan sends them. pub fn merge(mut self, other: Reach) -> Reach { + // Anyone a write other than a cancel reaches, on either side, hears more than a cancel. + let told_more: BTreeSet = self + .reached + .difference(&self.only_cancelled) + .chain(other.reached.difference(&other.only_cancelled)) + .cloned() + .collect(); + self.only_cancelled.extend(other.only_cancelled); + self.only_cancelled.retain(|a| !told_more.contains(a)); self.was.extend(other.was); self.reached.extend(other.reached); self.removed.extend(other.removed); @@ -467,16 +483,27 @@ impl Reach { return None; } let already = |a: &&String| self.was.contains(*a); + // When everything it reaches is cancelled, those are among `updated`, told it's cancelled. + let only_cancelled = |a: &&String| !self.cancelled && self.only_cancelled.contains(*a); + let kept = |a: &&String| !self.removed.contains(*a); Some(GuestNotice { updated: count( self.reached .iter() .filter(already) - .filter(|a| !self.removed.contains(*a)) + .filter(kept) + .filter(|a| !only_cancelled(a)) .count(), ), invited: count(self.reached.iter().filter(|a| !already(a)).count()), removed: count(self.removed.len()), + told_cancelled: count( + self.reached + .iter() + .filter(kept) + .filter(only_cancelled) + .count(), + ), cancelled: self.cancelled, people: count(self.reached.len()), emails: self.emails, @@ -523,6 +550,7 @@ pub fn row_delete_notice(guests: &[Attendee], own: &Own) -> Option Reach { emails: count(everyone.len()), reached: everyone.clone(), + only_cancelled: everyone.clone(), was: everyone, removed: BTreeSet::new(), complete: true, @@ -782,6 +810,7 @@ mod tests { updated, invited, removed, + told_cancelled: 0, cancelled: false, people, emails: people, @@ -963,6 +992,7 @@ mod tests { updated: 2, invited: 1, removed: 0, + told_cancelled: 0, cancelled: false, people: 3, emails: 3 + 2 + 2, @@ -975,6 +1005,99 @@ mod tests { assert!(blind.notice().is_some_and(|n| n.invited == 4)); } + /// C12c review round 3: a guest only a cancel reaches, in a plan that also changes something + /// else, is told that occurrence is cancelled, never of an update; anyone another write reaches + /// hears more than the cancel, whichever side of the merge, in whatever order. + #[test] + fn a_guest_only_a_cancel_reaches_is_told_of_the_cancel() { + let series = event(&[me()]); + let own = Own::of(&["cal@x.org"], &[&series]); + let until = body(json!({ "recurrence": ["RRULE:FREQ=WEEKLY;UNTIL=20261018"] })); + let cut = || Reach::of_patch(&series, &until, &own); + let occurrence = event(&[me(), a()]); + let cancel = || Reach::of_delete(&occurrence, &own); + let told = GuestNotice { + told_cancelled: 1, + people: 1, + emails: 1, + complete: true, + ..Default::default() + }; + assert!( + cut().merge(cancel()).notice() == Some(told.clone()), + "cut, then cancel" + ); + assert!( + cancel().merge(cut()).notice() == Some(told), + "cancel, then cut" + ); + // A write other than the cancel reaches A too: A hears of an update, on either side. + let shared = event(&[me(), a()]); + let retitle = || Reach::of_patch(&shared, &title(), &own); + let update = GuestNotice { + updated: 1, + people: 1, + emails: 2, + complete: true, + ..Default::default() + }; + for all in [ + cancel().merge(retitle()), + retitle().merge(cancel()), + cut().merge(cancel()).merge(retitle()), + retitle().merge(cut().merge(cancel())), + ] { + assert!(all.notice() == Some(update.clone())); + } + // Two cancels of A, and a guest only the other write reaches: one each. + let b_only = event(&[me(), b_guest()]); + let both = cancel() + .merge(Reach::of_patch(&b_only, &title(), &own)) + .merge(cancel()); + assert!( + both.notice() + == Some(GuestNotice { + updated: 1, + told_cancelled: 1, + people: 2, + emails: 3, + complete: true, + ..Default::default() + }) + ); + // Taken off as well (left off a new part): counted once, as taken off. + let gone = cancel() + .merge(Reach::of_insert( + &body(json!({ "attendees": [me()] })), + &own, + )) + .taking_off(BTreeSet::from([key("A@x.org")])); + assert!( + gone.notice() + == Some(GuestNotice { + removed: 1, + people: 1, + emails: 1, + complete: true, + ..Default::default() + }) + ); + // A plain delete, or a delete's plan, keeps its own words: everyone is told it's cancelled. + let deleted = GuestNotice { + updated: 1, + cancelled: true, + people: 1, + emails: 1, + complete: true, + ..Default::default() + }; + assert!(cancel().notice() == Some(deleted.clone()), "a plain delete"); + assert!( + cut().merge(cancel()).cancelling(true).notice() == Some(deleted), + "a delete's plan" + ); + } + /// A user's entry Google hasn't marked `self` (the new part's, as PM sent it) is still the user, /// in every set PM compares by who is on it. #[test] diff --git a/src-tauri/src/calendar_write/dto.rs b/src-tauri/src/calendar_write/dto.rs index dfdbff27..5b2ab28b 100644 --- a/src-tauri/src/calendar_write/dto.rs +++ b/src-tauri/src/calendar_write/dto.rs @@ -155,6 +155,9 @@ pub struct GuestNotice { pub invited: u32, /// Guests the change takes off: each is told. pub removed: u32, + /// Guests on an occurrence the change cancels and on nothing else it writes: each is told that + /// occurrence is cancelled, not of an update. Never among `updated`; none when `cancelled`. + pub told_cancelled: u32, /// The change deletes it (or every occurrence it reaches): `updated` are told it's cancelled. pub cancelled: bool, /// Different guests who'd hear, every write together. @@ -266,8 +269,6 @@ pub enum ReadOnlyReason { NotOrganizer, /// A one-off in PM that has since become a repeating event in Google: open one of its occurrences. BecameRepeating, - /// It has guests, who Google would email (until Send / Don't send lands). - HasGuests, /// A birthday, a Gmail event, focus time, out of office or a working location. SpecialType, /// Google has locked its title, time, place and description. @@ -382,13 +383,16 @@ pub enum WriteOutcome { /// it for settling; `offers` is what Google's state allows now; `from` is the day the change /// starts (`YYYY-MM-DD`, in the series' zone). Never an id or an etag (R5). `note`: a settle /// that didn't end the change says why here, while PM still holds it to settle (so its buttons - /// stay); left out for every other answer. + /// stay); left out for every other answer. `emails`: the offers that would email guests, since + /// the change went with Send (C12); left out when none would. PartialSplit { token: String, offers: Vec, from: String, #[serde(skip_serializing_if = "Option::is_none")] note: Option, + #[serde(skip_serializing_if = "Vec::is_empty")] + emails: Vec, }, /// The new part of a change PM couldn't finish was removed: the repeating event is as it was. SplitRemoved, @@ -664,11 +668,13 @@ pub enum ScopeRow { /// Changed in Google since the editor opened, on these fields. Conflict { fields: Vec }, /// A change PM couldn't finish holds this series (C9 design review M1): settle it first, as - /// [`WriteOutcome::PartialSplit`] says. + /// [`WriteOutcome::PartialSplit`] says, `emails` included. Pending { token: String, offers: Vec, from: String, + #[serde(skip_serializing_if = "Vec::is_empty")] + emails: Vec, }, } @@ -766,7 +772,7 @@ mod tests { complete: true, ..Default::default() }), - r#"{"outcome":"needs_choice","scope":false,"guests":{"updated":2,"invited":0,"removed":0,"cancelled":false,"people":2,"emails":2,"complete":true}}"#, + r#"{"outcome":"needs_choice","scope":false,"guests":{"updated":2,"invited":0,"removed":0,"told_cancelled":0,"cancelled":false,"people":2,"emails":2,"complete":true}}"#, ), ( WriteOutcome::needs_notify_again(GuestNotice { @@ -775,7 +781,7 @@ mod tests { emails: 1, ..Default::default() }), - r#"{"outcome":"needs_choice","scope":false,"guests":{"updated":0,"invited":1,"removed":0,"cancelled":false,"people":1,"emails":1,"complete":false},"again":true}"#, + r#"{"outcome":"needs_choice","scope":false,"guests":{"updated":0,"invited":1,"removed":0,"told_cancelled":0,"cancelled":false,"people":1,"emails":1,"complete":false},"again":true}"#, ), ( WriteOutcome::IgnoredByGoogle { split: false }, @@ -791,6 +797,7 @@ mod tests { offers: vec![SplitAction::Remove, SplitAction::Check], from: "2026-11-02".into(), note: None, + emails: Vec::new(), }, r#"{"outcome":"partial_split","token":"t","offers":["remove","check"],"from":"2026-11-02"}"#, ), @@ -800,9 +807,21 @@ mod tests { offers: vec![SplitAction::Check], from: "2026-11-02".into(), note: Some("Try again in a moment.".into()), + emails: Vec::new(), }, r#"{"outcome":"partial_split","token":"t","offers":["check"],"from":"2026-11-02","note":"Try again in a moment."}"#, ), + // C12: sent with Send, Remove would email guests. + ( + WriteOutcome::PartialSplit { + token: "t".into(), + offers: vec![SplitAction::Remove, SplitAction::Check], + from: "2026-11-02".into(), + note: None, + emails: vec![SplitAction::Remove], + }, + r#"{"outcome":"partial_split","token":"t","offers":["remove","check"],"from":"2026-11-02","emails":["remove"]}"#, + ), (WriteOutcome::SplitRemoved, r#"{"outcome":"split_removed"}"#), (WriteOutcome::NoChange, r#"{"outcome":"no_change"}"#), (WriteOutcome::Gone, r#"{"outcome":"gone"}"#), @@ -993,10 +1012,21 @@ mod tests { token: "t".into(), offers: vec![SplitAction::Finish], from: "2026-11-02".into(), + emails: Vec::new(), }) .unwrap(), r#"{"row":"pending","token":"t","offers":["finish"],"from":"2026-11-02"}"# ); + assert_eq!( + serde_json::to_string(&ScopeRow::Pending { + token: "t".into(), + offers: vec![SplitAction::Finish], + from: "2026-11-02".into(), + emails: vec![SplitAction::Finish], + }) + .unwrap(), + r#"{"row":"pending","token":"t","offers":["finish"],"from":"2026-11-02","emails":["finish"]}"# + ); // C12: a row that would email someone says who, in numbers. assert_eq!( serde_json::to_string(&ScopeRow::Offered { @@ -1011,7 +1041,7 @@ mod tests { }), }) .unwrap(), - r#"{"row":"offered","warnings":[],"token":"t","guests":{"updated":0,"invited":1,"removed":0,"cancelled":false,"people":1,"emails":1,"complete":true}}"# + r#"{"row":"offered","warnings":[],"token":"t","guests":{"updated":0,"invited":1,"removed":0,"told_cancelled":0,"cancelled":false,"people":1,"emails":1,"complete":true}}"# ); for (text, action) in [ (r#""remove""#, SplitAction::Remove), @@ -1226,7 +1256,7 @@ mod tests { }; assert!(asked.answered_by(&asked)); let read: GuestNotice = serde_json::from_str( - r#"{"updated":1,"invited":0,"removed":0,"cancelled":false,"people":1,"emails":1, + r#"{"updated":1,"invited":0,"removed":0,"told_cancelled":0,"cancelled":false,"people":1,"emails":1, "complete":true}"#, ) .unwrap(); diff --git a/src-tauri/src/calendar_write/gate.rs b/src-tauri/src/calendar_write/gate.rs index b85935ae..49ed33e7 100644 --- a/src-tauri/src/calendar_write/gate.rs +++ b/src-tauri/src/calendar_write/gate.rs @@ -6,13 +6,14 @@ //! save runs it again on a fresh copy from Google before sending anything. It fails closed: a role or //! an event type PM doesn't know reads as read-only, never as editable. //! -//! Events PM can change are the ones you organise on a calendar you can edit, with no guests, -//! repeating ones included: a change to an occurrence of one names which occurrences it's for -//! ("this event", "this and following", "all events"), and a write without that choice is asked -//! again (rule R9). Events with guests and invitations are read-only with a reason until the layers -//! that handle them (C12, C14) lift it, each replacing it with the rule it stood in for. A one-off -//! that Google has since turned into a repeating event is read-only with its own reason on every -//! path, since a write to it would change every occurrence with no choice asked. +//! Events PM can change are the ones you organise on a calendar you can edit. The first layers +//! edited only solo, one-off events; repeating events (C9) and events with guests (C12) are now +//! editable, each with the choice it stood in for (rule R9): a change to an occurrence names which +//! occurrences it's for ("this event", "this and following", "all events"), a change Google would +//! email guests about names whether it should, and a write without that choice is asked again. +//! Invitations stay read-only until C14. A one-off that Google has since turned into a repeating +//! event is read-only with its own reason on every path, since a write to it would change every +//! occurrence with no choice asked. use crate::calendar_editing::EditingStatus; @@ -37,8 +38,6 @@ pub struct EditFacts<'a> { /// would change every occurrence with no choice asked (C9 design §0.7, critique 2 M6). Known only /// from a fresh copy; the mirror never holds a series itself. pub series_master: bool, - /// Guests besides the calendar itself. - pub has_guests: bool, /// The description is formatted. Known only from a fresh copy (the mirror clips descriptions). pub html_description: bool, } @@ -107,7 +106,6 @@ pub fn create_block( locked: false, visibility, series_master: false, - has_guests: false, html_description: false, }) } @@ -143,9 +141,6 @@ fn blanket_reason(facts: &EditFacts) -> Option { if facts.series_master { return Some(ReadOnlyReason::BecameRepeating); } - if facts.has_guests { - return Some(ReadOnlyReason::HasGuests); - } None } @@ -165,7 +160,6 @@ mod tests { locked: false, visibility: None, series_master: false, - has_guests: false, html_description: false, } } @@ -175,8 +169,7 @@ mod tests { let perms = edit_rights(&editable()); assert!(perms.summary && perms.time && perms.location && perms.description); assert!(perms.show_as && perms.visibility && perms.delete); - // C12a: guests may change wherever the rest may (the gate still refuses an event that has - // any until C12c). + // Guests may change wherever the rest may (C12). assert!(perms.guests); assert!(perms.reasons.is_empty()); assert_eq!(edit_block(&editable()), None); @@ -322,22 +315,6 @@ mod tests { }, BecameRepeating, ), - ( - EditFacts { - has_guests: true, - ..e() - }, - HasGuests, - ), - // The first reason wins: a read-only shared calendar outranks "it has guests". - ( - EditFacts { - access_role: Some("reader"), - has_guests: true, - ..e() - }, - CalendarReadOnly, - ), // A read-only shared calendar outranks "it became a repeating event". ( EditFacts { diff --git a/src-tauri/src/calendar_write/series/mod.rs b/src-tauri/src/calendar_write/series/mod.rs index fc34e4d3..1fddc5fc 100644 --- a/src-tauri/src/calendar_write/series/mod.rs +++ b/src-tauri/src/calendar_write/series/mod.rs @@ -161,8 +161,6 @@ pub struct Master { pub organizer_self: bool, pub event_type: Option, pub locked: bool, - /// Anyone besides the user on its guest list. - pub guests: bool, } impl Master { @@ -228,14 +226,13 @@ impl Master { .unwrap_or(false), event_type: text("eventType"), locked: it.get("locked").and_then(Value::as_bool).unwrap_or(false), - guests: !attendee_set(it.get("attendees")).is_empty(), }) } /// Why nothing about the series may change, from its own facts. The calendar's role and the /// account's editing switch are the command layer's (gate.rs); a master can also be an invitation /// or a special kind of event, which the gate reads from the occurrence. Its guests are the - /// planners' to weigh ([`plan::SeriesInput::guests_allowed`]). + /// planners' to count, for the Send / Don't send choice (C12). pub fn read_only(&self) -> Option { if !matches!(self.event_type.as_deref(), None | Some("default")) { return Some(ReadOnlyReason::SpecialType); @@ -591,9 +588,6 @@ pub enum SeriesRefusal { /// An occurrence after the split has guests, attachments, a call, a label or a visibility of its /// own. CannotCarry(NaiveDate), - /// An occurrence the change would reach has guests of its own (until C12 brings Send / Don't - /// send). - GuestsElsewhere(NaiveDate), /// The series' description is formatted (HTML), and the draft would save it as plain text. FormattedDescription, /// Its visibility applies to every occurrence (Google), so it can't change for one. @@ -687,12 +681,6 @@ impl fmt::Display for SeriesRefusal { see it, which PM can't move to the new part.", day.format("%d-%m-%Y") ), - SeriesRefusal::GuestsElsewhere(day) => write!( - f, - "The occurrence on {} has guests of its own, and PM can't change events with \ - guests yet. Choose “This event” to change only this one.", - day.format("%d-%m-%Y") - ), SeriesRefusal::FormattedDescription => write!( f, "This repeating event's description has formatting, which saving it here as plain \ @@ -1210,7 +1198,6 @@ pub(crate) mod fixtures { pub instances: Option, pub sibling: bool, pub later: LaterPart, - pub guests_allowed: bool, } impl Case { @@ -1224,7 +1211,6 @@ pub(crate) mod fixtures { instances: None, sibling: false, later: LaterPart::None, - guests_allowed: false, } } @@ -1233,12 +1219,6 @@ pub(crate) mod fixtures { self } - /// As C12 will run it, a series with guests allowed. - pub fn guests(mut self) -> Case { - self.guests_allowed = true; - self - } - pub fn input(&self) -> SeriesInput<'_> { SeriesInput { calendar: CALENDAR, @@ -1254,7 +1234,6 @@ pub(crate) mod fixtures { instances: self.instances.as_ref(), sibling: self.sibling, later: self.later, - guests_allowed: self.guests_allowed, me: CALENDAR, } } diff --git a/src-tauri/src/calendar_write/series/plan.rs b/src-tauri/src/calendar_write/series/plan.rs index e33dd612..29e056e3 100644 --- a/src-tauri/src/calendar_write/series/plan.rs +++ b/src-tauri/src/calendar_write/series/plan.rs @@ -85,8 +85,8 @@ use super::diff::{Expect, Removes, SeriesDiff}; use super::rule::{plan_all_rule, plan_split_rule, rule_ask, RuleAsk}; use super::scope::{is_first, zones_changed, Scope}; use super::{ - attendee_set, classify_instance, excluded, google_before, guests_on, original_of, point_of, - Exceptions, FreshEvent, InstanceKind, LaterPart, Length, Listing, Master, SeriesRefusal, + attendee_set, classify_instance, excluded, google_before, original_of, point_of, Exceptions, + FreshEvent, InstanceKind, LaterPart, Length, Listing, Master, SeriesRefusal, }; /// Everything a series plan is built from. C9 makes the GETs; nothing here fetches. @@ -107,10 +107,6 @@ pub struct SeriesInput<'a> { /// Where the earliest later part of the series starts, which a new rule mustn't repeat over (C11 /// critique 1 H2). pub later: LaterPart, - /// Whether a series with guests may change: false until C12 brings invitations, whatever the - /// occurrence shows. One whose guests were removed on its own passes the gate, and a split would - /// invite the series' guests again (C8b review G5). - pub guests_allowed: bool, /// The address the account reads this calendar with: the user, never a guest Google emails, /// whatever a list PM built calls it (C12 critiques C1-H1, C2-H1). pub me: &'a str, @@ -1065,9 +1061,6 @@ fn prepare(input: &SeriesInput) -> Result<(Master, Point), NotPlanned> { if let Some(reason) = master.read_only() { return Err(NotPlanned::ReadOnly(reason)); } - if master.guests && !input.guests_allowed { - return Err(NotPlanned::ReadOnly(ReadOnlyReason::HasGuests)); - } if instance.get("recurringEventId").and_then(Value::as_str) != Some(master.id.as_str()) { return Err(SeriesRefusal::NotInSeries.into()); } @@ -1209,36 +1202,6 @@ pub(super) fn exceptions_from<'a>( Ok(out) } -/// Until C12 brings Send / Don't send, a plan that reaches past the occurrence picked mustn't reach -/// one with guests of its own (the series' own guests are refused in [`prepare`]): Google would -/// change or cancel it with nobody told, and no choice of who is. `from` is a split's or a cut's -/// occurrence (only those from it on are reached); `None` for the whole series. Only a complete -/// listing can show there's none, so an incomplete one refuses too, a change of text included; the -/// cross-check counts an occurrence with guests the listing lacks against it (C9a-2 review). -pub(super) fn no_guests_from( - master: &Master, - input: &SeriesInput, - from: Option, -) -> Result<(), NotPlanned> { - if input.guests_allowed { - return Ok(()); - } - if !input.exceptions.complete { - return Err(SeriesRefusal::Incomplete.into()); - } - let zone = master.start.zone(); - for item in own_exceptions(master, input.exceptions) { - if item.get("status").and_then(Value::as_str) == Some("cancelled") || !guests_on(item) { - continue; - } - let at = original_of(item).ok_or(SeriesRefusal::Unreadable)?; - if from.is_none_or(|from| !at.cmp_in(&from, zone).is_lt()) { - return Err(SeriesRefusal::GuestsElsewhere(at.day_in(zone)).into()); - } - } - Ok(()) -} - /// The gate reads the occurrence's description; a draft for the series writes the series' own, and /// would save a formatted one as plain text (C9 design review L7). pub(super) fn keeps_formatting( @@ -1395,10 +1358,9 @@ fn delete_plan( if input.sibling && scope != Scope::This { warnings.push(SeriesWarning::SplitSibling); } - // Every arm that deletes the whole series comes through here, guests checked. - let whole = |warnings: Vec| -> Result { - no_guests_from(&master, input, None)?; - Ok(SeriesPlan { + // Every arm that deletes the whole series comes through here. + let whole = |warnings: Vec| -> SeriesPlan { + SeriesPlan { scope: Scope::All, steps: vec![Step { op: Op::Delete { @@ -1432,7 +1394,7 @@ fn delete_plan( guests: None, chosen: None, own: Own::default(), - }) + } }; match scope { Scope::This => Ok(SeriesPlan { @@ -1471,33 +1433,30 @@ fn delete_plan( chosen: None, own: Own::default(), }), - Scope::All => whole(warnings), - Scope::Following if is_first(&master, input.fresh.instance) => whole(warnings), + Scope::All => Ok(whole(warnings)), + Scope::Following if is_first(&master, input.fresh.instance) => Ok(whole(warnings)), Scope::Following => match cut_at(&master, input, original)? { - None => whole(warnings), - Some(SplitCut { cut, .. }) => { - no_guests_from(&master, input, Some(original))?; - Ok(SeriesPlan { - scope, - steps: plan_cut_delete(&master, input, original, cut, notify)?, - warnings, - expect: vec![Expect::EndsBefore { - series: master.id.clone(), - at: original, - left: Vec::new(), - }], - removes: Removes::From { - series: master.id.clone(), - at: original, - left: Vec::new(), - }, - start: master.start, - rule_start: master.start, - guests: None, - chosen: None, - own: Own::default(), - }) - } + None => Ok(whole(warnings)), + Some(SplitCut { cut, .. }) => Ok(SeriesPlan { + scope, + steps: plan_cut_delete(&master, input, original, cut, notify)?, + warnings, + expect: vec![Expect::EndsBefore { + series: master.id.clone(), + at: original, + left: Vec::new(), + }], + removes: Removes::From { + series: master.id.clone(), + at: original, + left: Vec::new(), + }, + start: master.start, + rule_start: master.start, + guests: None, + chosen: None, + own: Own::default(), + }), }, } } @@ -2044,7 +2003,6 @@ fn plan_all( } let (instance, master_json) = (input.fresh.instance.json(), input.fresh.master.json()); input.opened.master.ok_or(SeriesRefusal::NoBase)?; - no_guests_from(master, input, None)?; keeps_formatting(master_json, draft)?; let text_only = EventPatchDraft { time: None, @@ -2295,7 +2253,6 @@ fn plan_split( let zone = master.start.zone(); let rest_start = rest_start(master, at)?; let exceptions = exceptions_from(master, input.exceptions, at)?; - no_guests_from(master, input, Some(at))?; let cancelled: Vec = exceptions .iter() .filter(|(item, _)| classify_instance(master_json, master, item) == InstanceKind::Cancelled) @@ -3202,8 +3159,7 @@ mod tests { } }; let invited = Case::new(guest(case.master.json()), guest(&monday_19())) - .exceptions(case.exceptions.items.iter().map(guest).collect()) - .guests(); + .exceptions(case.exceptions.items.iter().map(guest).collect()); let plan = plan_series_edit( Scope::Following, &invited.input(), @@ -3481,39 +3437,44 @@ mod tests { ) } - /// G5: a series with guests stays read-only until C12, whatever the occurrence shows. Once guests - /// are allowed, an occurrence whose guests were removed on its own is a change PM can't carry. + /// G5, C12: a series with guests plans like any other and says who it would email, whatever + /// the occurrence shows (C12 lifted the gate's refusal). An occurrence whose guests were removed + /// on its own is still a change PM can't carry. #[test] - fn a_series_with_guests_is_judged_by_its_own_guest_list() { + fn a_series_with_guests_plans_and_counts_them() { let no_guests_here = monday_19(); let case = Case::new(with_guests(), no_guests_here); - for scope in [Scope::All, Scope::Following] { - assert_eq!( - case.edit(scope, title("Retro")), - Err(NotPlanned::ReadOnly(ReadOnlyReason::HasGuests)) - ); - } - assert_eq!( - case.delete(Scope::Following), - Err(NotPlanned::ReadOnly(ReadOnlyReason::HasGuests)) - ); + let two = |cancelled: bool| GuestNotice { + updated: 2, + cancelled, + people: 2, + emails: 2, + complete: true, + ..GuestNotice::default() + }; + let all = case.edit(Scope::All, title("Retro")).unwrap(); + assert!(all.guests == Some(two(false)), "a and b get the update"); + assert!(all.needs_notify(), "unanswered, it may not run"); + let gone = case.delete(Scope::All).unwrap(); + assert!(gone.guests == Some(two(true)), "a and b are told"); + let split = case.edit(Scope::Following, title("Retro")).unwrap(); + assert!(split.guests.is_some() && split.needs_notify()); // Only the user on the list isn't a guest. let just_me = with( weekly(), "attendees", json!([{ "email": "me@example.com", "self": true }]), ); - assert!(Case::new(just_me, monday_19()) + let alone = Case::new(just_me, monday_19()) .edit(Scope::All, title("Retro")) - .is_ok()); + .unwrap(); + assert!(alone.guests.is_none() && !alone.needs_notify()); let solo = with( plain("2026-10-26T09:00:00Z"), "summary", json!("Solo review"), ); - let case = Case::new(with_guests(), monday_19()) - .exceptions(vec![solo]) - .guests(); + let case = Case::new(with_guests(), monday_19()).exceptions(vec![solo]); assert_eq!( case.edit(Scope::Following, title("Retro")), Err(NotPlanned::Refused(SeriesRefusal::CannotCarry(day( @@ -3527,7 +3488,7 @@ mod tests { /// shared property changed, does. #[test] fn a_split_watches_the_guests_it_copies_but_not_their_replies() { - let mut case = Case::new(with_guests(), monday_19()).guests(); + let mut case = Case::new(with_guests(), monday_19()); let replied = with( weekly(), "attendees", @@ -4787,58 +4748,61 @@ mod tests { assert_ne!(split(&busier, TAIL).choice_digest("s1"), token); } - /// Until C12, nothing past the occurrence picked reaches one with guests of its own: not a split - /// or a cut from before it, nor anything for every occurrence. "This event" elsewhere, and a split - /// after it, still go ahead; with guests allowed (C12), all of them do. + /// C12: an occurrence with guests of its own no longer holds back what reaches it (C9a-2's + /// refusal until Send / Don't send). A write to the whole series counts it as "at least", since + /// PM can't tell whether Google emails that occurrence's guests too (probe L-R13-4); "this and + /// following" past it still can't carry its guests onto the new part (Q3). #[test] - fn an_occurrence_with_guests_of_its_own_holds_back_what_reaches_it() { + fn an_occurrence_with_guests_of_its_own_is_counted_by_a_whole_series_write() { let invited = with( plain("2026-10-26T09:00:00Z"), "attendees", json!([{ "email": "a@example.com", "responseStatus": "needsAction" }]), ); - let held = Err(NotPlanned::Refused(SeriesRefusal::GuestsElsewhere(day( + let cant_carry = Err(NotPlanned::Refused(SeriesRefusal::CannotCarry(day( 2026, 10, 26, )))); + let at_least = |plan: SeriesPlan| { + plan.needs_notify() + && plan + .guests + .is_some_and(|g| !g.complete && g.people == 0 && g.emails == 0) + }; let case = Case::new(weekly(), monday_19()).exceptions(vec![invited.clone()]); assert!(case.edit(Scope::This, title("Retro")).is_ok()); assert!(case.delete(Scope::This).is_ok()); - assert_eq!(case.edit(Scope::Following, title("Retro")), held); - assert_eq!(case.edit(Scope::All, title("Retro")), held); - assert_eq!(case.delete(Scope::Following), held); - assert_eq!(case.delete(Scope::All), held); + assert_eq!(case.edit(Scope::Following, title("Retro")), cant_carry); + assert!(at_least(case.edit(Scope::All, title("Retro")).unwrap())); + assert!(at_least(case.delete(Scope::All).unwrap())); + // A cut cancels that occurrence on the way: its guest is told, by count. + let cut = case.delete(Scope::Following).unwrap(); + assert!(cut + .guests + .is_some_and(|g| g.cancelled && g.updated == 1 && g.complete)); let after = Case::new(weekly(), plain("2026-11-02T09:00:00Z")).exceptions(vec![invited.clone()]); - assert!(after.edit(Scope::Following, title("Retro")).is_ok()); + let split = after.edit(Scope::Following, title("Retro")).unwrap(); + assert!(split.guests.is_none(), "nobody past it to tell"); assert!(after.delete(Scope::Following).is_ok()); - assert_eq!(after.edit(Scope::All, title("Retro")), held); + assert!(at_least(after.edit(Scope::All, title("Retro")).unwrap())); // From the first occurrence, "this and following" is every occurrence. - let first = Case::new(weekly(), plain("2026-10-05T09:00:00+01:00")) - .exceptions(vec![invited.clone()]); - assert_eq!(first.delete(Scope::Following), held); - assert_eq!(first.edit(Scope::Following, title("Retro")), held); - let allowed = Case::new(weekly(), monday_19()) - .exceptions(vec![invited]) - .guests(); - assert!(allowed.delete(Scope::All).is_ok()); - } - - /// Until C12, only a whole listing shows that nothing a plan reaches has guests of its own, so a - /// short one refuses even a change of text to every occurrence, and deleting them. With guests - /// allowed, the change of text goes ahead and says "some" may keep or lose their changes; the - /// warning carries that, so the token tells "some" from a count. + let first = + Case::new(weekly(), plain("2026-10-05T09:00:00+01:00")).exceptions(vec![invited]); + assert!(at_least(first.delete(Scope::Following).unwrap())); + } + + /// The over-correction named (C12 §7.2): C9a-2's guests rule refused a short listing for a change + /// of text to every occurrence and for deleting them, since only a whole listing showed no + /// occurrence had guests. With guests allowed those run, and say "some" may keep or lose their + /// changes; the warning carries that, so the token tells "some" from a count. On a series with + /// guests the count is "at least" too. #[test] - fn a_short_listing_refuses_what_reaches_past_this_occurrence_until_c12() { + fn a_short_listing_lets_text_go_to_all_events_and_says_some() { let mut short = Case::new(weekly(), monday_19()); short.exceptions.complete = false; - let incomplete = Err(NotPlanned::Refused(SeriesRefusal::Incomplete)); assert!(short.edit(Scope::This, title("Retro")).is_ok()); - assert_eq!(short.edit(Scope::All, title("Retro")), incomplete); - assert_eq!(short.delete(Scope::All), incomplete); - assert_eq!(short.delete(Scope::Following), incomplete); - let mut allowed = Case::new(weekly(), monday_19()).guests(); - allowed.exceptions.complete = false; - let some = allowed.edit(Scope::All, title("Retro")).unwrap(); + assert!(short.delete(Scope::All).is_ok()); + let some = short.edit(Scope::All, title("Retro")).unwrap(); assert_eq!( some.warnings, [SeriesWarning::ExceptionsMayReset { @@ -4847,21 +4811,24 @@ mod tests { }] ); let seen = Case::new(weekly(), monday_19()) - .guests() .edit(Scope::All, title("Retro")) .unwrap(); assert!(seen.warnings.is_empty()); assert_ne!(some.choice_digest("s1"), seen.choice_digest("s1")); + let mut guests = Case::new(with_guests(), monday_19()); + guests.exceptions.complete = false; + let counted = guests.edit(Scope::All, title("Retro")).unwrap(); + assert!(counted + .guests + .is_some_and(|g| !g.complete && g.updated == 2 && g.people == 2)); // The same count, seen whole or not: two tokens. let retitled = with( plain("2026-10-26T09:00:00Z"), "summary", json!("Standup (room 4)"), ); - let whole = Case::new(weekly(), monday_19()) - .exceptions(vec![retitled]) - .guests(); - let mut part = Case::new(weekly(), monday_19()).guests(); + let whole = Case::new(weekly(), monday_19()).exceptions(vec![retitled]); + let mut part = Case::new(weekly(), monday_19()); part.exceptions = Exceptions { items: whole.exceptions.items.clone(), complete: false, @@ -4886,11 +4853,11 @@ mod tests { assert_ne!(whole.choice_digest("s1"), part.choice_digest("s1")); } - /// With guests allowed (C12), the guests rule no longer refuses a short listing, and the guards - /// that need it whole still do: a cut, a split, and a move of every occurrence. + /// No guests rule refuses a short listing any more (C12), and the guards that need it whole + /// still do: a cut, a split, and a move of every occurrence. #[test] - fn a_short_listing_still_refuses_cuts_splits_and_moves_with_guests_allowed() { - let mut short = Case::new(weekly(), monday_19()).guests(); + fn a_short_listing_still_refuses_cuts_splits_and_moves() { + let mut short = Case::new(weekly(), monday_19()); short.exceptions.complete = false; let incomplete = Err(NotPlanned::Refused(SeriesRefusal::Incomplete)); assert_eq!(short.delete(Scope::Following), incomplete); @@ -5193,13 +5160,12 @@ mod tests { } /// The weekly series and its Monday 19-10 occurrence, both with `list` as guests besides the - /// user, as C12c will plan them (guests allowed). + /// user. fn invited(list: &[&str]) -> Case { Case::new( guests_on_it(weekly(), list), guests_on_it(monday_19(), list), ) - .guests() } /// Who each step reaches, as the plan was stamped by. @@ -5220,6 +5186,7 @@ mod tests { updated, invited, removed, + told_cancelled: 0, cancelled: false, people: updated + invited + removed, emails, @@ -5328,6 +5295,7 @@ mod tests { updated: 2, invited: 1, removed: 0, + told_cancelled: 0, cancelled: false, people: 3, emails: 10, @@ -5399,7 +5367,7 @@ mod tests { /// C12 §6.2: a write to every occurrence is "at least" where an occurrence has a guest list of /// its own, or where PM couldn't see every occurrence changed on its own; a short listing asks - /// even with nobody seen (C12 critique C1, "short listings ask"), as C12c will run it. + /// even with nobody seen (C12 critique C1, "short listings ask"). #[test] fn a_whole_series_write_is_at_least_when_an_occurrence_has_guests_of_its_own() { let own_list = guests_on_it( @@ -5429,7 +5397,7 @@ mod tests { .unwrap(); assert!(plan.guests == Some(told(1, 0, 0, 1)), "exact"); // A short listing, on a series with nobody else on it. - let mut short = Case::new(weekly(), monday_19()).guests(); + let mut short = Case::new(weekly(), monday_19()); short.exceptions.complete = false; let plan = short.edit(Scope::All, title("Retro")).unwrap(); assert!( @@ -5626,7 +5594,7 @@ mod tests { v["attendees"][1]["responseStatus"] = json!("needsAction"); v }; - let case = Case::new(waiting(weekly()), waiting(monday_19())).guests(); + let case = Case::new(waiting(weekly()), waiting(monday_19())); let add_b = guests_to(&[(B, GuestState::Required)]); let conflict = Resend::Conflict(vec!["attendees".into()]); for (scope, now) in [(Scope::All, weekly()), (Scope::This, monday_19())] { @@ -5675,7 +5643,7 @@ mod tests { json!([{ "email": ME, "responseStatus": "accepted" }]), ) }; - let case = Case::new(unmarked(weekly()), unmarked(monday_19())).guests(); + let case = Case::new(unmarked(weekly()), unmarked(monday_19())); let input = SeriesInput { me: ME, ..case.input() @@ -5749,7 +5717,7 @@ mod tests { // A series whose list Google cut short: no new part copies part of it. let mut cut_short = guests_on_it(weekly(), &[A]); cut_short["attendeesOmitted"] = json!(true); - let short = Case::new(cut_short, guests_on_it(monday_19(), &[A])).guests(); + let short = Case::new(cut_short, guests_on_it(monday_19(), &[A])); assert!(refused_for( short.edit(Scope::Following, title("Retro")), SeriesRefusal::Guest(GuestError::Omitted) diff --git a/src-tauri/src/calendar_write/series/rule.rs b/src-tauri/src/calendar_write/series/rule.rs index c0faa852..6727716b 100644 --- a/src-tauri/src/calendar_write/series/rule.rs +++ b/src-tauri/src/calendar_write/series/rule.rs @@ -38,10 +38,10 @@ use super::super::time; use super::diff::{Expect, Removes}; use super::plan::{ all_plan, cancel_exception, cancelled_points, cut_step, draft_fields, exceptions_from, - insert_step, keeps_formatting, last_kept, no_guests_from, not_carried, own_exceptions, - rest_start, snapshot, split_unchanged_since_opened, start_of, tail_body, - unchanged_since_opened, when_nodes_in, NotPlanned, Op, Recheck, SeriesInput, SeriesPlan, - SeriesWarning, Shows, SplitCut, Stage, Step, Undo, TAIL_WATCH, + insert_step, keeps_formatting, last_kept, not_carried, own_exceptions, rest_start, snapshot, + split_unchanged_since_opened, start_of, tail_body, unchanged_since_opened, when_nodes_in, + NotPlanned, Op, Recheck, SeriesInput, SeriesPlan, SeriesWarning, Shows, SplitCut, Stage, Step, + Undo, TAIL_WATCH, }; use super::scope::{RuleChange, Scope}; use super::{ @@ -569,7 +569,6 @@ pub(super) fn plan_all_rule( ) -> Result { let (instance, master_json) = (input.fresh.instance.json(), input.fresh.master.json()); input.opened.master.ok_or(SeriesRefusal::NoBase)?; - no_guests_from(master, input, None)?; keeps_formatting(master_json, draft)?; every_line_readable(master)?; // The draft's time is read against the occurrence as Google holds it now, and a new rule moves @@ -856,7 +855,6 @@ pub(super) fn plan_split_rule( let zone = master.start.zone(); let rest = rest_start(master, at)?; let exceptions = exceptions_from(master, input.exceptions, at)?; - no_guests_from(master, input, Some(at))?; if draft.time.is_some() { // Read against the occurrence as Google holds it now: it must still be where the user saw it // (R2; C8b reviews ADV-1). @@ -3514,24 +3512,40 @@ mod tests { } } - /// Until C12, a change that would reach an occurrence with guests of its own is refused, for every - /// occurrence and from one on, a new rule or none. + /// C12: a rule change that reaches an occurrence with guests of its own is no longer refused + /// for it, but asks Send / Don't send first. For every occurrence, a new rule or none, it counts + /// them as "at least", since PM can't tell whether Google emails that occurrence's guests too + /// (probe L-R13-4); from one on, a new rule still can't carry its guests onto the new part (Q3), + /// and "Does not repeat" cancels that occurrence, its guest counted as told of the cancel, never + /// of an update: the plan's other writes don't reach them. #[test] - fn a_rule_change_refuses_to_reach_an_occurrence_with_guests() { + fn a_rule_change_that_reaches_an_occurrence_with_guests_asks_first() { let guests = with( plain("2026-10-26T09:00:00Z"), "attendees", json!([{ "email": "ana@example.com" }]), ); let case = Case::new(weekly(), monday_19()).exceptions(vec![guests]); - let refused = Err(NotPlanned::Refused(SeriesRefusal::GuestsElsewhere(day( - 2026, 10, 26, - )))); - for scope in [Scope::All, Scope::Following] { - for draft in [repeat(daily()), stop()] { - assert_eq!(case.edit(scope, draft), refused); - } + for draft in [repeat(daily()), stop()] { + let plan = case.edit(Scope::All, draft).unwrap(); + assert!(plan.needs_notify()); + assert!(plan + .guests + .is_some_and(|g| !g.complete && g.people == 0 && g.emails == 0)); } + assert_eq!( + case.edit(Scope::Following, repeat(daily())), + Err(NotPlanned::Refused(SeriesRefusal::CannotCarry(day( + 2026, 10, 26 + )))) + ); + let stopped = case.edit(Scope::Following, stop()).unwrap(); + assert!(stopped.needs_notify()); + assert!(stopped.guests.is_some_and(|g| g.updated == 0 + && g.told_cancelled == 1 + && !g.cancelled + && g.people == 1 + && g.complete)); } /// A formatted description is never saved over as plain text, whatever the rule. diff --git a/src-tauri/src/calendar_write/series/run.rs b/src-tauri/src/calendar_write/series/run.rs index 85b4fa4c..f5ebd1b2 100644 --- a/src-tauri/src/calendar_write/series/run.rs +++ b/src-tauri/src/calendar_write/series/run.rs @@ -1785,8 +1785,7 @@ mod tests { let case = Case::new( guests_on_it(weekly(), &["a@example.com"]), guests_on_it(monday_19(), &["a@example.com"]), - ) - .guests(); + ); let unanswered = case.edit(Scope::All, title("Retro")).unwrap(); assert!(unanswered.needs_notify()); assert_eq!(Run::new(unanswered).err(), Some(Unfit)); @@ -1906,7 +1905,7 @@ mod tests { v["attendees"][1]["responseStatus"] = json!("needsAction"); v }; - let case = Case::new(waiting(weekly()), waiting(monday_19())).guests(); + let case = Case::new(waiting(weekly()), waiting(monday_19())); let add_b = EventPatchDraft { guests: Some(vec![GuestOp { email: "b@example.com".into(), @@ -3332,7 +3331,6 @@ mod tests { let param = format!("sendUpdates={}", notify.as_param()); let case = Case::new(guest(weekly()), guest(monday_19())).exceptions(vec![guest(renamed())]); - let case = case.guests(); let plan = plan_series_edit( Scope::Following, &case.input(), @@ -3393,7 +3391,7 @@ mod tests { assert!(sent.last().unwrap().url.contains(¶m)); // A half-landed move put back, on a series retimed with this `sendUpdates`. - let case = Case::new(guest(weekly()), guest(monday_19())).guests(); + let case = Case::new(guest(weekly()), guest(monday_19())); let draft = at("09:00", "2026-10-20"); let plan = plan_series_edit(Scope::All, &case.input(), &draft, TAIL, Some(notify)).unwrap(); diff --git a/src-tauri/src/commands/calendar_edit.rs b/src-tauri/src/commands/calendar_edit.rs index bc4e59a3..0a21fa5c 100644 --- a/src-tauri/src/commands/calendar_edit.rs +++ b/src-tauri/src/commands/calendar_edit.rs @@ -60,7 +60,7 @@ use crate::calendar_write::series::diff::{self, Expect, GoneRow, Refetch, Remove use crate::calendar_write::series::journal::{self, Entry, Look, Orphan}; use crate::calendar_write::series::plan::{ marks_from, plan_series_delete, plan_series_edit, Fresh, NotPlanned, Op, Opened, SeriesInput, - SeriesPlan, SeriesWarning, Shows, Undo, + SeriesPlan, SeriesWarning, Shows, Step, Undo, }; use crate::calendar_write::series::run::{ Answer, End, Machine, Next, Ran, Rollback, Run, Settle, SettleAction, Settled, Unfit, Unsent, @@ -301,13 +301,6 @@ fn facts_from_fresh<'a>( .unwrap_or(false), visibility: text("visibility"), series_master: turned_series(fresh), - has_guests: fresh - .get("attendees") - .and_then(Value::as_array) - .is_some_and(|a| { - a.iter() - .any(|g| !g.get("self").and_then(Value::as_bool).unwrap_or(false)) - }), html_description: text("description").is_some_and(patch::description_is_html), } } @@ -334,7 +327,6 @@ fn facts_from_row<'a>(row: &'a CalendarEvent, calendar: &'a calendar::Calendar) // The mirror never holds a series itself (`calendar::parse_event`). An occurrence is // editable like a one-off here: which occurrences is asked at the write (rule R9). series_master: false, - has_guests: row.attendees.iter().any(|a| !a.is_self), html_description: false, } } @@ -944,6 +936,7 @@ async fn held_new_part( offers: view.offers, from: view.from, note: None, + emails: view.emails, }); } let failed = |e: Error| WriteOutcome::Failed { @@ -2221,10 +2214,6 @@ async fn send_delete( // --- repeating events (C9) --- -/// Whether a series with guests may change: not until C12 brings Send / Don't send, whatever the -/// occurrence shows (C8b review G5). -const GUESTS_ALLOWED: bool = false; - /// Pages read of a series' exceptions (100 a page) and of its occurrences (250 a page) before PM /// gives up and calls the listing incomplete. const EXCEPTION_PAGES: usize = 10; @@ -2591,11 +2580,6 @@ async fn gather( if let Some(reason) = master.read_only() { return Err(WriteOutcome::ReadOnly { reason }); } - if master.guests && !GUESTS_ALLOWED { - return Err(WriteOutcome::ReadOnly { - reason: ReadOnlyReason::HasGuests, - }); - } let calendar = target.remote_calendar.as_str(); let listed = match master_fresh.json().get("iCalUID").and_then(Value::as_str) { Some(ical) => { @@ -2775,6 +2759,7 @@ async fn choices( token: view.token.clone(), offers: view.offers.clone(), from: view.from.clone(), + emails: view.emails.clone(), }; } // One from before PM closed has no token to settle by: refused, with what to do. @@ -2846,7 +2831,6 @@ impl Planning { instances: self.series.counted.as_ref(), sibling: self.series.sibling, later: self.series.later, - guests_allowed: GUESTS_ALLOWED, me: &self.me, } } @@ -3346,6 +3330,7 @@ async fn prepared_for( offers: view.offers, from: view.from, note: None, + emails: view.emails, }); } } @@ -4594,10 +4579,36 @@ struct HeldSplit { tail_changed: bool, } +impl HeldSplit { + /// The offers that would email guests (C12): Remove deletes the new part with its insert's own + /// Send / Don't send, and Finish sends the rest of the plan, each step with its own. Only a change + /// that went with Send, to someone, emails anyone this way. + fn emails(&self) -> Vec { + let steps = &self.plan.steps; + let emails = |step: &Step| step.notify != Notify::None; + let insert = steps.iter().find(|s| matches!(s.op, Op::Insert { .. })); + self.offers + .iter() + .copied() + .filter(|action| match action { + SplitAction::Remove => insert.is_some_and(emails), + // What `Run::finish` sends: the steps after the one left unknown, which it proves. + SplitAction::Finish => match &self.ran { + Ran::Unknown { step, .. } => steps.iter().skip(step + 1).any(emails), + _ => false, + }, + SplitAction::Check => false, + }) + .collect() + } +} + /// What a blocked change or a choices row sees of a held split. struct HeldView { token: String, offers: Vec, + /// The offers that would email guests ([`HeldSplit::emails`]). + emails: Vec, from: String, master: String, tail: String, @@ -4628,11 +4639,12 @@ impl CalendarEditState { .is_ok_and(|running| running.contains(key)) } - /// What the split held under `token` allows now, and its day: `None` once it's settled or taken. - fn held_split(&self, token: &str) -> Option<(Vec, String)> { + /// What the split held under `token` allows now, its day, and which offers would email guests: + /// `None` once it's settled or taken. + fn held_split(&self, token: &str) -> Option<(Vec, String, Vec)> { let partials = self.partials.lock().ok()?; let held = partials.get(token)?; - Some((held.offers.clone(), held.facts.from.clone())) + Some((held.offers.clone(), held.facts.from.clone(), held.emails())) } fn hold_partial(&self, token: String, held: HeldSplit) { @@ -4653,6 +4665,7 @@ impl CalendarEditState { .map(|(token, h)| HeldView { token: token.clone(), offers: h.offers.clone(), + emails: h.emails(), from: h.facts.from.clone(), master: h.facts.master.clone(), tail: h.facts.tail.clone(), @@ -4857,6 +4870,7 @@ fn partial_outcome(token: &str, held: &HeldSplit) -> WriteOutcome { offers: held.offers.clone(), from: held.facts.from.clone(), note: None, + emails: held.emails(), } } @@ -5049,25 +5063,18 @@ async fn look_and_hold( drop_hash(host, cal); host.landed(cal); let token = token.unwrap_or_else(new_split_ref); - let from = facts.from.clone(); - host.edit().hold_partial( - token.clone(), - HeldSplit { - target: ctx.target.clone(), - plan, - ran, - facts, - offers: offers.clone(), - tail_seen, - tail_changed: false, - }, - ); - WriteOutcome::PartialSplit { - token, + let held = HeldSplit { + target: ctx.target.clone(), + plan, + ran, + facts, offers, - from, - note: None, - } + tail_seen, + tail_changed: false, + }; + let outcome = partial_outcome(&token, &held); + host.edit().hold_partial(token, held); + outcome } } } @@ -5337,7 +5344,7 @@ fn still_held(edit: &CalendarEditState, token: &str, outcome: WriteOutcome) -> W // Only creating an event answers this; a settle never does. | WriteOutcome::Ambiguous { .. } => return outcome, }; - let Some((offers, from)) = edit.held_split(token) else { + let Some((offers, from, emails)) = edit.held_split(token) else { return outcome; }; WriteOutcome::PartialSplit { @@ -5345,6 +5352,7 @@ fn still_held(edit: &CalendarEditState, token: &str, outcome: WriteOutcome) -> W offers, from, note: Some(note), + emails, } } @@ -6494,9 +6502,6 @@ fn read_only_message(reason: ReadOnlyReason) -> &'static str { ReadOnlyReason::BecameRepeating => { "This became a repeating event in Google. Refresh, then open one of its occurrences." } - ReadOnlyReason::HasGuests => { - "This event has guests, and PM can't change events with guests yet." - } ReadOnlyReason::SpecialType => { "Birthdays, events from Gmail, focus time, out of office and working locations can't \ be changed here." @@ -7284,13 +7289,13 @@ mod tests { "description": "agenda" }); let f = facts_from_fresh(&mine, &facts, EditingStatus::On); - assert!(f.organizer_self && !f.has_guests && f.html_description && !f.series_master); + assert!(f.organizer_self && f.html_description && !f.series_master); let invite = json!({ "organizer": { "email": "boss@x.com" }, "recurringEventId": "s1", "attendees": [{ "email": "me@x.com", "self": true }, { "email": "boss@x.com" }] }); let f = facts_from_fresh(&invite, &facts, EditingStatus::On); - assert!(!f.organizer_self && f.has_guests && !f.series_master); + assert!(!f.organizer_self && !f.series_master); assert_eq!(gate::edit_block(&f), Some(ReadOnlyReason::NotOrganizer)); // An occurrence, a one-off, and a one-off Google has since given a rule: only the last is a // series itself, with its own reason (C9 design critique 2 M6). @@ -7401,7 +7406,6 @@ mod tests { locked: true, visibility: None, series_master: false, - has_guests: false, html_description: false, }); let title = EventPatchDraft { @@ -7728,8 +7732,9 @@ mod tests { ); } - /// C12 §6.9: a series delete that would tell someone is never held, nor sent, unanswered. Until - /// the lift (C12c) no series with guests gets this far, so the order is read from the source. + /// C12 §6.9: a series delete that would tell someone is never held, nor sent, unanswered. The + /// send's check can't be reached by a plan held with its answer (the token binds who it tells), + /// so both orders are read from the source; the hold's is run below too. #[test] fn a_series_delete_is_never_held_or_sent_unanswered() { let source = include_str!("calendar_edit.rs"); @@ -8177,6 +8182,61 @@ mod tests { assert!(outcome == saved(), "deleted, as chosen"); } + /// C12c review: a delete from the editor asks with Google's list, which can be newer than PM's + /// row. Here the row shows A, Google's copy (what the editor showed) A and B, and the answer is + /// Send for those two: it is held, not asked again with the row's one, and at the send Google's + /// copy matches it, so both are emailed and it is deleted. + #[tokio::test] + async fn an_editor_delete_answered_for_googles_newer_list_is_held_and_deleted() { + let event = dentist(); + let me = json!({ "email": "me@example.com", "self": true, "organizer": true }); + let a = json!({ "email": "a@x.org", "responseStatus": "accepted" }); + let b = json!({ "email": "b@x.org", "responseStatus": "needsAction" }); + let stored = written( + &event, + &[("attendees", json!([me.clone(), a.clone()]))], + "\"s1\"", + ); + let host = TestHost::holding(std::slice::from_ref(&stored)); + let target = target(&event); + let row = host + .with_conn(|conn| row_guests(conn, &target.row_id, &own_of(&target, &[]))) + .unwrap(); + let two = GuestNotice { + updated: 2, + people: 2, + emails: 2, + cancelled: true, + complete: true, + ..Default::default() + }; + assert!( + row.as_ref().is_some_and(|r| r.people == 1), + "PM's row shows A alone" + ); + let answer = answered(Notify::All, two.clone()); + assert!(answer.before_hold(row) == Ok(()), "held, not asked again"); + let google = written(&event, &[("attendees", json!([me, a, b]))], "\"s2\""); + assert!( + attendees::delete_notice(&google, &own_of(&target, &[&google])) == Some(two), + "the editor's numbers are Google's" + ); + let script = Script::new(vec![ + Step::get("/events/solo", "", google), + told( + Method::Delete, + "/events/solo", + "\"s2\"", + "sendUpdates=all", + Scripted::Status(204), + ), + ]); + let seen = seen_by_user(&fresh_event(stored)); + let outcome = send_delete(&script, &*host, &target, &seen, &answer).await; + script.asked_all(); + assert!(outcome == saved(), "deleted, emailing both"); + } + /// C12 §6.10: a first guest added to a repeating event (allowed: it has none yet) is counted on /// every row the dialog offers. #[tokio::test] @@ -8242,37 +8302,374 @@ mod tests { etag, ) }; - let occurrences: Vec = october() - .iter() - .enumerate() - .map(|(i, e)| with_b(e, &format!("\"r{i}\""))) - .collect(); - let mut steps = october_reads(); + let occurrences: Vec = october() + .iter() + .enumerate() + .map(|(i, e)| with_b(e, &format!("\"r{i}\""))) + .collect(); + let mut steps = october_reads(); + steps.push(told( + Method::Patch, + "/events/abc", + "\"m1\"", + "sendUpdates=all", + Scripted::Json(with_b(&series, "\"m2\"")), + )); + steps.push(refetch_read(Scripted::Json(page(occurrences, None)))); + let script = Script::new(steps); + let (outcome, _) = save_told( + &host, + &script, + &instance, + &series, + draft, + Scope::All, + Some(Notify::All), + ) + .await; + script.asked_all(); + assert!(outcome == saved(), "saved, as chosen"); + assert!( + first_write(&script).body == Some(json!({ "attendees": [{ "email": "b@x.org" }] })), + "the series' list with the guest" + ); + } + + // --- guests on events you organise (C12c): the gate lifted --- + + /// `event` you organise with one guest, `a@x.org`, who has said yes. + fn with_a_guest(event: Value) -> Value { + with( + event, + "attendees", + json!([ + { "email": "me@example.com", "self": true, "organizer": true, + "responseStatus": "accepted" }, + { "email": "a@x.org", "responseStatus": "accepted" } + ]), + ) + } + + /// One guest already on it, told of a change. + fn one_updated() -> GuestNotice { + GuestNotice { + updated: 1, + people: 1, + emails: 1, + complete: true, + ..Default::default() + } + } + + /// One guest already on it, told it's cancelled. + fn one_told() -> GuestNotice { + GuestNotice { + cancelled: true, + ..one_updated() + } + } + + /// C12 §5.1, §6.9: deleting every occurrence of a series with a guest, unanswered, is asked + /// before anything is held, with only reads sent; with Send, the series' DELETE says so. + #[tokio::test] + async fn a_series_delete_with_guests_is_asked_before_its_hold() { + let guest = |event: Value| { + with( + event, + "attendees", + json!([{ "email": "a@example.com", "responseStatus": "accepted" }]), + ) + }; + let (series, instance) = (guest(weekly()), guest(monday_19())); + let window: Vec = october().into_iter().map(guest).collect(); + let host = TestHost::holding(&window); + let target = target(&instance); + let seen = seen_by_user(&fresh_event(instance.clone())); + let script = Script::new(reads( + &instance, + &series, + vec![series.clone()], + window.clone(), + )); + let ctx = writer(&script, &host, &target); + let asked = plan_held_delete(&ctx, &target.row_id, &seen, Scope::All, None, None).await; + script.finished(); + assert!( + matches!(&asked, Err(outcome) if *outcome == WriteOutcome::needs_notify(one_told())), + "asked, one guest told" + ); + assert!(script.sent().iter().all(|p| p.method == Method::Get)); + let mut steps = reads(&instance, &series, vec![series.clone()], window.clone()); + steps.extend(reads(&instance, &series, vec![series.clone()], window)); + steps.push(told( + Method::Delete, + "/events/abc", + "\"m1\"", + "sendUpdates=all", + Scripted::Status(204), + )); + steps.push(refetch_read(Scripted::Status(410))); + let script = Script::new(steps); + let outcome = + held_delete_told(&host, &script, &instance, Scope::All, Some(Notify::All)).await; + script.asked_all(); + assert!(outcome == saved(), "deleted, the guest told"); + } + + /// C12 §7.3: an event you organise with guests is editable, in the list and in the editor, its + /// guest list included; an invitation still isn't (C14). + #[test] + fn an_organisers_event_with_guests_is_editable() { + let (_d, conn) = store(); + let cal = "gcal:me@x.com:me@x.com"; + add_calendar(&conn, "gcal:me@x.com", "google", cal, Some("me@x.com")); + let guest = |email: &str, me: bool| calendar::Attendee { + email: Some(email.into()), + is_self: me, + organizer: me, + ..Default::default() + }; + let mine = CalendarEvent { + id: format!("{cal}:a"), + calendar_id: cal.into(), + summary: "Mine".into(), + start: "2026-10-12T09:00:00Z".into(), + organizer_self: true, + attendees: vec![guest("me@x.com", true), guest("a@x.org", false)], + ..Default::default() + }; + let invite = CalendarEvent { + id: format!("{cal}:b"), + organizer_self: false, + ..mine.clone() + }; + assert_eq!( + edit_blocks(&conn, &[mine, invite]).unwrap(), + vec![None, Some(ReadOnlyReason::NotOrganizer)], + "the list: editable when it's yours" + ); + let facts = CalendarFacts { + access_role: Some("owner".into()), + ..Default::default() + }; + let fresh = with_a_guest(dentist()); + let perms = gate::edit_rights(&facts_from_fresh(&fresh, &facts, EditingStatus::On)); + assert!( + perms.summary && perms.time && perms.guests && perms.delete && perms.reasons.is_empty(), + "the editor: every field, the guests too" + ); + } + + /// C12 §7.3: a title change on an event with a guest asks first, only the GET sent; Send goes + /// to Google with `all`, Don't send with `none`. + #[tokio::test] + async fn a_title_change_with_guests_asks_then_sends_all() { + let event = with_a_guest(dentist()); + let host = TestHost::holding(std::slice::from_ref(&event)); + let (outcome, script, _) = + save_one_off(&host, &event, target(&event), &title("Retro"), Vec::new()).await; + script.finished(); + assert!( + outcome == WriteOutcome::needs_notify(one_updated()), + "asked, one guest to update" + ); + assert_eq!(script.sent().len(), 1, "only the GET"); + for (notify, query) in [ + (Notify::All, "sendUpdates=all"), + (Notify::None, "sendUpdates=none"), + ] { + let host = TestHost::holding(std::slice::from_ref(&event)); + let (outcome, script, _) = save_one_off_told( + &host, + &event, + target(&event), + &title("Retro"), + vec![told( + Method::Patch, + "/events/solo", + "\"s1\"", + query, + Scripted::Json(retitled(&event, "\"s2\"")), + )], + answered(notify, one_updated()), + ) + .await; + script.asked_all(); + assert!(outcome == saved(), "saved, as chosen"); + } + } + + /// C12 §7.3: Busy/free is the user's own on their copy: a change to only it never asks, and + /// sends `none` (R7). + #[tokio::test] + async fn a_busy_free_change_with_guests_never_asks() { + let event = with_a_guest(dentist()); + let host = TestHost::holding(std::slice::from_ref(&event)); + let draft = EventPatchDraft { + show_as: Some(ShowAs::Free), + ..Default::default() + }; + let (outcome, script, _) = save_one_off( + &host, + &event, + target(&event), + &draft, + vec![told( + Method::Patch, + "/events/solo", + "\"s1\"", + "sendUpdates=none", + Scripted::Json(written( + &event, + &[("transparency", json!("transparent"))], + "\"s2\"", + )), + )], + ) + .await; + script.asked_all(); + assert!(outcome == saved(), "saved, nobody asked or emailed"); + } + + /// C12 §5.3: the answer given before the hold is the one the delete sends after it. + #[tokio::test] + async fn the_answer_is_taken_into_the_hold() { + let event = with_a_guest(dentist()); + let host = TestHost::holding(std::slice::from_ref(&event)); + let seen = seen_by_user(&fresh_event(event.clone())); + let script = Script::new(vec![ + Step::get("/events/solo", "", event.clone()), + told( + Method::Delete, + "/events/solo", + "\"s1\"", + "sendUpdates=all", + Scripted::Status(204), + ), + ]); + let outcome = send_delete( + &script, + &*host, + &target(&event), + &seen, + &answered(Notify::All, one_told()), + ) + .await; + script.asked_all(); + assert!(outcome == saved(), "deleted, the guest told"); + } + + /// C12 §5.3: a guest added in Google during the hold stops a delete nobody answered for: asked, + /// nothing deleted, and Google's copy, the guest on it, goes into the mirror, so a delete started + /// again asks before its hold (critique C1-H2). + #[tokio::test] + async fn guests_who_appear_during_the_hold_stop_the_delete() { + let event = dentist(); + let host = TestHost::holding(std::slice::from_ref(&event)); + let seen = seen_by_user(&fresh_event(event.clone())); + let invited = written(&with_a_guest(dentist()), &[], "\"s2\""); + let script = Script::new(vec![Step::get("/events/solo", "", invited)]); + let outcome = send_delete( + &script, + &*host, + &target(&event), + &seen, + &GuestAnswer::default(), + ) + .await; + script.finished(); + assert!( + outcome == WriteOutcome::needs_notify(one_told()), + "asked, nothing deleted" + ); + let own = own_of(&target(&event), &[]); + let asked = host + .with_conn(|conn| row_guests(conn, "cal:solo", &own)) + .unwrap(); + assert!(asked == Some(one_told()), "the next delete asks first"); + } + + /// C12 §7.3: "this and following" on a series with a guest, unanswered, is asked with every + /// email the split sends (the new part's invitation and the earlier part's update) and nothing + /// written; with Send, each step that reaches the guest goes with `all`. + #[tokio::test] + async fn a_split_with_guests_sends_each_step_its_answer_and_counts_every_email() { + let guest = |event: Value| { + with( + event, + "attendees", + json!([{ "email": "a@example.com", "responseStatus": "accepted" }]), + ) + }; + let (series, instance) = (guest(weekly()), guest(monday_19())); + let window: Vec = october().into_iter().map(guest).collect(); + let host = TestHost::holding(&window); + let reads = || reads(&instance, &series, vec![series.clone()], window.clone()); + let script = Script::new(reads()); + let (outcome, _) = save( + &host, + &script, + &instance, + &series, + title("Retro"), + Scope::Following, + ) + .await; + script.finished(); + let both = GuestNotice { + emails: 2, + ..one_updated() + }; + assert!( + outcome == WriteOutcome::needs_notify(both), + "asked, two emails to one guest" + ); + assert!( + script.sent().iter().all(|p| p.method == Method::Get), + "only reads" + ); + let plan = Case::new(series.clone(), instance.clone()) + .edit(Scope::Following, title("Retro")) + .unwrap(); + let Op::Patch { body: cut, .. } = &plan.steps[1].op else { + panic!("not a cut"); + }; + let mut steps = reads(); + steps.push(Step::new( + Method::Insert, + "/events", + "sendUpdates=all conferenceDataVersion=1", + Scripted::Json(tail_copy(&plan, "\"t1\"", SAVED_AT)), + )); steps.push(told( Method::Patch, "/events/abc", "\"m1\"", "sendUpdates=all", - Scripted::Json(with_b(&series, "\"m2\"")), + Scripted::Json(written( + &series, + &[("recurrence", cut["recurrence"].clone())], + "\"m2\"", + )), )); - steps.push(refetch_read(Scripted::Json(page(occurrences, None)))); + steps.push(refetch_read(Scripted::Json(page( + vec![window[0].clone()], + None, + )))); + steps.push(tail_refetch(Scripted::Json(page(tail_october(), None)))); let script = Script::new(steps); let (outcome, _) = save_told( &host, &script, &instance, &series, - draft, - Scope::All, + title("Retro"), + Scope::Following, Some(Notify::All), ) .await; script.asked_all(); - assert!(outcome == saved(), "saved, as chosen"); - assert!( - first_write(&script).body == Some(json!({ "attendees": [{ "email": "b@x.org" }] })), - "the series' list with the guest" - ); + assert!(outcome == saved(), "saved, each step as chosen"); } #[test] @@ -9045,9 +9442,10 @@ mod tests { assert_eq!(token(&following), token(&all)); } - /// A29: a listing by iCal UID that didn't run to its end refuses what reaches past the occurrence - /// picked, and the occurrences' own listing never stands in for it. Until C12 that includes a - /// change of text to all of them: only a whole listing shows none has guests of its own. + /// A29: a listing by iCal UID that didn't run to its end refuses a split or a cut past the + /// occurrence picked, and the occurrences' own listing never stands in for it. A change of text + /// to all of them, or deleting them, goes ahead since C12 (no guests rule needs the listing + /// whole any more), saying "some" may keep or lose their changes and "at least" who is told. #[tokio::test] async fn an_exceptions_page_that_fails_refuses_incomplete_never_reads_instances() { let (series, instance) = (weekly(), monday_19()); @@ -9071,7 +9469,19 @@ mod tests { ask(&script, &instance, edit(title("Retro")), opened, &["abc"]).await; assert!(script.sent()[3].url.contains("pageToken=p2")); assert!(matches!(this, ScopeRow::Offered { .. })); - assert_eq!((following, all), (incomplete(), incomplete())); + assert_eq!(following, incomplete()); + let at_least = |row: &ScopeRow, cancelled: bool| { + matches!(row, ScopeRow::Offered { guests: Some(g), .. } + if !g.complete && g.cancelled == cancelled && g.people == 0) + }; + assert!(at_least(&all, false), "every occurrence, at least"); + assert_eq!( + warnings(&all), + [SeriesWarningDto::ExceptionsMayReset { + count: 0, + complete: false + }] + ); let script = Script::new(failing(&instance)); let delete = ChoiceRequest::Delete { event_id: "cal:abc_20261019T080000Z".into(), @@ -9079,13 +9489,13 @@ mod tests { }; let (this, following, all) = ask(&script, &instance, delete, None, &["abc"]).await; assert!(matches!(this, ScopeRow::Offered { .. })); - assert_eq!((following, all), (incomplete(), incomplete())); + assert_eq!(following, incomplete()); + assert!(at_least(&all, true), "every occurrence deleted, at least"); } /// C9 design §0.8, review M3/L4: an occurrence moved or cancelled on its own that the listing by /// iCal UID misses refuses a split, and an "all events" move, which would otherwise lose or bring - /// it back. (Until C12 the guests rule refuses them first, a change of text to every occurrence - /// too; plan.rs tests each guard on its own with guests allowed.) + /// it back. A change of text to every occurrence goes ahead since C12, and says "some". #[tokio::test] async fn a_moved_occurrence_the_listing_misses_refuses_incomplete() { let series = weekly(); @@ -9102,10 +9512,14 @@ mod tests { let (this, following, all) = ask(&script, &instance, edit(title("Retro")), opened, &["abc"]).await; assert!(matches!(this, ScopeRow::Offered { .. })); + assert_eq!(following, incomplete(), "no split past a missed occurrence"); assert_eq!( - (&following, &all), - (&incomplete(), &incomplete()), - "{missed}" + warnings(&all), + [SeriesWarningDto::ExceptionsMayReset { + count: 0, + complete: false + }], + "a change of text to every occurrence, with some" ); let window = vec![missed.clone(), instance.clone()]; let script = Script::new(reads(&instance, &series, vec![series.clone()], window)); @@ -9179,39 +9593,38 @@ mod tests { assert!(matches!(all, ScopeRow::Offered { .. })); } - /// C8b review G5: a series with guests stays read-only until C12, even from an occurrence whose - /// guests were removed on its own. + /// C12 (C8b review G5's refusal lifted): a series with a guest offers every row, each saying who + /// it would email. "This and following" writes two events, the new part and the earlier one, + /// so the same guest gets two emails. #[tokio::test] - async fn a_series_with_a_guest_is_read_only_until_c12() { - let series = with( - weekly(), - "attendees", - json!([{ "email": "a@example.com", "responseStatus": "accepted" }]), - ); - let instance = monday_19(); - let script = Script::new(vec![ - instance_read(&instance), - Step::get("/events/abc", "", series.clone()), - ]); - let got = choices( - &script, - &*TestHost::holding(&[]), - &target(&instance), - EditingStatus::On, - &mirror(&["abc"]), - &edit(title("Retro")), - Some((&instance, Some(&series))), - ) - .await; + async fn a_series_with_guests_offers_every_row_with_its_emails() { + let guest = |event: Value| { + with( + event, + "attendees", + json!([{ "email": "a@example.com", "responseStatus": "accepted" }]), + ) + }; + let (series, instance) = (guest(weekly()), guest(monday_19())); + let window = october().into_iter().map(guest).collect(); + let script = Script::new(reads(&instance, &series, vec![series.clone()], window)); + let opened = Some((&instance, Some(&series))); + let (this, following, all) = + ask(&script, &instance, edit(title("Retro")), opened, &["abc"]).await; script.finished(); - assert_eq!( - got, - SeriesChoices::Refused { - result: WriteOutcome::ReadOnly { - reason: ReadOnlyReason::HasGuests - } - } - ); + let told = |emails| GuestNotice { + updated: 1, + people: 1, + emails, + complete: true, + ..Default::default() + }; + for (row, want) in [(&this, told(1)), (&following, told(2)), (&all, told(1))] { + assert!( + matches!(row, ScopeRow::Offered { guests: Some(g), .. } if *g == want), + "every row offered, with its emails" + ); + } } /// C9 design review L7: an occurrence with a plain description of its own, in a series whose @@ -9880,11 +10293,12 @@ mod tests { ); } - /// Until C12, a row that would reach an occurrence with guests of its own is refused: listed by - /// iCal UID (the planners) or only in Google's occurrences (here), and only from where it - /// reaches. "This event" on another occurrence is still offered. + /// C12: an occurrence with guests of its own is counted, not refused (C9a-2's refusal lifted), + /// whether listed by iCal UID or seen only in Google's occurrences. "All events" is offered as + /// "at least" (PM can't tell whether Google emails that occurrence's guests too); "this and + /// following" past it still can't carry its guests onto the new part (Q3). #[tokio::test] - async fn guests_on_another_occurrence_hold_back_the_rows_that_reach_it() { + async fn an_occurrence_with_guests_of_its_own_is_counted_not_refused() { let (series, instance) = (weekly(), monday_19()); let invited = |at: &str| { with( @@ -9894,12 +10308,15 @@ mod tests { ) }; let opened = Some((&instance, Some(&series))); - let held = |day: &str| ScopeRow::Refused { - message: format!( - "The occurrence on {day} has guests of its own, and PM can't change events with \ - guests yet. Choose “This event” to change only this one." - ), + let cant_carry = |y, m, d| ScopeRow::Refused { + message: SeriesRefusal::CannotCarry(chrono::NaiveDate::from_ymd_opt(y, m, d).unwrap()) + .to_string(), }; + let at_least = |row: &ScopeRow, cancelled: bool| { + matches!(row, ScopeRow::Offered { guests: Some(g), .. } + if !g.complete && g.cancelled == cancelled && g.people == 0) + }; + let nobody = |row: &ScopeRow| matches!(row, ScopeRow::Offered { guests: None, .. }); let later = invited("2026-10-26T09:00:00Z"); let script = Script::new(reads( &instance, @@ -9909,11 +10326,9 @@ mod tests { )); let (this, following, all) = ask(&script, &instance, edit(title("Retro")), opened, &["abc"]).await; - assert!(matches!(this, ScopeRow::Offered { .. })); - assert_eq!( - (&following, &all), - (&held("26-10-2026"), &held("26-10-2026")) - ); + assert!(nobody(&this)); + assert_eq!(following, cant_carry(2026, 10, 26)); + assert!(at_least(&all, false), "all events, at least"); let delete = || ChoiceRequest::Delete { event_id: "cal:abc_20261019T080000Z".into(), seen: seen_by_user(&fresh_event(instance.clone())), @@ -9925,8 +10340,14 @@ mod tests { vec![instance.clone(), later.clone()], )); let (this, following, all) = ask(&script, &instance, delete(), None, &["abc"]).await; - assert!(matches!(this, ScopeRow::Offered { .. })); - assert_eq!((following, all), (held("26-10-2026"), held("26-10-2026"))); + assert!(nobody(&this)); + // The cut cancels that occurrence: its guest is told, counted. + assert!( + matches!(&following, ScopeRow::Offered { guests: Some(g), .. } + if g.cancelled && g.updated == 1 && g.complete), + "its guest told of the cancellation" + ); + assert!(at_least(&all, true), "every occurrence deleted, at least"); // Before the occurrence picked: "this and following" doesn't reach it. let earlier = invited("2026-10-12T09:00:00+01:00"); let script = Script::new(reads( @@ -9937,10 +10358,10 @@ mod tests { )); let (_, following, all) = ask(&script, &instance, edit(title("Retro")), opened, &["abc"]).await; - assert!(matches!(following, ScopeRow::Offered { .. })); - assert_eq!(all, held("12-10-2026")); + assert!(nobody(&following)); + assert!(at_least(&all, false)); // Missing from the listing, seen only in Google's occurrences: the cross-check counts it - // against the listing, which then can't show that nothing reached has guests. + // against the listing, which a split needs whole; "All events" says "some" and "at least". let script = Script::new(reads( &instance, &series, @@ -9949,8 +10370,16 @@ mod tests { )); let (this, following, all) = ask(&script, &instance, edit(title("Retro")), opened, &["abc"]).await; - assert!(matches!(this, ScopeRow::Offered { .. })); - assert_eq!((following, all), (incomplete(), incomplete())); + assert!(nobody(&this)); + assert_eq!(following, incomplete()); + assert!(at_least(&all, false)); + assert_eq!( + warnings(&all), + [SeriesWarningDto::ExceptionsMayReset { + count: 0, + complete: false + }] + ); } /// A change to every occurrence says how many were changed on their own and may keep or lose @@ -11224,8 +11653,133 @@ mod tests { } } - /// Design critique 1: until C12 a guest who appeared in Google before the resend a 412 calls for - /// is a conflict, whatever the step watches; nothing is sent again. + /// C12 critiques C1-M5, C2-H3: under Send, a guest who appeared in Google before the resend a 412 + /// calls for is no one the user answered for, so the save stops there and the next one asks with + /// the new numbers, as a one-off's second round does; nothing is sent again. + #[tokio::test] + async fn a_guest_added_before_a_resend_under_send_is_a_conflict() { + let guest = |event: Value| { + with( + event, + "attendees", + json!([{ "email": "a@example.com", "responseStatus": "accepted" }]), + ) + }; + let (series, instance) = (guest(weekly()), guest(monday_19())); + let window: Vec = october().into_iter().map(guest).collect(); + let host = TestHost::holding(&window); + let invited = written( + &series, + &[( + "attendees", + json!([{ "email": "a@example.com" }, { "email": "b@example.com" }]), + )], + "\"m2\"", + ); + let mut steps = reads(&instance, &series, vec![series.clone()], window.clone()); + steps.extend([ + told( + Method::Patch, + "/events/abc", + "\"m1\"", + "sendUpdates=all", + Scripted::Status(412), + ), + Step::get("/events/abc", "", invited), + ]); + let script = Script::new(steps); + let (outcome, _) = save_told( + &host, + &script, + &instance, + &series, + title("Retro"), + Scope::All, + Some(Notify::All), + ) + .await; + script.asked_all(); + assert!( + outcome + == WriteOutcome::Conflict { + fields: vec!["attendees".into()] + }, + "stopped, to ask again" + ); + let writes = script + .sent() + .iter() + .filter(|r| r.method == Method::Patch) + .count(); + assert_eq!(writes, 1, "B is never emailed"); + } + + /// C12 decision 3 on a series with guests (C12c review): "All events" adds B under Send, and A + /// accepts in Google before the write lands, so it meets a 412. The guest list goes again built + /// on Google's copy then: A's reply stays, B is still added, and the answer still goes with it. + #[tokio::test] + async fn a_series_guest_change_sent_again_keeps_a_reply_that_landed_meanwhile() { + let a = |reply: &str| json!({ "email": "a@example.com", "responseStatus": reply }); + let waiting = |event: Value| with(event, "attendees", json!([a("needsAction")])); + let (series, instance) = (waiting(weekly()), waiting(monday_19())); + let window: Vec = october().into_iter().map(waiting).collect(); + let host = TestHost::holding(&window); + let replied = written(&series, &[("attendees", json!([a("accepted")]))], "\"m2\""); + let both = json!([a("accepted"), { "email": "b@example.com" }]); + let mut steps = reads(&instance, &series, vec![series.clone()], window.clone()); + steps.extend([ + told( + Method::Patch, + "/events/abc", + "\"m1\"", + "sendUpdates=all", + Scripted::Status(412), + ), + Step::get("/events/abc", "", replied.clone()), + told( + Method::Patch, + "/events/abc", + "\"m2\"", + "sendUpdates=all", + Scripted::Json(written(&replied, &[("attendees", both.clone())], "\"m3\"")), + ), + refetch_read(Scripted::Json(page( + window + .iter() + .enumerate() + .map(|(i, e)| written(e, &[("attendees", both.clone())], &format!("\"r{i}\""))) + .collect(), + None, + ))), + ]); + let script = Script::new(steps); + let (outcome, _) = save_told( + &host, + &script, + &instance, + &series, + adding_guest("b@example.com"), + Scope::All, + Some(Notify::All), + ) + .await; + script.asked_all(); + assert!(outcome == saved(), "landed"); + let patches: Vec = script + .sent() + .into_iter() + .filter(|r| r.method == Method::Patch) + .collect(); + assert!(patches.len() == 2, "sent once more"); + assert!( + patches[1].body.as_ref().map(|b| &b["attendees"]) == Some(&both), + "A's reply kept, B still added" + ); + } + + /// Design critique 1: a guest who appeared in Google before the resend a 412 calls for, on a + /// plan nobody was asked about (it reached nobody), is a conflict, whatever the step watches; + /// nothing is sent again (C12 §6.5). #[tokio::test] async fn a_guest_added_before_a_resend_is_a_conflict() { let host = TestHost::holding(&october()); @@ -12289,10 +12843,10 @@ mod tests { fn still_held_with(host: &TestHost, outcome: &WriteOutcome, token: &str, words: &str) { let held = host.edit.held_split(token); assert!(held.is_some(), "the change is still held"); - let (offers, from) = held.unwrap(); + let (offers, from, emails) = held.unwrap(); assert!( - matches!(outcome, WriteOutcome::PartialSplit { token: t, offers: o, from: f, note: Some(n) } - if t == token && *o == offers && *f == from && n == words), + matches!(outcome, WriteOutcome::PartialSplit { token: t, offers: o, from: f, note: Some(n), emails: e } + if t == token && *o == offers && *f == from && n == words && *e == emails), "the answer is the held change with the outcome's words" ); } @@ -12963,6 +13517,64 @@ mod tests { } } + /// C12 (critiques C1-M4, C2-M2): a change held part-way says which settles would email guests: + /// Remove when its new part went with Send, Finish when what it would still send does. Sent with + /// Don't send, or to nobody, neither; the answer, the held view and the dialog's row agree. + #[tokio::test] + async fn a_held_split_says_which_settles_would_email_guests() { + use SplitAction::{Finish, Remove}; + let plan = split_plan(Vec::new()); + let host = TestHost::holding(&october()); + let (held, _, _) = split_save( + &host, + vec![ + insert_step(lost()), + event_read(TAIL, lost()), + event_read(TAIL, Scripted::Json(tail_copy(&plan, "\"t1\"", SAVED_AT))), + ], + ) + .await; + assert_eq!(offers_of(&held), [Finish, Remove]); + assert!( + matches!(&held, WriteOutcome::PartialSplit { emails, .. } if emails.is_empty()), + "a series without guests emails nobody" + ); + let token = held_token(&held); + let stamp = |insert: Notify, rest: Notify| { + let mut partials = host.edit.partials.lock().unwrap(); + let split = partials.get_mut(&token).unwrap(); + for step in split.plan.steps.iter_mut() { + step.notify = if matches!(step.op, Op::Insert { .. }) { + insert + } else { + rest + }; + } + }; + for (insert, rest, want) in [ + (Notify::All, Notify::None, vec![Remove]), + (Notify::None, Notify::All, vec![Finish]), + (Notify::All, Notify::All, vec![Finish, Remove]), + (Notify::None, Notify::None, vec![]), + ] { + stamp(insert, rest); + let held = host.edit.held_split(&token).unwrap(); + assert!(held.2 == want, "the settles that would email"); + let view = host.edit.held_on(CALENDAR, "abc").unwrap(); + assert!(view.emails == want, "the dialog's row says the same"); + let held = host.edit.partials.lock().unwrap()[&token].clone(); + assert!( + matches!(partial_outcome(&token, &held), WriteOutcome::PartialSplit { emails, .. } if emails == want), + "and the answer" + ); + // A settle that leaves it held says so too, its buttons' emails with it. + assert!( + matches!(still_held(&host.edit, &token, WriteOutcome::Busy), WriteOutcome::PartialSplit { emails, .. } if emails == want), + "and a settle's answer" + ); + } + } + /// Review round 3: a split held after its rollback couldn't delete the new part, then the new /// part deleted in Google: a settle's first look settles it. The original cut (the cut ignored) /// says the part was removed; the original deleted in Google (the cut answered 404) says gone and @@ -13089,7 +13701,7 @@ mod tests { vec![uncut(), event_read(TAIL, Scripted::Json(renamed))], ) .await; - let from = host.edit.held_split(&token).map(|(_, from)| from); + let from = host.edit.held_split(&token).map(|(_, from, _)| from); let words = tail_changed_words(from.as_deref().unwrap_or_default()); still_held_with(&host, &refused, &token, &words); assert_eq!(offers_of(&refused), [SplitAction::Check]); @@ -14806,6 +15418,7 @@ mod tests { let view = HeldView { token: "t".into(), offers: Vec::new(), + emails: Vec::new(), from: "2026-10-19".into(), master: "abc".into(), tail: TAIL.into(), @@ -16678,37 +17291,45 @@ mod tests { } /// A read-only reason outranks the choice: asking which occurrences of an event PM can't - /// change would lead nowhere. + /// change would lead nowhere. An occurrence you organise with a guest has no such reason since + /// C12: it is asked which occurrences, like any other. #[tokio::test] async fn an_unscoped_save_on_a_read_only_occurrence_says_why_first() { - let event = with( + let guest = |event: Value| with(event, "attendees", json!([{ "email": "guest@x.com" }])); + let invitation = guest(with( dentist_occurrence(), - "attendees", - json!([{ "email": "guest@x.com" }]), - ); - let target = target(&event); - let host = TestHost::holding(std::slice::from_ref(&event)); - let session = keep_session(&host.edit, target.clone(), event.clone(), None, None).unwrap(); - let open = open_session(&host.edit, &session).unwrap().unwrap(); - let script = Script::new(vec![Step::get("/events/solo", "", event)]); - let outcome = save_event( - &script, - &*host, - &session, - &open, - &title("Retro"), - EditingStatus::On, - &GuestAnswer::default(), - ) - .await; - script.finished(); - assert!(matches!( - outcome, - WriteOutcome::ReadOnly { - reason: ReadOnlyReason::HasGuests - } + "organizer", + json!({ "email": "guest@x.com" }), )); - assert_eq!(script.sent().len(), 1); + for (event, want) in [ + ( + invitation, + WriteOutcome::ReadOnly { + reason: ReadOnlyReason::NotOrganizer, + }, + ), + (guest(dentist_occurrence()), WriteOutcome::needs_scope()), + ] { + let target = target(&event); + let host = TestHost::holding(std::slice::from_ref(&event)); + let session = + keep_session(&host.edit, target.clone(), event.clone(), None, None).unwrap(); + let open = open_session(&host.edit, &session).unwrap().unwrap(); + let script = Script::new(vec![Step::get("/events/solo", "", event)]); + let outcome = save_event( + &script, + &*host, + &session, + &open, + &title("Retro"), + EditingStatus::On, + &GuestAnswer::default(), + ) + .await; + script.finished(); + assert!(outcome == want, "the reason first, else which occurrences"); + assert_eq!(script.sent().len(), 1); + } } /// A held delete with no choice whose row turned out to be an occurrence (the mirror row looked @@ -16855,7 +17476,6 @@ mod tests { SeriesRefusal::Incomplete, SeriesRefusal::NoBase, SeriesRefusal::CannotCarry(day), - SeriesRefusal::GuestsElsewhere(day), SeriesRefusal::FormattedDescription, SeriesRefusal::VisibilityIsSeriesWide, SeriesRefusal::OnlyThisDiffers, @@ -16873,6 +17493,13 @@ mod tests { SeriesRefusal::NothingToKeep, SeriesRefusal::KeptHasOwn(day), SeriesRefusal::DeletedDayUnread(day), + // The guest refusals, which rows show now events with guests can change (C12). + SeriesRefusal::Guest(attendees::GuestError::NotAnAddress), + SeriesRefusal::Guest(attendees::GuestError::Repeated), + SeriesRefusal::Guest(attendees::GuestError::CantRemove), + SeriesRefusal::Guest(attendees::GuestError::Omitted), + SeriesRefusal::GuestsDifferHere, + SeriesRefusal::GuestPermissionsAreSeriesWide, ]) // Every repeat the editor can't save, as a row repeats it (C11 §9.8). .chain( @@ -16900,7 +17527,6 @@ mod tests { ReadOnlyReason::PrivateEvent, ReadOnlyReason::NotOrganizer, ReadOnlyReason::BecameRepeating, - ReadOnlyReason::HasGuests, ReadOnlyReason::SpecialType, ReadOnlyReason::Locked, ReadOnlyReason::HtmlDescription, diff --git a/src-tauri/tauri.conf.json b/src-tauri/tauri.conf.json index 2e42f033..bfa41eee 100644 --- a/src-tauri/tauri.conf.json +++ b/src-tauri/tauri.conf.json @@ -1,7 +1,7 @@ { "$schema": "https://schema.tauri.app/config/2", "productName": "PM", - "version": "3.154.2-alpha", + "version": "3.155.0-alpha", "identifier": "org.itsatlas.pm", "build": { "beforeDevCommand": "npm run dev", diff --git a/src/components/CalendarConnection.tsx b/src/components/CalendarConnection.tsx index 81f4492c..8d2ac254 100644 --- a/src/components/CalendarConnection.tsx +++ b/src/components/CalendarConnection.tsx @@ -626,7 +626,7 @@ const EDITING_COPY: Record< enable: true, }, on: { - text: "Editing on: PM can create events, one-off or repeating, on this account’s calendars, and change and delete events you organise without guests there, repeating ones included.", + text: "Editing on: PM can create events, one-off or repeating, on this account’s calendars, and change and delete events you organise there, repeating ones and ones with guests included.", action: "Turn off editing", enable: false, paused: true, diff --git a/src/components/calendar/CalendarView.test.tsx b/src/components/calendar/CalendarView.test.tsx index 3dbfdce6..de53b0af 100644 --- a/src/components/calendar/CalendarView.test.tsx +++ b/src/components/calendar/CalendarView.test.tsx @@ -18,6 +18,9 @@ import type { CalendarAccount, CalendarEvent, CalendarOverview, + EditLoad, + EventForEdit, + GuestNotice, Milestone, WriteOutcome, } from "../../lib/types"; @@ -382,3 +385,127 @@ describe("CalendarView: a new event", () => { expect(screen.queryByRole("dialog", { name: "New event" })).toBeNull(); }); }); + +// The Calendar tab's guest wiring (#884, C12c review): the editor gets the people already in the +// user's calendars for its guest box, and a delete the backend asks about guests reopens its dialog +// with the backend's numbers and the account Google sends the emails from, not the row's own count. +describe("CalendarView: guests", () => { + const ann = { + name: "Ann", + email: "ann@x.org", + response: "accepted", + optional: false, + organizer: false, + self: false, + } as const; + + /** Click the Standup chip, then a button of its details. */ + async function fromPopover(button: "Edit" | "Delete") { + fireEvent.click(screen.getByRole("button", { name: /Standup/ })); + fireEvent.click(await screen.findByRole("button", { name: button })); + } + + it("suggests people from the user's calendars in the editor's guest box", async () => { + rows = [standup, row({ id: "e2", uid: "u2", summary: "Lunch", attendees: [ann] })]; + const editable: EventForEdit = { + summary: "Standup", + location: "", + description: "", + description_html: false, + time: { + kind: "timed", + start_date: today, + start_time: "07:00", + start_zone: "Europe/London", + end_date: today, + end_time: "07:30", + end_zone: "Europe/London", + }, + start_at: `${today}T06:00:00Z`, + end_at: `${today}T06:30:00Z`, + show_as: "busy", + visibility: "default", + attachments: [], + html_link: null, + guests: [], + guests_can_modify: false, + guests_can_invite_others: true, + guests_can_see_other_guests: true, + guests_omitted: false, + organizer_email: "me@x.com", + }; + const load: EditLoad = { + outcome: "ready", + session: "s1", + event: editable, + permissions: { + summary: true, + time: true, + location: true, + description: true, + show_as: true, + visibility: true, + guests: true, + delete: true, + reasons: [], + }, + seen: null, + series: null, + }; + ipc.getCalendarEventForEdit.mockResolvedValue(load); + await mount(); + await fromPopover("Edit"); + const editor = await screen.findByRole("dialog", { name: "Edit event" }); + const box = await within(editor).findByRole("combobox", { name: "Add guests" }); + fireEvent.change(box, { target: { value: "ann" } }); + const option = within(within(editor).getByRole("listbox", { name: "Suggestions" })).getByRole( + "option", + ); + expect(option.textContent).toContain("Ann"); + expect(option.textContent).toContain("ann@x.org"); + }); + + it("reopens a delete the backend asks about guests with its numbers and the sending account", async () => { + const told: GuestNotice = { + updated: 1, + invited: 0, + removed: 0, + told_cancelled: 0, + cancelled: true, + people: 1, + emails: 1, + complete: true, + }; + ipc.deleteCalendarEvent.mockResolvedValue({ + outcome: "refused", + result: { outcome: "needs_choice", scope: false, guests: told }, + }); + await mount(); + await fromPopover("Delete"); + const first = screen.getByRole("dialog", { name: "Delete this event?" }); + // The row shows no guests: nothing to ask yet. + expect(within(first).queryByRole("radio")).toBeNull(); + fireEvent.click(within(first).getByRole("button", { name: "Delete" })); + expect(await screen.findByText("1 guest will be told it's cancelled.")).toBeTruthy(); + const again = screen.getByRole("dialog", { name: "Delete this event?" }); + expect(within(again).getByText("Google sends the emails from me@x.com.")).toBeTruthy(); + expect( + within(again).getByText( + "It wasn't deleted. PM found guests who would be told. Choose whether Google emails them.", + ), + ).toBeTruthy(); + const del = within(again).getByRole("button", { name: "Delete" }); + expect(del.disabled).toBe(true); + fireEvent.click(within(again).getByRole("radio", { name: /^Send/ })); + fireEvent.click(del); + await waitFor(() => expect(ipc.deleteCalendarEvent).toHaveBeenCalledTimes(2)); + expect(ipc.deleteCalendarEvent).toHaveBeenLastCalledWith( + standup.id, + expect.anything(), + null, + null, + "all", + told, + ); + }); +}); diff --git a/src/components/calendar/CalendarView.tsx b/src/components/calendar/CalendarView.tsx index 8a576162..e4f9703e 100644 --- a/src/components/calendar/CalendarView.tsx +++ b/src/components/calendar/CalendarView.tsx @@ -29,6 +29,7 @@ import { } from "../../lib/ipc"; import type { CalendarEvent, CalendarOverview, Milestone } from "../../lib/types"; import { reasonText } from "../../lib/calendarEdit/editReasons"; +import { guestSuggestions } from "../../lib/calendarEdit/guests"; import { createdWhere, createReasonFor, @@ -806,6 +807,12 @@ export function CalendarView({ onOpenProject, onOpenPinboard }: CalendarViewProp // dialogs (the editor among them) itself; the popover isn't one. const deleteDialog = useDeleteDialog(popoverOpen); const confirmDelete = deleteDialog.asked?.event ?? null; + // People already in the user's Google and Outlook calendars, for the editor's guest box: worked out + // here, over the rows this view holds, so no contacts are read (C12; never a subscription's). + const suggestions = useMemo( + () => guestSuggestions(events, overview?.calendars ?? [], null), + [events, overview], + ); // The address of the account a synced event's calendar belongs to. const accountOf = (ev: CalendarEvent): string | null => { const cal = overview?.calendars.find((c) => c.id === ev.calendar_id); @@ -1155,6 +1162,7 @@ export function CalendarView({ onOpenProject, onOpenPinboard }: CalendarViewProp row={editing} calendar={overview?.calendars.find((c) => c.id === editing.calendar_id) ?? null} account={accountOf(editing)} + suggestions={suggestions} milestone={ editing.uid ? (milestones.find((m) => m.event_uid && m.event_uid === editing.uid) ?? null) @@ -1214,8 +1222,10 @@ export function CalendarView({ onOpenProject, onOpenPinboard }: CalendarViewProp overview?.calendars.find((c) => c.id === confirmDelete?.calendar_id)?.name ?? null } when={confirmDelete ? whenText(confirmDelete) : ""} + from={confirmDelete ? accountOf(confirmDelete) : null} again={deleteDialog.asked?.reask?.again ?? false} forceRepeating={!!deleteDialog.asked?.reask} + guests={deleteDialog.asked?.guests ?? null} onClose={deleteDialog.close} onConfirm={deleteDialog.confirm} /> diff --git a/src/components/calendar/edit/DeleteEventDialog.test.tsx b/src/components/calendar/edit/DeleteEventDialog.test.tsx index c766b836..a6438893 100644 --- a/src/components/calendar/edit/DeleteEventDialog.test.tsx +++ b/src/components/calendar/edit/DeleteEventDialog.test.tsx @@ -6,12 +6,14 @@ // Deleting an event (#884): a one-off confirms as it always has; a repeating one asks which // occurrences first, like Google Calendar, from Google's answer for the row the user saw, and the // delete names the occurrences picked with their token. Words the choices say once are never lost. +// One with guests asks whether Google emails them, Delete waiting for the answer (C12). import { StrictMode } from "react"; import { act, cleanup, fireEvent, render, screen, waitFor } from "@testing-library/react"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import type { CalendarEvent, + GuestNotice, ScopeRow, SeriesChoices, SplitAction, @@ -62,7 +64,15 @@ const ready = ( }); const NOTICE = "PM closed before it finished an earlier change to this repeating event."; -function open(event: CalendarEvent, props: { again?: boolean; forceRepeating?: boolean } = {}) { +function open( + event: CalendarEvent, + props: { + again?: boolean; + forceRepeating?: boolean; + from?: string; + guests?: { notice: GuestNotice; again: boolean }; + } = {}, +) { const onConfirm = vi.fn(); const onClose = vi.fn(); const utils = render( @@ -95,8 +105,151 @@ describe("DeleteEventDialog", () => { const { onConfirm } = open(oneOff); expect(screen.getByRole("dialog", { name: "Delete this event?" })).toBeTruthy(); fireEvent.click(screen.getByRole("button", { name: "Delete" })); - expect(onConfirm).toHaveBeenCalledWith(oneOff, null); + expect(onConfirm).toHaveBeenCalledWith(oneOff, null, null, null); expect(choicesForDelete).not.toHaveBeenCalled(); + // No guests to tell: no email question. + expect(screen.queryByRole("radio")).toBeNull(); + }); + + // C12 §14.3: a one-off you organise with a guest asks whether Google emails the cancellation, in + // the same dialog; Delete waits for the answer and focus starts on Cancel. + it("asks a one-off with guests whether to email them, Delete waiting for the answer", () => { + const withGuest: CalendarEvent = { + ...oneOff, + organizer_self: true, + attendees: [ + { + name: null, + email: "me@x.com", + response: "accepted", + optional: false, + organizer: true, + self: true, + }, + { + name: "Ann", + email: "ann@x.org", + response: "accepted", + optional: false, + organizer: false, + self: false, + }, + ], + }; + const { onConfirm } = open(withGuest, { from: "me@x.com" }); + expect(screen.getByText("Send cancellation emails to guests?")).toBeTruthy(); + expect(screen.getByText("1 guest will be told it's cancelled.")).toBeTruthy(); + expect(screen.getByText("Google sends the emails from me@x.com.")).toBeTruthy(); + const del = screen.getByRole("button", { name: "Delete" }) as HTMLButtonElement; + expect(del.disabled).toBe(true); + expect(document.activeElement).toBe(screen.getByRole("button", { name: "Cancel" })); + fireEvent.click(del); + expect(onConfirm).not.toHaveBeenCalled(); + fireEvent.click(radio(/^Don't send/)); + expect(del.disabled).toBe(false); + fireEvent.click(del); + expect(onConfirm).toHaveBeenCalledWith(withGuest, null, "none", { + updated: 1, + invited: 0, + removed: 0, + told_cancelled: 0, + cancelled: true, + people: 1, + emails: 1, + complete: true, + }); + }); + + // C12 critique C1-H2: reopened with the backend's numbers (Google's copy had guests the row + // didn't), it asks with those, as a one-off, saying so when an answer came for others. + it("asks with the backend's numbers when reopened for the guests", () => { + const notice = { + updated: 2, + invited: 0, + removed: 0, + told_cancelled: 0, + cancelled: true, + people: 2, + emails: 2, + complete: true, + }; + const { onConfirm } = open(occurrence, { guests: { notice, again: true } }); + expect(screen.getByRole("dialog", { name: "Delete this event?" })).toBeTruthy(); + expect(choicesForDelete).not.toHaveBeenCalled(); + expect(screen.getByText("2 guests will be told it's cancelled.")).toBeTruthy(); + expect( + screen.getByText( + "It wasn't deleted. PM checked with Google before deleting, and these are the numbers it found.", + ), + ).toBeTruthy(); + expect(screen.queryByText(/PM found guests/)).toBeNull(); + fireEvent.click(radio(/^Send/)); + fireEvent.click(screen.getByRole("button", { name: "Delete" })); + expect(onConfirm).toHaveBeenCalledWith(occurrence, null, "all", notice); + }); + + // C12c review: reopened for guests when nobody had answered (Google's copy has guests the row + // didn't show, found before the hold or at the send), it says the delete didn't happen and why, + // rather than coming back as if nothing had been tried. Round 2: the words stay true when the + // backend asks from PM's older row after Google dropped the guests, so not "it has guests now". + it("says it wasn't deleted when reopened for guests nobody had answered about", () => { + const notice: GuestNotice = { + updated: 1, + invited: 0, + removed: 0, + told_cancelled: 0, + cancelled: true, + people: 1, + emails: 1, + complete: true, + }; + const { onConfirm } = open(oneOff, { from: "me@x.com", guests: { notice, again: false } }); + expect( + screen.getByText( + "It wasn't deleted. PM found guests who would be told. Choose whether Google emails them.", + ), + ).toBeTruthy(); + expect(screen.queryByText(/has guests now/)).toBeNull(); + expect(screen.queryByText(/PM checked with Google/)).toBeNull(); + expect(screen.getByText("1 guest will be told it's cancelled.")).toBeTruthy(); + fireEvent.click(radio(/^Don't send/)); + fireEvent.click(screen.getByRole("button", { name: "Delete" })); + expect(onConfirm).toHaveBeenCalledWith(oneOff, null, "none", notice); + }); + + it("says nothing of a delete that didn't happen when it opens the first time", () => { + open(oneOff); + expect(screen.queryByText(/wasn't deleted/)).toBeNull(); + }); + + // An answer is for the numbers on screen: new ones ask afresh (R9). + it("asks afresh when the numbers change under an answer", () => { + const notice: GuestNotice = { + updated: 2, + invited: 0, + removed: 0, + told_cancelled: 0, + cancelled: true, + people: 2, + emails: 2, + complete: true, + }; + const props = { + event: oneOff, + calendarName: "Personal", + when: "12-10", + onConfirm: vi.fn(), + onClose: vi.fn(), + }; + const { rerender } = render(); + fireEvent.click(radio(/^Send/)); + const del = screen.getByRole("button", { name: "Delete" }) as HTMLButtonElement; + expect(del.disabled).toBe(false); + const three = { ...notice, updated: 3, people: 3, emails: 3 }; + rerender(); + expect(screen.getByText("3 guests will be told it's cancelled.")).toBeTruthy(); + expect(radio(/^Send/).checked).toBe(false); + expect(del.disabled).toBe(true); }); it("asks which occurrences of a repeating one, and deletes those with their token", async () => { @@ -118,15 +271,45 @@ describe("DeleteEventDialog", () => { fireEvent.click(radio(/^This and following events/)); expect(screen.getByText(/can't restore these from its Trash/)).toBeTruthy(); fireEvent.click(del); - expect(onConfirm).toHaveBeenCalledWith(occurrence, { - scope: "following", - token: "f", - asAll: false, - }); + expect(onConfirm).toHaveBeenCalledWith( + occurrence, + { scope: "following", token: "f", asAll: false }, + null, + null, + ); // The held Undo still applies. expect(screen.getByText(/You can undo it for a few seconds/)).toBeTruthy(); }); + // C12 §14.3: a row that would tell guests asks there, and the answer goes with the token. + it("passes the email answer of a repeating one's row", async () => { + const notice: GuestNotice = { + updated: 1, + invited: 0, + removed: 0, + told_cancelled: 0, + cancelled: true, + people: 1, + emails: 1, + complete: true, + }; + choicesForDelete.mockResolvedValue( + ready({ all: { row: "offered", warnings: [], token: "a", guests: notice } }), + ); + const { onConfirm } = open(occurrence, { from: "me@x.com" }); + fireEvent.click(await screen.findByRole("radio", { name: /^All events/ })); + const del = screen.getByRole("button", { name: "Delete" }) as HTMLButtonElement; + expect(del.disabled).toBe(true); + fireEvent.click(radio(/^Send/)); + fireEvent.click(del); + expect(onConfirm).toHaveBeenCalledWith( + occurrence, + { scope: "all", token: "a", asAll: false }, + "all", + null, + ); + }); + it("says Google changed it, offering no rows", async () => { choicesForDelete.mockResolvedValue({ outcome: "refused", diff --git a/src/components/calendar/edit/DeleteEventDialog.tsx b/src/components/calendar/edit/DeleteEventDialog.tsx index e68eb8a5..f2cda028 100644 --- a/src/components/calendar/edit/DeleteEventDialog.tsx +++ b/src/components/calendar/edit/DeleteEventDialog.tsx @@ -8,14 +8,27 @@ // A repeating event asks which occurrences first, like Google Calendar; the held Undo still // applies. The choices are read from Google when the dialog opens, against what the user saw of the // row, and the delete sends the same, since the token binds it. +// +// An event with guests you organise also asks whether Google emails them the cancellation (C12), +// in this dialog: Delete stays off until Send or Don't send is picked, and the answer goes with the +// hold, so it's the one sent after the Undo window. A one-off's numbers come from its row, or from +// the backend when it asked again; a repeating event's rows carry their own. -import { useEffect, useRef, useState } from "react"; -import type { CalendarEvent, SeriesChoices, SplitAction } from "../../../lib/types"; +import { useEffect, useMemo, useRef, useState } from "react"; +import type { + CalendarEvent, + GuestNotice, + Notify, + SeriesChoices, + SplitAction, +} from "../../../lib/types"; import { deleteText } from "../../../lib/calendarEdit/editReasons"; +import { deleteNotice, notifyOf } from "../../../lib/calendarEdit/guests"; import { isRepeating, type Picked } from "../../../lib/calendarEdit/saveChoices"; import { parseLocal } from "../../../lib/calendar-layout"; import { formatDateLocal } from "../../../lib/format"; -import { ConfirmDialog } from "../../ui"; +import { Callout, ConfirmDialog } from "../../ui"; +import { NotifyChoice } from "./NotifyChoice"; import { SaveChoicesDialog, type ChoicesView } from "./SaveChoicesDialog"; import { choicesForDelete, noteSeriesNotice, settleSplit } from "./useEventWrites"; @@ -25,12 +38,25 @@ interface Props { calendarName: string | null; /** When it happens, as the popover said it. */ when: string; + /** The account the delete goes through: who Google sends guests' emails from, and never counted + * as a guest. */ + from?: string | null; /** Re-asked after a delete answered that the plan changed since the choice: the dialog says so. */ again?: boolean; /** Re-asked because the backend wants the occurrences chosen, though the row looked like a * one-off: ask, whatever the row says. */ forceRepeating?: boolean; - onConfirm: (event: CalendarEvent, picked: Picked | null) => void; + /** Re-asked because the backend wants Send or Don't send: its numbers, and `again` when an + * answer came for others. Asked as a one-off. */ + guests?: { notice: GuestNotice; again: boolean } | null; + /** The delete: the occurrences picked (a repeating event), Send or Don't send when it would email + * guests, and the numbers a one-off's question showed (null when nothing was asked). */ + onConfirm: ( + event: CalendarEvent, + picked: Picked | null, + notify: Notify | null, + asked: GuestNotice | null, + ) => void; onClose: () => void; } @@ -38,18 +64,21 @@ export function DeleteEventDialog({ event, calendarName, when, + from = null, again = false, forceRepeating = false, + guests = null, onConfirm, onClose, }: Props) { - if (event && (forceRepeating || isRepeating(event))) { + if (event && !guests && (forceRepeating || isRepeating(event))) { return ( ); } -/** The one-event confirm. */ +/** The one-event confirm, with the email question when it has guests to tell. */ function PlainDelete({ event, calendarName, when, + from, + guests, onConfirm, onClose, -}: Pick) { +}: Pick & { + from: string | null; + guests: { notice: GuestNotice; again: boolean } | null; +}) { const title = event ? event.summary.trim() || "(no title)" : ""; + // The backend's numbers when it asked again, else the row's own (null: nobody to tell). + const notice = useMemo( + () => guests?.notice ?? (event ? deleteNotice(event, from) : null), + [guests, event, from], + ); + // Send or Don't send, for the numbers on screen: none until picked, and none again for others. + const [notify, setNotify] = useState(null); + const [answeredFor, setAnsweredFor] = useState(notice); + if (notice !== answeredFor) { + setAnsweredFor(notice); + setNotify(null); + } return ( event && onConfirm(event, null)} + confirmDisabled={notice !== null && notify === null} + initialFocus="cancel" + // Delete is off until answered; were it pressed anyway, the backend asks (R9). Closing + // clears the event, so the next one asks afresh. + onConfirm={() => + event && onConfirm(event, null, notice && notify ? notifyOf(notify) : null, notice) + } onClose={onClose} >

@@ -93,6 +147,30 @@ function PlainDelete({

You can undo it for a few seconds. If you quit PM in that time, nothing is deleted.

+ {/* Reopened by the backend: the delete didn't happen, which is said first, whether or not + anyone had answered (C12c review). Neutral, as the editor's question is: the backend knows + only that its numbers differ, not why (C12 critique C2-L6). With no answer yet it may be + asking from PM's older row, after Google dropped the guests, so it doesn't say the event + "has guests now"; the send checks Google's copy either way (C12c review round 2). */} + {guests && ( + + {guests.again + ? "It wasn't deleted. PM checked with Google before deleting, and these are the numbers it found." + : "It wasn't deleted. PM found guests who would be told. Choose whether Google emails them."} + + )} + {notice && ( + + )} ); } @@ -102,6 +180,7 @@ function RepeatingDelete({ event, calendarName, when, + from, again, onConfirm, onClose, @@ -109,8 +188,9 @@ function RepeatingDelete({ event: CalendarEvent; calendarName: string | null; when: string; + from: string | null; again: boolean; - onConfirm: (event: CalendarEvent, picked: Picked | null) => void; + onConfirm: Props["onConfirm"]; onClose: () => void; }) { const title = event.summary.trim() || "(no title)"; @@ -198,7 +278,9 @@ function RepeatingDelete({ when={when} calendarName={calendarName} day={start ? formatDateLocal(start) : undefined} - onChoose={(picked) => onConfirm(event, picked)} + from={from} + // The token binds who each row tells, so no numbers go with the answer. + onChoose={(picked, notify) => onConfirm(event, picked, notify, null)} onSettle={(token, action) => void onSettle(token, action)} onClose={close} /> diff --git a/src/components/calendar/edit/EventEditor.test.tsx b/src/components/calendar/edit/EventEditor.test.tsx index 941dba18..156b72a4 100644 --- a/src/components/calendar/edit/EventEditor.test.tsx +++ b/src/components/calendar/edit/EventEditor.test.tsx @@ -16,7 +16,10 @@ import type { EventForEdit, EventPatchDraft, FieldPermissions, + GuestForEdit, + GuestNotice, Milestone, + Notify, RepeatPreset, RepeatRule, RepeatStart, @@ -28,6 +31,7 @@ import type { WriteOutcome, } from "../../../lib/types"; import type { Picked } from "../../../lib/calendarEdit/saveChoices"; +import type { GuestSuggestion } from "../../../lib/calendarEdit/guests"; type SaveAnswer = WriteOutcome & { newPartial?: boolean }; const openForEdit = vi.fn<(id: string) => Promise>(); @@ -38,6 +42,8 @@ const saveEdit = changes: EventPatchDraft, picked: Picked | null, title: string, + notify: Notify | null, + asked: GuestNotice | null, ) => Promise >(); const choicesForEdit = vi.fn<(session: string, draft: EventPatchDraft) => Promise>(); @@ -51,8 +57,16 @@ let deviceZone: string | null = "Europe/London"; vi.mock("./useEventWrites", () => ({ openForEdit: (id: string) => openForEdit(id), - // Every argument passes through, so a scope sent without its token couldn't pass for free. - saveEdit: (s: string, c: EventPatchDraft, p: Picked | null, t: string) => saveEdit(s, c, p, t), + // Every argument passes through, so a scope sent without its token, or an email answer sent + // without being asked, couldn't pass for free. + saveEdit: ( + s: string, + c: EventPatchDraft, + p: Picked | null, + t: string, + n: Notify | null, + a: GuestNotice | null, + ) => saveEdit(s, c, p, t, n, a), choicesForEdit: (s: string, d: EventPatchDraft) => choicesForEdit(s, d), settleSplit: (token: string, action: SplitAction, title: string) => settleSplit(token, action, title), @@ -164,6 +178,7 @@ async function open( row={clicked} calendar={null} account="me@x.com" + suggestions={NO_SUGGESTIONS} milestone={null} onClose={onClose} onDelete={onDelete} @@ -173,8 +188,12 @@ async function open( return onClose; } -/** A one-off's save: no scope, and the draft's title for its notices. */ -const UNSCOPED = [null, "Dentist"] as const; +/** No one to suggest as a guest. */ +const NO_SUGGESTIONS: readonly GuestSuggestion[] = []; + +/** A one-off's save: no scope, the draft's title for its notices, and nobody to email (no answer, + * no numbers). */ +const UNSCOPED = [null, "Dentist", null, null] as const; /** A rule from the repeat menu's set: `pattern`, every week or day, never ending. */ const ruleOf = (pattern: RepeatRule["pattern"], interval = 1): RepeatRule => ({ @@ -260,6 +279,8 @@ describe("EventEditor", () => { { summary: "Dentist (moved)" }, null, "Dentist (moved)", + null, + null, ); expect(noteSaved).toHaveBeenCalledWith( "Dentist (moved)", @@ -344,6 +365,8 @@ describe("EventEditor", () => { { location: "Room 2" }, null, "Dentist (Google)", + null, + null, ); }); @@ -798,7 +821,14 @@ describe("EventEditor", () => { it("says why it couldn't open", async () => { openForEdit.mockResolvedValue({ outcome: "reauth" }); render( - {}} />, + {}} + />, ); expect(await screen.findByText(/Google needs you to sign in again/)).toBeTruthy(); }); @@ -819,6 +849,8 @@ describe("EventEditor, a repeating event", () => { ...over, }) as SeriesChoices; const NOTICE = "PM closed before it finished an earlier change to this repeating event."; + /** A one-event save of the title "Gym": no scope, and no email answer yet. */ + const UNSCOPED_GYM = [null, "Gym", null, null] as const; /** Open the occurrence, retitle it, press Save: the choices dialog, and the editor's Save. */ async function retitle(title = "Gym") { @@ -850,6 +882,8 @@ describe("EventEditor, a repeating event", () => { { summary: "Gym" }, { scope: "all", token: "t-all", asAll: false }, "Gym", + null, + null, ); // The very object the choices were asked for: the token binds it. expect(saveEdit.mock.calls[0][1]).toBe(choicesForEdit.mock.calls[0][1]); @@ -1109,12 +1143,12 @@ describe("EventEditor, a repeating event", () => { openForEdit.mockResolvedValue(ready(event())); choicesForEdit.mockResolvedValue({ outcome: "refused", - result: { outcome: "read_only", reason: "has_guests" }, + result: { outcome: "read_only", reason: "special_type" }, }); await open(vi.fn(), undefined, occurrence); fireEvent.change(screen.getByLabelText("Title"), { target: { value: "Gym" } }); fireEvent.click(screen.getByRole("button", { name: "Save" })); - expect(await screen.findByText(/This event has guests/)).toBeTruthy(); + expect(await screen.findByText(/can't be changed here/)).toBeTruthy(); expect(choicesOpen()).toBeNull(); expect(saveEdit).not.toHaveBeenCalled(); }); @@ -1129,7 +1163,7 @@ describe("EventEditor, a repeating event", () => { fireEvent.click(screen.getByRole("button", { name: "Save" })); await waitFor(() => expect(onClose).toHaveBeenCalled()); expect(saveEdit).toHaveBeenCalledTimes(1); - expect(saveEdit).toHaveBeenCalledWith("s1", { summary: "Gym" }, null, "Gym"); + expect(saveEdit).toHaveBeenCalledWith("s1", { summary: "Gym" }, null, "Gym", null, null); expect(saveEdit.mock.calls[0][1]).toBe(choicesForEdit.mock.calls[0][1]); }); @@ -1159,14 +1193,16 @@ describe("EventEditor, a repeating event", () => { await waitFor(() => expect(document.activeElement).toBe(editorSave)); }); - // C12 §14.1: whether Google emails the guests is no choice of occurrences. Asking those would - // say "doesn't repeat" and send it again unanswered, round and round; it's said instead. - it("says a save needs the guests' answer, and never asks which occurrences", async () => { + // C12 §14.1, §14.2: whether Google emails the guests is no choice of occurrences. Asking those + // would say "doesn't repeat" and send it again unanswered, round and round; the email question + // asks it instead, with the backend's numbers (no "again": nobody had answered). + it("asks the backend's guests question itself, and never which occurrences", async () => { openForEdit.mockResolvedValue(ready(event())); const guests = { updated: 0, invited: 1, removed: 0, + told_cancelled: 0, cancelled: false, people: 1, emails: 1, @@ -1177,15 +1213,50 @@ describe("EventEditor, a repeating event", () => { const editorSave = screen.getByRole("button", { name: "Save" }); fireEvent.change(screen.getByLabelText("Title"), { target: { value: "Gym" } }); fireEvent.click(editorSave); - expect( - await screen.findByText("Choose whether Google emails the guests. Nothing was saved."), - ).toBeTruthy(); - await new Promise((r) => setTimeout(r, 20)); + const prompt = await screen.findByRole("dialog", { name: "Send invitation emails to guests?" }); + expect(within(prompt).getByText("1 guest new to it will get an invitation.")).toBeTruthy(); + expect(within(prompt).queryByText(/PM checked with Google/)).toBeNull(); expect(saveEdit).toHaveBeenCalledTimes(1); expect(choicesForEdit).not.toHaveBeenCalled(); expect(choicesOpen()).toBeNull(); expect(onClose).not.toHaveBeenCalled(); - await waitFor(() => expect(document.activeElement).toBe(editorSave)); + saveEdit.mockResolvedValueOnce({ outcome: "saved", warnings: [] }); + fireEvent.click(within(prompt).getByRole("radio", { name: /^Send/ })); + fireEvent.click(within(prompt).getByRole("button", { name: "Save" })); + await waitFor(() => expect(onClose).toHaveBeenCalled()); + expect(saveEdit).toHaveBeenLastCalledWith("s1", { summary: "Gym" }, null, "Gym", "all", guests); + }); + + // C12c review: the row still says it repeats, Google's copy no longer does, and it has guests. The + // one-event save that follows "doesn't repeat" is asked whether Google emails them: that question + // opens with the backend's numbers, rather than becoming a banner no Save could get past. + it("asks the guests question after the choices say it doesn't repeat", async () => { + openForEdit.mockResolvedValue(ready(event())); + choicesForEdit.mockResolvedValue({ outcome: "not_repeating" }); + const guests: GuestNotice = { + updated: 1, + invited: 0, + removed: 0, + told_cancelled: 0, + cancelled: false, + people: 1, + emails: 1, + complete: true, + }; + saveEdit.mockResolvedValueOnce({ outcome: "needs_choice", scope: false, guests }); + const onClose = await open(vi.fn(), undefined, occurrence); + fireEvent.change(screen.getByLabelText("Title"), { target: { value: "Gym" } }); + fireEvent.click(screen.getByRole("button", { name: "Save" })); + const prompt = await screen.findByRole("dialog", { name: "Send update emails to guests?" }); + expect(within(prompt).getByText("1 guest already invited will get an update.")).toBeTruthy(); + expect(screen.queryByText(/Choose whether Google emails the guests/)).toBeNull(); + expect(saveEdit).toHaveBeenCalledWith("s1", { summary: "Gym" }, ...UNSCOPED_GYM); + saveEdit.mockResolvedValueOnce({ outcome: "saved", warnings: [] }); + fireEvent.click(within(prompt).getByRole("radio", { name: /^Send/ })); + fireEvent.click(within(prompt).getByRole("button", { name: "Save" })); + await waitFor(() => expect(onClose).toHaveBeenCalled()); + expect(saveEdit).toHaveBeenCalledTimes(2); + expect(saveEdit).toHaveBeenLastCalledWith("s1", { summary: "Gym" }, null, "Gym", "all", guests); }); it("says why the choices couldn't be read, with focus back on Save", async () => { @@ -1401,6 +1472,8 @@ describe("EventEditor, how it repeats", () => { { recurrence: { kind: "once" } }, { scope: "all", token: "t-all", asAll: false }, "Dentist", + null, + null, ); // Its notice says one event is left (C11d review 1). expect(noteSaved).toHaveBeenCalledWith( @@ -1759,6 +1832,7 @@ describe("EventEditor, a linked milestone", () => { row={clicked} calendar={null} account="me@x.com" + suggestions={NO_SUGGESTIONS} milestone={launch} onClose={() => {}} /> @@ -1781,3 +1855,342 @@ describe("EventEditor, a linked milestone", () => { ).toBeTruthy(); }); }); + +// Guests (#884, C12c): the list in the editor, and Send / Don't send with no default. A one-off asks +// from the editor's own count before anything is sent; the backend's numbers ask again; Busy/free +// alone never asks; a repeating event's rows ask in the choices dialog. +describe("EventEditor, guests", () => { + const me: GuestForEdit = { + email: "me@x.com", + name: null, + response: "accepted", + optional: false, + organizer: true, + self: true, + room: false, + }; + const ann: GuestForEdit = { + email: "ann@x.org", + name: "Ann", + response: "accepted", + optional: false, + organizer: false, + self: false, + room: false, + }; + const withAnn = () => event({ guests: [me, ann] }); + /** Ann, told of a change. */ + const ONE: GuestNotice = { + updated: 1, + invited: 0, + removed: 0, + told_cancelled: 0, + cancelled: false, + people: 1, + emails: 1, + complete: true, + }; + const question = () => screen.queryByRole("dialog", { name: "Send update emails to guests?" }); + + /** Open the one-off with Ann, retitle it, press Save. */ + async function retitle() { + openForEdit.mockResolvedValue(ready(withAnn())); + const onClose = await open(); + const editorSave = screen.getByRole("button", { name: "Save" }); + fireEvent.change(screen.getByLabelText("Title"), { target: { value: "Gym" } }); + fireEvent.click(editorSave); + const prompt = await screen.findByRole("dialog", { name: "Send update emails to guests?" }); + return { onClose, editorSave, prompt }; + } + + it("a one-off with guests asks before saving, and Back returns to Save", async () => { + const { onClose, editorSave, prompt } = await retitle(); + const p = within(prompt); + expect(p.getByText("1 guest already invited will get an update.")).toBeTruthy(); + expect(p.getByText("Google sends the emails from me@x.com.")).toBeTruthy(); + // No default: Save waits for a pick, and focus starts on Back. + expect(p.getByRole("button", { name: "Save" }).disabled).toBe(true); + expect(p.getByRole("radio", { name: /^Send/ }).checked).toBe(false); + expect(document.activeElement).toBe(p.getByRole("button", { name: "Back" })); + expect(saveEdit).not.toHaveBeenCalled(); + fireEvent.click(p.getByRole("button", { name: "Back" })); + expect(question()).toBeNull(); + await waitFor(() => expect(document.activeElement).toBe(editorSave)); + expect(screen.getByLabelText("Title").value).toBe("Gym"); + expect(saveEdit).not.toHaveBeenCalled(); + expect(onClose).not.toHaveBeenCalled(); + }); + + it("Send saves with all, Don't send with none, each for the numbers it showed", async () => { + saveEdit.mockResolvedValue({ outcome: "saved", warnings: [] }); + for (const [label, notify] of [ + [/^Send/, "all"], + [/^Don't send/, "none"], + ] as const) { + const { onClose, prompt } = await retitle(); + fireEvent.click(within(prompt).getByRole("radio", { name: label })); + fireEvent.click(within(prompt).getByRole("button", { name: "Save" })); + await waitFor(() => expect(onClose).toHaveBeenCalled()); + expect(saveEdit).toHaveBeenLastCalledWith("s1", { summary: "Gym" }, null, "Gym", notify, ONE); + cleanup(); + } + }); + + // Nothing goes from behind the question (C12 critique C2-L3): the editor's own Save, Cancel and + // Delete do nothing while it's open, whatever reaches them. + it("holds the editor's own buttons while the email question is open", async () => { + openForEdit.mockResolvedValue(ready(withAnn())); + const onDelete = vi.fn(); + const onClose = await open(vi.fn(), onDelete); + const [editorSave, cancel, del] = ["Save", "Cancel", "Delete"].map((name) => + screen.getByRole("button", { name }), + ); + fireEvent.change(screen.getByLabelText("Title"), { target: { value: "Gym" } }); + fireEvent.click(editorSave); + await screen.findByRole("dialog", { name: "Send update emails to guests?" }); + for (const button of [editorSave, cancel, del]) fireEvent.click(button); + expect(screen.getAllByRole("dialog", { name: "Send update emails to guests?" })).toHaveLength( + 1, + ); + expect(screen.queryByRole("dialog", { name: /^Discard your changes/ })).toBeNull(); + expect(onClose).not.toHaveBeenCalled(); + expect(onDelete).not.toHaveBeenCalled(); + expect(saveEdit).not.toHaveBeenCalled(); + }); + + it("Escape closes only the email question", async () => { + const { onClose, prompt } = await retitle(); + fireEvent.keyDown(prompt, { key: "Escape" }); + expect(question()).toBeNull(); + expect(screen.queryByRole("dialog", { name: "Discard your changes?" })).toBeNull(); + expect(screen.getByRole("dialog", { name: "Edit event" })).toBeTruthy(); + expect(onClose).not.toHaveBeenCalled(); + expect(saveEdit).not.toHaveBeenCalled(); + }); + + it("a Busy/free change with guests saves without asking", async () => { + openForEdit.mockResolvedValue(ready(withAnn())); + saveEdit.mockResolvedValue({ outcome: "saved", warnings: [] }); + const onClose = await open(); + fireEvent.click(screen.getByRole("button", { name: "Free" })); + fireEvent.click(screen.getByRole("button", { name: "Save" })); + await waitFor(() => expect(onClose).toHaveBeenCalled()); + expect(question()).toBeNull(); + expect(saveEdit).toHaveBeenCalledWith("s1", { show_as: "free" }, ...UNSCOPED); + }); + + it("the backend's guests question opens the prompt with its numbers and the again line", async () => { + const two: GuestNotice = { ...ONE, updated: 2, people: 2, emails: 2 }; + saveEdit.mockResolvedValueOnce({ + outcome: "needs_choice", + scope: false, + guests: two, + again: true, + }); + const { onClose, editorSave, prompt } = await retitle(); + fireEvent.click(within(prompt).getByRole("radio", { name: /^Send/ })); + fireEvent.click(within(prompt).getByRole("button", { name: "Save" })); + const again = await screen.findByText("2 guests already invited will get an update."); + const reasked = within(again.closest('[role="dialog"]') as HTMLElement); + expect( + reasked.getByText( + "PM checked with Google before saving, and these are the numbers it found.", + ), + ).toBeTruthy(); + // Asked afresh: nothing picked. + expect(reasked.getByRole("button", { name: "Save" }).disabled).toBe(true); + // A Save from behind it never swaps the backend's numbers for the editor's own count. + fireEvent.click(editorSave); + expect(screen.getByText("2 guests already invited will get an update.")).toBeTruthy(); + expect(screen.queryByText("1 guest already invited will get an update.")).toBeNull(); + saveEdit.mockResolvedValueOnce({ outcome: "saved", warnings: [] }); + fireEvent.click(reasked.getByRole("radio", { name: /^Don't send/ })); + fireEvent.click(reasked.getByRole("button", { name: "Save" })); + await waitFor(() => expect(onClose).toHaveBeenCalled()); + expect(saveEdit).toHaveBeenLastCalledWith("s1", { summary: "Gym" }, null, "Gym", "none", two); + }); + + it("a second one in one Save is a banner", async () => { + const two: GuestNotice = { ...ONE, updated: 2, people: 2, emails: 2 }; + saveEdit.mockResolvedValue({ outcome: "needs_choice", scope: false, guests: two, again: true }); + const { onClose, editorSave, prompt } = await retitle(); + fireEvent.click(within(prompt).getByRole("radio", { name: /^Send/ })); + fireEvent.click(within(prompt).getByRole("button", { name: "Save" })); + const again = await screen.findByText("2 guests already invited will get an update."); + const reasked = within(again.closest('[role="dialog"]') as HTMLElement); + fireEvent.click(reasked.getByRole("radio", { name: /^Send/ })); + fireEvent.click(reasked.getByRole("button", { name: "Save" })); + expect( + await screen.findByText("Choose whether Google emails the guests. Nothing was saved."), + ).toBeTruthy(); + expect(screen.queryByText("2 guests already invited will get an update.")).toBeNull(); + expect(saveEdit).toHaveBeenCalledTimes(2); + expect(onClose).not.toHaveBeenCalled(); + await waitFor(() => expect(document.activeElement).toBe(editorSave)); + }); + + it("a repeating save passes the dialog's answer", async () => { + const occurrence: CalendarEvent = { ...row, series_id: "abc", recurring: true }; + openForEdit.mockResolvedValue(ready(withAnn())); + choicesForEdit.mockResolvedValue({ + outcome: "ready", + this: { row: "offered", warnings: [], token: "t-this", guests: ONE }, + following: { row: "offered", warnings: [], token: "t-following", guests: ONE }, + all: { row: "offered", warnings: [], token: "t-all", guests: ONE }, + }); + saveEdit.mockResolvedValue({ outcome: "saved", warnings: [] }); + const onClose = await open(vi.fn(), undefined, occurrence); + fireEvent.change(screen.getByLabelText("Title"), { target: { value: "Gym" } }); + fireEvent.click(screen.getByRole("button", { name: "Save" })); + const d = within(await screen.findByRole("dialog", { name: "Edit repeating event" })); + await d.findByRole("radio", { name: /^This event/ }); + // Asked inside the dialog, never in a second one. + expect(question()).toBeNull(); + fireEvent.click(d.getByRole("radio", { name: /^Send/ })); + fireEvent.click(d.getByRole("button", { name: "Save" })); + await waitFor(() => expect(onClose).toHaveBeenCalled()); + expect(saveEdit).toHaveBeenCalledWith( + "s1", + { summary: "Gym" }, + { scope: "this", token: "t-this", asAll: false }, + "Gym", + "all", + null, + ); + }); + + it("the guest field is disabled with the reason when guests can't change", async () => { + const locked: FieldPermissions = { + ...ALL, + summary: false, + time: false, + location: false, + description: false, + guests: false, + reasons: ["locked"], + }; + openForEdit.mockResolvedValue(ready(withAnn(), locked)); + await open(); + expect(screen.getByRole("combobox", { name: "Add guests" }).disabled).toBe( + true, + ); + expect(screen.getByText(/Google has locked this event's/)).toBeTruthy(); + // The list still shows. + expect(screen.getByText("Ann")).toBeTruthy(); + }); + + // A guest added is invited, and the guest already there hears of the change, as Google counts it. + it("adding a guest asks with an invitation, and sends the change to Google's list", async () => { + openForEdit.mockResolvedValue(ready(withAnn())); + saveEdit.mockResolvedValue({ outcome: "saved", warnings: [] }); + const onClose = await open(); + const box = screen.getByRole("combobox", { name: "Add guests" }); + fireEvent.change(box, { target: { value: "bo@x.org" } }); + fireEvent.keyDown(box, { key: "Enter" }); + fireEvent.click(screen.getByRole("button", { name: "Save" })); + const prompt = await screen.findByRole("dialog", { name: "Send update emails to guests?" }); + expect(within(prompt).getByText("1 guest new to it will get an invitation.")).toBeTruthy(); + expect(within(prompt).getByText("1 guest already invited will get an update.")).toBeTruthy(); + fireEvent.click(within(prompt).getByRole("radio", { name: /^Send/ })); + fireEvent.click(within(prompt).getByRole("button", { name: "Save" })); + await waitFor(() => expect(onClose).toHaveBeenCalled()); + expect(saveEdit).toHaveBeenCalledWith( + "s1", + { guests: [{ email: "bo@x.org", to: "required" }] }, + null, + "Dentist", + "all", + { ...ONE, invited: 1, people: 2, emails: 2 }, + ); + }); + + it("a rejected address stops the save", async () => { + openForEdit.mockResolvedValue(ready(withAnn())); + await open(); + const box = screen.getByRole("combobox", { name: "Add guests" }); + fireEvent.change(box, { target: { value: "bob at x" } }); + fireEvent.blur(box); + fireEvent.change(screen.getByLabelText("Title"), { target: { value: "Gym" } }); + fireEvent.click(screen.getByRole("button", { name: "Save" })); + await new Promise((r) => setTimeout(r, 20)); + expect(screen.getAllByText("“bob at x” isn't an email address.").length).toBeGreaterThan(0); + expect(question()).toBeNull(); + expect(saveEdit).not.toHaveBeenCalled(); + }); + + // C12c review: like a date it couldn't read, an address it couldn't read never reached the draft, + // so closing would lose it without a word: the editor asks first. + it("asks before closing when only an address it couldn't read stands", async () => { + openForEdit.mockResolvedValue(ready(withAnn())); + const onClose = await open(); + const box = screen.getByRole("combobox", { name: "Add guests" }); + box.focus(); + fireEvent.change(box, { target: { value: "bob at x" } }); + fireEvent.click(screen.getByRole("button", { name: "Cancel" })); + expect(await screen.findByText("Discard your changes?")).toBeTruthy(); + expect(onClose).not.toHaveBeenCalled(); + fireEvent.click(screen.getByRole("button", { name: "Discard" })); + expect(onClose).toHaveBeenCalled(); + expect(saveEdit).not.toHaveBeenCalled(); + }); + + // The field takes Google's list from the editor: a guest taken off and added back is Google's + // entry again, reply and all, so there's nothing to save and nobody to ask about. + it("a guest taken off and added back is Google's entry, so nothing is sent", async () => { + openForEdit.mockResolvedValue(ready(withAnn())); + const onClose = await open(); + fireEvent.click(screen.getByRole("button", { name: "Remove Ann" })); + expect(screen.queryByText("Ann")).toBeNull(); + const box = screen.getByRole("combobox", { name: "Add guests" }); + fireEvent.change(box, { target: { value: "ann@x.org" } }); + fireEvent.keyDown(box, { key: "Enter" }); + expect(screen.getByText("Ann")).toBeTruthy(); + fireEvent.click(screen.getByRole("button", { name: "Save" })); + await waitFor(() => expect(onClose).toHaveBeenCalled()); + expect(question()).toBeNull(); + expect(saveEdit).not.toHaveBeenCalled(); + }); + + // And the organiser's address from Google's copy, on the list or not: Google refuses any change + // to it, so the field never adds it. + it("never adds the organiser, who isn't on Google's list", async () => { + openForEdit.mockResolvedValue(ready(event({ guests: [ann], organizer_email: "boss@x.org" }))); + const onClose = await open(); + const box = screen.getByRole("combobox", { name: "Add guests" }); + fireEvent.change(box, { target: { value: "Boss@x.org" } }); + fireEvent.keyDown(box, { key: "Enter" }); + expect(screen.getByText("That's the organiser, who is on it already.")).toBeTruthy(); + expect(screen.queryByRole("button", { name: /^Remove Boss/ })).toBeNull(); + fireEvent.click(screen.getByRole("button", { name: "Save" })); + await waitFor(() => expect(onClose).toHaveBeenCalled()); + expect(saveEdit).not.toHaveBeenCalled(); + }); + + // C12 critique C1-H2: a delete from the editor asks about the guests Google's copy has, not the + // mirror's (possibly older) list. + it("deletes with the guest list Google's copy has", async () => { + openForEdit.mockResolvedValue(ready(withAnn())); + const onDelete = vi.fn(); + await open(vi.fn(), onDelete); + fireEvent.click(screen.getByRole("button", { name: "Delete" })); + expect(onDelete).toHaveBeenCalledTimes(1); + expect(onDelete.mock.calls[0][0].attendees).toEqual([ + { + name: null, + email: "me@x.com", + response: "accepted", + optional: false, + organizer: true, + self: true, + }, + { + name: "Ann", + email: "ann@x.org", + response: "accepted", + optional: false, + organizer: false, + self: false, + }, + ]); + }); +}); diff --git a/src/components/calendar/edit/EventEditor.tsx b/src/components/calendar/edit/EventEditor.tsx index 67bee862..3e8aa44a 100644 --- a/src/components/calendar/edit/EventEditor.tsx +++ b/src/components/calendar/edit/EventEditor.tsx @@ -1,13 +1,21 @@ // SPDX-FileCopyrightText: 2026 Bobby Yu // SPDX-License-Identifier: AGPL-3.0-or-later -// The event editor (#884): change one Google event you organise without guests, a repeating one -// included: for an occurrence, the choices dialog asks which occurrences first. It opens on Google's -// fresh copy (never the mirror, which clips text), greys out whatever the backend says can't change -// and says why, and saves only the fields the user changed. If the event changed in Google -// meanwhile on a field this save also sends, nothing is overwritten: the editor reloads Google's -// copy, keeps the user's other changes, and names the fields where Google's version now shows. A -// dirty draft asks before it is thrown away (closing, Escape, or Delete). +// The event editor (#884): change one Google event you organise, a repeating one or one with guests +// included: for an occurrence the choices dialog asks which occurrences first, and a change Google +// would email guests about asks whether it should. It opens on Google's fresh copy (never the +// mirror, which clips text), greys out whatever the backend says can't change and says why, and +// saves only the fields the user changed. If the event changed in Google meanwhile on a field this +// save also sends, nothing is overwritten: the editor reloads Google's copy, keeps the user's other +// changes, and names the fields where Google's version now shows. A dirty draft asks before it is +// thrown away (closing, Escape, or Delete). +// +// Guests (C12): the list, Optional, take off, and the three guest permissions, as Google Calendar +// has them, with suggestions from people already in the user's calendars. Send or Don't send has no +// default: a one-off's question opens from the editor's own count before anything is sent, and +// again with the backend's numbers if Google's copy reaches someone else; a repeating event's rows +// ask it inside the choices dialog. A change to only Busy/free emails nobody, so it never asks. +// Like the choices dialog, the question sits inside the editor's dialog, so Escape closes only it. // // The choices dialog sits inside the editor's, so it alone hears Escape and holds Tab. Its answer // binds this session and these very changes, so the save sends the object the choices were asked @@ -39,7 +47,9 @@ import type { EventForEdit, EventPatchDraft, FieldPermissions, + GuestNotice, Milestone, + Notify, SeenSummary, SeriesChoices, SeriesInfo, @@ -59,6 +69,7 @@ import { endsAfterStart, firstEndOnItsDate, followStart, + guestNotice, halfInstant, hasBadDate, rebaseDraft, @@ -90,9 +101,12 @@ import { } from "../../../lib/calendarEdit/editReasons"; import { conflictOf, isRepeating, type Picked } from "../../../lib/calendarEdit/saveChoices"; import { descriptionText } from "../../../lib/calendarEdit/descriptionText"; +import type { GuestSuggestion } from "../../../lib/calendarEdit/guests"; import { deviceTimeZoneOrNull, useDepth } from "../../../theme"; import { Button, Callout, ConfirmDialog, Dialog, Skeleton } from "../../ui"; import { CustomRecurrenceDialog } from "./CustomRecurrenceDialog"; +import { GuestField } from "./GuestField"; +import { NotifyPrompt } from "./NotifyPrompt"; import { NO_ZONE, RepeatRow, @@ -118,8 +132,12 @@ interface Props { /** The row that was clicked: its title while Google's copy loads, and its id. */ row: CalendarEvent; calendar: Calendar | null; - /** The account the save goes through. */ + /** The account the save goes through: who Google sends guests' emails from, and never suggested + * as a guest. */ account: string | null; + /** People already in the user's calendars, for the guest box (worked out by the calendar). + * Required, so a caller can't leave the box without them unnoticed. */ + suggestions: readonly GuestSuggestion[]; /** A milestone linked to this event, which moves with it. */ milestone: Milestone | null; /** Delete from the editor: it closes, and the calendar asks as the popover's Delete does. The row @@ -138,6 +156,15 @@ interface Loaded { series: SeriesInfo | null; } +/** The email question, open: the save it is for (the very changes the save sends, and the + * occurrences picked, if any), the numbers, and `again` when the backend asked after an answer. */ +interface Prompt { + what: Omit; + picked: Picked | null; + notice: GuestNotice; + again: boolean; +} + /** The choices dialog, open: what it was asked for, and what it shows. */ interface Asking { /** The session and the very changes the save sends: the token binds both. */ @@ -180,7 +207,15 @@ const firstDay = (t: TimeDraft) => (t.kind === "timed" ? t.start_date : t.first_ /** Said under the Repeat row of an occurrence whose repeating event PM couldn't read. */ export const UNREADABLE_REPEAT = "PM couldn't read how this repeats, so it can't change it here."; -export function EventEditor({ row, calendar, account, milestone, onDelete, onClose }: Props) { +export function EventEditor({ + row, + calendar, + account, + suggestions, + milestone, + onDelete, + onClose, +}: Props) { const { showPower } = useDepth(); const deviceZone = useMemo(() => deviceTimeZoneOrNull(), []); // The row is the safer sign of a repeating event: the series is null when Google's copy of it @@ -219,7 +254,24 @@ export function EventEditor({ row, calendar, account, milestone, onDelete, onClo const heldNotice = useRef(null); // Times this Save was asked again because the plan changed: once, then it says so. const againRef = useRef(0); + // Times the backend asked this Save whether Google emails the guests: once, then it says so. Its + // own count, so a "doesn't repeat" before it doesn't use up that one ask (C12c review). + const guestAskRef = useRef(0); const [settlingSplit, setSettlingSplit] = useState(false); + // The email question (null = closed), readable at once: no save or close goes from behind it. + const [prompt, setPromptState] = useState(null); + const promptRef = useRef(null); + const setPrompt = (next: Prompt | null) => { + promptRef.current = next; + setPromptState(next); + }; + // Text left in the guest box that isn't an address: no save until it's cleared or fixed. + const badGuestRef = useRef(null); + const [badGuest, setBadGuestState] = useState(null); + const setBadGuest = (text: string | null) => { + badGuestRef.current = text; + setBadGuestState(text); + }; // The Custom dialog, while open: no save goes from behind it. const customRef = useRef(false); const [customOpen, setCustomOpenState] = useState(false); @@ -304,18 +356,32 @@ export function EventEditor({ row, calendar, account, milestone, onDelete, onClo !!loaded && !!draftRef.current && changedFields(loaded.base, draftRef.current).length > 0; const requestClose = () => { - if (saving) return; + if (saving || promptRef.current) return; commitFocused(); - if (isDirty() || hasBadDate(badDatesRef.current)) setAskDiscard("close"); - else onClose(); + // Text in the guest box that isn't an address never reached the draft: it would be lost too. + if (isDirty() || hasBadDate(badDatesRef.current) || badGuestRef.current !== null) { + setAskDiscard("close"); + } else onClose(); }; - /** The row a delete from here compares: what the editor showed, not a possibly older mirror row. */ - const deleteRow = (): CalendarEvent => - loaded?.seen ? { ...row, ...loaded.seen, location: loaded.seen.location } : row; + /** The row a delete from here compares: what the editor showed, not a possibly older mirror row, + * with the guest list Google's copy has, so the delete asks about those (C12 critique C1-H2). */ + const deleteRow = (): CalendarEvent => { + if (!loaded) return row; + const attendees = loaded.event.guests.map((g) => ({ + name: g.name, + email: g.email, + response: g.response, + optional: g.optional, + organizer: g.organizer, + self: g.self, + })); + const seen = loaded.seen ? { ...loaded.seen, location: loaded.seen.location } : {}; + return { ...row, ...seen, attendees }; + }; const requestDelete = () => { - if (!onDelete || saving) return; + if (!onDelete || saving || promptRef.current) return; commitFocused(); if (isDirty()) setAskDiscard("delete"); else onDelete(deleteRow()); @@ -406,14 +472,16 @@ export function EventEditor({ row, calendar, account, milestone, onDelete, onClo repeating || repeat.saveable(fields.recurrence, repeatStartOf(fields.time)); async function save() { - // Refused before anything is blurred, so focus stays put (in the Custom dialog too). - if (!loaded || saving || asking || customRef.current) return; + // Refused before anything is blurred, so focus stays put (in the Custom dialog, or the email + // question, too). + if (!loaded || saving || asking || customRef.current || promptRef.current) return; const focused = document.activeElement; commitFocused(); const current = draftRef.current; if (!current) return; // A new Save: no earlier ask's count or words carry into it (C9b-1 critique 1 L8). againRef.current = 0; + guestAskRef.current = 0; heldNotice.current = null; // What stops this save is already said under When, as it happened; a banner repeating it would // outlive the fix. A date a field couldn't read puts focus on that field, which the line names @@ -424,10 +492,12 @@ export function EventEditor({ row, calendar, account, milestone, onDelete, onClo refocus(document.getElementById(refusedDateId(whenId, badDatesRef.current))); return; } - // A repeat the backend refused, or hasn't answered for yet, is said under the Repeat row. + // A repeat the backend refused, or hasn't answered for yet, is said under the Repeat row; text in + // the guest box that isn't an address, under the box and in the problem lines. if ( checkDraft(loaded.base, current, loaded.event).problems.length > 0 || - !repeatReady(current) + !repeatReady(current) || + badGuestRef.current !== null ) { setBanner(null); refocus(focused); @@ -446,8 +516,30 @@ export function EventEditor({ row, calendar, account, milestone, onDelete, onClo title: current.summary, from: firstDay(current.time), }; - if (repeating) void ask(what, false); - else await send(what, null); + if (repeating) { + // Each row says who it would email, and asks there. + void ask(what, false); + return; + } + // Who this would email, counted as the backend counts: asked before anything is sent, as + // Google's own question is. Nobody, and it goes (the backend has the last word either way). + const notice = guestNotice(loaded.base, current, loaded.event.guests_omitted, account); + if (notice) setPrompt({ what, picked: null, notice, again: false }); + else await send(what, null, null, null); + } + + /** The email question answered: it closes and the save goes with the answer, for its numbers. */ + function onNotify(n: Notify) { + const p = promptRef.current; + if (!p) return; + setPrompt(null); + void send(p.what, p.picked, n, p.notice); + } + + /** Back (or Escape) from the email question: nothing was sent, and the draft stays. */ + function onNotifyBack() { + setPrompt(null); + refocusSave(); } /** The choices found Google changed the event: rebase, with Save held until it's done (a Save @@ -501,7 +593,7 @@ export function EventEditor({ row, calendar, account, milestone, onDelete, onClo refocusSave(); return; } - await send(what, null); + await send(what, null, null, null); return; case "refused": closeChoices(); @@ -533,14 +625,15 @@ export function EventEditor({ row, calendar, account, milestone, onDelete, onClo } } - /** The user picked which occurrences: the dialog closes and the save goes, with the very changes - * the choices were asked for. */ - function onChoose(picked: Picked) { + /** The user picked which occurrences (and Send or Don't send, when that row would email guests): + * the dialog closes and the save goes, with the very changes the choices were asked for. The + * token binds who it emails, so no numbers go with the answer. */ + function onChoose(picked: Picked, notify: Notify | null) { if (!asking) return; const { session, changes, title, from } = asking; askSeq.current++; setAsking(null); - void send({ session, changes, title, from }, picked); + void send({ session, changes, title, from }, picked, notify, null); } /** Settle the earlier change the dialog's pending row names. Its notice says how it went; the @@ -554,14 +647,20 @@ export function EventEditor({ row, calendar, account, milestone, onDelete, onClo refocusSave(); } - /** Send the save, for `picked` when the choices were asked, and act on what came back. */ - async function send(what: Omit, picked: Picked | null) { + /** Send the save, for `picked` when the choices were asked, with Send or Don't send (`notify`) + * for the numbers `asked` when the save would email guests, and act on what came back. */ + async function send( + what: Omit, + picked: Picked | null, + notify: Notify | null, + asked: GuestNotice | null, + ) { setSaving(true); let stayOpen = true; - // The choices opened again from here: focus is theirs. + // The choices or the email question opened again from here: focus is theirs. let reasked = false; try { - const outcome = await saveEdit(what.session, what.changes, picked, what.title); + const outcome = await saveEdit(what.session, what.changes, picked, what.title, notify, asked); if (outcome.outcome !== "needs_choice") heldNotice.current = null; switch (outcome.outcome) { case "saved": @@ -582,10 +681,17 @@ export function EventEditor({ row, calendar, account, milestone, onDelete, onClo onClose(); return; case "needs_choice": - // Whether Google emails the guests (C12): never answered by choosing occurrences, whose - // dialog would say "doesn't repeat" and send it again unanswered, round and round. + // Whether Google emails the guests (C12): asked with the backend's numbers, never by + // choosing occurrences, whose dialog would say "doesn't repeat" and send it again + // unanswered, round and round. Once per Save: a second ask is said instead. if (outcome.guests && !outcome.scope) { - setBanner(outcomeText(outcome)); + if (++guestAskRef.current > 1) { + setBanner(outcomeText(outcome)); + return; + } + reasked = true; + // "Again" only when an answer was given: the numbers changed since the user chose. + setPrompt({ what, picked, notice: outcome.guests, again: notify !== null }); return; } // No choice sent, and the event repeats after all (R9): ask, saying nothing. @@ -612,7 +718,10 @@ export function EventEditor({ row, calendar, account, milestone, onDelete, onClo onClose(); return; } - setBanner({ tone: "warn", text: earlierChangeText(outcome.offers, outcome.from) }); + setBanner({ + tone: "warn", + text: earlierChangeText(outcome.offers, outcome.from, outcome.emails), + }); return; case "conflict": await rebaseAfterConflict(outcomeText(outcome).text); @@ -816,13 +925,35 @@ export function EventEditor({ row, calendar, account, milestone, onDelete, onClo ))} {/* What else would stop the save (a date a field couldn't read is said under When), shown as it happens rather than only on Save (each sentence once: a start and an end - in the same skipped hour say the same thing). */} - {[...new Set(check?.problems.map(problemText) ?? [])].map((text) => ( + in the same skipped hour say the same thing). Text left in the guest box that isn't + an address is one too, beside the box's own line under it. */} + {[ + ...new Set([ + ...(check?.problems.map(problemText) ?? []), + ...(badGuest !== null ? [problemText({ kind: "bad_guest", email: badGuest })] : []), + ]), + ].map((text) => (

{text}

))} + draftRef.current && setDraft({ ...draftRef.current, guests })} + onPerms={(guest_perms) => + draftRef.current && setDraft({ ...draftRef.current, guest_perms }) + } + onReject={setBadGuest} + /> + setCustomOpen(false)} /> )} + {/* The email question, nested the same way: Escape and Back close only it. */} + {prompt && ( + + )} {asking && ( void onSettle(token, action)} onClose={() => { diff --git a/src/components/calendar/edit/NotifyChoice.test.tsx b/src/components/calendar/edit/NotifyChoice.test.tsx index 903f4a5e..2b8d530d 100644 --- a/src/components/calendar/edit/NotifyChoice.test.tsx +++ b/src/components/calendar/edit/NotifyChoice.test.tsx @@ -23,6 +23,7 @@ const NOTICE: GuestNotice = { updated: 2, invited: 1, removed: 0, + told_cancelled: 0, cancelled: false, people: 3, emails: 3, diff --git a/src/components/calendar/edit/NotifyPrompt.test.tsx b/src/components/calendar/edit/NotifyPrompt.test.tsx index f44766c9..15d858da 100644 --- a/src/components/calendar/edit/NotifyPrompt.test.tsx +++ b/src/components/calendar/edit/NotifyPrompt.test.tsx @@ -25,6 +25,7 @@ const NOTICE: GuestNotice = { updated: 1, invited: 0, removed: 0, + told_cancelled: 0, cancelled: false, people: 1, emails: 1, diff --git a/src/components/calendar/edit/SaveChoicesDialog.test.tsx b/src/components/calendar/edit/SaveChoicesDialog.test.tsx index 3f7737f8..c5558828 100644 --- a/src/components/calendar/edit/SaveChoicesDialog.test.tsx +++ b/src/components/calendar/edit/SaveChoicesDialog.test.tsx @@ -291,6 +291,7 @@ describe("SaveChoicesDialog, a row that would email guests", () => { updated: 2, invited: 0, removed: 0, + told_cancelled: 0, cancelled: false, people: 2, emails: 2, diff --git a/src/components/calendar/edit/SaveChoicesDialog.tsx b/src/components/calendar/edit/SaveChoicesDialog.tsx index 74fcd1bd..7d6c0bc6 100644 --- a/src/components/calendar/edit/SaveChoicesDialog.tsx +++ b/src/components/calendar/edit/SaveChoicesDialog.tsx @@ -208,7 +208,7 @@ export function SaveChoicesDialog({ )} {pending && ( -

{pendingText(pending.offers, pending.from)}

+

{pendingText(pending.offers, pending.from, pending.emails)}

{SPLIT_ORDER.filter((a) => pending.offers.includes(a)).map((action) => (