From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <pbs-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
	by lore.proxmox.com (Postfix) with ESMTPS id 841B51FF166
	for <inbox@lore.proxmox.com>; Fri, 25 Oct 2024 12:39:54 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
	by firstgate.proxmox.com (Proxmox) with ESMTP id 3C72C1DCA3;
	Fri, 25 Oct 2024 12:39:54 +0200 (CEST)
Message-ID: <dc0c5cdb-04f8-4a4f-9500-2fed66fe2868@proxmox.com>
Date: Fri, 25 Oct 2024 12:39:50 +0200
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird Beta
To: Proxmox Backup Server development discussion
 <pbs-devel@lists.proxmox.com>, Christian Ebner <c.ebner@proxmox.com>
References: <20241018084242.144010-1-c.ebner@proxmox.com>
 <20241018084242.144010-22-c.ebner@proxmox.com>
Content-Language: en-US
From: Dominik Csapak <d.csapak@proxmox.com>
In-Reply-To: <20241018084242.144010-22-c.ebner@proxmox.com>
X-SPAM-LEVEL: Spam detection results:  0
 AWL 0.015 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
 RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
 Validity was blocked. See
 https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
 information.
 RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
 Validity was blocked. See
 https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
 information.
 RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
 Validity was blocked. See
 https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
 information.
 SPF_HELO_NONE           0.001 SPF: HELO does not publish an SPF Record
 SPF_PASS               -0.001 SPF: sender matches SPF record
 URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See
 http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more
 information. [params.store]
Subject: Re: [pbs-devel] [PATCH v5 proxmox-backup 21/31] ui: add view with
 separate grids for pull and push sync jobs
X-BeenThere: pbs-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox Backup Server development discussion
 <pbs-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pbs-devel>, 
 <mailto:pbs-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pbs-devel/>
List-Post: <mailto:pbs-devel@lists.proxmox.com>
List-Help: <mailto:pbs-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel>, 
 <mailto:pbs-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: Proxmox Backup Server development discussion
 <pbs-devel@lists.proxmox.com>
Content-Transfer-Encoding: 7bit
Content-Type: text/plain; charset="us-ascii"; Format="flowed"
Errors-To: pbs-devel-bounces@lists.proxmox.com
Sender: "pbs-devel" <pbs-devel-bounces@lists.proxmox.com>

one nit and one question inline

