Skip to content

Commit fe29dc8

Browse files
committed
ref(ui): Improve member loading in actor avatar
Right now if the member isn't present in the store when the avatar renders we'll never end up showing the users avatar
1 parent 4f24d94 commit fe29dc8

File tree

4 files changed

+78
-20
lines changed

4 files changed

+78
-20
lines changed

static/app/components/avatar/actorAvatar.spec.tsx

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,4 +96,32 @@ describe('ActorAvatar', function () {
9696
expect(await screen.findByText('CT')).toBeInTheDocument();
9797
expect(mockRequest).toHaveBeenCalled();
9898
});
99+
100+
it('should fetch a user not in the store', async function () {
101+
const organization = OrganizationFixture();
102+
103+
OrganizationStore.onUpdate(organization, {replace: true});
104+
105+
const user2 = UserFixture({id: '2', name: 'COOL USER'});
106+
107+
const mockRequest = MockApiClient.addMockResponse({
108+
url: `/organizations/${organization.slug}/members/`,
109+
method: 'GET',
110+
body: [{user: user2}],
111+
});
112+
113+
render(
114+
<ActorAvatar
115+
actor={{
116+
id: user2.id,
117+
name: user2.name,
118+
email: user2.email,
119+
type: 'user',
120+
}}
121+
/>
122+
);
123+
124+
expect(await screen.findByText('CU')).toBeInTheDocument();
125+
expect(mockRequest).toHaveBeenCalled();
126+
});
99127
});

static/app/components/avatar/actorAvatar.tsx

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
1+
import {useMemo} from 'react';
12
import * as Sentry from '@sentry/react';
23

34
import TeamAvatar from 'sentry/components/avatar/teamAvatar';
45
import UserAvatar from 'sentry/components/avatar/userAvatar';
5-
import LoadingIndicator from 'sentry/components/loadingIndicator';
6-
import MemberListStore from 'sentry/stores/memberListStore';
6+
import Placeholder from 'sentry/components/placeholder';
77
import type {Actor} from 'sentry/types';
8+
import {useMembers} from 'sentry/utils/useMembers';
89
import {useTeamsById} from 'sentry/utils/useTeamsById';
910

