* [pve-devel] [PATCH manager] ui: backup job overview: add filter field @ 2024-09-19 14:30 Dominik Csapak 2024-09-19 15:36 ` Aaron Lauterer 0 siblings, 1 reply; 5+ messages in thread From: Dominik Csapak @ 2024-09-19 14:30 UTC (permalink / raw) To: pve-devel so that users can easily search their jobs for comments, VMID and pool names, in case there are many backup jobs. This partially addresses #5721, since one can search for vmid when they are selected directly, but not when inside a pool. Still should be a useful addition for users with many backup jobs. Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> --- www/manager6/dc/Backup.js | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/www/manager6/dc/Backup.js b/www/manager6/dc/Backup.js index 381402ca..a96eb5bf 100644 --- a/www/manager6/dc/Backup.js +++ b/www/manager6/dc/Backup.js @@ -780,6 +780,45 @@ Ext.define('PVE.dc.BackupView', { '-', run_btn, '->', + { + xtype: 'textfield', + fieldLabel: gettext('Filter'), + autoEl: { + tag: 'div', + 'data-qtip': gettext('Filters by Comment, VMID or Pool name'), + }, + triggers: { + clear: { + cls: 'pmx-clear-trigger', + weight: -1, + hidden: true, + handler: function() { + this.setValue(''); + this.getTriggers().clear.setVisible(false); + }, + }, + }, + labelAlign: 'right', + listeners: { + change: { + fn: function(search, val) { + search.getTriggers().clear.setVisible(!!val); + store.clearFilter(); + store.filterBy((record) => { + let found = false; + for (const field of ['comment', 'vmid', 'pool']) { + if (record.data[field]?.toString().includes(val)) { + found = true; + break; + } + } + return found; + }); + }, + buffer: 250, + }, + }, + }, noBackupJobInfoButton, '-', { -- 2.39.2 _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [pve-devel] [PATCH manager] ui: backup job overview: add filter field 2024-09-19 14:30 [pve-devel] [PATCH manager] ui: backup job overview: add filter field Dominik Csapak @ 2024-09-19 15:36 ` Aaron Lauterer 2024-09-20 6:09 ` Dominik Csapak 0 siblings, 1 reply; 5+ messages in thread From: Aaron Lauterer @ 2024-09-19 15:36 UTC (permalink / raw) To: Proxmox VE development discussion, Dominik Csapak gave this a quick test spin and if I clear my search, the other backup jobs that did not match don't show up anymore. So resetting seems to be somewhat broken. On a more general level, the question is, if this approach will be sufficient or if we don't want something in the direction of "show me all backup jobs that cover this specific VMID". Which will need a bit more logic. Then the question is, do we want to handle that in the backend, where we already have it, or do we handle it purely in the frontend where we will need to replicate the logic. For example, all VMIDs on a node. all VMIDs except the selected. VMIDs in resource pools, ... On 2024-09-19 16:30, Dominik Csapak wrote: > so that users can easily search their jobs for comments, VMID and pool > names, in case there are many backup jobs. > > This partially addresses #5721, since one can search for vmid when they > are selected directly, but not when inside a pool. Still should be a > useful addition for users with many backup jobs. > > Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> > --- > www/manager6/dc/Backup.js | 39 +++++++++++++++++++++++++++++++++++++++ > 1 file changed, 39 insertions(+) > > diff --git a/www/manager6/dc/Backup.js b/www/manager6/dc/Backup.js > index 381402ca..a96eb5bf 100644 > --- a/www/manager6/dc/Backup.js > +++ b/www/manager6/dc/Backup.js > @@ -780,6 +780,45 @@ Ext.define('PVE.dc.BackupView', { > '-', > run_btn, > '->', > + { > + xtype: 'textfield', > + fieldLabel: gettext('Filter'), > + autoEl: { > + tag: 'div', > + 'data-qtip': gettext('Filters by Comment, VMID or Pool name'), > + }, > + triggers: { > + clear: { > + cls: 'pmx-clear-trigger', > + weight: -1, > + hidden: true, > + handler: function() { > + this.setValue(''); > + this.getTriggers().clear.setVisible(false); > + }, > + }, > + }, > + labelAlign: 'right', > + listeners: { > + change: { > + fn: function(search, val) { > + search.getTriggers().clear.setVisible(!!val); > + store.clearFilter(); > + store.filterBy((record) => { > + let found = false; > + for (const field of ['comment', 'vmid', 'pool']) { > + if (record.data[field]?.toString().includes(val)) { > + found = true; > + break; > + } > + } > + return found; > + }); > + }, > + buffer: 250, > + }, > + }, > + }, > noBackupJobInfoButton, > '-', > { _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [pve-devel] [PATCH manager] ui: backup job overview: add filter field 2024-09-19 15:36 ` Aaron Lauterer @ 2024-09-20 6:09 ` Dominik Csapak 2024-09-20 7:22 ` Aaron Lauterer 2024-09-20 8:29 ` Thomas Lamprecht 0 siblings, 2 replies; 5+ messages in thread From: Dominik Csapak @ 2024-09-20 6:09 UTC (permalink / raw) To: Aaron Lauterer, Proxmox VE development discussion On 9/19/24 17:36, Aaron Lauterer wrote: > gave this a quick test spin and if I clear my search, the other backup jobs that did not match don't > show up anymore. So resetting seems to be somewhat broken. interesting, tested here with chrome and firefox and it works fine. what exactly did you press/put in/etc. ? > > > On a more general level, the question is, if this approach will be sufficient or if we don't want > something in the direction of "show me all backup jobs that cover this specific VMID". Which will > need a bit more logic. Then the question is, do we want to handle that in the backend, where we > already have it, or do we handle it purely in the frontend where we will need to replicate the logic. > > For example, all VMIDs on a node. all VMIDs except the selected. VMIDs in resource pools, ... Fair, as that would "really" fix the bug. Just for the record, I did decide for a 'front-end' only approach because, * less api calls/parameters to maintain * it probably solves most use cases (and searching inside pools/etc. could also be done here) If we'd want to have a more comprehensive vmid <-> backup job lookup, i'd probably put it into the VM backup panel itself, or maybe as an addition to this filter as an explicit way to search for a vmid no hard feelings either way though :) > > On 2024-09-19 16:30, Dominik Csapak wrote: >> so that users can easily search their jobs for comments, VMID and pool >> names, in case there are many backup jobs. >> >> This partially addresses #5721, since one can search for vmid when they >> are selected directly, but not when inside a pool. Still should be a >> useful addition for users with many backup jobs. >> >> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> >> --- >> www/manager6/dc/Backup.js | 39 +++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 39 insertions(+) >> >> diff --git a/www/manager6/dc/Backup.js b/www/manager6/dc/Backup.js >> index 381402ca..a96eb5bf 100644 >> --- a/www/manager6/dc/Backup.js >> +++ b/www/manager6/dc/Backup.js >> @@ -780,6 +780,45 @@ Ext.define('PVE.dc.BackupView', { >> '-', >> run_btn, >> '->', >> + { >> + xtype: 'textfield', >> + fieldLabel: gettext('Filter'), >> + autoEl: { >> + tag: 'div', >> + 'data-qtip': gettext('Filters by Comment, VMID or Pool name'), >> + }, >> + triggers: { >> + clear: { >> + cls: 'pmx-clear-trigger', >> + weight: -1, >> + hidden: true, >> + handler: function() { >> + this.setValue(''); >> + this.getTriggers().clear.setVisible(false); >> + }, >> + }, >> + }, >> + labelAlign: 'right', >> + listeners: { >> + change: { >> + fn: function(search, val) { >> + search.getTriggers().clear.setVisible(!!val); >> + store.clearFilter(); >> + store.filterBy((record) => { >> + let found = false; >> + for (const field of ['comment', 'vmid', 'pool']) { >> + if (record.data[field]?.toString().includes(val)) { >> + found = true; >> + break; >> + } >> + } >> + return found; >> + }); >> + }, >> + buffer: 250, >> + }, >> + }, >> + }, >> noBackupJobInfoButton, >> '-', >> { > _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [pve-devel] [PATCH manager] ui: backup job overview: add filter field 2024-09-20 6:09 ` Dominik Csapak @ 2024-09-20 7:22 ` Aaron Lauterer 2024-09-20 8:29 ` Thomas Lamprecht 1 sibling, 0 replies; 5+ messages in thread From: Aaron Lauterer @ 2024-09-20 7:22 UTC (permalink / raw) To: Dominik Csapak, Proxmox VE development discussion after a quick debugging session we realized that if a backup job has no selection, we need to handle that. This is not a normal situation and I most likely had these after deleting the VMs and removing them from the job config. Inline where the fix can be placed: On 2024-09-20 08:09, Dominik Csapak wrote: > On 9/19/24 17:36, Aaron Lauterer wrote: >> gave this a quick test spin and if I clear my search, the other backup >> jobs that did not match don't show up anymore. So resetting seems to >> be somewhat broken. > > interesting, tested here with chrome and firefox and it works fine. > what exactly did you press/put in/etc. ? > >> >> >> On a more general level, the question is, if this approach will be >> sufficient or if we don't want something in the direction of "show me >> all backup jobs that cover this specific VMID". Which will need a bit >> more logic. Then the question is, do we want to handle that in the >> backend, where we already have it, or do we handle it purely in the >> frontend where we will need to replicate the logic. >> >> For example, all VMIDs on a node. all VMIDs except the selected. VMIDs >> in resource pools, ... > > Fair, as that would "really" fix the bug. Just for the record, I did decide > for a 'front-end' only approach because, > > * less api calls/parameters to maintain > * it probably solves most use cases (and searching inside pools/etc. > could also be done here) > > If we'd want to have a more comprehensive vmid <-> backup job lookup, > i'd probably put it into the VM backup panel itself, or maybe as an > addition to this filter > as an explicit way to search for a vmid > > no hard feelings either way though :) > >> >> On 2024-09-19 16:30, Dominik Csapak wrote: >>> so that users can easily search their jobs for comments, VMID and pool >>> names, in case there are many backup jobs. >>> >>> This partially addresses #5721, since one can search for vmid when they >>> are selected directly, but not when inside a pool. Still should be a >>> useful addition for users with many backup jobs. >>> >>> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> >>> --- >>> www/manager6/dc/Backup.js | 39 +++++++++++++++++++++++++++++++++++++++ >>> 1 file changed, 39 insertions(+) >>> >>> diff --git a/www/manager6/dc/Backup.js b/www/manager6/dc/Backup.js >>> index 381402ca..a96eb5bf 100644 >>> --- a/www/manager6/dc/Backup.js >>> +++ b/www/manager6/dc/Backup.js >>> @@ -780,6 +780,45 @@ Ext.define('PVE.dc.BackupView', { >>> '-', >>> run_btn, >>> '->', >>> + { >>> + xtype: 'textfield', >>> + fieldLabel: gettext('Filter'), >>> + autoEl: { >>> + tag: 'div', >>> + 'data-qtip': gettext('Filters by Comment, VMID or >>> Pool name'), >>> + }, >>> + triggers: { >>> + clear: { >>> + cls: 'pmx-clear-trigger', >>> + weight: -1, >>> + hidden: true, >>> + handler: function() { >>> + this.setValue(''); >>> + this.getTriggers().clear.setVisible(false); >>> + }, >>> + }, >>> + }, >>> + labelAlign: 'right', >>> + listeners: { >>> + change: { >>> + fn: function(search, val) { >>> + search.getTriggers().clear.setVisible(!!val); >>> + store.clearFilter(); here a if (!val) { return; } will fix the issue >>> + store.filterBy((record) => { >>> + let found = false; >>> + for (const field of ['comment', 'vmid', 'pool']) { >>> + if >>> (record.data[field]?.toString().includes(val)) { >>> + found = true; >>> + break; >>> + } >>> + } >>> + return found; >>> + }); >>> + }, >>> + buffer: 250, >>> + }, >>> + }, >>> + }, >>> noBackupJobInfoButton, >>> '-', >>> { >> > > _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [pve-devel] [PATCH manager] ui: backup job overview: add filter field 2024-09-20 6:09 ` Dominik Csapak 2024-09-20 7:22 ` Aaron Lauterer @ 2024-09-20 8:29 ` Thomas Lamprecht 1 sibling, 0 replies; 5+ messages in thread From: Thomas Lamprecht @ 2024-09-20 8:29 UTC (permalink / raw) To: Proxmox VE development discussion, Dominik Csapak, Aaron Lauterer Am 20/09/2024 um 08:09 schrieb Dominik Csapak: >> For example, all VMIDs on a node. all VMIDs except the selected. VMIDs in resource pools, ... > Fair, as that would "really" fix the bug. Just for the record, I did decide > for a 'front-end' only approach because, > > * less api calls/parameters to maintain > * it probably solves most use cases (and searching inside pools/etc. could also be done here) > > If we'd want to have a more comprehensive vmid <-> backup job lookup, > i'd probably put it into the VM backup panel itself, or maybe as an addition to this filter > as an explicit way to search for a vmid > > no hard feelings either way though 🙂 FWIW, with the general job system it could be also an option to make a generic job filter API where each job plugin can implement a method to decide what/how to search for. That way we could also create a "show backup jobs that guest is included in" at the virtual guest's backup panel, or even a notice that it isn't included in any (could be also done as a separate guest "health check" window to avoid the cost of querying this every time the guest is visited in the UI). Anyhow, as while I understand why you went for this approach, it surely is simpler and quicker to implement, I think that users could be confused by not seeing a job for a VMID because it's not explicitly listed. Especially once we get a tag-based selection, which would be great to have soonish, I think that manual VMID selection will become less popular that it is now already, as it's easier to re-use existing groupings compared to having to update jobs on every guest creation/destruction. One possibility could be to add this without VMIDs for now, albeit that's naturally not ideal as the prime use case is checking in which job, if at all, a VMID is included. In summary, I think it's better to go for a "complete" solution here, even if it means that we have to add some new API endpoints, if an evaluation shows that the generic job-filter API is feasible, adding that endpoint might give us a quite good ROI there though. If it doesn't seems to be easily possible we still could go for a purely frontend based solution. _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-09-20 8:29 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-09-19 14:30 [pve-devel] [PATCH manager] ui: backup job overview: add filter field Dominik Csapak 2024-09-19 15:36 ` Aaron Lauterer 2024-09-20 6:09 ` Dominik Csapak 2024-09-20 7:22 ` Aaron Lauterer 2024-09-20 8:29 ` Thomas Lamprecht
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox