Nit: for the commit title, prefixing like "ui: storage:" is preferred.
I really think we should start with the tree fully expanded, or we will
get shouted at by users relying on notes to find their backups ;)
@Thomas: The backups are now ordered ascending by date again. In PBS
it's also like that. Should that be changed or do we switch back in PVE?
Am 18.03.22 um 14:52 schrieb Matthias Heiserer:
> +Ext.define('PVE.storage.BackupView', {
> + extend: 'Ext.tree.Panel',
> alias: 'widget.pveStorageBackupView',
> + mixins: ['Proxmox.Mixin.CBind'],
> + rootVisible: false,
>
> - showColumns: ['name', 'notes', 'protected', 'date', 'format', 'size'],
> + title: gettext('Content'),
>
> - initComponent: function() {
> - var me = this;
> + cbindData: function(initialCfg) {
> + this.isPBS = initialCfg.pluginType === 'pbs';
That's part of what cbind should do for you if you return { isPBS: ...
}, and that's how it should be used to avoid breaking the abstraction.
But since the storage can change (in the guest backup view) this should
rather be a normal bind and be updated there.
> + return {};
> + },
> + isStorage: false,
Doesn't seem to be used.
>
> - var nodename = me.nodename = me.pveSelNode.data.node;
> - if (!nodename) {
> - throw "no node name specified";
> - }
> + controller: {
> + xclass: 'Ext.app.ViewController',
>
> - var storage = me.storage = me.pveSelNode.data.storage;
> - if (!storage) {
> - throw "no storage ID specified";
> - }
> + groupnameHelper: function(item) {
Nit: IMHO something like getGroupName() would be a bit more readable at
the call sites.
> + if (item.vmid) {
> + return PVE.Utils.get_backup_type(item.volid, item.format) +
> `/${item.vmid}`;
> + } else {
> + return 'Other';
> + }
> + },
>
> - me.content = 'backup';
> + init: function(view) {
> + let me = this;
> + me.storage = view?.pveSelNode?.data?.storage;
Nit: here you use ?., but below you assume it's set (which I suppose we
can).
> + me.nodename = view.nodename || view.pveSelNode.data.node;
Is there ever a case where it's not the selected node?
> + me.vmid = view.pveSelNode.data.vmid;
> + me.vmtype = view.pveSelNode.data.type;
>
> - var sm = me.sm = Ext.create('Ext.selection.RowModel', {});
> + me.store = Ext.create('Ext.data.Store', {
> + model: 'PVE.storage.BackupModel',
> + });
> + me.store.on('load', me.onLoad, me);
> + view.getStore().setConfig('filterer', 'bottomup');
> + view.getStore().setSorters(['text']);
>
> - var reload = function() {
> - me.store.load();
> - };
> + if (me.vmid) {
> + me.getView().getStore().filter({
> + property: 'vmid',
> + value: me.vmid,
> + exactMatch: true,
> + });
> + } else {
> + me.lookup('storagesel').setVisible(false);
> + me.lookup('backupNowButton').setVisible(false);
> + }
> + Proxmox.Utils.monStoreErrors(view, me.store);
> + },
>
> - let pruneButton = Ext.create('Proxmox.button.Button', {
> - text: gettext('Prune group'),
> - disabled: true,
> - selModel: sm,
> - setBackupGroup: function(backup) {
> - if (backup) {
> - let name = backup.text;
> - let vmid = backup.vmid;
> - let format = backup.format;
> + onLoad: function(store, records, success, operation) {
> + let me = this;
> + let view = me.getView();
> + let selection = view.getSelection()?.[0];
> + selection = selection?.parentNode?.data?.text
> +selection?.data?.volid;
Style nit: missing space after + and could use `${...}${...}` syntax
instead.
(...)
> + if (selection) {
> + let rootnode = view.getRootNode();
> + let selected;
> + rootnode.cascade(node => {
> + if (selected) {return false;} // skip if already found
Style nit: if body on the same line
> + let id = node.parentNode?.data?.text + node.data?.volid;
> + if (id === selection) {
> + selected = node;
> + return false;
> + }
> + return true;
> });
> - win.show();
> - win.on('destroy', reload);
> - },
> - });
> + view.setSelection(selected);
> + view.getView().focusRow(selected);
After removing a backup I get a
Uncaught TypeError: node is undefined
referencing this line.
> + }
> + Proxmox.Utils.setErrorMask(view, false);
> + },
>
(...)
> +
> + removeHandler: function(button, event, rec) {
> + let me = this;
> + const volid = rec.data.volid;
> + Proxmox.Utils.API2Request({
> + url:
> `/nodes/${me.nodename}/storage/${me.storage}/content//${volid}`,
Nit: duplicate / in line above
> + method: 'DELETE',
> + callback: () => me.reload(),
> + failure: response => Ext.Msg.alert(gettext('Error'),
> response.htmlStatus),
> + });
> + },
> +
> + searchKeyupFn: function(field) {
> + let me = this;
> + me.getView().getStore().getFilters().removeByKey('volid');
> + me.getView().getStore().filter([
> + {
> + property: 'volid',
> + value: field.getValue(),
> + anyMatch: true,
> + caseSensitive: false,
> + id: 'volid',
> },
> - verification: {
> - header: gettext('Verify State'),
> - dataIndex: 'verification',
> - renderer: PVE.Utils.render_backup_verification,
> + ]);
> + },
> +
> + searchClearHandler: function(field) {
> + field.triggers.clear.setVisible(false);
> + field.setValue(this.originalValue);
> + this.getView().getStore().clearFilter();
> + },
> +
> + searchChangeFn: function(field, newValue, oldValue) {
> + if (newValue !== field.originalValue) {
> + field.triggers.clear.setVisible(true);
> + }
> + },
> +
> + storageSelectorBoxReady: function(selector, width, height, eOpts) {
> + selector.setNodename(this.nodename);
> + },
Would cbind also be an option?
> +
> + storageSelectorChange: function(self, newValue, oldValue, eOpts) {
> + let me = this;
> + me.storage = newValue;
> + me.getView().getSelectionModel().deselectAll();
> + me.reload();
> + },
> +
> + backupNowHandler: function(button, event) {
> + let me = this;
> + Ext.create('PVE.window.Backup', {
> + nodename: me.nodename,
> + vmid: me.vmid,
> + vmtype: me.vmtype,
> + storage: me.storage,
> + listeners: {
> + close: () => me.reload(),
> },
> - };
> - }
> + }).show();
> + },
> + },
> +
> + columns: [
> + {
> + xtype: 'treecolumn',
> + header: gettext("Backup Group"),
> + dataIndex: 'text',
> + renderer: function(value, _metadata, record) {
> + if (record.phantom) { return value; }
Style nit: if body on the same line. Could be avoided using ternary ? here.
> + return PVE.Utils.render_storage_content(...arguments);
> + },
> + flex: 2,
> + },
> + {
> + header: gettext('Notes'),
> + flex: 1,
> + renderer: Ext.htmlEncode,
> + dataIndex: 'notes',
> + },
> + {
> + header: `<i class="fa fa-shield"></i>`,
> + tooltip: gettext('Protected'),
> + width: 30,
> + renderer: v => v ? `<i data-qtip="${gettext('Protected')}"
> class="fa fa-shield"></i>` : '',
Style nit: line too long
> + sorter: (a, b) => (b.data.protected || 0) - (a.data.protected || 0),
> + dataIndex: 'protected',
> + },
> + {
> + header: gettext('Date'),
> + width: 150,
> + dataIndex: 'ctime',
> + xtype: 'datecolumn',
> + format: 'Y-m-d H:i:s',
> + },
> + {
> + header: gettext('Format'),
> + width: 100,
> + dataIndex: 'format',
> + },
> + {
> + header: gettext('Size'),
> + width: 100,
> + renderer: Proxmox.Utils.format_size,
> + dataIndex: 'size',
> + },
> + {
> + header: gettext('Encrypted'),
> + dataIndex: 'encrypted',
> + renderer: PVE.Utils.render_backup_encryption,
> + cbind: {
> + hidden: '{!isPBS}',
> + },
> + },
> + {
> + header: gettext('Verify State'),
> + dataIndex: 'verification',
> + renderer: PVE.Utils.render_backup_verification,
> + cbind: {
> + hidden: '{!isPBS}',
> + },
> + },
> + ],
> +
> + tbar: [
> + {
> + xtype: 'button',
> + text: gettext('Backup now'),
> + handler: 'backupNowHandler',
> + reference: 'backupNowButton',
> + },
> + {
> + xtype: 'proxmoxButton',
> + text: gettext('Restore'),
> + handler: 'restoreHandler',
> + parentXType: "treepanel",
> + disabled: true,
> + enableFn: record => record.phantom === false,
Style nit: !record.phantom is shorter
Repeated below a few times
> + },
> + {
> + xtype: 'proxmoxButton',
> + text: gettext('File Restore'),
> + handler: 'restoreFilesHandler',
> + cbind: {
> + hidden: '{!isPBS}',
> + },
> + parentXType: "treepanel",
> + disabled: true,
> + enableFn: record => record.phantom === false,
> + },
> + {
> + xtype: 'proxmoxButton',
> + text: gettext('Show Configuration'),
> + handler: 'showConfigurationHandler',
> + parentXType: "treepanel",
> + disabled: true,
> + enableFn: record => record.phantom === false,
> + },
> + {
> + xtype: 'proxmoxButton',
> + text: gettext('Edit Notes'),
> + handler: 'editNotesHandler',
> + parentXType: "treepanel",
> + disabled: true,
> + enableFn: record => record.phantom === false,
> + },
> + {
> + xtype: 'proxmoxButton',
> + text: gettext('Change Protection'),
> + handler: 'changeProtectionHandler',
> + parentXType: "treepanel",
> + disabled: true,
> + enableFn: record => record.phantom === false,
> + },
> + '-',
> + {
> + xtype: 'proxmoxButton',
> + text: gettext('Prune group'),
> + setBackupGroup: function(backup) {
> + let me = this;
> + if (backup) {
> + let volid = backup.volid;
> + let vmid = backup.vmid;
> + let format = backup.format;
>
> - me.callParent();
> + let vmtype = PVE.Utils.get_backup_type(volid, format);
> + if (vmid && vmtype) {
> + me.setText(gettext('Prune group') + `
> ${vmtype}/${vmid}`);
> + me.vmid = vmid;
> + me.vmtype = vmtype;
> + me.setDisabled(false);
> + return;
> + }
> + }
> + me.setText(gettext('Prune group'));
> + me.vmid = null;
> + me.vmtype = null;
> + me.setDisabled(true);
> + },
> + handler: 'pruneGroupHandler',
> + parentXType: "treepanel",
> + disabled: true,
> + reference: 'pruneButton',
> + enableFn: () => true,
This makes the disabling in setBackupGroup not work as intended.
> + },
> + {
> + xtype: 'proxmoxButton',
> + text: gettext('Remove'),
> + handler: 'removeHandler',
> + parentXType: 'treepanel',
> + disabled: true,
> + enableFn: record => record.phantom === false &&
> !record?.data?.protected,
> + confirmMsg: function(rec) {
> + let name = rec.data.text;
> + return Ext.String.format(gettext('Are you sure you want to
> remove entry {0}'), `'${name}'`);
Sytle nit: line too long
> + },
> + },
> + '->',
> + {
> + xtype: 'pveStorageSelector',
> + fieldLabel: gettext('Storage'),
> + storageContent: 'backup',
> + reference: 'storagesel',
> + listeners: {
> + change: 'storageSelectorChange',
> + boxready: 'storageSelectorBoxReady',
> + },
>
> - me.store.getSorters().clear();
> - me.store.setSorters([
> - {
> - property: 'vmid',
> - direction: 'ASC',
> + },
> + gettext('Search') + ':',
> + ' ',
> + {
> + xtype: 'textfield',
> + width: 200,
> + enableKeyEvents: true,
> + emptyText: gettext('Name, Format'),
> + listeners: {
> + keyup: {
> + buffer: 500,
> + fn: 'searchKeyupFn',
> + },
> + change: 'searchChangeFn',
> },
> - {
> - property: 'vdate',
> - direction: 'DESC',
> + triggers: {
> + clear: {
> + cls: 'pmx-clear-trigger',
> + weight: -1,
> + hidden: true,
> + handler: 'searchClearHandler',
> + },
> },
> - ]);
> + },
> + ],
> +
> + listeners: {
> + activate: function() {
> + let me = this;
> + // only load on first activate to not load every tab switch
> + if (!me.firstLoad) {
> + me.getController().reload();
> + me.firstLoad = true;
> + }
This does not seem to apply here, because there are no tabs to switch
to/from. Switching to and back from a different storage content type
leads to me.firstLoad being undefined again. But I'd actually want to
see changes when doing so in any case ;)
> + },
> + selectionchange: function(model, srecords, eOpts) {
> + let pruneButton = this.getController().lookup('pruneButton');
> + if (srecords.length === 1) {
> + pruneButton.setBackupGroup(srecords[0].data);
> + } else {
> + pruneButton.setBackupGroup(null);
> + }
> + },
> },
> });
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
