Add support for pushing changes to a translation review

Finally got around to fixing the pre-receive hook to include quarantined
objects so the hook actually can run git commands on the not-yet-accepted
commits. As part of that, had to make sure git hook and eyeballs server
had the same path to the repo or confusion will appear.

11 files changed, 380 insertions, 100 deletions

diff --git a/docker/integration_test/docker-compose.yaml b/docker/integration_test/docker-compose.yaml
index cd832be..4883761 100644
--- a/docker/integration_test/docker-compose.yaml
+++ b/docker/integration_test/docker-compose.yaml
@@ -65,7 +65,7 @@ services:
       - '18000:8000'
     volumes:
       - 'it_git_auth:/git/auth'
-      - 'it_git_repos:/git/repos'
+      - 'it_git_repos:/srv/git'
 
 volumes:
   it_openldap_data:
diff --git a/docker/integration_test/web/Dockerfile b/docker/integration_test/web/Dockerfile
index b9b2ad7..b102d4d 100644
--- a/docker/integration_test/web/Dockerfile
+++ b/docker/integration_test/web/Dockerfile
@@ -8,19 +8,19 @@ COPY server/target/x86_64-unknown-linux-musl/debug/eyeballs-githook /app/eyeball
 COPY server/target/debug/eyeballs /app/eyeballs
 COPY docker/integration_test/web/setup.sh /app/setup.sh
 
-RUN mkdir -p /git/auth /git/repos
+RUN mkdir -p /git/auth /srv/git
 
 # git image runs as default git user, with uid 1000 gid 1000.
 # we need the same, but it can't be named git (as package git installs a git user)
 # so add another user with 1000 gid 1000 and make sure that shared files
-# (/git/auth and /git/repos) are owned by that user and not root.
+# (/git/auth and /srv/git) are owned by that user and not root.
 RUN useradd --no-create-home --uid 1000 --user-group -s /usr/bin/nologin alf
 
 RUN chown alf:alf /app
 RUN chown alf:alf /git/auth
-RUN chown alf:alf /git/repos
+RUN chown alf:alf /srv/git
 VOLUME /git/auth
-VOLUME /git/repos
+VOLUME /srv/git
 
 USER alf:alf
 
diff --git a/docker/integration_test/web/setup.sh b/docker/integration_test/web/setup.sh
index bd6c805..d8093b1 100755
--- a/docker/integration_test/web/setup.sh
+++ b/docker/integration_test/web/setup.sh
@@ -7,9 +7,9 @@ echo "session_max_age_days = 7" >> Rocket.toml
 echo "ldap_url = \"$LDAP_URL\"" >> Rocket.toml
 echo "ldap_users = \"ou=users,dc=example,dc=org\"" >> Rocket.toml
 echo "ldap_filter = \"(objectClass=posixAccount)\"" >> Rocket.toml
-echo "git_server_root = \"/git/repos\"" >> Rocket.toml
+echo "git_server_root = \"/srv/git\"" >> Rocket.toml
 echo "authorized_keys = \"/git/auth/authorized_keys\"" >> Rocket.toml
-echo "git_hook = \"/git/repos/eyeballs-githook\"" >> Rocket.toml
+echo "git_hook = \"/srv/git/eyeballs-githook\"" >> Rocket.toml
 echo "[default.databases.eyeballs]" >> Rocket.toml
 echo "url = \"$DB_URL\"" >> Rocket.toml
 
@@ -20,6 +20,6 @@ echo "Host remote_git" > /app/.ssh/config
 echo "  StrictHostKeyChecking no" >> /app/.ssh/config
 
 # Hardlinks cannot cross devices, so copy to the /git/repos mount.
-cp /app/eyeballs-githook /git/repos/eyeballs-githook
+cp /app/eyeballs-githook /srv/git/eyeballs-githook
 
 exec ./eyeballs
