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
Fully support project and group avatars #1506
Conversation
2bc06bb
to
7916da7
Compare
} | ||
type alias GroupAvatar | ||
return json.Marshal((*alias)(a)) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you explain why this is needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we wouldn't have this the field wouldn't be sent. In the current implementation it will send an empty string which effectively means to remove the avatar, e.g.:
curl --request PUT --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/api/v4/groups/22" \
--data "avatar="
@@ -420,6 +448,7 @@ func (s *GroupsService) TransferSubGroup(gid interface{}, opt *TransferSubGroupO | |||
type UpdateGroupOptions struct { | |||
Name *string `url:"name,omitempty" json:"name,omitempty"` | |||
Path *string `url:"path,omitempty" json:"path,omitempty"` | |||
Avatar *GroupAvatar `url:"-" json:"avatar,omitempty"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be:
Avatar *GroupAvatar `url:"-" json:"avatar,omitempty"` | |
Avatar *GroupAvatar `url:"-" json:"-"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmmm, good question. I think in case of UploadRequest()
the options are any ways never JSON encoded, but just form fields ... So, probably it doesn't matter. I have to double check.
Do you still need or want this? If so could you maybe cleanup the PR and reply to my questions? Then we can try to get it sorted... |
This change set adds support for project and group avatars in the same fashion this is already implemented for topics.
7916da7
to
02292ff
Compare
@svanharmelen I finally found some time to address this - sorry for the late response. Actually for both your question: I've pretty much just copy & pasted the approach from the topics endpoint. You've introduced the code here if I'm not mistaken. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @timofurrer... I'll take it 👍🏻
This change set adds support for project, group and user avatars in the
same fashion this is already implemented for topics.
Currently it's not possible to remove avatars from those API - I'm
exploring possible solutions
here.
Nonetheless, I think this PR can already be merged.