Bug: sort_order column was set to 0 when creating a task. Thank you tests

sql/migrations.sql

@@ -5,7 +5,7 @@ create table if not exists task
     id integer primary key autoincrement,
     title text not null,
     description text not null,
-    sort_order integer not null default 0,
+    sort_order integer not null,
     column_id integer,
     foreign key (column_id) references kb_column(id)
 );

src/app.rs

@@ -218,12 +218,13 @@ impl<'a> State<'a> {
             } else {
                 column.selected_task_idx -= 1;
             }
+            let task_idx = column.selected_task_idx;
 
             let col_id = column.id;
-            self.db_conn.swap_task_order(t2_id, t1_id)?;
+            self.db_conn.swap_task_order(t1_id, t2_id)?;
             self.db_conn
                 .set_selected_task_for_column(task_idx, col_id)?;
-        }
+       }
         Ok(())
     }
 

src/db.rs

@@ -86,7 +86,13 @@ impl DBConn {
         column_id: i64,
     ) -> Result<Task> {
         let mut stmt =
-            self.prepare("insert into task(title, description, column_id) values (?1, ?2, ?3)")?;
+            self.prepare(
+                "insert into task(title, description, column_id, sort_order)
+                values (?1, ?2, ?3,
+                  (coalesce(1 +
+                    (select sort_order from task
+                     where column_id = ?3 order by sort_order desc limit 1),
+                  0)))")?;
         stmt.execute(params![title, description, column_id])?;
         let id = self.last_insert_rowid();
         Ok(Task {
@@ -135,42 +141,56 @@ impl DBConn {
                   where column_id = ?2 order by sort_order desc limit 1),
                   0)
              where task.id = ?1",
-            )
+            )?;
-            .unwrap();
+        stmt.execute((&task.id, &target_column.id))?;
-        stmt.execute((&task.id, &target_column.id)).unwrap();
         self.set_selected_task_for_column(target_column.selected_task_idx, target_column.id)?;
         Ok(())
     }
 
+    /// This is a helper function in case we need to debug sort_order, because I ran into
+    /// a bug when I forgot to insert the sort_order when creating a task
+    #[allow(dead_code)]
+    fn get_sort_order(&self) -> Result<Vec<(i32, String, usize)>> {
+        let mut stmt = self.prepare(
+            "select id,title,sort_order from task where column_id = 1")?;
+        let mut rows = stmt.query(())?;
+
+        let mut tasks = Vec::new();
+        while let Some(row) = rows.next()? {
+            tasks.push((row.get(0)?, row.get(1)?, row.get(2)?,))
+        }
+        Ok(tasks)
+    }
+
     /// .
     ///
     /// # Panics
     ///
     /// Panics if something goes wrong with the SQL
     pub fn swap_task_order(&mut self, task1_id: i64, task2_id: i64) -> Result<()> {
-        let tx = self.transaction().unwrap();
+        let tx = self.transaction()?;
 
         tx.execute(
             "create temp table temp_order as select sort_order from task where id = ?1",
             [&task1_id],
-        )
+        )?;
-        .unwrap();
 
         tx.execute(
-            "update task set sort_order = (select sort_order from task where id = ?2)
+            "update task
-         where id = ?1",
+             set sort_order = (select sort_order from task where id = ?2)
+             where id = ?1",
             (task1_id, task2_id),
-        )
+        )?;
-        .unwrap();
 
         tx.execute(
             "update task set sort_order = (select sort_order from temp_order) where id = ?1",
             [&task2_id],
-        )
+        )?;
-        .unwrap();
+
-        tx.execute("drop table temp_order", ()).unwrap();
+        tx.execute("drop table temp_order", ())?;
+
+        tx.commit()?;
 
-        tx.commit().unwrap();
         Ok(())
     }
 
@@ -190,8 +210,9 @@ impl DBConn {
     pub fn get_selected_column(&self) -> Result<usize> {
         let mut stmt = self.prepare("select value from app_state where key = ?1")?;
         stmt.query_row(["selected_column"], |row| {
-            let value: String = row.get::<usize, String>(0).unwrap();
+            let value: String = row.get::<usize, String>(0)?;
-            Ok(value.parse::<usize>().unwrap())
+            value.parse::<usize>()
+                .map_err(|_| rusqlite::Error::InvalidQuery)
         })
     }
 

tests/app_tests.rs

@@ -1,12 +1,6 @@
-// - [X] Adding
-// - [X] Selecting general
-// - [X] Selecting top/bottom
-// - [X] Moving
-// - [X] Editing
-// - [X] Deleting
 #[cfg(test)]
 mod app_tests {
-    use anyhow::Error;
+    use anyhow::{Error};
     use kanban_tui::State;
     use rusqlite::Connection;
 
@@ -165,10 +159,12 @@ mod app_tests {
 
         state.add_new_task(String::from("T1"), String::from("D1"))?;
         state.add_new_task(String::from("T2"), String::from("D2"))?;
+
         state.select_previous_task()?;
         state.move_task_up()?;
         state.move_task_down()?;
         state.move_task_down()?;
+
         assert_eq!(&state.columns[0].tasks[1].title, "T1");
         assert_eq!(&state.columns[0].tasks[0].title, "T2");
         state.select_next_column()?;
@@ -176,12 +172,14 @@ mod app_tests {
         state.move_task_next_column()?;
         assert_eq!(state.columns[1].tasks.len(), 0);
         assert_eq!(state.columns[2].tasks.len(), 1);
+
         for _ in 0..5 {
             state.move_task_next_column()?;
         }
         for _ in 0..4 {
             state.move_task_previous_column()?;
         }
+
         assert_eq!(state.columns[0].tasks.len(), 3);
         assert_eq!(state.columns[1].tasks.len(), 0);
         assert_eq!(state.columns[2].tasks.len(), 0);
@@ -202,11 +200,11 @@ mod app_tests {
         // make sure everything was saved correctly
         let mut state = State::new(state.db_conn.0)?;
 
-        assert_eq!(state.get_selected_task().unwrap().title, "T2");
-        state.select_next_task()?;
         assert_eq!(state.get_selected_task().unwrap().title, "T1");
         state.select_next_task()?;
         assert_eq!(state.get_selected_task().unwrap().title, "T3");
+        state.select_next_task()?;
+        assert_eq!(state.get_selected_task().unwrap().title, "T3");
         state.select_first_task()?;
         assert_eq!(state.get_selected_task().unwrap().title, "T2");
         state.select_last_task()?;
@@ -256,8 +254,8 @@ mod app_tests {
         // make sure everything was saved correctly
         let state = State::new(state.db_conn.0)?;
 
-        assert_eq!(state.get_selected_task().unwrap().title, "T2");
+        assert_eq!(state.get_selected_task().unwrap().title, "T3");
-        assert_eq!(state.get_selected_task().unwrap().description, "D1");
+        assert_eq!(state.get_selected_task().unwrap().description, "D3");
 
         Ok(())
     }