Let fetch_tree also returns position and parent_guid #1289
Conversation
There was a problem hiding this comment.
This isn't exactly what I had expected, but it's probably less churn than adding fields to BookmarkTreeNode.
Can you make sure that we have test coverage here? E.g. if this code had done the wrong thing, do any tests fail? If not, can you add one that makes sure the positions are right?
|
Sure, I will add tests for |
No, for now this is fine (this function isn't used elsewhere), and is a lot simpler and easy for me to review than had you modified the big fetch_tree query :p. |
| @@ -1124,7 +1124,10 @@ fn inflate( | |||
|
|
|||
| /// Fetch the tree starting at the specified folder guid. | |||
| /// Returns a BookmarkTreeNode::Folder(_) | |||
There was a problem hiding this comment.
Ah, one more thing, this should be updated.
| /// Returns a BookmarkTreeNode::Folder(_) | |
| /// Returns a `BookmarkTreeNode::Folder(_)`, its parent's guid (if any), and | |
| /// position inside its parent. |
There was a problem hiding this comment.
Please re-review whenever you get a chance, thanks!
Originally, we used multiple SELECTs to populate the tree, and needed the read-only transaction to avoid interleaved writes. The multiple SELECTs were removed in #1289, so the transaction (1) isn't needed anymore, and (2) causes issues when calling `fetch_tree_with_depth` concurrently. Closes #5773.
Originally, we used multiple SELECTs to populate the tree, and needed the read-only transaction to avoid interleaved writes. The multiple SELECTs were removed in #1289, so the transaction (1) isn't needed anymore, and (2) causes issues when calling `fetch_tree_with_depth` concurrently. Closes #5773.
Originally, we used multiple SELECTs to populate the tree, and needed the read-only transaction to avoid interleaved writes. The multiple SELECTs were removed in #1289, so the transaction (1) isn't needed anymore, and (2) causes issues when calling `fetch_tree_with_depth` concurrently. Closes #5773.
fixes #808