From d44677cd004a2e02be79acf1b58db707626e2c86 Mon Sep 17 00:00:00 2001 From: "Javier G. Montoya S" Date: Tue, 30 Jun 2026 16:54:12 -0400 Subject: [PATCH] feat(chat-list): distinguish whether the local account left or was removed from each group --- crates/marmot-app/src/client/mod.rs | 41 ++++-- crates/marmot-app/src/client/sync.rs | 88 +++++++++-- crates/marmot-app/src/conversions.rs | 4 + crates/marmot-app/src/groups.rs | 7 +- crates/marmot-app/src/lib.rs | 6 +- crates/marmot-app/src/runtime/tests.rs | 1 + crates/marmot-app/tests/relay_runtime.rs | 15 +- .../src/conversions/chat_list.rs | 6 +- .../marmot-uniffi/src/conversions/common.rs | 22 +++ crates/marmot-uniffi/src/conversions/group.rs | 5 + crates/marmot-uniffi/src/conversions/mod.rs | 20 +++ .../storage-sqlite/src/account_projection.rs | 60 +++++++- .../src/account_projection/tests.rs | 1 + crates/storage-sqlite/src/chat_list.rs | 52 +++++-- crates/storage-sqlite/src/chat_list/tests.rs | 137 ++++++++++++++++-- crates/storage-sqlite/src/lib.rs | 2 +- crates/storage-sqlite/src/migrations.rs | 33 +++++ .../0022_chat_list_self_membership.rs | 24 +++ 18 files changed, 455 insertions(+), 69 deletions(-) create mode 100644 crates/storage-sqlite/src/migrations/0022_chat_list_self_membership.rs diff --git a/crates/marmot-app/src/client/mod.rs b/crates/marmot-app/src/client/mod.rs index 82136eae..bd40eaa6 100644 --- a/crates/marmot-app/src/client/mod.rs +++ b/crates/marmot-app/src/client/mod.rs @@ -40,7 +40,7 @@ use crate::{ AppMessageQuery, AppPerformanceTelemetry, AppQuarantinedGroup, AppRuntime, AppTransportRouting, GroupInviteDeclineResult, MarmotApp, MarmotRelayPlane, MarmotRelayPlaneAccountAdapter, MediaAttachmentReference, MediaDownloadResult, MediaUploadRequest, MediaUploadResult, - SendSummary, remember_seen_event, unix_now_seconds, + SelfMembership, SendSummary, remember_seen_event, unix_now_seconds, }; mod audit; @@ -690,20 +690,21 @@ impl AppClient { self.remember_published_reports(&effects); self.app.save_state(&self.state)?; self.queue_own_group_system_projection_updates(&effects); - // A local leave / decline departs the group just like an observed - // self-removal does. The inbound `observe_account_device_effects` path - // suppresses the account unread aggregate for self-removal, but our own - // relay echoes are skipped, so the locally initiated departure must - // suppress here too. No-op if no `account_groups` row exists yet, so it - // never resurrects pruned projection state. This is the source-of-truth - // write for the account unread aggregate, so propagate its error (like - // the nearby projection writes) rather than swallow it: a silently - // failed update would leave `account_unread_total()` returning an - // inflated badge after a leave that otherwise reports success. + // A local leave / decline is a voluntary departure, recorded as `Left` + // so the chat list can distinguish it from an involuntary removal. The + // inbound `observe_account_device_effects` path records this for an + // observed self-removal, but our own relay echoes are skipped, so the + // locally initiated departure must record (and thereby suppress) here + // too. No-op if no `account_groups` row exists yet, so it never + // resurrects pruned projection state. This is the source-of-truth write + // for the account unread aggregate, so propagate its error (like the + // nearby projection writes) rather than swallow it: a silently failed + // update would leave `account_unread_total()` returning an inflated + // badge after a leave that otherwise reports success. self.app.set_group_self_membership( &self.state.label, &hex::encode(group_id.as_slice()), - true, + SelfMembership::Left, )?; Ok(send_summary_from_effects(&effects)) } @@ -720,12 +721,19 @@ impl AppClient { /// For each row still carrying the default `'member'`, it asks the engine /// for the group's roster (`runtime.members`, sourced from the Marmot /// record's authoritative post-merge member set) and flips the row to - /// `'removed'` only when the call succeeds and the local account id is + /// `Removed` only when the call succeeds and the local account id is /// definitively absent. Engine errors / unknown groups are skipped so /// uncertainty never suppresses (matching the projection's existing /// invariant). The work is gated behind a once-only account-import marker, /// so subsequent opens are a single marker read and the hot path stays /// projection-only. + /// + /// A backfilled departure is recorded as `Removed`, not `Left`: roster + /// absence cannot tell us *why* the account is gone, and `Removed` + /// ("removed by someone") is the safer unknown bucket — claiming the user + /// voluntarily left a group an admin actually evicted them from is the more + /// misleading error. Both states suppress the unread aggregate identically, + /// so the choice only affects how the chat list labels the departure. pub(crate) fn backfill_self_membership_once(&self) -> Result<(), AppError> { if self .app @@ -753,8 +761,11 @@ impl AppClient { continue; }; if local_account_removed_from_roster(&members, &local_account_id_hex) { - self.app - .set_group_self_membership(&self.state.label, &group_id_hex, true)?; + self.app.set_group_self_membership( + &self.state.label, + &group_id_hex, + SelfMembership::Removed, + )?; } } self.app.mark_account_import_complete( diff --git a/crates/marmot-app/src/client/sync.rs b/crates/marmot-app/src/client/sync.rs index 61b2ce6a..7f995699 100644 --- a/crates/marmot-app/src/client/sync.rs +++ b/crates/marmot-app/src/client/sync.rs @@ -11,8 +11,8 @@ use crate::media::media_imeta_tags_are_valid; use crate::notifications; use crate::{ AppError, AppGroupAdminPolicyComponent, AppMessageProjection, SDK_DRAIN_WAIT, - SDK_FIRST_SYNC_WAIT, SyncSummary, TRANSPORT_CURSOR_MAX_FUTURE_SKEW, remember_seen_event, - unix_now_seconds, + SDK_FIRST_SYNC_WAIT, SelfMembership, SyncSummary, TRANSPORT_CURSOR_MAX_FUTURE_SKEW, + remember_seen_event, unix_now_seconds, }; use super::AppClient; @@ -479,12 +479,9 @@ impl AppClient { self.sync_runtime_groups().await?; } if let cgka_traits::engine::GroupEvent::GroupStateChanged { - group_id, - change: - cgka_traits::engine::GroupStateChange::MemberRemoved { member } - | cgka_traits::engine::GroupStateChange::MemberLeft { member }, - .. + group_id, change, .. } = event + && let Some((member, membership)) = member_departure(change) { let group_id_hex = hex::encode(group_id.as_slice()); let member_id_hex = hex::encode(member.as_slice()); @@ -494,7 +491,9 @@ impl AppClient { &member_id_hex, ); // Only the local account leaving / being removed suppresses our - // own unread aggregate for the group; a peer removal must not. + // own unread aggregate for the group; a peer departure must not. + // The recorded membership distinguishes a voluntary `Left` from + // an involuntary `Removed` so the chat list can tell them apart. // This projection write is the source of truth for the account // unread aggregate, so propagate its error (matching the nearby // timeline/message projection writes) instead of swallowing it: @@ -502,20 +501,26 @@ impl AppClient { // `account_unread_total()` returning an inflated badge after a // self-removal that sync otherwise reports as successful. if member_id_hex.eq_ignore_ascii_case(&local_account_id_hex) { - self.app - .set_group_self_membership(&self.state.label, &group_id_hex, true)?; + self.app.set_group_self_membership( + &self.state.label, + &group_id_hex, + membership, + )?; } } // A (re-)join or create restores the local account's membership so a // re-add after removal un-suppresses the group's unread count. Same - // source-of-truth write as the removal path above: propagate the + // source-of-truth write as the departure path above: propagate the // error rather than swallow it. if let cgka_traits::engine::GroupEvent::GroupJoined { group_id, .. } | cgka_traits::engine::GroupEvent::GroupCreated { group_id } = event { let group_id_hex = hex::encode(group_id.as_slice()); - self.app - .set_group_self_membership(&self.state.label, &group_id_hex, false)?; + self.app.set_group_self_membership( + &self.state.label, + &group_id_hex, + SelfMembership::Member, + )?; } } // Synthesize durable kind-1210 system rows from authenticated state @@ -585,6 +590,63 @@ fn clamped_transport_cursor( .unwrap_or(clamped) } +/// Classify a group state change that ends a member's participation, returning +/// the departing member alongside how that departure should be recorded for the +/// member: a `MemberLeft` self-removal is a voluntary [`SelfMembership::Left`]; +/// a `MemberRemoved` eviction by another member is [`SelfMembership::Removed`]. +/// Returns `None` for changes that are not departures. +fn member_departure( + change: &cgka_traits::engine::GroupStateChange, +) -> Option<(&cgka_traits::MemberId, SelfMembership)> { + use cgka_traits::engine::GroupStateChange; + match change { + GroupStateChange::MemberLeft { member } => Some((member, SelfMembership::Left)), + GroupStateChange::MemberRemoved { member } => Some((member, SelfMembership::Removed)), + _ => None, + } +} + +#[cfg(test)] +mod membership_change_tests { + use super::member_departure; + use crate::SelfMembership; + use cgka_traits::MemberId; + use cgka_traits::engine::GroupStateChange; + + #[test] + fn member_departure_distinguishes_self_leave_from_eviction() { + let member = MemberId::new(vec![0xaa]); + + // A SelfRemove proposal is a voluntary departure. + let left = GroupStateChange::MemberLeft { + member: member.clone(), + }; + let (subject, membership) = member_departure(&left).expect("MemberLeft is a departure"); + assert_eq!(subject, &member); + assert_eq!(membership, SelfMembership::Left); + + // An eviction by another member is an involuntary removal. + let removed = GroupStateChange::MemberRemoved { + member: member.clone(), + }; + let (subject, membership) = + member_departure(&removed).expect("MemberRemoved is a departure"); + assert_eq!(subject, &member); + assert_eq!(membership, SelfMembership::Removed); + } + + #[test] + fn member_departure_ignores_non_departures() { + let member = MemberId::new(vec![0xaa]); + let added = GroupStateChange::MemberAdded { + member: member.clone(), + }; + let admin = GroupStateChange::AdminAdded { member }; + assert!(member_departure(&added).is_none()); + assert!(member_departure(&admin).is_none()); + } +} + #[cfg(test)] mod transport_cursor_tests { use super::clamped_transport_cursor; diff --git a/crates/marmot-app/src/conversions.rs b/crates/marmot-app/src/conversions.rs index 59ea68fc..44ec6708 100644 --- a/crates/marmot-app/src/conversions.rs +++ b/crates/marmot-app/src/conversions.rs @@ -65,6 +65,9 @@ pub(crate) fn stored_group_from_app_group(group: &AppGroupRecord) -> StoredAccou admin_keys_hex: group.admin_policy.admins.join(","), archived: group.archived, pending_confirmation: group.pending_confirmation, + // Ignored by the projection save (owned by `set_group_self_membership`); + // carried for struct completeness and round-trip symmetry. + self_membership: group.self_membership, welcomer_account_id_hex: group.welcomer_account_id_hex.clone(), via_welcome_message_id_hex: group.via_welcome_message_id_hex.clone(), components: stored_components_from_app_group(group), @@ -177,6 +180,7 @@ pub(crate) fn app_group_from_stored_group( } group.archived = stored.archived; group.pending_confirmation = stored.pending_confirmation; + group.self_membership = stored.self_membership; group.welcomer_account_id_hex = stored.welcomer_account_id_hex; group.via_welcome_message_id_hex = stored.via_welcome_message_id_hex; Ok(group) diff --git a/crates/marmot-app/src/groups.rs b/crates/marmot-app/src/groups.rs index 018414d8..d5c01b04 100644 --- a/crates/marmot-app/src/groups.rs +++ b/crates/marmot-app/src/groups.rs @@ -28,7 +28,7 @@ use cgka_traits::{GroupId, TransportEndpoint, TransportGroupSubscription}; use serde::{Deserialize, Serialize}; use crate::media::media_imeta_tags_are_valid; -use crate::{AccountState, AppError, ReceivedMessage, SendSummary, SyncSummary}; +use crate::{AccountState, AppError, ReceivedMessage, SelfMembership, SendSummary, SyncSummary}; #[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq)] pub struct AppGroupRecord { @@ -52,6 +52,10 @@ pub struct AppGroupRecord { pub archived: bool, #[serde(default)] pub pending_confirmation: bool, + /// Whether the local account is still a member of this group, and if not, + /// whether it left voluntarily (`Left`) or was removed (`Removed`). + #[serde(default)] + pub self_membership: SelfMembership, #[serde(default, skip_serializing_if = "Option::is_none")] pub welcomer_account_id_hex: Option, #[serde(default, skip_serializing_if = "Option::is_none")] @@ -302,6 +306,7 @@ impl AppGroupRecord { encrypted_media: AppGroupEncryptedMediaComponent::disabled(), archived: false, pending_confirmation: false, + self_membership: SelfMembership::Member, welcomer_account_id_hex: None, via_welcome_message_id_hex: None, } diff --git a/crates/marmot-app/src/lib.rs b/crates/marmot-app/src/lib.rs index 67cc6110..a73cab0c 100644 --- a/crates/marmot-app/src/lib.rs +++ b/crates/marmot-app/src/lib.rs @@ -166,7 +166,7 @@ pub use relay_telemetry_export::{ }; pub use storage_sqlite::{ ChatListAvatar, ChatListMessagePreview, ChatListQuery, ChatListRow, MAX_TIMELINE_LIMIT, - TimelineMessageQuery, TimelineMessageRecord, TimelinePage, TimelinePagination, + SelfMembership, TimelineMessageQuery, TimelineMessageRecord, TimelinePage, TimelinePagination, TimelineReactionSummary, TimelineReplyPreview, TimelineUserReaction, }; pub use transport_nostr_adapter::{ @@ -1554,12 +1554,12 @@ impl MarmotApp { &self, account_ref: &str, group_id_hex: &str, - removed: bool, + membership: SelfMembership, ) -> Result<(), AppError> { let account = self.account_home().account(account_ref)?; self.ensure_account_state(&account.label)?; self.account_storage(&account.label)? - .set_group_self_membership(group_id_hex, removed)?; + .set_group_self_membership(group_id_hex, membership)?; Ok(()) } diff --git a/crates/marmot-app/src/runtime/tests.rs b/crates/marmot-app/src/runtime/tests.rs index 2d525ed5..5e3d7cab 100644 --- a/crates/marmot-app/src/runtime/tests.rs +++ b/crates/marmot-app/src/runtime/tests.rs @@ -952,6 +952,7 @@ fn chat_list_test_row(group_id_hex: &str, title: &str) -> ChatListRow { last_read_message_id_hex: None, last_read_timeline_at: None, updated_at: 0, + self_membership: crate::SelfMembership::Member, } } diff --git a/crates/marmot-app/tests/relay_runtime.rs b/crates/marmot-app/tests/relay_runtime.rs index 1c0ec664..48b85873 100644 --- a/crates/marmot-app/tests/relay_runtime.rs +++ b/crates/marmot-app/tests/relay_runtime.rs @@ -16,8 +16,8 @@ use marmot_app::{ AuditLogSettings, AuditLogTrackerConfig, AuditLogUploadSource, MarmotApp, MarmotAppConfig, MarmotAppEvent, MarmotAppRuntime, MediaAttachmentReference, MediaLocator, MediaUploadAttachmentRequest, MediaUploadRequest, MissingRelayListKind, NotificationWakeSource, - PushPlatform, RuntimeMessageUpdate, SignOutOptions, TimelineMessageQuery, TimelinePagination, - UserDirectorySearch, UserProfileMetadata, tag_value, + PushPlatform, RuntimeMessageUpdate, SelfMembership, SignOutOptions, TimelineMessageQuery, + TimelinePagination, UserDirectorySearch, UserProfileMetadata, tag_value, }; use nostr::base64::Engine as _; use nostr::base64::engine::general_purpose::STANDARD as BASE64_STANDARD; @@ -3008,6 +3008,17 @@ async fn local_leave_suppresses_account_unread_total() { 0, "a local leave must suppress the leaver's frozen account unread total" ); + + // The same leave records the chat-list row as a voluntary `Left` (not an + // involuntary `Removed`), end to end through the projection refresh, so the + // chat list can label the departure. + let bob_row = app + .chat_list("bob", true) + .unwrap() + .into_iter() + .find(|row| row.group_id_hex == group_id_hex) + .expect("bob's chat-list row survives the leave"); + assert_eq!(bob_row.self_membership, SelfMembership::Left); } #[tokio::test] diff --git a/crates/marmot-uniffi/src/conversions/chat_list.rs b/crates/marmot-uniffi/src/conversions/chat_list.rs index 036f17d6..8773451e 100644 --- a/crates/marmot-uniffi/src/conversions/chat_list.rs +++ b/crates/marmot-uniffi/src/conversions/chat_list.rs @@ -2,7 +2,7 @@ use marmot_app::{ChatListAvatar, ChatListMessagePreview, ChatListRow, RuntimeChatListUpdate}; -use super::common::markdown_content_tokens; +use super::common::{SelfMembershipFfi, markdown_content_tokens}; use crate::markdown::MarkdownDocumentFfi; #[derive(Clone, Debug, uniffi::Record)] @@ -72,6 +72,9 @@ pub struct ChatListRowFfi { pub last_read_message_id_hex: Option, pub last_read_timeline_at: Option, pub updated_at: u64, + /// Whether the local account is still a member of this group, and if not, + /// whether it left voluntarily or was removed. + pub self_membership: SelfMembershipFfi, } impl From for ChatListRowFfi { @@ -93,6 +96,7 @@ impl From for ChatListRowFfi { last_read_message_id_hex: value.last_read_message_id_hex, last_read_timeline_at: value.last_read_timeline_at, updated_at: value.updated_at, + self_membership: value.self_membership.into(), } } } diff --git a/crates/marmot-uniffi/src/conversions/common.rs b/crates/marmot-uniffi/src/conversions/common.rs index 0af9d12c..ef1dbb9e 100644 --- a/crates/marmot-uniffi/src/conversions/common.rs +++ b/crates/marmot-uniffi/src/conversions/common.rs @@ -1,9 +1,31 @@ //! Shared FFI helpers used across the conversion sub-modules. use cgka_traits::{GroupId, app_event::MARMOT_APP_EVENT_KIND_CHAT}; +use marmot_app::SelfMembership; use crate::markdown::{MarkdownDocumentFfi, parse_markdown_document}; +/// The local account's own membership in a group: an active `Member`, or a +/// terminal state describing how it left — `Left` (a voluntary self-removal or +/// declined invite) or `Removed` (evicted by another member). Surfaced on both +/// the chat-list row and the group-detail record. +#[derive(Clone, Copy, Debug, uniffi::Enum)] +pub enum SelfMembershipFfi { + Member, + Left, + Removed, +} + +impl From for SelfMembershipFfi { + fn from(value: SelfMembership) -> Self { + match value { + SelfMembership::Member => SelfMembershipFfi::Member, + SelfMembership::Left => SelfMembershipFfi::Left, + SelfMembership::Removed => SelfMembershipFfi::Removed, + } + } +} + /// One Nostr tag from an inner Marmot app event, e.g. `["e", ""]` or an /// `["imeta", …]` media descriptor. Host apps branch on the inner event `kind` /// plus these tags instead of a fixed payload enum. diff --git a/crates/marmot-uniffi/src/conversions/group.rs b/crates/marmot-uniffi/src/conversions/group.rs index f75d3eff..22d092e4 100644 --- a/crates/marmot-uniffi/src/conversions/group.rs +++ b/crates/marmot-uniffi/src/conversions/group.rs @@ -10,6 +10,7 @@ use marmot_app::{ }; use super::account::SendSummaryFfi; +use super::common::SelfMembershipFfi; use crate::errors::MarmotKitError; #[derive(Clone, Debug, uniffi::Record)] @@ -32,6 +33,9 @@ pub struct AppGroupRecordFfi { pub disappearing_message_secs: u64, pub archived: bool, pub pending_confirmation: bool, + /// Whether the local account is still a member of this group, and if not, + /// whether it left voluntarily or was removed. + pub self_membership: SelfMembershipFfi, pub welcomer_account_id_hex: Option, pub via_welcome_message_id_hex: Option, } @@ -64,6 +68,7 @@ impl From for AppGroupRecordFfi { disappearing_message_secs, archived: value.archived, pending_confirmation: value.pending_confirmation, + self_membership: value.self_membership.into(), welcomer_account_id_hex: value.welcomer_account_id_hex, via_welcome_message_id_hex: value.via_welcome_message_id_hex, } diff --git a/crates/marmot-uniffi/src/conversions/mod.rs b/crates/marmot-uniffi/src/conversions/mod.rs index 2d0f3d87..af310998 100644 --- a/crates/marmot-uniffi/src/conversions/mod.rs +++ b/crates/marmot-uniffi/src/conversions/mod.rs @@ -74,11 +74,31 @@ mod tests { disappearing_message_secs: 0, archived: false, pending_confirmation: false, + self_membership: SelfMembershipFfi::Member, welcomer_account_id_hex: None, via_welcome_message_id_hex: None, } } + #[test] + fn self_membership_ffi_preserves_each_variant() { + use marmot_app::SelfMembership; + // The FFI enum mirrors the app enum 1:1; a swapped Left/Removed here + // would mislabel departures on the apps' chat list and group detail. + assert!(matches!( + SelfMembershipFfi::from(SelfMembership::Member), + SelfMembershipFfi::Member + )); + assert!(matches!( + SelfMembershipFfi::from(SelfMembership::Left), + SelfMembershipFfi::Left + )); + assert!(matches!( + SelfMembershipFfi::from(SelfMembership::Removed), + SelfMembershipFfi::Removed + )); + } + fn member(member_id_hex: &str, is_admin: bool, is_self: bool) -> GroupMemberDetailsFfi { GroupMemberDetailsFfi { member_id_hex: member_id_hex.to_owned(), diff --git a/crates/storage-sqlite/src/account_projection.rs b/crates/storage-sqlite/src/account_projection.rs index de60cd39..314a3b81 100644 --- a/crates/storage-sqlite/src/account_projection.rs +++ b/crates/storage-sqlite/src/account_projection.rs @@ -7,6 +7,44 @@ use rusqlite::{ Connection, OptionalExtension, params, params_from_iter, types::{Type, Value}, }; +use serde::{Deserialize, Serialize}; + +/// The local account's own membership in a projected group. +/// +/// `Member` is the default and the fallback for unknown/forward-incompatible +/// state: uncertainty must never hide a conversation. `Left` and `Removed` are +/// both terminal "no longer a member" states that suppress the account's unread +/// aggregate; they differ only in *why* membership ended — `Left` is a +/// voluntary self-removal (including declining an invite), `Removed` is an +/// eviction by another member. +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] +pub enum SelfMembership { + #[default] + Member, + Left, + Removed, +} + +impl SelfMembership { + /// The persisted `account_groups.self_membership` text for this state. + pub(crate) fn as_str(self) -> &'static str { + match self { + SelfMembership::Member => "member", + SelfMembership::Left => "left", + SelfMembership::Removed => "removed", + } + } + + /// Reads persisted membership text. Unknown values fall back to `Member` so + /// a row written by a newer schema never suppresses its unread here. + pub(crate) fn from_storage(value: &str) -> Self { + match value { + "left" => SelfMembership::Left, + "removed" => SelfMembership::Removed, + _ => SelfMembership::Member, + } + } +} #[derive(Clone, Debug, Default, PartialEq, Eq)] pub struct StoredAccountState { @@ -32,6 +70,12 @@ pub struct StoredAccountGroup { pub pending_confirmation: bool, pub welcomer_account_id_hex: Option, pub via_welcome_message_id_hex: Option, + /// The local account's membership in this group. Read-only on this struct: + /// it is loaded from `account_groups` but owned exclusively by + /// [`SqliteAccountStorage::set_group_self_membership`], so the projection + /// save deliberately ignores it (a routine resave must not clobber a + /// membership change). New rows take the schema default `Member`. + pub self_membership: SelfMembership, pub components: Vec, } @@ -124,6 +168,7 @@ struct RawStoredAccountGroup { pending_confirmation: bool, welcomer_account_id_hex: Option, via_welcome_message_id_hex: Option, + self_membership: SelfMembership, } impl SqliteAccountStorage { @@ -179,7 +224,7 @@ impl SqliteAccountStorage { image_hash_hex, image_key_hex, image_nonce_hex, image_upload_key_hex, image_media_type, admin_keys_hex, archived, pending_confirmation, welcomer_account_id_hex, - via_welcome_message_id_hex + via_welcome_message_id_hex, self_membership FROM account_groups ORDER BY updated_at, group_id_hex", ) @@ -201,6 +246,7 @@ impl SqliteAccountStorage { pending_confirmation: row.get::<_, i64>(11)? != 0, welcomer_account_id_hex: row.get(12)?, via_welcome_message_id_hex: row.get(13)?, + self_membership: SelfMembership::from_storage(&row.get::<_, String>(14)?), }) }) .storage()? @@ -226,6 +272,7 @@ impl SqliteAccountStorage { pending_confirmation: raw.pending_confirmation, welcomer_account_id_hex: raw.welcomer_account_id_hex, via_welcome_message_id_hex: raw.via_welcome_message_id_hex, + self_membership: raw.self_membership, components, }); } @@ -413,24 +460,23 @@ impl SqliteAccountStorage { } /// Record the local account's own membership in `group_id_hex` so the - /// removed-group-suppressed unread aggregate can exclude groups the account - /// has left or been removed from. `removed = true` marks the group - /// `'removed'` (suppress its unread); `removed = false` re-affirms `'member'` + /// chat list and removed-group-suppressed unread aggregate reflect whether + /// the account is still in the group and, if not, how it left. `Left` and + /// `Removed` both suppress the group's unread; `Member` re-affirms it /// (preserve / un-suppress on re-add). No-op when the group has no /// `account_groups` row yet, so this never resurrects pruned projection /// state. pub fn set_group_self_membership( &self, group_id_hex: &str, - removed: bool, + membership: SelfMembership, ) -> StorageResult<()> { - let self_membership = if removed { "removed" } else { "member" }; self.lock()? .execute( "UPDATE account_groups SET self_membership = ?2 WHERE group_id_hex = ?1", - params![group_id_hex, self_membership], + params![group_id_hex, membership.as_str()], ) .storage()?; Ok(()) diff --git a/crates/storage-sqlite/src/account_projection/tests.rs b/crates/storage-sqlite/src/account_projection/tests.rs index 5c1a759f..2f7495a6 100644 --- a/crates/storage-sqlite/src/account_projection/tests.rs +++ b/crates/storage-sqlite/src/account_projection/tests.rs @@ -25,6 +25,7 @@ fn group(id: &str, name: &str) -> StoredAccountGroup { pending_confirmation: false, welcomer_account_id_hex: None, via_welcome_message_id_hex: None, + self_membership: SelfMembership::Member, components: vec![ StoredAccountGroupComponent { component_id: 0x8001, diff --git a/crates/storage-sqlite/src/chat_list.rs b/crates/storage-sqlite/src/chat_list.rs index 836b2840..eaad2138 100644 --- a/crates/storage-sqlite/src/chat_list.rs +++ b/crates/storage-sqlite/src/chat_list.rs @@ -1,6 +1,6 @@ use crate::{ - SqliteAccountStorage, SqliteResultExt, bool_i64, optional_u64_to_i64, u64_to_i64, - unix_now_seconds, + SelfMembership, SqliteAccountStorage, SqliteResultExt, bool_i64, optional_u64_to_i64, + u64_to_i64, unix_now_seconds, }; use cgka_traits::app_components::{GROUP_AVATAR_URL_COMPONENT_ID, decode_group_avatar_url_v1}; use cgka_traits::app_event::MARMOT_APP_EVENT_KIND_CHAT; @@ -75,6 +75,9 @@ pub struct ChatListRow { pub last_read_message_id_hex: Option, pub last_read_timeline_at: Option, pub updated_at: u64, + /// The local account's membership in this group (active member, left, or + /// removed). Denormalized from `account_groups.self_membership`. + pub self_membership: SelfMembership, } #[derive(Clone, Debug)] @@ -85,6 +88,7 @@ struct AccountGroupRow { profile_name: String, avatar_url: Option, avatar: Option, + self_membership: SelfMembership, } #[derive(Clone, Debug)] @@ -108,11 +112,11 @@ impl SqliteAccountStorage { /// Cheap unread aggregate over the materialized `chat_list_rows` /// projection. Reads only the projection table (a single grouped /// `COUNT`/`SUM`), so it does not materialize timelines or load a session. - /// Archived conversations are excluded. Groups whose local - /// `account_groups.self_membership` is known to be `'removed'` (the local - /// account left or was removed) are also excluded; unknown membership - /// (`'member'`, the default, or no matching `account_groups` row) preserves - /// the unread count so uncertainty never suppresses. + /// Archived conversations are excluded. Groups the local account is no + /// longer in — `account_groups.self_membership` of `'left'` or `'removed'` + /// — are also excluded; unknown membership (`'member'`, the default, or no + /// matching `account_groups` row) preserves the unread count so uncertainty + /// never suppresses. pub fn account_unread_total(&self) -> StorageResult { let conn = self.lock()?; conn.query_row( @@ -121,7 +125,7 @@ impl SqliteAccountStorage { FROM chat_list_rows AS row LEFT JOIN account_groups AS ag ON ag.group_id_hex = row.group_id_hex WHERE row.archived = 0 - AND COALESCE(ag.self_membership, 'member') != 'removed'", + AND COALESCE(ag.self_membership, 'member') NOT IN ('left', 'removed')", [], |row| Ok((row.get::<_, i64>(0)?, row.get::<_, i64>(1)?)), ) @@ -357,6 +361,14 @@ fn chat_list_projection_complete_tx( OR row.avatar_image_upload_key_hex IS NOT ag.image_upload_key_hex OR row.avatar_media_type IS NOT ag.image_media_type OR (row.avatar_url IS NOT NULL AND avatar_url.component_data_hex IS NULL) + -- Normalize like `SelfMembership::from_storage` (unknown -> + -- 'member'), matching what a rebuild stores, so a value from a + -- newer schema does not look perpetually stale. + OR row.self_membership IS NOT CASE ag.self_membership + WHEN 'left' THEN 'left' + WHEN 'removed' THEN 'removed' + ELSE 'member' + END OR row.updated_at < COALESCE(avatar_url.updated_at, 0) OR row.updated_at < ag.updated_at )", @@ -516,11 +528,12 @@ fn rebuild_chat_list_row_for_group_tx( last_message_id_hex, last_message_sender, last_message_preview, last_message_kind, last_message_timeline_at, last_message_deleted, unread_count, unread_mention_count, first_unread_message_id_hex, - last_read_message_id_hex, last_read_timeline_at, updated_at + last_read_message_id_hex, last_read_timeline_at, updated_at, + self_membership ) VALUES ( ?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, - ?11, ?12, ?13, ?14, ?15, ?16, ?17, ?18, ?19, ?20, ?21, ?22, ?23 + ?11, ?12, ?13, ?14, ?15, ?16, ?17, ?18, ?19, ?20, ?21, ?22, ?23, ?24 ) ON CONFLICT(group_id_hex) DO UPDATE SET archived = excluded.archived, @@ -544,7 +557,8 @@ fn rebuild_chat_list_row_for_group_tx( first_unread_message_id_hex = excluded.first_unread_message_id_hex, last_read_message_id_hex = excluded.last_read_message_id_hex, last_read_timeline_at = excluded.last_read_timeline_at, - updated_at = excluded.updated_at", + updated_at = excluded.updated_at, + self_membership = excluded.self_membership", params![ &group.group_id_hex, bool_i64(group.archived), @@ -599,6 +613,7 @@ fn rebuild_chat_list_row_for_group_tx( .and_then(|state| state.last_read_timeline_at) )?, u64_to_i64(now)?, + group.self_membership.as_str(), ], ) .storage()?; @@ -736,7 +751,8 @@ fn account_groups_tx(tx: &Connection) -> StorageResult> { .prepare( "SELECT ag.group_id_hex, ag.archived, ag.pending_confirmation, ag.profile_name, image_hash_hex, image_key_hex, image_nonce_hex, - image_upload_key_hex, image_media_type, avatar_url.component_data_hex + image_upload_key_hex, image_media_type, avatar_url.component_data_hex, + ag.self_membership FROM account_groups AS ag LEFT JOIN account_group_app_components AS avatar_url ON avatar_url.group_id_hex = ag.group_id_hex @@ -756,7 +772,8 @@ fn account_group_tx(tx: &Connection, group_id_hex: &str) -> StorageResult