Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion internal/providers/gitlab/properties_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,9 +143,17 @@
properties.RepoPropertyIsPrivate: true,
properties.RepoPropertyIsArchived: false,
properties.RepoPropertyIsFork: false,
properties.RepoPropertyLicense: "mit",

Check failure on line 146 in internal/providers/gitlab/properties_test.go

View workflow job for this annotation

GitHub Actions / lint / Run golangci-lint

undefined: properties.RepoPropertyLicense (typecheck)

Check failure on line 146 in internal/providers/gitlab/properties_test.go

View workflow job for this annotation

GitHub Actions / test / Unit testing

undefined: properties.RepoPropertyLicense

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is just:

Suggested change
properties.RepoPropertyLicense: "mit",
RepoPropertyLicense: "mit",

Since it's from this package, not from the shared properties package.

}),
wantErr: false,
gitLabServerMockFunc: func(w http.ResponseWriter, _ *http.Request) {
gitLabServerMockFunc: func(w http.ResponseWriter, r *http.Request) {
// Verify the query parameter you added is being sent

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same comment here as in a previous review -- will this comment make sense when reading the code in 3 months?

if r.URL.Query().Get("license") != "true" {
t.Errorf("expected query param license=true, got %s", r.URL.RawQuery)
w.WriteHeader(http.StatusBadRequest)
return
}

resp := &gitlab.Project{
ID: 1,
Name: "project-1",
Expand All @@ -156,6 +164,9 @@
Namespace: &gitlab.ProjectNamespace{
Path: "group",
},
License: &gitlab.ProjectLicense{
Name: "mit",
},
}

w.Header().Set("Content-Type", "application/json")
Expand Down
1 change: 1 addition & 0 deletions internal/providers/gitlab/repository_properties.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ func (c *gitlabClient) getGitLabProject(
if err != nil {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ?license=true is appended before the error check — the code appends the query param before checking if url.JoinPath returned an error. If JoinPath fails, projectURLPath would be an empty string and the append would produce just ?license=true. The correct order is:

projectURLPath, err := url.JoinPath("projects", url.PathEscape(upstreamID))
if err != nil {
    return nil, fmt.Errorf("failed to join URL path for project using upstream ID: %w", err)
}
projectURLPath = projectURLPath + "?license=true"

return nil, fmt.Errorf("failed to join URL path for project using upstream ID: %w", err)
}
projectURLPath = projectURLPath + "?license=true"

// NOTE: We're not using github.com/xanzy/go-gitlab to do the actual
// request here because of the way they form authentication for requests.
Expand Down
Loading