fix: do not pass string 'Deleted user' to AvatarWrapper

Signed-off-by: Maksim Sukharev <[email protected]>

src/components/CallView/shared/VideoVue.vue

@@ -49,10 +49,10 @@
 			<div v-if="showBackgroundAndAvatar"
 				:key="'backgroundAvatar'"
 				class="avatar-container">
-				<VideoBackground :display-name="participantName" :user="participantUserId" />
+				<VideoBackground :display-name="displayName" :user="participantUserId" />
 				<AvatarWrapper :id="participantUserId"
 					:token="token"
-					:name="participantName"
+					:name="displayName"
 					:source="participantActorType"
 					:size="avatarSize"
 					:loading="isLoading"
@@ -104,6 +104,7 @@ import { EventBus } from '../../../services/EventBus.ts'
 import { useCallViewStore } from '../../../stores/callView.js'
 import { useGuestNameStore } from '../../../stores/guestName.js'
 import attachMediaStream from '../../../utils/attachmediastream.js'
+import { getDisplayNameWithFallback } from '../../../utils/getDisplayName.ts'
 import { ConnectionState } from '../../../utils/webrtc/models/CallParticipantModel.js'
 import { placeholderImage } from '../Grid/gridPlaceholders.ts'
 
@@ -411,7 +412,7 @@ export default {
 			return null
 		},
 
-		participantName() {
+		displayName() {
 			if (this.model.attributes.name) {
 				return this.model.attributes.name
 			}
@@ -432,14 +433,11 @@ export default {
 				)
 			}
 
-			if (!participantName) {
-				participantName = this.peerData.displayName
-				if (!participantName && this.peerData.actorType === ATTENDEE.ACTOR_TYPE.GUESTS) {
-					participantName = t('spreed', 'Guest')
-				}
-			}
+			return participantName?.trim() ?? ''
+		},
 
