From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <d.csapak@proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256)
(No client certificate requested)
by lists.proxmox.com (Postfix) with ESMTPS id 294EA81FF
for <pve-devel@lists.proxmox.com>; Wed, 30 Aug 2023 12:45:47 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id 10502338ED
for <pve-devel@lists.proxmox.com>; Wed, 30 Aug 2023 12:45:17 +0200 (CEST)
Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com
[94.136.29.106])
(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256)
(No client certificate requested)
by firstgate.proxmox.com (Proxmox) with ESMTPS
for <pve-devel@lists.proxmox.com>; Wed, 30 Aug 2023 12:45:16 +0200 (CEST)
Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1])
by proxmox-new.maurer-it.com (Proxmox) with ESMTP id CBE7A475BD;
Wed, 30 Aug 2023 12:45:15 +0200 (CEST)
Message-ID: <b8ed6f6f-d62d-4f37-9b15-65fad2a0a01f@proxmox.com>
Date: Wed, 30 Aug 2023 12:45:14 +0200
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
Thomas Lamprecht <t.lamprecht@proxmox.com>,
Filip Schauer <f.schauer@proxmox.com>
References: <20230829111625.44426-1-f.schauer@proxmox.com>
<33338d8e-6f57-4211-b6c2-3d00a57fbcb8@proxmox.com>
Content-Language: en-US
From: Dominik Csapak <d.csapak@proxmox.com>
In-Reply-To: <33338d8e-6f57-4211-b6c2-3d00a57fbcb8@proxmox.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
X-SPAM-LEVEL: Spam detection results: 0
AWL 0.011 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
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. [proxmox.com]
Subject: Re: [pve-devel] [PATCH manager] fix #4531: Fix ACME plugin edit
form not detecting changes
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
X-List-Received-Date: Wed, 30 Aug 2023 10:45:47 -0000
On 8/30/23 11:07, Thomas Lamprecht wrote:
> meta: your repo tag is wrong, this is for widget-toolkit, not manager.
>
> off topic, while checking that I saw that we still have a (FWICT unused)
> copy of the old pveACMEPluginEditor module checked in in pve-manager, we
> should probably clean that up (no need for patches, this is more efficiently
> deleted directly in git, but some confirmation that this is unused, and if
> there are others that might be dangling around too, would be good).
we actually use that still, but with a few tweaks to the pve code
it would be redundant (iow. we have to replace the window
in dc/ACMEClusterView.js)
>
>
> Am 29/08/2023 um 13:16 schrieb Filip Schauer:
>> Fix the ACME plugin edit form not detecting dirtychanges to the values
>> of textfields properly.
>
> a bit more information about would be great here, like why this solution
> was choosen over adding the missing call directly, like:
>
> me.createdFields[key].checkDirty();
>
> As the previous code and resetOriginalValue() minus the checkDirty() call
> aren't exactly the same.
>
>>
>> Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
>> ---
>> src/window/ACMEPluginEdit.js | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/window/ACMEPluginEdit.js b/src/window/ACMEPluginEdit.js
>> index 0f21527..f6d1bcd 100644
>> --- a/src/window/ACMEPluginEdit.js
>> +++ b/src/window/ACMEPluginEdit.js
>> @@ -126,7 +126,7 @@ Ext.define('Proxmox.window.ACMEPluginEdit', {
>> for (const [key, value] of Object.entries(data)) {
>> if (me.createdFields[key]) {
>> me.createdFields[key].setValue(value);
>> - me.createdFields[key].originalValue = me.originalValues[key];
>
> me.originalValues is now unused, but still set – this seems off; I find it a
> bit weird that Dominik added this handling and explicitly wrote
> "we have to properly track the original values" in the original commit
> 45708891 ("ui: add ACMEPluginEdit window") from pve-manager repo..
>
> Your code might be even all right, I did not reviewed this in depth, but if
> it would, then it should also remove the "originalValues" field with a good
> reason in the commit message why that was wrong, or why it's now unnecessary.
i was the one to recommend that, and it worked, in some situations.
after looking at the code i now remember why i did not do that.
if you edit a value when selecting one plugin, then selecting a different
one with e.g. no schema, and then switching back, the new (dirty) value
is registered as the original one...
so to fix that we have to something like doing the check manually
( like Thomas mentioned above )
>
>
>> + me.createdFields[key].resetOriginalValue();
>> } else {
>> extradata.push(`${key}=${value}`);
>> }
>
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel