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

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

On Tue Dec 9, 2025 at 2:38 PM CET, Michael Köppl wrote:
> 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".

Will update it. Thank you!

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

It was not intended. Will remove it. 

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

thanks will udpate it. 

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



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