From dd2ad0598ed821312101ead8834ef7bf7d21688b Mon Sep 17 00:00:00 2001 From: Eh2406 Date: Wed, 22 Feb 2017 13:06:23 -0500 Subject: [PATCH 1/4] Sort authors for ui #467 --- src/version.rs | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/src/version.rs b/src/version.rs index 6316c56c178..d10a04e4066 100644 --- a/src/version.rs +++ b/src/version.rs @@ -166,17 +166,21 @@ impl Version { } pub fn authors(&self, conn: &GenericConnection) -> CargoResult> { - let stmt = conn.prepare("SELECT * FROM version_authors - WHERE version_id = $1")?; - let rows = stmt.query(&[&self.id])?; - rows.into_iter().map(|row| { + let stmt = try!(conn.prepare("SELECT * FROM version_authors + WHERE version_id = $1")); + let rows = try!(stmt.query(&[&self.id])); + let mut authors: CargoResult> = rows.into_iter().map(|row| { let user_id: Option = row.get("user_id"); let name: String = row.get("name"); Ok(match user_id { Some(id) => Author::User(User::find(conn, id)?), None => Author::Name(name), }) - }).collect() + }).collect(); + if let Ok(ref mut v) = authors { + v.sort_by(|ref a, ref b| a.name().cmp(&b.name())); + } + authors } pub fn add_author(&self, @@ -217,6 +221,15 @@ impl Model for Version { fn table_name(_: Option) -> &'static str { "versions" } } +impl Author { + fn name(&self) -> Option<&str> { + match self { + &Author::Name(ref n) => {Some(&n)}, + &Author::User(ref u) => {u.name.as_ref().map(String::as_str)} + } + } +} + /// Handles the `GET /versions` route. pub fn index(req: &mut Request) -> CargoResult { let conn = req.tx()?; From 93fd8e5d343c0d03b5c60a289443a5d57be0078e Mon Sep 17 00:00:00 2001 From: Eh2406 Date: Wed, 22 Feb 2017 13:44:54 -0500 Subject: [PATCH 2/4] try makes things cleaner --- src/version.rs | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/version.rs b/src/version.rs index d10a04e4066..c0b1857f225 100644 --- a/src/version.rs +++ b/src/version.rs @@ -166,21 +166,19 @@ impl Version { } pub fn authors(&self, conn: &GenericConnection) -> CargoResult> { - let stmt = try!(conn.prepare("SELECT * FROM version_authors - WHERE version_id = $1")); - let rows = try!(stmt.query(&[&self.id])); - let mut authors: CargoResult> = rows.into_iter().map(|row| { + let stmt = conn.prepare("SELECT * FROM version_authors + WHERE version_id = $1")?; + let rows = stmt.query(&[&self.id])?; + let mut authors = rows.into_iter().map(|row| { let user_id: Option = row.get("user_id"); let name: String = row.get("name"); Ok(match user_id { Some(id) => Author::User(User::find(conn, id)?), None => Author::Name(name), }) - }).collect(); - if let Ok(ref mut v) = authors { - v.sort_by(|ref a, ref b| a.name().cmp(&b.name())); - } - authors + }).collect::>>()?; + authors.sort_by(|ref a, ref b| a.name().cmp(&b.name())); + Ok(authors) } pub fn add_author(&self, From 78ddcf5cc6fcc26250f9345bb0a1c1aa6798125d Mon Sep 17 00:00:00 2001 From: Eh2406 Date: Sat, 4 Mar 2017 12:54:22 -0500 Subject: [PATCH 3/4] Sort authors in sql add a todo for why this works --- src/version.rs | 22 +++++----------------- 1 file changed, 5 insertions(+), 17 deletions(-) diff --git a/src/version.rs b/src/version.rs index c0b1857f225..a0c4a9b9e2c 100644 --- a/src/version.rs +++ b/src/version.rs @@ -34,7 +34,7 @@ pub struct Version { } pub enum Author { - User(User), + User(User), //TODO: not used. this should be removed. Name(String), } @@ -167,25 +167,22 @@ impl Version { pub fn authors(&self, conn: &GenericConnection) -> CargoResult> { let stmt = conn.prepare("SELECT * FROM version_authors - WHERE version_id = $1")?; + WHERE version_id = $1 + ORDER BY name ASC")?; let rows = stmt.query(&[&self.id])?; - let mut authors = rows.into_iter().map(|row| { + rows.into_iter().map(|row| { let user_id: Option = row.get("user_id"); let name: String = row.get("name"); Ok(match user_id { Some(id) => Author::User(User::find(conn, id)?), None => Author::Name(name), }) - }).collect::>>()?; - authors.sort_by(|ref a, ref b| a.name().cmp(&b.name())); - Ok(authors) + }).collect() } pub fn add_author(&self, conn: &GenericConnection, name: &str) -> CargoResult<()> { - println!("add author: {}", name); - // TODO: at least try to link `name` to a pre-existing user conn.execute("INSERT INTO version_authors (version_id, name) VALUES ($1, $2)", &[&self.id, &name])?; Ok(()) @@ -219,15 +216,6 @@ impl Model for Version { fn table_name(_: Option) -> &'static str { "versions" } } -impl Author { - fn name(&self) -> Option<&str> { - match self { - &Author::Name(ref n) => {Some(&n)}, - &Author::User(ref u) => {u.name.as_ref().map(String::as_str)} - } - } -} - /// Handles the `GET /versions` route. pub fn index(req: &mut Request) -> CargoResult { let conn = req.tx()?; From ccc7a40209b4dbf63d08c8f8e3c80c8475b86701 Mon Sep 17 00:00:00 2001 From: Eh2406 Date: Sat, 4 Mar 2017 13:34:37 -0500 Subject: [PATCH 4/4] Remove the User as Author from the back end. --- src/version.rs | 31 +++++++++++-------------------- 1 file changed, 11 insertions(+), 20 deletions(-) diff --git a/src/version.rs b/src/version.rs index a0c4a9b9e2c..d552e33ede6 100644 --- a/src/version.rs +++ b/src/version.rs @@ -10,7 +10,7 @@ use time::Duration; use time::Timespec; use url; -use {Model, Crate, User}; +use {Model, Crate}; use app::RequestApp; use db::RequestTransaction; use dependency::{Dependency, EncodableDependency, Kind}; @@ -33,9 +33,8 @@ pub struct Version { pub yanked: bool, } -pub enum Author { - User(User), //TODO: not used. this should be removed. - Name(String), +pub struct Author { + pub name: String } #[derive(RustcEncodable, RustcDecodable)] @@ -170,14 +169,9 @@ impl Version { WHERE version_id = $1 ORDER BY name ASC")?; let rows = stmt.query(&[&self.id])?; - rows.into_iter().map(|row| { - let user_id: Option = row.get("user_id"); - let name: String = row.get("name"); - Ok(match user_id { - Some(id) => Author::User(User::find(conn, id)?), - None => Author::Name(name), - }) - }).collect() + Ok(rows.into_iter().map(|row| { + Author { name: row.get("name") } + }).collect()) } pub fn add_author(&self, @@ -327,19 +321,16 @@ pub fn downloads(req: &mut Request) -> CargoResult { pub fn authors(req: &mut Request) -> CargoResult { let (version, _) = version_and_crate(req)?; let tx = req.tx()?; - let (mut users, mut names) = (Vec::new(), Vec::new()); - for author in version.authors(tx)?.into_iter() { - match author { - Author::User(u) => users.push(u.encodable()), - Author::Name(n) => names.push(n), - } - } + let names = version.authors(tx)?.into_iter().map(|a| a.name).collect(); + // It was imagined that we wold associate authors with users. + // This was never implemented. This complicated return struct + // is all that is left, hear for backwards compatibility. #[derive(RustcEncodable)] struct R { users: Vec<::user::EncodableUser>, meta: Meta } #[derive(RustcEncodable)] struct Meta { names: Vec } - Ok(req.json(&R{ users: users, meta: Meta { names: names } })) + Ok(req.json(&R{ users: vec![], meta: Meta { names: names } })) } /// Handles the `DELETE /crates/:crate_id/:version/yank` route.