Re: [pdm-devel] [PATCH datacenter-manager 5/5] fix: ui: add remove confirmation dialog with optional token deletion

["Michael Köppl" <m.koeppl@proxmox.com>]

4 comments inline

On Fri Dec 5, 2025 at 7:04 PM CET, Shan Shaji wrote:
> Previously, removing a remote did not remove it's token, which

nit: s/it\'s/its

> prevented users from re-adding the same remote later with the same token
> name. To fix it  a new checkbox option has been added to which when
> enabled the token will be deleted from the remote.

nit: was a bit difficult to grasp for me. Maybe something like "To fix
this, add a new checkbox that lets users choose to also delete the
token when deleting the remote".

>
> Signed-off-by: Shan Shaji <s.shaji@proxmox.com>
> ---
>  ui/src/remotes/config.rs        |  42 +++++++----
>  ui/src/remotes/mod.rs           |   3 +
>  ui/src/remotes/remove_remote.rs | 122 ++++++++++++++++++++++++++++++++
>  3 files changed, 154 insertions(+), 13 deletions(-)
>  create mode 100644 ui/src/remotes/remove_remote.rs
>
> diff --git a/ui/src/remotes/config.rs b/ui/src/remotes/config.rs
> index ac3c0f1..86cb3d6 100644
> --- a/ui/src/remotes/config.rs
> +++ b/ui/src/remotes/config.rs
> @@ -7,6 +7,7 @@ use anyhow::Error;
>  use proxmox_schema::property_string::PropertyString;
>  
>  use crate::remotes::edit_remote::EditRemote;
> +use crate::remotes::remove_remote::RemoveRemote;
>  //use pwt::widget::form::{Field, FormContext, InputType};
>  
>  use pdm_api_types::remotes::Remote;
> @@ -17,7 +18,7 @@ use proxmox_yew_comp::percent_encoding::percent_encode_component;
>  
>  //use proxmox_schema::api_types::{CERT_FINGERPRINT_SHA256_SCHEMA, DNS_NAME_OR_IP_SCHEMA};
>  
> -use serde_json::Value;
> +use serde_json::{json, Value};
>  use yew::virtual_dom::{Key, VComp, VNode};
>  
>  use pwt::prelude::*;
> @@ -31,9 +32,7 @@ use pwt::widget::{
>  //use pwt::widget::InputPanel;
>  
>  //use proxmox_yew_comp::EditWindow;
> -use proxmox_yew_comp::{
> -    ConfirmButton, LoadableComponent, LoadableComponentContext, LoadableComponentMaster,
> -};
> +use proxmox_yew_comp::{LoadableComponent, LoadableComponentContext, LoadableComponentMaster};
>  
>  use pdm_api_types::remotes::{NodeUrl, RemoteType};
>  
> @@ -41,10 +40,13 @@ async fn load_remotes() -> Result<Vec<Remote>, Error> {
>      proxmox_yew_comp::http_get("/remotes/remote", None).await
>  }
>  
> -async fn delete_item(key: Key) -> Result<(), Error> {
> +async fn delete_item(key: Key, delete_token: bool) -> Result<(), Error> {
>      let id = key.to_string();
> -    let url = format!("/remotes/remote/{}", percent_encode_component(&id));
> -    proxmox_yew_comp::http_delete(&url, None).await?;
> +    let param = Some(json!({
> +        "delete-token": delete_token,
> +    }));
> +    let url = format!("/remotes/remote/{}", percent_encode_component(&id),);

nit: is there a reason for this added comma at the end?

> +    proxmox_yew_comp::http_delete(&url, param).await?;
>      Ok(())
>  }
>  
> @@ -99,11 +101,12 @@ impl RemoteConfigPanel {
>  pub enum ViewState {
>      Add(RemoteType),
>      Edit,
> +    Remove,
>  }
>  
>  pub enum Msg {
>      SelectionChange,
> -    RemoveItem,
> +    RemoveItem(bool),
>  }
>  
>  pub struct PbsRemoteConfigPanel {
> @@ -146,11 +149,11 @@ impl LoadableComponent for PbsRemoteConfigPanel {
>      fn update(&mut self, ctx: &LoadableComponentContext<Self>, msg: Self::Message) -> bool {
>          match msg {
>              Msg::SelectionChange => true,
> -            Msg::RemoveItem => {
> +            Msg::RemoveItem(v) => {
>                  if let Some(key) = self.selection.selected_key() {
>                      let link = ctx.link();
>                      link.clone().spawn(async move {
> -                        if let Err(err) = delete_item(key).await {
> +                        if let Err(err) = delete_item(key, v).await {
>                              link.show_error(tr!("Unable to delete item"), err, true);
>                          }
>                          link.send_reload();
> @@ -195,10 +198,9 @@ impl LoadableComponent for PbsRemoteConfigPanel {
>                      .onclick(link.change_view_callback(|_| Some(ViewState::Edit))),
>              )
>              .with_child(
> -                ConfirmButton::new(tr!("Remove"))
> -                    .confirm_message(tr!("Are you sure you want to remove this remote?"))
> +                Button::new(tr!("Remove"))
>                      .disabled(disabled)
> -                    .on_activate(link.callback(|_| Msg::RemoveItem)),
> +                    .onclick(link.change_view_callback(|_| Some(ViewState::Remove))),
>              )
>              .with_flex_spacer()
>              .with_child({
> @@ -233,6 +235,7 @@ impl LoadableComponent for PbsRemoteConfigPanel {
>                  .selection
>                  .selected_key()
>                  .map(|key| self.create_edit_dialog(ctx, key)),
> +            ViewState::Remove => Some(self.create_remove_remote_dialog(ctx)),
>          }
>      }
>  }
> @@ -293,6 +296,19 @@ impl PbsRemoteConfigPanel {
>              .on_done(ctx.link().change_view_callback(|_| None))
>              .into()
>      }
> +
> +    fn create_remove_remote_dialog(&self, ctx: &LoadableComponentContext<Self>) -> Html {
> +        let link = ctx.link();
> +        let close = link.change_view_callback(|_| None);
> +
> +        RemoveRemote::new()
> +            .on_dismiss(close.clone())
> +            .on_confirm(Callback::from(move |v| {
> +                link.send_message(Msg::RemoveItem(v));
> +                link.change_view(None);
> +            }))
> +            .into()
> +    }
>  }
>  
>  impl From<RemoteConfigPanel> for VNode {
> diff --git a/ui/src/remotes/mod.rs b/ui/src/remotes/mod.rs
> index 603077c..6912ca9 100644
> --- a/ui/src/remotes/mod.rs
> +++ b/ui/src/remotes/mod.rs
> @@ -27,6 +27,9 @@ pub use tasks::RemoteTaskList;
>  mod updates;
>  pub use updates::UpdateTree;
>  
> +mod remove_remote;
> +pub use remove_remote::RemoveRemote;
> +
>  mod firewall;
>  pub use firewall::FirewallTree;
>  
> diff --git a/ui/src/remotes/remove_remote.rs b/ui/src/remotes/remove_remote.rs
> new file mode 100644
> index 0000000..e26d563
> --- /dev/null
> +++ b/ui/src/remotes/remove_remote.rs
> @@ -0,0 +1,122 @@
> +use std::rc::Rc;
> +
> +use yew::{
> +    html::IntoEventCallback,
> +    prelude::*,
> +    virtual_dom::{VComp, VNode},
> +};
> +
> +use pwt::{
> +    css::{AlignItems, FontColor, JustifyContent},
> +    props::{
> +        ContainerBuilder, CssPaddingBuilder, EventSubscriber, WidgetBuilder, WidgetStyleBuilder,
> +    },
> +    tr,
> +    widget::{form::Checkbox, Button, Column, Container, Dialog, Fa, Row},
> +};
> +use pwt_macros::builder;
> +
> +#[derive(PartialEq, Properties)]
> +#[builder]
> +pub struct RemoveRemote {
> +    /// A callback for an action that needs to be confirmed by the user.
> +    #[prop_or_default]
> +    #[builder_cb(IntoEventCallback, into_event_callback, bool)]
> +    pub on_confirm: Option<Callback<bool>>,
> +
> +    /// A callback that will trigger if the user dismisses the action.
> +    #[prop_or_default]
> +    #[builder_cb(IntoEventCallback, into_event_callback, ())]
> +    pub on_dismiss: Option<Callback<()>>,
> +}
> +
> +impl RemoveRemote {
> +    pub fn new() -> Self {
> +        yew::props!(Self {})
> +    }
> +}
> +
> +pub enum Msg {
> +    SelectCheckBox(bool),
> +}
> +
> +pub struct PdmRemoveRemote {
> +    delete_token: bool,
> +}
> +
> +impl Component for PdmRemoveRemote {
> +    type Message = Msg;
> +
> +    type Properties = RemoveRemote;
> +
> +    fn create(_ctx: &Context<Self>) -> Self {
> +        Self {
> +            delete_token: false,
> +        }
> +    }
> +
> +    fn update(&mut self, _ctx: &Context<Self>, msg: Self::Message) -> bool {
> +        match msg {
> +            Msg::SelectCheckBox(v) => {
> +                self.delete_token = v;
> +                true
> +            }
> +        }
> +    }
> +
> +    fn view(&self, ctx: &Context<Self>) -> Html {
> +        let props = ctx.props();
> +        let delete_token = self.delete_token.clone();

the clone() here is unnecessary

> +
> +        let on_confirm = props.on_confirm.clone();
> +        let on_dismiss = props.on_dismiss.clone();
> +
> +        Dialog::new(tr!("Confirm"))
> +            .on_close(on_dismiss.clone())
> +            .min_height(100)
> +            .with_child(
> +                Column::new()
> +                    .padding(4)
> +                    .gap(2)
> +                    .with_child(
> +                        Row::new()
> +                            .gap(2)
> +                            .class(AlignItems::Center)
> +                            .with_child(Container::new().class("pwt-message-sign").with_child(
> +                                Fa::new("exclamation-triangle").class(FontColor::Error),
> +                            ))
> +                            .with_child(tr!("Are you sure you want to remove this remote?")),
> +                    )
> +                    .with_child(
> +                        Checkbox::new()
> +                            .default(false)
> +                            .box_label(tr!("Delete API token from remote"))
> +                            .checked(self.delete_token)
> +                            .on_change(ctx.link().callback(|v| Msg::SelectCheckBox(v))),
> +                    )
> +                    .with_child(
> +                        Row::new()
> +                            .gap(2)
> +                            .class(JustifyContent::Center)
> +                            .with_child(Button::new(tr!("Yes")).onclick(move |_| {
> +                                if let Some(on_confirm) = &on_confirm {
> +                                    on_confirm.emit(delete_token);
> +                                }
> +                            }))
> +                            .with_child(Button::new(tr!("No")).onclick(move |_| {
> +                                if let Some(on_dismiss) = &on_dismiss {
> +                                    on_dismiss.emit(());
> +                                }
> +                            })),
> +                    ),
> +            )
> +            .into()
> +    }
> +}
> +
> +impl From<RemoveRemote> for VNode {
> +    fn from(val: RemoveRemote) -> Self {
> +        let comp = VComp::new::<PdmRemoveRemote>(Rc::new(val), None);
> +        VNode::from(comp)
> +    }
> +}



_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel