mirror of
https://github.com/go-gitea/gitea.git
synced 2026-03-14 11:56:41 -05:00
Do not track edits and deletions of pending review comments #8593
Closed
opened 2025-11-02 08:11:55 -06:00 by GiteaMirror
·
7 comments
No Branch/Tag Specified
main
release/v1.25
release/v1.24
release/v1.23
release/v1.22
release/v1.21
release/v1.20
release/v1.19
release/v1.18
release/v1.17
release/v1.16
release/v1.15
release/v1.14
release/v1.13
release/v1.12
release/v1.11
release/v1.10
release/v1.9
release/v1.8
v1.25.3
v1.25.2
v1.25.1
v1.25.0
v1.24.7
v1.25.0-rc0
v1.26.0-dev
v1.24.6
v1.24.5
v1.24.4
v1.24.3
v1.24.2
v1.24.1
v1.24.0
v1.23.8
v1.24.0-rc0
v1.25.0-dev
v1.23.7
v1.23.6
v1.23.5
v1.23.4
v1.23.3
v1.23.2
v1.23.1
v1.23.0
v1.23.0-rc0
v1.24.0-dev
v1.22.6
v1.22.5
v1.22.4
v1.22.3
v1.22.2
v1.22.1
v1.22.0
v1.23.0-dev
v1.22.0-rc1
v1.21.11
v1.22.0-rc0
v1.21.10
v1.21.9
v1.21.8
v1.21.7
v1.21.6
v1.21.5
v1.21.4
v1.21.3
v1.21.2
v1.20.6
v1.21.1
v1.21.0
v1.21.0-rc2
v1.21.0-rc1
v1.20.5
v1.22.0-dev
v1.21.0-rc0
v1.20.4
v1.20.3
v1.20.2
v1.20.1
v1.20.0
v1.19.4
v1.21.0-dev
v1.20.0-rc2
v1.20.0-rc1
v1.20.0-rc0
v1.19.3
v1.19.2
v1.19.1
v1.19.0
v1.19.0-rc1
v1.20.0-dev
v1.19.0-rc0
v1.18.5
v1.18.4
v1.18.3
v1.18.2
v1.18.1
v1.18.0
v1.17.4
v1.18.0-rc1
v1.19.0-dev
v1.18.0-rc0
v1.17.3
v1.17.2
v1.17.1
v1.17.0
v1.17.0-rc2
v1.16.9
v1.17.0-rc1
v1.18.0-dev
v1.16.8
v1.16.7
v1.16.6
v1.16.5
v1.16.4
v1.16.3
v1.16.2
v1.16.1
v1.16.0
v1.15.11
v1.17.0-dev
v1.16.0-rc1
v1.15.10
v1.15.9
v1.15.8
v1.15.7
v1.15.6
v1.15.5
v1.15.4
v1.15.3
v1.15.2
v1.15.1
v1.14.7
v1.15.0
v1.15.0-rc3
v1.14.6
v1.15.0-rc2
v1.14.5
v1.16.0-dev
v1.15.0-rc1
v1.14.4
v1.14.3
v1.14.2
v1.14.1
v1.14.0
v1.13.7
v1.14.0-rc2
v1.13.6
v1.13.5
v1.14.0-rc1
v1.15.0-dev
v1.13.4
v1.13.3
v1.13.2
v1.13.1
v1.13.0
v1.12.6
v1.13.0-rc2
v1.14.0-dev
v1.13.0-rc1
v1.12.5
v1.12.4
v1.12.3
v1.12.2
v1.12.1
v1.11.8
v1.12.0
v1.11.7
v1.12.0-rc2
v1.11.6
v1.12.0-rc1
v1.13.0-dev
v1.11.5
v1.11.4
v1.11.3
v1.10.6
v1.12.0-dev
v1.11.2
v1.10.5
v1.11.1
v1.10.4
v1.11.0
v1.11.0-rc2
v1.10.3
v1.11.0-rc1
v1.10.2
v1.10.1
v1.10.0
v1.9.6
v1.9.5
v1.10.0-rc2
v1.11.0-dev
v1.10.0-rc1
v1.9.4
v1.9.3
v1.9.2
v1.9.1
v1.9.0
v1.9.0-rc2
v1.10.0-dev
v1.9.0-rc1
v1.8.3
v1.8.2
v1.8.1
v1.8.0
v1.8.0-rc3
v1.7.6
v1.8.0-rc2
v1.7.5
v1.8.0-rc1
v1.9.0-dev
v1.7.4
v1.7.3
v1.7.2
v1.7.1
v1.7.0
v1.7.0-rc3
v1.6.4
v1.7.0-rc2
v1.6.3
v1.7.0-rc1
v1.7.0-dev
v1.6.2
v1.6.1
v1.6.0
v1.6.0-rc2
v1.5.3
v1.6.0-rc1
v1.6.0-dev
v1.5.2
v1.5.1
v1.5.0
v1.5.0-rc2
v1.5.0-rc1
v1.5.0-dev
v1.4.3
v1.4.2
v1.4.1
v1.4.0
v1.4.0-rc3
v1.4.0-rc2
v1.3.3
v1.4.0-rc1
v1.3.2
v1.3.1
v1.3.0
v1.3.0-rc2
v1.3.0-rc1
v1.2.3
v1.2.2
v1.2.1
v1.2.0
v1.2.0-rc3
v1.2.0-rc2
v1.1.4
v1.2.0-rc1
v1.1.3
v1.1.2
v1.1.1
v1.1.0
v1.0.2
v1.0.1
v1.0.0
v0.9.99
Labels
Clear labels
$20
$250
$50
$500
backport/done
💎 Bounty
docs-update-needed
good first issue
hacktoberfest
issue/bounty
issue/confirmed
issue/critical
issue/duplicate
issue/needs-feedback
issue/not-a-bug
issue/regression
issue/stale
issue/workaround
lgtm/need 2
modifies/api
modifies/translation
outdated/backport/v1.18
outdated/theme/markdown
outdated/theme/timetracker
performance/bigrepo
performance/cpu
performance/memory
performance/speed
pr/breaking
proposal/accepted
proposal/rejected
pr/wip
pull-request
reviewed/wontfix
💰 Rewarded
skip-changelog
status/blocked
topic/accessibility
topic/api
topic/authentication
topic/build
topic/code-linting
topic/commit-signing
topic/content-rendering
topic/deployment
topic/distribution
topic/federation
topic/gitea-actions
topic/issues
topic/lfs
topic/mobile
topic/moderation
topic/packages
topic/pr
topic/projects
topic/repo
topic/repo-migration
topic/security
topic/theme
topic/ui
topic/ui-interaction
topic/ux
topic/webhooks
topic/wiki
type/bug
type/deprecation
type/docs
type/enhancement
type/feature
type/miscellaneous
type/proposal
type/question
type/refactoring
type/summary
type/testing
type/upstream
Mirrored from GitHub Pull Request
Milestone
No items
No Milestone
Projects
Clear projects
No project
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: github-starred/gitea#8593
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Originally created by @delvh on GitHub (Feb 21, 2022).
Gitea Version
1.16.1-dev
Git Version
NA
Operating System
Linux
How are you running Gitea?
NA
Database
No response
Can you reproduce the bug on the Gitea demo site?
Yes, unfortunately
Log Gist
No response
Description
Currently, any edit to a comment will be logged in its change history.
For PR Reviews, however, edits should only be logged after the review was submitted.
Edits prior to that should simply be discarded.
Scope
There are two aspects that makes this issue way worse:
Keep in mind
The creation change should be adapted to not show the original text,
but the text of the last edit before submitting the review.
However, this appears to already be done within
models/issues/content_history.go#keepLimitedContentHistorysee https://try.gitea.io/delvh/kanban-test/pulls/16#issuecomment-113571 for example.
Screenshots
No response
@wxiaoguang commented on GitHub (Jul 5, 2022):
There is an old issue, it seems related.
@lunny commented on GitHub (Jul 5, 2022):
duplicated
@wxiaoguang commented on GitHub (Jul 5, 2022):
@delvh are you working on this? Should we keep this issue open, or use the old one?
@delvh commented on GitHub (Jul 5, 2022):
We can use the other one, and so far I'm not working on it because
a) I have close to no time right now (see my sparse interactions in the last few days)
b) I haven't found the root location of this "bug" yet, whenever I attempt to follow the trail, it leads me throughout the whole application...
@wxiaoguang commented on GitHub (Jul 5, 2022):
Hmm ... these 2 issues are related, but not exactly the same (#4388 is about sending notification before summit, this is about saving content history before submit). I think we can keep both open, just link them for further investigation.
@madxmike commented on GitHub (Aug 11, 2023):
Hi, I ended up being nerd snipped by @jolheiser on this issue last night. Heres the rundown:
There are 3 issues occuring during a pending PR Review:
I think its easier to start with showing what happens in the working case first:
Adding a new pending comment during a pending PR review does not send a webhook notification
88479e0dfc/routers/web/web.go (L1304)pull_service.CreateCodeCommentwith thependingReviewparameter set tofalsehttps://github.com/go-gitea/gitea/blob/main/routers/web/repo/pull_review.go#L76if !pendingReview && existsReview. For new comments this evaluates toif !true && false. As such, it does not go down that if path which contains a call to the notification service https://github.com/go-gitea/gitea/blob/main/services/pull/review.go#L116.As you see above, the new comment path never ends up calling the notification service, so no notifications are sent out. These notifications are eventually sent when the PR Review is published https://github.com/go-gitea/gitea/blob/main/services/pull/review.go#L310
Replying to a pending comment during a pending PR Review will send a webhook notification
When you press the "Reply" button on a Reply message during a pending PR Review it will call this endpoint

88479e0dfc/routers/web/web.go (L1304)with the"single_review"flag set to"true"This will go through the repo layer and call
pull_service.CreateCodeCommentwith thependingReviewparameter set tofalsehttps://github.com/go-gitea/gitea/blob/main/routers/web/repo/pull_review.go#L76This ends up in the pull review service https://github.com/go-gitea/gitea/blob/main/services/pull/review.go#L73 where there is a check
if !pendingReview && existsReview. For reply comments this evaluates toif !false && true. As such, it goes down the if path which contains a call to the notification service https://github.com/go-gitea/gitea/blob/main/services/pull/review.go#L116.A notification is sent out
The fix here would be to do some type of check around this notification call to ensure if the Review is pending or not when this path is called.
Deleting a pending comment during a pending PR Review will send a webhook notification
88479e0dfc/routers/web/web.go (L1052)issue_service.DeleteCommenthttps://github.com/go-gitea/gitea/blob/main/routers/web/repo/issue.go#L3121issue_service.DeleteCommentis a implementation that simply deletes the comment and sends a notification https://github.com/go-gitea/gitea/blob/main/services/issue/comments.go#L104The fix here would be to add a
DeleteCommentfunction to thepull_servicethat does much the same as thepull_service.CreateCodeComment. It needs to check if the review is pending, and if so dont sent a notification. Then add a layer in the pull review repo to handle the review case. Then create a new endpoint that goes to this repo.Editing a pending comment during a pending PR review will send a webhook notification
88479e0dfc/routers/web/web.go (L1051)issue_service.UpdateCommenthttps://github.com/go-gitea/gitea/blob/main/routers/web/repo/issue.go#L3065issue_service.UpdateCommentis an implementation that simply updates the comment and sends a notification https://github.com/go-gitea/gitea/blob/main/services/issue/comments.go#L72The fix here would be to add a
UpdateCommentfunction to thepull_servicethat does much the same as thepull_service.CreateCodeComment. It needs to check if the review is pending, and if so dont sent a notification. Then add a layer in the pull review repo to handle the review case. Then create a new endpoint that goes to this repo.@xgdgsc commented on GitHub (Jan 23, 2024):
And when you first time open a PR and assign someone. 2 notifications are sent that should be merged to one.