Start DM on first message (UI) #6051

Florian14 · 2022-05-13T16:17:00Z

Type of change

WIP Feature
Bugfix
Technical
Other :

Content

This PR displays a "fake" local timeline after selecting the users to create a DM. Sending a message or event is not handled yet, so the real room cannot be created for now.
The flow is hidden behind a debug feature flag accessible from the debug menu.

Motivation and context

UI: #5525

Screenshots / GIFs

start_dm_on_first_msg.mp4

Tests

Activate the start DM feature in the debug menu -> Features
On the home screen, select the first tab (Direct Messages)
Click on the fab button to start a DM, select several users and press "GO"
A kind of timeline should now appear, there is no action in the toolbar (except the close button)
It is not possible to access the room settings or details
Composer actions are not working (an error appears when we try to send an event to the room)
When we go back to the room list or relaunch the app, the "fake" room is deleted from the DB and disappears from the room list

Tested devices

Physical
Emulator
OS version(s): API 32

Checklist

Changes has been tested on an Android device or Android emulator with API 21
UI change has been tested on both light and dark themes
Accessibility has been taken into account. See https://github.com/vector-im/element-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-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
If you have modified the screen flow, or added new screens to the application, you have updated the test UiAllScreensSanityTest.allScreensTest()

github-actions · 2022-05-13T16:46:41Z

Unit Test Results

124 files ±0 124 suites ±0 2m 22s ⏱️ -3s
220 tests ±0 220 ✔️ ±0 0 💤 ±0 0 ❌ ±0
726 runs ±0 726 ✔️ ±0 0 💤 ±0 0 ❌ ±0

Results for commit 521ecfb. ± Comparison against base commit 8b2d6c8.

♻️ This comment has been updated with latest results.

...-android/src/main/java/org/matrix/android/sdk/internal/session/room/create/CreateRoomTask.kt

...oid/src/main/java/org/matrix/android/sdk/internal/session/room/create/CreateLocalRoomTask.kt

ouchadam · 2022-05-19T12:49:52Z

we can use the .wip extension for creating work in progress (behind a feature flag) change log entries

vector/src/main/res/layout/item_timeline_event_merged_room_creation_stub.xml

...oid/src/main/java/org/matrix/android/sdk/internal/session/room/create/CreateLocalRoomTask.kt

ouchadam · 2022-05-23T16:28:34Z

...oid/src/main/java/org/matrix/android/sdk/internal/session/room/create/CreateLocalRoomTask.kt

