Skip to content

Commit

Permalink
feat: do not display call feed back for short calls (WPB-15086) (#3277)
Browse files Browse the repository at this point in the history
* feat: do not display call feed back for short calls

* feat: detekt

* feat: unit test

* chore: address comments
  • Loading branch information
ohassine authored Feb 18, 2025
1 parent 7f5111d commit 4abd575
Show file tree
Hide file tree
Showing 10 changed files with 135 additions and 45 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -451,7 +451,7 @@ internal class CallDataSource(
val updatedCallMetadata = callMetadataProfile.data.toMutableMap().apply {
this[conversationId] = call.copy(
participants = participants,
maxParticipants = max(call.maxParticipants, participants.size + 1),
maxParticipants = max(call.maxParticipants, participants.size),
users = updatedUsers,
screenShareMetadata = updateScreenSharingMetadata(
metadata = call.screenShareMetadata,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ import com.wire.kalium.logic.feature.call.usecase.IsLastCallClosedUseCaseImpl
import com.wire.kalium.logic.feature.call.usecase.MuteCallUseCase
import com.wire.kalium.logic.feature.call.usecase.MuteCallUseCaseImpl
import com.wire.kalium.logic.feature.call.usecase.ObserveAskCallFeedbackUseCase
import com.wire.kalium.logic.feature.call.usecase.observeAskCallFeedbackUseCase
import com.wire.kalium.logic.feature.call.usecase.ObserveConferenceCallingEnabledUseCase
import com.wire.kalium.logic.feature.call.usecase.ObserveConferenceCallingEnabledUseCaseImpl
import com.wire.kalium.logic.feature.call.usecase.ObserveEndCallDueToConversationDegradationUseCase
Expand Down Expand Up @@ -229,7 +230,7 @@ class CallsScope internal constructor(
val observeEndCallDueToDegradationDialog: ObserveEndCallDueToConversationDegradationUseCase
get() = ObserveEndCallDueToConversationDegradationUseCaseImpl(EndCallResultListenerImpl)
val observeAskCallFeedbackUseCase: ObserveAskCallFeedbackUseCase
get() = ObserveAskCallFeedbackUseCase(EndCallResultListenerImpl)
get() = observeAskCallFeedbackUseCase(EndCallResultListenerImpl)

private val shouldAskCallFeedback: ShouldAskCallFeedbackUseCase by lazy {
ShouldAskCallFeedbackUseCase(userConfigRepository)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ package com.wire.kalium.logic.feature.call.usecase
import com.wire.kalium.logic.data.call.CallMetadata
import com.wire.kalium.logic.data.call.CallRepository
import com.wire.kalium.logic.data.call.CallScreenSharingMetadata
import com.wire.kalium.logic.data.call.CallStatus
import com.wire.kalium.logic.data.call.RecentlyEndedCallMetadata
import com.wire.kalium.logic.data.id.ConversationId
import com.wire.kalium.logic.data.id.SelfTeamIdProvider
Expand Down Expand Up @@ -57,7 +58,7 @@ class CreateAndPersistRecentlyEndedCallMetadataUseCaseImpl internal constructor(
val conversationServicesCount = conversationMembers.count { member -> member.user.userType == UserType.SERVICE }
val guestsCount = conversationMembers.count { member -> member.user.userType == UserType.GUEST }
val guestsProCount = conversationMembers.count { member -> member.user.userType == UserType.GUEST && member.user.teamId != null }
val isOutgoingCall = callerId.value == selfCallUser?.id?.value
val isOutgoingCall = callStatus == CallStatus.STARTED
val callDurationInSeconds = establishedTime?.let {
DateTimeUtil.calculateMillisDifference(it, DateTimeUtil.currentIsoDateTimeString()) / MILLIS_IN_SECOND
} ?: 0L
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,14 @@
*/
package com.wire.kalium.logic.feature.call.usecase

import com.wire.kalium.logic.feature.user.ShouldAskCallFeedbackUseCaseResult
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.MutableSharedFlow

interface EndCallResultListener {
suspend fun observeCallEndedResult(): Flow<EndCallResult>
suspend fun onCallEndedBecauseOfVerificationDegraded()
suspend fun onCallEndedAskForFeedback(shouldAsk: Boolean)
suspend fun onCallEndedAskForFeedback(shouldAskCallFeedback: ShouldAskCallFeedbackUseCaseResult)
}

/**
Expand All @@ -39,12 +40,12 @@ object EndCallResultListenerImpl : EndCallResultListener {
conversationCallEnded.emit(EndCallResult.VerificationDegraded)
}

override suspend fun onCallEndedAskForFeedback(shouldAsk: Boolean) {
conversationCallEnded.emit(EndCallResult.AskForFeedback(shouldAsk))
override suspend fun onCallEndedAskForFeedback(shouldAskCallFeedback: ShouldAskCallFeedbackUseCaseResult) {
conversationCallEnded.emit(EndCallResult.AskForFeedback(shouldAskCallFeedback))
}
}

sealed class EndCallResult {
data object VerificationDegraded : EndCallResult()
data class AskForFeedback(val shouldAsk: Boolean) : EndCallResult()
data class AskForFeedback(val shouldAskCallFeedback: ShouldAskCallFeedbackUseCaseResult) : EndCallResult()
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import com.wire.kalium.util.KaliumDispatcher
import com.wire.kalium.util.KaliumDispatcherImpl
import kotlinx.coroutines.flow.first
import kotlinx.coroutines.withContext
import kotlinx.datetime.toInstant

/**
* This use case is responsible for ending a call.
Expand Down Expand Up @@ -56,7 +57,7 @@ internal class EndCallUseCaseImpl(
* @param conversationId the id of the conversation for the call should be ended.
*/
override suspend operator fun invoke(conversationId: ConversationId) = withContext(dispatchers.default) {
callRepository.callsFlow().first().find {
val endedCall = callRepository.callsFlow().first().find {
// This use case can be invoked while joining the call or when the call is established.
it.conversationId == conversationId && it.status in listOf(
CallStatus.STARTED,
Expand All @@ -72,10 +73,11 @@ internal class EndCallUseCaseImpl(
callingLogger.d("[EndCallUseCase] -> Updating call status to CLOSED")
callRepository.updateCallStatusById(conversationId, CallStatus.CLOSED)
}
it
}

callManager.value.endCall(conversationId)
callRepository.updateIsCameraOnById(conversationId, false)
endCallListener.onCallEndedAskForFeedback(shouldAskCallFeedback())
endCallListener.onCallEndedAskForFeedback(shouldAskCallFeedback(endedCall?.establishedTime?.toInstant()))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,26 +17,26 @@
*/
package com.wire.kalium.logic.feature.call.usecase

import com.wire.kalium.logic.feature.user.ShouldAskCallFeedbackUseCaseResult
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.filterIsInstance
import kotlinx.coroutines.flow.map

/**
* The useCase for observing when the ongoing call was ended because of degradation of conversation verification status (Proteus or MLS)
* Use case to observe if we should ask for feedback after the call has ended.
*/
interface ObserveAskCallFeedbackUseCase {
/**
* @return [Flow] that emits only when the call was ended because of degradation of conversation verification status (Proteus or MLS)
* @return [Flow] that emits [ShouldAskCallFeedbackUseCaseResult] when the call has ended and we should ask for feedback.
*/
suspend operator fun invoke(): Flow<Boolean>
suspend operator fun invoke(): Flow<ShouldAskCallFeedbackUseCaseResult>
}

@Suppress("FunctionNaming")
internal fun ObserveAskCallFeedbackUseCase(
internal fun observeAskCallFeedbackUseCase(
endCallListener: EndCallResultListener
) = object : ObserveAskCallFeedbackUseCase {
override suspend fun invoke(): Flow<Boolean> =
override suspend fun invoke(): Flow<ShouldAskCallFeedbackUseCaseResult> =
endCallListener.observeCallEndedResult()
.filterIsInstance(EndCallResult.AskForFeedback::class)
.map { it.shouldAsk }
.map { it.shouldAskCallFeedback }
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see http://www.gnu.org/licenses/.
*
*/
package com.wire.kalium.logic.feature.user

Expand All @@ -24,23 +25,60 @@ import com.wire.kalium.util.DateTimeUtil
import kotlinx.datetime.Instant

/**
* Use case that returns [Boolean] if user should be asked for a feedback about call quality or not.
* Use case to determine if the call feedback should be asked.
*/
interface ShouldAskCallFeedbackUseCase {
/**
* @return [Boolean] if user should be asked for a feedback about call quality or not.
*/
suspend operator fun invoke(): Boolean
suspend operator fun invoke(
establishedTime: Instant?,
currentTime: Instant = DateTimeUtil.currentInstant()
): ShouldAskCallFeedbackUseCaseResult
}

@Suppress("FunctionNaming")
internal fun ShouldAskCallFeedbackUseCase(
userConfigRepository: UserConfigRepository
) = object : ShouldAskCallFeedbackUseCase {

override suspend fun invoke(): Boolean =
userConfigRepository.getNextTimeForCallFeedback().map {
override suspend fun invoke(
establishedTime: Instant?,
currentTime: Instant
): ShouldAskCallFeedbackUseCaseResult {
val callDurationInSeconds = establishedTime?.let {
DateTimeUtil.calculateMillisDifference(it, currentTime) / MILLIS_IN_SECOND
} ?: 0L

return when {
callDurationInSeconds in 1..<ONE_MINUTE -> {
ShouldAskCallFeedbackUseCaseResult.ShouldNotAskCallFeedback.CallDurationIsLessThanOneMinute(callDurationInSeconds)
}

!isNextTimeForCallFeedbackReached() -> {
ShouldAskCallFeedbackUseCaseResult.ShouldNotAskCallFeedback.NextTimeForCallFeedbackIsNotReached(callDurationInSeconds)
}

else -> {
ShouldAskCallFeedbackUseCaseResult.ShouldAskCallFeedback(callDurationInSeconds)
}
}
}

private suspend fun isNextTimeForCallFeedbackReached(): Boolean {
return userConfigRepository.getNextTimeForCallFeedback().map {
it > 0L && DateTimeUtil.currentInstant() > Instant.fromEpochMilliseconds(it)
}.getOrElse(true)
}
}

sealed class ShouldAskCallFeedbackUseCaseResult {
data class ShouldAskCallFeedback(val callDurationInSeconds: Long) : ShouldAskCallFeedbackUseCaseResult()
sealed class ShouldNotAskCallFeedback(val reason: String) : ShouldAskCallFeedbackUseCaseResult() {
data class CallDurationIsLessThanOneMinute(val callDurationInSeconds: Long) :
ShouldNotAskCallFeedback("Call duration is less than 1 minute")

data class NextTimeForCallFeedbackIsNotReached(val callDurationInSeconds: Long) :
ShouldNotAskCallFeedback("Next time for call feedback is not reached")
}
}

private const val MILLIS_IN_SECOND = 1_000L
private const val ONE_MINUTE = 60
Original file line number Diff line number Diff line change
Expand Up @@ -161,12 +161,29 @@ class CreateAndPersistRecentlyEndedCallMetadataUseCaseTest {

fun withOutgoingCall() = apply {
every { callRepository.getCallMetadataProfile() }
.returns(CallMetadataProfile(mapOf(CONVERSATION_ID to callMetadata())))
.returns(
CallMetadataProfile(
mapOf(
CONVERSATION_ID to callMetadata().copy(
callStatus = CallStatus.STARTED
)
)
)
)
}

fun withIncomingCall() = apply {
every { callRepository.getCallMetadataProfile() }
.returns(CallMetadataProfile(mapOf(CONVERSATION_ID to callMetadata().copy(callerId = CALLER_ID.copy(value = "external")))))
.returns(
CallMetadataProfile(
mapOf(
CONVERSATION_ID to callMetadata().copy(
callerId = CALLER_ID.copy(value = "external"),
callStatus = CallStatus.INCOMING
)
)
)
)
}

suspend fun withConversationMembers() = apply {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import com.wire.kalium.logic.data.conversation.Conversation
import com.wire.kalium.logic.data.id.ConversationId
import com.wire.kalium.logic.data.user.UserId
import com.wire.kalium.logic.feature.user.ShouldAskCallFeedbackUseCase
import com.wire.kalium.logic.feature.user.ShouldAskCallFeedbackUseCaseResult
import com.wire.kalium.logic.test_util.TestKaliumDispatcher
import com.wire.kalium.logic.util.arrangement.repository.CallManagerArrangement
import com.wire.kalium.logic.util.arrangement.repository.CallManagerArrangementImpl
Expand Down Expand Up @@ -69,7 +70,9 @@ class EndCallUseCaseTest {
}.wasInvoked(once)

coVerify {
arrangement.endCallResultListener.onCallEndedAskForFeedback(eq(false))
arrangement.endCallResultListener.onCallEndedAskForFeedback(
eq(ShouldAskCallFeedbackUseCaseResult.ShouldAskCallFeedback(100))
)
}.wasInvoked(once)
}

Expand Down Expand Up @@ -100,7 +103,9 @@ class EndCallUseCaseTest {
}.wasInvoked(once)

coVerify {
arrangement.endCallResultListener.onCallEndedAskForFeedback(eq(false))
arrangement.endCallResultListener.onCallEndedAskForFeedback(
eq(ShouldAskCallFeedbackUseCaseResult.ShouldAskCallFeedback(100))
)
}.wasInvoked(once)
}

Expand Down Expand Up @@ -131,7 +136,9 @@ class EndCallUseCaseTest {
}.wasInvoked(once)

coVerify {
arrangement.endCallResultListener.onCallEndedAskForFeedback(eq(false))
arrangement.endCallResultListener.onCallEndedAskForFeedback(
eq(ShouldAskCallFeedbackUseCaseResult.ShouldAskCallFeedback(100))
)
}.wasInvoked(once)
}

Expand Down Expand Up @@ -162,7 +169,9 @@ class EndCallUseCaseTest {
}.wasInvoked(once)

coVerify {
arrangement.endCallResultListener.onCallEndedAskForFeedback(eq(false))
arrangement.endCallResultListener.onCallEndedAskForFeedback(
eq(ShouldAskCallFeedbackUseCaseResult.ShouldAskCallFeedback(100))
)
}.wasInvoked(once)
}

Expand Down Expand Up @@ -192,7 +201,9 @@ class EndCallUseCaseTest {
}.wasNotInvoked()

coVerify {
arrangement.endCallResultListener.onCallEndedAskForFeedback(eq(false))
arrangement.endCallResultListener.onCallEndedAskForFeedback(
eq(ShouldAskCallFeedbackUseCaseResult.ShouldAskCallFeedback(100))
)
}.wasInvoked(once)
}

Expand Down Expand Up @@ -220,8 +231,10 @@ class EndCallUseCaseTest {
)
}

suspend fun withShouldAskCallFeedback(should: Boolean = false) {
coEvery { shouldAskCallFeedback.invoke() }.returns(should)
suspend fun withShouldAskCallFeedback(
result: ShouldAskCallFeedbackUseCaseResult = ShouldAskCallFeedbackUseCaseResult.ShouldAskCallFeedback(100)
) {
coEvery { shouldAskCallFeedback.invoke(any(), any()) }.returns(result)
}

suspend fun withOnCallEndedAskForFeedback() {
Expand Down
Loading

0 comments on commit 4abd575

Please sign in to comment.