From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id DA9FA1FF17A for ; Tue, 9 Dec 2025 14:37:49 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 50D6320BE; Tue, 9 Dec 2025 14:38:29 +0100 (CET) Date: Tue, 09 Dec 2025 14:38:24 +0100 Message-Id: From: =?utf-8?q?Michael_K=C3=B6ppl?= To: "Proxmox Datacenter Manager development discussion" Cc: "pdm-devel" Mime-Version: 1.0 X-Mailer: aerc 0.21.0 References: <20251205180447.441371-1-s.shaji@proxmox.com> <20251205180447.441371-6-s.shaji@proxmox.com> In-Reply-To: <20251205180447.441371-6-s.shaji@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1765287498813 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.037 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 SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pdm-devel] [PATCH datacenter-manager 5/5] fix: ui: add remove confirmation dialog with optional token deletion 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 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pdm-devel-bounces@lists.proxmox.com Sender: "pdm-devel" 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 > --- > 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, 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, 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) -> 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 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>, > + > + /// A callback that will trigger if the user dismisses the action. > + #[prop_or_default] > + #[builder_cb(IntoEventCallback, into_event_callback, ())] > + pub on_dismiss: Option>, > +} > + > +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 { > + delete_token: false, > + } > + } > + > + fn update(&mut self, _ctx: &Context, msg: Self::Message) -> bool { > + match msg { > + Msg::SelectCheckBox(v) => { > + self.delete_token = v; > + true > + } > + } > + } > + > + fn view(&self, ctx: &Context) -> 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 for VNode { > + fn from(val: RemoveRemote) -> Self { > + let comp = VComp::new::(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