Skip to content

Commit 2f64888

Browse files
committed
refactor(seam): single-source notification store; drop premature native dep
Two robustness fixes to the Phase-1 backend seam, replacing scaffolding that would not survive into later phases. Single source of truth for notifications - The topbar badge read a global Signal<Vec<Notification>> while the popover independently re-fetched the same feed via its own use_resource, so mark-all-read updated the badge but NOT the open popover list (and vice-versa) — a divergence that worsens as more views touch notifications. - The context store is now one Signal<AsyncState<Vec<Notification>>>, seeded once at the seam in app.rs and shared by the badge and the popover. User actions (mark-all-read / per-item read) mutate that one store, so the two cannot disagree. The popover projects the store through capability filtering via AsyncState::project (Loading/Empty/Error stay correct) and reloads on Retry through the shared Resource handle. - New reusable AsyncState methods (project, loaded_mut) and pure notification mutation helpers (mark_all_read, mark_read) are unit-tested. Drop the premature `native` feature - Enabling nodedb-client's `native` feature dragged a TLS + C-toolchain build tree (tokio-rustls, aws-lc-rs, aws-lc-sys, cmake) into every build — solely to hold an Option<NativeClient> field that was #[allow(dead_code)] and never read. - The feature and the field are removed until the phase that first constructs a client. NodeDbConnectionService stays as the test-covered Phase-2 seam impl; its object-safety/instantiability are proven by tests, not a throwaway runtime binding in app.rs. fmt + clippy -D warnings + nextest all green (37 tests, was 31).
1 parent 0c4e004 commit 2f64888

8 files changed

Lines changed: 203 additions & 164 deletions

File tree

Cargo.lock

Lines changed: 0 additions & 90 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,11 @@ repository = "https://github.com/NodeDB-Lab/nodedb-studio"
1313
homepage = "https://nodedb.dev"
1414

1515
[workspace.dependencies]
16-
# Internal NodeDB crates.
17-
nodedb-client = { version = "0.3.0", features = ["native"] }
16+
# Internal NodeDB crates. The `native` feature (real client transport) is
17+
# enabled in the phase that first constructs a NativeClient — it drags in a
18+
# TLS + C-toolchain build tree (tokio-rustls, aws-lc-rs, cmake), so it is not
19+
# carried while the seam impl is an inert stub.
20+
nodedb-client = "0.3.0"
1821
nodedb-types = "0.3.0"
1922
# Async
2023
async-trait = "0.1"

nodedb-studio/src/app.rs

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@ use dioxus::prelude::*;
1414

1515
use crate::modals::ModalHost;
1616
use crate::models::notification::Notification;
17+
use crate::services::async_state::AsyncState;
1718
use crate::services::connection_service::{ConnectionService, MockConnectionService};
18-
use crate::services::nodedb_service::NodeDbConnectionService;
1919
use crate::state::connection::ActiveConnection;
2020
use crate::state::connections_registry::SavedConnection;
2121
use crate::state::preferences::Preferences;
@@ -31,15 +31,17 @@ pub fn App() -> Element {
3131
// The single seam to the outside world. Provided as a trait object so a
3232
// real client can replace the mock without touching consumers.
3333
let service: Rc<dyn ConnectionService> = Rc::new(MockConnectionService);
34-
35-
// Compile-time check: the real-client stub must be instantiable and
36-
// object-safe behind the seam trait. Not the active impl yet — the mock is.
37-
let _real: Rc<dyn ConnectionService> = Rc::new(NodeDbConnectionService::default());
34+
// The real-client stub's instantiability + object-safety behind the seam is
35+
// proven by `nodedb_service::tests::stub_is_object_safe_behind_rc`, so it is
36+
// not constructed here on every render.
3837

3938
use_context_provider(|| service.clone());
4039
use_context_provider(|| Signal::new(None::<ActiveConnection>));
4140
let mut registry = use_context_provider(|| Signal::new(Vec::<SavedConnection>::new()));
42-
let mut notifications = use_context_provider(|| Signal::new(Vec::<Notification>::new()));
41+
// Single source of truth for notifications: the bell badge AND the popover
42+
// read this one store; user actions (mark-all-read / click) mutate it.
43+
let mut notifications =
44+
use_context_provider(|| Signal::new(AsyncState::<Vec<Notification>>::Loading));
4345
use_context_provider(|| Signal::new(Preferences::default()));
4446
// Modal state is provided here (not in Studio) because Preferences is
4547
// reachable while disconnected and via Cmd+, in either state.
@@ -49,20 +51,26 @@ pub fn App() -> Element {
4951
// mock resolves instantly; the real client awaits the network. The guard
5052
// rule: clone the Rc before the async block and `.set()` the signals only
5153
// AFTER the await resolves — never hold a read/write guard across `.await`.
52-
{
54+
// The Resource handle is shared via context so a view (the notification
55+
// popover's Retry) can reload the feed.
56+
let reload_feed = {
5357
let svc = service.clone();
5458
use_resource(move || {
5559
let svc = svc.clone();
5660
async move {
61+
// Reset to Loading at the start of each (re)load so a Retry shows
62+
// the spinner. `.set()` holds no guard across the awaits below.
63+
notifications.set(AsyncState::Loading);
5764
if let Ok(conns) = svc.list_connections().await {
5865
registry.set(conns);
5966
}
60-
if let Ok(notifs) = svc.notifications().await {
61-
notifications.set(notifs);
62-
}
67+
// Map the seam result straight into the store: Ok(empty)->Empty,
68+
// Ok(data)->Loaded, Err->Error.
69+
notifications.set(AsyncState::from_value(Some(svc.notifications().await)));
6370
}
64-
});
65-
}
71+
})
72+
};
73+
use_context_provider(|| reload_feed);
6674

