Merge pull request #3730 from gitbutlerapp/rename-snapshots-module-names

move snapshot creation on branch update to trait

crates/gitbutler-core/src/ops/snapshot.rs

@@ -1,12 +1,16 @@
+use std::vec;
+
 use crate::{
     ops::entry::{OperationType, SnapshotDetails},
-    virtual_branches::Branch,
+    virtual_branches::{branch::BranchUpdateRequest, Branch},
 };
 
 use super::{entry::Trailer, oplog::Oplog};
 
 pub trait Snapshoter {
     fn snapshot_deletion(&self, oplog: &dyn Oplog) -> anyhow::Result<()>;
+    fn snapshot_update(&self, oplog: &dyn Oplog, update: BranchUpdateRequest)
+        -> anyhow::Result<()>;
 }
 
 impl Snapshoter for Branch {
@@ -20,4 +24,67 @@ impl Snapshoter for Branch {
         oplog.create_snapshot(details)?;
         Ok(())
     }
+    fn snapshot_update(
+        &self,
+        oplog: &dyn Oplog,
+        update: BranchUpdateRequest,
+    ) -> anyhow::Result<()> {
+        let details = if update.ownership.is_some() {
+            SnapshotDetails::new(OperationType::MoveHunk)
+        } else if let Some(name) = update.name {
+            SnapshotDetails::new(OperationType::UpdateBranchName).with_trailers(vec![
+                Trailer {
+                    key: "before".to_string(),
+                    value: self.name.to_string(),
+                },
+                Trailer {
+                    key: "after".to_string(),
+                    value: name,
+                },
+            ])
+        } else if update.notes.is_some() {
+            SnapshotDetails::new(OperationType::UpdateBranchNotes)
+        } else if let Some(order) = update.order {
+            SnapshotDetails::new(OperationType::ReorderBranches).with_trailers(vec![
+                Trailer {
+                    key: "before".to_string(),
+                    value: self.order.to_string(),
+                },
+                Trailer {
+                    key: "after".to_string(),
+                    value: order.to_string(),
+                },
+            ])
+        } else if let Some(selected_for_changes) = update.selected_for_changes {
+            SnapshotDetails::new(OperationType::SelectDefaultVirtualBranch).with_trailers(vec![
+                Trailer {
+                    key: "before".to_string(),
+                    value: self.selected_for_changes.unwrap_or_default().to_string(),
+                },
+                Trailer {
+                    key: "after".to_string(),
+                    value: selected_for_changes.to_string(),
+                },
+            ])
+        } else if let Some(upstream) = update.upstream {
+            SnapshotDetails::new(OperationType::UpdateBranchRemoteName).with_trailers(vec![
+                Trailer {
+                    key: "before".to_string(),
+                    value: self
+                        .upstream
+                        .clone()
+                        .map(|r| r.to_string())
+                        .unwrap_or("".to_string()),
+                },
+                Trailer {
+                    key: "after".to_string(),
+                    value: upstream,
+                },
+            ])
+        } else {
+            SnapshotDetails::new(OperationType::GenericBranchUpdate)
+        };
+        oplog.create_snapshot(details)?;
+        Ok(())
+    }
 }

crates/gitbutler-core/src/virtual_branches/branch/mod.rs

@@ -69,7 +69,7 @@ impl Branch {
     }
 }
 
-#[derive(Debug, Serialize, Deserialize, Default)]
+#[derive(Debug, Serialize, Deserialize, Default, Clone)]
 pub struct BranchUpdateRequest {
     pub id: BranchId,
     pub name: Option<String>,

crates/gitbutler-core/src/virtual_branches/controller.rs

@@ -680,23 +680,7 @@ impl ControllerInner {
         let _permit = self.semaphore.acquire().await;
 
         self.with_verify_branch(project_id, |project_repository, _| {
-            let details = if branch_update.ownership.is_some() {
-                SnapshotDetails::new(OperationType::MoveHunk)
-            } else if branch_update.name.is_some() {
-                SnapshotDetails::new(OperationType::UpdateBranchName)
-            } else if branch_update.notes.is_some() {
-                SnapshotDetails::new(OperationType::UpdateBranchNotes)
-            } else if branch_update.order.is_some() {
-                SnapshotDetails::new(OperationType::ReorderBranches)
-            } else if branch_update.selected_for_changes.is_some() {
-                SnapshotDetails::new(OperationType::SelectDefaultVirtualBranch)
-            } else if branch_update.upstream.is_some() {
-                SnapshotDetails::new(OperationType::UpdateBranchRemoteName)
-            } else {
-                SnapshotDetails::new(OperationType::GenericBranchUpdate)
-            };
             super::update_branch(project_repository, branch_update)?;
-            let _ = project_repository.project().create_snapshot(details);
             Ok(())
         })
     }

crates/gitbutler-core/src/virtual_branches/virtual.rs

@@ -1389,11 +1389,11 @@ pub fn update_branch(
             _ => errors::UpdateBranchError::Other(error.into()),
         })?;
 
-    if let Some(ownership) = branch_update.ownership {
-        set_ownership(&vb_state, &mut branch, &ownership).context("failed to set ownership")?;
+    if let Some(ownership) = &branch_update.ownership {
+        set_ownership(&vb_state, &mut branch, ownership).context("failed to set ownership")?;
     }
 
-    if let Some(name) = branch_update.name {
+    if let Some(name) = &branch_update.name {
         let all_virtual_branches = vb_state
             .list_branches()
             .context("failed to read virtual branches")?;
@@ -1405,13 +1405,13 @@ pub fn update_branch(
                 .iter()
                 .map(|b| b.name.as_str())
                 .collect::<Vec<_>>(),
-            &name,
+            name,
         );
 
         project_repository.add_branch_reference(&branch)?;
     };
 
-    if let Some(updated_upstream) = branch_update.upstream {
+    if let Some(updated_upstream) = &branch_update.upstream {
         let Some(default_target) = vb_state
             .try_get_default_target()
             .context("failed to get default target")?
@@ -1431,14 +1431,14 @@ pub fn update_branch(
         let remote_branch = format!(
             "refs/remotes/{}/{}",
             upstream_remote,
-            normalize_branch_name(&updated_upstream)
+            normalize_branch_name(updated_upstream)
         )
         .parse::<git::RemoteRefname>()
         .unwrap();
         branch.upstream = Some(remote_branch);
     };
 
-    if let Some(notes) = branch_update.notes {
+    if let Some(notes) = branch_update.notes.clone() {
         branch.notes = notes;
     };
 
@@ -1467,6 +1467,7 @@ pub fn update_branch(
         .set_branch(branch.clone())
         .context("failed to write target branch")?;
 
+    _ = branch.snapshot_update(project_repository.project(), branch_update);
     Ok(branch)
 }