Browse Source

ref(ui): Improve member loading in actor avatar (#69002)

Right now if the member isn't present in the store when the avatar
renders we'll never end up showing the users avatar
Evan Purkhiser 11 months ago
parent
commit
92e8a0c6f5

+ 28 - 0
static/app/components/avatar/actorAvatar.spec.tsx

@@ -96,4 +96,32 @@ describe('ActorAvatar', function () {
     expect(await screen.findByText('CT')).toBeInTheDocument();
     expect(mockRequest).toHaveBeenCalled();
   });
+
+  it('should fetch a user not in the store', async function () {
+    const organization = OrganizationFixture();
+
+    OrganizationStore.onUpdate(organization, {replace: true});
+
+    const user2 = UserFixture({id: '2', name: 'COOL USER'});
+
+    const mockRequest = MockApiClient.addMockResponse({
+      url: `/organizations/${organization.slug}/members/`,
+      method: 'GET',
+      body: [{user: user2}],
+    });
+
+    render(
+      <ActorAvatar
+        actor={{
+          id: user2.id,
+          name: user2.name,
+          email: user2.email,
+          type: 'user',
+        }}
+      />
+    );
+
+    expect(await screen.findByText('CU')).toBeInTheDocument();
+    expect(mockRequest).toHaveBeenCalled();
+  });
 });

+ 25 - 5
static/app/components/avatar/actorAvatar.tsx

@@ -1,10 +1,11 @@
+import {useMemo} from 'react';
 import * as Sentry from '@sentry/react';
 
 import TeamAvatar from 'sentry/components/avatar/teamAvatar';
 import UserAvatar from 'sentry/components/avatar/userAvatar';
-import LoadingIndicator from 'sentry/components/loadingIndicator';
-import MemberListStore from 'sentry/stores/memberListStore';
+import Placeholder from 'sentry/components/placeholder';
 import type {Actor} from 'sentry/types';
+import {useMembers} from 'sentry/utils/useMembers';
 import {useTeamsById} from 'sentry/utils/useTeamsById';
 
 import type {BaseAvatarProps} from './baseAvatar';
