public inbox for pmg-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pmg-devel] [PATCH pmg-yew-quarantine-gui] execute actions when opening gui from a link from quarantine mails
@ 2025-10-29 14:44 Dominik Csapak
  2025-10-29 17:47 ` Thomas Lamprecht
  0 siblings, 1 reply; 2+ messages in thread
From: Dominik Csapak @ 2025-10-29 14:44 UTC (permalink / raw)
  To: pmg-devel

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 remove the ticket parameter (when we extrat that) with
the browsers `history` api, and the rest when we execute the action.

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 enables the actions when entering from a
quarantine link, it won't show a SnackBar (neither on error or success)
due to our `CatalogLoader` behavior. A fix for that is on the yew-devel
list [1].

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>
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/main.rs       | 22 +++++++++---
 src/page_login.rs |  5 +++
 src/spam_list.rs  | 89 +++++++++++++++++++++++++++++++++++++++--------
 3 files changed, 97 insertions(+), 19 deletions(-)

diff --git a/src/main.rs b/src/main.rs
index 8cd8a6b..bb0ecc1 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,6 @@ 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());
             }
             Msg::Logout => self.login_info = None,
         }
@@ -187,6 +184,21 @@ 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" => Ok(MailAction::Welcomelist),
+            "whitelist" => Ok(MailAction::Welcomelist),
+            "blocklist" => Ok(MailAction::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..f1e3406 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::remove_get_parameter;
+
 #[derive(Properties, PartialEq)]
 pub struct PageLogin {
     #[prop_or_default]
@@ -90,6 +92,9 @@ impl Component for PmgPageLogin {
                         Self::ticket_login(ctx, username.to_string(), ticket.to_string());
                         stop_ticket_refresh_loop();
                     }
+                    if let Err(err) = remove_get_parameter(Some("ticket")) {
+                        log::error!("could not remove ticket parameter: {err}");
+                    }
                 }
             }
         }
diff --git a/src/spam_list.rs b/src/spam_list.rs
index b870e12..0af31c6 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::{document, window};
 use js_sys::Date;
+use serde::{Deserialize, Serialize};
 use serde_json::Value;
-use std::rc::Rc;
+use url::{form_urlencoded, 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_get() {
+            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 call 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,52 @@ fn epoch_to_date(epoch: i64) -> String {
         date.get_date()
     )
 }
+
+fn extract_mail_action_from_get() -> Result<Option<(String, MailAction)>, Error> {
+    let document = document();
+    let location = document.location().unwrap();
+    let search = location.search().unwrap();
+    let param = web_sys::UrlSearchParams::new_with_str(&search).unwrap();
+
+    remove_get_parameter(None)?;
+
+    if let (Some(id), Some(action)) = (param.get("cselect"), param.get("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.
+///
+/// If no name is given, removes all parameters.
+pub fn remove_get_parameter(name: Option<&str>) -> Result<(), Error> {
+    let location = window().location();
+    let history = window().history().unwrap();
+
+    let mut url = Url::parse(
+        &location
+            .href()
+            .map_err(|err| format_err!("could not get location: {err:?}"))?,
+    )?;
+    if let Some(name) = name {
+        let mut query = form_urlencoded::Serializer::new(String::new());
+        for (key, value) in url.query_pairs() {
+            if key == name {
+                continue;
+            }
+            query.append_pair(&key, &value);
+        }
+
+        url.set_query(Some(&query.finish()));
+    } else {
+        url.set_query(None);
+    }
+
+    // build the new url without get parameters
+    history
+        .replace_state_with_url(&JsValue::null(), "", Some(url.as_str()))
+        .map_err(|err| format_err!("could not set url: {err:?}"))?;
+
+    Ok(())
+}
-- 
2.47.3



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


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [pmg-devel] [PATCH pmg-yew-quarantine-gui] execute actions when opening gui from a link from quarantine mails
  2025-10-29 14:44 [pmg-devel] [PATCH pmg-yew-quarantine-gui] execute actions when opening gui from a link from quarantine mails Dominik Csapak
@ 2025-10-29 17:47 ` Thomas Lamprecht
  0 siblings, 0 replies; 2+ messages in thread
From: Thomas Lamprecht @ 2025-10-29 17:47 UTC (permalink / raw)
  To: Dominik Csapak, pmg-devel

Am 29.10.25 um 15:45 schrieb Dominik Csapak:
> 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 remove the ticket parameter (when we extrat that) with
> the browsers `history` api, and the rest when we execute the action.
> 
> 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 enables the actions when entering from a
> quarantine link, it won't show a SnackBar (neither on error or success)
> due to our `CatalogLoader` behavior. A fix for that is on the yew-devel
> list [1].
> 
> 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

Looks OK to me in general principle, some comments (mostly nits and naming)
inline though, and would also like to get a T-b, e.g. from Stoiko (but I'm
not picky ;-))

> 
> Co-developed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  src/main.rs       | 22 +++++++++---
>  src/page_login.rs |  5 +++
>  src/spam_list.rs  | 89 +++++++++++++++++++++++++++++++++++++++--------
>  3 files changed, 97 insertions(+), 19 deletions(-)
> 
> diff --git a/src/main.rs b/src/main.rs
> index 8cd8a6b..bb0ecc1 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,6 @@ 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());
>              }
>              Msg::Logout => self.login_info = None,
>          }
> @@ -187,6 +184,21 @@ 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" => Ok(MailAction::Welcomelist),
> +            "whitelist" => Ok(MailAction::Welcomelist),

tiny nit: could be written as alternatives to better clarify that it's the same, i.e.:

"welcomelist" | "whitelist" => Ok(MailAction::Welcomelist),

> +            "blocklist" => Ok(MailAction::Blocklist),
> +            "blacklist" => Ok(MailAction::Blocklist),

similar nit above.

> +            _ => 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..f1e3406 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::remove_get_parameter;
> +
>  #[derive(Properties, PartialEq)]
>  pub struct PageLogin {
>      #[prop_or_default]
> @@ -90,6 +92,9 @@ impl Component for PmgPageLogin {
>                          Self::ticket_login(ctx, username.to_string(), ticket.to_string());
>                          stop_ticket_refresh_loop();
>                      }
> +                    if let Err(err) = remove_get_parameter(Some("ticket")) {
> +                        log::error!("could not remove ticket parameter: {err}");
> +                    }
>                  }
>              }
>          }
> diff --git a/src/spam_list.rs b/src/spam_list.rs
> index b870e12..0af31c6 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::{document, window};
>  use js_sys::Date;
> +use serde::{Deserialize, Serialize};
>  use serde_json::Value;
> -use std::rc::Rc;
> +use url::{form_urlencoded, 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_get() {
> +            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 call action: {err}")),

Would do s/call/execute/, as calling is IMO not that easy to understand.

> +                );
> +            }
> +        }
> +
>          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,52 @@ fn epoch_to_date(epoch: i64) -> String {
>          date.get_date()
>      )
>  }
> +
> +fn extract_mail_action_from_get() -> Result<Option<(String, MailAction)>, Error> {
> +    let document = document();
> +    let location = document.location().unwrap();
> +    let search = location.search().unwrap();
> +    let param = web_sys::UrlSearchParams::new_with_str(&search).unwrap();
> +
> +    remove_get_parameter(None)?;
> +
> +    if let (Some(id), Some(action)) = (param.get("cselect"), param.get("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.
> +///
> +/// If no name is given, removes all parameters.
> +pub fn remove_get_parameter(name: Option<&str>) -> Result<(), Error> {

"get" is a bit ambiguous here IMO as these parameters in the URL/location can
be send on any request. Maybe better use "search" or "query".

Also why delete here and extract above? In the backend we also got a quite frequently
used "extract_param" method for something that deletes and returns a specific parameter,
so might be fitting here. E.g.:

fn extract_query_parameter

Or even

fn extract_location_query_parameter


And for above extract_mail_action_from_get then either extract_mail_action_from_query or
extract_mail_action_from_location_query, respectively. Also s/get/query/ in the error
log message on call site.

> +    let location = window().location();
> +    let history = window().history().unwrap();
> +
> +    let mut url = Url::parse(
> +        &location
> +            .href()
> +            .map_err(|err| format_err!("could not get location: {err:?}"))?,
> +    )?;
> +    if let Some(name) = name {
> +        let mut query = form_urlencoded::Serializer::new(String::new());
> +        for (key, value) in url.query_pairs() {
> +            if key == name {
> +                continue;
> +            }
> +            query.append_pair(&key, &value);
> +        }
> +
> +        url.set_query(Some(&query.finish()));
> +    } else {
> +        url.set_query(None);
> +    }
> +
> +    // build the new url without get parameters
> +    history
> +        .replace_state_with_url(&JsValue::null(), "", Some(url.as_str()))
> +        .map_err(|err| format_err!("could not set url: {err:?}"))?;
> +
> +    Ok(())
> +}



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


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2025-10-29 17:47 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-10-29 14:44 [pmg-devel] [PATCH pmg-yew-quarantine-gui] execute actions when opening gui from a link from quarantine mails Dominik Csapak
2025-10-29 17:47 ` Thomas Lamprecht

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