public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Lukas Wagner <l.wagner@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
	Timothy Nicholson <t.nicholson@proxmox.com>
Subject: Re: [pve-devel] [PATCH manager] fix #5810: ui: show confirmation/warning dialog for sdn apply
Date: Wed, 30 Oct 2024 13:47:19 +0100	[thread overview]
Message-ID: <6c4074f9-de2e-4e62-afd9-603f8a9b3cfd@proxmox.com> (raw)
In-Reply-To: <20241030120205.85845-1-t.nicholson@proxmox.com>

On  2024-10-30 13:02, Timothy Nicholson wrote:
> Signed-off-by: Timothy Nicholson <t.nicholson@proxmox.com>
> ---
> This patch introduces a confirmation dialog for applying the SDN configuration. As stated in the bugzilla entry [1], the main point of this dialog is to warn the user that any pending network configuration changes on the node level will also be applied. If the user clicks yes, the configuration is applied as normal. A conditional warning that states which nodes have pending changes would be nice, however this information is not readily available in the frontend, meaning there would need to be an API call for each node to find pending changes. I'll leave it open to discussion whether this would be worth it.

Most of this should be part of the commit message itself, not the 'patch comment' (everything below the ---).
The patch comment is discarded by `git am` when the patch is applied. It should contain information
targeted at reviewers, such as open questions or the patch revision history. In the commit message,
which becomes part of the commit log, you put any information about the change that is relevant in the 
long term, e.g. the 'why' and 'how', so that we can understand how a line of code came to be in the
future. See [1] for more details.

Hope this clears things up! :)

[1] https://pve.proxmox.com/wiki/Developer_Documentation#Preparing_Patches

> 
> [1]: https://bugzilla.proxmox.com/show_bug.cgi?id=5810
> 
>  www/manager6/sdn/StatusView.js | 26 ++++++++++++++++++--------
>  1 file changed, 18 insertions(+), 8 deletions(-)
> 
> diff --git a/www/manager6/sdn/StatusView.js b/www/manager6/sdn/StatusView.js
> index 970aa919..46ebe69a 100644
> --- a/www/manager6/sdn/StatusView.js
> +++ b/www/manager6/sdn/StatusView.js
> @@ -41,14 +41,24 @@ Ext.define('PVE.sdn.StatusView', {
>  		{
>  		    text: gettext('Apply'),
>  		    handler: function() {
> -			Proxmox.Utils.API2Request({
> -			    url: '/cluster/sdn/',
> -			    method: 'PUT',
> -			    waitMsgTarget: me,
> -			    failure: function(response, opts) {
> -				Ext.Msg.alert(gettext('Error'), response.htmlStatus);
> -			    },
> -			});
> +			Ext.Msg.show({
> +			    title: gettext('Confirm'),
> +			    icon: Ext.Msg.WARNING,
> +			    msg: gettext('Any pending node network changes will also be applied. Proceed?'),
> +			    buttons: Ext.Msg.YESNO,
> +			    callback: function(btn) {
> +				if (btn === 'yes') {
> +				    Proxmox.Utils.API2Request({
> +					url: '/cluster/sdn/',
> +					method: 'PUT',
> +					waitMsgTarget: me,
> +					failure: function(response, opts) {
> +					    Ext.Msg.alert(gettext('Error'), response.htmlStatus);
> +					},
> +				    });
> +				}
> +			    }
> +			})
>  		    },
>  		},
>  	    ],

-- 
- Lukas


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


      reply	other threads:[~2024-10-30 12:47 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-30 12:02 Timothy Nicholson
2024-10-30 12:47 ` Lukas Wagner [this message]

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=6c4074f9-de2e-4e62-afd9-603f8a9b3cfd@proxmox.com \
    --to=l.wagner@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    --cc=t.nicholson@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