Query for published_by for multiple versions where needed

Author: carols10cents

src/controllers/krate/metadata.rs

@@ -107,9 +107,13 @@ pub fn show(req: &mut dyn Request) -> CargoResult<Response> {
     let conn = req.db_conn()?;
     let krate = Crate::by_name(name).first::<Crate>(&*conn)?;
 
-    let mut versions = krate.all_versions().load::<Version>(&*conn)?;
-    versions.sort_by(|a, b| b.num.cmp(&a.num));
-    let ids = versions.iter().map(|v| v.id).collect();
+    let mut versions_and_publishers: Vec<(Version, Option<User>)> = krate
+        .all_versions()
+        .left_outer_join(users::table)
+        .select((versions::all_columns, users::all_columns.nullable()))
+        .load(&*conn)?;
+    versions_and_publishers.sort_by(|a, b| b.0.num.cmp(&a.0.num));
+    let ids = versions_and_publishers.iter().map(|v| v.0.id).collect();
 
     let kws = CrateKeyword::belonging_to(&krate)
         .inner_join(keywords::table)
@@ -147,9 +151,9 @@ pub fn show(req: &mut dyn Request) -> CargoResult<Response> {
             false,
             recent_downloads,
         ),
-        versions: versions
+        versions: versions_and_publishers
             .into_iter()
-            .map(|v| v.encodable(&krate.name, None))
+            .map(|(v, pb)| v.encodable(&krate.name, pb))
             .collect(),
         keywords: kws.into_iter().map(|k| k.encodable()).collect(),
         categories: cats.into_iter().map(|k| k.encodable()).collect(),
@@ -186,11 +190,15 @@ pub fn versions(req: &mut dyn Request) -> CargoResult<Response> {
     let crate_name = &req.params()["crate_id"];
     let conn = req.db_conn()?;
     let krate = Crate::by_name(crate_name).first::<Crate>(&*conn)?;
-    let mut versions = krate.all_versions().load::<Version>(&*conn)?;
-    versions.sort_by(|a, b| b.num.cmp(&a.num));
-    let versions = versions
+    let mut versions_and_publishers: Vec<(Version, Option<User>)> = krate
+        .all_versions()
+        .left_outer_join(users::table)
+        .select((versions::all_columns, users::all_columns.nullable()))
+        .load(&*conn)?;
+    versions_and_publishers.sort_by(|a, b| b.0.num.cmp(&a.0.num));
+    let versions = versions_and_publishers
         .into_iter()
-        .map(|v| v.encodable(crate_name, None))
+        .map(|(v, pb)| v.encodable(crate_name, pb))
         .collect();
 
     #[derive(Serialize)]

src/tests/krate.rs

@@ -420,7 +420,7 @@ fn show() {
     let user = user.as_model();
 
     let krate = app.db(|conn| {
-        CrateBuilder::new("foo_show", user.id)
+        let krate = CrateBuilder::new("foo_show", user.id)
             .description("description")
             .documentation("https://example.com")
             .homepage("http://example.com")
@@ -430,7 +430,18 @@ fn show() {
             .keyword("kw1")
             .downloads(20)
             .recent_downloads(10)
-            .expect_build(conn)
+            .expect_build(conn);
+
+        // Make version 1.0.0 mimic a version published before we started recording who published
+        // versions
+        let none: Option<i32> = None;
+        update(versions::table)
+            .filter(versions::num.eq("1.0.0"))
+            .set(versions::published_by.eq(none))
+            .execute(conn)
+            .unwrap();
+
+        krate
     });
 
     let json = anon.show_crate("foo_show");
@@ -448,6 +459,7 @@ fn show() {
     assert_eq!(json.versions[0].id, versions[0]);
     assert_eq!(json.versions[0].krate, json.krate.id);
     assert_eq!(json.versions[0].num, "1.0.0");
+    assert!(json.versions[0].published_by.is_none());
     let suffix = "/api/v1/crates/foo_show/1.0.0/download";
     assert!(
         json.versions[0].dl_path.ends_with(suffix),
@@ -459,6 +471,10 @@ fn show() {
 
     assert_eq!(json.versions[1].num, "0.5.1");
     assert_eq!(json.versions[2].num, "0.5.0");
+    assert_eq!(
+        json.versions[1].published_by.as_ref().unwrap().login,
+        user.gh_login
+    );
 }
 
 #[test]
@@ -489,6 +505,14 @@ fn versions() {
             .version("1.0.0")
             .version("0.5.0")
             .expect_build(conn);
+        // Make version 1.0.0 mimic a version published before we started recording who published
+        // versions
+        let none: Option<i32> = None;
+        update(versions::table)
+            .filter(versions::num.eq("1.0.0"))
+            .set(versions::published_by.eq(none))
+            .execute(conn)
+            .unwrap();
     });
 
     let json: VersionsList = anon.get("/api/v1/crates/foo_versions/versions").good();
@@ -497,6 +521,11 @@ fn versions() {
     assert_eq!(json.versions[0].num, "1.0.0");
     assert_eq!(json.versions[1].num, "0.5.1");
     assert_eq!(json.versions[2].num, "0.5.0");
+    assert!(json.versions[0].published_by.is_none());
+    assert_eq!(
+        json.versions[1].published_by.as_ref().unwrap().login,
+        user.gh_login
+    );
 }
 
 #[test]