GitHub org/team names should be treated case insensitively (fixes #1167)

[cgati]

I believe this fixes the issue. If there are areas that I missed, I will be sure to update them as well!

Example usage output:

# cargo owner -a github:test-cratesio:dev --index file:///development/crates.io/tmp/index-co
    Updating registry `file:///development/crates.io/tmp/index-co`
       Owner team github:test-cratesio:dev has been added as an owner of crate test-crate

# cargo owner -a github:Test-Cratesio:Dev --index file:///development/crates.io/tmp/index-co
    Updating registry `file:///development/crates.io/tmp/index-co`
error: failed to invite owners to crate test-crate: api errors: `github:Test-Cratesio:Dev` is already an owner

# cargo owner -r github:Test-Cratesio:Dev --index file:///development/crates.io/tmp/index-co
    Updating registry `file:///development/crates.io/tmp/index-co`
       Owner removing ["github:Test-Cratesio:Dev"] from crate test-crate

# cargo owner -a github:Test-Cratesio:Dev --index file:///development/crates.io/tmp/index-co
    Updating registry `file:///development/crates.io/tmp/index-co`
       Owner team github:test-cratesio:dev has been added as an owner of crate test-crate

[carols10cents]

This is working great! Tested out adding and removing teams as owners with different cases and it all worked as I expected ❤️

I also pushed a commit that I think should make clippy happy, that's a new lint I haven't run into yet!

I would like to see a bit more coverage in the test though-- let me know if you're up for making this change, I'm also happy to take care of it!

The current test is checking that a new team can be added as an owner when the team name has uppercase letters in it. A few more things I'd like to see the test assert on:

• When asked for the crate's owners, the team name should be in all lowercase (to be sure we're converting all names to lowercase)
• If we then remove that team as an owner of the crate using different casing than both what the team name was added with and all lowercase, check that we remove the team as an owner successfully (to be sure we're converting user input to lowercase before comparing it to what we have stored)

This can be added to the test you've added. Let me know what you think!

[cgati]

Thank you for appeasing clippy, I was a bit stumped there for a bit. I'd be happy to make the testing changes as requested, I'll take a look at it soon!

[cgati]

I've begun work on adding the additional tests. The check for team ownership casing works great, however I run into issues with removing the team as an owner of a crate. Making the api call causes an error in the expected response from GitHub. Just commenting to let you know I've not forgotten about this, and am working on it.

[cgati]

I just wanted to get what I did have working in, which is just the check for if the team name is lowercase when asked for. I still have not been able to figure out what to do in the http-data file to get the team removal tests to pass.

What I've written is the following:

    ok_resp!(
        middle.call(
            req.with_path("/api/v1/crates/foo_mixed_case/owners")
                .with_method(Method::Delete)
                .with_body(body.as_bytes()),
        )
    );

    {
        let conn = app.diesel_database.get().unwrap();
        let krate = Crate::by_name("foo_mixed_case")
            .first::<Crate>(&*conn)
            .unwrap();
        assert_eq!(krate.owners(&*conn).unwrap().len(), 1);
    }
}

And I've tried a few different things in http-data to get this block to pass and I've been unsuccessful so far. I find it likely that I may just not fully understand what is needed for this to work at this time.

Edit: The sort of errors I am encountering are:

thread 'team::add_team_mixed_case' panicked at 'middle.call(req.with_path("/api/v1/crates/foo_mixed_case/owners").with_method(Method::Delete).with_body(body.as_bytes())) failed with: github didn't send a valid json response caused by missing field `state` at line 1 column 1168

[klnusbaum]

Hey folks. This PR seems stalled out. I'm happy to pick it up and help out. But I'm not sure what the proper etiquette is here. If I can help, what's the recommended way to do so? (create my own branch? try to keep working off of this one?)

[sgrif]

@klnusbaum Right now it just needs a rebase. If you (or @cgati) want to do so and ping me, I will review

[cgati]

@sgrif I went ahead and performed the rebase.

[sgrif]

bors: r+

[sgrif]

Thanks @cgati <3

[bors-voyager]

Build succeeded

• continuous-integration/travis-ci/push