Files are viewed on too new commits

[delvh]

Description

Apparently, the I've viewed this file implementation is half broken now.
It's still working without complaint because it's "dangling on a single thread that hasn't been destroyed yet":
I've put in a safe-guard that apparently is the only reason it's still working:

gitea/routers/web/repo/pull_review.go

Lines 277 to 279 in f5ce2ed

	if data.HeadCommitSHA == "" {
	data.HeadCommitSHA = pull.HeadCommitID
	}

For some reason, the head commit SHA isn't set (anymore?), so reviews always get assigned to the newest commit which isn't necessarily the commit someone has reviewed (in case they had their tab open for long enough that other commits were pushed)
I've traced this bug back to

gitea/templates/repo/diff/box.tmpl

Line 142 in f5ce2ed

<label data-link="{{$.Issue.Link}}/viewed-files" data-headcommit="{{$.PullHeadCommitID}}" class="viewed-file-form unselectable{{if $file.IsViewed}} viewed-file-checked-form{{end}}">

as apparently PullHeadCommitID is empty/not present.
However, I have no idea why this would be the case or if it has always been that way, and simply no one noticed.
(I doubt it, I remember testing the whole functionality, and I would have probably noticed it if the head-commit was always empty)

Gitea Version

1.20.0+dev-833-g040970c32

Can you reproduce the bug on the Gitea demo site?

Yes

Log Gist

No response

Screenshots

No response

Git Version

No response

Operating System

No response

How are you running Gitea?

try.gitea.io/ gitea.com and from compiled from source

Database

None

[sebastian-sauer]

I just checked this on my local machine and on gitea.com - for both 1.20 versions the headcommit is set.

I've checked this PR: https://gitea.com/xorm/xorm/pulls/2241/files

[delvh]

Now I'm confused…
How can this happen?

---

For me, it still isn't set for a private repo under my own user.
But yes, I can confirm that your link indeed displays the headcommit.

Could it be that (private?) user repos have this problem?
Or under what circumstances is the headcommit not set?

[lunny]

Now I'm confused… How can this happen?

For me, it still isn't set for a private repo under my own user. But yes, I can confirm that your link indeed displays the headcommit.

Could it be that (private?) user repos have this problem? Or under what circumstances is the headcommit not set?

Which repositories could you paste here as an example?

[delvh]

Well… I've just published my previously private test repo on gitea.com where I can reproduce this issue:
https://gitea.com/delvh/test/pulls/1/files

I don't know, however, how long I'll leave it public.

[sebastian-sauer]

Steps to reproduce:

• Create a PR with a branch that changed one file X
• when opening the files tab the headcommit is set.
• Merge the PR and delete the source branch of the PR
• Open the PR files tab again
• --> Now the headcommit of the file X will be empty.

[sebastian-sauer]

@delvh Looks like the context variable "PullHeadCommitID" is not set.
When using "AfterCommitID" it looks like it's working - but i'm not sure if this covers all cases. Any opinions?

https://github.com/go-gitea/gitea/blob/main/templates/repo/diff/box.tmpl#L150