Skip to content

backend: Add user email preference storing #1901

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Nov 22, 2019
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ALTER TABLE crate_owners DROP COLUMN email_notifications;
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ALTER TABLE crate_owners ADD COLUMN email_notifications BOOLEAN NOT NULL DEFAULT TRUE;
1 change: 1 addition & 0 deletions src/controllers/crate_owner_invitation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ fn accept_invite(
owner_id: user_id,
created_by: pending_crate_owner.invited_by_user_id,
owner_kind: OwnerKind::User as i32,
email_notifications: true,
})
.on_conflict(crate_owners::table.primary_key())
.do_update()
Expand Down
85 changes: 80 additions & 5 deletions src/controllers/user/me.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
use std::collections::HashMap;

use crate::controllers::prelude::*;

use crate::controllers::helpers::*;
use crate::email;
use crate::util::bad_request;
use crate::util::errors::CargoError;

use crate::models::{Email, Follow, NewEmail, User, Version};
use crate::schema::{crates, emails, follows, users, versions};
use crate::views::{EncodableMe, EncodableVersion};
use crate::models::{CrateOwner, Email, Follow, NewEmail, OwnerKind, User, Version};
use crate::schema::{crate_owners, crates, emails, follows, users, versions};
use crate::views::{EncodableMe, EncodableVersion, OwnedCrate};

/// Handles the `GET /me` route.
pub fn me(req: &mut dyn Request) -> CargoResult<Response> {
Expand All @@ -22,11 +24,11 @@ pub fn me(req: &mut dyn Request) -> CargoResult<Response> {
// perhaps adding `req.mut_extensions().insert(user)` to the
// update_user route, however this somehow does not seem to work

let id = req.user()?.id;
let user_id = req.user()?.id;
let conn = req.db_conn()?;

let (user, verified, email, verification_sent) = users::table
.find(id)
.find(user_id)
.left_join(emails::table)
.select((
users::all_columns,
Expand All @@ -36,12 +38,28 @@ pub fn me(req: &mut dyn Request) -> CargoResult<Response> {
))
.first::<(User, Option<bool>, Option<String>, bool)>(&*conn)?;

let owned_crates = crate_owners::table
.inner_join(crates::table)
.filter(crate_owners::owner_id.eq(user_id))
.filter(crate_owners::owner_kind.eq(OwnerKind::User as i32))
.select((crates::id, crates::name, crate_owners::email_notifications))
.order(crates::name.asc())
.load(&*conn)?
.into_iter()
.map(|(id, name, email_notifications)| OwnedCrate {
id,
name,
email_notifications,
})
.collect();

let verified = verified.unwrap_or(false);
let verification_sent = verified || verification_sent;
let user = User { email, ..user };

Ok(req.json(&EncodableMe {
user: user.encodable_private(verified, verification_sent),
owned_crates,
}))
}

Expand Down Expand Up @@ -212,3 +230,60 @@ pub fn regenerate_token_and_send(req: &mut dyn Request) -> CargoResult<Response>
}
Ok(req.json(&R { ok: true }))
}

/// Handles `PUT /me/email_notifications` route
pub fn update_email_notifications(req: &mut dyn Request) -> CargoResult<Response> {
use self::crate_owners::dsl::*;
// use diesel::update;
use diesel::pg::upsert::excluded;

#[derive(Deserialize)]
struct CrateEmailNotifications {
id: i32,
email_notifications: bool,
}

let mut body = String::new();
req.body().read_to_string(&mut body)?;
let updates: HashMap<i32, bool> = serde_json::from_str::<Vec<CrateEmailNotifications>>(&body)
.map_err(|_| bad_request("invalid json request"))?
.iter()
.map(|c| (c.id, c.email_notifications))
.collect();

let user = req.user()?;
let conn = req.db_conn()?;

// Build inserts from existing crates beloning to the current user
let to_insert = CrateOwner::belonging_to(user)
.select((crate_id, owner_id, owner_kind, email_notifications))
.filter(owner_kind.eq(OwnerKind::User as i32))
.load(&*conn)?
.into_iter()
.map(
|(c_id, o_id, o_kind, e_notifications): (i32, i32, i32, bool)| {
(
crate_id.eq(c_id),
owner_id.eq(o_id),
owner_kind.eq(o_kind),
email_notifications
.eq(updates.get(&c_id).unwrap_or(&e_notifications).to_owned()),
)
},
)
.collect::<Vec<_>>();

// Upsert crate owners; this should only actually exectute updates
diesel::insert_into(crate_owners)
.values(&to_insert)
.on_conflict((crate_id, owner_id, owner_kind))
.do_update()
.set(email_notifications.eq(excluded(email_notifications)))
.execute(&*conn)?;

#[derive(Serialize)]
struct R {
ok: bool,
}
Ok(req.json(&R { ok: true }))
}
2 changes: 2 additions & 0 deletions src/models/krate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,7 @@ impl<'a> NewCrate<'a> {
owner_id: user_id,
created_by: user_id,
owner_kind: OwnerKind::User as i32,
email_notifications: true,
};
diesel::insert_into(crate_owners::table)
.values(&owner)
Expand Down Expand Up @@ -455,6 +456,7 @@ impl Crate {
owner_id: owner.id(),
created_by: req_user.id,
owner_kind: OwnerKind::Team as i32,
email_notifications: true,
})
.on_conflict(crate_owners::table.primary_key())
.do_update()
Expand Down
1 change: 1 addition & 0 deletions src/models/owner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ pub struct CrateOwner {
pub owner_id: i32,
pub created_by: i32,
pub owner_kind: i32,
pub email_notifications: bool,
}

#[derive(Debug, Clone, Copy)]
Expand Down
4 changes: 4 additions & 0 deletions src/router.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,10 @@ pub fn build_router(app: &App) -> R404 {
"/me/crate_owner_invitations/:crate_id",
C(crate_owner_invitation::handle_invite),
);
api_router.put(
"/me/email_notifications",
C(user::me::update_email_notifications),
);
api_router.get("/summary", C(krate::metadata::summary));
api_router.put("/confirm/:email_token", C(user::me::confirm_user_email));
api_router.put(
Expand Down
6 changes: 6 additions & 0 deletions src/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,12 @@ table! {
///
/// (Automatically generated by Diesel.)
owner_kind -> Int4,
/// The `email_notifications` column of the `crate_owners` table.
///
/// Its SQL type is `Bool`.
///
/// (Automatically generated by Diesel.)
email_notifications -> Bool,
}
}

Expand Down
1 change: 1 addition & 0 deletions src/tasks/dump_db/dump-db.toml
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ created_by = "private"
deleted = "private"
updated_at = "public"
owner_kind = "public"
email_notifications = "private"

[crates.columns]
id = "public"
Expand Down
1 change: 1 addition & 0 deletions src/tests/all.rs
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,7 @@ fn add_team_to_crate(t: &Team, krate: &Crate, u: &User, conn: &PgConnection) ->
owner_id: t.id,
created_by: u.id,
owner_kind: 1, // Team owner kind is 1 according to owner.rs
email_notifications: true,
};

diesel::insert_into(crate_owners::table)
Expand Down
141 changes: 140 additions & 1 deletion src/tests/user.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ use crate::{
};
use cargo_registry::{
models::{Email, NewUser, User},
views::{EncodablePrivateUser, EncodablePublicUser, EncodableVersion},
schema::crate_owners,
views::{EncodablePrivateUser, EncodablePublicUser, EncodableVersion, OwnedCrate},
};

