public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Thomas Lamprecht <t.lamprecht@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
	Oguz Bektas <o.bektas@proxmox.com>,
	pbs-devel@lists.proxmox.com
Subject: Re: [pve-devel] [PATCH proxmox-backup] ui: add 'join info' copy/paste button on datastore summary
Date: Thu, 29 Oct 2020 16:29:28 +0100	[thread overview]
Message-ID: <f49ec9b2-6342-9296-7d8c-7c30b5660ae2@proxmox.com> (raw)
In-Reply-To: <20201029141902.988185-1-o.bektas@proxmox.com>

On 29.10.20 15:19, Oguz Bektas wrote:
> shows information about the datastore:
>  - datastore name
>  - hostname
>  - host x509 fingerprint
> 
> and creates the base64-encoded json blob to be pasted into PVE to add
> as storage.
> 

code looks familiar, no reference from where you took it? ;-)

Did you see no way to pull common stuff out from the PVE cluster join info this
was based on to widget toolkit?

Also, where's a user for this code? I.e., the PVE Storage Add and PBS Remote
add? It's a bit hard to review if one has only half of the picture..

> Signed-off-by: Oguz Bektas <o.bektas@proxmox.com>
> ---
> 
>  www/DataStoreSummary.js     | 35 ++++++++++++++-
>  www/Makefile                |  1 +
>  www/window/DataStoreJoin.js | 88 +++++++++++++++++++++++++++++++++++++
>  3 files changed, 123 insertions(+), 1 deletion(-)
>  create mode 100644 www/window/DataStoreJoin.js
> 
> diff --git a/www/DataStoreSummary.js b/www/DataStoreSummary.js
> index 93bd99a3..616e2cf7 100644
> --- a/www/DataStoreSummary.js
> +++ b/www/DataStoreSummary.js
> @@ -208,7 +208,40 @@ Ext.define('PBS.DataStoreSummary', {
>  	padding: 5,
>      },
>  
> -    tbar: ['->', { xtype: 'proxmoxRRDTypeSelector' }],
> +    tbar: [
> +	{
> +	    xtype: 'button',
> +	    text: gettext('Show Datastore Information'),
> +	    handler: function() {
> +		let me = this;
> +		let panel = me.up('panel');
> +		Proxmox.Utils.API2Request({

Why isn't this request not done by the "joininfowindow"?

The button handler could also be a simple wrapper around creating that window,
which should do the request then on creation.

Can be OK as is, but I'd then move this out as it gets a bit crowded here.

> +		    url: '/api2/extjs/nodes/localhost/status',
> +		    method: 'GET',
> +		    waitMsgTarget: panel,
> +		    failure: function(e) {
> +			console.log(e);
> +		    },
> +		    success: function(response) {
> +			let fingerprint = response.result.data.info.fingerprint;
> +			let hostname = window.location.hostname;
> +			let dsName = panel.datastore;
> +			Ext.create('PBS.JoinInfoWindow', {
> +			    joinInfo: {
> +				ipAddress: hostname,
> +				fingerprint: fingerprint,
> +				dsName: dsName,

s/dsName/datastore/ - clear, understandable names trumps saving a few letters.

> +			    },
> +			}).show();
> +		    },
> +		});
> +	    },
> +	},
> +	'->',
> +	{
> +	    xtype: 'proxmoxRRDTypeSelector',
> +	},
> +    ],
>  
>      items: [
>  	{
> diff --git a/www/Makefile b/www/Makefile
> index cba8bed5..42fe8724 100644
> --- a/www/Makefile
> +++ b/www/Makefile
> @@ -21,6 +21,7 @@ JSSRC=							\
>  	window/BackupFileDownloader.js			\
>  	window/CreateDirectory.js			\
>  	window/DataStoreEdit.js				\
> +	window/DataStoreJoin.js				\
>  	window/FileBrowser.js				\
>  	window/NotesEdit.js				\
>  	window/RemoteEdit.js				\
> diff --git a/www/window/DataStoreJoin.js b/www/window/DataStoreJoin.js
> new file mode 100644
> index 00000000..3a1dd87f
> --- /dev/null
> +++ b/www/window/DataStoreJoin.js
> @@ -0,0 +1,88 @@
> +Ext.define('PBS.JoinInfoWindow', {

join info is the wrong name here, use DatastoreInfo or something like that, at least
if that widget stays here and not in widget toolkit


> +	extend: 'Ext.window.Window',
> +	modal: true,
> +	width: 600,
> +	title: gettext('Datastore Join Information'),
> +	mixins: ['Proxmox.Mixin.CBind'],
> +	joinInfo: {
> +	    ipAddress: undefined,
> +	    fingerprint: undefined,
> +	    dsName: undefined,
> +	},
> +	layout: 'form',
> +	bodyPadding: '10 0',
> +	items: [
> +	{
> +	    xtype: 'container',
> +	    layout: 'form',
> +	    border: false,
> +	    padding: '0 10 10 10',
> +	    items: [
> +		{
> +		    xtype: 'textfield',
> +		    fieldLabel: gettext('x509 Fingerprint'),
> +		    inputId: 'fingerprintField',
> +		    cbind: {
> +			value: '{joinInfo.fingerprint}',
> +		    },
> +		    editable: false,
> +		},
> +		{
> +		    xtype: 'textfield',
> +		    fieldLabel: gettext('Hostname'),
> +		    editable: true,
> +		    inputId: 'hostnameField',
> +		    cbind: {
> +			value: '{joinInfo.ipAddress}',
> +		    },
> +		},
> +		{
> +		    xtype: 'textfield',
> +		    fieldLabel: gettext('Datastore Name'),
> +		    inputId: 'dsNameField',
> +		    cbind: {
> +			value: '{joinInfo.dsName}',
> +		    },
> +		    editable: false,
> +		},
> +		{
> +		    xtype: 'textarea',
> +		    inputId: 'SerializedJoinInfo',
> +		    fieldLabel: gettext('Join Information'),
> +		    grow: true,
> +		    editable: false,
> +		    cbind: {
> +			joinInfo: '{joinInfo}',
> +		    },
> +		    listeners: {
> +			afterrender: function(field) {
> +			    var jsons = Ext.JSON.encode(field.joinInfo);
> +			    var base64s = Ext.util.Base64.encode(jsons);
> +			    field.setValue(base64s)
> +			},
> +		    },
> +		},
> +	    ],
> +	},
> +	],
> +	buttons: [
> +	    {
> +		xtype: 'button',
> +		iconCls: 'fa fa-clipboard',
> +		handler: function(b) {
> +		    var el = document.getElementById('SerializedJoinInfo');
> +		    el.select();
> +		    document.execCommand("copy");
> +		},
> +		text: gettext('Copy'),
> +	    },
> +	    {
> +		text: gettext('Ok'),
> +		handler: function() {
> +		    this.up('window').close();
> +		},
> +	    },
> +	],
> +});
> +
> +
> 






      reply	other threads:[~2020-10-29 15:29 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-29 14:19 Oguz Bektas
2020-10-29 15:29 ` Thomas Lamprecht [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=f49ec9b2-6342-9296-7d8c-7c30b5660ae2@proxmox.com \
    --to=t.lamprecht@proxmox.com \
    --cc=o.bektas@proxmox.com \
    --cc=pbs-devel@lists.proxmox.com \
    --cc=pve-devel@lists.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