1011
import type {BaseAvatarProps} from './baseAvatar';
@@ -24,12 +25,32 @@ function LoadTeamAvatar({
2425
const team = teams.find(t => t.id === teamId);
2526

2627
if (isLoading) {
27-
return <LoadingIndicator mini />;
28+
const size = `${props.size}px`;
29+
return <Placeholder width={size} height={size} />;
2830
}
2931

3032
return <TeamAvatar team={team} {...props} />;
3133
}
3234

35+
/**
36+
* Wrapper to assist loading the user from api or store
37+
*/
38+
function LoadMemberAvatar({
39+
userId,
40+
...props
41+
}: {userId: string} & Omit<React.ComponentProps<typeof UserAvatar>, 'team'>) {
42+
const ids = useMemo(() => [userId], [userId]);
43+
const {members, fetching} = useMembers({ids});
44+
const user = members.find(u => u.id === userId);
45+
46+
if (fetching) {
47+
const size = `${props.size}px`;
48+
return <Placeholder shape="circle" width={size} height={size} />;
49+
}
50+
51+
return <UserAvatar user={user} {...props} />;
52+
}
53+
3354
function ActorAvatar({size = 24, hasTooltip = true, actor, ...props}: Props) {
3455
const otherProps = {
3556
size,
@@ -38,8 +59,7 @@ function ActorAvatar({size = 24, hasTooltip = true, actor, ...props}: Props) {
3859
};
3960

4061
if (actor.type === 'user') {
41-
const user = actor.id ? MemberListStore.getById(actor.id) ?? actor : actor;
42-
return <UserAvatar user={user} {...otherProps} />;
62+
return <LoadMemberAvatar userId={actor.id} {...otherProps} />;
4363
}
4464

4565
if (actor.type === 'team') {

static/app/components/avatar/index.spec.tsx

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,13 @@ import {OrganizationFixture} from 'sentry-fixture/organization';
33
import {ProjectFixture} from 'sentry-fixture/project';
44
import {SentryAppFixture} from 'sentry-fixture/sentryApp';
55
import {TeamFixture} from 'sentry-fixture/team';
6+
import {UserFixture} from 'sentry-fixture/user';
67

78
import {render, screen} from 'sentry-test/reactTestingLibrary';
89

910
import AvatarComponent from 'sentry/components/avatar';
1011
import ConfigStore from 'sentry/stores/configStore';
12+
import MemberListStore from 'sentry/stores/memberListStore';
1113
import type {Avatar} from 'sentry/types';
1214

1315
describe('Avatar', function () {
@@ -17,14 +19,14 @@ describe('Avatar', function () {
1719
avatarUrl: 'https://sentry.io/avatar/2d641b5d-8c74-44de-9cb6-fbd54701b35e/',
1820
};
1921

20-
const user = {
22+
const user = UserFixture({
2123
id: '1',
2224
name: 'Jane Bloggs',
2325
username: 'janebloggs@example.com',
2426
email: 'janebloggs@example.com',
2527
ip_address: '127.0.0.1',
2628
avatar,
27-
};
29+
});
2830

2931
window.__initialData = {
3032
links: {sentryUrl: 'https://sentry.io'},
@@ -121,7 +123,12 @@ describe('Avatar', function () {
121123
});
122124

123125
it('should show a gravatar when no avatar type is set and user has an email address', async function () {
124-
render(<AvatarComponent gravatar user={{...user, avatar: undefined}} />);
126+
render(
127+
<AvatarComponent
128+
gravatar
129+
user={{...user, options: undefined, avatar: undefined}}
130+
/>
131+
);
125132
await screen.findByRole('img');
126133

127134
const avatarElement = screen.getByTestId(`gravatar-avatar`);
@@ -175,13 +182,14 @@ describe('Avatar', function () {
175182
);
176183
});
177184

178-
it('can display a actor Avatar', function () {
185+
it('can display a actor Avatar', async function () {
179186
const actor = ActorFixture();
187+
MemberListStore.loadInitialData([user]);
180188

181189
render(<AvatarComponent actor={actor} />);
182190

183-
expect(screen.getByTestId(`letter_avatar-avatar`)).toBeInTheDocument();
184-
expect(screen.getByText('FB')).toBeInTheDocument();
191+
expect(await screen.findByTestId(`letter_avatar-avatar`)).toBeInTheDocument();
192+
expect(screen.getByText('JB')).toBeInTheDocument();
185193
});
186194

187195
it('displays platform list icons for project Avatar', function () {

static/app/views/dashboards/widgetCard/issueWidgetCard.spec.tsx

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import {OrganizationFixture} from 'sentry-fixture/organization';
2+
import {UserFixture} from 'sentry-fixture/user';
23

34
import {initializeOrg} from 'sentry-test/initializeOrg';
45
import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary';
@@ -44,6 +45,7 @@ describe('Dashboards > IssueWidgetCard', function () {
4445
},
4546
};
4647

48+
const user = UserFixture();
4749
const api = new MockApiClient();
4850

4951
beforeEach(function () {
@@ -56,9 +58,9 @@ describe('Dashboards > IssueWidgetCard', function () {
5658
shortId: 'ISSUE',
5759
assignedTo: {
5860
type: 'user',
59-
id: '2222222',
60-
name: 'dashboard user',
61-
email: 'dashboarduser@sentry.io',
61+
id: user.id,
62+
name: user.name,
63+
email: user.email,
6264
},
6365
lifetime: {count: 10, userCount: 5},
6466
count: 6,
@@ -70,7 +72,7 @@ describe('Dashboards > IssueWidgetCard', function () {
7072
MockApiClient.addMockResponse({
7173
url: '/organizations/org-slug/users/',
7274
method: 'GET',
73-
body: [],
75+
body: [user],
7476
});
7577
});
7678

@@ -79,7 +81,7 @@ describe('Dashboards > IssueWidgetCard', function () {
7981
});
8082

8183
it('renders with title and issues chart', async function () {
82-
MemberListStore.loadInitialData([]);
84+
MemberListStore.loadInitialData([user]);
8385
render(
8486
<WidgetCard
8587
api={api}
@@ -100,13 +102,13 @@ describe('Dashboards > IssueWidgetCard', function () {
100102
expect(await screen.findByText('assignee')).toBeInTheDocument();
101103
expect(screen.getByText('title')).toBeInTheDocument();
102104
expect(screen.getByText('issue')).toBeInTheDocument();
103-
expect(screen.getByText('DU')).toBeInTheDocument();
105+
expect(screen.getByText('FB')).toBeInTheDocument();
104106
expect(screen.getByText('ISSUE')).toBeInTheDocument();
105107
expect(
106108
screen.getByText('ChunkLoadError: Loading chunk app_bootstrap_index_tsx failed.')
107109
).toBeInTheDocument();
108-
await userEvent.hover(screen.getByTitle('dashboard user'));
109-
expect(await screen.findByText('Assigned to dashboard user')).toBeInTheDocument();
110+
await userEvent.hover(screen.getByTitle(user.name));
111+
expect(await screen.findByText(`Assigned to ${user.name}`)).toBeInTheDocument();
110112
});
111113

112114
it('opens in issues page', async function () {
@@ -186,7 +188,7 @@ describe('Dashboards > IssueWidgetCard', function () {
186188
});
187189

188190
it('maps lifetimeEvents and lifetimeUsers headers to more human readable', async function () {
189-
MemberListStore.loadInitialData([]);
191+
MemberListStore.loadInitialData([user]);
190192
render(
191193
<WidgetCard
192194
api={api}

0 commit comments

Comments
 (0)