public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: "Jakob Klocker" <j.klocker@proxmox.com>
To: "Proxmox VE development discussion" <pve-devel@lists.proxmox.com>
Subject: Re: [pve-devel] [PATCH manager v2] ui: pool create: allow selecting parent pool via selector
Date: Mon, 31 Aug 2026 15:43:00 +0200	[thread overview]
Message-ID: <DL368W8OLI2B.1ICBHKQSNU4ZE@proxmox.com> (raw)
In-Reply-To: <20260123074546.396323-1-d.csapak@proxmox.com>

Thanks for the patch.

Tested on top of current master. Went through:

- creating a top-level pool (no parent selected) -- submits the plain name
- creating child and grandchild pools via the selector (`a/b`, `a/b/c`)
- the name field rejects slashes, preventing pool creation
- selector is hidden in edit mode
- adding a child under parents with max depth shows an error
- checking that `parentPool` isn't submited

Not touched, but made sure it still works:

- Permissions -- gave a user rights to `a/b`, confirmed it can't edit `a`
- can't remove a parent while it still has children

Everything works as expected.

One thing that _could_ be added: hiding parents already at max
depth (3), since children can't be created under them. Though one 
could argue the current behavior -- keeping them and showing the
correct error -- is better, so users aren't confused about why those
pools are missing.

Code looks good to me as well, consider this:

Reviewed-by: Jakob Klocker <j.klocker@proxmox.com>
Tested-by: Jakob Klocker <j.klocker@proxmox.com>

On Fri Jan 23, 2026 at 8:42 AM CET, Dominik Csapak wrote:
> instead of having to type out the full path, e.g. 'foo/bar/baz',
> show a 'pool selector' to select the parent pool.
>
> Forbid using slashes in the name field so it's not possible to specify
> the structure with both the name and parent pool field.
>
> This makes creating nested pools more obvious and easier.
>
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> changes from v1:
> * always delete 'parentPool' from values so we don't accidentally submit it
> * only consider parentPool if it's a non-empty string
> * add regex to the namefield to disallow slashes
> * add emptyText to the poolselector
>
>  www/manager6/dc/PoolEdit.js | 52 +++++++++++++++++++++++++++----------
>  1 file changed, 39 insertions(+), 13 deletions(-)
>  [SNIP]




  parent reply	other threads:[~2026-08-31 13:43 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-23  7:42 [pve-devel] [PATCH manager v2] ui: pool create: allow selecting parent pool via selector Dominik Csapak
2026-08-31  9:26 ` Dominik Csapak
2026-08-31 13:43 ` Jakob Klocker [this message]
2026-08-31 14:03 ` Aaron Lauterer
2026-09-01  7:18 ` superseded: " Dominik Csapak

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=DL368W8OLI2B.1ICBHKQSNU4ZE@proxmox.com \
    --to=j.klocker@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