From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 ; 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 ; 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 ; 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: Date: Wed, 30 Aug 2023 12:45:14 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: Proxmox VE development discussion , Thomas Lamprecht , Filip Schauer References: <20230829111625.44426-1-f.schauer@proxmox.com> <33338d8e-6f57-4211-b6c2-3d00a57fbcb8@proxmox.com> Content-Language: en-US From: Dominik Csapak 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-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 >> --- >> 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