Skip to content

Conversation

lunny
Copy link
Member

@lunny lunny commented Jul 8, 2025

A quick fix for #34991

ValidateCommitsWithEmails will create a fake user for a git commit user with a related Gitea user. The UI should not display a link for such users.

@lunny lunny added type/bug backport/v1.24 This PR should be backported to Gitea 1.24 labels Jul 8, 2025
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jul 8, 2025
@github-actions github-actions bot added the modifies/templates This PR modifies the template files label Jul 8, 2025
@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 Jul 8, 2025
@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 Jul 8, 2025
@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Jul 8, 2025
@lunny lunny enabled auto-merge (squash) July 8, 2025 22:23
@silverwind
Copy link
Member

silverwind commented Jul 8, 2025

I don't think this has any relation to #34991. The missing user association happens in both commit list and PR commit history. It is fine in single commit view and latest commit. I think a more fundamental fix will be needed.

@lunny lunny merged commit f1b78f3 into go-gitea:main Jul 8, 2025
26 checks passed
@GiteaBot GiteaBot added this to the 1.25.0 milestone Jul 8, 2025
GiteaBot pushed a commit to GiteaBot/gitea that referenced this pull request Jul 8, 2025
A quick fix for go-gitea#34991 

`ValidateCommitsWithEmails` will create a fake user for a git commit
user with a related Gitea user. The UI should not display a link for
such users.
@GiteaBot GiteaBot added backport/done All backports for this PR have been created and removed reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. labels Jul 8, 2025
@wxiaoguang
Copy link
Contributor

I don't the fix is right.

The old logic doesn't set User field IIRC

And please add a test

@wxiaoguang
Copy link
Contributor

I don't think this has any relation to #34991. The missing user association happens in both commit list and PR commit history. It is fine in single commit view and latest commit. I think a more fundamental fix will be needed.

It is related but this fix is not the one supposed to be.

See #35003 (comment)

@lunny lunny deleted the lunny/fix_bug_commits_list branch July 9, 2025 01:50
zjjhot added a commit to zjjhot/gitea that referenced this pull request Jul 9, 2025
* giteaofficial/main:
  Refactor mail template and support preview (go-gitea#34990)
  [skip ci] Updated translations via Crowdin
  Fix bug when displaying git user avatar in commits list (go-gitea#35003)
  Tweak placement of diff file menu (go-gitea#34999)
  Start automerge check again after the conflict check and the schedule (go-gitea#34989)
  Refactor time tracker UI (go-gitea#34983)
  [skip ci] Updated translations via Crowdin
  Improve NuGet API Parity (go-gitea#21291) (go-gitea#34940)
@wxiaoguang
Copy link
Contributor

Please review the proper fix: Fix git commit committer parsing and add some tests #35007

@silverwind
Copy link
Member

silverwind commented Jul 9, 2025

Definitely not a right fix, the issues seen in #34991 affect multiple places in the UI and changing only one non-shared place in the template won't fix it properly.

@silverwind
Copy link
Member

This was obviously never tested, because otherwise the incorrectly formatted HTML comment would have been noticed:

image

@wxiaoguang
Copy link
Contributor

Hmm ... fortunately we have a proper fix now.

And since the proper fix is quite large, I would suggest either use a quick patch for 1.24 (only check ID!=0), or just don't backport.

I guess end users could bear it, not a serious bug in most cases, just a 404 page. We can release 1.25 soon if we'd like to land more fixes, actually 1.25 contains many important fixes including the container bug, which can't be backported to 1.24

@silverwind
Copy link
Member

Yes, I agree that a 404 link is a acceptable flaw. I think we need more rigorous integration tests to avoid bugs like this in the future.

wxiaoguang pushed a commit to GiteaBot/gitea that referenced this pull request Jul 10, 2025
A quick fix for go-gitea#34991

`ValidateCommitsWithEmails` will create a fake user for a git commit
user with a related Gitea user. The UI should not display a link for
such users.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/done All backports for this PR have been created backport/v1.24 This PR should be backported to Gitea 1.24 lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/templates This PR modifies the template files type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants