Merge #1069

1069: Email polish r=carols10cents

Just cleaning up a few small things before deploying email verification :)

Author: bors-voyager[bot]

app/components/email-input.js

@@ -26,6 +26,16 @@ export default Component.extend({
     }),
     isError: false,
     emailError: '',
+    disableResend: false,
+    resendButtonText: computed('disableResend', 'user.email_verification_sent', function() {
+        if (this.get('disableResend')) {
+            return 'Sent!';
+        } else if (this.get('user.email_verification_sent')) {
+            return 'Resend';
+        } else {
+            return 'Send verification email';
+        }
+    }),
 
     actions: {
         editEmail() {
@@ -55,7 +65,11 @@ export default Component.extend({
 
             user.set('email', userEmail);
             user.save()
-                .then(() => this.set('serverError', null))
+                .then(() => {
+                    this.set('serverError', null);
+                    this.set('user.email_verification_sent', true);
+                    this.set('user.email_verified', false);
+                })
                 .catch(err => {
                     let msg;
                     if (err.errors && err.errors[0] && err.errors[0].detail) {
@@ -70,6 +84,7 @@ export default Component.extend({
 
             this.set('isEditing', false);
             this.set('notValidEmail', false);
+            this.set('disableResend', false);
         },
 
         cancelEdit() {
@@ -80,25 +95,19 @@ export default Component.extend({
         resendEmail() {
             let user = this.get('user');
 
-            this.get('ajax').raw(`/api/v1/users/${user.id}/resend`, { method: 'PUT',
-                user: {
-                    avatar: user.avatar,
-                    email: user.email,
-                    email_verified: user.email_verified,
-                    kind: user.kind,
-                    login: user.login,
-                    name: user.name,
-                    url: user.url
-                }
-            }).catch((error) => {
-                if (error.payload) {
-                    this.set('isError', true);
-                    this.set('emailError', `Error in resending message: ${error.payload.errors[0].detail}`);
-                } else {
-                    this.set('isError', true);
-                    this.set('emailError', 'Unknown error in resending message');
-                }
-            });
+            this.get('ajax').raw(`/api/v1/users/${user.id}/resend`, {
+                method: 'PUT'
+            })
+                .then(() => this.set('disableResend', true))
+                .catch((error) => {
+                    if (error.payload) {
+                        this.set('isError', true);
+                        this.set('emailError', `Error in resending message: ${error.payload.errors[0].detail}`);
+                    } else {
+                        this.set('isError', true);
+                        this.set('emailError', 'Unknown error in resending message');
+                    }
+                });
         }
     }
 });

app/models/user.js

@@ -3,6 +3,7 @@ import DS from 'ember-data';
 export default DS.Model.extend({
     email: DS.attr('string'),
     email_verified: DS.attr('boolean'),
+    email_verification_sent: DS.attr('boolean'),
     name: DS.attr('string'),
     login: DS.attr('string'),
     avatar: DS.attr('string'),

app/styles/me.scss

@@ -86,14 +86,24 @@
         }
         .email {
             @include flex(20);
+            .verified {
+                color: green;
+                font-weight: bold;
+            }
         }
         .actions {
             @include display-flex;
             @include align-items(center);
             img { margin-left: 10px }
         }
         .email-form {
+            @include flex(10);
             display: inline-flex;
+            justify-content: space-between;
+            flex-wrap: wrap;
+            input.ember-text-field {
+                width: 400px;
+            }
         }
         .space-right {
             margin-right: 10px;

app/templates/components/email-input.hbs

@@ -30,7 +30,12 @@
             <dt>Email</dt>
         </div>
         <div class='email'>
-            <dd class='no-space-left'>{{ user.email }}</dd>
+            <dd class='no-space-left'>
+                {{ user.email }}
+                {{#if user.email_verified}}
+                    <span class='verified'>Verified!</span>
+                {{/if}}
+            </dd>
         </div>
         <div class='actions'>
             <button class='small yellow-button space-left' {{action 'editEmail'}}>Edit</button>
@@ -39,10 +44,13 @@
     {{#if emailNotVerified }}
         <div class='row'>
             <div class='label'>
+                {{#if user.email_verification_sent}}
+                    <p class='small-text'>We have sent a verification email to your address.</p>
+                {{/if}}
                 <p class='small-text'>Your email has not yet been verified.</p>
             </div>
             <div class='actions'>
-                <button class='small yellow-button space-left' {{action 'resendEmail'}}>Resend</button>
+                <button class='small yellow-button space-left' {{action 'resendEmail'}} disabled={{disableResend}}>{{resendButtonText}}</button>
             </div>
         </div>
     {{/if}}

src/tests/user.rs

@@ -4,7 +4,8 @@ use conduit::{Handler, Method};
 
 use cargo_registry::token::ApiToken;
 use cargo_registry::krate::EncodableCrate;
-use cargo_registry::user::{User, NewUser, EncodablePrivateUser, EncodablePublicUser, Email, Token};
+use cargo_registry::user::{User, NewUser, EncodablePrivateUser, EncodablePublicUser, Email,
+                           NewEmail, Token};
 use cargo_registry::version::EncodableVersion;
 
 use diesel::prelude::*;
@@ -417,6 +418,8 @@ fn test_email_get_and_put() {
     let r = ::json::<R>(&mut response);
     assert_eq!(r.user.email.unwrap(), "[email protected]");
     assert_eq!(r.user.login, "mango");
+    assert!(!r.user.email_verified);
+    assert!(r.user.email_verification_sent);
 }
 
 /*  Given a crates.io user, check to make sure that the user
@@ -607,6 +610,8 @@ fn test_insert_into_email_table_with_email_change() {
     let r = ::json::<R>(&mut response);
     assert_eq!(r.user.email.unwrap(), "[email protected]");
     assert_eq!(r.user.login, "potato");
+    assert!(!r.user.email_verified);
+    assert!(r.user.email_verification_sent);
 
     let body = r#"{"user":{"email":"[email protected]","name":"potato","login":"potato","avatar":"https://avatars0.githubusercontent.com","url":"https://github.com/potato","kind":null}}"#;
     let mut response = ok_resp!(
@@ -638,6 +643,8 @@ fn test_insert_into_email_table_with_email_change() {
     ));
     let r = ::json::<R>(&mut response);
     assert_eq!(r.user.email.unwrap(), "[email protected]");
+    assert!(!r.user.email_verified);
+    assert!(r.user.email_verification_sent);
     assert_eq!(r.user.login, "potato");
 }
 
@@ -703,4 +710,55 @@ fn test_confirm_user_email() {
     assert_eq!(r.user.email.unwrap(), "[email protected]");
     assert_eq!(r.user.login, "potato");
     assert!(r.user.email_verified);
+    assert!(r.user.email_verification_sent);
+}
+
+/* Given a user who existed before we added email confirmation,
+   test that `email_verification_sent` is false so that we don't
+   make the user think we've sent an email when we haven't.
+*/
+#[test]
+fn test_existing_user_email() {
+    use cargo_registry::schema::{emails, users};
+    use diesel::insert;
+
+    #[derive(Deserialize)]
+    struct R {
+        user: EncodablePrivateUser,
+    }
+
+    let (_b, app, middle) = ::app();
+    let mut req = ::req(app.clone(), Method::Get, "/me");
+    {
+        let conn = app.diesel_database.get().unwrap();
+        let user = ::new_user("potahto");
+
+        // Deliberately not using User::create_or_update since that
+        // will try to send a verification email; we want to simulate
+        // a user who already had an email before we added verification.
+        let user = insert(&user)
+            .into(users::table)
+            .get_result::<User>(&*conn)
+            .unwrap();
+
+        println!("{:?}", user);
+
+        let email = NewEmail {
+            user_id: user.id,
+            email: String::from("[email protected]"),
+            verified: false,
+        };
+
+        insert(&email).into(emails::table).execute(&*conn).unwrap();
+
+        ::sign_in_as(&mut req, &user);
+    }
+
+    let mut response = ok_resp!(middle.call(
+        req.with_path("/api/v1/me").with_method(Method::Get),
+    ));
+    let r = ::json::<R>(&mut response);
+    assert_eq!(r.user.email.unwrap(), "[email protected]");
+    assert!(!r.user.email_verified);
+    assert!(!r.user.email_verification_sent);
 }

src/user/mod.rs

@@ -207,6 +207,7 @@ pub struct EncodablePrivateUser {
     pub login: String,
     pub email: Option<String>,
     pub email_verified: bool,
+    pub email_verification_sent: bool,
     pub name: Option<String>,
     pub avatar: Option<String>,
     pub url: Option<String>,
@@ -240,7 +241,11 @@ impl User {
     }
 
     /// Converts this `User` model into an `EncodablePrivateUser` for JSON serialization.
-    pub fn encodable_private(self, email_verified: bool) -> EncodablePrivateUser {
+    pub fn encodable_private(
+        self,
+        email_verified: bool,
+        email_verification_sent: bool,
+    ) -> EncodablePrivateUser {
         let User {
             id,
             email,
@@ -254,6 +259,7 @@ impl User {
             id: id,
             email: email,
             email_verified,
+            email_verification_sent,
             avatar: gh_avatar,
             login: gh_login,
             name: name,
@@ -415,16 +421,31 @@ pub fn me(req: &mut Request) -> CargoResult<Response> {
 
     use self::users::dsl::{users, id};
     use self::emails::dsl::{emails, user_id};
+    use diesel::select;
+    use diesel::expression::dsl::exists;
 
     let u_id = req.user()?.id;
     let conn = req.db_conn()?;
 
     let user_info = users.filter(id.eq(u_id)).first::<User>(&*conn)?;
     let email_result = emails.filter(user_id.eq(u_id)).first::<Email>(&*conn);
 
-    let (email, verified): (Option<String>, bool) = match email_result {
-        Ok(response) => (Some(response.email), response.verified),
-        Err(_) => (None, false),
+    let (email, verified, verification_sent): (Option<String>, bool, bool) = match email_result {
+        Ok(email_record) => {
+            let verification_sent = if email_record.verified {
+                true
+            } else {
+                select(exists(Token::belonging_to(&email_record)))
+                    .get_result(&*conn)
+                    .unwrap_or(false)
+            };
+            (
+                Some(email_record.email),
+                email_record.verified,
+                verification_sent,
+            )
+        }
+        Err(_) => (None, false, false),
     };
 
     let user = User {
@@ -436,7 +457,9 @@ pub fn me(req: &mut Request) -> CargoResult<Response> {
     struct R {
         user: EncodablePrivateUser,
     }
-    Ok(req.json(&R { user: user.encodable_private(verified) }))
+    Ok(req.json(&R {
+        user: user.encodable_private(verified, verification_sent),
+    }))
 }
 
 /// Handles the `GET /users/:user_id` route.
@@ -692,8 +715,6 @@ pub fn regenerate_token_and_send(req: &mut Request) -> CargoResult<Response> {
     use time;
     use self::tokens::dsl::email_id;
 
-    let mut body = String::new();
-    req.body().read_to_string(&mut body)?;
     let user = req.user()?;
     let name = &req.params()["user_id"].parse::<i32>().ok().unwrap();
     let conn = req.db_conn()?;