Fix wrong display of recently pushed notification #25812
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
GiteaBot
added
the
lgtm/need 2
pull-request-size
bot
added
the
size/XS
|
duplicated with #25795 |
|
It does sound like it solves a different issue. |
|
Have no enough time to test this PR, I will test it later. |
lunny
approved these changes
Jul 10, 2023
GiteaBot
added
lgtm/need 1
delvh
reviewed
Jul 10, 2023
models/git/branch.go
Outdated
| @@ -391,7 +391,7 @@ func FindRecentlyPushedNewBranches(ctx context.Context, repoID, userID int64) (B | |||
| "issue.is_closed": false, | |||
| }) | |||
| err := db.GetEngine(ctx). | |||
| Where("pusher_id=? AND is_deleted=?", userID, false). | |||
| Where("repo_id=? AND pusher_id=? AND is_deleted=?", repoID, userID, false). | |||
There was a problem hiding this comment.
The problem I see with this is the following:
Now, you won't be able to see this message in forks.
I've thought about the logic in the previous PR and thought it seems sensible.
While I can also see your problem, the question is now which of the two is better?
Or is there a third approach to still include forks, i.e. additionally storing a ForkedFromID or something like that?
There was a problem hiding this comment.
I think it's difficult to get all forked repositories. Maybe only the first level forked?
There was a problem hiding this comment.
I meant the other way round as it is a one-to-n relation: Each fork only has a single parent, so we can check for repoID = fork or parent.
pull-request-size
bot
added
size/M
|
#25795 not looks good to me. It is not good to detect branch by name, as we may have same branch names in different repos. Added some screenshots to the pr description. |
pull-request-size
bot
added
size/L
GiteaBot
added
lgtm/need 2
|
We can add a test for this new feature later. |
wxiaoguang
reviewed
May 21, 2024
wxiaoguang
reviewed
May 21, 2024
yp05327
force-pushed
the
fix-incorrect-recently-pushed-new-branches-check
branch
from
59b9311
to
8cea058
Compare
wxiaoguang
reviewed
May 21, 2024
wxiaoguang
approved these changes
May 21, 2024
GiteaBot
added
lgtm/done
lunny
added
the
reviewed/wait-merge
|
I was unable to create a backport for 1.22. @yp05327, please send one manually. 🍵 |
GiteaBot
added
backport/manual
yp05327
added a commit
to yp05327/gitea
that referenced
this pull request
May 22, 2024
There's a bug in go-gitea#25715: If user pushed a commit into another repo with same branch name, the no-related repo will display the recently pushed notification incorrectly. It is simple to fix this, we should match the repo id in the sql query.  The latest commit is 2 weeks ago.  The notification comes from another repo with same branch name:  After: In forked repo:  New PR Link will redirect to the original repo:  In the original repo:  New PR Link:  In the same repo:  New PR Link:  08/15 Update: Follow go-gitea#26257, added permission check and logic fix mentioned in go-gitea#26257 (comment) 2024/04/25 Update: Fix go-gitea#30611 --------- Co-authored-by: silverwind <me@silverwind.io> Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com> Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
zjjhot
added a commit
to zjjhot/gitea
that referenced
this pull request
May 22, 2024
* giteaofficial/main: Add nix flake for dev shell (go-gitea#30967) [skip ci] Updated translations via Crowdin Fix wrong display of recently pushed notification (go-gitea#25812) use existing oauth grant for public client (go-gitea#31015) Fix automerge will not work because of some events haven't been triggered (go-gitea#30780)
lunny
added a commit
that referenced
this pull request
May 23, 2024
Backport #25812 ~~ps: removed some new codes in `tests/integration/pull_merge_test.go`~~ --------- Co-authored-by: silverwind <me@silverwind.io> Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com> Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There's a bug in #25715:
If user pushed a commit into another repo with same branch name, the no-related repo will display the recently pushed notification incorrectly.
It is simple to fix this, we should match the repo id in the sql query.
The latest commit is 2 weeks ago.
The notification comes from another repo with same branch name:
After:
In forked repo:
New PR Link will redirect to the original repo:
In the original repo:
New PR Link:
In the same repo:
New PR Link:
08/15 Update:
Follow #26257, added permission check and logic fix mentioned in #26257 (comment)
2024/04/25 Update:
Fix #30611