From b5096d34f5aa2ee58550bf74368fcd7c2b64f9c5 Mon Sep 17 00:00:00 2001 From: TicClick Date: Wed, 25 Feb 2026 20:34:50 +0100 Subject: [PATCH] recognize conflicts on file deletion --- src/helpers/conflicts.rs | 7 ++- src/helpers/conflicts/tests.rs | 104 +++++++++++++++++++++++++++++++++ src/test.rs | 62 +++++++++++++++++++- 3 files changed, 169 insertions(+), 4 deletions(-) diff --git a/src/helpers/conflicts.rs b/src/helpers/conflicts.rs index f039365..ce4a51d 100644 --- a/src/helpers/conflicts.rs +++ b/src/helpers/conflicts.rs @@ -164,7 +164,12 @@ pub fn compare_pulls( // (https://github.com/TicClick/observatory/issues/17) let article_expr = regex::Regex::new(r"^(..|..-..)\.md$").unwrap(); let article_filter = |p: &&unidiff::PatchedFile| -> bool { - let parts = std::path::Path::new(&p.target_file); + // For deleted files, target_file is /dev/null -- check source_file instead + let file_to_check = match p.target_file.contains("/dev/null") { + true => &p.source_file, + false => &p.target_file, + }; + let parts = std::path::Path::new(file_to_check); if let Some(filename) = parts.file_name() { if let Some(filename_cstr) = filename.to_str() { article_expr.is_match(filename_cstr) diff --git a/src/helpers/conflicts/tests.rs b/src/helpers/conflicts/tests.rs index 5ccd4c7..2018c90 100644 --- a/src/helpers/conflicts/tests.rs +++ b/src/helpers/conflicts/tests.rs @@ -186,3 +186,107 @@ async fn new_translation_marked_as_incomplete() { )] ); } + +#[tokio::test] +async fn deleted_file_conflict_with_modification() { + let mut gh = test::GitHubServer::new().await; + + let pulls = gh.pulls.entry("test/repo".into()).or_default(); + let modifying_pull = crate::structs::PullRequest { + id: 1, + number: 1, + state: "open".to_string(), + title: "Modify file".to_string(), + user: crate::structs::Actor { + id: 1, + login: "user1".to_string(), + }, + html_url: gh.url.pull_url("test/repo", 1), + created_at: chrono::Utc::now(), + updated_at: chrono::Utc::now(), + diff: Some(test::make_simple_diff(&["wiki/Article/en.md"])), + merged_at: None, + merged: false, + }; + pulls.insert(1, modifying_pull.clone()); + + let deleting_pull = crate::structs::PullRequest { + id: 2, + number: 2, + state: "open".to_string(), + title: "Delete file".to_string(), + user: crate::structs::Actor { + id: 2, + login: "user2".to_string(), + }, + html_url: gh.url.pull_url("test/repo", 2), + created_at: chrono::Utc::now(), + updated_at: chrono::Utc::now(), + diff: Some(test::make_deleted_file_diff(&["wiki/Article/en.md"])), + merged_at: None, + merged: false, + }; + pulls.insert(2, deleting_pull.clone()); + + assert_eq!( + compare_pulls(&deleting_pull, &modifying_pull), + vec![Conflict::overlap( + 2, + 1, + gh.url.pull_url("test/repo", 1), + vec!["wiki/Article/en.md".to_string()], + )] + ); +} + +#[tokio::test] +async fn deleted_file_conflict_with_addition() { + let mut gh = test::GitHubServer::new().await; + + let pulls = gh.pulls.entry("test/repo".into()).or_default(); + let adding_pull = crate::structs::PullRequest { + id: 1, + number: 1, + state: "open".to_string(), + title: "Add file".to_string(), + user: crate::structs::Actor { + id: 1, + login: "user1".to_string(), + }, + html_url: gh.url.pull_url("test/repo", 1), + created_at: chrono::Utc::now(), + updated_at: chrono::Utc::now(), + diff: Some(test::make_added_file_diff(&["wiki/Article/de.md"])), + merged_at: None, + merged: false, + }; + pulls.insert(1, adding_pull.clone()); + + let deleting_pull = crate::structs::PullRequest { + id: 2, + number: 2, + state: "open".to_string(), + title: "Delete file".to_string(), + user: crate::structs::Actor { + id: 2, + login: "user2".to_string(), + }, + html_url: gh.url.pull_url("test/repo", 2), + created_at: chrono::Utc::now(), + updated_at: chrono::Utc::now(), + diff: Some(test::make_deleted_file_diff(&["wiki/Article/de.md"])), + merged_at: None, + merged: false, + }; + pulls.insert(2, deleting_pull.clone()); + + assert_eq!( + compare_pulls(&deleting_pull, &adding_pull), + vec![Conflict::overlap( + 2, + 1, + gh.url.pull_url("test/repo", 1), + vec!["wiki/Article/de.md".to_string()], + )] + ); +} diff --git a/src/test.rs b/src/test.rs index 335990c..1c41cfc 100644 --- a/src/test.rs +++ b/src/test.rs @@ -16,12 +16,12 @@ index 5483f282a0a..2c8c1482b97 100644 --- a/{0} +++ b/{1} @@ -5,6 +5,7 @@ - + ## Test article - + + Do whatever you want. - + That's it, that's the article."#, file_name, file_name ) @@ -30,6 +30,62 @@ index 5483f282a0a..2c8c1482b97 100644 unidiff::PatchSet::from_str(&diff.join("\n")).unwrap() } +pub fn make_added_file_diff(file_names: &[&str]) -> unidiff::PatchSet { + let diff: Vec = file_names + .iter() + .map(|file_name| { + format!( + r#"diff --git a/{0} b/{0} +new file mode 100644 +index 0000000000..2c8c1482b97 +--- /dev/null ++++ b/{0} +@@ -0,0 +1,11 @@ ++--- ++tags: ++ - test ++--- ++ ++## Test article ++ ++Do whatever you want. ++ ++That's it, that's the article."#, + file_name + ) + }) + .collect(); + unidiff::PatchSet::from_str(&diff.join("\n")).unwrap() +} + +pub fn make_deleted_file_diff(file_names: &[&str]) -> unidiff::PatchSet { + let diff: Vec = file_names + .iter() + .map(|file_name| { + format!( + r#"diff --git a/{0} b/{0} +deleted file mode 100644 +index 2c8c1482b97..0000000000 +--- a/{0} ++++ /dev/null +@@ -1,11 +0,0 @@ +---- +-tags: +- - test +---- +- +-## Test article +- +-Do whatever you want. +- +-That's it, that's the article."#, + file_name + ) + }) + .collect(); + unidiff::PatchSet::from_str(&diff.join("\n")).unwrap() +} + pub struct GitHubServer { pub server: mockito::ServerGuard, pub url: GitHub,