Reset command #56
Reset command #56
Conversation
| @@ -34,68 +34,72 @@ pub fn import_fasta( | |||
| change.id, | |||
| ); | |||
|
|
|||
| if !Collection::exists(conn, name) { | |||
There was a problem hiding this comment.
This was not intentional behavior. Where only a single fasta could ever be added to a collection.
| @@ -117,7 +117,7 @@ impl BlockGroup { | |||
| None => { | |||
| conn | |||
| .query_row( | |||
| "select id from block_group where collection_name = ?1 and sample_name is null and name = ?3", | |||
| "select id from block_group where collection_name = ?1 and sample_name is null and name = ?2", | |||
There was a problem hiding this comment.
bug fix, no ?3 in the params
There was a problem hiding this comment.
👍 I missed it in review, too
| .unwrap(); | ||
| rows.next().unwrap().unwrap() | ||
|
|
||
| match stmt.query_row((name,), |row| { |
There was a problem hiding this comment.
makes it get-or-create
| @@ -38,7 +39,7 @@ impl Operation { | |||
| if let Some(op_id) = current_op { | |||
| let count: i32 = conn | |||
| .query_row( | |||
| "select count(*) from operation where branch_id = ?1 AND parent_id = ?2", | |||
| "select count(*) from operation where branch_id = ?1 AND parent_id = ?2 AND id not in (select operation_id from branch_masked_operations where branch_id = ?1);", | |||
There was a problem hiding this comment.
This is an update to the bifurcation projection to not consider operations in the masked operation list
| @@ -98,7 +99,7 @@ impl Operation { | |||
| graph.add_node(op.id); | |||
| if let Some(v) = op.parent_id { | |||
| graph.add_node(v); | |||
| graph.add_edge(op.id, v, ()); | |||
| graph.add_edge(v, op.id, ()); | |||
There was a problem hiding this comment.
this was a bug, the branch edges were reversed :(
| @@ -135,7 +136,7 @@ impl Operation { | |||
| directed_graph.edges_directed(last_node, Direction::Incoming) | |||
| { | |||
| if edge_src == node { | |||
| patch_path.push((node, Direction::Incoming, last_node)); | |||
| patch_path.push((last_node, Direction::Incoming, node)); | |||
There was a problem hiding this comment.
goes w/ the above bug fix
| let creation_id = branch.start_operation_id.unwrap_or(1); | ||
|
|
||
| let mut dfs = DfsPostOrder::new(&graph, creation_id); |
There was a problem hiding this comment.
this actually was just working because i had the graph in reverse orientation. Fixed it by using the Reversed graph method to actually look at ancestors.
| @@ -673,6 +724,209 @@ mod tests { | |||
| assert_eq!(ops, vec![1, 6, 7, 8, 9, 10, 11]); | |||
| } | |||
|
|
|||
| #[test] | |||
| fn test_graph_representation() { | |||
There was a problem hiding this comment.
This test was added to make sure the branch graph was what i expected it to be
| } | ||
|
|
||
| #[test] | ||
| fn test_path_between() { |
There was a problem hiding this comment.
this ensures the paths between operations is what we expect
| @@ -791,4 +1045,67 @@ mod tests { | |||
| change.id, | |||
| ); | |||
| } | |||
|
|
|||
| #[test] | |||
| fn test_bifurcation_allowed_on_reset() { | |||
There was a problem hiding this comment.
this ensures we do not enforce the bifurcation protection if someone has reset a branch to a point
| let new_seq = NewSequence::from(sequence).save(conn); | ||
| assert_eq!(new_seq.hash, sequence.hash); | ||
| println!("dep seq is {sequence:?}"); | ||
| if !Sequence::is_delimiter_hash(&sequence.hash) { |
There was a problem hiding this comment.
because the sequence field isn't set, the start/end node hashes weren't matching
There was a problem hiding this comment.
Be aware that the is_delimiter_hash method is going away, but it should be possible to determine start/end nodes in the new model
| @@ -470,15 +518,15 @@ pub fn move_to(conn: &Connection, operation_conn: &Connection, operation: &Opera | |||
| } | |||
| for (operation_id, direction, next_op) in path.iter() { | |||
| match direction { | |||
| Direction::Outgoing => { | |||
| Direction::Incoming => { | |||
There was a problem hiding this comment.
fix from the inverted graph
| ); | ||
|
|
||
| assert_eq!( | ||
| Operation::get_path_between(op_conn, 3, 7), |
This allows users to reset their branch to a previous state, detaching the commits from the head of the branch. I used a mask approach where the commits are hidden and not deleted. It could make it simpler to restore this way.