Location sharing: use Room member avatar instead of profile avatar.

bmarty · bmarty · commit 6ea0129beeb6 · 2024-02-02T18:36:29.000+01:00
diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/room/model/livelocation/LiveLocationShareAggregatedSummary.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/room/model/livelocation/LiveLocationShareAggregatedSummary.kt
@@ -22,6 +22,7 @@ import org.matrix.android.sdk.api.session.room.model.message.MessageBeaconLocati
  * Aggregation info concerning a live location share.
  */
 data class LiveLocationShareAggregatedSummary(
+        val roomId: String?,
         val userId: String?,
         /**
          * Indicate whether the live is currently running.
diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/database/mapper/LiveLocationShareAggregatedSummaryMapper.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/database/mapper/LiveLocationShareAggregatedSummaryMapper.kt
@@ -28,6 +28,7 @@ internal class LiveLocationShareAggregatedSummaryMapper @Inject constructor() :
 
     override fun map(entity: LiveLocationShareAggregatedSummaryEntity): LiveLocationShareAggregatedSummary {
         return LiveLocationShareAggregatedSummary(
+                roomId = entity.roomId,
                 userId = entity.userId,
                 isActive = entity.isActive,
                 endOfLiveTimestampMillis = entity.endOfLiveTimestampMillis,
diff --git a/matrix-sdk-android/src/test/java/org/matrix/android/sdk/internal/database/mapper/LiveLocationShareAggregatedSummaryMapperTest.kt b/matrix-sdk-android/src/test/java/org/matrix/android/sdk/internal/database/mapper/LiveLocationShareAggregatedSummaryMapperTest.kt
@@ -24,6 +24,7 @@ import org.matrix.android.sdk.api.session.room.model.message.LocationInfo
 import org.matrix.android.sdk.api.session.room.model.message.MessageBeaconLocationDataContent
 import org.matrix.android.sdk.internal.database.model.livelocation.LiveLocationShareAggregatedSummaryEntity
 
+private const val ANY_ROOM_ID = "a-room-id"
 private const val ANY_USER_ID = "a-user-id"
 private const val ANY_ACTIVE_STATE = true
 private const val ANY_TIMEOUT = 123L
@@ -40,6 +41,7 @@ class LiveLocationShareAggregatedSummaryMapperTest {
         val summary = mapper.map(entity)
 
         summary shouldBeEqualTo LiveLocationShareAggregatedSummary(
+                roomId = ANY_ROOM_ID,
                 userId = ANY_USER_ID,
                 isActive = ANY_ACTIVE_STATE,
                 endOfLiveTimestampMillis = ANY_TIMEOUT,
@@ -48,6 +50,7 @@ class LiveLocationShareAggregatedSummaryMapperTest {
     }
 
     private fun anEntity(content: MessageBeaconLocationDataContent) = LiveLocationShareAggregatedSummaryEntity(
+            roomId = ANY_ROOM_ID,
             userId = ANY_USER_ID,
             isActive = ANY_ACTIVE_STATE,
             endOfLiveTimestampMillis = ANY_TIMEOUT,
diff --git a/matrix-sdk-android/src/test/java/org/matrix/android/sdk/internal/session/room/location/DefaultLocationSharingServiceTest.kt b/matrix-sdk-android/src/test/java/org/matrix/android/sdk/internal/session/room/location/DefaultLocationSharingServiceTest.kt
@@ -229,6 +229,7 @@ internal class DefaultLocationSharingServiceTest {
     fun `livedata of live summaries is correctly computed`() {
         val entity = LiveLocationShareAggregatedSummaryEntity()
         val summary = LiveLocationShareAggregatedSummary(
+                roomId = A_ROOM_ID,
                 userId = "",
                 isActive = true,
                 endOfLiveTimestampMillis = 123,
@@ -255,6 +256,7 @@ internal class DefaultLocationSharingServiceTest {
     fun `given an event id when getting livedata on corresponding live summary then it is correctly computed`() {
         val entity = LiveLocationShareAggregatedSummaryEntity()
         val summary = LiveLocationShareAggregatedSummary(
+                roomId = A_ROOM_ID,
                 userId = "",
                 isActive = true,
                 endOfLiveTimestampMillis = 123,
diff --git a/vector/src/main/java/im/vector/app/core/epoxy/bottomsheet/BottomSheetMessagePreviewItem.kt b/vector/src/main/java/im/vector/app/core/epoxy/bottomsheet/BottomSheetMessagePreviewItem.kt
@@ -103,7 +103,8 @@ abstract class BottomSheetMessagePreviewItem : VectorEpoxyModel<BottomSheetMessa
                     .apply(RequestOptions.centerCropTransform())
                     .into(holder.staticMapImageView)
 
-            safeLocationUiData.locationPinProvider.create(safeLocationUiData.locationOwnerId) { pinDrawable ->
+            val pinMatrixItem = matrixItem.takeIf { safeLocationUiData.locationOwnerId != null }
+            safeLocationUiData.locationPinProvider.create(pinMatrixItem) { pinDrawable ->
                 // we are not using Glide since it does not display it correctly when there is no user photo
                 holder.staticMapPinImageView.setImageDrawable(pinDrawable)
             }
diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/factory/LiveLocationShareMessageItemFactory.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/factory/LiveLocationShareMessageItemFactory.kt
@@ -114,7 +114,7 @@ class LiveLocationShareMessageItemFactory @Inject constructor(
                 .locationUrl(locationUrl)
                 .mapWidth(width)
                 .mapHeight(height)
-                .locationUserId(attributes.informationData.senderId)
+                .pinMatrixItem(attributes.informationData.matrixItem)
                 .locationPinProvider(locationPinProvider)
                 .highlighted(highlight)
                 .leftGuideline(avatarSizeProvider.leftGuideline)
diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/factory/MessageItemFactory.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/factory/MessageItemFactory.kt
@@ -233,14 +233,14 @@ class MessageItemFactory @Inject constructor(
             urlMapProvider.buildStaticMapUrl(it, INITIAL_MAP_ZOOM_IN_TIMELINE, width, height)
         }
 
-        val locationUserId = if (locationContent.isSelfLocation()) informationData.senderId else null
+        val pinMatrixItem = if (locationContent.isSelfLocation()) informationData.matrixItem else null
 
         return MessageLocationItem_()
                 .attributes(attributes)
                 .locationUrl(locationUrl)
                 .mapWidth(width)
                 .mapHeight(height)
-                .locationUserId(locationUserId)
+                .pinMatrixItem(pinMatrixItem)
                 .locationPinProvider(locationPinProvider)
                 .highlighted(highlight)
                 .leftGuideline(avatarSizeProvider.leftGuideline)
diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/helper/LocationPinProvider.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/helper/LocationPinProvider.kt
@@ -25,13 +25,10 @@ import androidx.core.graphics.drawable.DrawableCompat
 import com.bumptech.glide.request.target.CustomTarget
 import com.bumptech.glide.request.transition.Transition
 import im.vector.app.R
-import im.vector.app.core.di.ActiveSessionHolder
 import im.vector.app.core.glide.GlideApp
 import im.vector.app.core.utils.DimensionConverter
 import im.vector.app.features.home.AvatarRenderer
-import org.matrix.android.sdk.api.session.getUserOrDefault
 import org.matrix.android.sdk.api.util.MatrixItem
-import org.matrix.android.sdk.api.util.toMatrixItem
 import timber.log.Timber
 import javax.inject.Inject
 import javax.inject.Singleton
@@ -44,61 +41,53 @@ private data class CachedDrawable(
 @Singleton
 class LocationPinProvider @Inject constructor(
         private val context: Context,
-        private val activeSessionHolder: ActiveSessionHolder,
         private val dimensionConverter: DimensionConverter,
         private val avatarRenderer: AvatarRenderer,
         private val matrixItemColorProvider: MatrixItemColorProvider
 ) {
-    private val cache = LruCache<MatrixItem.UserItem, CachedDrawable>(32)
+    private val cache = LruCache<MatrixItem, CachedDrawable>(32)
 
     private val glideRequests by lazy {
         GlideApp.with(context)
     }
 
     /**
      * Creates a pin drawable. If userId is null then a generic pin drawable will be created.
-     * @param userId userId that will be used to retrieve user avatar
+     * @param matrixUser user that will be used to retrieve user avatar
      * @param callback Pin drawable will be sent through the callback
      */
-    fun create(userId: String?, callback: (Drawable) -> Unit) {
-        if (userId == null) {
+    fun create(matrixUser: MatrixItem?, callback: (Drawable) -> Unit) {
+        if (matrixUser == null) {
             callback(ContextCompat.getDrawable(context, R.drawable.ic_location_pin)!!)
             return
         }
+        val size = dimensionConverter.dpToPx(44)
+        avatarRenderer.render(glideRequests, matrixUser, object : CustomTarget<Drawable>(size, size) {
+            override fun onResourceReady(resource: Drawable, transition: Transition<in Drawable>?) {
+                Timber.d("## Location: onResourceReady")
+                val pinDrawable = createPinDrawable(matrixUser, resource, isError = false)
+                callback(pinDrawable)
+            }
 
-        activeSessionHolder
-                .getActiveSession()
-                .getUserOrDefault(userId)
-                .toMatrixItem()
-                .let { userItem ->
-                    val size = dimensionConverter.dpToPx(44)
-                    avatarRenderer.render(glideRequests, userItem, object : CustomTarget<Drawable>(size, size) {
-                        override fun onResourceReady(resource: Drawable, transition: Transition<in Drawable>?) {
-                            Timber.d("## Location: onResourceReady")
-                            val pinDrawable = createPinDrawable(userItem, resource, isError = false)
-                            callback(pinDrawable)
-                        }
+            override fun onLoadCleared(placeholder: Drawable?) {
+                // Is it possible? Put placeholder instead?
+                // FIXME The doc says it has to be implemented and should free resources
+                Timber.d("## Location: onLoadCleared")
+            }
 
-                        override fun onLoadCleared(placeholder: Drawable?) {
-                            // Is it possible? Put placeholder instead?
-                            // FIXME The doc says it has to be implemented and should free resources
-                            Timber.d("## Location: onLoadCleared")
-                        }
-
-                        override fun onLoadFailed(errorDrawable: Drawable?) {
-                            // Note: `onLoadFailed` is also called when the user has no avatarUrl
-                            // and the errorDrawable is actually the placeholder.
-                            Timber.w("## Location: onLoadFailed")
-                            errorDrawable ?: return
-                            val pinDrawable = createPinDrawable(userItem, errorDrawable, isError = true)
-                            callback(pinDrawable)
-                        }
-                    })
-                }
+            override fun onLoadFailed(errorDrawable: Drawable?) {
+                // Note: `onLoadFailed` is also called when the user has no avatarUrl
+                // and the errorDrawable is actually the placeholder.
+                Timber.w("## Location: onLoadFailed")
+                errorDrawable ?: return
+                val pinDrawable = createPinDrawable(matrixUser, errorDrawable, isError = true)
+                callback(pinDrawable)
+            }
+        })
     }
 
     private fun createPinDrawable(
-            userItem: MatrixItem.UserItem,
+            userItem: MatrixItem,
             drawable: Drawable,
             isError: Boolean,
     ): Drawable {
diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/AbsMessageLocationItem.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/AbsMessageLocationItem.kt
@@ -38,6 +38,7 @@ import im.vector.app.features.home.room.detail.timeline.style.TimelineMessageLay
 import im.vector.app.features.home.room.detail.timeline.style.granularRoundedCorners
 import im.vector.app.features.location.MapLoadingErrorView
 import im.vector.app.features.location.MapLoadingErrorViewState
+import org.matrix.android.sdk.api.util.MatrixItem
 
 abstract class AbsMessageLocationItem<H : AbsMessageLocationItem.Holder>(
         @LayoutRes layoutId: Int = R.layout.item_timeline_event_base
@@ -47,7 +48,7 @@ abstract class AbsMessageLocationItem<H : AbsMessageLocationItem.Holder>(
     var locationUrl: String? = null
 
     @EpoxyAttribute
-    var locationUserId: String? = null
+    var pinMatrixItem: MatrixItem? = null
 
     @EpoxyAttribute
     var mapWidth: Int = 0
@@ -103,7 +104,7 @@ abstract class AbsMessageLocationItem<H : AbsMessageLocationItem.Holder>(
                             dataSource: DataSource?,
                             isFirstResource: Boolean
                     ): Boolean {
-                        locationPinProvider?.create(locationUserId) { pinDrawable ->
+                        locationPinProvider?.create(pinMatrixItem) { pinDrawable ->
                             // we are not using Glide since it does not display it correctly when there is no user photo
                             holder.staticMapPinImageView.setImageDrawable(pinDrawable)
                         }
diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageLiveLocationItem.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageLiveLocationItem.kt
@@ -49,7 +49,7 @@ abstract class MessageLiveLocationItem : AbsMessageLocationItem<MessageLiveLocat
 
     private fun bindLiveLocationBanner(holder: Holder) {
         // TODO in a future PR add check on device id to confirm that is the one that sent the beacon
-        val isEmitter = currentUserId != null && currentUserId == locationUserId
+        val isEmitter = currentUserId != null && currentUserId == pinMatrixItem?.id
         val messageLayout = attributes.informationData.messageLayout
         val viewState = buildViewState(holder, messageLayout, isEmitter)
         holder.liveLocationRunningBanner.isVisible = true
diff --git a/vector/src/main/java/im/vector/app/features/location/LocationSharingViewModel.kt b/vector/src/main/java/im/vector/app/features/location/LocationSharingViewModel.kt
@@ -106,11 +106,13 @@ class LocationSharingViewModel @AssistedInject constructor(
 
     private fun updatePin(isUserPin: Boolean? = true) {
         if (isUserPin.orFalse()) {
-            locationPinProvider.create(userId = session.myUserId) {
+            val matrixItem = room.membershipService().getRoomMember(session.myUserId)?.toMatrixItem()
+                    ?: session.getUserOrDefault(session.myUserId).toMatrixItem()
+            locationPinProvider.create(matrixItem) {
                 updatePinDrawableInState(it)
             }
         } else {
-            locationPinProvider.create(userId = null) {
+            locationPinProvider.create(null) {
                 updatePinDrawableInState(it)
             }
         }
diff --git a/vector/src/main/java/im/vector/app/features/location/live/map/UserLiveLocationViewStateMapper.kt b/vector/src/main/java/im/vector/app/features/location/live/map/UserLiveLocationViewStateMapper.kt
@@ -20,8 +20,10 @@ import im.vector.app.core.di.ActiveSessionHolder
 import im.vector.app.features.home.room.detail.timeline.helper.LocationPinProvider
 import im.vector.app.features.location.toLocationData
 import kotlinx.coroutines.suspendCancellableCoroutine
+import org.matrix.android.sdk.api.session.getRoom
 import org.matrix.android.sdk.api.session.getUserOrDefault
 import org.matrix.android.sdk.api.session.room.model.livelocation.LiveLocationShareAggregatedSummary
+import org.matrix.android.sdk.api.util.MatrixItem
 import org.matrix.android.sdk.api.util.toMatrixItem
 import javax.inject.Inject
 
@@ -43,11 +45,21 @@ class UserLiveLocationViewStateMapper @Inject constructor(
                         // do nothing on cancellation
                     }
                     else -> {
-                        locationPinProvider.create(userId) { pinDrawable ->
-                            val session = activeSessionHolder.getActiveSession()
+                        val session = activeSessionHolder.getActiveSession()
+                        val roomId = liveLocationShareAggregatedSummary.roomId
+                        val matrixItem = if (roomId != null) {
+                            session.getRoom(roomId)
+                                    ?.membershipService()
+                                    ?.getRoomMember(userId)
+                                    ?.toMatrixItem()
+                                    ?: MatrixItem.UserItem(userId)
+                        } else {
+                            session.getUserOrDefault(userId).toMatrixItem()
+                        }
+                        locationPinProvider.create(matrixItem) { pinDrawable ->
                             val locationTimestampMillis = liveLocationShareAggregatedSummary.lastLocationDataContent?.getBestTimestampMillis()
                             val viewState = UserLiveLocationViewState(
-                                    matrixItem = session.getUserOrDefault(userId).toMatrixItem(),
+                                    matrixItem = matrixItem,
                                     pinDrawable = pinDrawable,
                                     locationData = locationData,
                                     endOfLiveTimestampMillis = liveLocationShareAggregatedSummary.endOfLiveTimestampMillis,
diff --git a/vector/src/main/java/im/vector/app/features/location/preview/LocationPreviewViewModel.kt b/vector/src/main/java/im/vector/app/features/location/preview/LocationPreviewViewModel.kt
@@ -30,6 +30,8 @@ import kotlinx.coroutines.flow.launchIn
 import kotlinx.coroutines.flow.onEach
 import kotlinx.coroutines.launch
 import org.matrix.android.sdk.api.session.Session
+import org.matrix.android.sdk.api.util.MatrixItem
+import org.matrix.android.sdk.api.util.toMatrixItem
 
 class LocationPreviewViewModel @AssistedInject constructor(
         @Assisted private val initialState: LocationPreviewViewState,
@@ -46,12 +48,23 @@ class LocationPreviewViewModel @AssistedInject constructor(
     companion object : MavericksViewModelFactory<LocationPreviewViewModel, LocationPreviewViewState> by hiltMavericksViewModelFactory()
 
     init {
-        initPin(initialState.pinUserId)
+        val matrixItem = if (initialState.roomId != null && initialState.pinUserId != null) {
+            session
+                    .roomService()
+                    .getRoom(initialState.roomId)
+                    ?.membershipService()
+                    ?.getRoomMember(initialState.pinUserId)
+                    ?.toMatrixItem()
+                    ?: MatrixItem.UserItem(initialState.pinUserId)
+        } else {
+            null
+        }
+        initPin(matrixItem)
         initLocationTracking()
     }
 
-    private fun initPin(userId: String?) {
-        locationPinProvider.create(userId) { pinDrawable ->
+    private fun initPin(matrixItem: MatrixItem?) {
+        locationPinProvider.create(matrixItem) { pinDrawable ->
             setState { copy(pinDrawable = pinDrawable) }
         }
     }
diff --git a/vector/src/main/java/im/vector/app/features/location/preview/LocationPreviewViewState.kt b/vector/src/main/java/im/vector/app/features/location/preview/LocationPreviewViewState.kt
@@ -23,6 +23,7 @@ import im.vector.app.features.location.LocationSharingArgs
 
 data class LocationPreviewViewState(
         val pinLocationData: LocationData? = null,
+        val roomId: String? = null,
         val pinUserId: String? = null,
         val pinDrawable: Drawable? = null,
         val loadingMapHasFailed: Boolean = false,
@@ -32,6 +33,7 @@ data class LocationPreviewViewState(
 
     constructor(args: LocationSharingArgs) : this(
             pinLocationData = args.initialLocationData,
+            roomId = args.roomId,
             pinUserId = args.locationOwnerId,
     )
 }
diff --git a/vector/src/test/java/im/vector/app/features/location/live/GetLiveLocationShareSummaryUseCaseTest.kt b/vector/src/test/java/im/vector/app/features/location/live/GetLiveLocationShareSummaryUseCaseTest.kt
@@ -54,6 +54,7 @@ class GetLiveLocationShareSummaryUseCaseTest {
     @Test
     fun `given a room id and event id when calling use case then flow on summary is returned`() = runTest {
         val summary = LiveLocationShareAggregatedSummary(
+                roomId = A_ROOM_ID,
                 userId = "userId",
                 isActive = true,
                 endOfLiveTimestampMillis = 123,
diff --git a/vector/src/test/java/im/vector/app/features/location/live/map/GetListOfUserLiveLocationUseCaseTest.kt b/vector/src/test/java/im/vector/app/features/location/live/map/GetListOfUserLiveLocationUseCaseTest.kt
@@ -59,18 +59,21 @@ class GetListOfUserLiveLocationUseCaseTest {
     @Test
     fun `given a room id then the correct flow of view states list is collected`() = runTest {
         val summary1 = LiveLocationShareAggregatedSummary(
+                roomId = A_ROOM_ID,
                 userId = "userId1",
                 isActive = true,
                 endOfLiveTimestampMillis = 123,
                 lastLocationDataContent = MessageBeaconLocationDataContent()
         )
         val summary2 = LiveLocationShareAggregatedSummary(
+                roomId = A_ROOM_ID,
                 userId = "userId2",
                 isActive = true,
                 endOfLiveTimestampMillis = 1234,
                 lastLocationDataContent = MessageBeaconLocationDataContent()
         )
         val summary3 = LiveLocationShareAggregatedSummary(
+                roomId = A_ROOM_ID,
                 userId = "userId3",
                 isActive = true,
                 endOfLiveTimestampMillis = 1234,
diff --git a/vector/src/test/java/im/vector/app/features/location/live/map/UserLiveLocationViewStateMapperTest.kt b/vector/src/test/java/im/vector/app/features/location/live/map/UserLiveLocationViewStateMapperTest.kt
@@ -72,6 +72,7 @@ class UserLiveLocationViewStateMapperTest {
     @Test
     fun `given a summary with invalid data then result is null`() = runTest {
         val summary1 = LiveLocationShareAggregatedSummary(
+                roomId = null,
                 userId = null,
                 isActive = true,
                 endOfLiveTimestampMillis = null,
@@ -98,17 +99,19 @@ class UserLiveLocationViewStateMapperTest {
                 unstableTimestampMillis = A_LOCATION_TIMESTAMP
         )
         val summary = LiveLocationShareAggregatedSummary(
+                roomId = null,
                 userId = A_USER_ID,
                 isActive = A_IS_ACTIVE,
                 endOfLiveTimestampMillis = A_END_OF_LIVE_TIMESTAMP,
                 lastLocationDataContent = locationDataContent,
         )
-        locationPinProvider.givenCreateForUserId(A_USER_ID, pinDrawable)
+        val matrixItem = MatrixItem.UserItem(id = A_USER_ID, displayName = A_USER_DISPLAY_NAME, avatarUrl = "")
+        locationPinProvider.givenCreateForMatrixItem(matrixItem, pinDrawable)
 
         val viewState = userLiveLocationViewStateMapper.map(summary)
 
         val expectedViewState = UserLiveLocationViewState(
-                matrixItem = MatrixItem.UserItem(id = A_USER_ID, displayName = A_USER_DISPLAY_NAME, avatarUrl = ""),
+                matrixItem = matrixItem,
                 pinDrawable = pinDrawable,
                 locationData = LocationData(
                         latitude = A_LATITUDE,
diff --git a/vector/src/test/java/im/vector/app/test/fakes/FakeLocationPinProvider.kt b/vector/src/test/java/im/vector/app/test/fakes/FakeLocationPinProvider.kt

Original file line number	Diff line number	Diff line change
`@@ -103,7 +103,8 @@ abstract class BottomSheetMessagePreviewItem : VectorEpoxyModel<BottomSheetMessa`
`103`	`103`	`.apply(RequestOptions.centerCropTransform())`
`104`	`104`	`.into(holder.staticMapImageView)`
`105`	`105`
`106`		`- safeLocationUiData.locationPinProvider.create(safeLocationUiData.locationOwnerId) { pinDrawable ->`
	`106`	`+ val pinMatrixItem = matrixItem.takeIf { safeLocationUiData.locationOwnerId != null }`
	`107`	`+ safeLocationUiData.locationPinProvider.create(pinMatrixItem) { pinDrawable ->`
`107`	`108`	`// we are not using Glide since it does not display it correctly when there is no user photo`
`108`	`109`	`holder.staticMapPinImageView.setImageDrawable(pinDrawable)`
`109`	`110`	`}`