Integrate Element Call with widget API #1581

jmartinesp · 2023-10-16T10:06:47Z

Type of change

Feature
Bugfix
Technical
Other :

Content

Lots of changes, sorry for the huge PR:

Added a feature flag to enable/disable the EC widget integration. Disabling it will hide the create/join call button in the chat room screen.
Moved logic from ElementCallActivity to CallScreenPresenter.
Created CallType to separate EC SPA calls from internal room calls.
Created associated CallScreenPresenter for CallScreenView.
Integrated the Widget Driver Rust code with the WebView using RustWidgetDriver and WebViewWidgetMessageInterceptor, which relay messages from Rust to the widget in the WebView and viceversa.
Added an EC base URL customization setting in advanced settings and the related UI components.
Created an appconfig module to store config values the clients might need to customize (including the EC default base url).
Added lots of abstractions so the code could be properly tested.

Motivation and context

We needed to integrate the EC Widget soon, since the branch was starting to drift apart from develop. With this, the base API and integration of the widget API should be done, although we still need the final Rust code to be merged and included for the calls to work properly (note that this was tested with another branch with a PoC of that code and calls worked fine besides some bugs).

Screenshots / GIFs

These are included in the changed files.

Tests

Go to Settings -> Developer options. Enable 'Element call in rooms'.
Go to any room.
Join a call by tapping on the 'video call' icon (the call will get stuck at 'loading...').
Go back to Settings -> Advanced settings. Customize the EC base URL.
Join another call, make sure the base URL is used (the call will probably be completely broken).

Tested devices

Physical
Emulator
OS version(s): 11

Checklist

Changes have been tested on an Android device or Android emulator with API 23
UI change has been tested on both light and dark themes
Accessibility has been taken into account. See https://github.com/vector-im/element-x-android/blob/develop/CONTRIBUTING.md#accessibility
Pull request is based on the develop branch
Pull request includes a new file under ./changelog.d. See https://github.com/vector-im/element-x-android/blob/develop/CONTRIBUTING.md#changelog
Pull request includes screenshots or videos if containing UI changes
Pull request includes a sign off
You've made a self review of your PR

github-actions · 2023-10-16T10:31:07Z

📱 Scan the QR code below to install the build (arm64 only) for this PR.

If you can't scan the QR code you can install the build via this link: https://i.diawi.com/AdX6GV

codecov · 2023-10-16T14:55:25Z

Codecov Report

Attention: 266 lines in your changes are missing coverage. Please review.

Comparison is base (a814c4a) 58.92% compared to head (aeb91e7) 58.88%.

❗ Current head aeb91e7 differs from pull request most recent head 4ab5611. Consider uploading reports for the commit 4ab5611 to get more accurate results

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #1581      +/-   ##
===========================================
- Coverage    58.92%   58.88%   -0.05%     
===========================================
  Files         1202     1219      +17     
  Lines        30989    31414     +425     
  Branches      6345     6443      +98     
===========================================
+ Hits         18261    18498     +237     
- Misses        9981    10113     +132     
- Partials      2747     2803      +56

Files	Coverage Δ
...o/element/android/appconfig/MatrixConfiguration.kt	`100.00% <ø> (ø)`
...ent/android/features/call/CallForegroundService.kt	`0.00% <ø> (ø)`
...otlin/io/element/android/features/call/CallType.kt	`100.00% <100.00%> (ø)`
...lement/android/features/call/data/WidgetMessage.kt	`100.00% <100.00%> (ø)`
...lement/android/features/call/ui/CallScreeEvents.kt	`100.00% <100.00%> (ø)`
...lement/android/features/call/ui/CallScreenState.kt	`100.00% <100.00%> (ø)`
...ndroid/features/call/utils/CallIntentDataParser.kt	`95.23% <ø> (ø)`
.../android/features/call/utils/CallWidgetProvider.kt	`100.00% <100.00%> (ø)`
...d/features/call/utils/DefaultCallWidgetProvider.kt	`100.00% <100.00%> (ø)`
...oid/features/call/utils/WidgetMessageSerializer.kt	`100.00% <100.00%> (ø)`
... and 44 more

... and 12 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

bmarty

Great work, thanks!
I have made some (minor) remarks.
Also I have tested what could be tested on my emu, and this is working as written in the description.

bmarty · 2023-10-19T10:28:41Z

appnav/src/main/kotlin/io/element/android/appnav/RootFlowNode.kt

