diff --git a/src/controllers/krate/owners.rs b/src/controllers/krate/owners.rs index 854af54ef1c..74e92b5c919 100644 --- a/src/controllers/krate/owners.rs +++ b/src/controllers/krate/owners.rs @@ -4,7 +4,7 @@ use crate::controllers::krate::CratePath; use crate::models::krate::OwnerRemoveError; use crate::models::{ krate::NewOwnerInvite, token::EndpointScope, CrateOwner, NewCrateOwnerInvitation, - NewCrateOwnerInvitationOutcome, OwnerKind, + NewCrateOwnerInvitationOutcome, }; use crate::models::{Crate, Owner, Rights, Team, User}; use crate::util::errors::{bad_request, crate_not_found, custom, AppResult, BoxedAppError}; @@ -15,7 +15,6 @@ use axum::Json; use axum_extra::json; use axum_extra::response::ErasedJson; use chrono::Utc; -use crates_io_database::schema::crate_owners; use crates_io_github::GitHubClient; use diesel::prelude::*; use diesel_async::scoped_futures::ScopedFutureExt; @@ -342,18 +341,12 @@ async fn add_team_owner( // Teams are added as owners immediately, since the above call ensures // the user is a team member. - diesel::insert_into(crate_owners::table) - .values(&CrateOwner { - crate_id: krate.id, - owner_id: team.id, - created_by: req_user.id, - owner_kind: OwnerKind::Team, - email_notifications: true, - }) - .on_conflict(crate_owners::table.primary_key()) - .do_update() - .set(crate_owners::deleted.eq(false)) - .execute(conn) + CrateOwner::builder() + .crate_id(krate.id) + .team_id(team.id) + .created_by(req_user.id) + .build() + .insert(conn) .await?; Ok(NewOwnerInvite::Team(team)) diff --git a/src/models/crate_owner_invitation.rs b/src/models/crate_owner_invitation.rs index 7a677da875e..da5a2e9c07b 100644 --- a/src/models/crate_owner_invitation.rs +++ b/src/models/crate_owner_invitation.rs @@ -5,7 +5,7 @@ use diesel_async::{AsyncConnection, AsyncPgConnection, RunQueryDsl}; use secrecy::SecretString; use crate::models::CrateOwner; -use crate::schema::{crate_owner_invitations, crate_owners, crates}; +use crate::schema::{crate_owner_invitations, crates}; #[derive(Debug)] pub enum NewCrateOwnerInvitationOutcome { @@ -101,13 +101,7 @@ impl CrateOwnerInvitation { conn.transaction(|conn| { async move { - diesel::insert_into(crate_owners::table) - .values(CrateOwner::from_invite(&self)) - .on_conflict(crate_owners::table.primary_key()) - .do_update() - .set(crate_owners::deleted.eq(false)) - .execute(conn) - .await?; + CrateOwner::from_invite(&self).insert(conn).await?; diesel::delete(&self).execute(conn).await?; diff --git a/src/models/krate.rs b/src/models/krate.rs index eca74eec68f..01cf6937544 100644 --- a/src/models/krate.rs +++ b/src/models/krate.rs @@ -128,17 +128,12 @@ impl NewCrate<'_> { .get_result(conn) .await?; - let owner = CrateOwner { - crate_id: krate.id, - owner_id: user_id, - created_by: user_id, - owner_kind: OwnerKind::User, - email_notifications: true, - }; - - diesel::insert_into(crate_owners::table) - .values(&owner) - .execute(conn) + CrateOwner::builder() + .crate_id(krate.id) + .user_id(user_id) + .created_by(user_id) + .build() + .insert(conn) .await?; Ok(krate) diff --git a/src/models/owner.rs b/src/models/owner.rs index 7f061da4700..13bc6d36880 100644 --- a/src/models/owner.rs +++ b/src/models/owner.rs @@ -1,11 +1,14 @@ +use bon::Builder; use diesel::pg::Pg; use diesel::prelude::*; +use diesel_async::{AsyncPgConnection, RunQueryDsl}; +use self::crate_owner_builder::{SetOwnerId, SetOwnerKind}; use crate::models::{Crate, CrateOwnerInvitation, Team, User}; use crate::schema::crate_owners; use crates_io_diesel_helpers::pg_enum; -#[derive(Insertable, Associations, Identifiable, Debug, Clone, Copy)] +#[derive(Insertable, Associations, Identifiable, Debug, Clone, Copy, Builder)] #[diesel( table_name = crate_owners, check_for_backend(diesel::pg::Pg), @@ -16,12 +19,33 @@ use crates_io_diesel_helpers::pg_enum; )] pub struct CrateOwner { pub crate_id: i32, + #[builder(setters(vis = "pub(self)"))] pub owner_id: i32, pub created_by: i32, + #[builder(setters(vis = "pub(self)"))] pub owner_kind: OwnerKind, + #[builder(default = true)] pub email_notifications: bool, } +impl CrateOwnerBuilder { + pub fn team_id(self, team_id: i32) -> CrateOwnerBuilder>> + where + S::OwnerId: crate_owner_builder::IsUnset, + S::OwnerKind: crate_owner_builder::IsUnset, + { + self.owner_kind(OwnerKind::Team).owner_id(team_id) + } + + pub fn user_id(self, user_id: i32) -> CrateOwnerBuilder>> + where + S::OwnerId: crate_owner_builder::IsUnset, + S::OwnerKind: crate_owner_builder::IsUnset, + { + self.owner_kind(OwnerKind::User).owner_id(user_id) + } +} + type BoxedQuery<'a> = crate_owners::BoxedQuery<'a, Pg, crate_owners::SqlType>; impl CrateOwner { @@ -35,13 +59,24 @@ impl CrateOwner { } pub fn from_invite(invite: &CrateOwnerInvitation) -> Self { - Self { - crate_id: invite.crate_id, - owner_id: invite.invited_user_id, - created_by: invite.invited_by_user_id, - owner_kind: OwnerKind::User, - email_notifications: true, - } + CrateOwner::builder() + .crate_id(invite.crate_id) + .user_id(invite.invited_user_id) + .created_by(invite.invited_by_user_id) + .build() + } + + /// Inserts the crate owner into the database, or removes the `deleted` flag + /// if the record already exists. + pub async fn insert(&self, conn: &mut AsyncPgConnection) -> QueryResult<()> { + diesel::insert_into(crate_owners::table) + .values(self) + .on_conflict(crate_owners::table.primary_key()) + .do_update() + .set(crate_owners::deleted.eq(false)) + .execute(conn) + .await + .map(|_| ()) } } diff --git a/src/tests/issues/issue2736.rs b/src/tests/issues/issue2736.rs index 4c845fee767..18b50231ff6 100644 --- a/src/tests/issues/issue2736.rs +++ b/src/tests/issues/issue2736.rs @@ -1,7 +1,7 @@ -use crate::models::{CrateOwner, OwnerKind}; +use crate::models::CrateOwner; use crate::tests::builders::CrateBuilder; use crate::tests::util::{RequestHelper, TestApp}; -use crates_io_database::schema::{crate_owners, users}; +use crates_io_database::schema::users; use diesel::prelude::*; use diesel_async::RunQueryDsl; use http::StatusCode; @@ -23,15 +23,12 @@ async fn test_issue_2736() -> anyhow::Result<()> { .expect_build(&mut conn) .await; - diesel::insert_into(crate_owners::table) - .values(CrateOwner { - crate_id: krate.id, - owner_id: foo1.as_model().id, - created_by: someone_else.as_model().id, - owner_kind: OwnerKind::User, - email_notifications: true, - }) - .execute(&mut conn) + CrateOwner::builder() + .crate_id(krate.id) + .user_id(foo1.as_model().id) + .created_by(someone_else.as_model().id) + .build() + .insert(&mut conn) .await?; // - `foo` deleted their GitHub account (but crates.io has no real knowledge of this) diff --git a/src/tests/mod.rs b/src/tests/mod.rs index a39e9409a63..6a4d141022e 100644 --- a/src/tests/mod.rs +++ b/src/tests/mod.rs @@ -1,5 +1,4 @@ -use crate::models::{Crate, CrateOwner, NewCategory, NewTeam, NewUser, OwnerKind, Team, User}; -use crate::schema::crate_owners; +use crate::models::{Crate, CrateOwner, NewCategory, NewTeam, NewUser, Team, User}; use crate::tests::util::{RequestHelper, TestApp}; use crate::views::{ EncodableCategory, EncodableCategoryWithSubcategories, EncodableCrate, EncodableKeyword, @@ -8,7 +7,7 @@ use crate::views::{ use crate::tests::util::github::next_gh_id; use diesel::prelude::*; -use diesel_async::{AsyncPgConnection, RunQueryDsl}; +use diesel_async::AsyncPgConnection; mod account_lock; mod authentication; @@ -114,23 +113,13 @@ pub async fn add_team_to_crate( u: &User, conn: &mut AsyncPgConnection, ) -> QueryResult<()> { - let crate_owner = CrateOwner { - crate_id: krate.id, - owner_id: t.id, - created_by: u.id, - owner_kind: OwnerKind::Team, - email_notifications: true, - }; - - diesel::insert_into(crate_owners::table) - .values(&crate_owner) - .on_conflict(crate_owners::table.primary_key()) - .do_update() - .set(crate_owners::deleted.eq(false)) - .execute(conn) - .await?; - - Ok(()) + CrateOwner::builder() + .crate_id(krate.id) + .team_id(t.id) + .created_by(u.id) + .build() + .insert(conn) + .await } fn new_category<'a>(category: &'a str, slug: &'a str, description: &'a str) -> NewCategory<'a> { diff --git a/src/tests/routes/crates/owners/remove.rs b/src/tests/routes/crates/owners/remove.rs index f226ec457c4..f667a704d68 100644 --- a/src/tests/routes/crates/owners/remove.rs +++ b/src/tests/routes/crates/owners/remove.rs @@ -1,7 +1,6 @@ -use crate::models::{CrateOwner, OwnerKind}; +use crate::models::CrateOwner; use crate::tests::builders::CrateBuilder; use crate::tests::util::{RequestHelper, TestApp}; -use crates_io_database::schema::crate_owners; use crates_io_github::{GitHubOrganization, GitHubTeam, GitHubTeamMembership, MockGitHubClient}; use http::StatusCode; use insta::assert_snapshot; @@ -83,8 +82,6 @@ async fn test_unknown_team() { #[tokio::test(flavor = "multi_thread")] async fn test_remove_uppercase_user() { - use diesel_async::RunQueryDsl; - let (app, _, cookie) = TestApp::full().with_user().await; let user2 = app.db_new_user("user2").await; let mut conn = app.db_conn().await; @@ -93,15 +90,12 @@ async fn test_remove_uppercase_user() { .expect_build(&mut conn) .await; - diesel::insert_into(crate_owners::table) - .values(CrateOwner { - crate_id: krate.id, - owner_id: user2.as_model().id, - created_by: cookie.as_model().id, - owner_kind: OwnerKind::User, - email_notifications: true, - }) - .execute(&mut conn) + CrateOwner::builder() + .crate_id(krate.id) + .user_id(user2.as_model().id) + .created_by(cookie.as_model().id) + .build() + .insert(&mut conn) .await .unwrap();