diff --git a/src/controllers/krate/metadata.rs b/src/controllers/krate/metadata.rs index 9e02718f8c3..c5040e52920 100644 --- a/src/controllers/krate/metadata.rs +++ b/src/controllers/krate/metadata.rs @@ -7,7 +7,7 @@ use crate::controllers::prelude::*; use crate::models::{ Category, Crate, CrateCategory, CrateKeyword, CrateVersions, Keyword, RecentCrateDownloads, - User, Version, + User, Version, VersionOwnerAction, }; use crate::schema::*; use crate::views::{ @@ -105,13 +105,26 @@ pub fn show(req: &mut dyn Request) -> AppResult { let conn = req.db_conn()?; let krate = Crate::by_name(name).first::(&*conn)?; - let mut versions_and_publishers: Vec<(Version, Option)> = krate + let mut versions_and_publishers = krate .all_versions() .left_outer_join(users::table) .select((versions::all_columns, users::all_columns.nullable())) - .load(&*conn)?; + .load::<(Version, Option)>(&*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 versions = versions_and_publishers + .iter() + .map(|(v, _)| v) + .cloned() + .collect::>(); + let versions_publishers_and_audit_actions = versions_and_publishers + .into_iter() + .zip(VersionOwnerAction::for_versions(&conn, &versions)?.into_iter()) + .map(|((v, pb), aas)| (v, pb, aas)) + .collect::>(); + let ids = versions_publishers_and_audit_actions + .iter() + .map(|v| v.0.id) + .collect(); let kws = CrateKeyword::belonging_to(&krate) .inner_join(keywords::table) @@ -149,9 +162,9 @@ pub fn show(req: &mut dyn Request) -> AppResult { false, recent_downloads, ), - versions: versions_and_publishers + versions: versions_publishers_and_audit_actions .into_iter() - .map(|(v, pb)| v.encodable(&krate.name, pb)) + .map(|(v, pb, aas)| v.encodable(&krate.name, pb, aas)) .collect(), keywords: kws.into_iter().map(Keyword::encodable).collect(), categories: cats.into_iter().map(Category::encodable).collect(), @@ -193,9 +206,15 @@ pub fn versions(req: &mut dyn Request) -> AppResult { .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 + .iter() + .map(|(v, _)| v) + .cloned() + .collect::>(); let versions = versions_and_publishers .into_iter() - .map(|(v, pb)| v.encodable(crate_name, pb)) + .zip(VersionOwnerAction::for_versions(&conn, &versions)?.into_iter()) + .map(|((v, pb), aas)| v.encodable(crate_name, pb, aas)) .collect(); #[derive(Serialize)] @@ -220,7 +239,7 @@ pub fn reverse_dependencies(req: &mut dyn Request) -> AppResult { let version_ids: Vec = rev_deps.iter().map(|dep| dep.version_id).collect(); - let versions = versions::table + let versions_and_publishers = versions::table .filter(versions::id.eq(any(version_ids))) .inner_join(crates::table) .left_outer_join(users::table) @@ -229,9 +248,18 @@ pub fn reverse_dependencies(req: &mut dyn Request) -> AppResult { crates::name, users::all_columns.nullable(), )) - .load::<(Version, String, Option)>(&*conn)? + .load::<(Version, String, Option)>(&*conn)?; + let versions = versions_and_publishers + .iter() + .map(|(v, _, _)| v) + .cloned() + .collect::>(); + let versions = versions_and_publishers .into_iter() - .map(|(version, krate_name, user)| version.encodable(&krate_name, user)) + .zip(VersionOwnerAction::for_versions(&conn, &versions)?.into_iter()) + .map(|((version, krate_name, published_by), actions)| { + version.encodable(&krate_name, published_by, actions) + }) .collect(); #[derive(Serialize)] diff --git a/src/controllers/user/me.rs b/src/controllers/user/me.rs index a67681f17d3..7fda17839cb 100644 --- a/src/controllers/user/me.rs +++ b/src/controllers/user/me.rs @@ -7,7 +7,9 @@ use crate::email; use crate::util::bad_request; use crate::util::errors::AppError; -use crate::models::{CrateOwner, Email, Follow, NewEmail, OwnerKind, User, Version}; +use crate::models::{ + CrateOwner, Email, Follow, NewEmail, OwnerKind, User, Version, VersionOwnerAction, +}; use crate::schema::{crate_owners, crates, emails, follows, users, versions}; use crate::views::{EncodableMe, EncodableVersion, OwnedCrate}; @@ -80,12 +82,19 @@ pub fn updates(req: &mut dyn Request) -> AppResult { )) .paginate(&req.query())? .load::<(Version, String, Option)>(&*conn)?; - let more = data.next_page_params().is_some(); + let versions = data.iter().map(|(v, _, _)| v).cloned().collect::>(); + let data = data + .into_iter() + .zip(VersionOwnerAction::for_versions(&conn, &versions)?.into_iter()) + .map(|((v, cn, pb), voas)| (v, cn, pb, voas)) + .collect::>(); let versions = data .into_iter() - .map(|(version, crate_name, published_by)| version.encodable(&crate_name, published_by)) + .map(|(version, crate_name, published_by, actions)| { + version.encodable(&crate_name, published_by, actions) + }) .collect(); #[derive(Serialize)] @@ -234,7 +243,7 @@ pub fn update_email_notifications(req: &mut dyn Request) -> AppResult let user = req.user()?; let conn = req.db_conn()?; - // Build inserts from existing crates beloning to the current user + // Build inserts from existing crates belonging to the current user let to_insert = CrateOwner::by_owner_kind(OwnerKind::User) .filter(owner_id.eq(user.id)) .select((crate_id, owner_id, owner_kind, email_notifications)) diff --git a/src/controllers/version/deprecated.rs b/src/controllers/version/deprecated.rs index 9246a9b8a1f..cec24a54a50 100644 --- a/src/controllers/version/deprecated.rs +++ b/src/controllers/version/deprecated.rs @@ -7,7 +7,7 @@ use crate::controllers::prelude::*; -use crate::models::{Crate, User, Version}; +use crate::models::{Crate, User, Version, VersionOwnerAction}; use crate::schema::*; use crate::views::EncodableVersion; @@ -22,7 +22,7 @@ pub fn index(req: &mut dyn Request) -> AppResult { .filter_map(|(ref a, ref b)| if *a == "ids[]" { b.parse().ok() } else { None }) .collect::>(); - let versions = versions::table + let versions_and_publishers = versions::table .inner_join(crates::table) .left_outer_join(users::table) .select(( @@ -31,9 +31,18 @@ pub fn index(req: &mut dyn Request) -> AppResult { users::all_columns.nullable(), )) .filter(versions::id.eq(any(ids))) - .load::<(Version, String, Option)>(&*conn)? + .load::<(Version, String, Option)>(&*conn)?; + let versions = versions_and_publishers + .iter() + .map(|(v, _, _)| v) + .cloned() + .collect::>(); + let versions = versions_and_publishers .into_iter() - .map(|(version, crate_name, published_by)| version.encodable(&crate_name, published_by)) + .zip(VersionOwnerAction::for_versions(&conn, &versions)?.into_iter()) + .map(|((version, crate_name, published_by), actions)| { + version.encodable(&crate_name, published_by, actions) + }) .collect(); #[derive(Serialize)] @@ -60,12 +69,13 @@ pub fn show_by_id(req: &mut dyn Request) -> AppResult { users::all_columns.nullable(), )) .first(&*conn)?; + let audit_actions = VersionOwnerAction::by_version(&conn, &version)?; #[derive(Serialize)] struct R { version: EncodableVersion, } Ok(req.json(&R { - version: version.encodable(&krate.name, published_by), + version: version.encodable(&krate.name, published_by, audit_actions), })) } diff --git a/src/controllers/version/metadata.rs b/src/controllers/version/metadata.rs index 9b985944a4f..ac3f9976717 100644 --- a/src/controllers/version/metadata.rs +++ b/src/controllers/version/metadata.rs @@ -6,6 +6,7 @@ use crate::controllers::prelude::*; +use crate::models::VersionOwnerAction; use crate::schema::*; use crate::views::{EncodableDependency, EncodablePublicUser, EncodableVersion}; @@ -70,12 +71,13 @@ pub fn show(req: &mut dyn Request) -> AppResult { let (version, krate) = version_and_crate(req)?; let conn = req.db_conn()?; let published_by = version.published_by(&conn); + let actions = VersionOwnerAction::by_version(&conn, &version)?; #[derive(Serialize)] struct R { version: EncodableVersion, } Ok(req.json(&R { - version: version.encodable(&krate.name, published_by), + version: version.encodable(&krate.name, published_by, actions), })) } diff --git a/src/models/action.rs b/src/models/action.rs index d764f2b7d1b..7efedc9acb3 100644 --- a/src/models/action.rs +++ b/src/models/action.rs @@ -20,6 +20,24 @@ pub enum VersionAction { Unyank = 2, } +impl Into<&'static str> for VersionAction { + fn into(self) -> &'static str { + match self { + VersionAction::Publish => "publish", + VersionAction::Yank => "yank", + VersionAction::Unyank => "unyank", + } + } +} + +impl Into for VersionAction { + fn into(self) -> String { + let string: &'static str = self.into(); + + string.into() + } +} + impl FromSql for VersionAction { fn from_sql(bytes: Option<&[u8]>) -> deserialize::Result { match >::from_sql(bytes)? { @@ -52,22 +70,30 @@ pub struct VersionOwnerAction { } impl VersionOwnerAction { - pub fn all(conn: &PgConnection) -> QueryResult> { + pub fn all(conn: &PgConnection) -> QueryResult> { version_owner_actions::table.load(conn) } - pub fn by_version_id_and_action( - conn: &PgConnection, - version_id_: i32, - action_: VersionAction, - ) -> QueryResult> { - use version_owner_actions::dsl::{action, version_id}; + pub fn by_version(conn: &PgConnection, version: &Version) -> QueryResult> { + use version_owner_actions::dsl::version_id; version_owner_actions::table - .filter(version_id.eq(version_id_)) - .filter(action.eq(action_)) + .filter(version_id.eq(version.id)) + .inner_join(users::table) + .order(version_owner_actions::dsl::id) .load(conn) } + + pub fn for_versions( + conn: &PgConnection, + versions: &[Version], + ) -> QueryResult>> { + Ok(Self::belonging_to(versions) + .inner_join(users::table) + .order(version_owner_actions::dsl::id) + .load::<(VersionOwnerAction, User)>(conn)? + .grouped_by(versions)) + } } pub fn insert_version_owner_action( diff --git a/src/models/version.rs b/src/models/version.rs index 4a140e1dde0..0ee71275d64 100644 --- a/src/models/version.rs +++ b/src/models/version.rs @@ -5,9 +5,9 @@ use diesel::prelude::*; use crate::util::{cargo_err, AppResult}; -use crate::models::{Crate, Dependency, User}; +use crate::models::{Crate, Dependency, User, VersionOwnerAction}; use crate::schema::*; -use crate::views::{EncodableVersion, EncodableVersionLinks}; +use crate::views::{EncodableAuditAction, EncodableVersion, EncodableVersionLinks}; // Queryable has a custom implementation below #[derive(Clone, Identifiable, Associations, Debug, Queryable, Deserialize, Serialize)] @@ -38,7 +38,12 @@ pub struct NewVersion { } impl Version { - pub fn encodable(self, crate_name: &str, published_by: Option) -> EncodableVersion { + pub fn encodable( + self, + crate_name: &str, + published_by: Option, + audit_actions: Vec<(VersionOwnerAction, User)>, + ) -> EncodableVersion { let Version { id, num, @@ -71,6 +76,14 @@ impl Version { }, crate_size, published_by: published_by.map(User::encodable_public), + audit_actions: audit_actions + .into_iter() + .map(|(audit_action, user)| EncodableAuditAction { + action: audit_action.action.into(), + user: User::encodable_public(user), + time: audit_action.time, + }) + .collect(), } } diff --git a/src/tests/krate.rs b/src/tests/krate.rs index 69ef45962fa..82815951f77 100644 --- a/src/tests/krate.rs +++ b/src/tests/krate.rs @@ -1590,7 +1590,7 @@ fn publish_after_yank_max_version() { #[test] fn publish_records_an_audit_action() { - use cargo_registry::models::{VersionAction, VersionOwnerAction}; + use cargo_registry::models::VersionOwnerAction; let (app, anon, _, token) = TestApp::full().with_token(); @@ -1600,85 +1600,59 @@ fn publish_records_an_audit_action() { let crate_to_publish = PublishBuilder::new("fyk"); token.enqueue_publish(crate_to_publish).good(); - // make sure it has one publish audit action - // do this as a full integration test once the api is in place. + // Make sure it has one publish audit action let json = anon.show_version("fyk", "1.0.0"); - let version_id = json.version.id; + let actions = json.version.audit_actions; - app.db(|conn| { - let actions = VersionOwnerAction::all(conn).unwrap(); - assert_eq!(actions.len(), 1); - let action = actions[0]; - assert_eq!(action.version_id, version_id); - assert_eq!(action.user_id, token.as_model().user_id); - assert_eq!(action.api_token_id, Some(token.as_model().id)); - assert_eq!(action.action, VersionAction::Publish); - }); + assert_eq!(actions.len(), 1); + let action = &actions[0]; + assert_eq!(action.action, "publish"); + assert_eq!(action.user.id, token.as_model().user_id); } #[test] fn yank_records_an_audit_action() { - use cargo_registry::models::{VersionAction, VersionOwnerAction}; - - let (app, anon, _, token) = TestApp::full().with_token(); + let (_, anon, _, token) = TestApp::full().with_token(); // Upload a new crate, putting it in the git index let crate_to_publish = PublishBuilder::new("fyk"); token.enqueue_publish(crate_to_publish).good(); - let json = anon.show_version("fyk", "1.0.0"); - let version_id = json.version.id; - - // yank it + // Yank it token.yank("fyk", "1.0.0").good(); - // make sure it has one publish and one yank audit action - // do this as a full integration test once the api is in place. - app.db(|conn| { - let actions = - VersionOwnerAction::by_version_id_and_action(conn, version_id, VersionAction::Yank) - .unwrap(); - assert_eq!(actions.len(), 1); - let action = actions[0]; - assert_eq!(action.version_id, version_id); - assert_eq!(action.user_id, token.as_model().user_id); - assert_eq!(action.api_token_id, Some(token.as_model().id)); - assert_eq!(action.action, VersionAction::Yank); - }); + // Make sure it has one publish and one yank audit action + let json = anon.show_version("fyk", "1.0.0"); + let actions = json.version.audit_actions; + + assert_eq!(actions.len(), 2); + let action = &actions[1]; + assert_eq!(action.action, "yank"); + assert_eq!(action.user.id, token.as_model().user_id); } #[test] fn unyank_records_an_audit_action() { - use cargo_registry::models::{VersionAction, VersionOwnerAction}; - - let (app, anon, _, token) = TestApp::full().with_token(); + let (_, anon, _, token) = TestApp::full().with_token(); // Upload a new crate let crate_to_publish = PublishBuilder::new("fyk"); token.enqueue_publish(crate_to_publish).good(); - let json = anon.show_version("fyk", "1.0.0"); - let version_id = json.version.id; - - // yank version 1.0.0 + // Yank version 1.0.0 token.yank("fyk", "1.0.0").good(); - // unyank version 1.0.0 + // Unyank version 1.0.0 token.unyank("fyk", "1.0.0").good(); - // make sure it has one publish, one yank, and one unyank audit action - // do this as a full integration test once the api is in place. - app.db(|conn| { - let actions = - VersionOwnerAction::by_version_id_and_action(conn, version_id, VersionAction::Unyank) - .unwrap(); - assert_eq!(actions.len(), 1); - let action = actions[0]; - assert_eq!(action.version_id, version_id); - assert_eq!(action.user_id, token.as_model().user_id); - assert_eq!(action.api_token_id, Some(token.as_model().id)); - assert_eq!(action.action, VersionAction::Unyank); - }); + // Make sure it has one publish, one yank, and one unyank audit action + let json = anon.show_version("fyk", "1.0.0"); + let actions = json.version.audit_actions; + + assert_eq!(actions.len(), 3); + let action = &actions[2]; + assert_eq!(action.action, "unyank"); + assert_eq!(action.user.id, token.as_model().user_id); } #[test] diff --git a/src/views.rs b/src/views.rs index cf29c4f2a61..981578e8be7 100644 --- a/src/views.rs +++ b/src/views.rs @@ -188,6 +188,14 @@ pub struct EncodablePublicUser { pub url: Option, } +#[derive(Deserialize, Serialize, Debug)] +pub struct EncodableAuditAction { + pub action: String, + pub user: EncodablePublicUser, + #[serde(with = "rfc3339")] + pub time: NaiveDateTime, +} + #[derive(Serialize, Deserialize, Debug)] pub struct EncodableVersion { pub id: i32, @@ -209,6 +217,7 @@ pub struct EncodableVersion { pub links: EncodableVersionLinks, pub crate_size: Option, pub published_by: Option, + pub audit_actions: Vec, } #[derive(Serialize, Deserialize, Debug)] @@ -313,6 +322,17 @@ mod tests { }, crate_size: Some(1234), published_by: None, + audit_actions: vec![EncodableAuditAction { + action: "publish".to_string(), + user: EncodablePublicUser { + id: 0, + login: String::new(), + name: None, + avatar: None, + url: None, + }, + time: NaiveDate::from_ymd(2017, 1, 6).and_hms(14, 23, 12), + }], }; let json = serde_json::to_string(&ver).unwrap(); assert!(json @@ -323,6 +343,10 @@ mod tests { .as_str() .find(r#""created_at":"2017-01-06T14:23:12+00:00""#) .is_some()); + assert!(json + .as_str() + .find(r#""time":"2017-01-06T14:23:12+00:00""#) + .is_some()); } #[test]