nobot fix

as specified in tests

CHANGELOG.md

@@ -1,5 +1,12 @@
 # Changelog
 
+## v0.4.5
+- Ignore #nobot in bio if the user is also a member
+
+## v0.4.4
+- Fix some failing tests
+- Lowercase the domain when normalizing an account
+
 ## v0.4.3
 - Fix hashtag not working in a mention
 

Cargo.lock

@@ -328,7 +328,7 @@ checksum = "e88a8acf291dafb59c2d96e8f59828f3838bb1a70398823ade51a84de6a6deed"
 
 [[package]]
 name = "fedigroups"
-version = "0.4.3"
+version = "0.4.4"
 dependencies = [
  "anyhow",
  "clap",

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "fedigroups"
-version = "0.4.3"
+version = "0.4.5"
 authors = ["Ondřej Hruška <ondra@ondrovo.com>"]
 edition = "2018"
 publish = false

src/error.rs

@@ -8,6 +8,8 @@ pub enum GroupError {
     UserIsBanned,
     #[error("User opted out from the group")]
     UserOptedOut,
+    #[error("User opted out from the group using #nobot")]
+    UserOptedOutNobot,
     #[error("Server could not be banned because there are admin users on it")]
     AdminsOnServer,
     #[error("Config error: {0}")]

src/group_handler/handle_mention.rs

@@ -2,25 +2,21 @@ use std::cmp::Ordering;
 use std::collections::HashSet;
 use std::time::Duration;
 
-use elefren::{FediClient, SearchType, StatusBuilder};
 use elefren::entities::account::Account;
 use elefren::entities::prelude::Status;
 use elefren::status_builder::Visibility;
+use elefren::{FediClient, SearchType, StatusBuilder};
 
-use crate::command::{RE_NOBOT_TAG, StatusCommand};
+use crate::command::{StatusCommand, RE_NOBOT_TAG};
 use crate::error::GroupError;
 use crate::group_handler::GroupHandle;
-use crate::store::CommonConfig;
 use crate::store::group_config::GroupConfig;
+use crate::store::CommonConfig;
 use crate::tr::TranslationTable;
 use crate::utils;
-use crate::utils::{LogError, normalize_acct, VisExt};
+use crate::utils::{normalize_acct, LogError, VisExt};
 
-use crate::{
-    grp_debug,
-    grp_warn,
-    grp_info
-};
+use crate::{grp_debug, grp_info, grp_warn};
 
 pub struct ProcessMention<'a> {
     status: Status,
@@ -51,7 +47,7 @@ impl<'a> ProcessMention<'a> {
             self.client
                 .search_v2(acct, !followed, Some(SearchType::Accounts), Some(1), followed),
         )
-            .await
+        .await
         {
             Err(_) => {
                 grp_warn!(self, "Account lookup timeout!");
@@ -98,9 +94,9 @@ impl<'a> ProcessMention<'a> {
         let mut to_add = String::new();
         for m in members {
             to_add.push_str(&if admins.contains(&m) {
-                crate::tr!(self, "user_list_entry_admin", user=m)
+                crate::tr!(self, "user_list_entry_admin", user = m)
             } else {
-                crate::tr!(self, "user_list_entry", user=m)
+                crate::tr!(self, "user_list_entry", user = m)
             });
         }
         self.add_reply(&to_add);
@@ -149,8 +145,7 @@ impl<'a> ProcessMention<'a> {
     }
 
     async fn reblog_status(&self) {
-        self.client.reblog(&self.status.id)
-            .await.log_error("Failed to reblog status");
+        self.client.reblog(&self.status.id).await.log_error("Failed to reblog status");
         self.delay_after_post().await;
     }
 
@@ -419,7 +414,7 @@ impl<'a> ProcessMention<'a> {
 
     async fn cmd_undo(&mut self) -> Result<(), GroupError> {
         if let (Some(ref parent_account_id), Some(ref parent_status_id)) =
-        (&self.status.in_reply_to_account_id, &self.status.in_reply_to_id)
+            (&self.status.in_reply_to_account_id, &self.status.in_reply_to_id)
         {
             if parent_account_id == &self.group_account.id {
                 // This is a post sent by the group user, likely an announcement.
@@ -675,7 +670,11 @@ impl<'a> ProcessMention<'a> {
         };
 
         if self.config.is_member_only() {
-            self.add_reply(crate::tr!(self, "help_group_info_closed", membership = &membership_line));
+            self.add_reply(crate::tr!(
+                self,
+                "help_group_info_closed",
+                membership = &membership_line
+            ));
         } else {
             self.add_reply(crate::tr!(self, "help_group_info_open", membership = &membership_line));
         }
@@ -709,7 +708,7 @@ impl<'a> ProcessMention<'a> {
         let mut to_add = String::new();
 
         for t in tags {
-            to_add.push_str(&crate::tr!(self, "tag_list_entry", tag=t));
+            to_add.push_str(&crate::tr!(self, "tag_list_entry", tag = t));
         }
         self.add_reply(to_add);
     }
@@ -774,11 +773,11 @@ impl<'a> ProcessMention<'a> {
         // Try to unfollow
         let account = self.client.get_account(id).await?;
         let bio = utils::strip_html(&account.note);
-        if RE_NOBOT_TAG.is_match(&bio) {
-            // #nobot
-            Err(GroupError::UserOptedOut)
+        let normalized = normalize_acct(&account.acct, &self.group_acct)?;
+        if RE_NOBOT_TAG.is_match(&bio) && !self.config.is_member(&normalized) {
+            // #nobot in a non-member account
+            Err(GroupError::UserOptedOutNobot)
         } else {
-            let normalized = normalize_acct(&account.acct, &self.group_acct)?;
             if self.config.is_banned(&normalized) {
                 Err(GroupError::UserIsBanned)
             } else if self.config.is_optout(&normalized) {

src/group_handler/mod.rs

@@ -42,7 +42,7 @@ pub struct GroupInternal {
 impl Default for GroupInternal {
     fn default() -> Self {
         Self {
-            recently_seen_notif_statuses: VecDeque::new()
+            recently_seen_notif_statuses: VecDeque::new(),
         }
     }
 }
@@ -552,8 +552,7 @@ impl GroupHandle {
 
             self.config.set_member(notif_acct, true).log_error("Fail add a member");
 
-            crate::tr!(self, "mention_prefix", user = notif_acct)
-                + &crate::tr!(self, "welcome_public")
+            crate::tr!(self, "mention_prefix", user = notif_acct) + &crate::tr!(self, "welcome_public")
         };
 
         let post = StatusBuilder::new()

src/main.rs

@@ -76,10 +76,8 @@ async fn main() -> anyhow::Result<()> {
 
     let default_level = 3;
 
-    let level = (
-        default_level as isize
-            + args.occurrences_of("verbose") as isize
-            - args.occurrences_of("quiet") as isize)
+    let level = (default_level as isize + args.occurrences_of("verbose") as isize
+        - args.occurrences_of("quiet") as isize)
         .clamp(0, LEVELS.len() as isize) as usize;
 
     env_logger::Builder::new()

src/store/common_config.rs

@@ -1,6 +1,6 @@
-use std::collections::HashMap;
 use crate::store::DEFAULT_LOCALE_NAME;
 use crate::tr::TranslationTable;
+use std::collections::HashMap;
 
 #[derive(Debug, Clone, Serialize, Deserialize)]
 #[serde(default, deny_unknown_fields)]
@@ -29,7 +29,7 @@ pub struct CommonConfig {
     /// This is a work-around for servers that stop sending notifs after a while.
     pub socket_retire_time_s: f64,
     #[serde(skip)]
-    pub tr : HashMap<String, TranslationTable>,
+    pub tr: HashMap<String, TranslationTable>,
 }
 
 impl Default for CommonConfig {
@@ -52,10 +52,10 @@ impl Default for CommonConfig {
 }
 
 impl CommonConfig {
-    pub fn tr(&self, lang : &str) -> &TranslationTable {
+    pub fn tr(&self, lang: &str) -> &TranslationTable {
         match self.tr.get(lang) {
             Some(tr) => tr,
-            None => self.tr.get(DEFAULT_LOCALE_NAME).expect("default locale is not loaded")
+            None => self.tr.get(DEFAULT_LOCALE_NAME).expect("default locale is not loaded"),
         }
     }
 }

src/store/group_config.rs

@@ -4,7 +4,7 @@ use std::path::{Path, PathBuf};
 use elefren::AppData;
 
 use crate::error::GroupError;
-use crate::store::{DEFAULT_LOCALE_NAME, CommonConfig};
+use crate::store::{CommonConfig, DEFAULT_LOCALE_NAME};
 use crate::tr::TranslationTable;
 
 #[derive(Debug, Clone, Serialize, Deserialize)]
@@ -206,7 +206,7 @@ async fn load_locale_override_file(locale_path: impl AsRef<Path>) -> Result<Opti
     let locale_path = locale_path.as_ref();
     if locale_path.is_file() {
         let f = tokio::fs::read(&locale_path).await?;
-        let opt : TranslationTable = json5::from_str(&String::from_utf8_lossy(&f))?;
+        let opt: TranslationTable = json5::from_str(&String::from_utf8_lossy(&f))?;
         Ok(Some(opt))
     } else {
         Ok(None)
@@ -259,7 +259,11 @@ impl GroupConfig {
     }
 
     /// (re)init using new authorization
-    pub(crate) async fn initialize_by_appdata(acct: String, appdata: AppData, group_dir: PathBuf) -> Result<(), GroupError> {
+    pub(crate) async fn initialize_by_appdata(
+        acct: String,
+        appdata: AppData,
+        group_dir: PathBuf,
+    ) -> Result<(), GroupError> {
         if !group_dir.is_dir() {
             debug!("Creating group directory");
             tokio::fs::create_dir_all(&group_dir).await?;
@@ -306,12 +310,17 @@ impl GroupConfig {
         /* state */
         let state = load_or_create_state_file(state_path).await?;
 
-        let g = GroupConfig { config, control, state, _group_tr: TranslationTable::new() };
+        let g = GroupConfig {
+            config,
+            control,
+            state,
+            _group_tr: TranslationTable::new(),
+        };
         g.warn_of_bad_config();
         Ok(())
     }
 
-    pub(crate) async fn from_dir(group_dir: PathBuf, cc : &CommonConfig) -> Result<Self, GroupError> {
+    pub(crate) async fn from_dir(group_dir: PathBuf, cc: &CommonConfig) -> Result<Self, GroupError> {
         let config_path = group_dir.join("config.json");
         let control_path = group_dir.join("control.json");
         let state_path = group_dir.join("state.json");
@@ -338,7 +347,12 @@ impl GroupConfig {
             }
         }
 
-        let g = GroupConfig { config, control, state, _group_tr: tr };
+        let g = GroupConfig {
+            config,
+            control,
+            state,
+            _group_tr: tr,
+        };
         g.warn_of_bad_config();
         Ok(g)
     }
@@ -458,7 +472,7 @@ impl GroupConfig {
     /// Check if the user's server is banned
     fn is_users_server_banned(&self, acct: &str) -> bool {
         let server = acct_to_server(acct);
-        self.is_server_banned(server)
+        self.is_server_banned(&server)
     }
 
     pub(crate) fn can_write(&self, acct: &str) -> bool {
@@ -574,8 +588,8 @@ impl GroupConfig {
     }
 }
 
-fn acct_to_server(acct: &str) -> &str {
-    acct.split('@').nth(1).unwrap_or_default()
+fn acct_to_server(acct: &str) -> String {
+    crate::utils::acct_to_server(acct).unwrap_or_default()
 }
 
 #[cfg(test)]
@@ -588,15 +602,15 @@ mod tests {
             config: Default::default(),
             control: Default::default(),
             state: Default::default(),
-            _group_tr: Default::default()
+            _group_tr: Default::default(),
         }
     }
 
     #[test]
     fn test_acct_to_server() {
-        assert_eq!("pikachu.rocks", acct_to_server("raichu@pikachu.rocks"));
-        assert_eq!("pikachu.rocks", acct_to_server("m@pikachu.rocks"));
-        assert_eq!("", acct_to_server("what"));
+        assert_eq!("pikachu.rocks".to_string(), acct_to_server("raichu@pikachu.rocks"));
+        assert_eq!("pikachu.rocks".to_string(), acct_to_server("m@pikachu.rocks"));
+        assert_eq!("".to_string(), acct_to_server("what"));
     }
 
     #[test]
@@ -666,13 +680,15 @@ mod tests {
         assert!(!group.is_banned("piggo@piggo.space"), "user not banned by default");
 
         group.ban_user("piggo@piggo.space", true).unwrap();
-        assert!(group.is_member("piggo@piggo.space"), "still member even if banned");
+        assert!(!group.is_member("piggo@piggo.space"), "banned user is kicked");
         assert!(group.is_banned("piggo@piggo.space"), "banned user is banned");
 
         assert!(!group.can_write("piggo@piggo.space"), "banned member can't post");
 
         // unban
         group.ban_user("piggo@piggo.space", false).unwrap();
+        assert!(!group.can_write("piggo@piggo.space"), "unbanned member is still kicked");
+        group.set_member("piggo@piggo.space", true).unwrap();
         assert!(group.can_write("piggo@piggo.space"), "un-ban works");
     }
 

src/store/mod.rs

@@ -9,9 +9,9 @@ use crate::group_handler::{GroupHandle, GroupInternal};
 
 pub mod common_config;
 pub mod group_config;
+use crate::tr::TranslationTable;
 pub use common_config::CommonConfig;
 pub use group_config::GroupConfig;
-use crate::tr::TranslationTable;
 
 #[derive(Debug, Default)]
 pub struct ConfigStore {
@@ -32,8 +32,8 @@ pub struct StoreOptions {
     pub store_dir: String,
 }
 
-const DEFAULT_LOCALE_NAME : &str = "en";
-const DEFAULT_LOCALE_JSON : &str = include_str!("../../locales/en.json");
+const DEFAULT_LOCALE_NAME: &str = "en";
+const DEFAULT_LOCALE_JSON: &str = include_str!("../../locales/en.json");
 
 impl ConfigStore {
     /// Create a new instance of the store.
@@ -214,7 +214,7 @@ impl ConfigStore {
                         Ok(f) => {
                             let locale_name = path.file_stem().unwrap_or_default().to_string_lossy();
                             self.load_locale(&locale_name, &String::from_utf8_lossy(&f), false);
-                        },
+                        }
                         Err(e) => {
                             error!("Failed to read locale file {}: {}", path.display(), e);
                         }
@@ -234,10 +234,12 @@ impl ConfigStore {
                 for (k, v) in def_tr.entries() {
                     if !tr.translation_exists(k) {
                         if self.config.validate_locales {
-                            warn!("locale \"{}\" is missing \"{}\", default: {:?}",
+                            warn!(
+                                "locale \"{}\" is missing \"{}\", default: {:?}",
                                 locale_name,
                                 k,
-                                def_tr.get_translation_raw(k).unwrap());
+                                def_tr.get_translation_raw(k).unwrap()
+                            );
                         }
                         tr.add_translation(k, v);
                     }

src/tr.rs

@@ -2,7 +2,7 @@
 
 use std::collections::HashMap;
 
-#[derive(Debug,Clone,Serialize,Deserialize,Default)]
+#[derive(Debug, Clone, Serialize, Deserialize, Default)]
 pub struct TranslationTable {
     #[serde(flatten)]
     entries: HashMap<String, String>,
@@ -15,24 +15,24 @@ impl TranslationTable {
     }
 
     /// Iterate all entries
-    pub fn entries(&self) -> impl Iterator<Item=(&String, &String)> {
+    pub fn entries(&self) -> impl Iterator<Item = (&String, &String)> {
         self.entries.iter()
     }
 
-    pub fn get_translation_raw(&self, key : &str) -> Option<&str> {
+    pub fn get_translation_raw(&self, key: &str) -> Option<&str> {
         self.entries.get(key).map(|s| s.as_str())
     }
 
     /// Add or update a translation
-    pub fn add_translation(&mut self, key : impl ToString, subs : impl ToString) {
+    pub fn add_translation(&mut self, key: impl ToString, subs: impl ToString) {
         self.entries.insert(key.to_string(), subs.to_string());
     }
 
-    pub fn translation_exists(&self, key : &str) -> bool {
+    pub fn translation_exists(&self, key: &str) -> bool {
         self.entries.contains_key(key)
     }
 
-    pub fn subs(&self, key : &str, substitutions: &[&str]) -> String {
+    pub fn subs(&self, key: &str, substitutions: &[&str]) -> String {
         match self.entries.get(key) {
             Some(s) => {
                 // TODO optimize
@@ -45,7 +45,7 @@ impl TranslationTable {
                 }
                 s
             }
-            None => key.to_owned()
+            None => key.to_owned(),
         }
     }
 }
@@ -56,12 +56,11 @@ mod tests {
 
     #[test]
     fn deser_tr_table() {
-        let tr : TranslationTable = serde_json::from_str(r#"{"foo":"bar"}"#).unwrap();
+        let tr: TranslationTable = serde_json::from_str(r#"{"foo":"bar"}"#).unwrap();
         assert_eq!("bar", tr.subs("foo", &[]));
         assert_eq!("xxx", tr.subs("xxx", &[]));
     }
 
-
     #[test]
     fn subs() {
         let mut tr = TranslationTable::new();

src/utils.rs

@@ -19,8 +19,8 @@ impl<V, E: Error> LogError for Result<V, E> {
     }
 }
 
-pub(crate) fn acct_to_server(acct: &str) -> Option<&str> {
-    acct.trim_start_matches('@').split('@').nth(1)
+pub(crate) fn acct_to_server(acct: &str) -> Option<String> {
+    acct.trim_start_matches('@').split('@').nth(1).map(|s| s.to_lowercase())
 }
 
 pub(crate) fn normalize_acct(acct: &str, group: &str) -> Result<String, GroupError> {
@@ -45,8 +45,8 @@ mod test {
 
     #[test]
     fn test_acct_to_server() {
-        assert_eq!(Some("novak"), acct_to_server("pepa@novak"));
-        assert_eq!(Some("banana.co.uk"), acct_to_server("@pepa@banana.co.uk"));
+        assert_eq!(Some("novak".to_string()), acct_to_server("pepa@novak"));
+        assert_eq!(Some("banana.co.uk".to_string()), acct_to_server("@pepa@banana.co.uk"));
         assert_eq!(None, acct_to_server("probably_local"));
     }
 
@@ -82,11 +82,11 @@ mod test {
         );
         assert_eq!(
             Ok("piggo@piggo.space".into()),
-            normalize_acct("piGGgo@pIggo.spaCe", "uhh")
+            normalize_acct("piGGo@pIggo.spaCe", "uhh")
         );
         assert_eq!(
             Ok("piggo@banana.nana".into()),
-            normalize_acct("piGGgo", "foo@baNANA.nana")
+            normalize_acct("piGGo", "foo@baNANA.nana")
         );
         assert_eq!(Err(GroupError::BadConfig("_".into())), normalize_acct("piggo", "uhh"));
     }