diff --git a/ui/src/common/ImageUploadOverlay.jsx b/ui/src/common/ImageUploadOverlay.jsx index 119426c73..03144ddc0 100644 --- a/ui/src/common/ImageUploadOverlay.jsx +++ b/ui/src/common/ImageUploadOverlay.jsx @@ -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') diff --git a/ui/src/layout/UserMenu.jsx b/ui/src/layout/UserMenu.jsx index 49c64928d..1b465ed35 100644 --- a/ui/src/layout/UserMenu.jsx +++ b/ui/src/layout/UserMenu.jsx @@ -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) { diff --git a/ui/src/layout/UserMenu.test.jsx b/ui/src/layout/UserMenu.test.jsx index 15c42e81b..4a21b94ca 100644 --- a/ui/src/layout/UserMenu.test.jsx +++ b/ui/src/layout/UserMenu.test.jsx @@ -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(Logout} />) + return render(Logout} />) } describe('', () => { - 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('', () => { '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(Logout} />) + + 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(Logout} />) + + await vi.waitFor(() => + expect(screen.queryByRole('img')).not.toBeInTheDocument(), + ) + }) }) diff --git a/ui/src/setupTests.js b/ui/src/setupTests.js index ddb999f3c..7cb46e09c 100644 --- a/ui/src/setupTests.js +++ b/ui/src/setupTests.js @@ -14,6 +14,9 @@ const localStorageMock = (function () { setItem: function (key, value) { store[key] = value.toString() }, + removeItem: function (key) { + delete store[key] + }, clear: function () { store = {} }, diff --git a/ui/src/user/UserEdit.jsx b/ui/src/user/UserEdit.jsx index b6523bc2b..567709832 100644 --- a/ui/src/user/UserEdit.jsx +++ b/ui/src/user/UserEdit.jsx @@ -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', diff --git a/ui/src/user/UserEdit.test.jsx b/ui/src/user/UserEdit.test.jsx index 0f5dd5a1c..52e955ed7 100644 --- a/ui/src/user/UserEdit.test.jsx +++ b/ui/src/user/UserEdit.test.jsx @@ -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 }) =>
{subTitle}
, - ImageUploadOverlay: ({ canEdit, messages }) => - canEdit ? : null, + ImageUploadOverlay: ({ canEdit, messages, onImageChange }) => + canEdit ? ( + <> + + + + ) : null, })) vi.mock('../subsonic', () => ({ @@ -235,6 +250,42 @@ describe('', () => { 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(