public inbox for pdm-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: "Shannon Sterz" <s.sterz@proxmox.com>
To: "Shan Shaji" <s.shaji@proxmox.com>
Cc: Proxmox Datacenter Manager development discussion
	<pdm-devel@lists.proxmox.com>
Subject: Re: [pdm-devel] [PATCH datacenter-manager] fix #6797: add dialog to view remote subscriptions
Date: Wed, 22 Oct 2025 14:25:54 +0200	[thread overview]
Message-ID: <DDOULCBCHI7W.K36ON2QN0NM3@proxmox.com> (raw)
In-Reply-To: <20251022114655.237125-1-s.shaji@proxmox.com>

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.

>  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)

> -            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

> +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


  reply	other threads:[~2025-10-22 12:25 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-22 11:46 Shan Shaji
2025-10-22 12:25 ` Shannon Sterz [this message]
2025-10-22 13:18   ` Shan Shaji
2025-10-22 14:33     ` Shan Shaji

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DDOULCBCHI7W.K36ON2QN0NM3@proxmox.com \
    --to=s.sterz@proxmox.com \
    --cc=pdm-devel@lists.proxmox.com \
    --cc=s.shaji@proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal