Refactored to require handlers to specify the components they rely on and to specify when they are ready. This fixes a lot of bugs with the UI when users try to press buttons while the application is still loading.

2024-07-17 19:55:10 -06:00
parent 9104b7c356
commit d84e7dfcab
49 changed files with 5143 additions and 265 deletions
@@ -1,5 +1,11 @@
 #[cfg(test)]
 mod tests {
+  use std::cmp::Ordering;
+
+  use chrono::DateTime;
+  use pretty_assertions::{assert_eq, assert_str_eq};
+  use strum::IntoEnumIterator;
+
  use crate::app::key_binding::DEFAULT_KEYBINDINGS;
  use crate::app::App;
  use crate::event::Key;
@@ -10,10 +16,6 @@ mod tests {
  };
  use crate::models::servarr_data::radarr::radarr_data::{ActiveRadarrBlock, BLOCKLIST_BLOCKS};
  use crate::models::stateful_table::SortOption;
-  use chrono::DateTime;
-  use pretty_assertions::{assert_eq, assert_str_eq};
-  use std::cmp::Ordering;
-  use strum::IntoEnumIterator;

  mod test_handle_scroll_up_and_down {
    use pretty_assertions::{assert_eq, assert_str_eq};
@@ -35,6 +37,49 @@ mod tests {
      to_string
    );

+    #[rstest]
+    fn test_blocklist_scroll_no_op_when_not_ready(
+      #[values(DEFAULT_KEYBINDINGS.up.key, DEFAULT_KEYBINDINGS.down.key)] key: Key,
+    ) {
+      let mut app = App::default();
+      app.is_loading = true;
+      app
+        .data
+        .radarr_data
+        .blocklist
+        .set_items(simple_stateful_iterable_vec!(
+          BlocklistItem,
+          String,
+          source_title
+        ));
+
+      BlocklistHandler::with(&key, &mut app, &ActiveRadarrBlock::Blocklist, &None).handle();
+
+      assert_str_eq!(
+        app
+          .data
+          .radarr_data
+          .blocklist
+          .current_selection()
+          .source_title
+          .to_string(),
+        "Test 1"
+      );
+
+      BlocklistHandler::with(&key, &mut app, &ActiveRadarrBlock::Blocklist, &None).handle();
+
+      assert_str_eq!(
+        app
+          .data
+          .radarr_data
+          .blocklist
+          .current_selection()
+          .source_title
+          .to_string(),
+        "Test 1"
+      );
+    }
+
    #[rstest]
    fn test_blocklist_sort_scroll(
      #[values(DEFAULT_KEYBINDINGS.up.key, DEFAULT_KEYBINDINGS.down.key)] key: Key,
@@ -92,10 +137,12 @@ mod tests {
  }

  mod test_handle_home_end {
-    use super::*;
+    use pretty_assertions::{assert_eq, assert_str_eq};
+
    use crate::models::radarr_models::BlocklistItem;
    use crate::{extended_stateful_iterable_vec, test_iterable_home_and_end};
-    use pretty_assertions::{assert_eq, assert_str_eq};
+
+    use super::*;

    test_iterable_home_and_end!(
      test_blocklist_home_and_end,
@@ -108,6 +155,59 @@ mod tests {
      to_string
    );

+    #[test]
+    fn test_blocklist_home_and_end_no_op_when_not_ready() {
+      let mut app = App::default();
+      app.is_loading = true;
+      app
+        .data
+        .radarr_data
+        .blocklist
+        .set_items(extended_stateful_iterable_vec!(
+          BlocklistItem,
+          String,
+          source_title
+        ));
+
+      BlocklistHandler::with(
+        &DEFAULT_KEYBINDINGS.end.key,
+        &mut app,
+        &ActiveRadarrBlock::Blocklist,
+        &None,
+      )
+      .handle();
+
+      assert_str_eq!(
+        app
+          .data
+          .radarr_data
+          .blocklist
+          .current_selection()
+          .source_title
+          .to_string(),
+        "Test 1"
+      );
+
+      BlocklistHandler::with(
+        &DEFAULT_KEYBINDINGS.home.key,
+        &mut app,
+        &ActiveRadarrBlock::Blocklist,
+        &None,
+      )
+      .handle();
+
+      assert_str_eq!(
+        app
+          .data
+          .radarr_data
+          .blocklist
+          .current_selection()
+          .source_title
+          .to_string(),
+        "Test 1"
+      );
+    }
+
    #[test]
    fn test_blocklist_sort_home_end() {
      let blocklist_field_vec = sort_options();
@@ -157,30 +257,51 @@ mod tests {
  }

  mod test_handle_delete {
-    use super::*;
-    use crate::assert_delete_prompt;
    use pretty_assertions::assert_eq;

+    use super::*;
+
    const DELETE_KEY: Key = DEFAULT_KEYBINDINGS.delete.key;

    #[test]
    fn test_delete_blocklist_item_prompt() {
-      assert_delete_prompt!(
-        BlocklistHandler,
-        ActiveRadarrBlock::Blocklist,
-        ActiveRadarrBlock::DeleteBlocklistItemPrompt
+      let mut app = App::default();
+      app.data.radarr_data.blocklist.set_items(blocklist_vec());
+
+      BlocklistHandler::with(&DELETE_KEY, &mut app, &ActiveRadarrBlock::Blocklist, &None).handle();
+
+      assert_eq!(
+        app.get_current_route(),
+        &ActiveRadarrBlock::DeleteBlocklistItemPrompt.into()
+      );
+    }
+
+    #[test]
+    fn test_delete_blocklist_item_no_op_when_not_ready() {
+      let mut app = App::default();
+      app.is_loading = true;
+      app.push_navigation_stack(ActiveRadarrBlock::Blocklist.into());
+      app.data.radarr_data.blocklist.set_items(blocklist_vec());
+
+      BlocklistHandler::with(&DELETE_KEY, &mut app, &ActiveRadarrBlock::Blocklist, &None).handle();
+
+      assert_eq!(
+        app.get_current_route(),
+        &ActiveRadarrBlock::Blocklist.into()
      );
    }
  }

  mod test_handle_left_right_action {
-    use super::*;
    use pretty_assertions::assert_eq;
    use rstest::rstest;

-    #[test]
-    fn test_blocklist_tab_left() {
+    use super::*;
+
+    #[rstest]
+    fn test_blocklist_tab_left(#[values(true, false)] is_ready: bool) {
      let mut app = App::default();
+      app.is_loading = is_ready;
      app.data.radarr_data.main_tabs.set_index(3);

      BlocklistHandler::with(
@@ -201,9 +322,10 @@ mod tests {
      );
    }

-    #[test]
-    fn test_blocklist_tab_right() {
+    #[rstest]
+    fn test_blocklist_tab_right(#[values(true, false)] is_ready: bool) {
      let mut app = App::default();
+      app.is_loading = is_ready;
      app.data.radarr_data.main_tabs.set_index(3);

      BlocklistHandler::with(
@@ -246,14 +368,44 @@ mod tests {
  }

  mod test_handle_submit {
-    use crate::network::radarr_network::RadarrEvent;
    use pretty_assertions::assert_eq;
    use rstest::rstest;

+    use crate::network::radarr_network::RadarrEvent;
+
    use super::*;

    const SUBMIT_KEY: Key = DEFAULT_KEYBINDINGS.submit.key;

+    #[test]
+    fn test_blocklist_submit() {
+      let mut app = App::default();
+      app.data.radarr_data.blocklist.set_items(blocklist_vec());
+      app.push_navigation_stack(ActiveRadarrBlock::Blocklist.into());
+
+      BlocklistHandler::with(&SUBMIT_KEY, &mut app, &ActiveRadarrBlock::Blocklist, &None).handle();
+
+      assert_eq!(
+        app.get_current_route(),
+        &ActiveRadarrBlock::BlocklistItemDetails.into()
+      );
+    }
+
+    #[test]
+    fn test_blocklist_submit_no_op_when_not_ready() {
+      let mut app = App::default();
+      app.is_loading = true;
+      app.data.radarr_data.blocklist.set_items(blocklist_vec());
+      app.push_navigation_stack(ActiveRadarrBlock::Blocklist.into());
+
+      BlocklistHandler::with(&SUBMIT_KEY, &mut app, &ActiveRadarrBlock::Blocklist, &None).handle();
+
+      assert_eq!(
+        app.get_current_route(),
+        &ActiveRadarrBlock::Blocklist.into()
+      );
+    }
+
    #[rstest]
    #[case(
      ActiveRadarrBlock::Blocklist,
@@ -271,6 +423,7 @@ mod tests {
      #[case] expected_action: RadarrEvent,
    ) {
      let mut app = App::default();
+      app.data.radarr_data.blocklist.set_items(blocklist_vec());
      app.data.radarr_data.prompt_confirm = true;
      app.push_navigation_stack(base_route.into());
      app.push_navigation_stack(prompt_block.into());
@@ -294,6 +447,7 @@ mod tests {
      prompt_block: ActiveRadarrBlock,
    ) {
      let mut app = App::default();
+      app.data.radarr_data.blocklist.set_items(blocklist_vec());
      app.push_navigation_stack(ActiveRadarrBlock::Blocklist.into());
      app.push_navigation_stack(prompt_block.into());

@@ -337,11 +491,13 @@ mod tests {
  }

  mod test_handle_esc {
-    use super::*;
-    use crate::handlers::radarr_handlers::downloads::DownloadsHandler;
    use pretty_assertions::assert_eq;
    use rstest::rstest;

+    use crate::handlers::radarr_handlers::downloads::DownloadsHandler;
+
+    use super::*;
+
    const ESC_KEY: Key = DEFAULT_KEYBINDINGS.esc.key;

    #[rstest]
@@ -408,9 +564,10 @@ mod tests {
      );
    }

-    #[test]
-    fn test_default_esc() {
+    #[rstest]
+    fn test_default_esc(#[values(true, false)] is_ready: bool) {
      let mut app = App::default();
+      app.is_loading = is_ready;
      app.error = "test error".to_owned().into();
      app.push_navigation_stack(ActiveRadarrBlock::Blocklist.into());
      app.push_navigation_stack(ActiveRadarrBlock::Blocklist.into());
@@ -426,18 +583,57 @@ mod tests {
  }

  mod test_handle_key_char {
-    use super::*;
-    use crate::assert_refresh_key;
    use pretty_assertions::assert_eq;

+    use super::*;
+
    #[test]
    fn test_refresh_blocklist_key() {
-      assert_refresh_key!(BlocklistHandler, ActiveRadarrBlock::Blocklist);
+      let mut app = App::default();
+      app.data.radarr_data.blocklist.set_items(blocklist_vec());
+      app.push_navigation_stack(ActiveRadarrBlock::Blocklist.into());
+
+      BlocklistHandler::with(
+        &DEFAULT_KEYBINDINGS.refresh.key,
+        &mut app,
+        &ActiveRadarrBlock::Blocklist,
+        &None,
+      )
+      .handle();
+
+      assert_eq!(
+        app.get_current_route(),
+        &ActiveRadarrBlock::Blocklist.into()
+      );
+      assert!(app.should_refresh);
+    }
+
+    #[test]
+    fn test_refresh_blocklist_key_no_op_when_not_ready() {
+      let mut app = App::default();
+      app.is_loading = true;
+      app.data.radarr_data.blocklist.set_items(blocklist_vec());
+      app.push_navigation_stack(ActiveRadarrBlock::Blocklist.into());
+
+      BlocklistHandler::with(
+        &DEFAULT_KEYBINDINGS.refresh.key,
+        &mut app,
+        &ActiveRadarrBlock::Blocklist,
+        &None,
+      )
+      .handle();
+
+      assert_eq!(
+        app.get_current_route(),
+        &ActiveRadarrBlock::Blocklist.into()
+      );
+      assert!(!app.should_refresh);
    }

    #[test]
    fn test_clear_blocklist_key() {
      let mut app = App::default();
+      app.data.radarr_data.blocklist.set_items(blocklist_vec());

      BlocklistHandler::with(
        &DEFAULT_KEYBINDINGS.clear.key,
@@ -453,9 +649,31 @@ mod tests {
      );
    }

+    #[test]
+    fn test_clear_blocklist_key_no_op_when_not_ready() {
+      let mut app = App::default();
+      app.is_loading = true;
+      app.push_navigation_stack(ActiveRadarrBlock::Blocklist.into());
+      app.data.radarr_data.blocklist.set_items(blocklist_vec());
+
+      BlocklistHandler::with(
+        &DEFAULT_KEYBINDINGS.clear.key,
+        &mut app,
+        &ActiveRadarrBlock::Blocklist,
+        &None,
+      )
+      .handle();
+
+      assert_eq!(
+        app.get_current_route(),
+        &ActiveRadarrBlock::Blocklist.into()
+      );
+    }
+
    #[test]
    fn test_sort_key() {
      let mut app = App::default();
+      app.data.radarr_data.blocklist.set_items(blocklist_vec());

      BlocklistHandler::with(
        &DEFAULT_KEYBINDINGS.sort.key,
@@ -475,6 +693,29 @@ mod tests {
      );
      assert!(!app.data.radarr_data.blocklist.sort_asc);
    }
+
+    #[test]
+    fn test_sort_key_no_op_when_not_ready() {
+      let mut app = App::default();
+      app.is_loading = true;
+      app.push_navigation_stack(ActiveRadarrBlock::Blocklist.into());
+      app.data.radarr_data.blocklist.set_items(blocklist_vec());
+
+      BlocklistHandler::with(
+        &DEFAULT_KEYBINDINGS.sort.key,
+        &mut app,
+        &ActiveRadarrBlock::Blocklist,
+        &None,
+      )
+      .handle();
+
+      assert_eq!(
+        app.get_current_route(),
+        &ActiveRadarrBlock::Blocklist.into()
+      );
+      assert!(app.data.radarr_data.blocklist.sort.is_none());
+      assert!(!app.data.radarr_data.blocklist.sort_asc);
+    }
  }

  #[test]
@@ -612,6 +853,67 @@ mod tests {
    assert_str_eq!(sort_option.name, "Date");
  }

+  #[test]
+  fn test_blocklist_handler_accepts() {
+    ActiveRadarrBlock::iter().for_each(|active_radarr_block| {
+      if BLOCKLIST_BLOCKS.contains(&active_radarr_block) {
+        assert!(BlocklistHandler::accepts(&active_radarr_block));
+      } else {
+        assert!(!BlocklistHandler::accepts(&active_radarr_block));
+      }
+    })
+  }
+
+  #[test]
+  fn test_blocklist_handler_not_ready_when_loading() {
+    let mut app = App::default();
+    app.is_loading = true;
+
+    let handler = BlocklistHandler::with(
+      &DEFAULT_KEYBINDINGS.esc.key,
+      &mut app,
+      &ActiveRadarrBlock::Blocklist,
+      &None,
+    );
+
+    assert!(!handler.is_ready());
+  }
+
+  #[test]
+  fn test_blocklist_handler_not_ready_when_blocklist_is_empty() {
+    let mut app = App::default();
+    app.is_loading = false;
+
+    let handler = BlocklistHandler::with(
+      &DEFAULT_KEYBINDINGS.esc.key,
+      &mut app,
+      &ActiveRadarrBlock::Blocklist,
+      &None,
+    );
+
+    assert!(!handler.is_ready());
+  }
+
+  #[test]
+  fn test_blocklist_handler_ready_when_not_loading_and_blocklist_is_not_empty() {
+    let mut app = App::default();
+    app.is_loading = false;
+    app
+      .data
+      .radarr_data
+      .blocklist
+      .set_items(vec![BlocklistItem::default()]);
+
+    let handler = BlocklistHandler::with(
+      &DEFAULT_KEYBINDINGS.esc.key,
+      &mut app,
+      &ActiveRadarrBlock::Blocklist,
+      &None,
+    );
+
+    assert!(handler.is_ready());
+  }
+
  fn blocklist_vec() -> Vec<BlocklistItem> {
    vec![
      BlocklistItem {
@@ -692,15 +994,4 @@ mod tests {
      }),
    }]
  }
-
-  #[test]
-  fn test_blocklist_handler_accepts() {
-    ActiveRadarrBlock::iter().for_each(|active_radarr_block| {
-      if BLOCKLIST_BLOCKS.contains(&active_radarr_block) {
-        assert!(BlocklistHandler::accepts(&active_radarr_block));
-      } else {
-        assert!(!BlocklistHandler::accepts(&active_radarr_block));
-      }
-    })
-  }
 }
@@ -43,6 +43,10 @@ impl<'a, 'b> KeyEventHandler<'a, 'b, ActiveRadarrBlock> for BlocklistHandler<'a,
    self.key
  }

+  fn is_ready(&self) -> bool {
+    !self.app.is_loading && !self.app.data.radarr_data.blocklist.is_empty()
+  }
+
  fn handle_scroll_up(&mut self) {
    match self.active_radarr_block {
      ActiveRadarrBlock::Blocklist => self.app.data.radarr_data.blocklist.scroll_up(),