Re: [pdm-devel] [PATCH datacenter-manager] fix #6797: add dialog to view remote subscriptions

["Shan Shaji" <s.shaji@proxmox.com>]

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 <s.shaji@proxmox.com>
>> ---
>>  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<Vec<RemoteSubscriptions>, Error>),
>> +    OpenSubscriptionsDialog,
>> +    CloseSubscriptionsDialog,
>> +}
>> +
>>  struct PdmSubscriptionInfo {
>>      status: Vec<RemoteSubscriptions>,
>>      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<Vec<RemoteSubscriptions>, 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<Self>, 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<Dialog>`. you can then simply pass that to
>   with_optional_child (or at least simplify the boolean statement below)

makes sense. Will add a `Option<Dialog>` field. 

>> -            Err(_) => self.status = Vec::new(),
>>          }
>> -
>> -        self.loading = false;
>> -
>> -        true
>>      }
>>
>> -    fn view(&self, _ctx: &yew::Context<Self>) -> yew::Html {
>> +    fn view(&self, ctx: &yew::Context<Self>) -> 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<Self>) -> 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<SubscriptionInfo> for VNode {
>>      fn from(val: SubscriptionInfo) -> Self {
>>          let comp = VComp::new::<PdmSubscriptionInfo>(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<RemoteSubscriptions>,
>> +}
>> +
>> +impl SubscriptionsList {
>> +    pub fn new(subscriptions: Vec<RemoteSubscriptions>) -> Self {
>> +        yew::props!(Self { subscriptions })
>> +    }
>> +}
>> +
>> +pub struct PdmSubscriptionsList {
>> +    store: TreeStore<SubscriptionTreeEntry>,
>> +}
>> +
>> +#[derive(Clone, PartialEq)]
>> +struct RemoteEntry {
>> +    name: String,
>> +    state: RemoteSubscriptionState,
>> +    error: Option<String>,
>> +}
>> +
>> +#[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>) -> 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<Self>) -> Html {
>> +        DataTable::new(columns(self.store.clone()), self.store.clone())
>> +            .class(Overflow::Auto)
>> +            .into()
>> +    }
>> +}
>> +
>> +fn columns(
>> +    store: TreeStore<SubscriptionTreeEntry>,
>> +) -> Rc<Vec<DataTableHeader<SubscriptionTreeEntry>>> {
>> +    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! { <span class="pwt-font-label-small">{error}</span> }.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<RemoteSubscriptions> {
>> +    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<SubscriptionsList> for VNode {
>> +    fn from(val: SubscriptionsList) -> Self {
>> +        let comp = VComp::new::<PdmSubscriptionsList>(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