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
search: Create the "my" repo group #15093
Conversation
Codecov Report
@@ Coverage Diff @@
## main #15093 +/- ##
==========================================
+ Coverage 52.60% 54.02% +1.41%
==========================================
Files 1610 1472 -138
Lines 80573 76765 -3808
Branches 6958 5513 -1445
==========================================
- Hits 42389 41472 -917
+ Misses 34357 31509 -2848
+ Partials 3827 3784 -43
*This pull request uses carry forward flags. Click here to find out more.
|
This change adds the "my" repo group which is available for users that are allowed to create their own external services. It will contain all repos that they have added.
09b6587
to
d1a4e25
Compare
Notifying subscribers in CODENOTIFY files for diff 259b02e...8525a2c.
|
internal/db/repos.go
Outdated
func (s *RepoStore) GetRepoNamesByUser(ctx context.Context, userID int32) ([]string, error) { | ||
s.ensureStore() | ||
return basestore.ScanStrings(s.Query(ctx, sqlf.Sprintf(` | ||
SELECT DISTINCT (repo.name) from 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.
Do we need the DISTINCT here?
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.
Yes, they may have added the same repo from more than one external service
} | ||
|
||
// 🚨 SECURITY: This enforces repository permissions | ||
repos, err = authzFilter(ctx, repos, authz.Read) |
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.
qq: Is it possible to utilize/modify s.getReposBySQL
so we only have one place to call authzFilter
which soon will be removed?
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 considered it but the new query need to join over another table and I think it would make s.getReposBySQL
When is authzFilter
being removed? What is the new plan?
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.
When is authzFilter being removed? What is the new plan?
@ryanslade authzFilter
will be removed after #13708 is merged.
I considered it but the new query need to join over another table and I think it would make s.getReposBySQL
I'll see what I can do by the time.
This change adds the "my" repo group which is available for users that
are allowed to create their own external services. It will contain all
repos that they have added.
Closes: #14919