On 10/18/24 10:42, Christian Ebner wrote:
> Show sync jobs in pull and in push direction in two separate grids,
> visually separating them to limit possible misconfiguration.
> 
> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
> ---
> changes since version 4:
> - no changes
> 
> changes since version 3:
> - no changes
> 
>   www/Makefile                   |  1 +
>   www/config/SyncPullPushView.js | 60 ++++++++++++++++++++++++++++++++++
>   www/config/SyncView.js         | 11 ++++++-
>   www/datastore/DataStoreList.js |  2 +-
>   www/datastore/Panel.js         |  2 +-
>   5 files changed, 73 insertions(+), 3 deletions(-)
>   create mode 100644 www/config/SyncPullPushView.js
> 
> diff --git a/www/Makefile b/www/Makefile
> index 609a0ba67..d35e81283 100644
> --- a/www/Makefile
> +++ b/www/Makefile
> @@ -61,6 +61,7 @@ JSSRC=							\
>   	config/TrafficControlView.js			\
>   	config/ACLView.js				\
>   	config/SyncView.js				\
> +	config/SyncPullPushView.js			\
>   	config/VerifyView.js				\
>   	config/PruneView.js				\
>   	config/GCView.js				\
> diff --git a/www/config/SyncPullPushView.js b/www/config/SyncPullPushView.js
> new file mode 100644
> index 000000000..1588207c9
> --- /dev/null
> +++ b/www/config/SyncPullPushView.js
> @@ -0,0 +1,60 @@
> +Ext.define('PBS.config.SyncPullPush', {
> +    extend: 'Ext.panel.Panel',
> +    alias: 'widget.pbsSyncJobPullPushView',
> +    title: gettext('Sync Jobs'),
> +
> +    mixins: ['Proxmox.Mixin.CBind'],
> +
> +    layout: {
> +	type: 'vbox',
> +	align: 'stretch',
> +	multi: true,
> +    },
> +    defaults: {
> +	collapsible: false,
> +	margin: '7 10 3 10',

nit: not really a fan of such "specific" values (though i know we have them
all over the place...

couldn't it work to use e.g. bodyPadding: 5 on the panel and
margin: 5 to the defaults, that way the layout is such that we have

5 px bodyPadding from the top,bottom,right,left
around each child again 5px margin
which sums up to 10px on the outer borders
and 10px between childs

> +    },
> +    scrollable: true,
> +    items: [
> +	{
> +	    xtype: 'pbsSyncJobView',
> +	    itemId: 'syncJobsPull',
> +	    syncDirection: 'pull',
> +	    cbind: {
> +		datastore: '{datastore}',
> +	    },
> +	    minHeight: 125, // shows at least one line of content

just to feed my curiosity (and it's not mentioned here)
why is this height

> +	},
> +	{
> +	    xtype: 'splitter',
> +	    performCollapse: false,
> +	},
> +	{
> +	    xtype: 'pbsSyncJobView',
> +	    itemId: 'syncJobsPush',
> +	    syncDirection: 'push',
> +	    cbind: {
> +		datastore: '{datastore}',
> +	    },
> +	    flex: 1,
> +	    minHeight: 160, // shows at least one line of content

different than this height?

the columns should take the same amount of space no?
son one line should take the same height for both panels?
(or am i missing something here?)

> +	},
> +    ],
> +    initComponent: function() {
> +	let me = this;
> +
> +	let subPanelIds = me.items.map(el => el.itemId).filter(id => !!id);
> +
> +	me.callParent();
> +
> +	for (const itemId of subPanelIds) {
> +	    let component = me.getComponent(itemId);
> +	    component.relayEvents(me, ['activate', 'deactivate', 'destroy']);
> +	}
> +    },
> +
> +    cbindData: function(initialConfig) {
> +        let me = this;
> +        me.datastore = initialConfig.datastore ? initialConfig.datastore : undefined;
> +    },
> +});
> diff --git a/www/config/SyncView.js b/www/config/SyncView.js
> index 4669a23e2..68a147615 100644
> --- a/www/config/SyncView.js
> +++ b/www/config/SyncView.js
> @@ -29,7 +29,7 @@ Ext.define('PBS.config.SyncJobView', {
>       stateful: true,
>       stateId: 'grid-sync-jobs-v1',
>   
> -    title: gettext('Sync Jobs'),
> +    title: gettext('Sync Jobs - Pull Direction'),
>   
>       controller: {
>   	xclass: 'Ext.app.ViewController',
> @@ -39,6 +39,7 @@ Ext.define('PBS.config.SyncJobView', {
>   	    let view = me.getView();
>               Ext.create('PBS.window.SyncJobEdit', {
>   		datastore: view.datastore,
> +		syncDirection: view.syncDirection,
>   		listeners: {
>   		    destroy: function() {
>   			me.reload();
> @@ -55,6 +56,7 @@ Ext.define('PBS.config.SyncJobView', {
>   
>               Ext.create('PBS.window.SyncJobEdit', {
>   		datastore: view.datastore,
> +		syncDirection: view.syncDirection,
>                   id: selection[0].data.id,
>   		listeners: {
>   		    destroy: function() {
> @@ -117,6 +119,9 @@ Ext.define('PBS.config.SyncJobView', {
>   	    if (view.datastore !== undefined) {
>   		params.store = view.datastore;
>   	    }
> +	    if (view.syncDirection !== undefined) {
> +		params["sync-direction"] = view.syncDirection;
> +	    }
>   	    view.getStore().rstore.getProxy().setExtraParams(params);
>   	    Proxmox.Utils.monStoreErrors(view, view.getStore().rstore);
>   	},
> @@ -303,6 +308,10 @@ Ext.define('PBS.config.SyncJobView', {
>   	    }
>   	}
>   
> +	if (me.syncDirection === 'push') {
> +	    me.title = gettext('Sync Jobs - Push Direction');
> +	}
> +
>   	me.callParent();
>       },
>   });
> diff --git a/www/datastore/DataStoreList.js b/www/datastore/DataStoreList.js
> index fc68cfc10..22ef18540 100644
> --- a/www/datastore/DataStoreList.js
> +++ b/www/datastore/DataStoreList.js
> @@ -239,7 +239,7 @@ Ext.define('PBS.datastore.DataStores', {
>   	{
>   	    iconCls: 'fa fa-refresh',
>   	    itemId: 'syncjobs',
> -	    xtype: 'pbsSyncJobView',
> +	    xtype: 'pbsSyncJobPullPushView',
>   	},
>   	{
>   	    iconCls: 'fa fa-check-circle',
> diff --git a/www/datastore/Panel.js b/www/datastore/Panel.js
> index ad9fc10fe..e1da7cfac 100644
> --- a/www/datastore/Panel.js
> +++ b/www/datastore/Panel.js
> @@ -68,7 +68,7 @@ Ext.define('PBS.DataStorePanel', {
>   	{
>   	    iconCls: 'fa fa-refresh',
>   	    itemId: 'syncjobs',
> -	    xtype: 'pbsSyncJobView',
> +	    xtype: 'pbsSyncJobPullPushView',
>   	    cbind: {
>   		datastore: '{datastore}',
>   	    },



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