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

Visibility field is always empty #2002

Open
janderssonse opened this issue Aug 31, 2024 · 4 comments
Open

Visibility field is always empty #2002

janderssonse opened this issue Aug 31, 2024 · 4 comments

Comments

@janderssonse
Copy link

Hello and thanks for all work with the library! I think a found a bug -> the visibility field is always empty. Tried the library on a few repositories and this seems consistent. Here is an example to illustrate:

package main

import (
	"fmt"

	"github.com/xanzy/go-gitlab"
)

func main() {
	git, _:= gitlab.NewClient("")

	gitlabProject,_, _ := git.Projects.GetProject("hanklank"+"/"+"quark-demo-api", nil)
	fmt.Println(gitlabProject)
}

image

@svanharmelen
Copy link
Member

I would suggest to first determine if this is a "bug" of this package or a "feature" of the GitLab API 😉 Probably making the same API call using cURL and comparing the response will tell us which one it is.

@heidiberry
Copy link
Contributor

The response from the GitLab API depends on whether the request is authenticated and what permissions the authenticated user has. In the case of the above, if you provide no token to the request, most of the fields listed in the API are not returned, including visibility:

{
    "id": 38582100,
    "description": null,
    "name": "quark-demo-api",
    "name_with_namespace": "Josef Andersson / quark-demo-api",
    "path": "quark-demo-api",
    "path_with_namespace": "hanklank/quark-demo-api",
    "created_at": "2022-08-14T20:27:24.727Z",
    "default_branch": "main",
    "tag_list": [],
    "topics": [],
    "ssh_url_to_repo": "[email protected]:hanklank/quark-demo-api.git",
    "http_url_to_repo": "https://gitlab.com/hanklank/quark-demo-api.git",
    "web_url": "https://gitlab.com/hanklank/quark-demo-api",
    "readme_url": "https://gitlab.com/hanklank/quark-demo-api/-/blob/main/README.md",
    "forks_count": 0,
    "avatar_url": null,
    "star_count": 0,
    "last_activity_at": "2024-08-29T15:21:23.341Z",
    "namespace": {
        "id": 888681,
        "name": "Josef Andersson",
        "path": "hanklank",
        "kind": "user",
        "full_path": "hanklank",
        "parent_id": null,
        "avatar_url": "https://secure.gravatar.com/avatar/f1b08a3f2bc8cec7323d0e550d586a5a626c4cddd676887729de7717251c3736?s=80\u0026d=identicon",
        "web_url": "https://gitlab.com/hanklank"
    }
}

This results in the missing fields in the resulting Project object getting default values like an empty string or false for booleans. This is confusing as you wouldn't know if that was the real value or a consequence of it being missing from the response.

To make it clearer, the function could return a different version of the Project struct depending on the authentication status of the request. Or documentation could be added to explain the behaviour.

@svanharmelen
Copy link
Member

Returning a different struct based on that kind of conditions is both hard (how to determine those conditions in this package) and not possible (in Go I don't see how I can return two different structs as these are methods so generics are not supported).

I think the only "real" solution would be to make all fields pointers, just like in the option structs. But that forces everyone to do nil checks continuously on each and every field someone want to access which I don't think is a very nice or ergonomic user experience.

So if this is inline with the results of the API (so it's not a bug) I'm not really open to change this for now.

@janderssonse
Copy link
Author

janderssonse commented Sep 8, 2024

Returning a different struct based on that kind of conditions is both hard (how to determine those conditions in this package) and not possible (in Go I don't see how I can return two different structs as these are methods so generics are not supported).

I think the only "real" solution would be to make all fields pointers, just like in the option structs. But that forces everyone to do nil checks continuously on each and every field someone want to access which I don't think is a very nice or ergonomic user experience.

Either way everyone will have to make check if there is a value or not. So the it does not matter much if it is pointers or not, even if one could argue that nil-values would be more natural (or not - it's arguable). But for example, I was expecting the Visibility field to always be present for public projects, with a value of "public", regardless of token given or not.

So if this is inline with the results of the API (so it's not a bug) I'm not really open to change this for now.
Understandable. One small but easy improvement would be if a line was added to the documentation about the behaviour @heidiberry describs, as to not catch any other users by surprise.

Thanks to @heidiberry for the explaination and to @svanharmelen for the ongoing work. I suggest this issue could be closed if the behaviour was documented, as to help future users getting it right and avoid confusion, almost a one liner should be enough.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants