[pve-devel] [PATCH manager v2 0/5] ACME node adaptions for plugins

[Dominik Csapak]

On 5/6/20 8:11 PM, Thomas Lamprecht wrote:
> On 5/6/20 4:31 PM, Dominik Csapak wrote:
>> this series adapts the node->certificates->acme panel to include
>> an 'accountselector' and to be able to add/edit/remove single domains,
>> including ones with a plugin
>>
>> changes from v1:
>> * drop fieldLabel in ACMEAccountSelector
>> * reword 'Account' in 'Used Account'
>> * use different approach to change the account
>>    (use viewModel and a displayfield/combobox/editbutton to better
>>    see when the account actually changes)
>>
>> Dominik Csapak (5):
>>    ui: add ACME selector formfields for account and plugins
>>    ui: Parser: add printACME
>>    ui: Utils: add helper functions for acme domains
>>    ui: node/ACME: add ACMEDomainEdit
>>    ui: node/ACME: rework ACME grid for plugin based domains
>>
>>   www/manager6/Makefile                    |   2 +
>>   www/manager6/Parser.js                   |   7 +
>>   www/manager6/Utils.js                    |  23 +
>>   www/manager6/form/ACMEAccountSelector.js |  21 +
>>   www/manager6/form/ACMEPluginSelector.js  |  19 +
>>   www/manager6/node/ACME.js                | 617 ++++++++++++++++-------
>>   6 files changed, 517 insertions(+), 172 deletions(-)
>>   create mode 100644 www/manager6/form/ACMEAccountSelector.js
>>   create mode 100644 www/manager6/form/ACMEPluginSelector.js
>>
> 
> applied series, but this wasn't much tested, or the wrong thing was sent..

hi, sorry yes should have tested more (was a bit in a hurry yesterday)

> I had to fix the acme undefined access exceptions I mentioned to you for v1
> off-list,

i actually tested this, but did not trigger it, but looking at the code
now, yes i can see where it has tripped up

> the "show button to go to account page" didn't worked at all (no code
> doing it included),

yes, sorry, after my initial tests with this, i did actually never
remove my acme accounts again....

> I fixed that but threw it out for something different
> all together.. 

looks good AFAICS

> renamed: form/ACMEAPiSelector.js -> form/ACMEAPISelector.js > (Which I also mentioned for v1..) and threw in a few smaller followups.

but this was in my other series (which you applied the day before),

anyway, i would've been happy to send a v3 (you can ofc fix up my 
patches, but no need to generate more work for yourself :) )

> 
> The store glitches still on load for the Certificates ACME view, I increased
> the interval to 10 seconds as I really did not wanted to investigate that too.

i think what you mean with 'glitch' is the loading mask on account 
verification... after looking at the thing again, i am not so sure 
anymore that we actually need that (we already have a list off accounts
in the accountselector). i'll see if i can improve that bit

> 
> The DC -> ACME ones could profit from update/diffstore to get changes more
> live, even if seldom made - they are really cheap to query.
> 

yes makes sense, i found also some other issues (backend as well)
i'll send some patches later today

thanks for fixing my bugs :)