Skip to content

Make owner invites case-insensitive #2194

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 3 commits into from
Dec 6, 2020
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion src/controllers/user/other.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use crate::views::EncodablePublicUser;
pub fn show(req: &mut dyn RequestExt) -> EndpointResult {
use self::users::dsl::{gh_login, id, users};

let name = &req.params()["user_id"].to_lowercase();
let name = crate::lower(&req.params()["user_id"]);
let conn = req.db_conn()?;
let user: User = users
.filter(crate::lower(gh_login).eq(name))
Expand Down
2 changes: 1 addition & 1 deletion src/models/owner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ impl Owner {
)?))
} else {
users::table
.filter(users::gh_login.eq(name))
.filter(crate::lower(users::gh_login).eq(name.to_lowercase()))
.filter(users::gh_id.ne(-1))
.order(users::gh_id.desc())
.first(conn)
Expand Down
20 changes: 10 additions & 10 deletions src/tests/owners.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,9 @@ fn new_crate_owner() {
let crate_to_publish = PublishBuilder::new("foo_owner").version("1.0.0");
token.enqueue_publish(crate_to_publish).good();

// Add the second user as an owner
let user2 = app.db_new_user("bar");
token.add_user_owner("foo_owner", user2.as_model());
// Add the second user as an owner (with a different case to make sure that works)
let user2 = app.db_new_user("Bar");
token.add_user_owner("foo_owner", "BAR");

// accept invitation for user to be added as owner
let krate: Crate = app.db(|conn| Crate::by_name("foo_owner").first(conn).unwrap());
Expand All @@ -104,7 +104,7 @@ fn new_crate_owner() {
let crates = user2.search_by_user_id(user2.as_model().id);
assert_eq!(crates.crates.len(), 1);

// And upload a new crate as the second user
// And upload a new version as the second user
let crate_to_publish = PublishBuilder::new("foo_owner").version("2.0.0");
user2
.db_new_token("bar_token")
Expand All @@ -119,7 +119,7 @@ fn create_and_add_owner(
krate: &Crate,
) -> MockCookieUser {
let user = app.db_new_user(username);
token.add_user_owner(&krate.name, user.as_model());
token.add_user_owner(&krate.name, username);
user.accept_ownership_invitation(&krate.name, krate.id);
user
}
Expand Down Expand Up @@ -301,7 +301,7 @@ fn invitations_list() {
let krate = app.db(|conn| CrateBuilder::new("invited_crate", owner.id).expect_build(conn));

let user = app.db_new_user("invited_user");
token.add_user_owner("invited_crate", user.as_model());
token.add_user_owner("invited_crate", "invited_user");

let json = user.list_invitations();
assert_eq!(json.crate_owner_invitations.len(), 1);
Expand All @@ -327,7 +327,7 @@ fn test_accept_invitation() {
let krate = app.db(|conn| CrateBuilder::new("accept_invitation", owner.id).expect_build(conn));

// Invite a new owner
owner_token.add_user_owner("accept_invitation", invited_user.as_model());
owner_token.add_user_owner("accept_invitation", "user_bar");

// New owner accepts the invitation
invited_user.accept_ownership_invitation(&krate.name, krate.id);
Expand All @@ -354,7 +354,7 @@ fn test_decline_invitation() {
let krate = app.db(|conn| CrateBuilder::new("decline_invitation", owner.id).expect_build(conn));

// Invite a new owner
owner_token.add_user_owner("decline_invitation", invited_user.as_model());
owner_token.add_user_owner("decline_invitation", "user_bar");

// Invited user declines the invitation
invited_user.decline_ownership_invitation(&krate.name, krate.id);
Expand Down Expand Up @@ -395,7 +395,7 @@ fn inactive_users_dont_get_invitations() {

let invited_user = app.db_new_user(invited_gh_login);

owner_token.add_user_owner(krate_name, invited_user.as_model());
owner_token.add_user_owner(krate_name, "user_bar");

let json = invited_user.list_invitations();
assert_eq!(json.crate_owner_invitations.len(), 1);
Expand All @@ -419,7 +419,7 @@ fn highest_gh_id_is_most_recent_account_we_know_of() {
CrateBuilder::new(krate_name, owner.id).expect_build(conn);
});

owner_token.add_user_owner(krate_name, invited_user.as_model());
owner_token.add_user_owner(krate_name, "user_bar");

let json = invited_user.list_invitations();
assert_eq!(json.crate_owner_invitations.len(), 1);
Expand Down
8 changes: 4 additions & 4 deletions src/tests/user.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,14 +142,14 @@ fn me() {
#[test]
fn show() {
let (app, anon, _) = TestApp::init().with_user();
app.db_new_user("bar");
app.db_new_user("Bar");

let json: UserShowPublicResponse = anon.get("/api/v1/users/foo").good();
assert_eq!("foo", json.user.login);

let json: UserShowPublicResponse = anon.get("/api/v1/users/bar").good();
assert_eq!("bar", json.user.login);
assert_eq!(Some("https://github.com/bar".into()), json.user.url);
let json: UserShowPublicResponse = anon.get("/api/v1/users/bAr").good();
assert_eq!("Bar", json.user.login);
assert_eq!(Some("https://github.com/Bar".into()), json.user.url);
}

#[test]
Expand Down
4 changes: 2 additions & 2 deletions src/tests/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -543,8 +543,8 @@ impl MockTokenUser {
}

/// Add a user as an owner for a crate.
pub fn add_user_owner(&self, krate_name: &str, user: &User) {
self.add_named_owner(krate_name, &user.gh_login).good();
pub fn add_user_owner(&self, krate_name: &str, username: &str) {
self.add_named_owner(krate_name, username).good();
}
}

Expand Down