@@ -24,12 +25,32 @@ function LoadTeamAvatar({
   const team = teams.find(t => t.id === teamId);
 
   if (isLoading) {
-    return <LoadingIndicator mini />;
+    const size = `${props.size}px`;
+    return <Placeholder width={size} height={size} />;
   }
 
   return <TeamAvatar team={team} {...props} />;
 }
 
+/**
+ * Wrapper to assist loading the user from api or store
+ */
+function LoadMemberAvatar({
+  userId,
+  ...props
+}: {userId: string} & Omit<React.ComponentProps<typeof UserAvatar>, 'team'>) {
+  const ids = useMemo(() => [userId], [userId]);
+  const {members, fetching} = useMembers({ids});
+  const user = members.find(u => u.id === userId);
+
+  if (fetching) {
+    const size = `${props.size}px`;
+    return <Placeholder shape="circle" width={size} height={size} />;
+  }
+
+  return <UserAvatar user={user} {...props} />;
+}
+
 function ActorAvatar({size = 24, hasTooltip = true, actor, ...props}: Props) {
   const otherProps = {
     size,
@@ -38,8 +59,7 @@ function ActorAvatar({size = 24, hasTooltip = true, actor, ...props}: Props) {
   };
 
   if (actor.type === 'user') {
-    const user = actor.id ? MemberListStore.getById(actor.id) ?? actor : actor;
-    return <UserAvatar user={user} {...otherProps} />;
+    return <LoadMemberAvatar userId={actor.id} {...otherProps} />;
   }
 
   if (actor.type === 'team') {

+ 14 - 6
static/app/components/avatar/index.spec.tsx

@@ -3,11 +3,13 @@ import {OrganizationFixture} from 'sentry-fixture/organization';
 import {ProjectFixture} from 'sentry-fixture/project';
 import {SentryAppFixture} from 'sentry-fixture/sentryApp';
 import {TeamFixture} from 'sentry-fixture/team';
+import {UserFixture} from 'sentry-fixture/user';
 
 import {render, screen} from 'sentry-test/reactTestingLibrary';
 
 import AvatarComponent from 'sentry/components/avatar';
 import ConfigStore from 'sentry/stores/configStore';
+import MemberListStore from 'sentry/stores/memberListStore';
 import type {Avatar} from 'sentry/types';
 
 describe('Avatar', function () {
@@ -17,14 +19,14 @@ describe('Avatar', function () {
     avatarUrl: 'https://sentry.io/avatar/2d641b5d-8c74-44de-9cb6-fbd54701b35e/',
   };
 
-  const user = {
+  const user = UserFixture({
     id: '1',
     name: 'Jane Bloggs',
     username: 'janebloggs@example.com',
     email: 'janebloggs@example.com',
     ip_address: '127.0.0.1',
     avatar,
-  };
+  });
 
   window.__initialData = {
     links: {sentryUrl: 'https://sentry.io'},
@@ -121,7 +123,12 @@ describe('Avatar', function () {
     });
 
     it('should show a gravatar when no avatar type is set and user has an email address', async function () {
-      render(<AvatarComponent gravatar user={{...user, avatar: undefined}} />);
+      render(
+        <AvatarComponent
+          gravatar
+          user={{...user, options: undefined, avatar: undefined}}
+        />
+      );
       await screen.findByRole('img');
 
       const avatarElement = screen.getByTestId(`gravatar-avatar`);
@@ -175,13 +182,14 @@ describe('Avatar', function () {
       );
     });
 
-    it('can display a actor Avatar', function () {
+    it('can display a actor Avatar', async function () {
       const actor = ActorFixture();
+      MemberListStore.loadInitialData([user]);
 
       render(<AvatarComponent actor={actor} />);
 
-      expect(screen.getByTestId(`letter_avatar-avatar`)).toBeInTheDocument();
-      expect(screen.getByText('FB')).toBeInTheDocument();
+      expect(await screen.findByTestId(`letter_avatar-avatar`)).toBeInTheDocument();
+      expect(screen.getByText('JB')).toBeInTheDocument();
     });
 
     it('displays platform list icons for project Avatar', function () {

+ 11 - 9
static/app/views/dashboards/widgetCard/issueWidgetCard.spec.tsx

@@ -1,4 +1,5 @@
 import {OrganizationFixture} from 'sentry-fixture/organization';
+import {UserFixture} from 'sentry-fixture/user';
 
 import {initializeOrg} from 'sentry-test/initializeOrg';
 import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary';
@@ -44,6 +45,7 @@ describe('Dashboards > IssueWidgetCard', function () {
     },
   };
 
+  const user = UserFixture();
   const api = new MockApiClient();
 
   beforeEach(function () {
@@ -56,9 +58,9 @@ describe('Dashboards > IssueWidgetCard', function () {
           shortId: 'ISSUE',
           assignedTo: {
             type: 'user',
-            id: '2222222',
-            name: 'dashboard user',
-            email: 'dashboarduser@sentry.io',
+            id: user.id,
+            name: user.name,
+            email: user.email,
           },
           lifetime: {count: 10, userCount: 5},
           count: 6,
@@ -70,7 +72,7 @@ describe('Dashboards > IssueWidgetCard', function () {
     MockApiClient.addMockResponse({
       url: '/organizations/org-slug/users/',
       method: 'GET',
-      body: [],
+      body: [user],
     });
   });
 
@@ -79,7 +81,7 @@ describe('Dashboards > IssueWidgetCard', function () {
   });
 
   it('renders with title and issues chart', async function () {
-    MemberListStore.loadInitialData([]);
+    MemberListStore.loadInitialData([user]);
     render(
       <WidgetCard
         api={api}
@@ -100,13 +102,13 @@ describe('Dashboards > IssueWidgetCard', function () {
     expect(await screen.findByText('assignee')).toBeInTheDocument();
     expect(screen.getByText('title')).toBeInTheDocument();
     expect(screen.getByText('issue')).toBeInTheDocument();
-    expect(screen.getByText('DU')).toBeInTheDocument();
+    expect(screen.getByText('FB')).toBeInTheDocument();
     expect(screen.getByText('ISSUE')).toBeInTheDocument();
     expect(
       screen.getByText('ChunkLoadError: Loading chunk app_bootstrap_index_tsx failed.')
     ).toBeInTheDocument();
-    await userEvent.hover(screen.getByTitle('dashboard user'));
-    expect(await screen.findByText('Assigned to dashboard user')).toBeInTheDocument();
+    await userEvent.hover(screen.getByTitle(user.name));
+    expect(await screen.findByText(`Assigned to ${user.name}`)).toBeInTheDocument();
   });
 
   it('opens in issues page', async function () {
@@ -186,7 +188,7 @@ describe('Dashboards > IssueWidgetCard', function () {
   });
 
   it('maps lifetimeEvents and lifetimeUsers headers to more human readable', async function () {
-    MemberListStore.loadInitialData([]);
+    MemberListStore.loadInitialData([user]);
     render(
       <WidgetCard
         api={api}