-
Notifications
You must be signed in to change notification settings - Fork 238
Adapt to changes in SDK #2836
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adapt to changes in SDK #2836
Changes from 6 commits
173f329
d11473a
678cbdf
5bcac5c
59d9069
68de2b9
4332159
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
/* | ||
* Copyright (c) 2024 New Vector Ltd | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
package io.element.android.features.migration.impl.migrations | ||
|
||
import com.squareup.anvil.annotations.ContributesMultibinding | ||
import io.element.android.features.rageshake.api.logs.LogFilesRemover | ||
import io.element.android.libraries.di.AppScope | ||
import javax.inject.Inject | ||
|
||
/** | ||
* This performs the same operation as [AppMigration01], since we need to clear the local logs again. | ||
*/ | ||
@ContributesMultibinding(AppScope::class) | ||
class AppMigration03 @Inject constructor( | ||
private val logFilesRemover: LogFilesRemover, | ||
) : AppMigration01(logFilesRemover) { | ||
override val order: Int = 3 | ||
} | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
/* | ||
* Copyright (c) 2024 New Vector Ltd | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
package io.element.android.features.migration.impl.migrations | ||
|
||
import io.element.android.features.rageshake.test.logs.FakeLogFilesRemover | ||
import kotlinx.coroutines.test.runTest | ||
import org.junit.Test | ||
|
||
class AppMigration03Test { | ||
@Test | ||
fun `test migration`() = runTest { | ||
val logsFileRemover = FakeLogFilesRemover() | ||
val migration = AppMigration03(logsFileRemover) | ||
|
||
migration.migrate() | ||
|
||
logsFileRemover.performLambda.assertions().isCalledOnce() | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -65,7 +65,7 @@ class RoomDetailsEditPresenter @Inject constructor( | |
// just erase the local value when the room field has changed | ||
var roomAvatarUri by rememberSaveable(room.avatarUrl) { mutableStateOf(room.avatarUrl?.toUri()) } | ||
|
||
var roomName by rememberSaveable { mutableStateOf((room.name ?: room.displayName).trim()) } | ||
var roomName by rememberSaveable { mutableStateOf(room.displayName.trim()) } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need to use the new field It will fix the form per-filled with a computed room name like "Alice, Bob and 3 others". There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using |
||
var roomTopic by rememberSaveable { mutableStateOf(room.topic?.trim()) } | ||
|
||
val saveButtonEnabled by remember( | ||
|
@@ -76,7 +76,7 @@ class RoomDetailsEditPresenter @Inject constructor( | |
) { | ||
derivedStateOf { | ||
roomAvatarUri?.toString()?.trim() != room.avatarUrl?.toUri()?.toString()?.trim() || | ||
roomName.trim() != (room.name ?: room.displayName).trim() || | ||
roomName.trim() != room.displayName.trim() || | ||
roomTopic.orEmpty().trim() != room.topic.orEmpty().trim() | ||
} | ||
} | ||
|
@@ -168,7 +168,7 @@ class RoomDetailsEditPresenter @Inject constructor( | |
Timber.e(it, "Failed to set room topic") | ||
}) | ||
} | ||
if (name.isNotEmpty() && name.trim() != room.name.orEmpty().trim()) { | ||
if (name.isNotEmpty() && name.trim() != room.displayName.trim()) { | ||
results.add(room.setName(name).onFailure { | ||
Timber.e(it, "Failed to set room name") | ||
}) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -39,14 +39,12 @@ interface RoomList { | |
|
||
/** | ||
* The source of the room list data. | ||
* All: all rooms except invites. | ||
* Invites: only invites. | ||
* All: all rooms. | ||
* | ||
* To apply some dynamic filtering on top of that, use [DynamicRoomList]. | ||
*/ | ||
enum class Source { | ||
All, | ||
Invites, | ||
All | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is becoming a bit useless if there is only one value, but I think we could keep this enum for clarity? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I wanted to keep it in case we'd need separate lists in the future again for some reason. |
||
} | ||
|
||
/** | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -96,6 +96,7 @@ import kotlinx.coroutines.flow.flow | |
import kotlinx.coroutines.flow.map | ||
import kotlinx.coroutines.flow.stateIn | ||
import kotlinx.coroutines.launch | ||
import kotlinx.coroutines.runBlocking | ||
import kotlinx.coroutines.withContext | ||
import kotlinx.coroutines.withTimeout | ||
import org.matrix.rustcomponents.sdk.BackupState | ||
|
@@ -145,12 +146,7 @@ class RustMatrixClient( | |
dispatchers = dispatchers, | ||
) | ||
private val notificationProcessSetup = NotificationProcessSetup.SingleProcess(syncService) | ||
private val notificationClient = client.notificationClient(notificationProcessSetup) | ||
.use { builder -> | ||
builder | ||
.filterByPushRules() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This API has vanished, I guess this is OK |
||
.finish() | ||
} | ||
private val notificationClient = runBlocking { client.notificationClient(notificationProcessSetup) } | ||
private val notificationService = RustNotificationService(sessionId, notificationClient, dispatchers, clock) | ||
private val notificationSettingsService = RustNotificationSettingsService(client, dispatchers) | ||
.apply { start() } | ||
|
@@ -465,7 +461,7 @@ class RustMatrixClient( | |
|
||
override suspend fun resolveRoomAlias(roomAlias: RoomAlias): Result<RoomId> = withContext(sessionDispatcher) { | ||
runCatching { | ||
client.resolveRoomAlias(roomAlias.value).let(::RoomId) | ||
client.resolveRoomAlias(roomAlias.value).roomId.let(::RoomId) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I will improve this and use the returned |
||
} | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's OK, but maybe it will be clearer using composition.
What do you think about:
Consequently,
AppMigration01
does not need to beopen