Skip to content
This repository has been archived by the owner on Dec 10, 2024. It is now read-only.

Add Users.UploadAvatar() and add Avatar to UserOptions #1965

Merged
merged 5 commits into from
Jul 4, 2024

Conversation

andreaswwilson
Copy link
Contributor

First time committing to external github-repo, so I hope this is the correct way.
This fixes #1871
I have not added feature for downloading avatar since this is not part of the gitlab api.

Copy link
Member

@svanharmelen svanharmelen left a comment

Choose a reason for hiding this comment

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

Thanks for your PR!

I have a small cosmetic request (see inline comment), but additionally I wonder how this is expected to work.

In other APIs that have to option to set an avatar we also have to upload the avatar in that same request (see this example).

Is that different for the user create and update APIs?

users.go Outdated Show resolved Hide resolved
users.go Outdated Show resolved Hide resolved
@andreaswwilson
Copy link
Contributor Author

Thank you for your feedback and for pointing me in the correct direction.
I have updated CreateUser and ModifyUser to follow the existing pattern used for groups and projects.
Since i am not an administrator for any gitlab instance i have not been able to test the modified methods on a real instance.

@svanharmelen
Copy link
Member

While the PR now looks good, I do prefer this is checked/tested before we merge it. I also don't have access to a GitLab instance (believe it or not 😏), but maybe @PatrickRice-KSC or @timofurrer can help out and give it a quick check/test?

@RicePatrick
Copy link
Contributor

@svanharmelen - Sure, I'll give it a test! Also, do you want me to get you a contributors license for GitLab? I'm happy to help with that 😆

@RicePatrick
Copy link
Contributor

I created 3 different tests for this PR, testing:

  1. Uploading an avatar to a current user
  2. Modifying an existing user to have a new avatar
  3. Creating a user with an avatar

All 3 tests passed properly :)

Here is the code I used:

func TestUserAvatar(t *testing.T) {
	client, err := NewClient("glpat-ACCTEST1234567890123", WithBaseURL("http://localhost:8085/api/v4"))
	if err != nil {
		t.Fatal(err)
	}

	avatar, err := os.OpenFile("testdata/avatar.png", os.O_RDONLY, 0644)
	if err != nil {
		t.Fatal(err)
	}

	user, resp, err := client.Users.UploadAvatar(avatar, "avatar.png")
	assert.Nil(t, err)
	assert.Equal(t, 200, resp.StatusCode)
	assert.NotNil(t, user.AvatarURL)
}

func TestModifyUserAvatar(t *testing.T) {
	client, err := NewClient("glpat-ACCTEST1234567890123", WithBaseURL("http://localhost:8085/api/v4"))
	if err != nil {
		t.Fatal(err)
	}

	avatar, err := os.OpenFile("testdata/avatar.png", os.O_RDONLY, 0644)
	if err != nil {
		t.Fatal(err)
	}

	userAvatar := &UserAvatar{
		Image:    avatar,
		Filename: "testdata/avatar.png",
	}

	user, resp, err := client.Users.ModifyUser(1, &ModifyUserOptions{
		Avatar: userAvatar,
	})

	assert.Nil(t, err)
	assert.Equal(t, 200, resp.StatusCode)
	assert.NotNil(t, user.AvatarURL)
}

func TestCreateUserAvatar(t *testing.T) {
	client, err := NewClient("glpat-ACCTEST1234567890123", WithBaseURL("http://localhost:8085/api/v4"))
	if err != nil {
		t.Fatal(err)
	}

	avatar, err := os.OpenFile("testdata/avatar.png", os.O_RDONLY, 0644)
	if err != nil {
		t.Fatal(err)
	}

	userAvatar := &UserAvatar{
		Image:    avatar,
		Filename: "testdata/avatar.png",
	}

	user, resp, err := client.Users.CreateUser(&CreateUserOptions{
		Email:               Ptr("[email protected]"),
		Name:                Ptr("PetTheKitty"),
		Username:            Ptr("PetTheKitty"),
		ForceRandomPassword: Ptr(true),
		Avatar:              userAvatar,
	})

	assert.Nil(t, err)
	assert.Equal(t, 201, resp.StatusCode)
	assert.NotNil(t, user.AvatarURL)
}

users_test.go Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

@andreaswwilson | @svanharmelen - Should we have 2 additional unit tests here, one for ModifyUser and one for CreateUser to tests out the updated if statements in those functions? 🤔

Copy link
Member

Choose a reason for hiding this comment

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

Would probably be a good idea 👍🏻

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added some tests. What do you think about them?

@svanharmelen
Copy link
Member

Thanks you so much for your (quick) help @PatrickRice-KSC! Really appreciated it ❤️

Copy link
Member

@svanharmelen svanharmelen left a comment

Choose a reason for hiding this comment

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

Thanks @andreaswwilson! LGMT 👍🏻

@svanharmelen svanharmelen merged commit f0095cd into xanzy:main Jul 4, 2024
3 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Uploading of user avatar missing in CreateUserOptions and ModifyUserOptions
3 participants