[server] For GitLab projects without an owner avatar, fall back to the namespace avatar, or generate the default GitLab avatar #8824

jankeromnes · 2022-03-15T12:51:34Z

Description

For GitLab projects belonging to a group, it seems that there is no owner.avatar_url to be found here:

gitpod/components/server/ee/src/gitlab/gitlab-app-support.ts

Line 46 in fc68e47

const accountAvatarUrl = anyProject.owner?.avatar_url as string;

Example group project from GitLab's API:

{
  "id": 23116976,
  "description": "",
  "name": "sample-project",
  "name_with_namespace": "reproduction-group / sample-project",
  /* ... (no "owner") ... */
  "namespace": {
    "id": 10408914,
    "name": "reproduction-group",
    /* ... */
    "avatar_url": "/uploads/-/system/group/avatar/10408914/gitpod-ua.png",
    "web_url": "https://gitlab.com/groups/reproduction-group"
  },
  /* ... */
}

As you can see, there is no "owner", but there is an avatar URL on the "namespace" (although it's relative).

When there is no owner avatar URL, this Pull Request falls back to the namespace avatar URL if available.

EDIT: When no avatar is available, it generates a SVG avatar that looks like GitLab's own default group avatar.

Related Issue(s)

Fixes #5330

How to test

Connect with GitLab
Be part of a group which has an avatar (e.g. https://gitlab.com/reproduction-group)
Go to /new -- the GitLab group should have the set avatar visible

Release Notes

[server] For GitLab projects without an owner avatar, fall back to the namespace avatar, or generate the default GitLab avatar

Documentation

jankeromnes · 2022-03-15T12:53:49Z

Note: When there is no avatar, it would be nice to use the same default avatar as GitLab (i.e. a big letter with a pastel background). However, I couldn't find how to generate this, so for now I left the avatar URL empty when there is no avatar.

components/server/ee/src/gitlab/gitlab-app-support.ts

gtsiolis · 2022-03-15T15:06:35Z

Looking at this now! 👀

gtsiolis

Thanks for picking this up, @jankeromnes!

Commented below about two outstanding issues related to UX here.

components/server/ee/src/gitlab/gitlab-app-support.ts

gtsiolis · 2022-03-15T15:16:10Z

components/server/ee/src/gitlab/gitlab-app-support.ts

+            return anyProject.owner.avatar_url;
+        }
+        if (anyProject.namespace?.avatar_url) {
+            let url = anyProject.namespace.avatar_url;


issue: Not sure if this is the correct line to comment this but the avatar seems broken now while before it was just a white blank space, see screenshots below. But maybe this is an improvement step towards resolving this issue?

fyi: The one before the last one below is the gitlab-org group which has an avatar.

BEFORE AFTER

Ah, thanks for catching this @gtsiolis. If you inspect the broken icons, what does the avatar URL look like?

@jankeromnes for all of the ones with a missing avatar, including the gitlab-org group which has an avatar, I see the following in dev tools:

praise: Thanks for fixing this, work like a charm. ✨

The groups with custom avatars still seems broken. What do you think of opening a follow up issue for this to keep track of this?

I wonder what's special about these groups 🤔 needs more debugging (e.g. maybe you can invite me to one of these, or I can create another Pull Request with more debug logs and ask you to reproduce in there).

Filed follow-up issue: #9112

It seems to be working ok now for all but one of the groups in there, posted a comment in #9112 (comment).

jankeromnes · 2022-03-15T18:04:59Z

Taking this back to draft as there is some work left to do.

stale · 2022-03-26T20:24:43Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

jankeromnes · 2022-03-28T17:45:21Z

Ready for another round of reviews! 🚀

I've implemented a default avatar generator similar to GitLab's one (thanks @gtsiolis for linking the relevant code! 🎯) so there should always be an avatar now.

I've also implemented more specific types (instead of using any), based on the official types from our library (gitbeaker) with the additional missing fields.

gtsiolis · 2022-03-28T18:45:13Z

Taking another look now! 👀

gtsiolis

Took another look and left some more comments below. Thanks, @jankeromnes! 🏀

components/server/ee/src/gitlab/gitlab-app-support.ts

gtsiolis · 2022-03-28T18:42:38Z

components/server/ee/src/gitlab/gitlab-app-support.ts

+        // - https://gitlab.com/gitlab-org/gitlab-foss/-/blob/84b4743475246e91dc78c3f25f9b335c40be84cd/app/assets/stylesheets/startup/startup-general.scss#L420-422
+        const text = owner.name[0].toUpperCase();
+        const backgroundColor = ["#fcf1ef", "#f4f0ff", "#f1f1ff", "#e9f3fc", "#ecf4ee", "#fdf1dd", "#f0f0f0"][
+            owner.id % 7


praise: It so much fun to see the same logic here. Good memories! 🦊

components/server/ee/src/gitlab/gitlab-app-support.ts

jankeromnes · 2022-03-31T06:30:11Z

Briefly taking this back to Draft for some quick debugging.

jankeromnes · 2022-03-31T12:13:45Z

Phew. Okay now this should be ready to rock'n'roll. 🎸😁

@gtsiolis please take another look when you have some time. 🙏

(For what it's worth, I initially estimated this work as a 0.25 day effort / 1 iteration -- looks like I was about 10x off on both. 😅)

easyCZ

Code-wise looks good to me. Adding hold to resolve remaining questions.

/hold

components/server/ee/src/gitlab/gitlab-app-support.ts

easyCZ · 2022-04-04T08:10:26Z

components/dashboard/src/projects/NewProject.tsx

@@ -237,7 +237,11 @@ export default function NewProject() {

    const accounts = new Map<string, { avatarUrl: string }>();
    reposInAccounts.forEach((r) => {
-        if (!accounts.has(r.account)) accounts.set(r.account, { avatarUrl: r.accountAvatarUrl });
+        if (!accounts.has(r.account)) {


👍 Much nicer on the eyes

gtsiolis · 2022-04-04T10:36:39Z

Taking another look at this now! 👀

gtsiolis

LGTM, @jankeromnes! 🔮

gtsiolis · 2022-04-04T10:47:33Z

components/server/ee/src/gitlab/gitlab-app-support.ts

+        const backgroundColor = ["#fcf1ef", "#f4f0ff", "#f1f1ff", "#e9f3fc", "#ecf4ee", "#fdf1dd", "#f0f0f0"][
+            owner.id % 7
+        ];
+        // Uppercase first character of the name, support emojis, default to whitespace.


praise: Lovely comment! 💯

components/server/ee/src/gitlab/gitlab-app-support.ts

…e top-level group avatar, or generate the default GitLab avatar

jankeromnes · 2022-04-04T16:44:51Z

Nits addressed & follow-ups filed! 🎉

@easyCZ or @gtsiolis could you please re-approve?

/unhold

gtsiolis

Works like a charm, @jankeromnes! 🔮

Thanks for also opening the follow up issue in #8824 (comment) to track one potentially minor edge case. 💯

Approving for UX, but you'll still need an approval from someone from the WebApp team.

Waiting on code owner review from gitpod-io/engineering-webapp.

jankeromnes requested a review from a team March 15, 2022 12:51

roboquat added release-note size/S labels Mar 15, 2022

github-actions bot added the team: webapp Issue belongs to the WebApp team label Mar 15, 2022

jankeromnes requested a review from gtsiolis March 15, 2022 13:03

andrew-farries reviewed Mar 15, 2022

View reviewed changes

components/server/ee/src/gitlab/gitlab-app-support.ts Outdated Show resolved Hide resolved

gtsiolis reviewed Mar 15, 2022

View reviewed changes

jankeromnes marked this pull request as draft March 15, 2022 18:05

roboquat added the do-not-merge/work-in-progress label Mar 15, 2022

stale bot added the meta: stale This issue/PR is stale and will be closed soon label Mar 26, 2022

gtsiolis removed the meta: stale This issue/PR is stale and will be closed soon label Mar 28, 2022

jankeromnes force-pushed the jx/fix-gitlab-group-avatars branch from 2e92b16 to 84c3c59 Compare March 28, 2022 14:03

roboquat added size/M and removed size/S labels Mar 28, 2022

jankeromnes force-pushed the jx/fix-gitlab-group-avatars branch from 84c3c59 to 7e5c93e Compare March 28, 2022 17:07

jankeromnes changed the title ~~[server] For GitLab projects without an owner avatar, fall back to the namespace avatar~~ [server] For GitLab projects without an owner avatar, fall back to the namespace avatar, or generate the default GitLab avatar Mar 28, 2022

jankeromnes force-pushed the jx/fix-gitlab-group-avatars branch 2 times, most recently from 4eb07ee to 25f5c1c Compare March 28, 2022 17:42

jankeromnes marked this pull request as ready for review March 28, 2022 17:43

roboquat removed the do-not-merge/work-in-progress label Mar 28, 2022

jankeromnes requested review from gtsiolis and a team March 28, 2022 17:45

gtsiolis reviewed Mar 28, 2022

View reviewed changes

components/server/ee/src/gitlab/gitlab-app-support.ts Outdated Show resolved Hide resolved

jankeromnes force-pushed the jx/fix-gitlab-group-avatars branch from 25f5c1c to ab4af90 Compare March 30, 2022 14:42

jankeromnes requested a review from gtsiolis March 30, 2022 14:58

jankeromnes marked this pull request as draft March 31, 2022 06:30

roboquat added the do-not-merge/work-in-progress label Mar 31, 2022

jankeromnes force-pushed the jx/fix-gitlab-group-avatars branch 2 times, most recently from 17db447 to 55d9ce7 Compare March 31, 2022 09:54

jankeromnes marked this pull request as ready for review March 31, 2022 12:14

roboquat removed the do-not-merge/work-in-progress label Mar 31, 2022

easyCZ previously approved these changes Apr 4, 2022

View reviewed changes

roboquat added the do-not-merge/hold label Apr 4, 2022

gtsiolis previously approved these changes Apr 4, 2022

View reviewed changes

[server] For GitLab projects without an owner avatar, fall back to th…

075997a

…e top-level group avatar, or generate the default GitLab avatar

jankeromnes dismissed stale reviews from gtsiolis and easyCZ via 075997a April 4, 2022 16:19

jankeromnes force-pushed the jx/fix-gitlab-group-avatars branch from 55d9ce7 to 075997a Compare April 4, 2022 16:19

jankeromnes mentioned this pull request Apr 4, 2022

[New Project] The avatar of some GitLab groups seems broken #9112

Open

roboquat removed the do-not-merge/hold label Apr 4, 2022

gtsiolis approved these changes Apr 4, 2022

View reviewed changes

jankeromnes requested review from easyCZ and a team April 4, 2022 17:04

andrew-farries approved these changes Apr 4, 2022

View reviewed changes

roboquat merged commit 1d52870 into main Apr 4, 2022

roboquat deleted the jx/fix-gitlab-group-avatars branch April 4, 2022 17:24

roboquat added deployed: webapp Meta team change is running in production deployed Change is completely running in production labels Apr 5, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[server] For GitLab projects without an owner avatar, fall back to the namespace avatar, or generate the default GitLab avatar #8824

[server] For GitLab projects without an owner avatar, fall back to the namespace avatar, or generate the default GitLab avatar #8824

jankeromnes commented Mar 15, 2022 •

edited

Loading

jankeromnes commented Mar 15, 2022

gtsiolis commented Mar 15, 2022 •

edited by werft-gitpod-dev-com bot

Loading

gtsiolis left a comment

gtsiolis Mar 15, 2022 •

edited

Loading

jankeromnes Mar 15, 2022

gtsiolis Mar 15, 2022

gtsiolis Mar 28, 2022

gtsiolis Apr 4, 2022

jankeromnes Apr 4, 2022

gtsiolis Apr 4, 2022

jankeromnes commented Mar 15, 2022

stale bot commented Mar 26, 2022

jankeromnes commented Mar 28, 2022 •

edited by werft-gitpod-dev-com bot

Loading

gtsiolis commented Mar 28, 2022 •

edited by werft-gitpod-dev-com bot

Loading

gtsiolis left a comment

gtsiolis Mar 28, 2022

jankeromnes commented Mar 31, 2022

jankeromnes commented Mar 31, 2022 •

edited

Loading

easyCZ left a comment

easyCZ Apr 4, 2022

gtsiolis commented Apr 4, 2022 •

edited by werft-gitpod-dev-com bot

Loading

gtsiolis left a comment

gtsiolis Apr 4, 2022

jankeromnes commented Apr 4, 2022 •

edited by werft-gitpod-dev-com bot

Loading

gtsiolis left a comment

[server] For GitLab projects without an owner avatar, fall back to the namespace avatar, or generate the default GitLab avatar #8824

[server] For GitLab projects without an owner avatar, fall back to the namespace avatar, or generate the default GitLab avatar #8824

Conversation

jankeromnes commented Mar 15, 2022 • edited Loading

Description

Related Issue(s)

How to test

Release Notes

Documentation

jankeromnes commented Mar 15, 2022

gtsiolis commented Mar 15, 2022 • edited by werft-gitpod-dev-com bot Loading

gtsiolis left a comment

Choose a reason for hiding this comment

gtsiolis Mar 15, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jankeromnes commented Mar 15, 2022

stale bot commented Mar 26, 2022

jankeromnes commented Mar 28, 2022 • edited by werft-gitpod-dev-com bot Loading

gtsiolis commented Mar 28, 2022 • edited by werft-gitpod-dev-com bot Loading

gtsiolis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jankeromnes commented Mar 31, 2022

jankeromnes commented Mar 31, 2022 • edited Loading

easyCZ left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gtsiolis commented Apr 4, 2022 • edited by werft-gitpod-dev-com bot Loading

gtsiolis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jankeromnes commented Apr 4, 2022 • edited by werft-gitpod-dev-com bot Loading

gtsiolis left a comment

Choose a reason for hiding this comment

jankeromnes commented Mar 15, 2022 •

edited

Loading

gtsiolis commented Mar 15, 2022 •

edited by werft-gitpod-dev-com bot

Loading

gtsiolis Mar 15, 2022 •

edited

Loading

jankeromnes commented Mar 28, 2022 •

edited by werft-gitpod-dev-com bot

Loading

gtsiolis commented Mar 28, 2022 •

edited by werft-gitpod-dev-com bot

Loading

jankeromnes commented Mar 31, 2022 •

edited

Loading

gtsiolis commented Apr 4, 2022 •

edited by werft-gitpod-dev-com bot

Loading

jankeromnes commented Apr 4, 2022 •

edited by werft-gitpod-dev-com bot

Loading