move the updating of "last fetch timestamp" logic

Removing the updating of project from the virtual branch controller - this is the only dependency between the two controllers and likely indicates that the "last fetched at" should live somewhere else.
It will serve us well to not mutate the project state from the virtual branches domain

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,
-    VirtualBranchesHandle,
+    target, BaseBranch, NameConflitResolution, RemoteBranchFile, 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> {
-        let mut project_repository = project_repository::Repository::open(project)?;
+    ) -> Result<FetchResult> {
+        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");
             }
         }
-
-        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)
+        Ok(project_data_last_fetched)
     }
 
     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 base_branch = handle
+        let projects = handle.state::<projects::Controller>();
+        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)
     }