+
+        // Wait for room to be created in DB
+        try {
+            awaitNotEmptyResult(realmConfiguration, TimeUnit.MINUTES.toMillis(1L)) { realm ->


for my understanding, are we relying on another task to asynchronously update the room entity? if possible it would be safer to sequentially run the tasks rather than rely on a timeout (is this a common pattern in the sdk?)

@ouchadam I agree with you, I'd like to remove it but I have a non-systematic but recurrent crash when I try to navigate to the local room (in the stacktrace, it seems that we try to navigate to un unknown room). It's a bit confusing because the awaitTransaction seems ok, I can log the entity before returning the roomId from the task. Did I miss something else?

I'm not too familiar with this area, would need to delegate to @bmarty / @ganfra to answer that

it looks like other tasks are using the same pattern of awaitNotEmptyResult, not a blocker for me, was curious 😄

The second task is waiting to get the room from the sync but in the current case, the room is created locally and put in DB, so we should not have to wait for it 😕

Sorry for the cross conversations, I did not see @ouchadam 's comments at first sight.
Please see my suggestion at #6051 (comment)

Maybe just replacing with this line can help (I let you do some test):

// Ensure that Realm is up to date before returning the roomId. monarchy.doWithRealm { it.refresh() }

It continues crashing...

ouchadam · 2022-05-25T09:12:39Z

matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/room/RoomService.kt

+     * This room will not be synchronized with the server and will not come back from the sync, so all the events related to this room will be generated
+     * locally.
+     */
+    suspend fun createLocalRoom(createRoomParams: CreateRoomParams): String


thinking out loud about about the api design~ not a blocker!

from a consumer api perspective, are local rooms semantically the same as normal rooms? would we handle interacting with rooms differently if they're normal vs local?

I'm wondering if CreateRoomParams could have a isLocal option and reusing createRoom and leaveRoom 🤔

ouchadam

LGTM 💯

fedrunov

LGTM

Add CreateLocalRoomTask interface and remove DI annotations

* develop: (174 commits) Bump libphonenumber from 8.12.50 to 8.12.51 LoadRoomMember: fix presence Cleanup LoadRoomMembers: add changelog LoadRoomMembers: handle room member event a bit more efficiently LoadRoomMembers: exclude Membership.Leave LoadRoomMembers: divide by chunk Bump soloader from 0.10.3 to 0.10.4 Code review fix. Try no using the gradle daemon on CI Harmonize values of `CI_GRADLE_ARG_PROPERTIES` removing unused dependencies and marking soloader and ignored from dependency check (as it's dynamic) Remove non necessary prefix in logs Adding changelog entry Updating the unit tests Stopping existing active live when starting a new one Avoid multiple PR from Dependabot when Flipper is upgraded. Change context inside the get live summary use case Use a TestDispatcher in the FakeSession Code review fixes. ...

* develop: (91 commits) Remove unused import Update versions Update CHANGES Improve readability. Weblate: also clean trads Changelog Format file Add android:hasFragileUserData="true" to the manifest. See details in #2352 Clean the TODO delete (UnusedResource not compatible with string template) showing a toast on password reset confirmation lifting duplicated event_base layout to the base class, with the option for children to override using vector model for consistency Replace 5 manual steps to 1 command line step Translated using Weblate (Italian) Translated using Weblate (Swedish) Translated using Weblate (Swedish) Translated using Weblate (Russian) extracting common breaker background selection to ftue extensions removing unused imports adding changelog entry ...

bmarty

I focused my review on the SDK part.

...droid/src/main/java/org/matrix/android/sdk/api/session/room/model/localecho/RoomLocalEcho.kt

bmarty · 2022-07-04T09:54:27Z

...oid/src/main/java/org/matrix/android/sdk/internal/session/room/create/CreateLocalRoomTask.kt

+            createLocalRoomSummaryEntity(realm, roomId, createRoomBody)
+        }
+
+        // Wait for room to be created in DB


I do not think we need to wait in this case. We are not waiting for a sync response including the new room.

@bmarty I agree with you but during my tests, I had a frequent crash because the roomId was returned before having the object in DB. Adam did the same comment, did I miss something?

Maybe just replacing with this line can help (I let you do some test):

// Ensure that Realm is up to date before returning the roomId. monarchy.doWithRealm { it.refresh() }

...-android/src/main/java/org/matrix/android/sdk/internal/session/room/create/CreateRoomTask.kt

...oid/src/main/java/org/matrix/android/sdk/internal/session/room/delete/DeleteLocalRoomTask.kt

...droid/src/main/java/org/matrix/android/sdk/internal/session/room/timeline/DefaultTimeline.kt

vector/src/debug/java/im/vector/app/features/debug/features/DebugVectorFeatures.kt

...oid/src/main/java/org/matrix/android/sdk/internal/session/room/create/CreateLocalRoomTask.kt

* develop: (156 commits) adding test case for showing html entities are processed adding tests around the event html rendering - the test helper is a little hacky in order to covert the spans to something human readable removing extra line adding changelog entry overriding the default list handler with an implementation that takes into account the initial starting position trigger CI Use executeTransactionAwait (need realm refresh in this case) Bump flipper from 0.152.0 to 0.153.0 Use executeTransactionAwait (need realm refresh in this case) generating 1.4.27 changelog and updating version Fixing crash when sharing plain text, such as a url Fix crashes when opening Thread (#6463) Timeline: fix validation of timeline event changes Fix ConcurrentModificationException on BackgroundDetectionObserver Fix crashes when opening Thread (#6463) suppressing unused string resource Changelog Fix ConcurrentModificationException on BackgroundDetectionObserver Fix typo adding changelog entry ...

gaelledel

Approved given we change the title to "Direct message" (singular)

This reverts commit 8a5a47c.

* develop: Group if together io.element.android:opusencoder:1.0.3 - should fix lint false positive issues Indentation, move comment above the block. Do the check on towncrier only for PR targeting develop branch Format file io.element.android:opusencoder:1.0.1 Add a VectorFeatures to force usage of the library OpusEncoder Add dependency to opusencoder and remove module from this project shorter name Add GitHub action to check for a towncrier file

* develop: Comment the GHA towncrier, there is a syntax error

bmarty

Thanks for the update!

sonarqubecloud · 2022-07-12T12:11:54Z

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

0.0% Coverage
0.3% Duplication

Florian14 force-pushed the feature/fre/start_dm_on_first_msg branch 7 times, most recently from 0bef89f to f62b4d0 Compare May 18, 2022 09:32

Florian14 marked this pull request as ready for review May 18, 2022 09:54

Florian14 requested review from a team, ouchadam and fedrunov and removed request for a team May 18, 2022 09:54

Florian14 commented May 18, 2022

View reviewed changes

...-android/src/main/java/org/matrix/android/sdk/internal/session/room/create/CreateRoomTask.kt Outdated Show resolved Hide resolved

Florian14 requested a review from gaelledel May 18, 2022 10:07

ouchadam reviewed May 19, 2022

View reviewed changes

...oid/src/main/java/org/matrix/android/sdk/internal/session/room/create/CreateLocalRoomTask.kt Outdated Show resolved Hide resolved

ouchadam reviewed May 19, 2022

View reviewed changes

...oid/src/main/java/org/matrix/android/sdk/internal/session/room/create/CreateLocalRoomTask.kt Outdated Show resolved Hide resolved

Florian14 commented May 23, 2022

View reviewed changes

vector/src/main/res/layout/item_timeline_event_merged_room_creation_stub.xml Outdated Show resolved Hide resolved

Florian14 mentioned this pull request May 23, 2022

Create the DM when sending an event #6127

Merged

15 tasks

ouchadam reviewed May 23, 2022

View reviewed changes

...oid/src/main/java/org/matrix/android/sdk/internal/session/room/create/CreateLocalRoomTask.kt Outdated Show resolved Hide resolved

ouchadam reviewed May 23, 2022

View reviewed changes

ouchadam reviewed May 25, 2022

View reviewed changes

ouchadam approved these changes May 25, 2022

View reviewed changes

fedrunov approved these changes May 25, 2022

View reviewed changes

giomfo mentioned this pull request Jun 9, 2022

Create a screen to prepare DM creation (= timeline without room) tchapgouv/tchap-android#515

Closed

Florian Renaud added 4 commits June 18, 2022 00:09

Start DM - Add feature flag

c9ab092

Fix VectorFeature pref key

ba3d350

Start DM - Rename action button to "go"

26d1a12

Start DM - display a local room before creating the real one

10d683a

Add CreateLocalRoomTask interface and remove DI annotations

Florian Renaud added 7 commits June 30, 2022 11:48

Show date separator in local room timeline

71320e4

Split code in MergedRoomCreationItem

c7db896

Update room description style

7415623

Set current ts for local events age

0dad4cb

Remove unused imports

c25edfa

bmarty requested changes Jul 4, 2022

View reviewed changes

bmarty reviewed Jul 4, 2022

View reviewed changes

...oid/src/main/java/org/matrix/android/sdk/internal/session/room/create/CreateLocalRoomTask.kt Outdated Show resolved Hide resolved

bmarty reviewed Jul 4, 2022

View reviewed changes

...oid/src/main/java/org/matrix/android/sdk/internal/session/room/create/CreateLocalRoomTask.kt Outdated Show resolved Hide resolved

Florian Renaud added 6 commits July 4, 2022 17:38

Ensure that Realm is up to date before returning the roomId

8a5a47c

Fix logs when deleting local room

f4b50f1

Delete local room related entities with cascade

0d9cd2b

Set local room members as loaded

e89bb0e

Create local events using local echo

2b6bfc1

Fix copyright date

414dc52

bmarty mentioned this pull request Jul 4, 2022

Fix copy paste issue #6456

Merged

gaelledel approved these changes Jul 11, 2022

View reviewed changes

Florian Renaud added 4 commits July 11, 2022 15:24

Update the title of the local room timeline

fb87d31

Add margin after the action button of the user list toolbar

a10a8ce

Change method visibility

fdb9ed8

Revert "Ensure that Realm is up to date before returning the roomId"

fca4df3

This reverts commit 8a5a47c.

Florian14 requested a review from bmarty July 11, 2022 15:22

Florian Renaud added 2 commits July 12, 2022 09:14

Merge branch 'develop' into feature/fre/start_dm_on_first_msg

b8cdf9d

* develop: Comment the GHA towncrier, there is a syntax error

bmarty approved these changes Jul 12, 2022

View reviewed changes

Florian14 merged commit 909ce29 into develop Jul 12, 2022

Florian14 deleted the feature/fre/start_dm_on_first_msg branch July 12, 2022 13:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Start DM on first message (UI) #6051

Start DM on first message (UI) #6051

Florian14 commented May 13, 2022 •

edited

Loading

github-actions bot commented May 13, 2022 •

edited

Loading

ouchadam commented May 19, 2022

ouchadam May 23, 2022

Florian14 May 25, 2022

ouchadam May 25, 2022

Florian14 May 25, 2022

bmarty Jul 4, 2022

Florian14 Jul 5, 2022

ouchadam May 25, 2022 •

edited

Loading

ouchadam left a comment

fedrunov left a comment

bmarty left a comment

bmarty Jul 4, 2022

Florian14 Jul 4, 2022

bmarty Jul 4, 2022

gaelledel left a comment

bmarty left a comment

sonarqubecloud bot commented Jul 12, 2022

Start DM on first message (UI) #6051

Start DM on first message (UI) #6051

Conversation

Florian14 commented May 13, 2022 • edited Loading

Type of change

Content

Motivation and context

Screenshots / GIFs

Tests

Tested devices

Checklist

github-actions bot commented May 13, 2022 • edited Loading

Unit Test Results

ouchadam commented May 19, 2022

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

ouchadam May 25, 2022 • edited Loading

Choose a reason for hiding this comment

ouchadam left a comment

Choose a reason for hiding this comment

fedrunov left a comment

Choose a reason for hiding this comment

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

gaelledel left a comment

Choose a reason for hiding this comment

bmarty left a comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Jul 12, 2022

Florian14 commented May 13, 2022 •

edited

Loading

github-actions bot commented May 13, 2022 •

edited

Loading

ouchadam May 25, 2022 •

edited

Loading