diff --git a/server/common/src/git.rs b/server/common/src/git.rs
index e396d8a..37995b3 100644
--- a/server/common/src/git.rs
+++ b/server/common/src/git.rs
@@ -83,6 +83,22 @@ pub struct GitFile {
     cursor: Cursor<Vec<u8>>,
 }
 
+pub struct Env {
+    pub object_dir: Option<String>,
+    pub alt_object_dirs: Option<String>,
+}
+
+impl Env {
+    fn apply(&self, cmd: &mut Command) {
+        if let Some(value) = &self.object_dir {
+            cmd.env("GIT_OBJECT_DIRECTORY", value);
+        }
+        if let Some(value) = &self.alt_object_dirs {
+            cmd.env("GIT_ALTERNATE_OBJECT_DIRECTORIES", value);
+        }
+    }
+}
+
 impl GitFile {
     pub fn new(data: Vec<u8>) -> Self {
         GitFile {
@@ -403,6 +419,22 @@ impl RepoData {
         self.check(&mut cmd).await
     }
 
+    async fn is_ancestor_with_env(
+        &self,
+        repo: &Repository,
+        ancestor: &str,
+        commit: &str,
+        env: &Env,
+    ) -> Result<bool, Error> {
+        let mut cmd = self.git_cmd(repo);
+        cmd.arg("merge-base")
+            .arg("--is-ancestor")
+            .arg(ancestor)
+            .arg(commit);
+        env.apply(&mut cmd);
+        self.check(&mut cmd).await
+    }
+
     async fn is_equal_content(
         &self,
         repo: &Repository,
@@ -419,13 +451,24 @@ impl RepoData {
     }
 
     async fn get_author(&self, repo: &Repository, commit: &str) -> Result<User, Error> {
-        self.get_log_format(repo, commit, "%an%x00%al%x00%ae")
+        self.get_log_format(repo, commit, "%an%x00%al%x00%ae", None)
             .map_ok(parse_user)
             .await
     }
 
     async fn get_commiter(&self, repo: &Repository, commit: &str) -> Result<User, Error> {
-        self.get_log_format(repo, commit, "%cn%x00%cl%x00%ce")
+        self.get_log_format(repo, commit, "%cn%x00%cl%x00%ce", None)
+            .map_ok(parse_user)
+            .await
+    }
+
+    async fn get_commiter_with_env(
+        &self,
+        repo: &Repository,
+        commit: &str,
+        env: &Env,
+    ) -> Result<User, Error> {
+        self.get_log_format(repo, commit, "%cn%x00%cl%x00%ce", Some(env))
             .map_ok(parse_user)
             .await
     }
@@ -473,6 +516,7 @@ impl RepoData {
         repo: &Repository,
         commit: &str,
         format: &str,
+        maybe_env: Option<&Env>,
     ) -> Result<String, Error> {
         let mut cmd = self.git_cmd(repo);
         cmd.arg("log")
@@ -481,6 +525,9 @@ impl RepoData {
             .arg("--no-mailmap")
             .arg(format!("--pretty=format:{format}"))
             .arg(commit);
+        if let Some(env) = maybe_env {
+            env.apply(&mut cmd);
+        }
         self.output(&mut cmd).await
     }
 
@@ -693,6 +740,21 @@ impl Repository {
             .await
     }
 
+    pub async fn is_ancestor_with_env(
+        &self,
+        ancestor: impl Into<String>,
+        commit: impl Into<String>,
+        env: &Env,
+    ) -> Result<bool, Error> {
+        let ancestor = ancestor.into();
+        let commit = commit.into();
+
+        let data = self.lock.read().await;
+
+        data.is_ancestor_with_env(self, ancestor.as_str(), commit.as_str(), env)
+            .await
+    }
+
     pub async fn is_equal_content(
         &self,
         commit1: impl Into<String>,
@@ -720,6 +782,17 @@ impl Repository {
         data.get_commiter(self, commit.as_str()).await
     }
 
+    pub async fn get_commiter_with_env(
+        &self,
+        commit: impl Into<String>,
+        env: &Env,
+    ) -> Result<User, Error> {
+        let commit = commit.into();
+        let data = self.lock.read().await;
+
+        data.get_commiter_with_env(self, commit.as_str(), env).await
+    }
+
     pub async fn delete_branch(&self, branch: impl Into<String>) -> Result<(), Error> {
         let branch = branch.into();
         let data = self.lock.read().await;
diff --git a/server/common/src/git_socket.rs b/server/common/src/git_socket.rs
index a4805be..6e1c7d4 100644
--- a/server/common/src/git_socket.rs
+++ b/server/common/src/git_socket.rs
@@ -5,16 +5,14 @@ pub struct GitReceive {
     pub old_value: String,
     pub new_value: String,
     pub reference: String,
-    // Only set for pre hooks, because server can't read the objects the pre-hook has not yet
-    // accepted, so to be able to validate the commiter, send them. Also only set if new_value
-    // is not empty.
-    pub commiter: Option<String>,
 }
 
 #[derive(Deserialize, Serialize)]
 pub struct GitHookRequest {
     pub pre: bool,
     pub receive: Vec<GitReceive>,
+    pub object_dir: Option<String>,
+    pub alt_object_dirs: Option<String>,
 }
 
 #[derive(Deserialize, Serialize)]
diff --git a/server/hook/src/githook.rs b/server/hook/src/githook.rs
index 3a27e2c..0897dfc 100644
--- a/server/hook/src/githook.rs
+++ b/server/hook/src/githook.rs
@@ -1,5 +1,6 @@
 use rmp_serde::{decode, Serializer};
 use serde::ser::Serialize;
+use std::env;
 use std::error::Error;
 use std::fmt;
 use std::os::unix::net::UnixStream;
@@ -49,6 +50,8 @@ async fn main() -> Result<(), Box<dyn Error>> {
     let mut request = git_socket::GitHookRequest {
         pre,
         receive: Vec::new(),
+        object_dir: env::var("GIT_OBJECT_DIRECTORY").ok(),
+        alt_object_dirs: env::var("GIT_ALTERNATE_OBJECT_DIRECTORIES").ok(),
     };
 
     let repo = git::Repository::new(
@@ -64,18 +67,10 @@ async fn main() -> Result<(), Box<dyn Error>> {
         let data: Vec<&str> = line.split(' ').collect();
 
         if data.len() == 3 {
-            let mut commiter: Option<String> = None;
-            if pre && data[1] != git::EMPTY {
-                if let Ok(user) = repo.get_commiter(data[1]).await {
-                    commiter = Some(user.username);
-                }
-            }
-
             request.receive.push(git_socket::GitReceive {
                 old_value: data[0].to_string(),
                 new_value: data[1].to_string(),
                 reference: data[2].to_string(),
-                commiter,
             })
         }
     }
diff --git a/server/src/git_root.rs b/server/src/git_root.rs
index f68ed92..ec741c4 100644
--- a/server/src/git_root.rs
+++ b/server/src/git_root.rs
@@ -74,7 +74,7 @@ impl Roots {
 
     pub async fn new_translation_review(
         &self,
-        db: &Db,
+        db: &mut DbConnection,
         project_id: &str,
         translation_reviewid: u64,
         base: &str,
@@ -89,38 +89,7 @@ impl Roots {
             }
         }
 
-        let mut entries = repo
-            .ls_tree(base, true)
-            .await
-            .map_err(|e| anyhow::Error::new(e))?;
-        entries.retain(|e| e.object_type == git::ObjectType::BLOB);
-        let grits = entries
-            .iter()
-            .filter(|x| x.path.ends_with(".grd"))
-            .map(|x| x.path.to_string())
-            .collect::<Vec<String>>();
-        let entries = Arc::new(entries);
-        let strings = trans::collect_strings_with_opener(grits, move |path| {
-            for entry in &*entries {
-                if entry.path == path {
-                    let rt = tokio::runtime::Handle::current();
-                    let object_name = entry.object_name.clone();
-                    let repo = repo.clone();
-                    return rt.block_on(async move {
-                        repo.cat_file(git::ObjectType::BLOB, object_name)
-                            .await
-                            .map(|x| BufReader::new(x))
-                            .map_err(|e| anyhow::Error::new(e))
-                    });
-                }
-            }
-            Err(anyhow::Error::msg(format!("No such file: {path}")))
-        })
-        .await?;
-
-        trans::review_add_strings(db, translation_reviewid, strings, true).await?;
-
-        Ok(())
+        trans_review_add_strings(db, repo, translation_reviewid, base, true).await
     }
 
     pub async fn fetch_branch(&self, project_id: &str, branch: &str) -> anyhow::Result<String> {
@@ -181,10 +150,52 @@ fn valid_branch_name(name: &str) -> bool {
     name.starts_with("refs/heads/") && is_printable(name)
 }
 
+async fn trans_review_add_strings(
+    db: &mut DbConnection,
+    repo: Arc<git::Repository>,
+    translation_reviewid: u64,
+    commit: &str,
+    base: bool,
+) -> Result<(), anyhow::Error> {
+    let mut entries = repo
+        .ls_tree(commit, true)
+        .await
+        .map_err(|e| anyhow::Error::new(e))?;
+    entries.retain(|e| e.object_type == git::ObjectType::BLOB);
+    let grits = entries
+        .iter()
+        .filter(|x| x.path.ends_with(".grd"))
+        .map(|x| x.path.to_string())
+        .collect::<Vec<String>>();
+    let entries = Arc::new(entries);
+    let strings = trans::collect_strings_with_opener(grits, move |path| {
+        for entry in &*entries {
+            if entry.path == path {
+                let rt = tokio::runtime::Handle::current();
+                let object_name = entry.object_name.clone();
+                let repo = repo.clone();
+                return rt.block_on(async move {
+                    repo.cat_file(git::ObjectType::BLOB, object_name)
+                        .await
+                        .map(|x| BufReader::new(x))
+                        .map_err(|e| anyhow::Error::new(e))
+                });
+            }
+        }
+        Err(anyhow::Error::msg(format!("No such file: {path}")))
+    })
+    .await?;
+
+    trans::review_add_strings(db, translation_reviewid, strings, base).await?;
+
+    Ok(())
+}
+
 async fn git_process_prehook(
-    repo: &git::Repository,
+    repo: Arc<git::Repository>,
     mut db: DbConnection,
     receive: &Vec<git_socket::GitReceive>,
+    env: &git::Env,
 ) -> Result<git_socket::GitHookResponse, IoError> {
     let mut errors: Vec<String> = Vec::new();
 
@@ -213,13 +224,15 @@ async fn git_process_prehook(
         let branch = row.reference.strip_prefix("refs/heads/").unwrap();
 
         if row.new_value != git::EMPTY {
-            match row.commiter {
-                Some(ref commiter) => match sqlx::query!(
+            match repo.get_commiter_with_env(&row.new_value, env).await {
+                Ok(commiter) => match sqlx::query!(
                     "SELECT id FROM users WHERE id=? AND dn IS NOT NULL",
-                    commiter,
+                    commiter.username,
                 )
                 .fetch_one(&mut *db)
-                .map_err(|_| IoError::new(format!("{branch}: Unknown commiter {}", commiter)))
+                .map_err(|_| {
+                    IoError::new(format!("{branch}: Unknown commiter {}", commiter.username))
+                })
                 .await
                 {
                     Ok(_) => {}
@@ -229,7 +242,7 @@ async fn git_process_prehook(
                         continue;
                     }
                 },
-                None => {
+                Err(_) => {
                     error!("{branch}: Missing commiter");
                     errors.push(format!("{branch}: Missing commiter"));
                     continue;
@@ -237,31 +250,67 @@ async fn git_process_prehook(
             }
         }
 
-        if row.old_value == git::EMPTY {
+        let translation_review_id;
+
+        if branch.starts_with("t/") {
+            // Translation review
+
+            translation_review_id = branch[2..].parse::<u64>().ok();
+            if translation_review_id.is_none() {
+                error!("{branch}: Invalid translation review branch");
+                errors.push(format!("{branch}: Invalid translation review branch"));
+            }
+        } else {
+            translation_review_id = None;
+        }
+
+        if row.old_value == git::EMPTY && translation_review_id.is_none() {
             // Creating new review, nothing to check (yet).
             continue;
         }
 
-        let result = sqlx::query!(
-            "SELECT state, rewrite FROM reviews WHERE project=? AND branch=?",
-            repo.project_id().unwrap_or(""),
-            branch
-        )
-        .fetch_one(&mut *db)
-        .map_ok(|r| {
-            (
-                api_model::ReviewState::try_from(r.state).unwrap(),
-                api_model::Rewrite::try_from(r.rewrite).unwrap(),
-            )
-        })
-        .map_err(|_| IoError::new(format!("{branch}: Unknown branch")))
-        .await;
+        let result = match translation_review_id {
+            None => {
+                sqlx::query!(
+                    "SELECT state, rewrite FROM reviews WHERE project=? AND branch=?",
+                    repo.project_id().unwrap_or(""),
+                    branch
+                )
+                .fetch_one(&mut *db)
+                .map_ok(|r| {
+                    (
+                        api_model::ReviewState::try_from(r.state).unwrap(),
+                        api_model::Rewrite::try_from(r.rewrite).unwrap(),
+                        "".to_string(),
+                    )
+                })
+                .map_err(|_| IoError::new(format!("{branch}: Unknown branch")))
+                .await
+            }
+            Some(id) => {
+                sqlx::query!(
+                    "SELECT state, base FROM translation_reviews WHERE project=? AND id=?",
+                    repo.project_id().unwrap_or(""),
+                    id,
+                )
+                .fetch_one(&mut *db)
+                .map_ok(|r| {
+                    (
+                        api_model::ReviewState::try_from(r.state).unwrap(),
+                        api_model::Rewrite::Disabled,
+                        r.base,
+                    )
+                })
+                .map_err(|_| IoError::new(format!("{branch}: No such translation review")))
+                .await
+            }
+        };
 
         if row.new_value == git::EMPTY {
             // Do not allow to delete branch if there is a review connected to the branch.
             // All branches should be connected to a branch, but in case of errors this might
             // be relevant.
-            if result.is_ok() {
+            if result.is_ok() || translation_review_id.is_some() {
                 error!("{branch}: Not allowed to delete branch, delete review instead.");
                 errors.push(format!(
                     "{branch}: Not allowed to delete branch, delete review instead."
@@ -270,7 +319,7 @@ async fn git_process_prehook(
             continue;
         }
 
-        let (state, rewrite) = match result {
+        let (state, rewrite, base) = match result {
             Ok(data) => data,
             Err(e) => {
                 error!("{e:?}");
@@ -294,11 +343,25 @@ async fn git_process_prehook(
             api_model::ReviewState::Open => {}
         }
 
+        let history_origin = if translation_review_id.is_some() {
+            &base
+        } else {
+            &row.old_value
+        };
+
         // Check for fast forward, if so we can skip the rest of the checks.
-        let is_fast_forward = repo
-            .is_ancestor(&row.old_value, &row.new_value)
+        let is_fast_forward = match repo
+            .is_ancestor_with_env(history_origin, &row.new_value, env)
             .map_err(|e| IoError::new(format!("{branch}: {}", e)))
-            .await?;
+            .await
+        {
+            Ok(ret) => ret,
+            Err(e) => {
+                error!("{e:?}");
+                errors.push(e.message);
+                continue;
+            }
+        };
         if is_fast_forward {
             continue;
         }
@@ -317,11 +380,18 @@ async fn git_process_prehook(
             }
             api_model::Rewrite::Rebase => {}
             api_model::Rewrite::Disabled => {
-                error!("{}: Non fast-forward not allowed", row.reference);
-                errors.push(format!(
-                    "Non fast-forward not allowed for review: {}",
-                    row.reference
-                ));
+                if translation_review_id.is_none() {
+                    error!("{}: Non fast-forward not allowed", row.reference);
+                    errors.push(format!(
+                        "Non fast-forward not allowed for review: {}",
+                        row.reference
+                    ));
+                } else {
+                    error!("{branch}: Must be based on translation review base {base}");
+                    errors.push(format!(
+                        "{branch}: Must be based on translation review base {base}"
+                    ));
+                }
             }
         }
     }
@@ -340,12 +410,13 @@ async fn git_process_prehook(
 }
 
 async fn git_process_posthook(
-    repo: &git::Repository,
+    repo: Arc<git::Repository>,
     mut db: DbConnection,
     receive: &Vec<git_socket::GitReceive>,
 ) -> git_socket::GitHookResponse {
     let mut messages: Vec<String> = Vec::new();
-    let mut updated: Vec<u64> = Vec::new();
+    let mut updated_reviews: Vec<u64> = Vec::new();
+    let mut updated_translation_reviews: Vec<u64> = Vec::new();
 
     for row in receive {
         trace!(
@@ -357,6 +428,49 @@ async fn git_process_posthook(
 
         let branch = row.reference.strip_prefix("refs/heads/").unwrap();
 
+        if branch.starts_with("t/") {
+            // pre-hook already checked format
+            let id = branch[2..].parse::<u64>().unwrap();
+
+            match sqlx::query!(
+                "UPDATE translation_reviews SET head=? WHERE project=? AND id=?",
+                row.new_value,
+                repo.project_id(),
+                id,
+            )
+            .execute(&mut *db)
+            .map_err(|e| IoError::new(format!("Database error: {e:?}")))
+            .await
+            {
+                Ok(_) => {
+                    match trans_review_add_strings(
+                        &mut db,
+                        repo.clone(),
+                        id,
+                        row.new_value.as_str(),
+                        false,
+                    )
+                    .await
+                    {
+                        Ok(_) => {
+                            updated_translation_reviews.push(id);
+                            messages.push(format!("{branch}: Translation review updated"));
+                        }
+                        Err(e) => {
+                            error!("{e:?}");
+                            messages.push(format!("{branch}: Error {e}",));
+                        }
+                    }
+                }
+                Err(e) => {
+                    error!("{e:?}");
+                    messages.push(format!("{branch}: Error {e}",));
+                }
+            }
+
+            continue;
+        }
+
         if row.old_value == git::EMPTY {
             let commiter = match repo.get_commiter(row.reference.as_str()).await {
                 Ok(user) => user,
@@ -380,7 +494,7 @@ async fn git_process_posthook(
             .await
             {
                 Ok(result) => {
-                    updated.push(result.last_insert_id());
+                    updated_reviews.push(result.last_insert_id());
                     messages.push(format!(
                         "{branch}: Review draft created, finalize at {}",
                         "TODO"
@@ -416,7 +530,7 @@ async fn git_process_posthook(
                                     "{branch}: Review draft created, finalize at {}",
                                     "TODO"
                                 ));
-                                updated.push(id);
+                                updated_reviews.push(id);
                             }
                             Err(e) => {
                                 error!("{e:?}");
@@ -425,7 +539,7 @@ async fn git_process_posthook(
                         }
                     }
                     api_model::Rewrite::Disabled => {
-                        updated.push(id);
+                        updated_reviews.push(id);
                     }
                 },
                 Err(e) => {
@@ -443,7 +557,7 @@ async fn git_process_posthook(
 }
 
 async fn git_socket_process(
-    repo: &git::Repository,
+    repo: Arc<git::Repository>,
     db: DbConnection,
     stream: UnixStream,
 ) -> Result<(), IoError> {
@@ -460,8 +574,13 @@ async fn git_socket_process(
     .map_err(|e| IoError::new(e.to_string()))
     .await??;
 
+    let env = git::Env {
+        object_dir: request.object_dir,
+        alt_object_dirs: request.alt_object_dirs,
+    };
+
     let response = if request.pre {
-        git_process_prehook(repo, db, &request.receive).await?
+        git_process_prehook(repo, db, &request.receive, &env).await?
     } else {
         git_process_posthook(repo, db, &request.receive).await
     };
@@ -492,9 +611,7 @@ async fn git_socket_listen(
                 match db.get().await {
                     Ok(conn) => {
                         let repo2 = repo.clone();
-                        tokio::spawn(async move {
-                            git_socket_process(repo2.as_ref(), conn, stream).await
-                        });
+                        tokio::spawn(async move { git_socket_process(repo2, conn, stream).await });
                     }
                     Err(_) => { /* unable to access db */ }
                 }
diff --git a/server/src/main.rs b/server/src/main.rs
index 8c59b23..e4dff68 100644
--- a/server/src/main.rs
+++ b/server/src/main.rs
@@ -1208,7 +1208,6 @@ async fn get_translation_review_users(
 )]
 #[post("/translation/<projectid>/new", data = "<data>")]
 async fn translation_review_new(
-    db: &Db,
     mut conn: Connection<Db>,
     roots_state: &State<git_root::Roots>,
     session: auth::Session,
@@ -1259,7 +1258,7 @@ async fn translation_review_new(
     }
 
     roots_state
-        .new_translation_review(db, projectid, translation_reviewid, base.as_str())
+        .new_translation_review(&mut conn, projectid, translation_reviewid, base.as_str())
         .map_err(|e| Custom(Status::InternalServerError, format!("{e}")))
         .await?;
 
diff --git a/server/src/trans.rs b/server/src/trans.rs
index c4e3b45..3f47c8b 100644
--- a/server/src/trans.rs
+++ b/server/src/trans.rs
@@ -4,6 +4,7 @@ use anyhow;
 use futures::stream::TryStreamExt;
 use rocket_db_pools::{sqlx, Database, Pool};
 use sorted_insert::SortedInsertByKey;
+use sqlx::Acquire;
 use std::borrow::Cow;
 use std::collections::{HashMap, HashSet};
 use std::fs::File;
@@ -262,7 +263,7 @@ struct TranslationString {
 }
 
 pub async fn review_add_strings(
-    db: &Db,
+    db: &mut DbConnection,
     translation_reviewid: u64,
     strings: Vec<api_model::LocalizationString>,
     base: bool,
diff --git a/server/tests/common/mod.rs b/server/tests/common/mod.rs
index 0a9556a..62795de 100644
--- a/server/tests/common/mod.rs
+++ b/server/tests/common/mod.rs
@@ -138,7 +138,7 @@ const STRINGS_EN_GB_XLF: &str = r#"<?xml version="1.0" encoding="UTF-8"?>
 </body>
 </file>
 </xliff>"#;
-const STRINGS_SV_XLF: &str = r#"<?xml version="1.0" encoding="UTF-8"?>
+pub const STRINGS_SV_XLF: &str = r#"<?xml version="1.0" encoding="UTF-8"?>
 <xliff xmlns="urn:oasis:names:tc:xliff:document:1.2" version="1.2">
 <file datatype="xml" source-language="en-US" original="strings.grd" target-language="sv">
 <body>
diff --git a/server/tests/integration_test.rs b/server/tests/integration_test.rs
index 40804df..03180cd 100644
--- a/server/tests/integration_test.rs
+++ b/server/tests/integration_test.rs
@@ -160,6 +160,8 @@ async fn test_translation_review_create(ctx: &mut common::DockerComposeContext)
         .await
         .expect("create translation review");
 
+    // Check that all strings are unchanged, no change pushed yet
+
     for _ in 0..5 {
         let strings = common::list_translation_strings(ctx, &mut client1, review.id)
             .await
@@ -228,4 +230,99 @@ async fn test_translation_review_create(ctx: &mut common::DockerComposeContext)
         }
         sleep(Duration::from_millis(500));
     }
+
+    ctx.git_clone("client1").await.expect("git clone user01");
+    {
+        let dir = ctx.git_dir("client1");
+        ctx.git_cmd("client1", &["config", "set", "user.name", "John Smith"])
+            .await
+            .expect("config set");
+        ctx.git_cmd(
+            "client1",
+            &["config", "set", "user.email", "user01@example.org"],
+        )
+        .await
+        .expect("config set");
+        fs::write(
+            dir.join("translations/strings_sv.xlf"),
+            common::STRINGS_SV_XLF.replace("Primära teststrängen", "Primära test strängen"),
+        )
+        .await
+        .expect("update strings_sv.xlf");
+        ctx.git_cmd("client1", &["add", "translations/strings_sv.xlf"])
+            .await
+            .expect("git add");
+        ctx.git_cmd("client1", &["commit", "-m", "Update Swedish translation"])
+            .await
+            .expect("git commit");
+        ctx.git_cmd(
+            "client1",
+            &["push", "origin", format!("HEAD:t/{}", review.id).as_str()],
+        )
+        .await
+        .expect("git push");
+    }
+
+    let strings = common::list_translation_strings(ctx, &mut client1, review.id)
+        .await
+        .expect("list strings");
+    assert_eq!(
+        strings.strings,
+        vec![
+            api_model::LocalizationString {
+                id: "EXTRA_STRING".to_string(),
+                file: "extra.grdp".to_string(),
+                description: "Some description".to_string(),
+                meaning: "".to_string(),
+                source: "Extra string, gray".to_string(),
+                placeholders: vec![],
+                placeholder_offset: vec![],
+                translations: vec![
+                    api_model::TranslationString {
+                        language: "en-gb".to_string(),
+                        translation: "Extra string, grey".to_string(),
+                        placeholder_offset: vec![],
+                        state: api_model::TranslationState::Unchanged,
+                        comment: "".to_string(),
+                        reviewer: None,
+                    },
+                    api_model::TranslationString {
+                        language: "sv".to_string(),
+                        translation: "Extra sträng, grå".to_string(),
+                        placeholder_offset: vec![],
+                        state: api_model::TranslationState::Unchanged,
+                        comment: "".to_string(),
+                        reviewer: None,
+                    }
+                ],
+            },
+            api_model::LocalizationString {
+                id: "MAIN_STRING".to_string(),
+                file: "strings.grd".to_string(),
+                description: "Description".to_string(),
+                meaning: "".to_string(),
+                source: "Main test string".to_string(),
+                placeholders: vec![],
+                placeholder_offset: vec![],
+                translations: vec![
+                    api_model::TranslationString {
+                        language: "en-gb".to_string(),
+                        translation: "Main test string".to_string(),
+                        placeholder_offset: vec![],
+                        state: api_model::TranslationState::Unchanged,
+                        comment: "".to_string(),
+                        reviewer: None,
+                    },
+                    api_model::TranslationString {
+                        language: "sv".to_string(),
+                        translation: "Primära test strängen".to_string(),
+                        placeholder_offset: vec![],
+                        state: api_model::TranslationState::Unreviewed,
+                        comment: "".to_string(),
+                        reviewer: None,
+                    }
+                ],
+            },
+        ]
+    );
 }