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

[Dominik Csapak]

On 3/11/20 11:46 AM, Thomas Lamprecht wrote:
> 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(',');

that is exactly what does not work because
params is now a reference to the Extraparams of the proxy (which
we do not want to modify)

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

yeah this is better

> 
> 
> 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),
>>
>