Query for published_by with queries for version and crate

When we need published_by

Author: carols10cents

src/controllers/krate/metadata.rs

@@ -6,7 +6,8 @@
 
 use crate::controllers::prelude::*;
 use crate::models::{
-    Category, Crate, CrateCategory, CrateDownload, CrateKeyword, CrateVersions, Keyword, Version,
+    Category, Crate, CrateCategory, CrateDownload, CrateKeyword, CrateVersions, Keyword, User,
+    Version,
 };
 use crate::schema::*;
 use crate::views::{
@@ -218,10 +219,15 @@ pub fn reverse_dependencies(req: &mut dyn Request) -> CargoResult<Response> {
     let versions = versions::table
         .filter(versions::id.eq(any(version_ids)))
         .inner_join(crates::table)
-        .select((versions::all_columns, crates::name))
-        .load::<(Version, String)>(&*conn)?
+        .left_outer_join(users::table)
+        .select((
+            versions::all_columns,
+            crates::name,
+            users::all_columns.nullable(),
+        ))
+        .load::<(Version, String, Option<User>)>(&*conn)?
         .into_iter()
-        .map(|(version, krate_name)| version.encodable(&krate_name, None))
+        .map(|(version, krate_name, published_by)| version.encodable(&krate_name, published_by))
         .collect();
 
     #[derive(Serialize)]

src/controllers/user/me.rs

@@ -55,11 +55,16 @@ pub fn updates(req: &mut dyn Request) -> CargoResult<Response> {
     let followed_crates = Follow::belonging_to(user).select(follows::crate_id);
     let data = versions::table
         .inner_join(crates::table)
+        .left_outer_join(users::table)
         .filter(crates::id.eq(any(followed_crates)))
         .order(versions::created_at.desc())
-        .select((versions::all_columns, crates::name))
+        .select((
+            versions::all_columns,
+            crates::name,
+            users::all_columns.nullable(),
+        ))
         .paginate(limit, offset)
-        .load::<((Version, String), i64)>(&*conn)?;
+        .load::<((Version, String, Option<User>), i64)>(&*conn)?;
 
     let more = data
         .get(0)
@@ -68,7 +73,9 @@ pub fn updates(req: &mut dyn Request) -> CargoResult<Response> {
 
     let versions = data
         .into_iter()
-        .map(|((version, crate_name), _)| version.encodable(&crate_name, None))
+        .map(|((version, crate_name, published_by), _)| {
+            version.encodable(&crate_name, published_by)
+        })
         .collect();
 
     #[derive(Serialize)]

src/controllers/version/deprecated.rs

@@ -7,7 +7,7 @@
 
 use crate::controllers::prelude::*;
 
-use crate::models::{Crate, Version};
+use crate::models::{Crate, User, Version};
 use crate::schema::*;
 use crate::views::EncodableVersion;
 
@@ -24,11 +24,16 @@ pub fn index(req: &mut dyn Request) -> CargoResult<Response> {
 
     let versions = versions::table
         .inner_join(crates::table)
-        .select((versions::all_columns, crates::name))
+        .left_outer_join(users::table)
+        .select((
+            versions::all_columns,
+            crates::name,
+            users::all_columns.nullable(),
+        ))
         .filter(versions::id.eq(any(ids)))
-        .load::<(Version, String)>(&*conn)?
+        .load::<(Version, String, Option<User>)>(&*conn)?
         .into_iter()
-        .map(|(version, crate_name)| version.encodable(&crate_name, None))
+        .map(|(version, crate_name, published_by)| version.encodable(&crate_name, published_by))
         .collect();
 
     #[derive(Serialize)]
@@ -45,17 +50,22 @@ pub fn show_by_id(req: &mut dyn Request) -> CargoResult<Response> {
     let id = &req.params()["version_id"];
     let id = id.parse().unwrap_or(0);
     let conn = req.db_conn()?;
-    let (version, krate): (Version, Crate) = versions::table
+    let (version, krate, published_by): (Version, Crate, Option<User>) = versions::table
         .find(id)
         .inner_join(crates::table)
-        .select((versions::all_columns, crate::models::krate::ALL_COLUMNS))
+        .left_outer_join(users::table)
+        .select((
+            versions::all_columns,
+            crate::models::krate::ALL_COLUMNS,
+            users::all_columns.nullable(),
+        ))
         .first(&*conn)?;
 
     #[derive(Serialize)]
     struct R {
         version: EncodableVersion,
     }
     Ok(req.json(&R {
-        version: version.encodable(&krate.name, None),
+        version: version.encodable(&krate.name, published_by),
     }))
 }

src/tests/krate.rs

@@ -1752,6 +1752,46 @@ fn yanked_versions_not_included_in_reverse_dependencies() {
     assert_eq!(deps.meta.total, 0);
 }
 
+#[test]
+fn reverse_dependencies_includes_published_by_user_when_present() {
+    let (app, anon, user) = TestApp::init().with_user();
+    let user = user.as_model();
+
+    app.db(|conn| {
+        let c1 = CrateBuilder::new("c1", user.id)
+            .version("1.0.0")
+            .expect_build(conn);
+        CrateBuilder::new("c2", user.id)
+            .version(VersionBuilder::new("2.0.0").dependency(&c1, None))
+            .expect_build(conn);
+
+        // Make c2's version (and,incidentally, c1's, but that doesn't matter) mimic a version
+        // published before we started recording who published versions
+        let none: Option<i32> = None;
+        update(versions::table)
+            .set(versions::published_by.eq(none))
+            .execute(conn)
+            .unwrap();
+
+        // c3's version will have the published by info recorded
+        CrateBuilder::new("c3", user.id)
+            .version(VersionBuilder::new("3.0.0").dependency(&c1, None))
+            .expect_build(conn);
+    });
+
+    let deps = anon.reverse_dependencies("c1");
+    assert_eq!(deps.versions.len(), 2);
+
+    let c2_version = deps.versions.iter().find(|v| v.krate == "c2").unwrap();
+    assert!(c2_version.published_by.is_none());
+
+    let c3_version = deps.versions.iter().find(|v| v.krate == "c3").unwrap();
+    assert_eq!(
+        c3_version.published_by.as_ref().unwrap().login,
+        user.gh_login
+    );
+}
+
 #[test]
 fn author_license_and_description_required() {
     let (_, _, _, token) = TestApp::init().with_token();

src/tests/user.rs

@@ -142,6 +142,9 @@ fn crates_by_user_id_not_including_deleted_owners() {
 
 #[test]
 fn following() {
+    use cargo_registry::schema::versions;
+    use diesel::update;
+
     #[derive(Deserialize)]
     struct R {
         versions: Vec<EncodableVersion>,
@@ -153,12 +156,21 @@ fn following() {
     }
 
     let (app, _, user) = TestApp::init().with_user();
-    let user_id = user.as_model().id;
+    let user_model = user.as_model();
+    let user_id = user_model.id;
     app.db(|conn| {
         CrateBuilder::new("foo_fighters", user_id)
             .version(VersionBuilder::new("1.0.0"))
             .expect_build(conn);
 
+        // Make foo_fighters's version mimic a version published before we started recording who
+        // published versions
+        let none: Option<i32> = None;
+        update(versions::table)
+            .set(versions::published_by.eq(none))
+            .execute(conn)
+            .unwrap();
+
         CrateBuilder::new("bar_fighters", user_id)
             .version(VersionBuilder::new("1.0.0"))
             .expect_build(conn);
@@ -176,6 +188,21 @@ fn following() {
     let r: R = user.get("/api/v1/me/updates").good();
     assert_eq!(r.versions.len(), 2);
     assert_eq!(r.meta.more, false);
+    let foo_version = r
+        .versions
+        .iter()
+        .find(|v| v.krate == "foo_fighters")
+        .unwrap();
+    assert!(foo_version.published_by.is_none());
+    let bar_version = r
+        .versions
+        .iter()
+        .find(|v| v.krate == "bar_fighters")
+        .unwrap();
+    assert_eq!(
+        bar_version.published_by.as_ref().unwrap().login,
+        user_model.gh_login
+    );
 
     let r: R = user
         .get_with_query("/api/v1/me/updates", "per_page=1")