use diesel::prelude::*;
Expand All @@ -25,13 +26,20 @@ pub struct UserShowPublicResponse {
#[derive(Deserialize)]
pub struct UserShowPrivateResponse {
pub user: EncodablePrivateUser,
pub owned_crates: Vec<OwnedCrate>,
}

#[derive(Deserialize)]
struct UserStats {
total_downloads: i64,
}

#[derive(Serialize)]
struct EmailNotificationsUpdate {
id: i32,
email_notifications: bool,
}

impl crate::util::MockCookieUser {
fn show_me(&self) -> UserShowPrivateResponse {
let url = "/api/v1/me";
Expand Down Expand Up @@ -65,6 +73,14 @@ impl crate::util::MockCookieUser {
let url = format!("/api/v1/confirm/{}", email_token);
self.put(&url, &[]).good()
}

fn update_email_notifications(&self, updates: Vec<EmailNotificationsUpdate>) -> OkBool {
self.put(
"/api/v1/me/email_notifications",
json!(updates).to_string().as_bytes(),
)
.good()
}
}

impl crate::util::MockAnonymousUser {
Expand Down Expand Up @@ -108,9 +124,14 @@ fn me() {
anon.get(url).assert_forbidden();

let user = app.db_new_user("foo");
app.db(|conn| {
CrateBuilder::new("foo_my_packages", user.as_model().id).expect_build(conn);
});

let json = user.show_me();

assert_eq!(json.user.email, user.as_model().email);
assert_eq!(json.owned_crates.len(), 1);
}

#[test]
Expand Down Expand Up @@ -569,3 +590,121 @@ fn test_existing_user_email() {
assert!(!json.user.email_verified);
assert!(!json.user.email_verification_sent);
}

/* A user should be able to update the email notifications for crates they own. Only the crates that
were sent in the request should be updated to the corresponding `email_notifications` value.
*/
#[test]
fn test_update_email_notifications() {
let (app, _, user) = TestApp::init().with_user();

let my_crates = app.db(|conn| {
vec![
CrateBuilder::new("test_package", user.as_model().id).expect_build(&conn),
CrateBuilder::new("another_package", user.as_model().id).expect_build(&conn),
]
});

let a_id = my_crates.get(0).unwrap().id;
let b_id = my_crates.get(1).unwrap().id;

// Update crate_a: email_notifications = false
// crate_a should be false, crate_b should be true
user.update_email_notifications(vec![EmailNotificationsUpdate {
id: a_id,
email_notifications: false,
}]);
let json = user.show_me();

assert_eq!(
json.owned_crates
.iter()
.find(|c| c.id == a_id)
.unwrap()
.email_notifications,
false
);
assert_eq!(
json.owned_crates
.iter()
.find(|c| c.id == b_id)
.unwrap()
.email_notifications,
true
);

// Update crate_b: email_notifications = false
// Both should be false now
user.update_email_notifications(vec![EmailNotificationsUpdate {
id: b_id,
email_notifications: false,
}]);
let json = user.show_me();

assert_eq!(
json.owned_crates
.iter()
.find(|c| c.id == a_id)
.unwrap()
.email_notifications,
false
);
assert_eq!(
json.owned_crates
.iter()
.find(|c| c.id == b_id)
.unwrap()
.email_notifications,
false
);

// Update crate_a and crate_b: email_notifications = true
// Both should be true
user.update_email_notifications(vec![
EmailNotificationsUpdate {
id: a_id,
email_notifications: true,
},
EmailNotificationsUpdate {
id: b_id,
email_notifications: true,
},
]);
let json = user.show_me();

json.owned_crates.iter().for_each(|c| {
assert!(c.email_notifications);
})
}

/* A user should not be able to update the `email_notifications` value for a crate that is not
owned by them.
*/
#[test]
fn test_update_email_notifications_not_owned() {
let (app, _, user) = TestApp::init().with_user();

let not_my_crate = app.db(|conn| {
let u = new_user("arbitrary_username")
.create_or_update(&conn)
.unwrap();
CrateBuilder::new("test_package", u.id).expect_build(&conn)
});

user.update_email_notifications(vec![EmailNotificationsUpdate {
id: not_my_crate.id,
email_notifications: false,
}]);

let email_notifications = app
.db(|conn| {
crate_owners::table
.select(crate_owners::email_notifications)
.filter(crate_owners::crate_id.eq(not_my_crate.id))
.first::<bool>(&*conn)
})
.unwrap();

// There should be no change to the `email_notifications` value for a crate not belonging to me
assert!(email_notifications);
}
8 changes: 8 additions & 0 deletions src/views.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,9 +149,17 @@ pub struct EncodableApiTokenWithToken {
pub last_used_at: Option<NaiveDateTime>,
}

#[derive(Deserialize, Serialize, Debug)]
pub struct OwnedCrate {
pub id: i32,
pub name: String,
pub email_notifications: bool,
}

#[derive(Serialize, Deserialize, Debug)]
pub struct EncodableMe {
pub user: EncodablePrivateUser,
pub owned_crates: Vec<OwnedCrate>,
}

/// The serialization format for the `User` model.
Expand Down