Handle all user errors in add_subscription_dialog

Cargo.lock

@@ -1641,7 +1641,7 @@ dependencies = [
 
 [[package]]
 name = "notify"
-version = "0.1.0"
+version = "0.1.1"
 dependencies = [
  "anyhow",
  "ashpd",

ntfy-daemon/src/lib.rs

@@ -21,6 +21,8 @@ pub struct SharedEnv {
 pub enum Error {
     #[error("topic {0} must not be empty and must contain only alphanumeric characters and _ (underscore)")]
     InvalidTopic(String),
+    #[error("invalid server base url {0:?}")]
+    InvalidServer(#[from] url::ParseError),
     #[error("duplicate message")]
     DuplicateMessage,
     #[error("can't parse the minimum set of required fields from the message {0}")]

ntfy-daemon/src/models.rs

@@ -121,23 +121,31 @@ pub struct Subscription {
 }
 
 impl Subscription {
-    pub fn build_url(server: &str, topic: &str, since: u64) -> anyhow::Result<url::Url> {
+    pub fn build_url(server: &str, topic: &str, since: u64) -> Result<url::Url, crate::Error> {
         let mut url = url::Url::parse(server)?;
         url.path_segments_mut()
-            .map_err(|_| anyhow::anyhow!("url can't be base"))?
+            .map_err(|_| url::ParseError::RelativeUrlWithCannotBeABaseBase)?
             .push(&topic)
             .push("json");
         url.query_pairs_mut()
             .append_pair("since", &since.to_string());
         Ok(url)
     }
-    pub fn validate(self) -> anyhow::Result<Self> {
-        validate_topic(&self.topic)?;
-        Self::build_url(&self.server, &self.topic, 0)?;
+    pub fn validate(self) -> Result<Self, Vec<crate::Error>> {
+        let mut errs = vec![];
+        if let Err(e) = validate_topic(&self.topic) {
+            errs.push(e);
+        };
+        if let Err(e) = Self::build_url(&self.server, &self.topic, 0) {
+            errs.push(e);
+        };
+        if errs.len() > 0 {
+            return Err(errs);
+        }
         Ok(self)
     }
-    pub fn builder(server: String, topic: String) -> SubscriptionBuilder {
-        SubscriptionBuilder::new(server, topic)
+    pub fn builder(topic: String) -> SubscriptionBuilder {
+        SubscriptionBuilder::new(topic)
     }
 }
 
@@ -153,9 +161,9 @@ pub struct SubscriptionBuilder {
 }
 
 impl SubscriptionBuilder {
-    pub fn new(server: String, topic: String) -> Self {
+    pub fn new(topic: String) -> Self {
         Self {
-            server,
+            server: "https://ntfy.sh".to_string(),
             topic,
             muted: false,
             archived: false,
@@ -165,6 +173,11 @@ impl SubscriptionBuilder {
         }
     }
 
+    pub fn server(mut self, server: String) -> Self {
+        self.server = server;
+        self
+    }
+
     pub fn muted(mut self, muted: bool) -> Self {
         self.muted = muted;
         self
@@ -190,7 +203,7 @@ impl SubscriptionBuilder {
         self
     }
 
-    pub fn build(self) -> anyhow::Result<Subscription> {
+    pub fn build(self) -> Result<Subscription, Vec<Error>> {
         let res = Subscription {
             server: self.server,
             topic: self.topic,

ntfy-daemon/src/system_client.rs

@@ -401,17 +401,11 @@ impl system_notifier::Server for SystemNotifier {
     ) -> capnp::capability::Promise<(), capnp::Error> {
         let topic = pry!(pry!(params.get()).get_topic());
         let server: &str = pry!(pry!(params.get()).get_server());
-        let server = if server.is_empty() {
-            "https://ntfy.sh"
-        } else {
-            ""
-        };
 
-        let subscription = pry!(
-            models::Subscription::builder(server.to_owned(), topic.to_owned())
-                .build()
-                .map_err(|e| capnp::Error::failed(e.to_string()))
-        );
+        let subscription = pry!(models::Subscription::builder(topic.to_owned())
+            .server(server.to_string())
+            .build()
+            .map_err(|e| capnp::Error::failed(format!("{:?}", e))));
         let sub: Promise<subscription::Client, capnp::Error> = self.watch(subscription.clone());
 
         let mut db = self.env.db.clone();

src/widgets/add_subscription_dialog.rs

@@ -8,13 +8,18 @@ use gtk::gio;
 use gtk::glib;
 use ntfy_daemon::models;
 
+#[derive(Default, Debug, Clone)]
+pub struct Widgets {
+    pub topic_entry: adw::EntryRow,
+    pub server_entry: adw::EntryRow,
+    pub server_expander: adw::ExpanderRow,
+    pub sub_btn: gtk::Button,
+}
 mod imp {
     pub use super::*;
     #[derive(Debug, Default)]
     pub struct AddSubscriptionDialog {
-        pub topic_entry: RefCell<adw::EntryRow>,
-        pub server_entry: RefCell<adw::EntryRow>,
-        pub sub_btn: RefCell<gtk::Button>,
+        pub widgets: RefCell<Widgets>,
     }
 
     #[glib::object_subclass]
@@ -32,7 +37,6 @@ mod imp {
             );
             klass.install_action("default.activate", None, |this, _, _| {
                 this.emit_subscribe_request();
-                this.close();
             });
         }
     }
@@ -112,7 +116,7 @@ impl AddSubscriptionDialog {
                                     }
                                 }
                             },
-                            append = &adw::ExpanderRow {
+                            append: server_expander = &adw::ExpanderRow {
                                 set_title: "Custom server...",
                                 set_enable_expansion: false,
                                 set_show_enable_switch: true,
@@ -129,7 +133,6 @@ impl AddSubscriptionDialog {
                             set_sensitive: false,
                             connect_clicked[obj] => move |_| {
                                 obj.emit_subscribe_request();
-                                obj.close();
                             }
                         }
                     },
@@ -142,34 +145,58 @@ impl AddSubscriptionDialog {
         topic_entry.delegate().unwrap().connect_changed(move |_| {
             txc.send(()).unwrap();
         });
+        let txc = tx.clone();
+        server_entry.delegate().unwrap().connect_changed(move |_| {
+            txc.send(()).unwrap();
+        });
+        server_expander.connect_enable_expansion_notify(move |_| {
+            tx.send(()).unwrap();
+        });
         let rx = crate::async_utils::debounce_channel(std::time::Duration::from_millis(500), rx);
         let objc = obj.clone();
         rx.attach(None, move |_| {
             objc.check_errors();
             glib::ControlFlow::Continue
         });
-        imp.topic_entry.replace(topic_entry);
-        imp.server_entry.replace(server_entry);
-        imp.sub_btn.replace(sub_btn);
+        imp.widgets.replace(Widgets {
+            topic_entry,
+            server_expander,
+            server_entry,
+            sub_btn,
+        });
 
         obj.set_content(Some(&toolbar_view));
     }
-    pub fn topic(&self) -> String {
-        self.imp().topic_entry.borrow().text().to_string()
-    }
-    pub fn server(&self) -> String {
-        self.imp().server_entry.borrow().text().to_string()
+    pub fn subscription(&self) -> Result<models::Subscription, Vec<ntfy_daemon::Error>> {
+        let w = { self.imp().widgets.borrow().clone() };
+        let mut sub = models::Subscription::builder(w.topic_entry.text().to_string());
+        if w.server_expander.enables_expansion() {
+            sub = sub.server(w.server_entry.text().to_string());
+        }
+
+        sub.build()
     }
     fn check_errors(&self) {
-        let imp = self.imp();
-        let topic_entry = imp.topic_entry.borrow().clone();
-        let sub_btn = imp.sub_btn.borrow().clone();
-        if let Err(_) = models::validate_topic(&topic_entry.delegate().unwrap().text()) {
-            topic_entry.add_css_class("error");
-            sub_btn.set_sensitive(false);
-        } else {
-            topic_entry.remove_css_class("error");
-            sub_btn.set_sensitive(true);
+        let w = { self.imp().widgets.borrow().clone() };
+        let sub = self.subscription();
+
+        w.server_entry.remove_css_class("error");
+        w.topic_entry.remove_css_class("error");
+        w.sub_btn.set_sensitive(true);
+
+        if let Err(errs) = sub {
+            w.sub_btn.set_sensitive(false);
+            for e in errs {
+                match e {
+                    ntfy_daemon::Error::InvalidTopic(_) => {
+                        w.topic_entry.add_css_class("error");
+                    }
+                    ntfy_daemon::Error::InvalidServer(_) => {
+                        w.server_entry.add_css_class("error");
+                    }
+                    _ => {}
+                }
+            }
         }
     }
     fn emit_subscribe_request(&self) {

src/widgets/window.rs

@@ -119,7 +119,15 @@ mod imp {
             let this = self.obj().clone();
             let dc = dialog.clone();
             dialog.connect_local("subscribe-request", true, move |_| {
-                this.add_subscription(&dc.server(), &dc.topic());
+                let sub = match dc.subscription() {
+                    Ok(sub) => sub,
+                    Err(e) => {
+                        warn!(errors = ?e, "trying to add invalid subscription");
+                        return None;
+                    }
+                };
+                this.add_subscription(sub);
+                dc.close();
                 None
             });
         }
@@ -246,11 +254,11 @@ impl NotifyWindow {
             });
     }
 
-    fn add_subscription(&self, server: &str, topic: &str) {
+    fn add_subscription(&self, sub: models::Subscription) {
         let mut req = self.notifier().subscribe_request();
 
-        req.get().set_server(server);
-        req.get().set_topic(topic);
+        req.get().set_server(&sub.server);
+        req.get().set_topic(&sub.topic);
         let res = req.send();
         let this = self.clone();
         self.spawn_with_near_toast(async move {