Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 7 additions & 14 deletions src/controllers/krate/owners.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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;
Expand Down Expand Up @@ -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))
Expand Down
10 changes: 2 additions & 8 deletions src/models/crate_owner_invitation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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?;

Expand Down
17 changes: 6 additions & 11 deletions src/models/krate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
51 changes: 43 additions & 8 deletions src/models/owner.rs
Original file line number Diff line number Diff line change
@@ -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),
Expand All @@ -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<S: crate_owner_builder::State> CrateOwnerBuilder<S> {
pub fn team_id(self, team_id: i32) -> CrateOwnerBuilder<SetOwnerId<SetOwnerKind<S>>>
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<SetOwnerId<SetOwnerKind<S>>>
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 {
Expand All @@ -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(|_| ())
}
}

Expand Down
19 changes: 8 additions & 11 deletions src/tests/issues/issue2736.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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)
Expand Down
29 changes: 9 additions & 20 deletions src/tests/mod.rs
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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;
Expand Down Expand Up @@ -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> {
Expand Down
20 changes: 7 additions & 13 deletions src/tests/routes/crates/owners/remove.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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();

Expand Down