diff --git a/crates/crates_io_database/src/models/team.rs b/crates/crates_io_database/src/models/team.rs index 83c95b1769..7c77525791 100644 --- a/crates/crates_io_database/src/models/team.rs +++ b/crates/crates_io_database/src/models/team.rs @@ -40,13 +40,33 @@ impl NewTeam<'_> { pub async fn create_or_update(&self, conn: &mut AsyncPgConnection) -> QueryResult { use diesel::insert_into; - insert_into(teams::table) - .values(self) - .on_conflict(teams::github_id) - .do_update() - .set(self) - .get_result(conn) - .await + // First try to update an existing record with matching org_id and login + let update_result = diesel::update( + teams::table.filter( + teams::org_id + .eq(self.org_id) + .and(teams::login.eq(self.login)), + ), + ) + .set(self) + .returning(Team::as_returning()) + .get_result(conn) + .await; + + match update_result { + Ok(team) => Ok(team), + Err(diesel::result::Error::NotFound) => { + // No existing record found, try to insert + insert_into(teams::table) + .values(self) + .on_conflict(teams::github_id) + .do_update() + .set(self) + .get_result(conn) + .await + } + Err(e) => Err(e), + } } } diff --git a/src/tests/team.rs b/src/tests/team.rs index d0bb994724..3a35a9db3b 100644 --- a/src/tests/team.rs +++ b/src/tests/team.rs @@ -124,6 +124,61 @@ async fn add_renamed_team() -> anyhow::Result<()> { Ok(()) } +/// Test adding a team that has the same name as an existing team, but a different ID +/// This would happen if a team was deleted in GitHub and then recreated with the same name. +#[tokio::test(flavor = "multi_thread")] +async fn add_team_with_same_name_different_id() -> anyhow::Result<()> { + let (app, anon) = TestApp::init().empty().await; + let mut conn = app.db_conn().await; + let user = app.db_new_user("user-all-teams").await; + let token = user.db_new_token("arbitrary token name").await; + let owner_id = user.as_model().id; + + CrateBuilder::new("foo_same_name_different_id", owner_id) + .expect_build(&mut conn) + .await; + + let original_team = NewTeam::builder() + .login("github:test-org:core") + .org_id(1000) + .github_id(2001) + .build() + .create_or_update(&mut conn) + .await?; + + let new_team = NewTeam::builder() + // same team name + .login(&original_team.login) + // same org ID + .org_id(original_team.org_id) + // different team ID + .github_id(original_team.github_id + 1) + .build() + .create_or_update(&mut conn) + .await?; + + assert_eq!( + crate::schema::teams::table + .count() + .get_result::(&mut conn) + .await?, + 1 + ); + token + .add_named_owner("foo_same_name_different_id", "github:test-org:core") + .await + .good(); + let json = anon + .crate_owner_teams("foo_same_name_different_id") + .await + .good(); + + assert_eq!(json.teams.len(), 1); + assert_eq!(json.teams[0].login, "github:test-org:core"); + assert_eq!(json.teams[0].id, new_team.id); + Ok(()) +} + /// Test adding team names with mixed case, when on the team #[tokio::test(flavor = "multi_thread")] async fn add_team_mixed_case() -> anyhow::Result<()> {