Skip to content

Commit 3816fba

Browse files
committed
Support optional fields in update_user() endpoint
Currently only the `email` field is supported, but that is about to change. Unfortunately with `serde_json` it is hard to differentiate between `email: null` and the `email` field not existing, since both are decoded as `Option::None`. Since our frontend does not send `email: null` and the endpoint does not appear to be used via token authentication this seems like a viable change. Side note: This endpoint is behaving like a `PATCH` endpoint, but is currently using the `PUT` HTTP method for legacy reasons.
1 parent 1df883d commit 3816fba

File tree

2 files changed

+59
-43
lines changed

2 files changed

+59
-43
lines changed

src/controllers/user/update.rs

Lines changed: 36 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -44,45 +44,44 @@ pub async fn update_user(
4444
return Err(bad_request("current user does not match requested user"));
4545
}
4646

47-
let user_email = match &user_update.user.email {
48-
Some(email) => email.trim(),
49-
None => return Err(bad_request("empty email rejected")),
50-
};
47+
if let Some(user_email) = &user_update.user.email {
48+
let user_email = user_email.trim();
5149

52-
if user_email.is_empty() {
53-
return Err(bad_request("empty email rejected"));
54-
}
50+
if user_email.is_empty() {
51+
return Err(bad_request("empty email rejected"));
52+
}
53+
54+
user_email
55+
.parse::<Address>()
56+
.map_err(|_| bad_request("invalid email address"))?;
57+
58+
let new_email = NewEmail {
59+
user_id: user.id,
60+
email: user_email,
61+
};
62+
63+
let token = diesel::insert_into(emails::table)
64+
.values(&new_email)
65+
.on_conflict(emails::user_id)
66+
.do_update()
67+
.set(&new_email)
68+
.returning(emails::token)
69+
.get_result(conn)
70+
.map(SecretString::new)
71+
.map_err(|_| server_error("Error in creating token"))?;
72+
73+
// This swallows any errors that occur while attempting to send the email. Some users have
74+
// an invalid email set in their GitHub profile, and we should let them sign in even though
75+
// we're trying to silently use their invalid address during signup and can't send them an
76+
// email. They'll then have to provide a valid email address.
77+
let email = UserConfirmEmail {
78+
user_name: &user.gh_login,
79+
domain: &state.emails.domain,
80+
token,
81+
};
5582

56-
user_email
57-
.parse::<Address>()
58-
.map_err(|_| bad_request("invalid email address"))?;
59-
60-
let new_email = NewEmail {
61-
user_id: user.id,
62-
email: user_email,
63-
};
64-
65-
let token = diesel::insert_into(emails::table)
66-
.values(&new_email)
67-
.on_conflict(emails::user_id)
68-
.do_update()
69-
.set(&new_email)
70-
.returning(emails::token)
71-
.get_result(conn)
72-
.map(SecretString::new)
73-
.map_err(|_| server_error("Error in creating token"))?;
74-
75-
// This swallows any errors that occur while attempting to send the email. Some users have
76-
// an invalid email set in their GitHub profile, and we should let them sign in even though
77-
// we're trying to silently use their invalid address during signup and can't send them an
78-
// email. They'll then have to provide a valid email address.
79-
let email = UserConfirmEmail {
80-
user_name: &user.gh_login,
81-
domain: &state.emails.domain,
82-
token,
83-
};
84-
85-
let _ = state.emails.send(user_email, email);
83+
let _ = state.emails.send(user_email, email);
84+
}
8685

8786
ok_true()
8887
})

src/tests/routes/users/update.rs

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -45,13 +45,30 @@ async fn test_empty_email_not_added() {
4545
response.json(),
4646
json!({ "errors": [{ "detail": "empty email rejected" }] })
4747
);
48+
}
4849

49-
let response = user.update_email_more_control(model.id, None).await;
50-
assert_eq!(response.status(), StatusCode::BAD_REQUEST);
51-
assert_eq!(
52-
response.json(),
53-
json!({ "errors": [{ "detail": "empty email rejected" }] })
54-
);
50+
#[tokio::test(flavor = "multi_thread")]
51+
async fn test_ignore_empty() {
52+
let (_app, _anon, user) = TestApp::init().with_user();
53+
let model = user.as_model();
54+
55+
let url = format!("/api/v1/users/{}", model.id);
56+
let payload = json!({"user": {}});
57+
let response = user.put::<()>(&url, payload.to_string()).await;
58+
assert_eq!(response.status(), StatusCode::OK);
59+
assert_snapshot!(response.text(), @r###"{"ok":true}"###);
60+
}
61+
62+
#[tokio::test(flavor = "multi_thread")]
63+
async fn test_ignore_nulls() {
64+
let (_app, _anon, user) = TestApp::init().with_user();
65+
let model = user.as_model();
66+
67+
let url = format!("/api/v1/users/{}", model.id);
68+
let payload = json!({"user": { "email": null }});
69+
let response = user.put::<()>(&url, payload.to_string()).await;
70+
assert_eq!(response.status(), StatusCode::OK);
71+
assert_snapshot!(response.text(), @r###"{"ok":true}"###);
5572
}
5673

5774
/// Check to make sure that neither other signed in users nor anonymous users can edit another

0 commit comments

Comments
 (0)