-			return participantName
+		participantName() {
+			return getDisplayNameWithFallback(this.displayName, this.participantActorType)
 		},
 
 		isSpeaking() {

src/components/MessagesList/MessagesGroup/Message/MessagePart/Reactions.vue

@@ -91,6 +91,7 @@ import ReactionsList from './ReactionsList.vue'
 import { ATTENDEE } from '../../../../../constants.js'
 import { useGuestNameStore } from '../../../../../stores/guestName.js'
 import { useReactionsStore } from '../../../../../stores/reactions.js'
+import { getDisplayNameWithFallback } from '../../../../../utils/getDisplayName.ts'
 
 export default {
 	name: 'Reactions',
@@ -230,12 +231,7 @@ export default {
 				return this.guestNameStore.getGuestNameWithGuestSuffix(this.token, reactingParticipant.actorId)
 			}
 
-			const displayName = reactingParticipant.actorDisplayName.trim()
-			if (displayName === '') {
-				return t('spreed', 'Deleted user')
-			}
-
-			return displayName
+			return getDisplayNameWithFallback(reactingParticipant.actorDisplayName, reactingParticipant.actorType)
 		},
 
 		reactionsCount(reaction) {

src/components/MessagesList/MessagesGroup/Message/MessagePart/ReactionsList.vue

@@ -32,7 +32,7 @@
 							:source="item.actorType"
 							:size="AVATAR.SIZE.SMALL"
 							disable-menu />
-						<span class="reactions-item__name">{{ item.actorDisplayName }}</span>
+						<span class="reactions-item__name">{{ item.actorDisplayNameWithFallback }}</span>
 						<span class="reactions-item__emojis">
 							{{ item.reaction?.join('') ?? reactionFilter }}
 						</span>
@@ -57,6 +57,7 @@ import AvatarWrapper from '../../../../AvatarWrapper/AvatarWrapper.vue'
 
 import { ATTENDEE, AVATAR } from '../../../../../constants.js'
 import { useGuestNameStore } from '../../../../../stores/guestName.js'
+import { getDisplayNameWithFallback } from '../../../../../utils/getDisplayName.ts'
 
 export default {
 
@@ -112,17 +113,20 @@ export default {
 				actors.forEach(actor => {
 					const key = `${actor.actorId}-${actor.actorType}`
 					const actorDisplayName = this.getDisplayNameForReaction(actor)
+					const actorDisplayNameWithFallback = getDisplayNameWithFallback(actorDisplayName, actor.actorType)
 
 					modifiedDetailedReactions[reaction].push({
 						...actor,
-						actorDisplayName
+						actorDisplayName,
+						actorDisplayNameWithFallback,
 					})
 
 					if (mergedReactionsMap[key]) {
 						mergedReactionsMap[key].reaction.push(reaction)
 					} else {
 						mergedReactionsMap[key] = {
 							actorDisplayName,
+							actorDisplayNameWithFallback,
 							actorId: actor.actorId,
 							actorType: actor.actorType,
 							reaction: [reaction]
@@ -150,12 +154,7 @@ export default {
 				return this.guestNameStore.getGuestNameWithGuestSuffix(this.token, reactingParticipant.actorId)
 			}
 
-			const displayName = reactingParticipant.actorDisplayName.trim()
-			if (displayName === '') {
-				return t('spreed', 'Deleted user')
-			}
-
-			return displayName
+			return reactingParticipant.actorDisplayName.trim()
 		},
 
 		handleTabClick(reaction) {

src/components/MessagesList/MessagesGroup/MessagesGroup.vue

@@ -83,10 +83,11 @@ export default {
 			remoteServer,
 			lastEditor,
 			actorDisplayName,
+			actorDisplayNameWithFallback,
 		} = useMessageInfo(firstMessage)
 
 		const actorInfo = computed(() => {
-			return [actorDisplayName.value, remoteServer.value, lastEditor.value]
+			return [actorDisplayNameWithFallback.value, remoteServer.value, lastEditor.value]
 				.filter(value => value).join(' ')
 		})
 

src/components/PollViewer/PollVotersDetails.vue

@@ -29,7 +29,7 @@
 				class="poll-voters-details__list-item">
 				<AvatarWrapper :id="item.actorId"
 					:token="token"
-					:name="getDisplayName(item)"
+					:name="item.actorDisplayName.trim()"
 					:source="item.actorType"
 					:size="AVATAR.SIZE.EXTRA_SMALL"
 					disable-menu />
@@ -49,7 +49,8 @@ import NcPopover from '@nextcloud/vue/dist/Components/NcPopover.js'
 
 import AvatarWrapper from '../AvatarWrapper/AvatarWrapper.vue'
 
-import { ATTENDEE, AVATAR } from '../../constants.js'
+import { AVATAR } from '../../constants.js'
+import { getDisplayNameWithFallback } from '../../utils/getDisplayName.ts'
 
 export default {
 
@@ -80,15 +81,7 @@ export default {
 	methods: {
 		t,
 		getDisplayName(item) {
-			if (item.actorDisplayName === '' && item.actorType === ATTENDEE.ACTOR_TYPE.GUESTS) {
-				return t('spreed', 'Guest')
-			}
-
-			if (item.actorType === 'deleted_users') {
-				return t('spreed', 'Deleted user')
-			}
-
-			return item.actorDisplayName
+			return getDisplayNameWithFallback(item.actorDisplayName, item.actorType)
 		},
 	},
 }

src/components/Quote.vue

@@ -113,10 +113,11 @@ export default {
 			remoteServer,
 			lastEditor,
 			actorDisplayName,
+			actorDisplayNameWithFallback,
 		} = useMessageInfo(message)
 
 		const actorInfo = computed(() => {
-			return [actorDisplayName.value, remoteServer.value, lastEditor.value]
+			return [actorDisplayNameWithFallback.value, remoteServer.value, lastEditor.value]
 				.filter(value => value).join(' ')
 		})
 

src/components/RightSidebar/Participants/Participant.spec.js

@@ -145,7 +145,7 @@ describe('Participant.vue', () => {
 			const avatarEl = wrapper.findComponent(AvatarWrapper)
 			expect(avatarEl.exists()).toBe(true)
 
-			expect(avatarEl.props('name')).toBe('Guest')
+			expect(avatarEl.props('name')).toBe('')
 		})
 
 		test('renders avatar with unknown name when empty', () => {
@@ -154,7 +154,7 @@ describe('Participant.vue', () => {
 			const avatarEl = wrapper.findComponent(AvatarWrapper)
 			expect(avatarEl.exists()).toBe(true)
 
-			expect(avatarEl.props('name')).toBe('Deleted user')
+			expect(avatarEl.props('name')).toBe('')
 		})
 
 		test('renders offline avatar when no sessions exist', () => {

src/components/RightSidebar/Participants/Participant.vue

@@ -17,7 +17,7 @@
 		<template #icon>
 			<AvatarWrapper :id="participant.actorId"
 				:token="token"
-				:name="computedName"
+				:name="displayName"
 				:source="participant.actorType"
 				disable-tooltip
 				:show-user-status="showUserStatus"
@@ -358,6 +358,7 @@ import {
 } from '../../../services/callsService.js'
 import { hasTalkFeature } from '../../../services/CapabilitiesManager.ts'
 import { formattedTime } from '../../../utils/formattedTime.ts'
+import { getDisplayNameWithFallback } from '../../../utils/getDisplayName.ts'
 import { readableNumber } from '../../../utils/readableNumber.ts'
 import { getPreloadedUserStatus, getStatusMessage } from '../../../utils/userStatus.ts'
 
@@ -548,18 +549,12 @@ export default {
 				&& this.participant.inCall === PARTICIPANT.CALL_FLAG.DISCONNECTED
 		},
 
-		computedName() {
-			const displayName = this.participant.displayName.trim()
-
-			if (displayName === '' && (this.isGuestActor || this.isEmailActor)) {
-				return t('spreed', 'Guest')
-			}
-
-			if (displayName === '') {
-				return t('spreed', 'Deleted user')
-			}
+		displayName() {
+			return this.participant.displayName.trim()
+		},
 
-			return displayName
+		computedName() {
+			return getDisplayNameWithFallback(this.participant.displayName, this.participant.actorType)
 		},
 
 		attendeeId() {
@@ -695,14 +690,14 @@ export default {
 			switch (this.participant.actorType) {
 			case ATTENDEE.ACTOR_TYPE.GROUPS:
 				return t('spreed', 'Do you really want to remove group "{displayName}" and its members from this conversation?',
-					this.participant, undefined, { escape: false, sanitize: false })
+					{ displayName: this.computedName }, undefined, { escape: false, sanitize: false })
 			case ATTENDEE.ACTOR_TYPE.CIRCLES:
 				return t('spreed', 'Do you really want to remove team "{displayName}" and its members from this conversation?',
-					this.participant, undefined, { escape: false, sanitize: false })
+					{ displayName: this.computedName }, undefined, { escape: false, sanitize: false })
 			case ATTENDEE.ACTOR_TYPE.USERS:
 			default:
 				return t('spreed', 'Do you really want to remove {displayName} from this conversation?',
-					this.participant, undefined, { escape: false, sanitize: false })
+					{ displayName: this.computedName }, undefined, { escape: false, sanitize: false })
 			}
 		},
 
@@ -832,10 +827,10 @@ export default {
 					token: this.token,
 					attendeeId: this.attendeeId,
 				})
-				showSuccess(t('spreed', 'Notification was sent to {displayName}', { displayName: this.participant.displayName }))
+				showSuccess(t('spreed', 'Notification was sent to {displayName}', { displayName: this.computedName }))
 			} catch (error) {
 				console.error(error)
-				showError(t('spreed', 'Could not send notification to {displayName}', { displayName: this.participant.displayName }))
+				showError(t('spreed', 'Could not send notification to {displayName}', { displayName: this.computedName }))
 			}
 		},
 

src/composables/__tests__/useMessageInfo.spec.js

@@ -352,13 +352,14 @@ describe('message actions', () => {
 		expect(result.actorDisplayName.value).toBe('guest name')
 	})
 
-	test('return "deleted user" for messages from deleted users without display name', () => {
+	test('return "Deleted user" as a fallback for messages from deleted users without display name', () => {
 		// Arrange
 		message.value.actorDisplayName = ''
 		// Act
 		const result = useMessageInfo(message)
 		// Assert
-		expect(result.actorDisplayName.value).toBe('Deleted user')
+		expect(result.actorDisplayName.value).toBe('')
+		expect(result.actorDisplayNameWithFallback.value).toBe('Deleted user')
 	})
 
 	describe('edited messages', () => {

src/composables/useMessageInfo.js

@@ -13,6 +13,7 @@ import { useStore } from './useStore.js'
 import { ATTENDEE, CONVERSATION } from '../constants.js'
 import { hasTalkFeature } from '../services/CapabilitiesManager.ts'
 import { useGuestNameStore } from '../stores/guestName.js'
+import { getDisplayNameWithFallback } from '../utils/getDisplayName.ts'
 
 /**
  * Check whether the user can edit the message or not
@@ -37,6 +38,10 @@ export function useMessageInfo(message = ref({})) {
 			isConversationReadOnly: computed(() => false),
 			isFileShareWithoutCaption: computed(() => false),
 			isFileShare: computed(() => false),
+			remoteServer: computed(() => ''),
+			lastEditor: computed(() => ''),
+			actorDisplayName: computed(() => ''),
+			actorDisplayNameWithFallback: computed(() => ''),
 		}
 	}
 
@@ -101,14 +106,16 @@ export function useMessageInfo(message = ref({})) {
 	})
 
 	const actorDisplayName = computed(() => {
-		if (message.value.actorType === ATTENDEE.ACTOR_TYPE.GUESTS) {
+		if ([ATTENDEE.ACTOR_TYPE.GUESTS, ATTENDEE.ACTOR_TYPE.EMAILS].includes(message.value.actorType)) {
 			const guestNameStore = useGuestNameStore()
 			return guestNameStore.getGuestName(message.value.token, message.value.actorId)
 		} else {
-			const displayName = message.value.actorDisplayName.trim()
-			return displayName === '' ? t('spreed', 'Deleted user') : displayName
+			return message.value.actorDisplayName.trim()
 		}
+	})
 
+	const actorDisplayNameWithFallback = computed(() => {
+		return getDisplayNameWithFallback(actorDisplayName.value, message.value.actorType)
 	})
 
 	return {
@@ -123,6 +130,6 @@ export function useMessageInfo(message = ref({})) {
 		remoteServer,
 		lastEditor,
 		actorDisplayName,
+		actorDisplayNameWithFallback,
 	}
-
 }

src/utils/getDisplayName.ts

@@ -0,0 +1,24 @@
+/**
+ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+import { t } from '@nextcloud/l10n'
+
+import { ATTENDEE } from '../constants.js'
+
+export const getDisplayNameWithFallback = function(displayName: string, source: string): string {
+	if (displayName?.trim()) {
+		return displayName.trim()
+	}
+
+	if ([ATTENDEE.ACTOR_TYPE.GUESTS, ATTENDEE.ACTOR_TYPE.EMAILS].includes(source)) {
+		return t('spreed', 'Guest')
+	}
+
+	if (source === ATTENDEE.ACTOR_TYPE.DELETED_USERS) {
+		return t('spreed', 'Deleted user')
+	}
+
+	// Fallback to 'Deleted user': should not happen, but can not be empty either
+	return t('spreed', 'Deleted user')
+}