6775
let active = use_context::<Signal<Option<ActiveConnection>>>();
6876

nodedb-studio/src/components/popovers/notification_popover.rs

Lines changed: 32 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,23 @@
11
//! Notification popover: capability-filtered, grouped list with a bell badge,
22
//! mark-all-read, and click-to-navigate.
33
//!
4-
//! The rendered list is fetched through the async `ConnectionService` seam via
5-
//! `use_resource`, then mapped to `AsyncState` and rendered through the shared
6-
//! `AsyncView` — the canonical async-UI pattern. Capability filtering is applied
7-
//! to the `Ok` payload *before* `AsyncState::from_value`, so `Empty` reflects
8-
//! what THIS connection can see (not the raw feed). The Error state offers a
9-
//! Retry gated on `StudioError::is_retriable()`, wired to `Resource::restart()`.
10-
//!
11-
//! The topbar bell badge derives from a separate global `Signal<Vec<Notification>>`
12-
//! seeded in `app.rs`; mark-all-read / per-item clicks mutate that signal so the
13-
//! badge stays in sync. Converging the badge and this list onto one source is
14-
//! deferred to a later phase.
15-
16-
use std::rc::Rc;
4+
//! Single source of truth: both this popover and the topbar bell badge read the
5+
//! one shared `Signal<AsyncState<Vec<Notification>>>` store (seeded once at the
6+
//! seam in `app.rs`). The popover `project`s that store through capability
7+
//! filtering — so `Empty` reflects what THIS connection sees — then renders the
8+
//! `Loading`/`Empty`/`Error` states through the shared `AsyncView` and the loaded
9+
//! list itself. Mark-all-read / per-item clicks MUTATE the store, so the badge
10+
//! and the list never diverge. The Error-state Retry reloads the feed via the
11+
//! shared `Resource` handle, gated on `StudioError::is_retriable()`.
1712
1813
use dioxus::prelude::*;
1914

2015
use crate::components::async_view::AsyncView;
2116
use crate::models::notification::{Notification, NotificationTarget};
2217
use crate::routes::Route;
2318
use crate::services::async_state::AsyncState;
24-
use crate::services::connection_service::ConnectionService;
2519
use crate::state::connection::{ActiveConnection, Capabilities};
26-
use crate::state::notifications::visible;
20+
use crate::state::notifications::{mark_all_read, mark_read, visible};
2721
use crate::state::ui::Popover;
2822

