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

Return an empty string when a repo has no avatar in the repo API #31187

Merged
merged 4 commits into from
Jun 1, 2024

Conversation

kemzeb
Copy link
Contributor

@kemzeb kemzeb commented May 30, 2024

Resolves #31167.

#30885 changed the behavior of repo.AvatarLink() where it can now take the empty string and append it to the app data URL. This does not point to a valid avatar image URL, and, as the issue mentions, previous Gitea versions returned the empty string.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label May 30, 2024
@pull-request-size pull-request-size bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label May 30, 2024
@github-actions github-actions bot added the modifies/go Pull requests that update Go code label May 30, 2024
@lunny lunny requested a review from wxiaoguang May 31, 2024 01:57
models/repo/avatar.go Outdated Show resolved Hide resolved
Copy link
Contributor

@wxiaoguang wxiaoguang left a comment

Choose a reason for hiding this comment

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

Oops, I forgot that the link could be empty

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels May 31, 2024
models/repo/avatar.go Outdated Show resolved Hide resolved
@silverwind
Copy link
Member

Does models/user/avatar.go:94 need the same treatment? Share a function maybe?

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels May 31, 2024
@delvh delvh changed the title Have repo API return the empty URL string when a repo has no avatar Return an empty string when a repo has no avatar in the repo API May 31, 2024
@kemzeb
Copy link
Contributor Author

kemzeb commented Jun 1, 2024

Does models/user/avatar.go:94 need the same treatment? Share a function maybe?

After reading this comment and taking a gander at this function, I don't believe we need to consider this as it looks like the user will always have an avatar

@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Jun 1, 2024
@lunny lunny enabled auto-merge (squash) June 1, 2024 11:23
@lunny lunny merged commit ab458ce into go-gitea:main Jun 1, 2024
26 checks passed
@GiteaBot GiteaBot added this to the 1.23.0 milestone Jun 1, 2024
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Jun 1, 2024
@kemzeb kemzeb deleted the fix-repo-api branch June 1, 2024 17:32
zjjhot added a commit to zjjhot/gitea that referenced this pull request Jun 3, 2024
* giteaofficial/main:
  [skip ci] Updated licenses and gitignores
  Fix overflow in issue card (go-gitea#31203)
  Fix agit checkout command line hint & fix ShowMergeInstructions checking (go-gitea#31219)
  Fix the possible migration failure on 286 with postgres 16 (go-gitea#31209)
  Only update poster in issue/comment list if it has been loaded (go-gitea#31216)
  Return an empty string when a repo has no avatar in the repo API (go-gitea#31187)
  Split sanitizer functions and fine-tune some tests (go-gitea#31192)
  Performance improvements for pull request list API (go-gitea#30490)
  Fix URL In Gitea Actions Badge Docs (go-gitea#31191)
@jpraet jpraet added type/bug backport/v1.22 This PR should be backported to Gitea 1.22 labels Jul 4, 2024
@jpraet
Copy link
Member

jpraet commented Jul 5, 2024

@yardenshoham any idea what's missing to get the GiteaBot to backport this PR? Thanks

@yardenshoham
Copy link
Member

It creates backports whenever a new commit is pushed. There were no new commits since you added the label.

jpraet pushed a commit to jpraet/gitea that referenced this pull request Jul 5, 2024
…gitea#31187)

Backport go-gitea#31187

Resolves go-gitea#31167.

go-gitea#30885 changed the behavior of
`repo.AvatarLink()` where it can now take the empty string and append it
to the app data URL. This does not point to a valid avatar image URL,
and, as the issue mentions, previous Gitea versions returned the empty
string.

---------

Co-authored-by: wxiaoguang <[email protected]>
@jpraet jpraet added the backport/done All backports for this PR have been created label Jul 5, 2024
@silverwind
Copy link
Member

It creates backports whenever a new commit is pushed. There were no new commits since you added the label.

I recall it had backported before even without any new commits, was that changed?

@yardenshoham
Copy link
Member

No. New commits were always the only trigger.

@silverwind
Copy link
Member

silverwind commented Jul 5, 2024

Imho, it should also watch when backport/* labels change on a merged PR.

@yardenshoham
Copy link
Member

It's a bit of a problem because we only have one filesystem so concurrent cherry-picks would happen.

6543 pushed a commit that referenced this pull request Jul 5, 2024
) (#31567)

Backport #31187

Resolves #31167.

#30885 changed the behavior of
`repo.AvatarLink()` where it can now take the empty string and append it
to the app data URL. This does not point to a valid avatar image URL,
and, as the issue mentions, previous Gitea versions returned the empty
string.

Co-authored-by: Kemal Zebari <[email protected]>
Co-authored-by: wxiaoguang <[email protected]>
@silverwind
Copy link
Member

silverwind commented Jul 5, 2024

Hmm, sounds like you need some form of distributed locking. Redis could be used for example.

@go-gitea go-gitea locked as resolved and limited conversation to collaborators Sep 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport/done All backports for this PR have been created backport/v1.22 This PR should be backported to Gitea 1.22 lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/go Pull requests that update Go code size/S Denotes a PR that changes 10-29 lines, ignoring generated files. type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect API response for repositories without avatar
8 participants