Skip to content

fix(dashboards): Use avatarType from API to render correct avatar in revision list#114336

Merged
skaasten merged 14 commits intomasterfrom
shaunkaasten/dain-1579-use-avatar-type
Apr 30, 2026
Merged

fix(dashboards): Use avatarType from API to render correct avatar in revision list#114336
skaasten merged 14 commits intomasterfrom
shaunkaasten/dain-1579-use-avatar-type

Conversation

@skaasten
Copy link
Copy Markdown
Contributor

@skaasten skaasten commented Apr 29, 2026

After #114186, we have the avatar type being returned in the revision list endpoint.

We should pass that into the avatar component so that it properly shows initials vs upload vs gravatar.

The current-version row in the modal passes a full User object whose avatar info is nested under avatar.avatarType. That field is now explicitly mapped to the flat createdBy prop shape so it follows the same rendering path as historical revision rows.

skaasten and others added 4 commits April 29, 2026 11:06
…revision list

The avatar was hardcoded to 'upload' type for any non-null avatarUrl, causing
gravatar users to have their avatar URL treated as an upload URL.

Now avatarType is passed through from the API response and used to construct
the avatar object correctly, so gravatar, upload, and letter avatars all
render as intended.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…item

The current version item was passing a User object directly as createdBy,
but User has avatar fields nested under avatar.avatarType rather than at the
top level. Map to the expected shape so the avatar renders correctly.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Verify that passing avatarType: 'gravatar' renders a gravatar-avatar
container, confirming the correct avatar rendering path is used.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The CAROL fixture was missing avatarType after the prop shape changed
from avatarUrl-based to avatarType-based avatar rendering.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@linear-code
Copy link
Copy Markdown

linear-code Bot commented Apr 29, 2026

@github-actions github-actions Bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Apr 29, 2026
@skaasten skaasten marked this pull request as ready for review April 29, 2026 18:45
@skaasten skaasten requested a review from a team as a code owner April 29, 2026 18:45
Comment thread static/app/views/dashboards/hooks/useDashboardRevisions.tsx Outdated
Copy link
Copy Markdown
Member

@nikkikapadia nikkikapadia left a comment

Choose a reason for hiding this comment

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

🚢

Replace inline object type with a named Pick<AvatarUser, ...> type in
useDashboardRevisions.tsx, and use Avatar['avatarType'] for precise
avatarType typing. Update revisionListItem.tsx to reference the shared
RevisionCreatedBy type instead of repeating the inline shape.

Co-Authored-By: Claude Sonnet 4.6 <noreply@example.com>
Copy link
Copy Markdown
Contributor

@DominikB2014 DominikB2014 left a comment

Choose a reason for hiding this comment

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

lgtm, one thing i did notice is AvatarUser has `avatarUrl in it, is it possible to use it from there too?

TypeScript widens string literals in const object declarations, making
'upload' typed as string instead of Avatar['avatarType']. Add as const
to preserve the literal type.

Co-Authored-By: Claude Sonnet 4.6 <noreply@example.com>
AvatarUser already carries avatarUrl, so include it in the Pick instead
of declaring a separate field. Convert Avatar.avatarUrl (string | null)
to undefined at the call site so it matches AvatarUser's avatarUrl type.

Co-Authored-By: Claude Sonnet 4.6 <noreply@example.com>
AvatarUser.avatarUrl is string | undefined (not null), so omit the
field rather than passing null to satisfy the Pick-based type.

Co-Authored-By: Claude Sonnet 4.6 <noreply@example.com>
Add an isUser type guard (discriminated by ip_address, which exists on
AvatarUser but not RevisionCreatedBy's Pick) so the component handles
both shapes. Pass a User directly to UserAvatar; construct the adapter
object only for the flat RevisionCreatedBy API shape.

This lets dashboardRevisions.tsx pass dashboardCreatedBy directly
instead of manually remapping to the flat shape.

Co-Authored-By: Claude Sonnet 4.6 <noreply@example.com>
…sions

Transform the flat avatarType/avatarUrl fields from the revisions API
into a nested avatar object in the hook. RevisionCreatedBy is now
Pick<AvatarUser, 'id' | 'name' | 'email' | 'avatar' | 'avatarUrl'>,
so User is structurally assignable and dashboardRevisions.tsx can pass
dashboardCreatedBy directly without any mapping. The component no longer
needs to handle two different createdBy shapes.

Co-Authored-By: Claude Sonnet 4.6 <noreply@example.com>
Omit<RawDashboardRevision, 'createdBy'> was unnecessary indirection
since both types are local. Write DashboardRevision out directly.

Co-Authored-By: Claude Sonnet 4.6 <noreply@example.com>
…f spreading

Spreading ...raw included the raw createdBy only to immediately override
it. List fields explicitly.

Co-Authored-By: Claude Sonnet 4.6 <noreply@example.com>
knip flagged it as an unused export since no file imports it directly.
Callers use DashboardRevision['createdBy'] instead.

Co-Authored-By: Claude Sonnet 4 <noreply@example.com>
Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit d2ecbf0. Configure here.

Comment thread static/app/views/dashboards/hooks/useDashboardRevisions.tsx Outdated
Copy link
Copy Markdown
Member

@nikkikapadia nikkikapadia left a comment

Choose a reason for hiding this comment

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

nice!

…visions

data?.map(normalizeRevision) in the return statement created a new array
on every render, breaking React Query's referential stability. Wrapping in
useMemo keyed on data restores stable references so RevisionListItem
children don't re-render unnecessarily when unrelated state changes.

Co-Authored-By: Claude Sonnet 4 <noreply@example.com>
@skaasten skaasten merged commit 89d85db into master Apr 30, 2026
64 checks passed
@skaasten skaasten deleted the shaunkaasten/dain-1579-use-avatar-type branch April 30, 2026 17:26
cleptric pushed a commit that referenced this pull request May 5, 2026
…revision list (#114336)

After #114186, we have the avatar type being returned in the revision
list endpoint.

We should pass that into the avatar component so that it properly shows
initials vs upload vs gravatar.

The current-version row in the modal passes a full `User` object whose
avatar info is nested under `avatar.avatarType`. That field is now
explicitly mapped to the flat `createdBy` prop shape so it follows the
same rendering path as historical revision rows.

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@example.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants