all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: "Stefan Sterz" <s.sterz@proxmox.com>
To: "Friedrich Weber" <f.weber@proxmox.com>,
	"Proxmox VE development discussion" <pve-devel@lists.proxmox.com>
Subject: Re: [pve-devel] [PATCH widget-toolkit 3/3] window: edit: avoid shared object for extra request params
Date: Thu, 04 Apr 2024 11:23:56 +0200	[thread overview]
Message-ID: <D0B89ODDN2EF.32UYX8ZQQZHU5@proxmox.com> (raw)
In-Reply-To: <a61ff6e8-c79c-4539-a2b6-acacc79c6440@proxmox.com>

-- >8 snip 8< --
> >
> > i did a quick an dirty test and using a constructor like this seems to
> > rule out this class of bug completelly:
> >
> > ```js
> >     constructor: function(conf) {
> >         let me = this;
> >         me.extraRequestParams = {};
> >         me.initConfig(conf);
> >         me.callParent();
> >     },
> > ```
> >
> > basically it configures the edit window as usual, but overwrites the
> > `extraRequestParams` object for each instance with a new empty object.
> > so there are no more shared objects :) could you check whether that also
> > fixes the other instances?
> >
> > [1]: https://docs-devel.sencha.com/extjs/7.0.0/classic/Ext.window.Window.html#method-constructor
>
> Nifty, didn't think about a constructor solution. Such a general
> solution would be way more elegant, thanks for suggesting it!
>
> However, this particular constructor seems to break the pattern of
> defining `extraRequestParams` in the subclass properties, as done by
> `PVE.Pool.AddVM` [1]. With the constructor above, the API request done
> by `AddVM` seems to be missing the `allow-move` parameter.
>
> Looks like once `PVE.Pool.AddVM` is instantiated and the constructor is
> called, `extraRequestParams` with `allow-move` is only defined in
> `me.__proto__`, so `me.extraRequestParams = {}` essentially shadows it
> with an empty object, losing the `allow-move`.
>

not sure what you mean by that, if an `PVE.Pool.AddVM` is instantiated,
the `extraRequestParams` is already set, so it isn't just in `__proto__`
for me. but yeah, the problem is correct as `me.extraRequestParams = {}`
overwrites the field.

> Do you have an idea how to fix this? Maybe making a copy of
> `extraRequestParams` would work (I suppose the overhead of creating a
> new object for all edit window (subclass) instances is negligible).
>
> [1]
> https://git.proxmox.com/?p=pve-manager.git;a=blob;f=www/manager6/grid/PoolMembers.js;h=75f20cab;hb=4b06efb5#l9

this worked for me, can you confirm that this also does what it should
for you?

```js
    extraRequestParams: undefined,

    constructor: function(conf) {
        let me = this;
        if (!me.extraRequestParams) {
                me.extraRequestParams = {};
        }
        me.initConfig(conf);
        me.callParent();
    },
```




  reply	other threads:[~2024-04-04  9:23 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-03  9:10 [pve-devel] [PATCH manager/widget-toolkit 0/3] ui: avoid UI bugs due to shared " Friedrich Weber
2024-04-03  9:10 ` [pve-devel] [PATCH manager 1/3] ui: pool members: avoid setting request parameter for all edit windows Friedrich Weber
2024-04-03  9:10 ` [pve-devel] [PATCH manager 2/3] ui: pool members: avoid sharing object for extra request parameters Friedrich Weber
2024-04-03  9:10 ` [pve-devel] [PATCH widget-toolkit 3/3] window: edit: avoid shared object for extra request params Friedrich Weber
2024-04-04  8:22   ` Stefan Sterz
2024-04-04  9:01     ` Friedrich Weber
2024-04-04  9:23       ` Stefan Sterz [this message]
2024-04-04 10:10         ` Friedrich Weber
2024-04-04 10:54           ` Stefan Sterz
2024-04-04 11:04             ` Stefan Sterz
2024-04-04 10:59           ` Thomas Lamprecht
2024-04-04 11:28             ` Friedrich Weber

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=D0B89ODDN2EF.32UYX8ZQQZHU5@proxmox.com \
    --to=s.sterz@proxmox.com \
    --cc=f.weber@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal