Merge pull request #4245 from gitbutlerapp/Virtual-branch

move the updating of "last fetch timestamp" logic

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

@@ -1,18 +1,17 @@
 use crate::{
     git::BranchExt,
     ops::entry::{OperationKind, SnapshotDetails},
+    projects::FetchResult,
     types::ReferenceName,
 };
 use anyhow::Result;
 use std::{path::Path, sync::Arc};
 
-use anyhow::Context;
 use tokio::{sync::Semaphore, task::JoinHandle};
 
 use super::{
     branch::{BranchId, BranchOwnershipClaims},
-    target, target_to_base_branch, BaseBranch, NameConflitResolution, RemoteBranchFile,
+    target, BaseBranch, NameConflitResolution, RemoteBranchFile, VirtualBranchesHandle,
-    VirtualBranchesHandle,
 };
 use crate::{
     git, project_repository,
@@ -21,18 +20,16 @@ use crate::{
 
 #[derive(Clone)]
 pub struct Controller {
-    projects: projects::Controller,
     helper: git::credentials::Helper,
 
     semaphore: Arc<Semaphore>,
 }
 
 impl Controller {
-    pub fn new(projects: projects::Controller, helper: git::credentials::Helper) -> Self {
+    pub fn new(helper: git::credentials::Helper) -> Self {
         Self {
             semaphore: Arc::new(Semaphore::new(1)),
 
-            projects,
             helper,
         }
     }
@@ -446,8 +443,8 @@ impl Controller {
         &self,
         project: &Project,
         askpass: Option<String>,
-    ) -> Result<BaseBranch> {
+    ) -> Result<FetchResult> {
-        let mut project_repository = project_repository::Repository::open(project)?;
+        let project_repository = project_repository::Repository::open(project)?;
 
         let remotes = project_repository.remotes()?;
         let fetch_results: Vec<Result<(), _>> = remotes
@@ -481,23 +478,7 @@ impl Controller {
                 tracing::warn!(?err, "fetch from push-remote failed");
             }
         }
-
+        Ok(project_data_last_fetched)
-        let updated_project = self
-            .projects
-            .update(&projects::UpdateRequest {
-                id: project.id,
-                project_data_last_fetched: Some(project_data_last_fetched),
-                ..Default::default()
-            })
-            .await
-            .context("failed to update project")?;
-
-        project_repository.set_project(&updated_project);
-
-        let base_branch = target_to_base_branch(&project_repository, &default_target)
-            .context("failed to convert target to base branch")?;
-
-        Ok(base_branch)
     }
 
     pub async fn move_commit(

crates/gitbutler-core/tests/suite/virtual_branches/fetch_from_remotes.rs

@@ -1,39 +0,0 @@
-use super::*;
-
-#[tokio::test]
-async fn should_update_last_fetched() {
-    let Test {
-        project,
-        projects,
-        controller,
-        ..
-    } = &Test::default();
-
-    controller
-        .set_base_branch(project, &"refs/remotes/origin/master".parse().unwrap())
-        .await
-        .unwrap();
-
-    let before_fetch = controller.get_base_branch_data(project).await.unwrap();
-    assert!(before_fetch.last_fetched_ms.is_none());
-
-    let fetch = controller.fetch_from_remotes(project, None).await.unwrap();
-    assert!(fetch.last_fetched_ms.is_some());
-
-    let project = &projects.get(project.id).unwrap();
-    let after_fetch = controller.get_base_branch_data(project).await.unwrap();
-    assert!(after_fetch.last_fetched_ms.is_some());
-    assert_eq!(fetch.last_fetched_ms, after_fetch.last_fetched_ms);
-
-    let second_fetch = controller.fetch_from_remotes(project, None).await.unwrap();
-    assert!(second_fetch.last_fetched_ms.is_some());
-    assert_ne!(fetch.last_fetched_ms, second_fetch.last_fetched_ms);
-
-    let project = &projects.get(project.id).unwrap();
-    let after_second_fetch = controller.get_base_branch_data(project).await.unwrap();
-    assert!(after_second_fetch.last_fetched_ms.is_some());
-    assert_eq!(
-        second_fetch.last_fetched_ms,
-        after_second_fetch.last_fetched_ms
-    );
-}

crates/gitbutler-core/tests/suite/virtual_branches/init.rs

@@ -8,7 +8,7 @@ async fn twice() {
 
     let test_project = TestProject::default();
 
-    let controller = Controller::new(projects.clone(), helper);
+    let controller = Controller::new(helper);
 
     {
         let project = projects

crates/gitbutler-core/tests/suite/virtual_branches/mod.rs

@@ -42,7 +42,7 @@ impl Default for Test {
         Self {
             repository: test_project,
             project_id: project.id,
-            controller: Controller::new(projects.clone(), helper),
+            controller: Controller::new(helper),
             projects,
             project,
             data_dir: Some(data_dir),
@@ -66,7 +66,6 @@ mod convert_to_real_branch;
 mod create_commit;
 mod create_virtual_branch_from_branch;
 mod delete_virtual_branch;
-mod fetch_from_remotes;
 mod init;
 mod insert_blank_commit;
 mod move_commit_file;

crates/gitbutler-tauri/src/main.rs

@@ -138,7 +138,6 @@ fn main() {
                     app_handle.manage(git_credentials_controller.clone());
 
                     app_handle.manage(gitbutler_core::virtual_branches::controller::Controller::new(
-                        projects_controller.clone(),
                         git_credentials_controller.clone(),
                     ));
 

crates/gitbutler-tauri/src/virtual_branches.rs

@@ -485,15 +485,33 @@ pub mod commands {
         project_id: ProjectId,
         action: Option<String>,
     ) -> Result<BaseBranch, Error> {
-        let project = handle.state::<projects::Controller>().get(project_id)?;
+        let projects = handle.state::<projects::Controller>();
-        let base_branch = handle
+        let project = projects.get(project_id)?;
+
+        let project_data_last_fetched = handle
             .state::<Controller>()
             .fetch_from_remotes(
                 &project,
                 Some(action.unwrap_or_else(|| "unknown".to_string())),
             )
             .await?;
-        emit_vbranches(&handle, project_id).await;
+
+        // Updates the project controller with the last fetched timestamp
+        //
+        // TODO: This cross dependency likely indicates that last_fetched is stored in the wrong place - value is coupled with virtual branches state
+        projects
+            .update(&projects::UpdateRequest {
+                id: project.id,
+                project_data_last_fetched: Some(project_data_last_fetched),
+                ..Default::default()
+            })
+            .await
+            .context("failed to update project with last fetched timestamp")?;
+
+        let base_branch = handle
+            .state::<Controller>()
+            .get_base_branch_data(&project)
+            .await?;
         Ok(base_branch)
     }