Verified Commit 1860eef2 authored by Zubeen's avatar Zubeen 🤖 Committed by GitLab
Browse files

fix(groups): avoid panic on nil options and empty avatar in CreateGroup/UpdateGroup

Changelog: Improvements
parent 23eb0a84
Loading
Loading
Loading
Loading
+2 −2
Changes for groups.go: 2 added lines, 2 removed lines.
Original line number Diff line number Diff line
@@ -720,7 +720,7 @@ func (s *GroupsService) CreateGroup(opt *CreateGroupOptions, options ...RequestO
		withAPIOpts(opt),
		withRequestOpts(options...),
	}
	if opt.Avatar != nil {
	if opt != nil && opt.Avatar != nil && (opt.Avatar.Filename != "" || opt.Avatar.Image != nil) {
		if opt.DefaultBranchProtectionDefaults != nil && (len(opt.DefaultBranchProtectionDefaults.AllowedToMerge) > 1 || len(opt.DefaultBranchProtectionDefaults.AllowedToPush) > 1) {
			return nil, nil, errors.New("multiple access levels for allowed_to_merge or allowed_to_push are not permitted when an Avatar is also specified as it will result in unexpected behavior")
		}
@@ -825,7 +825,7 @@ func (s *GroupsService) UpdateGroup(gid any, opt *UpdateGroupOptions, options ..
		withAPIOpts(opt),
		withRequestOpts(options...),
	}
	if opt.Avatar != nil && (opt.Avatar.Filename != "" || opt.Avatar.Image != nil) {
	if opt != nil && opt.Avatar != nil && (opt.Avatar.Filename != "" || opt.Avatar.Image != nil) {
		if opt.DefaultBranchProtectionDefaults != nil && (len(opt.DefaultBranchProtectionDefaults.AllowedToMerge) > 1 || len(opt.DefaultBranchProtectionDefaults.AllowedToPush) > 1) {
			return nil, nil, errors.New("multiple access levels for allowed_to_merge or allowed_to_push are not permitted when an Avatar is also specified as it will result in unexpected behavior")
		}
+60 −0
Changes for groups_test.go: 60 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -402,6 +402,47 @@ func TestCreateGroup(t *testing.T) {
	assert.Equal(t, want, group)
}

func TestCreateGroup_NilOptions(t *testing.T) {
	t.Parallel()
	mux, client := setup(t)

	mux.HandleFunc("/api/v4/groups",
		func(w http.ResponseWriter, r *http.Request) {
			testMethod(t, r, http.MethodPost)
			fmt.Fprint(w, `{"id": 1}`)
		})

	// GIVEN nil options
	// WHEN creating a group
	group, _, err := client.Groups.CreateGroup(nil)

	// THEN no panic and the request is sent
	require.NoError(t, err)
	assert.Equal(t, &Group{ID: 1}, group)
}

func TestCreateGroup_EmptyAvatar(t *testing.T) {
	t.Parallel()
	mux, client := setup(t)

	mux.HandleFunc("/api/v4/groups",
		func(w http.ResponseWriter, r *http.Request) {
			testMethod(t, r, http.MethodPost)
			assert.NotContains(t, r.Header.Get("Content-Type"), "multipart")
			fmt.Fprint(w, `{"id": 1}`)
		})

	// GIVEN an avatar without image or filename
	opt := &CreateGroupOptions{Name: new("g"), Path: new("g"), Avatar: &GroupAvatar{}}

	// WHEN creating a group
	group, _, err := client.Groups.CreateGroup(opt)

	// THEN no panic and no upload is attempted
	require.NoError(t, err)
	assert.Equal(t, &Group{ID: 1}, group)
}

func TestCreateGroup_FullAttributes(t *testing.T) {
	t.Parallel()
	mux, client := setup(t)
@@ -743,6 +784,25 @@ func TestUpdateGroup(t *testing.T) {
	assert.Equal(t, want, group)
}

func TestUpdateGroup_NilOptions(t *testing.T) {
	t.Parallel()
	mux, client := setup(t)

	mux.HandleFunc("/api/v4/groups/1",
		func(w http.ResponseWriter, r *http.Request) {
			testMethod(t, r, http.MethodPut)
			fmt.Fprint(w, `{"id": 1}`)
		})

	// GIVEN nil options
	// WHEN updating a group
	group, _, err := client.Groups.UpdateGroup(1, nil)

	// THEN no panic and the request is sent
	require.NoError(t, err)
	assert.Equal(t, &Group{ID: 1}, group)
}

func TestUpdateGroupWithDefaultBranch(t *testing.T) {
	t.Parallel()
	mux, client := setup(t)