Intermittent wrong comment count on PRs

[noerw]

• Gitea version (or commit ref): 1.16.0+dev-109-gd17f555fe

• Git version: ?

• Operating system: linux

• Database (use [x]):

  • MySQL

• Can you reproduce the bug at https://try.gitea.io:

  • Yes (provide example URL): https://gitea.com/gitea/tea/pulls/391, https://gitea.com/gitea/tea/pulls

• Log gist:

Description

A PR is listed as having comments, while none show up in the PR detail view (see gitea.com example above.)

This may be about approving reviews, and is probably caused by a fairly recent change, as an older PR has an approval too, but does not list additional comments that are not visible on the PR detail page.

The API also returns a higher comment count, while not returning any comments

Screenshots

[noerw]

hmm, now the comment count on that specific issue is correct, without further action (to my knowledge).. 🤔

[noerw]

This is a regression of #16075:
It adds reviews to the comment counter on a PR, but

• this does not check if the review has a comment body
• this deviates from the logic of CheckRepoStats(), which runs via cronjob and corrects the counter again

I'm not sure if we actually should add reviews to the comment counter. It intuitively makes sense, but brings issues in other aspects of Gitea:
This count is also returned in the API, and is likely to be used as a indication how many comments to fetch for an issue. Problem is, the comments API does not return reviews as comments, leading to bad state in API clients.
So either that API needs to be changed as well to return review comments as well (seems also intuitive), or we should just revert the change from #16075

[noerw]

github does the following:

• in the UI, they count any comment (including code comments) that has a body.
• the /pulls/{index}/comments API endpoint only returns non-review comments
• the /pulls/{index} endpoint returns two separate counters comments + review_comments

this feels like a reasonable solution, that could even be backported

[lunny]

It seems now it's correct. I think maybe we could close this one and remove it from v1.15.7 .

[noerw]

No this bug is not fixed yet, just reproduced on 1.15.6 and gitea.com.
I'm currently short on time, but I'll try to come up with a fix in the coming days

[lunny]

If we follow Github's method, we need a new number column but I think it's difficult to add a new column in a stable version. Maybe we can do it in v1.16 if possible. I'll move this to v1.15.8

[lunny]

• https://gitea.com/gitea/tea/pulls

Could you have time to confirm if it's gone?

[noerw]

fyi: bug is still present.
@lunny I'll submit a bugfix for 1.15.10 that reverts this two line change of #16075 so we at least have consistent state back.

In a followup for 1.16 or 1.17 we can aim at implementing PR comment counts similar to github with separate counters.