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

New pull request notification will show wrong branch in a special case #32769

Closed
yp05327 opened this issue Dec 9, 2024 · 7 comments · Fixed by #33215
Closed

New pull request notification will show wrong branch in a special case #32769

yp05327 opened this issue Dec 9, 2024 · 7 comments · Fixed by #33215
Assignees
Labels

Comments

@yp05327
Copy link
Contributor

yp05327 commented Dec 9, 2024

Description

Sorry, this is a bug come from my PR #25812

Reproduce:

  1. create a new repo A, create a file in it
  2. fork it to repo B
  3. push a commit (the most easy way is creating a new file in web) in repo A's main branch

Then you will see:
In repo A, the branch in notification is repo B's main branch, actually the expected branch is repo A's main branch.
image
In the fork (repo B), it show the repo A's main branch, actually the expected behavior is showing nothing.
image

Reason:
For newly created empty branch (based on default branch), I want to ignore the notification, so I added this in the SQL query.
image
And this cause this bug.

In repo A, it will ignore the newly pushed commit because of the condition I mentioned above, and the default branch's latest commit id is changed. So in repo B's main branch, the latest commit id is not same to the repo A's default branch's latest commit id, so it will be picked up as a recently updated branch.

Don't have a good idea about how to fix it now. If someone is interested, it is welcome to create a PR for it.

Gitea Version

latest

Can you reproduce the bug on the Gitea demo site?

No

Log Gist

No response

Screenshots

No response

Git Version

No response

Operating System

No response

How are you running Gitea?

build

Database

None

@cclvi256
Copy link

In my trial like this, only the picture 2 is successfully re-produced, and there's nothing like the senario like picture 1. And I reckon that this is just normal.

By the way, maybe we can add a button to "Syncronise Forked Repository" like GitHub.

@lunny
Copy link
Member

lunny commented Dec 16, 2024

By the way, maybe we can add a button to "Syncronise Forked Repository" like GitHub.

The feature has been implemented which will be released in v1.23.0

@changchaishi
Copy link
Contributor

The feature has been implemented which will be released in v1.23.0

Hi @lunny, may I ask if the release will include the fix to this issue? If not, I would like to work on this issue.

@lunny
Copy link
Member

lunny commented Jan 6, 2025

The feature has been implemented which will be released in v1.23.0

Hi @lunny, may I ask if the release will include the fix to this issue? If not, I would like to work on this issue.

I don't think so. I just answered the off-topic question. You are welcome to send a pull request.

@yp05327
Copy link
Contributor Author

yp05327 commented Jan 6, 2025

@changchaishi
You can ask @lunny assigning this issue to you, if you are interested in fixing it.
As there's no PR now, everyone can send a PR to fix it. But if someone is assigned, then others may not work on it any more.
Also it is not related to the schedule of the release. If the PR can be finished and merged before the release, then it will be included.

ps: I have no idea about how to fix it, it is really complex. I'm looking forward to the solution.

@changchaishi
Copy link
Contributor

Hi sharing my thoughts here.

Maybe it is not as hard as it seems, as in the branches view, we already have logic that tells whether a branch is worth a PR for the base branch. We can transplant this logic from to forked repo branch to the base default branch. So we always catch the right branches to show.

Simply put, the behavior should be:

  • target's commit to the branch is fresh (less than 2 hours)
  • Then do a GetUpstreamDivergingInfo for the ahead counter
    • if any ahead to the base branch is greater than 0, include them
  • the rest follow the current prepareRecentlyPushedNewBranches filter logic

Could this be the solution?

@yp05327
Copy link
Contributor Author

yp05327 commented Jan 7, 2025

Could this be the solution?

Send a PR with a test for this case is the simple way I think.

lunny pushed a commit that referenced this issue Jan 17, 2025
Fixes #32769 by the logic from pr #33192

---------

Co-authored-by: wxiaoguang <[email protected]>
wxiaoguang pushed a commit to wxiaoguang/gitea that referenced this issue Jan 18, 2025
Fixes go-gitea#32769 by the logic from pr go-gitea#33192

---------

Co-authored-by: wxiaoguang <[email protected]>
# Conflicts:
#	services/repository/merge_upstream.go
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants