Skip to content
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

Added gitlab avatar support #56

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

savitojs
Copy link

@savitojs savitojs commented Jan 1, 2022

Added GitLab avatar support, merely any addition to the logic but APIs.
Only works for gitlab.com (No self-hosting GitLab avatar support), self-hosting may be using GTK's prefs settings.

Fixes #36

Signed-off-by: Savitoj Singh [email protected]

Signed-off-by: Savitoj Singh <[email protected]>
Copy link
Owner

@jwendell jwendell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! I made some comments below.

let _httpSession = new Soup.Session({user_agent: 'jwendell/gnome-shell-extension-timezone'});
let url = 'https://gitlab.com/api/v4/avatar?email=%s'.format(this.gitlab);
let message = new Soup.Message({method: 'GET', uri: new Soup.URI(url)});
if (this._gitlabToken) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where does this field come from?

let url = 'https://gitlab.com/api/v4/avatar?email=%s'.format(this.gitlab);
let message = new Soup.Message({method: 'GET', uri: new Soup.URI(url)});
if (this._gitlabToken) {
message.request_headers.append("Authorization", "token " + this._gitlabToken);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to https://docs.gitlab.com/ee/api/#oauth2-tokens should this header be Authorization: Bearer <your_access_token> instead?


_httpSession.queue_message(message, Lang.bind(this, function(session, message) {
if (message.status_code != Soup.KnownStatusCode.OK) {
log('Response code "%d" getting data from gitlab for user email address %s. Got: %s'.format(message.status_code, this.github, message.response_body.data));
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/github/gitlab/

if (!this.avatar && p.avatar_url)
this.avatar = p.avatar_url;

if (!this.name && p.name)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at https://docs.gitlab.com/ee/api/avatar.html it seems the avatar_url is the only field in the response.
Perhaps you could use the /users endpoint and get all fields like we do with github?

if (!this.name && p.name)
this.name = p.name;

if (!this.city && p.location)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

@@ -29,6 +29,10 @@
"type": "string",
"description": "Gravatar ID (person's email)"
},
"gitlab": {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perhaps we could make gitlab a struct containing host and email, with host defaulting to gitlab.com? Thus we would support different self-hosted instances of gitlab.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support GitLab
2 participants