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

Fully support project and group avatars #1506

Merged
merged 2 commits into from Nov 18, 2022

Conversation

timofurrer
Copy link
Contributor

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.

@timofurrer timofurrer marked this pull request as draft July 16, 2022 09:14
@timofurrer timofurrer marked this pull request as ready for review July 16, 2022 10:11
}
type alias GroupAvatar
return json.Marshal((*alias)(a))
}
Copy link
Member

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?

Copy link
Contributor Author

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"`
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be:

Suggested change
Avatar *GroupAvatar `url:"-" json:"avatar,omitempty"`
Avatar *GroupAvatar `url:"-" json:"-"`

Copy link
Contributor Author

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.

@svanharmelen
Copy link
Member

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.
@timofurrer timofurrer changed the title Fully support project, group and user avatars Fully support project and group avatars Nov 17, 2022
@timofurrer
Copy link
Contributor Author

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...

@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.

Copy link
Member

@svanharmelen svanharmelen left a 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 👍🏻

@svanharmelen svanharmelen merged commit fce9772 into xanzy:master Nov 18, 2022
3 checks passed
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.

None yet

2 participants