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

Question: expected results of models.GetRepositoryAccesses()? #10068

Closed
guillep2k opened this issue Jan 30, 2020 · 6 comments
Closed

Question: expected results of models.GetRepositoryAccesses()? #10068

guillep2k opened this issue Jan 30, 2020 · 6 comments

Comments

@guillep2k
Copy link
Member

  • Gitea version (or commit ref): 04cbdf5 (current master)

While working on #9787, I need to rewrite models.GetRepositoryAccesses(). This function is only used by ListMyRepos() as a complement of GetUserRepositories(). ListMyRepos() works the API function:

// swagger:operation GET /user/repos user userCurrentListRepos

GetRepositoryAccesses() is supposed to return all the repositories that the user has access to but does not own. The problem is that the unit test goes like:

func TestUser_GetRepositoryAccesses(t *testing.T) {
assert.NoError(t, PrepareTestDatabase())
user1 := AssertExistsAndLoadBean(t, &User{ID: 1}).(*User)
accesses, err := user1.GetRepositoryAccesses()
assert.NoError(t, err)
assert.Len(t, accesses, 0)

With the current data, this function should fail because user1 is a site administrator, so it's got access to all repositories. The test asserts that the function returns zero results.

My question is: is the unit test/fixture set wrong, or is it that the function is expected to ignore the is_admin flag to return a shorter list?

@lunny
Copy link
Member

lunny commented Jan 30, 2020

I think maybe the function GetRepositoryAccesses is not expected.

I think we should refactor all SearchRepository functions.

@zeripath
Copy link
Contributor

It's an old broken function that I wanted to remove when I fixed search repositories but realised I couldn't as it is part of the API. It should be deprecated.

@guillep2k
Copy link
Member Author

Thanks! So, what's the API expected to do (with some detail)? Maybe it's easier for me to rewrite the endpoint rather than it parts? Where is it used?
I'm afraid that a slow deprecation spanning a couple of versions is not an option from me, since I'm removing the access table: I either get it right or I remove it.

@zeripath
Copy link
Contributor

Ah I see. I think it literally listed if a user was a collaborator or not.

@guillep2k
Copy link
Member Author

Thanks! So it deals with explicit accesses. 👍

@guillep2k
Copy link
Member Author

I think we should refactor all SearchRepository functions.

@lunny I'm on it (#9787)

@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants