From: Stoiko Ivanov <s.ivanov@proxmox.com>
To: Dominik Csapak <d.csapak@proxmox.com>
Cc: pmg-devel@lists.proxmox.com
Subject: Re: [PATCH pmg-api/pmg-docs/pmg-gui 0/5] add optional consent-banner to be shown before login
Date: Mon, 8 Jun 2026 09:42:03 +0200 [thread overview]
Message-ID: <20260608094203.4186cfb9@rosa.proxmox.com> (raw)
In-Reply-To: <080831a6-4b04-4030-b174-0fac7710f877@proxmox.com>
Thanks for checking the patches!
On Mon, 8 Jun 2026 09:19:32 +0200
Dominik Csapak <d.csapak@proxmox.com> wrote:
> had a look over the code and tested a bit.
>
> a few high level points:
>
> * not sure if we would benefit from refactoring the 'consent-text'
> schema/format into pve-common (this was recently also done for the
> 'location', but since we only have it in two places currently
> i'd tend not to, so this looks currently fine to me
As it's 2 places I'd also tend to keep it in pve-cluster/pmg-api for now.
>
> * the consent text will not be shown in the quarantine view.
> my guess is that users who need the consent text also need it in the
> quarantine (maybe even before automatically logging in?)
> should we add a separate consent text? we could always add this later
> too though. If we add this, we'd also have to add it in the mobile
> quarantine of course.
Hm - did not consider that - and if there's demand for it and we implement
it I'd definitely say it needs to be 2 separate texts, or to have the
option to enable it for the admin-interface only.
The UX of clicking on an action in your report mail and then having to
click on the consent banner before it gets done seems like it might make
sense to not send the action links in those cases (but users who enable
the consent-banner for the quarantine interface can adapt their
report-templates).
For me this can be done at a later point.
>
>
> Aside from these, consider the patches:
>
> Reviewed-by: Dominik Csapak <d.csapak@proxmox.com>
> Tested-by: Dominik Csapak <d.csapak@proxmox.com>
>
>
> On 6/8/26 8:57 AM, Stoiko Ivanov wrote:
> > The idea is taken, and the code mostly copied from PVE, where this was
> > added to address:
> > https://bugzilla.proxmox.com/show_bug.cgi?id=5463
> >
> > pmg-docs:
> >
> > Stoiko Ivanov (1):
> > pmgconfig: add short documentation of the consent banner
> >
> > pmgconfig.adoc | 7 +++++++
> > 1 file changed, 7 insertions(+)
> >
> >
> > pmg-api:
> >
> > Stoiko Ivanov (2):
> > config: add consent-text key
> > pmgproxy: pass consent-text as template variable to index
> >
> > src/PMG/Config.pm | 7 +++++++
> > src/PMG/Service/pmgproxy.pm | 2 ++
> > 2 files changed, 9 insertions(+)
> >
> >
> > pmg-gui:
> >
> > Stoiko Ivanov (2):
> > login: show optional consent-banner before login
> > system options: add online help link for consent banner
> >
> > js/LoginView.js | 11 +++++++++++
> > js/SystemOptions.js | 8 ++++++++
> > pmg-index.html.tt | 1 +
> > 3 files changed, 20 insertions(+)
> >
> >
> > Summary over all repositories:
> > 6 files changed, 36 insertions(+), 0 deletions(-)
> >
>
next prev parent reply other threads:[~2026-06-08 7:42 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-06-08 6:56 [PATCH pmg-api/pmg-docs/pmg-gui 0/5] add optional consent-banner to be shown before login Stoiko Ivanov
2026-06-08 6:56 ` [PATCH pmg-docs 1/5] pmgconfig: add short documentation of the consent banner Stoiko Ivanov
2026-06-08 6:56 ` [PATCH pmg-api 2/5] config: add consent-text key Stoiko Ivanov
2026-06-08 6:56 ` [PATCH pmg-api 3/5] pmgproxy: pass consent-text as template variable to index Stoiko Ivanov
2026-06-08 6:56 ` [PATCH pmg-gui 4/5] login: show optional consent-banner before login Stoiko Ivanov
2026-06-08 6:56 ` [PATCH pmg-gui 5/5] system options: add online help link for consent banner Stoiko Ivanov
2026-06-08 7:19 ` [PATCH pmg-api/pmg-docs/pmg-gui 0/5] add optional consent-banner to be shown before login Dominik Csapak
2026-06-08 7:42 ` Stoiko Ivanov [this message]
2026-06-08 11:01 ` applied: " Thomas Lamprecht
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=20260608094203.4186cfb9@rosa.proxmox.com \
--to=s.ivanov@proxmox.com \
--cc=d.csapak@proxmox.com \
--cc=pmg-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