public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: "Daniel Kral" <d.kral@proxmox.com>
To: "Proxmox VE development discussion" <pve-devel@lists.proxmox.com>
Subject: Re: [pve-devel] [PATCH manager 7/7] ui: cpu flag selector: query CPU flag list via API
Date: Thu, 06 Nov 2025 17:57:26 +0100	[thread overview]
Message-ID: <DE1RRET4YD8L.3J4SE4C6DO6CQ@proxmox.com> (raw)
In-Reply-To: <20251031122834.62482-8-f.ebner@proxmox.com>

On Fri Oct 31, 2025 at 1:27 PM CET, Fiona Ebner wrote:
> There is coupling between the store and value for the top-level grid,
> which has a field mixin. Becuase of this, the setValue() method needs
> to update the 'state' value for the records in the store accordingly.
> This can however only be done if the store is already loaded, so the
> store gets a refresh() callback. It also remembers which value it was
> already adjusted for for potential subsequent setValue() calls.
>
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
>
> Happy to hear suggestions if this is too involved already! The grid
> itself is a field (via mixin) and the 'state' values in the store are
> coupled with the field value, so I couldn't figure out a simpler way
> yet.

-->8 snip 8<---

>      getValue: function () {
>          let me = this;
>          let store = me.getStore();
> +
> +        if (!store.isLoaded()) {
> +            return me.value;
> +        }
> +
>          let flags = '';
>  
>          // ExtJS does not has a nice getAllRecords interface for stores :/
>          store.queryBy(Ext.returnTrue).each(function (rec) {

unrelated to the patch, but shouldn't `store.getData()` return all the
records? `store.getData().getSource()` would be the equivalent, but as
the store for cpu capabilities isn't filtered right now, the former
should work.

>              let s = rec.get('state');
>              if (s && s !== '=') {
> -                let f = rec.get('flag');
> +                let f = rec.get('name');
>                  if (flags === '') {
>                      flags = s + f;
>                  } else {
> @@ -88,33 +68,41 @@ Ext.define('PVE.form.VMCPUFlagSelector', {
>          return flags;
>      },
>  
> -    setValue: function (value) {
> +    // Adjusts the store for the current value and determines the unkown flags based on what the
> +    // store does not know.
> +    adjustStoreForValue: function () {
>          let me = this;
>          let store = me.getStore();
> -
> -        me.value = value || '';
> +        let value = me.value;
>  
>          me.unkownFlags = [];
>  
> -        me.getStore()
> -            .queryBy(Ext.returnTrue)
> -            .each(function (rec) {
> -                rec.set('state', '=');
> -            });
> +        store.queryBy(Ext.returnTrue).each((rec) => rec.set('state', '='));

same here

>  
>          let flags = value ? value.split(';') : [];
>          flags.forEach(function (flag) {
>              let sign = flag.substr(0, 1);
>              flag = flag.substr(1);
>  
> -            let rec = store.findRecord('flag', flag, 0, false, true, true);
> +            let rec = store.findRecord('name', flag, 0, false, true, true);
>              if (rec !== null) {
>                  rec.set('state', sign);
>              } else {
>                  me.unkownFlags.push(flag);
>              }
>          });
> -        store.reload();
> +
> +        store.adjustedForValue = value;
> +    },
> +
> +    setValue: function (value) {
> +        let me = this;
> +
> +        me.value = value || '';
> +
> +        if (me.getStore().isLoaded()) {
> +            me.adjustForValue();

Shouldn't this be `me.adjustStoreForValue();`? :)

I haven't yet found a way to trigger this though - neither in the Create
wizard nor the CPU modal with and without values - but maybe my
debugging strategy here is a bit wrong. How did you trigger it?

> +        } // if not yet loaded, the store will trigger the function
>  
>          let res = me.mixins.field.setValue.call(me, value);
>  
> @@ -184,11 +172,11 @@ Ext.define('PVE.form.VMCPUFlagSelector', {
>              },
>          },
>          {
> -            dataIndex: 'flag',
> +            dataIndex: 'name',
>              width: 100,
>          },
>          {
> -            dataIndex: 'desc',
> +            dataIndex: 'description',
>              cellWrap: true,
>              flex: 1,
>          },
> @@ -197,12 +185,8 @@ Ext.define('PVE.form.VMCPUFlagSelector', {
>      initComponent: function () {
>          let me = this;
>  
> -        // static class store, thus gets not recreated, so ensure defaults are set!
> -        me.getStore().data.forEach(function (v) {
> -            v.state = '=';
> -        });
> -
>          me.value = me.originalValue = '';
> +        me.store.view = me;
>  
>          me.callParent(arguments);
>      },



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


  reply	other threads:[~2025-11-06 16:57 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-31 12:27 [pve-devel] [PATCH-SERIES qemu-server/manager 0/7] VM CPU flags: introduce vendor-agnostic 'nested-virt' CPU flag Fiona Ebner
2025-10-31 12:27 ` [pve-devel] [PATCH qemu-server 1/7] cpu config: style fix: avoid multiline post-if expressions Fiona Ebner
2025-11-06 18:03   ` [pve-devel] applied: " Thomas Lamprecht
2025-10-31 12:27 ` [pve-devel] [PATCH qemu-server 2/7] cpu config: style fix: avoid overly long ternary conditional expression Fiona Ebner
2025-11-06 18:03   ` [pve-devel] applied: " Thomas Lamprecht
2025-10-31 12:27 ` [pve-devel] [PATCH qemu-server 3/7] api: add endpoint for querying available cpu flags Fiona Ebner
2025-11-06 18:02   ` Thomas Lamprecht
2025-11-06 18:17     ` Thomas Lamprecht
2025-11-07  9:05     ` Fiona Ebner
2025-10-31 12:27 ` [pve-devel] [PATCH qemu-server 4/7] cpu config: introduce vendor-agnostic 'nested-virt' CPU flag Fiona Ebner
2025-11-06 17:00   ` Daniel Kral
2025-11-07  9:12     ` Fiona Ebner
2025-10-31 12:27 ` [pve-devel] [PATCH manager 5/7] api: capabilities: register module for VM CPU flags Fiona Ebner
2025-10-31 12:27 ` [pve-devel] [PATCH manager 6/7] ui: cpu flag selector: code style: use 'let' for declarations Fiona Ebner
2025-10-31 12:27 ` [pve-devel] [PATCH manager 7/7] ui: cpu flag selector: query CPU flag list via API Fiona Ebner
2025-11-06 16:57   ` Daniel Kral [this message]
2025-11-07  9:30     ` Fiona Ebner
2025-11-06 17:13 ` [pve-devel] [PATCH-SERIES qemu-server/manager 0/7] VM CPU flags: introduce vendor-agnostic 'nested-virt' CPU flag Daniel Kral
2025-11-07 11:33   ` Fiona Ebner
2025-11-07 12:13     ` Daniel Kral

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DE1RRET4YD8L.3J4SE4C6DO6CQ@proxmox.com \
    --to=d.kral@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal