mirror of
https://github.com/navidrome/navidrome.git
synced 2026-10-13 21:07:10 +02:00
fix(ui): keep the user menu avatar in sync after upload or removal
avatarTag was a login-time snapshot, so uploading an avatar left the menu on the old icon and removing one left it pointing at getAvatar, which now falls through to the Navidrome logo placeholder. The edit page now updates the stored tag when the record is the logged-in user, and the menu re-reads the identity on refresh.
This commit is contained in:
parent
56ec411cae
commit
ae8b736d07
6 changed files with 132 additions and 19 deletions
|
|
@ -88,7 +88,7 @@ export const ImageUploadOverlay = ({
|
|||
body: formData,
|
||||
})
|
||||
notify(msg.uploaded, 'success')
|
||||
if (onImageChange) onImageChange()
|
||||
if (onImageChange) onImageChange(true)
|
||||
refresh()
|
||||
} catch (err) {
|
||||
notify(msg.uploadError, 'warning')
|
||||
|
|
@ -117,7 +117,7 @@ export const ImageUploadOverlay = ({
|
|||
method: 'DELETE',
|
||||
})
|
||||
notify(msg.removed, 'success')
|
||||
if (onImageChange) onImageChange()
|
||||
if (onImageChange) onImageChange(false)
|
||||
refresh()
|
||||
} catch (err) {
|
||||
notify(msg.removeError, 'warning')
|
||||
|
|
|
|||
|
|
@ -7,7 +7,7 @@ import {
|
|||
useState,
|
||||
} from 'react'
|
||||
import PropTypes from 'prop-types'
|
||||
import { useTranslate, useGetIdentity } from 'react-admin'
|
||||
import { useTranslate, useGetIdentity, useVersion } from 'react-admin'
|
||||
import {
|
||||
Tooltip,
|
||||
IconButton,
|
||||
|
|
@ -52,15 +52,24 @@ const UserMenu = (props) => {
|
|||
const [anchorEl, setAnchorEl] = useState(null)
|
||||
const translate = useTranslate()
|
||||
const { loaded, identity } = useGetIdentity()
|
||||
const version = useVersion()
|
||||
const [avatarTag, setAvatarTag] = useState(null)
|
||||
const classes = useStyles(props)
|
||||
const dispatch = useDispatch()
|
||||
|
||||
const { children, label, icon, logout } = props
|
||||
|
||||
// identity.id is the username (see authProvider.getIdentity); avatarTag is only set when an avatar was uploaded
|
||||
const avatarUrl = identity?.avatarTag
|
||||
? `${subsonic.getAvatarUrl(identity.id)}&_=${identity.avatarTag}`
|
||||
: identity?.avatar
|
||||
// The tag is only set when an avatar was uploaded, and it changes mid-session on upload or
|
||||
// removal, so the identity is re-read on every refresh instead of trusting the login snapshot.
|
||||
useEffect(() => {
|
||||
authProvider.getIdentity().then((user) => setAvatarTag(user?.avatarTag))
|
||||
}, [version])
|
||||
|
||||
// identity.id is the username (see authProvider.getIdentity)
|
||||
const avatarUrl =
|
||||
avatarTag && identity?.id
|
||||
? `${subsonic.getAvatarUrl(identity.id)}&_=${avatarTag}`
|
||||
: identity?.avatar
|
||||
|
||||
useEffect(() => {
|
||||
if (config.devActivityPanel) {
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
import React from 'react'
|
||||
import { render, screen } from '@testing-library/react'
|
||||
import { describe, it, expect, vi } from 'vitest'
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest'
|
||||
import UserMenu from './UserMenu'
|
||||
|
||||
vi.mock('../subsonic', () => ({
|
||||
|
|
@ -14,34 +14,42 @@ vi.mock('react-redux', () => ({
|
|||
}))
|
||||
|
||||
let mockIdentity
|
||||
let mockVersion
|
||||
vi.mock('react-admin', () => ({
|
||||
useTranslate: () => (x) => x,
|
||||
useGetIdentity: () => ({ loaded: true, identity: mockIdentity }),
|
||||
useVersion: () => mockVersion,
|
||||
}))
|
||||
|
||||
const renderUserMenu = (identity) => {
|
||||
mockIdentity = identity
|
||||
render(<UserMenu label="menu.settings" logout={<div>Logout</div>} />)
|
||||
return render(<UserMenu label="menu.settings" logout={<div>Logout</div>} />)
|
||||
}
|
||||
|
||||
describe('<UserMenu />', () => {
|
||||
it('uses the uploaded avatar when the identity has an avatarTag', () => {
|
||||
renderUserMenu({ id: 'u1', username: 'deluan', avatarTag: 'abc123' })
|
||||
expect(screen.getByRole('img')).toHaveAttribute(
|
||||
beforeEach(() => {
|
||||
localStorage.clear()
|
||||
mockVersion = 1
|
||||
})
|
||||
|
||||
it('uses the uploaded avatar when there is an avatar tag', async () => {
|
||||
localStorage.setItem('avatarTag', 'abc123')
|
||||
renderUserMenu({ id: 'deluan', fullName: 'Deluan' })
|
||||
expect(await screen.findByRole('img')).toHaveAttribute(
|
||||
'src',
|
||||
expect.stringContaining('getAvatar'),
|
||||
)
|
||||
})
|
||||
|
||||
it('falls back to the generic icon with no avatar and no gravatar', () => {
|
||||
renderUserMenu({ id: 'u1', username: 'deluan' })
|
||||
renderUserMenu({ id: 'deluan', fullName: 'Deluan' })
|
||||
expect(screen.queryByRole('img')).not.toBeInTheDocument()
|
||||
})
|
||||
|
||||
it('uses the gravatar url when there is no avatarTag', () => {
|
||||
it('uses the gravatar url when there is no avatar tag', () => {
|
||||
renderUserMenu({
|
||||
id: 'u1',
|
||||
username: 'deluan',
|
||||
id: 'deluan',
|
||||
fullName: 'Deluan',
|
||||
avatar: 'https://gravatar/u1',
|
||||
})
|
||||
expect(screen.getByRole('img')).toHaveAttribute(
|
||||
|
|
@ -49,4 +57,32 @@ describe('<UserMenu />', () => {
|
|||
'https://gravatar/u1',
|
||||
)
|
||||
})
|
||||
|
||||
it('picks up an avatar uploaded during the session', async () => {
|
||||
const { rerender } = renderUserMenu({ id: 'deluan', fullName: 'Deluan' })
|
||||
expect(screen.queryByRole('img')).not.toBeInTheDocument()
|
||||
|
||||
localStorage.setItem('avatarTag', 'newtag')
|
||||
mockVersion = 2
|
||||
rerender(<UserMenu label="menu.settings" logout={<div>Logout</div>} />)
|
||||
|
||||
expect(await screen.findByRole('img')).toHaveAttribute(
|
||||
'src',
|
||||
expect.stringContaining('newtag'),
|
||||
)
|
||||
})
|
||||
|
||||
it('goes back to the generic icon when the avatar is removed during the session', async () => {
|
||||
localStorage.setItem('avatarTag', 'abc123')
|
||||
const { rerender } = renderUserMenu({ id: 'deluan', fullName: 'Deluan' })
|
||||
expect(await screen.findByRole('img')).toBeInTheDocument()
|
||||
|
||||
localStorage.removeItem('avatarTag')
|
||||
mockVersion = 2
|
||||
rerender(<UserMenu label="menu.settings" logout={<div>Logout</div>} />)
|
||||
|
||||
await vi.waitFor(() =>
|
||||
expect(screen.queryByRole('img')).not.toBeInTheDocument(),
|
||||
)
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -14,6 +14,9 @@ const localStorageMock = (function () {
|
|||
setItem: function (key, value) {
|
||||
store[key] = value.toString()
|
||||
},
|
||||
removeItem: function (key) {
|
||||
delete store[key]
|
||||
},
|
||||
clear: function () {
|
||||
store = {}
|
||||
},
|
||||
|
|
|
|||
|
|
@ -87,6 +87,19 @@ const AvatarField = () => {
|
|||
const canEdit = config.enableUserAvatarUpload && (isAdmin || isMyself)
|
||||
const classes = useAvatarStyles()
|
||||
|
||||
const handleImageChange = useCallback(
|
||||
(hasImage) => {
|
||||
if (!isMyself) return
|
||||
// Only a cache-buster: the server sends the authoritative ETag, so any changing value works
|
||||
if (hasImage) {
|
||||
localStorage.setItem('avatarTag', Date.now().toString())
|
||||
} else {
|
||||
localStorage.removeItem('avatarTag')
|
||||
}
|
||||
},
|
||||
[isMyself],
|
||||
)
|
||||
|
||||
if (!record?.id) return null
|
||||
|
||||
return (
|
||||
|
|
@ -104,6 +117,7 @@ const AvatarField = () => {
|
|||
entityType="user"
|
||||
entityId={record.id}
|
||||
hasUploadedImage={!!record.uploadedImage}
|
||||
onImageChange={handleImageChange}
|
||||
canEdit={canEdit}
|
||||
messages={{
|
||||
uploaded: 'message.avatarUploaded',
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
import * as React from 'react'
|
||||
import { render, screen } from '@testing-library/react'
|
||||
import { render, screen, fireEvent } from '@testing-library/react'
|
||||
import UserEdit from './UserEdit'
|
||||
import config from '../config'
|
||||
import { describe, it, expect, vi, afterEach } from 'vitest'
|
||||
|
|
@ -86,8 +86,23 @@ vi.mock('./DeleteUserButton', () => ({
|
|||
|
||||
vi.mock('../common', () => ({
|
||||
Title: ({ subTitle }) => <div data-testid="title">{subTitle}</div>,
|
||||
ImageUploadOverlay: ({ canEdit, messages }) =>
|
||||
canEdit ? <button aria-label={messages.uploadLabel}>upload</button> : null,
|
||||
ImageUploadOverlay: ({ canEdit, messages, onImageChange }) =>
|
||||
canEdit ? (
|
||||
<>
|
||||
<button
|
||||
aria-label={messages.uploadLabel}
|
||||
onClick={() => onImageChange(true)}
|
||||
>
|
||||
upload
|
||||
</button>
|
||||
<button
|
||||
aria-label={messages.removeLabel}
|
||||
onClick={() => onImageChange(false)}
|
||||
>
|
||||
remove
|
||||
</button>
|
||||
</>
|
||||
) : null,
|
||||
}))
|
||||
|
||||
vi.mock('../subsonic', () => ({
|
||||
|
|
@ -235,6 +250,42 @@ describe('<UserEdit />', () => {
|
|||
expect(screen.getByLabelText('message.uploadAvatar')).toBeInTheDocument()
|
||||
})
|
||||
|
||||
it('stores a new avatar tag when the user uploads their own avatar', () => {
|
||||
config.enableUserAvatarUpload = true
|
||||
renderUserEdit({ id: 'u1', userName: 'deluan' }, { isMyself: true })
|
||||
|
||||
fireEvent.click(screen.getByLabelText('message.uploadAvatar'))
|
||||
|
||||
expect(localStorage.getItem('avatarTag')).toBeTruthy()
|
||||
})
|
||||
|
||||
it('removes the avatar tag when the user removes their own avatar', () => {
|
||||
config.enableUserAvatarUpload = true
|
||||
localStorage.setItem('avatarTag', 'oldtag')
|
||||
renderUserEdit(
|
||||
{ id: 'u1', userName: 'deluan', uploadedImage: 'u1_deluan.png' },
|
||||
{ isMyself: true },
|
||||
)
|
||||
|
||||
fireEvent.click(screen.getByLabelText('message.removeAvatar'))
|
||||
|
||||
expect(localStorage.getItem('avatarTag')).toBeNull()
|
||||
})
|
||||
|
||||
it('does not touch the admin own tag when editing another user', () => {
|
||||
config.enableUserAvatarUpload = true
|
||||
renderUserEdit(
|
||||
{ id: 'u1', userName: 'deluan', uploadedImage: 'u1_deluan.png' },
|
||||
{ isMyself: false, role: 'admin' },
|
||||
)
|
||||
localStorage.setItem('avatarTag', 'mytag')
|
||||
|
||||
fireEvent.click(screen.getByLabelText('message.uploadAvatar'))
|
||||
fireEvent.click(screen.getByLabelText('message.removeAvatar'))
|
||||
|
||||
expect(localStorage.getItem('avatarTag')).toEqual('mytag')
|
||||
})
|
||||
|
||||
it('hides the control when the feature is off and the viewer is not an admin', () => {
|
||||
config.enableUserAvatarUpload = false
|
||||
renderUserEdit(
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue