Re: [pbs-devel] [PATCH v2 proxmox-backup 1/2] fix: ui: sync job: edit rate limit based on sync direction

[Dominik Csapak <d.csapak@proxmox.com>]

Looks good to me now, if we want to have a unified parameter here,
we can still add it later too.

Reviewed-by: Dominik Csapak <d.csapak@proxmox.com>
Tested-by: Dominik Csapak <d.csapak@proxmox.com>

On 3/18/25 10:47, Christian Ebner wrote:
> Commit 9aa213b8 ("ui: sync job: adapt edit window to be used for pull
> and push") adapted the sync job edit so jobs in both, push and pull
> can be edited using the same window. This however did not include the
> switching of the direction to which the http client rate limit is
> applied to.
> 
> Fix this by further adding the edit field for `rate-out` and
> conditionally hide the less useful rate limit direction (rate-out for
> pull and rate-in for push). This allows to preserve the values if
> explicitly set via the sync job config.
> 
> Reported in the community forum:
> https://forum.proxmox.com/threads/163414/
> 
> Fixes: 9aa213b8 ("ui: sync job: adapt edit window to be used for pull and push")
> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
> ---
> changes since version 1 (thanks at Dominik for feedback):
> - add additional rate limit field, but hide less useful one based on
>    sync direction
> - preserve values of unchanged rate limit for less useful direction if
>    set via config.
> 
>   www/window/SyncJobEdit.js | 39 +++++++++++++++++++++++++++++++++------
>   1 file changed, 33 insertions(+), 6 deletions(-)
> 
> diff --git a/www/window/SyncJobEdit.js b/www/window/SyncJobEdit.js
> index bcd2f2fb2..683babaf2 100644
> --- a/www/window/SyncJobEdit.js
> +++ b/www/window/SyncJobEdit.js
> @@ -72,9 +72,16 @@ Ext.define('PBS.window.SyncJobEdit', {
>   
>   	init: function() {
>   	    let view = this.getView();
> -	    if (view.syncDirectionPush && view.datastore !== undefined) {
> -		let localNs = view.down('pbsNamespaceSelector[name=ns]').getValue();
> -		view.down('pbsGroupFilter').setLocalNamespace(view.datastore, localNs);
> +	    // Cannot use cbind to hide rate limit field depending on sync direction,
> +	    // set it in init as workaround.
> +	    if (view.syncDirectionPush) {
> +		view.down('pmxBandwidthField[name=rate-in]').setHidden(true);
> +		if (view.datastore !== undefined) {
> +		    let localNs = view.down('pbsNamespaceSelector[name=ns]').getValue();
> +		    view.down('pbsGroupFilter').setLocalNamespace(view.datastore, localNs);
> +		}
> +	    } else {
> +		view.down('pmxBandwidthField[name=rate-out]').setHidden(true);
>   	    }
>   	},
>       },
> @@ -89,6 +96,15 @@ Ext.define('PBS.window.SyncJobEdit', {
>   	me.callParent([values]);
>       },
>   
> +    getVisibleRateLimitField: function() {
> +	let me = this;
> +	if (me.syncDirectionPush) {
> +	    return me.down('field[name=rate-out]');
> +	} else {
> +	    return me.down('field[name=rate-in]');
> +	}
> +    },
> +
>       items: {
>   	xtype: 'tabpanel',
>   	bodyPadding: 10,
> @@ -105,6 +121,7 @@ Ext.define('PBS.window.SyncJobEdit', {
>   		    }
>   		    if (!me.isCreate) {
>   			PBS.Utils.delete_if_default(values, 'rate-in');
> +			PBS.Utils.delete_if_default(values, 'rate-out');
>   			PBS.Utils.delete_if_default(values, 'remote');
>   			if (typeof values.delete === 'string') {
>   			    values.delete = values.delete.split(',');
> @@ -189,7 +206,17 @@ Ext.define('PBS.window.SyncJobEdit', {
>   			fieldLabel: gettext('Rate Limit'),
>   			emptyText: gettext('Unlimited'),
>   			submitAutoScaledSizeUnit: true,
> -			// NOTE: handle deleteEmpty in onGetValues due to bandwidth field having a cbind too
> +			// NOTE: handle deleteEmpty in onGetValues due to bandwidth field having a cbind too,
> +			// further hide rate limit field depending on sync direction in controller init.
> +		    },
> +		    {
> +			xtype: 'pmxBandwidthField',
> +			name: 'rate-out',
> +			fieldLabel: gettext('Rate Limit'),
> +			emptyText: gettext('Unlimited'),
> +			submitAutoScaledSizeUnit: true,
> +			// NOTE: handle deleteEmpty in onGetValues due to bandwidth field having a cbind too,
> +			// further hide rate limit field depending on sync direction in controller init.
>   		    },
>   		],
>   
> @@ -221,7 +248,7 @@ Ext.define('PBS.window.SyncJobEdit', {
>   				let me = this;
>   				let form = me.up('pbsSyncJobEdit');
>   				let nsField = form.down('field[name=remote-ns]');
> -				let rateLimitField = form.down('field[name=rate-in]');
> +				let rateLimitField = form.getVisibleRateLimitField();
>   				let remoteField = form.down('field[name=remote]');
>   				let storeField = form.down('field[name=remote-store]');
>   
> @@ -263,7 +290,7 @@ Ext.define('PBS.window.SyncJobEdit', {
>   				let me = this;
>   				let remoteStoreField = me.up('pbsSyncJobEdit').down('field[name=remote-store]');
>   				remoteStoreField.setRemote(value);
> -				let rateLimitField = me.up('pbsSyncJobEdit').down('field[name=rate-in]');
> +				let rateLimitField = me.up('pbsSyncJobEdit').getVisibleRateLimitField();
>   				rateLimitField.setDisabled(!value);
>   				if (!value) {
>   				    rateLimitField.setValue(null);



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