all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Stoiko Ivanov <s.ivanov@proxmox.com>
To: Dominik Csapak <d.csapak@proxmox.com>
Cc: pmg-devel@lists.proxmox.com
Subject: Re: [pmg-devel] [PATCH pmg-yew-quarantine-gui v3] execute actions when opening gui from a link from quarantine mails
Date: Fri, 31 Oct 2025 16:37:11 +0100	[thread overview]
Message-ID: <20251031163711.49049776@rosa.proxmox.com> (raw)
In-Reply-To: <20251030103209.1949668-1-d.csapak@proxmox.com>

Thanks for picking this up and the cleaner (and working) implementation!

gave it a spin on my test-setup - but had some small issues while building.
It worked fine with:
* applying your yew-widget-toolkit patch[0] on top of 
  602f5a4 ("bump version to 0.7.3") (neither current master nor the repo
  up to your commit (3a437d19c3bee74ec6ebdfef328e5e48c6f073f5) worked for me)
* keeping everything else on the state from our devel-repo
  (as I ran into build-errors with current master of yew-widget-toolkit, I first
  tried installing some of its dependencies from their repos, but the issues remained)
* applying this on current master of pmg-yew-quarantine-gui)

[0] https://lore.proxmox.com/yew-devel/20251029143617.18134-1-d.csapak@proxmox.com/T/#u

one tiny nit inline:
On Thu, 30 Oct 2025 11:26:16 +0100
Dominik Csapak <d.csapak@proxmox.com> wrote:

> The PMG quarantine mails can contain links that should do some actions
> directly, e.g. deliver or delete a mail. This works for our desktop gui
> and worked in the old Framework7 mobile ui, but was forgotten to be
> implemented in the new rust based gui.
> 
> This patch implements it in `SpamList` in the 'create' method.
> 
> To make it work properly we have to omit the `location.replace` when
> doing a login, otherwise we lose the get parameters and refresh the
> page. Instead we do a http_set_auth (like in create).
> 
> A SnackBar is also added on successful execution of an action.
> 
> Parts of this patch are take from Stoiko's first approach[0].
> 
> Note that while this patch only works with the catalog loader fix[1].
> Without that, a ticket via login will be set, but since we don't
> refresh the page anymore the catalog loader discards the state of the
> login page and the 'on_login' callback is not called anymore which
> means that the login page will still be shown.
> 
> 0: https://lore.proxmox.com/pmg-devel/20251028163628.79739-1-s.ivanov@proxmox.com/
> 1: https://lore.proxmox.com/yew-devel/20251029143617.18134-1-d.csapak@proxmox.com/T/#u
> 
> Co-developed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
s/-off//

apart from this:
Consider this:
Reviewed-by: Stoiko Ivanov <s.ivanov@proxmox.com>
Tested-by: Stoiko Ivanov <s.ivanov@proxmox.com>



> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> chnages from v2:
> * changed 'call' to 'execute'
> * improved match statement for the FromStr implementation
> * added a missing 'http_set_auth' that should be triggered from the
>   login page callback
> * changed the logic from 'remove get parameter' to 'extract_query_parameter'
>   so we can extract single parameters directly. this also now uses
>   the UrlSearchParams interface instead of using form_urlencoded
> * after additional testing, i noticed that this only works with the
>   catalog loader change (see details in commit message) so we have
>   to bump pwt/yew-comp and depend on the new versions for this patch..
>   (i could make it work without it, but for that there would be
>   even more changes necessary, and the catalog loader fix is already
>   applied. no sense in workarounding stuff that is already fixed i guess)
> 
>  src/main.rs       | 24 +++++++++++---
>  src/page_login.rs | 22 ++++++++-----
>  src/spam_list.rs  | 80 ++++++++++++++++++++++++++++++++++++++---------
>  3 files changed, 99 insertions(+), 27 deletions(-)
> 
> diff --git a/src/main.rs b/src/main.rs
> index 8cd8a6b..364f12a 100644
> --- a/src/main.rs
> +++ b/src/main.rs
> @@ -14,8 +14,8 @@ pub use page_not_found::PageNotFound;
>  mod page_login;
>  pub use page_login::PageLogin;
>  
> -use anyhow::Error;
> -use gloo_utils::{document, format::JsValueSerdeExt};
> +use anyhow::{format_err, Error};
> +use gloo_utils::format::JsValueSerdeExt;
>  use serde::Deserialize;
>  use serde_json::{json, Value};
>  use wasm_bindgen::JsValue;
> @@ -158,9 +158,10 @@ impl Component for PmgQuarantineApp {
>          match msg {
>              Msg::Login(info) => {
>                  self.login_info = Some(info.clone());
> -                let document = document();
> -                let location = document.location().unwrap();
> -                let _ = location.replace(&location.pathname().unwrap());
> +                http_set_auth(info.clone());
> +                if info.ticket.to_string().starts_with("PMGQUAR:") {
> +                    stop_ticket_refresh_loop();
> +                }
>              }
>              Msg::Logout => self.login_info = None,
>          }
> @@ -187,6 +188,19 @@ impl std::fmt::Display for MailAction {
>      }
>  }
>  
> +impl std::str::FromStr for MailAction {
> +    type Err = anyhow::Error;
> +    fn from_str(s: &str) -> Result<Self, Self::Err> {
> +        match s {
> +            "deliver" => Ok(MailAction::Deliver),
> +            "delete" => Ok(MailAction::Delete),
> +            "welcomelist" | "whitelist" => Ok(MailAction::Welcomelist),
> +            "blocklist" | "blacklist" => Ok(MailAction::Blocklist),
> +            _ => Err(format_err!("unknown quarantine action")),
> +        }
> +    }
> +}
> +
>  pub(crate) async fn mail_action(id: &str, action: MailAction) -> Result<Value, Error> {
>      let param = json!({
>          "action": action.to_string(),
> diff --git a/src/page_login.rs b/src/page_login.rs
> index d285209..9185856 100644
> --- a/src/page_login.rs
> +++ b/src/page_login.rs
> @@ -20,6 +20,8 @@ use proxmox_yew_comp::{
>      http_login, start_ticket_refresh_loop, stop_ticket_refresh_loop, LoginPanel,
>  };
>  
> +use crate::spam_list::extract_query_parameter;
> +
>  #[derive(Properties, PartialEq)]
>  pub struct PageLogin {
>      #[prop_or_default]
> @@ -81,16 +83,20 @@ impl Component for PmgPageLogin {
>          let location = document.location().unwrap();
>          let path = location.pathname().unwrap();
>          if path == "/quarantine" {
> -            let search = location.search().unwrap();
> -            let param = web_sys::UrlSearchParams::new_with_str(&search).unwrap();
> -            if let Some(ticket) = param.get("ticket") {
> -                let ticket = percent_decode_str(&ticket).decode_utf8_lossy();
> -                if ticket.starts_with("PMGQUAR:") {
> -                    if let Some(username) = ticket.split(":").nth(1) {
> -                        Self::ticket_login(ctx, username.to_string(), ticket.to_string());
> -                        stop_ticket_refresh_loop();
> +            match extract_query_parameter("ticket") {
> +                Ok(Some(ticket)) => {
> +                    let ticket = percent_decode_str(&ticket).decode_utf8_lossy();
> +                    if ticket.starts_with("PMGQUAR:") {
> +                        if let Some(username) = ticket.split(":").nth(1) {
> +                            Self::ticket_login(ctx, username.to_string(), ticket.to_string());
> +                            stop_ticket_refresh_loop();
> +                        }
>                      }
>                  }
> +                Ok(None) => {}
> +                Err(err) => {
> +                    log::error!("could not extract 'ticket' parameter from query: {err}");
> +                }
>              }
>          }
>  
> diff --git a/src/spam_list.rs b/src/spam_list.rs
> index b870e12..b5a9dcb 100644
> --- a/src/spam_list.rs
> +++ b/src/spam_list.rs
> @@ -1,10 +1,14 @@
> -use anyhow::Error;
> -use wasm_bindgen::JsValue;
> +use std::{rc::Rc, str::FromStr};
>  
> -use core::clone::Clone;
> +use anyhow::{format_err, Error};
> +use gloo_utils::window;
>  use js_sys::Date;
> +use serde::{Deserialize, Serialize};
>  use serde_json::Value;
> -use std::rc::Rc;
> +use url::Url;
> +use wasm_bindgen::JsValue;
> +use yew::html::{IntoEventCallback, IntoPropValue};
> +use yew::virtual_dom::{VComp, VNode};
>  
>  use pwt::{
>      css::{AlignItems, ColorScheme, FlexFit, Opacity, Overflow},
> @@ -12,17 +16,10 @@ use pwt::{
>      touch::{Slidable, SlidableAction, SnackBar, SnackBarContextExt},
>      widget::{error_message, Container, Fa, List, ListTile, Progress, Row},
>  };
> -use yew::{
> -    html::{IntoEventCallback, IntoPropValue},
> -    virtual_dom::{VComp, VNode},
> -};
> -//use yew::html::IntoEventCallback;
>  
>  use proxmox_yew_comp::http_get;
>  use pwt::widget::Column;
>  
> -use serde::{Deserialize, Serialize};
> -
>  use crate::{mail_action, MailAction};
>  
>  #[derive(Copy, Clone, Serialize, Default, PartialEq)]
> @@ -110,6 +107,19 @@ impl Component for PmgSpamList {
>  
>      fn create(ctx: &Context<Self>) -> Self {
>          let me = Self { data: None };
> +
> +        match extract_mail_action_from_query_params() {
> +            Ok(None) => {}
> +            Ok(Some((id, action))) => {
> +                ctx.link().send_message(Msg::Action(id, action));
> +            }
> +            Err(err) => {
> +                ctx.link().show_snackbar(
> +                    SnackBar::new().message(format!("could not execute action: {err}")),
> +                );
> +            }
> +        }
> +
>          me.load(ctx);
>          me
>      }
> @@ -145,9 +155,11 @@ impl Component for PmgSpamList {
>              Msg::Action(id, action) => {
>                  let link = ctx.link().clone();
>                  wasm_bindgen_futures::spawn_local(async move {
> -                    if let Err(err) = mail_action(&id, action).await {
> -                        link.show_snackbar(SnackBar::new().message(err.to_string()));
> -                    }
> +                    let msg = match mail_action(&id, action).await {
> +                        Ok(_) => tr!("Action '{0}' successful", action),
> +                        Err(err) => err.to_string(),
> +                    };
> +                    link.show_snackbar(SnackBar::new().message(msg));
>                      link.send_message(Msg::Reload);
>                  });
>                  return false;
> @@ -299,3 +311,43 @@ fn epoch_to_date(epoch: i64) -> String {
>          date.get_date()
>      )
>  }
> +
> +fn extract_mail_action_from_query_params() -> Result<Option<(String, MailAction)>, Error> {
> +    let id = extract_query_parameter("cselect")?;
> +    let action = extract_query_parameter("action")?;
> +
> +    if let (Some(id), Some(action)) = (id, action) {
> +        let action = MailAction::from_str(&action)?;
> +        return Ok(Some((id, action)));
> +    }
> +    Ok(None)
> +}
> +
> +/// Removes `name` parameter from the get values via the browser `history` object and returns it
> +/// if it exists.
> +pub fn extract_query_parameter(name: &str) -> Result<Option<String>, Error> {
> +    let location = window().location();
> +    let history = window().history().unwrap();
> +    let search = location.search().unwrap();
> +    let param = web_sys::UrlSearchParams::new_with_str(&search).unwrap();
> +
> +    if let Some(value) = param.get(name) {
> +        param.delete(name);
> +
> +        let mut url = Url::parse(
> +            &location
> +                .href()
> +                .map_err(|err| format_err!("could not get location: {err:?}"))?,
> +        )?;
> +        let query: String = param.to_string().into();
> +
> +        url.set_query(Some(&query));
> +        history
> +            .replace_state_with_url(&JsValue::null(), "", Some(url.as_str()))
> +            .map_err(|err| format_err!("could not set url: {err:?}"))?;
> +
> +        return Ok(Some(value));
> +    }
> +
> +    Ok(None)
> +}



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


      reply	other threads:[~2025-10-31 15:37 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-30 10:26 Dominik Csapak
2025-10-31 15:37 ` Stoiko Ivanov [this message]

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=20251031163711.49049776@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 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