Skip to content

Commit 5619ad2

Browse files
committed
Handle team update scenario where GitHub team has been recreated
1 parent 067c12e commit 5619ad2

File tree

2 files changed

+82
-7
lines changed

2 files changed

+82
-7
lines changed

crates/crates_io_database/src/models/team.rs

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -40,13 +40,33 @@ impl NewTeam<'_> {
4040
pub async fn create_or_update(&self, conn: &mut AsyncPgConnection) -> QueryResult<Team> {
4141
use diesel::insert_into;
4242

43-
insert_into(teams::table)
44-
.values(self)
45-
.on_conflict(teams::github_id)
46-
.do_update()
47-
.set(self)
48-
.get_result(conn)
49-
.await
43+
// First try to update an existing record with matching org_id and login
44+
let update_result = diesel::update(
45+
teams::table.filter(
46+
teams::org_id
47+
.eq(self.org_id)
48+
.and(teams::login.eq(self.login)),
49+
),
50+
)
51+
.set(self)
52+
.returning(Team::as_returning())
53+
.get_result(conn)
54+
.await;
55+
56+
match update_result {
57+
Ok(team) => Ok(team),
58+
Err(diesel::result::Error::NotFound) => {
59+
// No existing record found, try to insert
60+
insert_into(teams::table)
61+
.values(self)
62+
.on_conflict(teams::github_id)
63+
.do_update()
64+
.set(self)
65+
.get_result(conn)
66+
.await
67+
}
68+
Err(e) => Err(e),
69+
}
5070
}
5171
}
5272

src/tests/team.rs

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,61 @@ async fn add_renamed_team() -> anyhow::Result<()> {
124124
Ok(())
125125
}
126126

127+
/// Test adding a team that has the same name as an existing team, but a different ID
128+
/// This would happen if a team was deleted in GitHub and then recreated with the same name.
129+
#[tokio::test(flavor = "multi_thread")]
130+
async fn add_team_with_same_name_different_id() -> anyhow::Result<()> {
131+
let (app, anon) = TestApp::init().empty().await;
132+
let mut conn = app.db_conn().await;
133+
let user = app.db_new_user("user-all-teams").await;
134+
let token = user.db_new_token("arbitrary token name").await;
135+
let owner_id = user.as_model().id;
136+
137+
CrateBuilder::new("foo_same_name_different_id", owner_id)
138+
.expect_build(&mut conn)
139+
.await;
140+
141+
let original_team = NewTeam::builder()
142+
.login("github:test-org:core")
143+
.org_id(1000)
144+
.github_id(2001)
145+
.build()
146+
.create_or_update(&mut conn)
147+
.await?;
148+
149+
let new_team = NewTeam::builder()
150+
// same team name
151+
.login(&original_team.login)
152+
// same org ID
153+
.org_id(original_team.org_id)
154+
// different team ID
155+
.github_id(original_team.github_id + 1)
156+
.build()
157+
.create_or_update(&mut conn)
158+
.await?;
159+
160+
assert_eq!(
161+
crate::schema::teams::table
162+
.count()
163+
.get_result::<i64>(&mut conn)
164+
.await?,
165+
1
166+
);
167+
token
168+
.add_named_owner("foo_same_name_different_id", "github:test-org:core")
169+
.await
170+
.good();
171+
let json = anon
172+
.crate_owner_teams("foo_same_name_different_id")
173+
.await
174+
.good();
175+
176+
assert_eq!(json.teams.len(), 1);
177+
assert_eq!(json.teams[0].login, "github:test-org:core");
178+
assert_eq!(json.teams[0].id, new_team.id);
179+
Ok(())
180+
}
181+
127182
/// Test adding team names with mixed case, when on the team
128183
#[tokio::test(flavor = "multi_thread")]
129184
async fn add_team_mixed_case() -> anyhow::Result<()> {

0 commit comments

Comments
 (0)