From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 540FA1FF183 for ; Wed, 22 Oct 2025 15:18:39 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 5F39B18CBA; Wed, 22 Oct 2025 15:19:05 +0200 (CEST) Mime-Version: 1.0 Date: Wed, 22 Oct 2025 15:18:30 +0200 Message-Id: From: "Shan Shaji" To: "Shannon Sterz" X-Mailer: aerc 0.20.0 References: <20251022114655.237125-1-s.shaji@proxmox.com> In-Reply-To: X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1761139103636 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.110 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [mod.rs] Subject: Re: [pdm-devel] [PATCH datacenter-manager] fix #6797: add dialog to view remote subscriptions X-BeenThere: pdm-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Datacenter Manager development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox Datacenter Manager development discussion Cc: Proxmox Datacenter Manager development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pdm-devel-bounces@lists.proxmox.com Sender: "pdm-devel" On Wed Oct 22, 2025 at 2:25 PM CEST, Shannon Sterz wrote: > On Wed Oct 22, 2025 at 1:46 PM CEST, Shan Shaji wrote: >> When clicking on the subscription status panel, no details appeared that >> will help the user to see the subscription details of remote/nodes. For >> example if a remote had a "mixed" subscription state, users couldn't see >> which remote had that state. >> >> Fixed the issue by adding a subscription dialog. Now, when the users >> click the subscription status panel, a dialog opens showing the remote >> subscription state and the individual subscriptions for each node. >> >> Signed-off-by: Shan Shaji >> --- >> ui/src/dashboard/mod.rs | 3 + >> ui/src/dashboard/subscription_info.rs | 81 ++++++++-- >> ui/src/dashboard/subscriptions_list.rs | 206 +++++++++++++++++++++++++ >> 3 files changed, 275 insertions(+), 15 deletions(-) >> create mode 100644 ui/src/dashboard/subscriptions_list.rs >> >> diff --git a/ui/src/dashboard/mod.rs b/ui/src/dashboard/mod.rs >> index f54c509..ec5df9b 100644 >> --- a/ui/src/dashboard/mod.rs >> +++ b/ui/src/dashboard/mod.rs >> @@ -42,6 +42,9 @@ pub use top_entities::TopEntities; >> mod subscription_info; >> pub use subscription_info::SubscriptionInfo; >> >> +mod subscriptions_list; >> +pub use subscriptions_list::SubscriptionsList; >> + >> mod remote_panel; >> use remote_panel::RemotePanel; >> >> diff --git a/ui/src/dashboard/subscription_info.rs b/ui/src/dashboard/subscription_info.rs >> index 08658d1..a1d2305 100644 >> --- a/ui/src/dashboard/subscription_info.rs >> +++ b/ui/src/dashboard/subscription_info.rs >> @@ -1,3 +1,8 @@ >> +use core::{ >> + clone::Clone, >> + convert::{From, Into}, >> + option::Option::None, >> +}; > > nit, in edition 2021 none of these need specific `use` statements. you > can just remove this. > > side note: you may want to check your lsp/editor configuration to fix > this. the packaged rust-analyzer never adds these for me. Thank you! will update it. >> use std::rc::Rc; >> >> use anyhow::Error; >> @@ -12,14 +17,16 @@ use pwt::{ >> css::{AlignItems, FlexFit, JustifyContent, TextAlign}, >> prelude::tr, >> props::{ >> - ContainerBuilder, CssBorderBuilder, CssPaddingBuilder, WidgetBuilder, WidgetStyleBuilder, >> + ContainerBuilder, CssBorderBuilder, CssPaddingBuilder, EventSubscriber, WidgetBuilder, >> + WidgetStyleBuilder, >> }, >> - widget::{Column, Container, Fa, Panel, Row}, >> + widget::{Column, Container, Dialog, Fa, Panel, Row}, >> AsyncPool, >> }; >> >> use pdm_api_types::subscription::{RemoteSubscriptionState, RemoteSubscriptions}; >> >> +use super::SubscriptionsList; >> #[derive(Properties, PartialEq)] >> pub struct SubscriptionInfo {} >> >> @@ -29,10 +36,17 @@ impl SubscriptionInfo { >> } >> } >> >> +enum Msg { >> + LoadResults(Result, Error>), >> + OpenSubscriptionsDialog, >> + CloseSubscriptionsDialog, >> +} >> + >> struct PdmSubscriptionInfo { >> status: Vec, >> loading: bool, >> _async_pool: AsyncPool, >> + dialog_open: bool, >> } >> >> fn render_subscription_status(subs: &[RemoteSubscriptions]) -> Row { >> @@ -100,7 +114,7 @@ fn render_subscription_status(subs: &[RemoteSubscriptions]) -> Row { >> } >> >> impl Component for PdmSubscriptionInfo { >> - type Message = Result, Error>; >> + type Message = Msg; >> >> type Properties = SubscriptionInfo; >> >> @@ -108,31 +122,40 @@ impl Component for PdmSubscriptionInfo { >> let link = ctx.link().clone(); >> let mut _async_pool = AsyncPool::new(); >> _async_pool.spawn(async move { >> - let result = http_get("/resources/subscription", None).await; >> - link.send_message(result); >> + let result = http_get("/resources/subscription?verbose=true", None).await; >> + link.send_message(Msg::LoadResults(result)); >> }); >> >> Self { >> status: Vec::new(), >> loading: true, >> _async_pool, >> + dialog_open: false, >> } >> } >> >> fn update(&mut self, _ctx: &yew::Context, msg: Self::Message) -> bool { >> match msg { >> - Ok(result) => { >> - self.status = result; >> + Msg::LoadResults(result) => { >> + self.status = match result { >> + Ok(result) => result, >> + Err(_) => Vec::new(), >> + }; >> + self.loading = false; >> + true >> + } >> + Msg::OpenSubscriptionsDialog => { >> + self.dialog_open = true; >> + true >> + } >> + Msg::CloseSubscriptionsDialog => { >> + self.dialog_open = false; >> + true >> } > > this is a bit of a nit, but imo this is very verbose. imo you could get > away with either using: > > - a single message that toggles this boolean > - a message that takes a dialog as a parameter and a field in the > component that is `Option`. you can then simply pass that to > with_optional_child (or at least simplify the boolean statement below) makes sense. Will add a `Option` field. >> - Err(_) => self.status = Vec::new(), >> } >> - >> - self.loading = false; >> - >> - true >> } >> >> - fn view(&self, _ctx: &yew::Context) -> yew::Html { >> + fn view(&self, ctx: &yew::Context) -> yew::Html { >> let title: Html = Row::new() >> .class(AlignItems::Center) >> .gap(2) >> @@ -140,12 +163,23 @@ impl Component for PdmSubscriptionInfo { >> .with_child(tr!("Subscription Status")) >> .into(); >> >> - Panel::new() >> + let panel = Panel::new() >> .flex(1.0) >> .width(500) >> .min_height(150) >> .title(title) >> - .border(true) >> + .border(true); >> + >> + let is_panel_clickable = !self.loading && !self.status.is_empty(); >> + let panel = if is_panel_clickable { >> + panel >> + .onclick(ctx.link().callback(|_| Msg::OpenSubscriptionsDialog)) >> + .style("cursor", "pointer") >> + } else { >> + panel >> + }; >> + >> + panel >> .with_child( >> Column::new() >> .class(FlexFit) >> @@ -157,12 +191,29 @@ impl Component for PdmSubscriptionInfo { >> ) >> .with_optional_child( >> (!self.loading).then_some(render_subscription_status(&self.status)), >> + ) >> + .with_optional_child( >> + (self.dialog_open && is_panel_clickable).then_some(self.dialog_view(ctx)), >> ), >> ) >> .into() >> } >> } >> >> +impl PdmSubscriptionInfo { >> + fn dialog_view(&self, ctx: &yew::Context) -> Html { >> + Dialog::new(tr!("Your Subscriptions")) >> + .resizable(true) >> + .width(500) >> + .height(400) >> + .min_width(200) >> + .min_height(50) >> + .with_child(SubscriptionsList::new(self.status.clone())) >> + .on_close(ctx.link().callback(|_| Msg::CloseSubscriptionsDialog)) >> + .into() >> + } >> +} >> + >> impl From for VNode { >> fn from(val: SubscriptionInfo) -> Self { >> let comp = VComp::new::(Rc::new(val), None); >> diff --git a/ui/src/dashboard/subscriptions_list.rs b/ui/src/dashboard/subscriptions_list.rs >> new file mode 100644 >> index 0000000..f7af974 >> --- /dev/null >> +++ b/ui/src/dashboard/subscriptions_list.rs >> @@ -0,0 +1,206 @@ >> +use pdm_api_types::subscription::{ >> + RemoteSubscriptionState, RemoteSubscriptions, SubscriptionLevel, >> +}; >> +use proxmox_yew_comp::Status; >> +use pwt::{ >> + css::{AlignItems, Overflow}, >> + props::{ContainerBuilder, ExtractPrimaryKey, WidgetBuilder}, >> + state::{KeyedSlabTree, TreeStore}, >> + tr, >> + widget::{ >> + data_table::{DataTable, DataTableColumn, DataTableHeader}, >> + Fa, Row, >> + }, >> +}; >> +use yew::{ >> + html, >> + virtual_dom::{Key, VComp, VNode}, >> + Html, >> +}; >> + > > nit: we generally prefer module level imports and also group and order > them in this way: > > std -> generic imports (serde, anyhow, yew) -> proxmox* crates -> > project crates Thank you for explaining it. makes sense, will update it. >> +use core::{clone::Clone, cmp::Ord}; > > nit: this doesn't need to be `use`d > >> +use std::rc::Rc; >> +use yew::{Component, Properties}; >> + >> +#[derive(Properties, PartialEq)] >> +pub struct SubscriptionsList { >> + subscriptions: Vec, >> +} >> + >> +impl SubscriptionsList { >> + pub fn new(subscriptions: Vec) -> Self { >> + yew::props!(Self { subscriptions }) >> + } >> +} >> + >> +pub struct PdmSubscriptionsList { >> + store: TreeStore, >> +} >> + >> +#[derive(Clone, PartialEq)] >> +struct RemoteEntry { >> + name: String, >> + state: RemoteSubscriptionState, >> + error: Option, >> +} >> + >> +#[derive(Clone, PartialEq)] >> +struct NodeEntry { >> + remote: String, >> + name: String, >> + level: SubscriptionLevel, >> +} >> + >> +#[derive(Clone, PartialEq)] >> +enum SubscriptionTreeEntry { >> + Root, >> + Remote(RemoteEntry), >> + Node(NodeEntry), >> +} >> + >> +impl SubscriptionTreeEntry { >> + fn name(&self) -> &str { >> + match self { >> + SubscriptionTreeEntry::Root => "", >> + SubscriptionTreeEntry::Remote(remote_entry) => &remote_entry.name, >> + SubscriptionTreeEntry::Node(node_entry) => &node_entry.name, >> + } >> + } >> +} >> + >> +impl ExtractPrimaryKey for SubscriptionTreeEntry { >> + fn extract_key(&self) -> Key { >> + match self { >> + SubscriptionTreeEntry::Root => Key::from("root"), >> + SubscriptionTreeEntry::Remote(remote) => Key::from(format!("{}", remote.name)), >> + SubscriptionTreeEntry::Node(node) => { >> + Key::from(format!("{}/{}", node.remote, node.name)) >> + } >> + } >> + } >> +} >> + >> +impl Component for PdmSubscriptionsList { >> + type Message = (); >> + type Properties = SubscriptionsList; >> + >> + fn create(ctx: &yew::Context) -> Self { >> + let subscriptions = sort_subscriptions(&ctx.props().subscriptions); >> + >> + let store = TreeStore::new().view_root(false); >> + let mut tree = KeyedSlabTree::new(); >> + let mut root = tree.set_root(SubscriptionTreeEntry::Root); >> + root.set_expanded(true); >> + >> + for remote in subscriptions { >> + let mut remote_node = root.append(SubscriptionTreeEntry::Remote(RemoteEntry { >> + name: remote.remote.clone(), >> + state: remote.state.clone(), >> + error: remote.error.clone(), >> + })); >> + >> + if let Some(node_status) = remote.node_status.as_ref() { >> + if node_status.is_empty() { >> + continue; >> + } >> + >> + for (node_name, info) in node_status { >> + if let Some(info) = info { >> + remote_node.append(SubscriptionTreeEntry::Node(NodeEntry { >> + remote: remote.remote.clone(), >> + name: node_name.clone(), >> + level: info.level.clone(), >> + })); >> + } >> + } >> + } >> + } >> + >> + store.write().update_root_tree(tree); >> + Self { store } >> + } >> + >> + fn view(&self, _ctx: &yew::Context) -> Html { >> + DataTable::new(columns(self.store.clone()), self.store.clone()) >> + .class(Overflow::Auto) >> + .into() >> + } >> +} >> + >> +fn columns( >> + store: TreeStore, >> +) -> Rc>> { >> + let tree_column = DataTableColumn::new(tr!("Remote")) >> + .tree_column(store) >> + .render(|entry: &SubscriptionTreeEntry| { >> + let row = Row::new().class(AlignItems::Center).gap(2); >> + match entry { >> + SubscriptionTreeEntry::Remote(remote) => row.with_child(remote.name.clone()).into(), >> + SubscriptionTreeEntry::Node(node) => row >> + .with_child(Fa::new("server")) >> + .with_child(node.name.clone()) >> + .into(), >> + SubscriptionTreeEntry::Root => row.into(), >> + } >> + }) >> + .sorter(|a: &SubscriptionTreeEntry, b: &SubscriptionTreeEntry| a.name().cmp(b.name())) >> + .into(); >> + >> + let subscription_column = DataTableColumn::new(tr!("Subcription")) >> + .render(|entry: &SubscriptionTreeEntry| match entry { >> + SubscriptionTreeEntry::Node(node) => { >> + let text = match node.level { >> + SubscriptionLevel::None => "None", >> + SubscriptionLevel::Basic => "Basic", >> + SubscriptionLevel::Community => "Community", >> + SubscriptionLevel::Premium => "Premium", >> + SubscriptionLevel::Standard => "Standard", >> + SubscriptionLevel::Unknown => "Unknown", >> + }; >> + text.into() >> + } >> + SubscriptionTreeEntry::Remote(remote) => { >> + if let Some(error) = &remote.error { >> + html! { {error} }.into() >> + } else { >> + let icon = match remote.state { >> + RemoteSubscriptionState::Mixed => Fa::from(Status::Warning), >> + RemoteSubscriptionState::Active => Fa::from(Status::Success), >> + _ => Fa::from(Status::Unknown), >> + }; >> + >> + let text = match remote.state { >> + RemoteSubscriptionState::None => "None", >> + RemoteSubscriptionState::Unknown => "Unknown", >> + RemoteSubscriptionState::Mixed => "Mixed", >> + RemoteSubscriptionState::Active => "Active", >> + }; >> + >> + Row::new().gap(2).with_child(icon).with_child(text).into() >> + } >> + } >> + SubscriptionTreeEntry::Root => "".into(), >> + }) >> + .into(); >> + >> + Rc::new(vec![tree_column, subscription_column]) >> +} >> + >> +fn sort_subscriptions(subs: &[RemoteSubscriptions]) -> Vec { >> + let mut subscriptions = subs.to_vec(); >> + subscriptions.sort_by_key(|rs| match rs.state { >> + RemoteSubscriptionState::None => 0, >> + RemoteSubscriptionState::Unknown => 1, >> + RemoteSubscriptionState::Mixed => 2, >> + RemoteSubscriptionState::Active => 3, >> + }); >> + subscriptions >> +} >> + >> + >> +impl From for VNode { >> + fn from(val: SubscriptionsList) -> Self { >> + let comp = VComp::new::(Rc::new(val), None); >> + VNode::from(comp) >> + } >> +} > > other than the comments above, looks fine to me. _______________________________________________ pdm-devel mailing list pdm-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel