From dfd89f1539308cb6c8656dfd3bd3f6c53e10cdd2 Mon Sep 17 00:00:00 2001 From: "Carol (Nichols || Goulding)" Date: Thu, 31 Aug 2017 19:59:31 -0400 Subject: [PATCH 1/3] Create a table for crate invitations --- .../2017-08-31-230457_invitations/down.sql | 1 + .../2017-08-31-230457_invitations/up.sql | 7 ++++ src/schema.rs | 33 +++++++++++++++++++ 3 files changed, 41 insertions(+) create mode 100644 migrations/2017-08-31-230457_invitations/down.sql create mode 100644 migrations/2017-08-31-230457_invitations/up.sql diff --git a/migrations/2017-08-31-230457_invitations/down.sql b/migrations/2017-08-31-230457_invitations/down.sql new file mode 100644 index 0000000000..ce15c61a0e --- /dev/null +++ b/migrations/2017-08-31-230457_invitations/down.sql @@ -0,0 +1 @@ +DROP TABLE crate_owner_invitations; \ No newline at end of file diff --git a/migrations/2017-08-31-230457_invitations/up.sql b/migrations/2017-08-31-230457_invitations/up.sql new file mode 100644 index 0000000000..19d07843ef --- /dev/null +++ b/migrations/2017-08-31-230457_invitations/up.sql @@ -0,0 +1,7 @@ +CREATE TABLE crate_owner_invitations ( + invited_user_id INTEGER NOT NULL REFERENCES users (id) ON DELETE CASCADE, + invited_by_user_id INTEGER NOT NULL REFERENCES users (id) ON DELETE CASCADE, + crate_id INTEGER NOT NULL REFERENCES crates (id) ON DELETE CASCADE, + created_at TIMESTAMP NOT NULL DEFAULT now(), + PRIMARY KEY (invited_user_id, crate_id) +); diff --git a/src/schema.rs b/src/schema.rs index 9d55898cb5..80ef28cc82 100644 --- a/src/schema.rs +++ b/src/schema.rs @@ -138,6 +138,38 @@ table! { } } +table! { + /// Representation of the `crate_owner_invitations` table. + /// + /// (Automatically generated by Diesel.) + crate_owner_invitations (invited_user_id, crate_id) { + /// The `invited_user_id` column of the `crate_owner_invitations` table. + /// + /// Its SQL type is `Int4`. + /// + /// (Automatically generated by Diesel.) + invited_user_id -> Int4, + /// The `invited_by_user_id` column of the `crate_owner_invitations` table. + /// + /// Its SQL type is `Int4`. + /// + /// (Automatically generated by Diesel.) + invited_by_user_id -> Int4, + /// The `crate_id` column of the `crate_owner_invitations` table. + /// + /// Its SQL type is `Int4`. + /// + /// (Automatically generated by Diesel.) + crate_id -> Int4, + /// The `created_at` column of the `crate_owner_invitations` table. + /// + /// Its SQL type is `Timestamp`. + /// + /// (Automatically generated by Diesel.) + created_at -> Timestamp, + } +} + table! { /// Representation of the `crate_owners` table. /// @@ -720,3 +752,4 @@ joinable!(version_downloads -> versions (version_id)); joinable!(crate_owners -> teams (owner_id)); joinable!(crate_owners -> users (owner_id)); joinable!(readme_rendering -> versions (version_id)); +joinable!(crate_owner_invitations -> crates (crate_id)); From b3808ad2478dd9c847850ecbb01d93b0e693050f Mon Sep 17 00:00:00 2001 From: "Carol (Nichols || Goulding)" Date: Sun, 3 Sep 2017 22:19:23 -0400 Subject: [PATCH 2/3] Move existing ownership-related tests to their own file --- src/tests/all.rs | 27 +- ...new_crate_owner => owners_new_crate_owner} | 0 src/tests/krate.rs | 310 +----------------- src/tests/owners.rs | 293 +++++++++++++++++ 4 files changed, 319 insertions(+), 311 deletions(-) rename src/tests/http-data/{krate_new_crate_owner => owners_new_crate_owner} (100%) create mode 100644 src/tests/owners.rs diff --git a/src/tests/all.rs b/src/tests/all.rs index 3d3456061f..308e5f8c9a 100644 --- a/src/tests/all.rs +++ b/src/tests/all.rs @@ -31,8 +31,8 @@ use cargo_registry::app::App; use cargo_registry::category::NewCategory; use cargo_registry::dependency::NewDependency; use cargo_registry::keyword::Keyword; -use cargo_registry::krate::{NewCrate, CrateDownload}; -use cargo_registry::schema::dependencies; +use cargo_registry::krate::{NewCrate, CrateDownload, EncodableCrate}; +use cargo_registry::schema::*; use cargo_registry::upload as u; use cargo_registry::user::NewUser; use cargo_registry::owner::{CrateOwner, NewTeam, Team}; @@ -43,7 +43,6 @@ use conduit::{Request, Method}; use conduit_test::MockRequest; use diesel::prelude::*; use diesel::pg::upsert::*; -use cargo_registry::schema::*; macro_rules! t { ($e:expr) => ( @@ -89,6 +88,7 @@ mod category; mod git; mod keyword; mod krate; +mod owners; mod record; mod schema_details; mod team; @@ -96,6 +96,27 @@ mod token; mod user; mod version; +#[derive(Deserialize)] +struct GoodCrate { + #[serde(rename = "crate")] + krate: EncodableCrate, + warnings: Warnings, +} +#[derive(Deserialize)] +struct CrateList { + crates: Vec, + meta: CrateMeta, +} +#[derive(Deserialize)] +struct Warnings { + invalid_categories: Vec, + invalid_badges: Vec, +} +#[derive(Deserialize)] +struct CrateMeta { + total: i32, +} + fn app() -> (record::Bomb, Arc, conduit_middleware::MiddlewareBuilder) { dotenv::dotenv().ok(); git::init(); diff --git a/src/tests/http-data/krate_new_crate_owner b/src/tests/http-data/owners_new_crate_owner similarity index 100% rename from src/tests/http-data/krate_new_crate_owner rename to src/tests/http-data/owners_new_crate_owner diff --git a/src/tests/krate.rs b/src/tests/krate.rs index 30eedc27df..7c2f72a6ee 100644 --- a/src/tests/krate.rs +++ b/src/tests/krate.rs @@ -18,39 +18,19 @@ use cargo_registry::keyword::EncodableKeyword; use cargo_registry::krate::{Crate, EncodableCrate, MAX_NAME_LENGTH}; use cargo_registry::token::ApiToken; -use cargo_registry::owner::EncodableOwner; use cargo_registry::schema::{versions, crates}; use cargo_registry::upload as u; -use cargo_registry::user::EncodablePublicUser; use cargo_registry::version::EncodableVersion; use cargo_registry::category::Category; -#[derive(Deserialize)] -struct CrateList { - crates: Vec, - meta: CrateMeta, -} +use {CrateList, GoodCrate, CrateMeta}; + #[derive(Deserialize)] struct VersionsList { versions: Vec, } #[derive(Deserialize)] -struct CrateMeta { - total: i32, -} -#[derive(Deserialize)] -struct Warnings { - invalid_categories: Vec, - invalid_badges: Vec, -} -#[derive(Deserialize)] -struct GoodCrate { - #[serde(rename = "crate")] - krate: EncodableCrate, - warnings: Warnings, -} -#[derive(Deserialize)] struct CrateResponse { #[serde(rename = "crate")] krate: EncodableCrate, @@ -71,14 +51,6 @@ struct RevDeps { struct Downloads { version_downloads: Vec, } -#[derive(Deserialize)] -struct TeamResponse { - teams: Vec, -} -#[derive(Deserialize)] -struct UserResponse { - users: Vec, -} fn new_crate(name: &str) -> u::NewCrate { u::NewCrate { @@ -768,68 +740,6 @@ fn new_krate_bad_name() { } } -#[test] -fn new_crate_owner() { - #[derive(Deserialize)] - struct O { - ok: bool, - } - - let (_b, app, middle) = ::app(); - - // Create a crate under one user - let mut req = ::new_req(app.clone(), "foo_owner", "1.0.0"); - ::sign_in(&mut req, &app); - let u2; - { - let conn = app.diesel_database.get().unwrap(); - u2 = ::new_user("bar").create_or_update(&conn).unwrap(); - } - let mut response = ok_resp!(middle.call(&mut req)); - ::json::(&mut response); - - // Flag the second user as an owner - let body = r#"{"users":["bar"]}"#; - let mut response = ok_resp!( - middle.call( - req.with_path("/api/v1/crates/foo_owner/owners") - .with_method(Method::Put) - .with_body(body.as_bytes()), - ) - ); - assert!(::json::(&mut response).ok); - bad_resp!( - middle.call( - req.with_path("/api/v1/crates/foo_owner/owners") - .with_method(Method::Put) - .with_body(body.as_bytes()), - ) - ); - - // Make sure this shows up as one of their crates. - let query = format!("user_id={}", u2.id); - let mut response = ok_resp!( - middle.call( - req.with_path("/api/v1/crates") - .with_method(Method::Get) - .with_query(&query), - ) - ); - assert_eq!(::json::(&mut response).crates.len(), 1); - - // And upload a new crate as the first user - let body = ::new_req_body_version_2(::krate("foo_owner")); - ::sign_in_as(&mut req, &u2); - let mut response = ok_resp!( - middle.call( - req.with_path("/api/v1/crates/new") - .with_method(Method::Put) - .with_body(&body), - ) - ); - ::json::(&mut response); -} - #[test] fn valid_feature_names() { assert!(Crate::valid_feature_name("foo")); @@ -1266,133 +1176,6 @@ fn following() { assert_eq!(::json::(&mut response).crates.len(), 0); } -// Ensures that so long as at least one owner remains associated with the crate, -// a user can still remove their own login as an owner -#[test] -fn owners_can_remove_self() { - #[derive(Deserialize)] - struct R { - users: Vec, - } - #[derive(Deserialize)] - struct O { - ok: bool, - } - - let (_b, app, middle) = ::app(); - let mut req = ::req( - app.clone(), - Method::Get, - "/api/v1/crates/owners_selfremove/owners", - ); - { - let conn = app.diesel_database.get().unwrap(); - ::new_user("secondowner").create_or_update(&conn).unwrap(); - let user = ::new_user("firstowner").create_or_update(&conn).unwrap(); - ::sign_in_as(&mut req, &user); - ::CrateBuilder::new("owners_selfremove", user.id).expect_build(&conn); - } - - let mut response = ok_resp!(middle.call(&mut req)); - let r: R = ::json(&mut response); - assert_eq!(r.users.len(), 1); - - // Deleting yourself when you're the only owner isn't allowed. - let body = r#"{"users":["firstowner"]}"#; - let mut response = ok_resp!(middle.call(req.with_method(Method::Delete).with_body( - body.as_bytes(), - ))); - let json = ::json::<::Bad>(&mut response); - assert!(json.errors[0].detail.contains( - "cannot remove the sole owner of a crate", - )); - - let body = r#"{"users":["secondowner"]}"#; - let mut response = ok_resp!(middle.call(req.with_method(Method::Put).with_body( - body.as_bytes(), - ))); - assert!(::json::(&mut response).ok); - - // Deleting yourself when there are other owners is allowed. - let body = r#"{"users":["firstowner"]}"#; - let mut response = ok_resp!(middle.call(req.with_method(Method::Delete).with_body( - body.as_bytes(), - ))); - assert!(::json::(&mut response).ok); - - // After you delete yourself, you no longer have permisions to manage the crate. - let body = r#"{"users":["secondowner"]}"#; - let mut response = ok_resp!(middle.call(req.with_method(Method::Delete).with_body( - body.as_bytes(), - ))); - let json = ::json::<::Bad>(&mut response); - assert!(json.errors[0].detail.contains( - "only owners have permission to modify owners", - )); -} - -#[test] -fn owners() { - #[derive(Deserialize)] - struct R { - users: Vec, - } - #[derive(Deserialize)] - struct O { - ok: bool, - } - - let (_b, app, middle) = ::app(); - let mut req = ::req(app.clone(), Method::Get, "/api/v1/crates/foo_owners/owners"); - { - let conn = app.diesel_database.get().unwrap(); - ::new_user("foobar").create_or_update(&conn).unwrap(); - let user = ::new_user("foo").create_or_update(&conn).unwrap(); - ::sign_in_as(&mut req, &user); - ::CrateBuilder::new("foo_owners", user.id).expect_build(&conn); - } - - let mut response = ok_resp!(middle.call(&mut req)); - let r: R = ::json(&mut response); - assert_eq!(r.users.len(), 1); - - let mut response = ok_resp!(middle.call(req.with_method(Method::Get))); - let r: R = ::json(&mut response); - assert_eq!(r.users.len(), 1); - - let body = r#"{"users":["foobar"]}"#; - let mut response = ok_resp!(middle.call(req.with_method(Method::Put).with_body( - body.as_bytes(), - ))); - assert!(::json::(&mut response).ok); - - let mut response = ok_resp!(middle.call(req.with_method(Method::Get))); - let r: R = ::json(&mut response); - assert_eq!(r.users.len(), 2); - - let body = r#"{"users":["foobar"]}"#; - let mut response = ok_resp!(middle.call(req.with_method(Method::Delete).with_body( - body.as_bytes(), - ))); - assert!(::json::(&mut response).ok); - - let mut response = ok_resp!(middle.call(req.with_method(Method::Get))); - let r: R = ::json(&mut response); - assert_eq!(r.users.len(), 1); - - let body = r#"{"users":["foo"]}"#; - let mut response = ok_resp!(middle.call(req.with_method(Method::Delete).with_body( - body.as_bytes(), - ))); - ::json::<::Bad>(&mut response); - - let body = r#"{"users":["foobar"]}"#; - let mut response = ok_resp!(middle.call(req.with_method(Method::Put).with_body( - body.as_bytes(), - ))); - assert!(::json::(&mut response).ok); -} - #[test] fn yank() { #[derive(Deserialize)] @@ -1976,95 +1759,6 @@ fn author_license_and_description_required() { ); } -/* Testing the crate ownership between two crates and one team. - Given two crates, one crate owned by both a team and a user, - one only owned by a user, check that the CrateList returned - for the user_id contains only the crates owned by that user, - and that the CrateList returned for the team_id contains - only crates owned by that team. -*/ -#[test] -fn check_ownership_two_crates() { - let (_b, app, middle) = ::app(); - - let (krate_owned_by_team, team) = { - let conn = app.diesel_database.get().unwrap(); - let u = ::new_user("user_foo").create_or_update(&conn).unwrap(); - let t = ::new_team("team_foo").create_or_update(&conn).unwrap(); - let krate = ::CrateBuilder::new("foo", u.id).expect_build(&conn); - ::add_team_to_crate(&t, &krate, &u, &conn).unwrap(); - (krate, t) - }; - - let (krate_not_owned_by_team, user) = { - let conn = app.diesel_database.get().unwrap(); - let u = ::new_user("user_bar").create_or_update(&conn).unwrap(); - (::CrateBuilder::new("bar", u.id).expect_build(&conn), u) - }; - - let mut req = ::req(app.clone(), Method::Get, "/api/v1/crates"); - - let query = format!("user_id={}", user.id); - let mut response = ok_resp!(middle.call(req.with_query(&query))); - let json: CrateList = ::json(&mut response); - - assert_eq!(json.crates[0].name, krate_not_owned_by_team.name); - assert_eq!(json.crates.len(), 1); - - let query = format!("team_id={}", team.id); - let mut response = ok_resp!(middle.call(req.with_query(&query))); - - let json: CrateList = ::json(&mut response); - assert_eq!(json.crates.len(), 1); - assert_eq!(json.crates[0].name, krate_owned_by_team.name); -} - -/* Given a crate owned by both a team and a user, check that the - JSON returned by the /owner_team route and /owner_user route - contains the correct kind of owner - - Note that in this case function new_team must take a team name - of form github:org_name:team_name as that is the format - EncodableOwner::encodable is expecting -*/ -#[test] -fn check_ownership_one_crate() { - let (_b, app, middle) = ::app(); - - let (team, user) = { - let conn = app.diesel_database.get().unwrap(); - let u = ::new_user("user_cat").create_or_update(&conn).unwrap(); - let t = ::new_team("github:test_org:team_sloth") - .create_or_update(&conn) - .unwrap(); - let krate = ::CrateBuilder::new("best_crate", u.id).expect_build(&conn); - ::add_team_to_crate(&t, &krate, &u, &conn).unwrap(); - (t, u) - }; - - let mut req = ::req( - app.clone(), - Method::Get, - "/api/v1/crates/best_crate/owner_team", - ); - let mut response = ok_resp!(middle.call(&mut req)); - let json: TeamResponse = ::json(&mut response); - - assert_eq!(json.teams[0].kind, "team"); - assert_eq!(json.teams[0].name, team.name); - - let mut req = ::req( - app.clone(), - Method::Get, - "/api/v1/crates/best_crate/owner_user", - ); - let mut response = ok_resp!(middle.call(&mut req)); - let json: UserResponse = ::json(&mut response); - - assert_eq!(json.users[0].kind, "user"); - assert_eq!(json.users[0].name, user.name); -} - /* Given two crates, one with downloads less than 90 days ago, the other with all downloads greater than 90 days ago, check that the order returned is by recent downloads, descending. Check diff --git a/src/tests/owners.rs b/src/tests/owners.rs new file mode 100644 index 0000000000..add8dd5b0a --- /dev/null +++ b/src/tests/owners.rs @@ -0,0 +1,293 @@ +use {CrateList, GoodCrate}; + +use cargo_registry::owner::EncodableOwner; +use cargo_registry::user::EncodablePublicUser; + +use conduit::{Handler, Method}; + +#[derive(Deserialize)] +struct TeamResponse { + teams: Vec, +} +#[derive(Deserialize)] +struct UserResponse { + users: Vec, +} + +#[test] +fn new_crate_owner() { + #[derive(Deserialize)] + struct O { + ok: bool, + } + + let (_b, app, middle) = ::app(); + + // Create a crate under one user + let mut req = ::new_req(app.clone(), "foo_owner", "1.0.0"); + ::sign_in(&mut req, &app); + let u2; + { + let conn = app.diesel_database.get().unwrap(); + u2 = ::new_user("bar").create_or_update(&conn).unwrap(); + } + let mut response = ok_resp!(middle.call(&mut req)); + ::json::(&mut response); + + // Flag the second user as an owner + let body = r#"{"users":["bar"]}"#; + let mut response = ok_resp!( + middle.call( + req.with_path("/api/v1/crates/foo_owner/owners") + .with_method(Method::Put) + .with_body(body.as_bytes()), + ) + ); + assert!(::json::(&mut response).ok); + bad_resp!( + middle.call( + req.with_path("/api/v1/crates/foo_owner/owners") + .with_method(Method::Put) + .with_body(body.as_bytes()), + ) + ); + + // Make sure this shows up as one of their crates. + let query = format!("user_id={}", u2.id); + let mut response = ok_resp!( + middle.call( + req.with_path("/api/v1/crates") + .with_method(Method::Get) + .with_query(&query), + ) + ); + assert_eq!(::json::(&mut response).crates.len(), 1); + + // And upload a new crate as the first user + let body = ::new_req_body_version_2(::krate("foo_owner")); + ::sign_in_as(&mut req, &u2); + let mut response = ok_resp!( + middle.call( + req.with_path("/api/v1/crates/new") + .with_method(Method::Put) + .with_body(&body), + ) + ); + ::json::(&mut response); +} + +// Ensures that so long as at least one owner remains associated with the crate, +// a user can still remove their own login as an owner +#[test] +fn owners_can_remove_self() { + #[derive(Deserialize)] + struct R { + users: Vec, + } + #[derive(Deserialize)] + struct O { + ok: bool, + } + + let (_b, app, middle) = ::app(); + let mut req = ::req( + app.clone(), + Method::Get, + "/api/v1/crates/owners_selfremove/owners", + ); + { + let conn = app.diesel_database.get().unwrap(); + ::new_user("secondowner").create_or_update(&conn).unwrap(); + let user = ::new_user("firstowner").create_or_update(&conn).unwrap(); + ::sign_in_as(&mut req, &user); + ::CrateBuilder::new("owners_selfremove", user.id).expect_build(&conn); + } + + let mut response = ok_resp!(middle.call(&mut req)); + let r: R = ::json(&mut response); + assert_eq!(r.users.len(), 1); + + // Deleting yourself when you're the only owner isn't allowed. + let body = r#"{"users":["firstowner"]}"#; + let mut response = ok_resp!(middle.call(req.with_method(Method::Delete).with_body( + body.as_bytes(), + ))); + let json = ::json::<::Bad>(&mut response); + assert!(json.errors[0].detail.contains( + "cannot remove the sole owner of a crate", + )); + + let body = r#"{"users":["secondowner"]}"#; + let mut response = ok_resp!(middle.call(req.with_method(Method::Put).with_body( + body.as_bytes(), + ))); + assert!(::json::(&mut response).ok); + + // Deleting yourself when there are other owners is allowed. + let body = r#"{"users":["firstowner"]}"#; + let mut response = ok_resp!(middle.call(req.with_method(Method::Delete).with_body( + body.as_bytes(), + ))); + assert!(::json::(&mut response).ok); + + // After you delete yourself, you no longer have permisions to manage the crate. + let body = r#"{"users":["secondowner"]}"#; + let mut response = ok_resp!(middle.call(req.with_method(Method::Delete).with_body( + body.as_bytes(), + ))); + let json = ::json::<::Bad>(&mut response); + assert!(json.errors[0].detail.contains( + "only owners have permission to modify owners", + )); +} + +#[test] +fn owners() { + #[derive(Deserialize)] + struct R { + users: Vec, + } + #[derive(Deserialize)] + struct O { + ok: bool, + } + + let (_b, app, middle) = ::app(); + let mut req = ::req(app.clone(), Method::Get, "/api/v1/crates/foo_owners/owners"); + { + let conn = app.diesel_database.get().unwrap(); + ::new_user("foobar").create_or_update(&conn).unwrap(); + let user = ::new_user("foo").create_or_update(&conn).unwrap(); + ::sign_in_as(&mut req, &user); + ::CrateBuilder::new("foo_owners", user.id).expect_build(&conn); + } + + let mut response = ok_resp!(middle.call(&mut req)); + let r: R = ::json(&mut response); + assert_eq!(r.users.len(), 1); + + let mut response = ok_resp!(middle.call(req.with_method(Method::Get))); + let r: R = ::json(&mut response); + assert_eq!(r.users.len(), 1); + + let body = r#"{"users":["foobar"]}"#; + let mut response = ok_resp!(middle.call(req.with_method(Method::Put).with_body( + body.as_bytes(), + ))); + assert!(::json::(&mut response).ok); + + let mut response = ok_resp!(middle.call(req.with_method(Method::Get))); + let r: R = ::json(&mut response); + assert_eq!(r.users.len(), 2); + + let body = r#"{"users":["foobar"]}"#; + let mut response = ok_resp!(middle.call(req.with_method(Method::Delete).with_body( + body.as_bytes(), + ))); + assert!(::json::(&mut response).ok); + + let mut response = ok_resp!(middle.call(req.with_method(Method::Get))); + let r: R = ::json(&mut response); + assert_eq!(r.users.len(), 1); + + let body = r#"{"users":["foo"]}"#; + let mut response = ok_resp!(middle.call(req.with_method(Method::Delete).with_body( + body.as_bytes(), + ))); + ::json::<::Bad>(&mut response); + + let body = r#"{"users":["foobar"]}"#; + let mut response = ok_resp!(middle.call(req.with_method(Method::Put).with_body( + body.as_bytes(), + ))); + assert!(::json::(&mut response).ok); +} + +/* Testing the crate ownership between two crates and one team. + Given two crates, one crate owned by both a team and a user, + one only owned by a user, check that the CrateList returned + for the user_id contains only the crates owned by that user, + and that the CrateList returned for the team_id contains + only crates owned by that team. +*/ +#[test] +fn check_ownership_two_crates() { + let (_b, app, middle) = ::app(); + + let (krate_owned_by_team, team) = { + let conn = app.diesel_database.get().unwrap(); + let u = ::new_user("user_foo").create_or_update(&conn).unwrap(); + let t = ::new_team("team_foo").create_or_update(&conn).unwrap(); + let krate = ::CrateBuilder::new("foo", u.id).expect_build(&conn); + ::add_team_to_crate(&t, &krate, &u, &conn).unwrap(); + (krate, t) + }; + + let (krate_not_owned_by_team, user) = { + let conn = app.diesel_database.get().unwrap(); + let u = ::new_user("user_bar").create_or_update(&conn).unwrap(); + (::CrateBuilder::new("bar", u.id).expect_build(&conn), u) + }; + + let mut req = ::req(app.clone(), Method::Get, "/api/v1/crates"); + + let query = format!("user_id={}", user.id); + let mut response = ok_resp!(middle.call(req.with_query(&query))); + let json: CrateList = ::json(&mut response); + + assert_eq!(json.crates[0].name, krate_not_owned_by_team.name); + assert_eq!(json.crates.len(), 1); + + let query = format!("team_id={}", team.id); + let mut response = ok_resp!(middle.call(req.with_query(&query))); + + let json: CrateList = ::json(&mut response); + assert_eq!(json.crates.len(), 1); + assert_eq!(json.crates[0].name, krate_owned_by_team.name); +} + +/* Given a crate owned by both a team and a user, check that the + JSON returned by the /owner_team route and /owner_user route + contains the correct kind of owner + + Note that in this case function new_team must take a team name + of form github:org_name:team_name as that is the format + EncodableOwner::encodable is expecting +*/ +#[test] +fn check_ownership_one_crate() { + let (_b, app, middle) = ::app(); + + let (team, user) = { + let conn = app.diesel_database.get().unwrap(); + let u = ::new_user("user_cat").create_or_update(&conn).unwrap(); + let t = ::new_team("github:test_org:team_sloth") + .create_or_update(&conn) + .unwrap(); + let krate = ::CrateBuilder::new("best_crate", u.id).expect_build(&conn); + ::add_team_to_crate(&t, &krate, &u, &conn).unwrap(); + (t, u) + }; + + let mut req = ::req( + app.clone(), + Method::Get, + "/api/v1/crates/best_crate/owner_team", + ); + let mut response = ok_resp!(middle.call(&mut req)); + let json: TeamResponse = ::json(&mut response); + + assert_eq!(json.teams[0].kind, "team"); + assert_eq!(json.teams[0].name, team.name); + + let mut req = ::req( + app.clone(), + Method::Get, + "/api/v1/crates/best_crate/owner_user", + ); + let mut response = ok_resp!(middle.call(&mut req)); + let json: UserResponse = ::json(&mut response); + + assert_eq!(json.users[0].kind, "user"); + assert_eq!(json.users[0].name, user.name); +} From b0985090c6ff53ce31ff3dce47ea850bf343adf0 Mon Sep 17 00:00:00 2001 From: "Carol (Nichols || Goulding)" Date: Fri, 1 Sep 2017 10:30:00 -0400 Subject: [PATCH 3/3] Implement listing of crate invitations Connects to #959. --- src/crate_owner_invitation.rs | 82 +++++++++++++++++++++++++++++++++++ src/lib.rs | 5 +++ src/tests/owners.rs | 76 ++++++++++++++++++++++++++++++++ 3 files changed, 163 insertions(+) create mode 100644 src/crate_owner_invitation.rs diff --git a/src/crate_owner_invitation.rs b/src/crate_owner_invitation.rs new file mode 100644 index 0000000000..9a47aa6066 --- /dev/null +++ b/src/crate_owner_invitation.rs @@ -0,0 +1,82 @@ +use conduit::{Request, Response}; +use diesel::prelude::*; +use time::Timespec; + +use db::RequestTransaction; +use schema::{crate_owner_invitations, users, crates}; +use user::RequestUser; +use util::errors::CargoResult; +use util::RequestUtils; + +/// The model representing a row in the `crate_owner_invitations` database table. +#[derive(Clone, Copy, Debug, PartialEq, Eq, Identifiable, Queryable)] +#[primary_key(invited_user_id, crate_id)] +pub struct CrateOwnerInvitation { + pub invited_user_id: i32, + pub invited_by_user_id: i32, + pub crate_id: i32, + pub created_at: Timespec, +} + +#[derive(Insertable, Clone, Copy, Debug)] +#[table_name = "crate_owner_invitations"] +pub struct NewCrateOwnerInvitation { + pub invited_user_id: i32, + pub invited_by_user_id: i32, + pub crate_id: i32, +} + +impl CrateOwnerInvitation { + pub fn invited_by_username(&self, conn: &PgConnection) -> String { + users::table + .find(self.invited_by_user_id) + .select(users::gh_login) + .first(&*conn) + .unwrap_or_else(|_| String::from("(unknown username)")) + } + + pub fn crate_name(&self, conn: &PgConnection) -> String { + crates::table + .find(self.crate_id) + .select(crates::name) + .first(&*conn) + .unwrap_or_else(|_| String::from("(unknown crate name)")) + } + + pub fn encodable(self, conn: &PgConnection) -> EncodableCrateOwnerInvitation { + EncodableCrateOwnerInvitation { + invited_by_username: self.invited_by_username(conn), + crate_name: self.crate_name(conn), + crate_id: self.crate_id, + created_at: ::encode_time(self.created_at), + } + } +} + +/// The serialization format for the `CrateOwnerInvitation` model. +#[derive(Deserialize, Serialize, Debug)] +pub struct EncodableCrateOwnerInvitation { + pub invited_by_username: String, + pub crate_name: String, + pub crate_id: i32, + pub created_at: String, +} + +/// Handles the `GET /me/crate_owner_invitations` route. +pub fn list(req: &mut Request) -> CargoResult { + let conn = &*req.db_conn()?; + let user_id = req.user()?.id; + + let invitations = crate_owner_invitations::table + .filter(crate_owner_invitations::invited_user_id.eq(user_id)) + .load::(&*conn)? + .into_iter() + .map(|i| i.encodable(conn)) + .collect(); + + #[derive(Serialize)] + struct R { + invitations: Vec, + } + Ok(req.json(&R { invitations })) +} diff --git a/src/lib.rs b/src/lib.rs index 139f27fc1c..642ce4f627 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -76,6 +76,7 @@ pub mod badge; pub mod categories; pub mod category; pub mod config; +pub mod crate_owner_invitation; pub mod db; pub mod dependency; pub mod dist; @@ -183,6 +184,10 @@ pub fn middleware(app: Arc) -> MiddlewareBuilder { api_router.get("/me/tokens", C(token::list)); api_router.post("/me/tokens", C(token::new)); api_router.delete("/me/tokens/:id", C(token::revoke)); + api_router.get( + "/me/crate_owner_invitations", + C(crate_owner_invitation::list), + ); api_router.get("/summary", C(krate::summary)); let api_router = Arc::new(R404(api_router)); diff --git a/src/tests/owners.rs b/src/tests/owners.rs index add8dd5b0a..d2a5d56adf 100644 --- a/src/tests/owners.rs +++ b/src/tests/owners.rs @@ -2,8 +2,13 @@ use {CrateList, GoodCrate}; use cargo_registry::owner::EncodableOwner; use cargo_registry::user::EncodablePublicUser; +use cargo_registry::crate_owner_invitation::{EncodableCrateOwnerInvitation, + NewCrateOwnerInvitation}; +use cargo_registry::schema::crate_owner_invitations; use conduit::{Handler, Method}; +use diesel; +use diesel::prelude::*; #[derive(Deserialize)] struct TeamResponse { @@ -291,3 +296,74 @@ fn check_ownership_one_crate() { assert_eq!(json.users[0].kind, "user"); assert_eq!(json.users[0].name, user.name); } + +#[test] +fn invitations_are_empty_by_default() { + #[derive(Deserialize)] + struct R { + invitations: Vec, + } + + let (_b, app, middle) = ::app(); + let mut req = ::req( + app.clone(), + Method::Get, + "/api/v1/me/crate_owner_invitations", + ); + + let user = { + let conn = app.diesel_database.get().unwrap(); + ::new_user("user_no_invites") + .create_or_update(&conn) + .unwrap() + }; + ::sign_in_as(&mut req, &user); + + let mut response = ok_resp!(middle.call(&mut req)); + let json: R = ::json(&mut response); + + assert_eq!(json.invitations.len(), 0); +} + +#[test] +fn invitations_list() { + #[derive(Deserialize)] + struct R { + invitations: Vec, + } + + let (_b, app, middle) = ::app(); + let mut req = ::req( + app.clone(), + Method::Get, + "/api/v1/me/crate_owner_invitations", + ); + let (krate, user) = { + let conn = app.diesel_database.get().unwrap(); + let owner = ::new_user("inviting_user").create_or_update(&conn).unwrap(); + let user = ::new_user("invited_user").create_or_update(&conn).unwrap(); + let krate = ::CrateBuilder::new("invited_crate", owner.id).expect_build(&conn); + + // This should be replaced by an actual call to the route that `owner --add` hits once + // that route creates an invitation. + let invitation = NewCrateOwnerInvitation { + invited_by_user_id: owner.id, + invited_user_id: user.id, + crate_id: krate.id, + }; + diesel::insert(&invitation) + .into(crate_owner_invitations::table) + .execute(&*conn) + .unwrap(); + (krate, user) + }; + ::sign_in_as(&mut req, &user); + + let mut response = ok_resp!(middle.call(&mut req)); + let json: R = ::json(&mut response); + + assert_eq!(json.invitations.len(), 1); + assert_eq!(json.invitations[0].invited_by_username, "inviting_user"); + assert_eq!(json.invitations[0].crate_name, "invited_crate"); + assert_eq!(json.invitations[0].crate_id, krate.id); +}