gitea
gitea copied to clipboard
go-gitea
Fix wrong display of recently pushed notification
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 https://github.com/go-gitea/gitea/pull/26257#discussion_r1294085203
2024/04/25 Update: Fix #30611
#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. Instead, I used latest commit id, then you can also avoid display notification from both no-change branch in forked repo and the original repo.
Added some screenshots to the pr description.
We can add a test for this new feature later. Maybe not in this PR, it is not easy to do this, as we need to make many changes in db and git data.
There's a problem if we do not use branch id to link the head branch and the pull request. If we merged a PR, deleted the branch(ID:1) of that PR and created a new branch(ID:2) with the same name, we will never get the new PR notification, because when we search existing PR, the new created branch(ID:2) will be linked to the merged PR, and it will be excepted. 🤔
And the pull branch link in this PR will direct to the existing branch(ID:2) not the deleted branch(ID:1). 🤔
And the delete branch button will enable again
I'm coding the test, and notice that maybe we need to check the repo access permission here, but not sure about this now.
Maybe user can get new branches notification from no accessible repo, as the repoCond does not look good.
So I change the PR status to draft now.
Found a new problem:
If user A fork a repo which have a recently commit pushed by user B, user A will see the notification immediately,
as branch.pusher will be exchanged to user A, but actually it is committed by user B.
@lunny
So what is the definition of branch.pusher?
Found a new problem: If user A fork a repo which have a recently commit pushed by user B, user A will see the notification immediately, as
branch.pusherwill be exchanged to user A, but actually it is committed by user B.@lunny So what is the definition of
branch.pusher?
Pusher is a user which have registered in this Gitea instance which is different from committer. Git Committer maybe not a user of this instance. i.e. I pushed git kernel to my Gitea instance. I'm the pusher, but I haven't committed any commit to this repository.
Pusher is a user which have registered in this Gitea instance which is different from committer. Git Committer maybe not a user of this instance. i.e. I pushed git kernel to my Gitea instance. I'm the pusher, but I haven't committed any commit to this repository.
I see. I tried this in GitHub, and got the same result. So this is not a problem any more. 😄 But GitHub will only fork main branch by default, then user will not see the notification by default.
The question in my mind is: could the existing test fixtures be reused for testing?
I am not a fan of adding a lot of new fixtures for each new feature, otherwise, years later, there would be hundreds or thousands of git repositories for testing? Then would any one can tell their usage or is is possible to maintain them?
Of course it is better to reuse the existing fixtures in some simple tests. At first, I did this. But I found that for a complex test, reusing them will break lots of tests and become a hard work. I cannot say that this will bring us more benefits in maintenance as we need to take much more time in the loop of fixing the broken tests (maybe more newly broken tests appear during the fixing).
If you really want to reuse them, I think removing test codes in this PR and do that in another one maybe better, as this work will take too much time in coding and reviewing. How about you.
But I found that for a complex test, reusing them will break lots of tests and become a hard work.
How does it break? Each test uses the original fixtures, everything is reset.
For example: There's a repo A which seems that can be reused in your new test. And you decided to reuse it. You finished your new test, and it worked well. So you pushed the codes, but got lots of CI failures. Then you checked logs, and noticed that repo A can not be reused in your new test as there are several tests you didn't notice at the beginning are using repo A and your changes will cause conflicts in the end. For example, you can not change something related to repo A as the existing test A will check them, but you have to because you need these changes in your new test. Then you rewrite the codes and try to reuse repo B. But after taking several hours to finish your work, you notice that repo B can also not be reused for the same reason. Then this will bring you into a loop of deciding which one to reuse, finishing your codes, and resolving the conflicts.
CI failure seems related
api_org_test.go:94:
Error Trace: /home/runner/work/gitea/gitea/tests/integration/api_org_test.go:94
/home/runner/work/gitea/gitea/tests/integration/git_helper_for_declarative_test.go:87
/home/runner/work/gitea/gitea/tests/integration/api_org_test.go:28
Error: "[%!s(*structs.User=&{1 user1 user1 User One [email protected] https://secure.gravatar.com/avatar/111d68d06e2d317b5a59c2c6c5bad808?d=identicon en-US true {0 63828797734 <nil>} {0 62135596800 <nil>} false true false public 0 0 0}) %!s(*structs.User=&{12 user12 user12 User 12 [email protected] https://secure.gravatar.com/avatar/ef1debc1364806281c42eeedfdeb943b?d=identicon false {0 62135596800 <nil>} {0 62135596800 <nil>} false true false public 0 0 0})]" should have 1 item(s), but has 2
Test: TestAPIOrgCreate