-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Add team option to grant rights for all organization repositories #6791
Conversation
Behaviour is the same for Owners teams and teams which includes all repositories. |
Drone fails |
There's still a change in the gitea sdk to add the new field There is a PR go-gitea/go-sdk#166 opened for this change. |
Codecov Report
@@ Coverage Diff @@
## master #6791 +/- ##
=========================================
+ Coverage 41.19% 41.2% +0.01%
=========================================
Files 469 470 +1
Lines 63536 63591 +55
=========================================
+ Hits 26171 26205 +34
- Misses 33943 33960 +17
- Partials 3422 3426 +4
Continue to review full report at Codecov.
|
Merge to master. |
Please resolve the conflicts |
@lunny conflicts resolved |
// addAllRepositories adds all repositories to the team. | ||
// If the team already has some repositories they will be left unchanged. | ||
func (t *Team) addAllRepositories(e Engine) error { | ||
err := e.Iterate(&Repository{OwnerID: t.OrgID}, func(i int, bean interface{}) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not just change includesAllRepository to true?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All repositories need to be added to the team because access rights management won't work without them.
It is the same behavior as for owners teams.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK. And I think we should use a better method to batch insert a slice of team_repo
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure we can use a better method to batch insert a slice of team_repo
but the method addRepository
do a lot more than that.
imho I don't think the batch insert will be worth it compared to the whole processing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assume there are 1000 repositories on this organization. And the code here is not work with sqlite database.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It still return 500 on my local test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't reproduce the issue. I must do it wrong.
I build with TAGS="bindata sqlite sqlite_unlock_notify" make clean generate build
I tested on Linux and Windows.
Do the UT pass on your local machine?
Can you please give me the steps which lead to the error 500?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's what I compiled locally. I'm on macOS. Just grant team to all organization repositories.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The UT must fail, don't they?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Problematic in pgSQL as well (500). After some debugging I found hasTeamRepo get this error: lib/pq#635
I would like to have a warning when pressing 'Update Setting' if includes_all_repositories was changed from false to true. Something like "This will add access for the team access to all 1118 repositories in the organization" Ok/Cancel. |
Please resolve conflicts |
@ngourdon doesn't seem to be active right now. Can I take over this (create a new PR)? |
replaced by #8688 |
Fix #6409 and #2654
related PR go-gitea/go-sdk#166