Add token_count to GET /api/v1/me response
#3071
Conversation
| @@ -29,6 +30,11 @@ pub fn me(req: &mut dyn RequestExt) -> EndpointResult { | |||
| )) | |||
| .first(&*conn)?; | |||
|
|
|||
| let token_count = ApiToken::belonging_to(&user) | |||
| .filter(api_tokens::revoked.eq(false)) | |||
There was a problem hiding this comment.
I think it would make me feel better if one of the tokens in the test was revoked and we asserted that it didn't count towards this total.
There was a problem hiding this comment.
thinking some more about it, I wonder if this revoked: false query really makes sense for us here. if our primary goal is to teach the user that he needs to create his first token to interact with cargo then, if he has a revoked token already, he apparently doesn't need to be taught this anymore :D
|
|
||
| #[derive(Serialize, Deserialize, Debug, Clone, Copy)] | ||
| pub struct EncodableMeMeta { | ||
| pub token_count: i64, |
There was a problem hiding this comment.
I wonder, what's the criteria for information being in EncodableMe or EncodableMeMeta? Should owned_crates be in EncodableMeMeta, for example?
There was a problem hiding this comment.
thanks for asking this question. when I was working on this it seemed obvious to me, but now I'm not so sure anymore either. 😅
I'm now wondering if it would be better to instead include the list of token IDs, like in a proper relationship declaration, but that would probably slightly slow down the query and for what we're trying to build it doesn't make a difference.
Maybe my confusion comes from using user for both other (GitHub) users, but also the logged in user account, but with exta information in the latter case.
There was a problem hiding this comment.
I thought about this a little bit more. I think my reason for putting this in meta was that this is derived data, which corresponds to the number of tokens that will be returned from the GET /api/v1/me/tokens request. It is not actually a property on the user resource, but rather the number of token relationships that this user has.
Should
owned_cratesbe inEncodableMeMeta, for example?
I guess it would be best if this was encoded as a proper relationship instead
|
☔ The latest upstream changes (presumably #3173) made this pull request unmergeable. Please resolve the merge conflicts. |
Turbo87
added
C-enhancement ✨
This PR is partly extracted from #1952 to finally resolve issue #19.
The PR adds a
meta: { token_count: 42 }object to theGET /api/v1/meresponse, which can be used to display a banner, flash message, or other indicator on the frontend to signal to the user that he has not created an API token yet. The exact implementation for the frontend will follow in another PR and does not need to be discussed here.r? @carols10cents since you did the original review on #1952. let me know if you don't currently have time for the review :)