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
     prev parent 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox