Merge pull request #10565 from Turbo87/owner-deps

models/owners: Remove `App` dependency

Author: Turbo87

src/controllers/krate/owners.rs

@@ -244,6 +244,7 @@ async fn modify_owners(
                         )),
 
                             // An opaque error occurred.
+                            Err(OwnerAddError::Diesel(e)) => return Err(e.into()),
                             Err(OwnerAddError::AppError(e)) => return Err(e),
                         }
                     }
@@ -290,56 +291,78 @@ async fn add_owner(
     krate: &Crate,
     login: &str,
 ) -> Result<NewOwnerInvite, OwnerAddError> {
-    use diesel::insert_into;
-
-    let owner = Owner::find_or_create_by_login(app, conn, req_user, login).await?;
-    match owner {
-        // Users are invited and must accept before being added
-        Owner::User(user) => {
-            let expires_at = Utc::now() + app.config.ownership_invitations_expiration;
-            let invite = NewCrateOwnerInvitation {
-                invited_user_id: user.id,
-                invited_by_user_id: req_user.id,
-                crate_id: krate.id,
-                expires_at,
-            };
-
-            let creation_ret = invite.create(conn).await.map_err(BoxedAppError::from)?;
-
-            match creation_ret {
-                NewCrateOwnerInvitationOutcome::InviteCreated { plaintext_token } => {
-                    Ok(NewOwnerInvite::User(user, plaintext_token))
-                }
-                NewCrateOwnerInvitationOutcome::AlreadyExists => {
-                    Err(OwnerAddError::AlreadyInvited(Box::new(user)))
-                }
-            }
+    if login.contains(':') {
+        add_team_owner(app, conn, req_user, krate, login).await
+    } else {
+        invite_user_owner(app, conn, req_user, krate, login).await
+    }
+}
+
+async fn invite_user_owner(
+    app: &App,
+    conn: &mut AsyncPgConnection,
+    req_user: &User,
+    krate: &Crate,
+    login: &str,
+) -> Result<NewOwnerInvite, OwnerAddError> {
+    let user = User::find_by_login(conn, login)
+        .await
+        .optional()?
+        .ok_or_else(|| bad_request(format_args!("could not find user with login `{login}`")))?;
+
+    // Users are invited and must accept before being added
+    let expires_at = Utc::now() + app.config.ownership_invitations_expiration;
+    let invite = NewCrateOwnerInvitation {
+        invited_user_id: user.id,
+        invited_by_user_id: req_user.id,
+        crate_id: krate.id,
+        expires_at,
+    };
+
+    match invite.create(conn).await? {
+        NewCrateOwnerInvitationOutcome::InviteCreated { plaintext_token } => {
+            Ok(NewOwnerInvite::User(user, plaintext_token))
         }
-        // Teams are added as owners immediately
-        Owner::Team(team) => {
-            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)
-                .await
-                .map_err(BoxedAppError::from)?;
-
-            Ok(NewOwnerInvite::Team(team))
+        NewCrateOwnerInvitationOutcome::AlreadyExists => {
+            Err(OwnerAddError::AlreadyInvited(Box::new(user)))
         }
     }
 }
 
+async fn add_team_owner(
+    app: &App,
+    conn: &mut AsyncPgConnection,
+    req_user: &User,
+    krate: &Crate,
+    login: &str,
+) -> Result<NewOwnerInvite, OwnerAddError> {
+    // Always recreate teams to get the most up-to-date GitHub ID
+    let team = Team::create_or_update(app, conn, login, req_user).await?;
+
+    // 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)
+        .await?;
+
+    Ok(NewOwnerInvite::Team(team))
+}
+
 /// Error results from a [`add_owner()`] model call.
 #[derive(Debug, Error)]
 enum OwnerAddError {
+    #[error(transparent)]
+    Diesel(#[from] diesel::result::Error),
     /// An opaque [`BoxedAppError`].
     #[error("{0}")] // AppError does not impl Error
     AppError(BoxedAppError),

src/models/owner.rs

@@ -1,8 +1,5 @@
-use crate::app::App;
-use crate::util::errors::{bad_request, AppResult};
 use diesel::pg::Pg;
 use diesel::prelude::*;
-use diesel_async::AsyncPgConnection;
 
 use crate::models::{Crate, CrateOwnerInvitation, Team, User};
 use crate::schema::crate_owners;
@@ -63,32 +60,6 @@ pub enum Owner {
 }
 
 impl Owner {
-    /// Finds the owner by name. Always recreates teams to get the most
-    /// up-to-date GitHub ID. Fails out if the user isn't found in the
-    /// database, the team isn't found on GitHub, or if the user isn't a member
-    /// of the team on GitHub.
-    ///
-    /// May be a user's GH login or a full team name. This is case
-    /// sensitive.
-    pub async fn find_or_create_by_login(
-        app: &App,
-        conn: &mut AsyncPgConnection,
-        req_user: &User,
-        name: &str,
-    ) -> AppResult<Owner> {
-        if name.contains(':') {
-            Ok(Owner::Team(
-                Team::create_or_update(app, conn, name, req_user).await?,
-            ))
-        } else {
-            User::find_by_login(conn, name)
-                .await
-                .optional()?
-                .map(Owner::User)
-                .ok_or_else(|| bad_request(format_args!("could not find user with login `{name}`")))
-        }
-    }
-
     pub fn kind(&self) -> i32 {
         match self {
             Owner::User(_) => OwnerKind::User as i32,