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

Email notification for pushes to head branch of closed pull request #23440

Closed
brechtvl opened this issue Mar 13, 2023 · 3 comments · Fixed by #23462
Closed

Email notification for pushes to head branch of closed pull request #23440

brechtvl opened this issue Mar 13, 2023 · 3 comments · Fixed by #23462
Labels

Comments

@brechtvl
Copy link
Contributor

brechtvl commented Mar 13, 2023

Description

  • Enable Settings > Account > And Your Own Notifications (for easier testing with 1 account)
  • Fork a repository
  • Commit a change to the default branch in the base repository
  • Make a pull request on the fork to merge the latest default branch into the fork
  • Close pull request without merging
  • Commit a change to the default branch in the base repository
  • Get an email notification for the closed pull request
  • Pull request itself shows no new commit

I'm guessing this started happening after #23189, since I only saw it after updating to a recent version of Gitea.

I think there are two problems:

  • Email notifications should not be sent out for closed pull requests in this case. This can lead to potentially a lot emails depending if someone is using this workflow to update their fork.
  • Closed pull requests get commit comments but they are hidden. The amount of additional comments can grow effectively quadratically, as more closed pull requests accumulate and more commits are made to the default branch.

Gitea Version

43c1362 (main)

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?

Both own build and try.gitea.io.

Database

None

@brechtvl
Copy link
Contributor Author

CC @sillyguodong

@sillyguodong
Copy link
Contributor

CC @sillyguodong

copy that, I will check it.

@sillyguodong
Copy link
Contributor

  • Email notifications should not be sent out for closed pull requests in this case. This can lead to potentially a lot emails depending if someone is using this workflow to update their fork.

oh, I made a mistake and fix in #23462

lunny added a commit that referenced this issue Mar 15, 2023
Close #23440
Cause by #23189
In #23189, we should insert a comment record into db when pushing a
commit to the PR, even if the PR is closed.
But should skip sending any notification in this case.

---------

Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
GiteaBot pushed a commit to GiteaBot/gitea that referenced this issue Mar 15, 2023
)

Close go-gitea#23440
Cause by go-gitea#23189
In go-gitea#23189, we should insert a comment record into db when pushing a
commit to the PR, even if the PR is closed.
But should skip sending any notification in this case.

---------

Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
KN4CK3R added a commit that referenced this issue Mar 15, 2023
…3492)

Backport #23462 by @sillyguodong

Close #23440
Cause by #23189
In #23189, we should insert a comment record into db when pushing a
commit to the PR, even if the PR is closed.
But should skip sending any notification in this case.

Co-authored-by: sillyguodong <33891828+sillyguodong@users.noreply.github.com>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
Co-authored-by: KN4CK3R <admin@oldschoolhack.me>
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants