Fix dialog validation on submit

koofr · Oct 13, 2023 · e9a7f23 · e9a7f23
1 parent 6027323
commit e9a7f23
Show file tree

Hide file tree

Showing 5 changed files with 243 additions and 110 deletions.
diff --git a/vault-core-tests/tests/integration/repo_files_browsers_tests.rs b/vault-core-tests/tests/integration/repo_files_browsers_tests.rs
@@ -1,7 +1,8 @@
-use futures::FutureExt;
+use futures::{join, FutureExt};
 use similar_asserts::assert_eq;
 use vault_core::{
     common::state::Status,
+    dialogs,
     repo_files::{
         errors::LoadFilesError,
         state::{RepoFilesSort, RepoFilesSortField},
@@ -391,3 +392,102 @@ fn expected_browsers_state(
         next_id: NextId(2),
     }
 }
+
+#[test]
+fn test_create_dir() {
+    with_repo(|fixture| {
+        async move {
+            let (browser_id, load_future) = fixture.vault.repo_files_browsers_create(
+                &fixture.repo_id,
+                "/",
+                RepoFilesBrowserOptions { select_name: None },
+            );
+            load_future.await.unwrap();
+
+            let create_dir_future = fixture.vault.repo_files_browsers_create_dir(browser_id);
+
+            let dialog_vault = fixture.vault.clone();
+            let dialog_future = fixture.fake_remote.tokio_runtime.spawn(async move {
+                let wait_store = dialog_vault.store.clone();
+                let dialog_id =
+                    store::wait_for(wait_store.clone(), &[store::Event::Dialogs], move || {
+                        wait_store.with_state(|state| {
+                            dialogs::selectors::select_dialogs(state)
+                                .iter()
+                                .next()
+                                .map(|dialog| dialog.id.clone())
+                        })
+                    })
+                    .await;
+
+                dialog_vault.dialogs_set_input_value(dialog_id, "dir".into());
+
+                dialog_vault.dialogs_confirm(dialog_id);
+            });
+
+            let (create_dir_res, _) = join!(create_dir_future, dialog_future);
+            let (name, path) = create_dir_res.unwrap();
+
+            assert_eq!(name, "dir");
+            assert_eq!(path, "/dir");
+
+            fixture.vault.repo_files_browsers_destroy(browser_id);
+        }
+        .boxed()
+    });
+}
+
+#[test]
+fn test_create_dir_validation() {
+    with_repo(|fixture| {
+        async move {
+            let (browser_id, load_future) = fixture.vault.repo_files_browsers_create(
+                &fixture.repo_id,
+                "/",
+                RepoFilesBrowserOptions { select_name: None },
+            );
+            load_future.await.unwrap();
+
+            let create_dir_future = fixture.vault.repo_files_browsers_create_dir(browser_id);
+
+            let dialog_vault = fixture.vault.clone();
+            let dialog_future = fixture.fake_remote.tokio_runtime.spawn(async move {
+                let wait_store = dialog_vault.store.clone();
+                let dialog_id =
+                    store::wait_for(wait_store.clone(), &[store::Event::Dialogs], move || {
+                        wait_store.with_state(|state| {
+                            dialogs::selectors::select_dialogs(state)
+                                .iter()
+                                .next()
+                                .map(|dialog| dialog.id.clone())
+                        })
+                    })
+                    .await;
+
+                dialog_vault.dialogs_set_input_value(dialog_id, "/".into());
+
+                assert!(!dialog_vault.store.with_state(|state| {
+                    dialogs::selectors::select_dialog(state, dialog_id)
+                        .unwrap()
+                        .is_input_value_valid
+                }));
+
+                dialog_vault.dialogs_confirm(dialog_id);
+
+                assert!(dialog_vault.store.with_state(|state| {
+                    dialogs::selectors::select_dialog(state, dialog_id).is_none()
+                }));
+            });
+
+            let (create_dir_res, _) = join!(create_dir_future, dialog_future);
+
+            assert_eq!(
+                create_dir_res.unwrap_err().to_string(),
+                "Invalid name or path"
+            );
+
+            fixture.vault.repo_files_browsers_destroy(browser_id);
+        }
+        .boxed()
+    });
+}
diff --git a/vault-core/src/dialogs/service.rs b/vault-core/src/dialogs/service.rs
@@ -15,7 +15,7 @@ use super::{
 pub struct DialogsService {
     store: Arc<store::Store>,
     input_value_validators:
-        Arc<RwLock<HashMap<u32, Box<dyn Fn(&str) -> bool + Send + Sync + 'static>>>>,
+        Arc<RwLock<HashMap<u32, Box<dyn Fn(String) -> bool + Send + Sync + 'static>>>>,
     results: Arc<RwLock<HashMap<u32, oneshot::Sender<Option<String>>>>>,
 }
 
@@ -34,7 +34,6 @@ impl DialogsService {
             title,
             message: None,
             input_value: String::from(""),
-            input_value_validator: None,
             input_value_selected: None,
             input_placeholder: None,
             confirm_button_text: String::from("Ok"),
@@ -49,7 +48,6 @@ impl DialogsService {
             title: String::from("Are you sure?"),
             message: None,
             input_value: String::from(""),
-            input_value_validator: None,
             input_value_selected: None,
             input_placeholder: None,
             confirm_button_text: String::from("Yes"),
@@ -64,7 +62,6 @@ impl DialogsService {
             title,
             message: None,
             input_value: String::from(""),
-            input_value_validator: None,
             input_value_selected: None,
             input_placeholder: None,
             confirm_button_text: String::from("Ok"),
@@ -74,28 +71,57 @@ impl DialogsService {
     }
 
     pub async fn show(&self, options: DialogShowOptions) -> Option<String> {
-        let mut options = options;
+        let dialog_id = self.store.mutate(|state, notify, _, _| {
+            notify(store::Event::Dialogs);
 
-        let input_value_validator = options.input_value_validator.take();
+            mutations::get_next_id(state)
+        });
 
+        let (result_sender, result_receiver) = oneshot::channel();
+
+        self.results
+            .write()
+            .unwrap()
+            .insert(dialog_id, result_sender);
+
+        self.store.mutate(|state, notify, _, _| {
+            notify(store::Event::Dialogs);
+
+            mutations::show(state, dialog_id, options, true)
+        });
+
+        result_receiver.await.unwrap()
+    }
+
+    pub async fn show_validator<Error, Validator>(
+        &self,
+        options: DialogShowOptions,
+        input_value_validator: Validator,
+    ) -> Option<Result<String, Error>>
+    where
+        Error: std::error::Error,
+        Validator: Fn(String) -> Result<String, Error> + Send + Sync + 'static,
+    {
         let dialog_id = self.store.mutate(|state, notify, _, _| {
             notify(store::Event::Dialogs);
 
             mutations::get_next_id(state)
         });
 
-        let is_input_value_valid = match input_value_validator {
-            Some(input_value_validator) => {
-                let is_input_value_valid = input_value_validator(&options.input_value);
+        let input_value_validator = Arc::new(input_value_validator);
 
-                self.input_value_validators
-                    .write()
-                    .unwrap()
-                    .insert(dialog_id, input_value_validator);
+        let is_input_value_valid = {
+            let validator = input_value_validator.clone();
+            let validator = Box::new(move |value| (validator.as_ref())(value).is_ok());
 
-                is_input_value_valid
-            }
-            None => true,
+            let is_input_value_valid = validator(options.input_value.clone());
+
+            self.input_value_validators
+                .write()
+                .unwrap()
+                .insert(dialog_id, validator);
+
+            is_input_value_valid
         };
 
         let (result_sender, result_receiver) = oneshot::channel();
@@ -111,7 +137,10 @@ impl DialogsService {
             mutations::show(state, dialog_id, options, is_input_value_valid)
         });
 
-        result_receiver.await.unwrap()
+        result_receiver
+            .await
+            .unwrap()
+            .map(|value| (input_value_validator.as_ref())(value))
     }
 
     pub fn remove(&self, dialog_id: u32) {
@@ -128,7 +157,6 @@ impl DialogsService {
     }
 
     pub fn confirm(&self, dialog_id: u32) {
-        // TODO check confirm_button_enabled and is_input_value_valid
         let value = match self.store.with_state(|state| {
             selectors::select_dialog(state, dialog_id).map(|dialog| dialog.input_value.clone())
         }) {
@@ -152,12 +180,14 @@ impl DialogsService {
     }
 
     pub fn set_input_value(&self, dialog_id: u32, value: String) {
-        let input_value_validators = self.input_value_validators.read().unwrap();
-        let input_value_validator = input_value_validators.get(&dialog_id);
-        let is_valid = input_value_validator
-            .map(|validator| validator(&value))
-            .unwrap_or(true);
-        drop(input_value_validators);
+        let is_valid = {
+            let input_value_validators = self.input_value_validators.read().unwrap();
+            let input_value_validator = input_value_validators.get(&dialog_id);
+
+            input_value_validator
+                .map(|validator| validator(value.clone()))
+                .unwrap_or(true)
+        };
 
         self.store.mutate(|state, notify, _, _| {
             notify(store::Event::Dialogs);

diff --git a/vault-core/src/dialogs/state.rs b/vault-core/src/dialogs/state.rs
@@ -20,7 +20,6 @@ pub struct DialogShowOptions {
     pub title: String,
     pub message: Option<String>,
     pub input_value: String,
-    pub input_value_validator: Option<Box<dyn Fn(&str) -> bool + Send + Sync + 'static>>,
     pub input_value_selected: Option<String>,
     pub input_placeholder: Option<String>,
     pub confirm_button_text: String,

diff --git a/vault-core/src/remote_files/service.rs b/vault-core/src/remote_files/service.rs
@@ -214,28 +214,29 @@ impl RemoteFilesService {
 
         let name = match self
             .dialogs_service
-            .show(dialogs::state::DialogShowOptions {
-                input_value_validator: Some(Box::new(move |value| {
-                    input_value_validator_store
-                        .with_state(|state| {
-                            selectors::select_check_new_name_valid(
-                                state,
-                                &input_value_validator_mount_id,
-                                &input_value_validator_parent_path,
-                                value,
-                            )
-                        })
-                        .is_ok()
-                })),
-                input_placeholder: Some(String::from("Folder name")),
-                confirm_button_text: String::from("Create folder"),
-                ..self
-                    .dialogs_service
-                    .build_prompt(String::from("Enter new folder name"))
-            })
+            .show_validator(
+                dialogs::state::DialogShowOptions {
+                    input_placeholder: Some(String::from("Folder name")),
+                    confirm_button_text: String::from("Create folder"),
+                    ..self
+                        .dialogs_service
+                        .build_prompt(String::from("Enter new folder name"))
+                },
+                move |value| {
+                    input_value_validator_store.with_state(|state| {
+                        selectors::select_check_new_name_valid(
+                            state,
+                            &input_value_validator_mount_id,
+                            &input_value_validator_parent_path,
+                            &value,
+                        )
+                        .map(|_| value)
+                    })
+                },
+            )
             .await
         {
-            Some(name) => name,
+            Some(name) => name?,
             None => return Err(CreateDirError::Canceled),
         };