Team page #790
Team page #790
Conversation
…conflicting id problem; owner, owner_team and owner_user are currently all rendered on the page for comparison purposes fixes rust-lang#765
…n user page due to recently resolved github link issue
src/krate.rs
Outdated
| @@ -598,6 +602,32 @@ impl Crate { | |||
| Ok(users.chain(teams).collect()) | |||
| } | |||
|
|
|||
| pub fn owner_team(&self, conn: &PgConnection) -> CargoResult<Vec<Owner>> { | |||
There was a problem hiding this comment.
Crate has a lot of behavior on it as is. What do you think about putting this on Team instead and calling it Team::owning(crate: &Crate, conn: &PgConnection)
src/krate.rs
Outdated
| Ok(teams.collect()) | ||
| } | ||
|
|
||
| pub fn owner_user(&self, conn: &PgConnection) -> CargoResult<Vec<Owner>> { |
There was a problem hiding this comment.
Crate has a lot of behavior on it as is. What do you think about putting this on User instead and calling it User::owning(crate: &Crate, conn: &PgConnection)
| @@ -864,6 +894,15 @@ pub fn index(req: &mut Request) -> CargoResult<Response> { | |||
| .filter(crate_owners::owner_kind.eq(OwnerKind::User as i32)), | |||
| ), | |||
| ); | |||
| } else if let Some(team_id) = params.get("team_id").and_then(|s| s.parse::<i32>().ok()) { | |||
There was a problem hiding this comment.
Is there a reason to put this on index instead of adding a new route?
There was a problem hiding this comment.
For consistency with how getting a user's crates works, mostly. If we're going to pull this out into its own route, then I think users should be pulled out too, which imo should be a separate PR. WDYT?
There was a problem hiding this comment.
I think that users should be pulled out as well, but I don't think that should block this PR being a separate route, if it's not significantly more work for this PR.
| @@ -1423,6 +1462,42 @@ pub fn owners(req: &mut Request) -> CargoResult<Response> { | |||
| Ok(req.json(&R { users: owners })) | |||
| } | |||
|
|
|||
| /// Handles the `GET /crates/:crate_id/owner_team` route. | |||
| pub fn owner_team(req: &mut Request) -> CargoResult<Response> { | |||
There was a problem hiding this comment.
What do you think about making this a single route and adding a ?kind parameter if needed?
There was a problem hiding this comment.
We tried, but the structure of the response has user: vs team: change that ember cares about, and I suck at ember and I'm not sure why it cares about that. If we duplicate the response structure too, then it's most of the function that's still duplicated :-/
I wouldn't be opposed to refactoring this in the future, it would just take me more time in ember than we'd like to spend right now :P
src/tests/all.rs
Outdated
| } | ||
|
|
||
| fn add_team_to_crate(t: &Team, krate: &Crate, u: &User, conn: &PgConnection) -> CargoResult<()> { | ||
|
|
There was a problem hiding this comment.
You're just talking about cutting the newline here, right? #ambiguousemoji
| diesel::insert(&crate_owner.on_conflict( | ||
| crate_owners::table.primary_key(), | ||
| do_update().set(crate_owners::deleted.eq(false)), | ||
| )).into(crate_owners::table) |
There was a problem hiding this comment.
Is this the indentation rustfmt came up with?
app/routes/team.js
Outdated
| }, | ||
| (e) => { | ||
| if (e.errors.any(e => e.detail === 'Not Found')) { | ||
| console.log(params); |
There was a problem hiding this comment.
Looks like a debugging console.log left in here :)
src/owner.rs
Outdated
| )).into(teams::table) | ||
| .get_result(conn) | ||
| .map_err(Into::into) | ||
| fn get_org(login: &str) -> String { |
There was a problem hiding this comment.
I think we can refactor this a bit into one function that returns two string slices in a tuple-- I'll explain what I mean in person :)
| one only owned by a user, check that the CrateList returned | ||
| for the user_id contains only the crates owned by that user, | ||
| and that the CrateList returned for the team_id contains | ||
| only crates owned by that team. |
| @@ -864,6 +894,15 @@ pub fn index(req: &mut Request) -> CargoResult<Response> { | |||
| .filter(crate_owners::owner_kind.eq(OwnerKind::User as i32)), | |||
| ), | |||
| ); | |||
| } else if let Some(team_id) = params.get("team_id").and_then(|s| s.parse::<i32>().ok()) { | |||
There was a problem hiding this comment.
For consistency with how getting a user's crates works, mostly. If we're going to pull this out into its own route, then I think users should be pulled out too, which imo should be a separate PR. WDYT?
Creates a team page listing all crates associated with given team.
Fixes #409