2923
/// Where a notification navigates when clicked.
@@ -45,11 +39,12 @@ fn target_route(target: NotificationTarget) -> Route {
4539

4640
#[component]
4741
pub fn NotificationPopover() -> Element {
48-
// Global signal kept for mark-all-read + per-item read + topbar badge sync.
49-
let mut notifs = use_context::<Signal<Vec<Notification>>>();
42+
// The one shared store (mutated here for mark-all-read + per-item read) and
43+
// the shared reload handle backing the Error-state Retry.
44+
let mut store = use_context::<Signal<AsyncState<Vec<Notification>>>>();
45+
let mut reload = use_context::<Resource<()>>();
5046
let mut popover = use_context::<Signal<Option<Popover>>>();
5147
let active = use_context::<Signal<Option<ActiveConnection>>>();
52-
let service = use_context::<Rc<dyn ConnectionService>>();
5348
let nav = use_navigator();
5449

5550
// Capability gate (unchanged): no connection -> render nothing.
@@ -58,30 +53,18 @@ pub fn NotificationPopover() -> Element {
5853
None => return rsx! {},
5954
};
6055

61-
// Fetch the feed through the async seam. Clone the Rc BEFORE the async block;
62-
// never hold a signal/Resource guard across `.await`.
63-
let mut feed = use_resource(move || {
64-
let service = service.clone();
65-
async move { service.notifications().await } // Result<Vec<Notification>, StudioError>
66-
});
67-
68-
// Clone the resource value out of its guard (StudioError is Clone), mapping
69-
// the Ok payload to the capability-filtered list so `Empty` reflects what THIS
70-
// connection sees — then derive every UI state from one `AsyncState`. This is
71-
// the same `from_value` mapping the unit tests exercise; the render path and
72-
// the tests share one code path.
73-
let state: AsyncState<Vec<Notification>> = {
74-
let mapped = feed
75-
.read()
76-
.clone()
77-
.map(|res| res.map(|list| visible(&list[..], &caps).cloned().collect::<Vec<_>>()));
78-
AsyncState::from_value(mapped)
79-
}; // guard dropped here — nothing held across the render below
56+
// Project the shared store through capability filtering into an owned view
57+
// state: `Empty` re-derives from the filtered list, so it reflects what THIS
58+
// connection sees. The read guard is dropped at the end of this statement —
59+
// nothing is held across the render below.
60+
let view: AsyncState<Vec<Notification>> = store
61+
.read()
62+
.project(|raw| visible(raw, &caps).cloned().collect());
8063

8164
// Build the grouped list ONLY for the Loaded case (preserves first-seen order).
8265
let mut groups: Vec<(String, Vec<Notification>)> = Vec::new();
8366
let mut unread = 0usize;
84-
if let Some(items) = state.loaded() {
67+
if let Some(items) = view.loaded() {
8568
unread = items.iter().filter(|n| n.unread).count();
8669
for n in items {
8770
match groups.iter_mut().find(|(g, _)| g == &n.group) {
@@ -103,21 +86,23 @@ pub fn NotificationPopover() -> Element {
10386
h4 { "Notifications " span { class: "count", "{count_label}" } }
10487
button {
10588
onclick: move |_| {
106-
// Mark-all-read mutates the GLOBAL signal so the topbar badge updates.
107-
for n in notifs.write().iter_mut() { n.unread = false; }
89+
// Mutate the shared store; the badge re-derives from it.
90+
if let Some(items) = store.write().loaded_mut() {
91+
mark_all_read(items);
92+
}
10893
},
10994
"Mark all read"
11095
}
11196
}
11297
div { class: "notif-list",
11398
// Loading / Empty / Error -> shared AsyncView, driven by AsyncState.
11499
AsyncView {
115-
loading: state.is_loading(),
116-
empty: state.is_empty(),
117-
error: state.error_message(),
118-
retriable: state.is_retriable(),
100+
loading: view.is_loading(),
101+
empty: view.is_empty(),
102+
error: view.error_message(),
103+
retriable: view.is_retriable(),
119104
empty_message: "No notifications for this connection".to_string(),
120-
on_retry: move |_| feed.restart(),
105+
on_retry: move |_| reload.restart(),
121106
}
122107
// Loaded -> the existing grouped list markup.
123108
if !groups.is_empty() {
@@ -133,8 +118,8 @@ pub fn NotificationPopover() -> Element {
133118
key: "{id}",
134119
class: "{item_class}",
135120
onclick: move |_| {
136-
for x in notifs.write().iter_mut() {
137-
if x.id == id { x.unread = false; }
121+
if let Some(items) = store.write().loaded_mut() {
122+
mark_read(items, &id);
138123
}
139124
popover.set(None);
140125
nav.push(route.clone());

0 commit comments

Comments
 (0)