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 5/5] fix: ui: add remove confirmation dialog with optional token deletion
Date: Tue, 09 Dec 2025 10:36:11 +0100 [thread overview]
Message-ID: <DETL1JIP2BOC.2ZWKW6WVZBWDH@proxmox.com> (raw)
In-Reply-To: <20251205180447.441371-6-s.shaji@proxmox.com>
On Fri Dec 5, 2025 at 7:04 PM CET, Shan Shaji wrote:
> Previously, removing a remote did not remove it's token, which
> 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.
>
> 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),);
> + 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))),
nit: we generally try to use `on_activate` where possible nowadays.
> )
> .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},
> +};
nit: use module level import for new components :)
> +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();
> +
> + 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
next prev parent reply other threads:[~2025-12-09 9:35 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-12-05 18:04 [pdm-devel] [PATCH datacenter-manager/proxmox 0/6] fix #6914: add option to remove already existing token Shan Shaji
2025-12-05 18:04 ` [pdm-devel] [PATCH datacenter-manager 1/5] server: pbs-client: add delete admin token method Shan Shaji
2025-12-09 9:36 ` Shannon Sterz
2025-12-05 18:04 ` [pdm-devel] [PATCH datacenter-manager 2/5] server: api: add support to optionally delete token from remote Shan Shaji
2025-12-09 9:36 ` Shannon Sterz
2025-12-09 9:54 ` Shan Shaji
2025-12-09 12:34 ` Michael Köppl
2025-12-09 13:37 ` Shan Shaji
2025-12-05 18:04 ` [pdm-devel] [PATCH datacenter-manager 3/5] pdm-client: accept `delete-token` argument for deleting api token Shan Shaji
2025-12-09 9:36 ` Shannon Sterz
2025-12-09 13:08 ` Michael Köppl
2025-12-05 18:04 ` [pdm-devel] [PATCH datacenter-manager 4/5] cli: client: add `delete-token` option to delete token from remote Shan Shaji
2025-12-09 14:52 ` Michael Köppl
2025-12-09 16:25 ` Shan Shaji
2025-12-05 18:04 ` [pdm-devel] [PATCH datacenter-manager 5/5] fix: ui: add remove confirmation dialog with optional token deletion Shan Shaji
2025-12-09 9:36 ` Shannon Sterz [this message]
2025-12-09 10:08 ` Shan Shaji
2025-12-09 13:38 ` Michael Köppl
2025-12-09 13:52 ` Shan Shaji
2025-12-05 18:04 ` [pdm-devel] [PATCH proxmox 1/1] pve-api-types: generate missing `delete_token` method Shan Shaji
2025-12-09 9:37 ` Shannon Sterz
2025-12-09 14:56 ` [pdm-devel] [PATCH datacenter-manager/proxmox 0/6] fix #6914: add option to remove already existing token Michael Köppl
2025-12-09 17:02 ` Shan Shaji
2025-12-10 16:40 ` 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=DETL1JIP2BOC.2ZWKW6WVZBWDH@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