@@ -63,7 +63,7 @@ import kotlinx.parcelize.Parcelize
 import timber.log.Timber

 @ContributesNode(AppScope::class)
-class RootFlowNode @AssistedInject constructor(
+        class RootFlowNode @AssistedInject constructor(


Small formatting issue.

bmarty · 2023-10-19T10:29:08Z

appconfig/build.gradle.kts

+ */
+plugins {
+    id("java-library")
+    id("com.android.lint")


We do not need lint here I think

bmarty · 2023-10-19T10:29:27Z

appconfig/build.gradle.kts

+dependencies {
+    testImplementation(libs.test.junit)
+    testImplementation(libs.test.truth)
+}


Maybe remove the dependencies block, this is not used

bmarty · 2023-10-19T10:33:15Z

features/call/src/main/kotlin/io/element/android/features/call/ui/CallScreenPresenter.kt

+    private val clock: SystemClock,
+    private val dispatchers: CoroutineDispatchers,
+    @Assisted private val callType: CallType,
+    @Assisted private val navigator: CallScreenNavigator,


Neat: in the project, we use to put the @Assisted parameters first. Maybe a new Konsist test could check that.

bmarty · 2023-10-19T10:34:27Z

features/call/src/main/kotlin/io/element/android/features/call/ui/CallScreenPresenter.kt

+
+    @AssistedFactory
+    interface Factory {
+        fun create(inputs: CallType, navigator: CallScreenNavigator): CallScreenPresenter


Suggested change

fun create(inputs: CallType, navigator: CallScreenNavigator): CallScreenPresenter

fun create(callType: CallType, navigator: CallScreenNavigator): CallScreenPresenter

?

bmarty · 2023-10-19T11:03:00Z

features/call/src/test/kotlin/io/element/android/features/call/ui/CallScreenPresenterTest.kt

+
+        fun givenInterceptedMessage(message: String) {
+            interceptedMessages.tryEmit(message)
+        }


Maybe extract all those Fake classes to their own file?

bmarty · 2023-10-19T11:04:15Z

...ures/call/src/test/kotlin/io/element/android/features/call/utils/CallIntentDataParserTest.kt


-import com.google.common.truth.Truth.assertThat
+import com.google.common.truth.Truth


I prefer the previous import, the code looks cleaner. But happy with this until we decide and add a Konsist test for this :)

bmarty · 2023-10-19T11:04:50Z

...ain/kotlin/io/element/android/features/login/impl/accountprovider/AccountProviderProvider.kt

@@ -32,7 +31,7 @@ open class AccountProviderProvider : PreviewParameterProvider<AccountProvider> {
 }

 fun anAccountProvider() = AccountProvider(
-    url = LoginConstants.MATRIX_ORG_URL,
+    url = io.element.android.appconfig.AuthenticationConfig.MATRIX_ORG_URL,


Maybe add an import? (probably due to refacto)

Yes, the refactor broke the imports somehow.

bmarty · 2023-10-19T11:05:14Z

...ement/android/features/login/impl/screens/searchaccountprovider/SearchAccountProviderView.kt

@@ -196,7 +195,7 @@ fun SearchAccountProviderView(

 @Composable
 private fun HomeserverData.toAccountProvider(): AccountProvider {
-    val isMatrixOrg = homeserverUrl == LoginConstants.MATRIX_ORG_URL
+    val isMatrixOrg = homeserverUrl == io.element.android.appconfig.AuthenticationConfig.MATRIX_ORG_URL


bmarty · 2023-10-19T11:07:02Z

...ures/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesView.kt

+        actions = {
+            if (inRoomCallsEnabled) {
+                IconButton(onClick = onJoinCallClicked) {
+                    Icon(CommonDrawables.ic_compound_video_call, contentDescription = "Join call")


i18n for the content description? This is maybe a temporary string, since the button is also used to create a call.

I've removed it until we can get the call state and decide whether we should display join/create call here.

ElementBot · 2023-10-19T13:38:25Z

	Warnings
⚠️	libraries/ui-strings/src/main/res/values-cs/translations.xml#L187 - For locale "cs" (Czech) the following quantity should also be defined: `many` (e.g. "10.0 dne")
⚠️	libraries/ui-strings/src/main/res/values-cs/translations.xml#L192 - For locale "cs" (Czech) the following quantity should also be defined: `many` (e.g. "10.0 dne")
⚠️	libraries/ui-strings/src/main/res/values-sk/translations.xml#L187 - For locale "sk" (Slovak) the following quantity should also be defined: `many` (e.g. "10.0 dňa")
⚠️	libraries/ui-strings/src/main/res/values-sk/translations.xml#L192 - For locale "sk" (Slovak) the following quantity should also be defined: `many` (e.g. "10.0 dňa")

Generated by 🚫 dangerJS against 4ab5611

- Add `appconfig` module and extract constants that can be overridden in forks there. - Add an Element Call feature flag, disabled by default. - Refactor the whole `ElementCallActivity`, move most logic out of it. - Integrate with the Rust Widget Driver API (note the Rust SDK version used in this PR lacks some needed changes to make the calls actually work). - Handle calls differently based on `CallType`. - Add UI to create/join a call.

sonarqubecloud · 2023-10-19T15:14:05Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

jmartinesp force-pushed the feature/jme/call-widget-integration branch 3 times, most recently from e361242 to 0731578 Compare October 17, 2023 14:11

jmartinesp changed the title ~~Integrate Element Call with widget API (needs Rust SDK updates)~~ Integrate Element Call with widget API Oct 18, 2023

jmartinesp marked this pull request as ready for review October 18, 2023 08:30

jmartinesp requested a review from a team as a code owner October 18, 2023 08:30

jmartinesp requested review from bmarty and removed request for a team October 18, 2023 08:30

bmarty approved these changes Oct 19, 2023

View reviewed changes

jmartinesp force-pushed the feature/jme/call-widget-integration branch from aeb91e7 to 1d6f935 Compare October 19, 2023 12:21

jmartinesp enabled auto-merge (squash) October 19, 2023 12:22

jmartinesp force-pushed the feature/jme/call-widget-integration branch from 1d6f935 to 874c137 Compare October 19, 2023 13:36

jmartinesp and others added 2 commits October 19, 2023 16:56

Update screenshots

4ab5611

jmartinesp disabled auto-merge October 19, 2023 14:57

jmartinesp force-pushed the feature/jme/call-widget-integration branch from f77ba1d to 4ab5611 Compare October 19, 2023 14:57

jmartinesp enabled auto-merge (squash) October 19, 2023 14:58

jmartinesp merged commit 46f78ef into develop Oct 19, 2023
12 checks passed

jmartinesp deleted the feature/jme/call-widget-integration branch October 19, 2023 15:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Integrate Element Call with widget API #1581

Integrate Element Call with widget API #1581

jmartinesp commented Oct 16, 2023 •

edited

Loading

github-actions bot commented Oct 16, 2023 •

edited

Loading

codecov bot commented Oct 16, 2023 •

edited

Loading

bmarty left a comment

bmarty Oct 19, 2023

bmarty Oct 19, 2023

bmarty Oct 19, 2023

bmarty Oct 19, 2023

bmarty Oct 19, 2023

bmarty Oct 19, 2023

bmarty Oct 19, 2023

bmarty Oct 19, 2023

jmartinesp Oct 19, 2023

bmarty Oct 19, 2023

bmarty Oct 19, 2023

jmartinesp Oct 19, 2023

ElementBot commented Oct 19, 2023 •

edited

Loading

sonarqubecloud bot commented Oct 19, 2023

	fun create(inputs: CallType, navigator: CallScreenNavigator): CallScreenPresenter
	fun create(callType: CallType, navigator: CallScreenNavigator): CallScreenPresenter


		import com.google.common.truth.Truth.assertThat
		import com.google.common.truth.Truth

Integrate Element Call with widget API #1581

Integrate Element Call with widget API #1581

Conversation

jmartinesp commented Oct 16, 2023 • edited Loading

Type of change

Content

Motivation and context

Screenshots / GIFs

Tests

Tested devices

Checklist

github-actions bot commented Oct 16, 2023 • edited Loading

codecov bot commented Oct 16, 2023 • edited Loading

Codecov Report

bmarty left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ElementBot commented Oct 19, 2023 • edited Loading

sonarqubecloud bot commented Oct 19, 2023

jmartinesp commented Oct 16, 2023 •

edited

Loading

github-actions bot commented Oct 16, 2023 •

edited

Loading

codecov bot commented Oct 16, 2023 •

edited

Loading

ElementBot commented Oct 19, 2023 •

edited

Loading