Code review and testing fixes:

- fix(dm-share): kotlin-review fixes (alias dot-boundary match + transient feed doc)
- fix(dm-share): address code-review findings (intent consume, media helper, manifest sync)
- fix(dm-share): make the picker one-shot so backing out doesn't duplicate drafts
- fix(dm): avoid duplicate drafts on abort by rotating draft tag after the async save
This commit is contained in:
davotoula
2026-06-01 21:29:41 +02:00
parent 8d715e5730
commit e6a512db42
9 changed files with 91 additions and 14 deletions
+3
View File
@@ -218,6 +218,9 @@
</intent-filter>
</activity>
<!-- "Send as DM" share target. The android:name simple class ("ShareAsDMAlias")
is matched at runtime by ShareIntentRouting.SHARE_AS_DM_ALIAS_SIMPLE_NAME;
keep the two in sync when renaming. -->
<activity-alias
android:name=".ui.ShareAsDMAlias"
android:exported="true"
@@ -0,0 +1,42 @@
/*
* Copyright (c) 2025 Vitor Pamplona
*
* Permission is hereby granted, free of charge, to any person obtaining a copy of
* this software and associated documentation files (the "Software"), to deal in
* the Software without restriction, including without limitation the rights to use,
* copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the
* Software, and to permit persons to whom the Software is furnished to do so,
* subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in all
* copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
* FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
* COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN
* AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
* WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
*/
package com.vitorpamplona.amethyst.ui.actions.uploads
import android.content.Context
import androidx.core.net.toUri
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.withContext
/**
* Resolves a shared content-URI string (from an Android SEND intent) into a
* [SelectedMedia] by reading its MIME type off the main thread. Returns null for
* a null/blank URI. Shared by the share-to-compose pre-fill paths (DM chatroom,
* group DM, …) so the URI→MIME→SelectedMedia logic lives in one place.
*/
suspend fun resolveSharedMedia(
context: Context,
uriString: String?,
): SelectedMedia? =
uriString?.ifBlank { null }?.toUri()?.let { uri ->
withContext(Dispatchers.IO) {
SelectedMedia(uri, context.contentResolver.getType(uri))
}
}
@@ -625,6 +625,14 @@ private fun NavigateIfIntentRequested(
} else {
nav.newStack(Route.NewShortNote(message = message, attachment = media.toString()))
}
// Consume the launch intent so a later recomposition can't re-fire
// newStack for the same share (the isBaseRoute guard is a non-reactive
// snapshot and stops guarding once we navigate past the destination,
// e.g. into a chat via the one-shot picker). Clearing the action also
// lets the else-branch register the onNewIntent listener for the rest
// of this session.
activity.intent.action = null
} else {
var newAccount by remember { mutableStateOf<String?>(null) }
@@ -26,8 +26,13 @@ package com.vitorpamplona.amethyst.ui.navigation
* component class name of the launching intent (the activity-alias name).
*/
object ShareIntentRouting {
/** Simple class name of the activity-alias declared in AndroidManifest.xml. */
/**
* Simple class name of the `<activity-alias>` declared in AndroidManifest.xml
* (android:name=".ui.ShareAsDMAlias"). MUST stay in sync with the manifest —
* renaming the alias there without updating this constant silently routes
* "Send as DM" shares to the New Post composer (no build error).
*/
const val SHARE_AS_DM_ALIAS_SIMPLE_NAME = "ShareAsDMAlias"
fun isShareAsDm(componentClassName: String?): Boolean = componentClassName?.endsWith(SHARE_AS_DM_ALIAS_SIMPLE_NAME) == true
fun isShareAsDm(componentClassName: String?): Boolean = componentClassName?.endsWith(".$SHARE_AS_DM_ALIAS_SIMPLE_NAME") == true
}
@@ -31,10 +31,9 @@ import androidx.compose.runtime.rememberCoroutineScope
import androidx.compose.ui.Modifier
import androidx.compose.ui.platform.LocalContext
import androidx.compose.ui.unit.dp
import androidx.core.net.toUri
import androidx.lifecycle.viewmodel.compose.viewModel
import com.vitorpamplona.amethyst.service.relayClient.reqCommand.event.EventFinderFilterAssemblerSubscription
import com.vitorpamplona.amethyst.ui.actions.uploads.SelectedMedia
import com.vitorpamplona.amethyst.ui.actions.uploads.resolveSharedMedia
import com.vitorpamplona.amethyst.ui.feeds.WatchLifecycleAndUpdateModel
import com.vitorpamplona.amethyst.ui.navigation.navs.INav
import com.vitorpamplona.amethyst.ui.note.LoadAddressableNote
@@ -50,9 +49,7 @@ import com.vitorpamplona.quartz.nip01Core.core.HexKey
import com.vitorpamplona.quartz.nip17Dm.base.ChatroomKey
import com.vitorpamplona.quartz.nip17Dm.settings.ChatMessageRelayListEvent
import kotlinx.collections.immutable.persistentListOf
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.launch
import kotlinx.coroutines.withContext
@Composable
fun ChatroomView(
@@ -122,11 +119,8 @@ fun ChatroomView(
val context = LocalContext.current
if (attachmentUri != null) {
LaunchedEffect(key1 = attachmentUri) {
attachmentUri.ifBlank { null }?.toUri()?.let { uri ->
withContext(Dispatchers.IO) {
val mediaType = context.contentResolver.getType(uri)
newPostModel.pickedMedia(persistentListOf(SelectedMedia(uri, mediaType)))
}
resolveSharedMedia(context, attachmentUri)?.let {
newPostModel.pickedMedia(persistentListOf(it))
}
}
}
@@ -114,9 +114,15 @@ fun PrivateMessageEditFieldRow(
if (channelScreenModel.message.text.isNotBlank()) {
accountViewModel.launchSigner {
channelScreenModel.sendDraftSync()
// Rotate the draft tag only AFTER the async save completes. Doing it
// synchronously here (before launchSigner runs) would make sendDraftSync
// persist under a freshly-rotated tag, duplicating the draft. See the
// matching order in NewGroupDMScreen.
channelScreenModel.cancel()
}
} else {
channelScreenModel.cancel()
}
channelScreenModel.cancel()
nav.popBack()
}
@@ -36,6 +36,14 @@ class ShareToDMNav(
private val attachment: String?,
) : INav by delegate {
override fun nav(route: Route) {
delegate.nav(ShareToDMRouteRewriter.rewrite(route, message, attachment))
val rewritten = ShareToDMRouteRewriter.rewrite(route, message, attachment)
if (route is Route.Room) {
// One-shot: replace the picker in the back stack so backing out of the
// chat exits the share flow instead of returning to the picker, which
// would re-inject the shared text on re-tap and create duplicate drafts.
delegate.popUpTo(rewritten, Route.ShareToDM::class)
} else {
delegate.nav(rewritten)
}
}
}
@@ -55,6 +55,12 @@ fun ShareToDMScreen(
accountViewModel: AccountViewModel,
nav: INav,
) {
// Deliberately a screen-scoped, transient FeedContentState (not wired into
// AccountFeedContentStates like dmKnown/dmNew). The share picker is a one-shot,
// short-lived screen, so it owns its feed via viewModelScope and relies on
// WatchLifecycleAndUpdateModel to load/refresh on entry and resume rather than
// on the always-on additive update loop. Account switch recreates it (the
// remember key), which is correct for a transient picker.
val feedContentState =
remember(accountViewModel) {
FeedContentState(
@@ -85,7 +91,7 @@ fun ShareToDMScreen(
.clickable(
role = Role.Button,
onClickLabel = stringRes(R.string.share_to_dm_start_new),
) { nav.nav(Route.NewGroupDM(message = message, attachment = attachment)) }
) { nav.popUpTo(Route.NewGroupDM(message = message, attachment = attachment), Route.ShareToDM::class) }
.padding(16.dp),
)
@@ -46,4 +46,9 @@ class ShareIntentRoutingTest {
fun rejectsNull() {
assertFalse(ShareIntentRouting.isShareAsDm(null))
}
@Test
fun rejectsSuffixThatIsNotASimpleName() {
assertFalse(ShareIntentRouting.isShareAsDm("com.evil.XShareAsDMAlias"))
}
}