[pmg-devel] [PATCH pmg-gui 1/1] fix #2632: use alternative api call for user black/whitelist

[Thomas Lamprecht]

On 3/9/20 12:18 PM, Dominik Csapak wrote:
> which expects the entry as parameter instead of part of the path
> this allows all special characters to be included in the entry
> 
> in the url, some characters were problematic (e.g. '/') and not properly
> handled by the api in the backend
> 
> we use Ext.apply (which copies the entries) here instead of setting
> the value in 'params', since we get a reference to the proxy parameters
> and do not want to change those, else we would overwrite them for all
> subsequent api calls, like the reload of the list
> 
> Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
> ---
>  js/UserBlackWhiteList.js | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/js/UserBlackWhiteList.js b/js/UserBlackWhiteList.js
> index c103d54..23c4fa4 100644
> --- a/js/UserBlackWhiteList.js
> +++ b/js/UserBlackWhiteList.js
> @@ -73,11 +73,11 @@ Ext.define('PMG.UserBlackWhiteList', {
>  
>  	    var url = '/quarantine/' + me.listname + '/';
>  
> -	    url += records.map(function(rec) {
> -		return encodeURIComponent(rec.getId());
> -	    }).join(',');
> -
> -	    var params = me.getStore().getProxy().getExtraParams() || {};
> +	    let params = Ext.apply({
> +		    address: records.map((rec) => rec.getId()).join(',')
> +		},
> +		me.getStore().getProxy().getExtraParams()
> +	    );
>  

Hmm, OK but as expressed it was slightly confusing to me.. if (theoretically) extraParams
has an "address" member we would also override ours here (apply vs. applyIf)

maybe we could go for

let params = me.getStore().getProxy().getExtraParams() || {};
params.address = records.map((rec) => rec.getId()).join(',');

or 

let params = {
    address: records.map((rec) => rec.getId()).join(','),
};
Ext.applyIf(params, me.getStore().getProxy().getExtraParams());


Either way, IMO the distiction between the extra parameters and the assembly of
the always explicit "address" parameter helps me to read and understand the code
slightly faster, independent of which way it's expressed.

Rather a nit though, code sematic looks OK as is.

>  	    Proxmox.Utils.API2Request({
>  		url: url + '?' + Ext.Object.toQueryString(params),
>