fix: put proposals in the room that signs, and open the queue when one row is not all of it
The Proposals button sat behind `mlsGroupState == null`, so it was offered in
NIP-17 rooms and withheld from Marmot ones. That is the wrong way round rather
than a gap: FrostSigningManager signs "in its #admins room", and an #admins
room is a Marmot group.
**Where a group actually proposes.** `FrostSigningManager.broadcast` writes a
signing message as an unprocessed `MarmotInnerEvent`, which `NotaryViewModel`
MLS-encrypts and broadcasts as a kind:445 -- "the same path every other event
in a Marmot group takes, which is why this needs no transport of its own". A
NIP-17 room holds no MLS state for that path to use, and since 9107b81
`encryptAndSendMarmotInnerEvent` throws `MarmotMissingChatGroupException` on
exactly that rather than silently doing nothing. So the button was shown in the
one kind of room where a proposal cannot leave the device, and hidden in the
kind where every proposal this app has made actually lives.
The Shared Key button beside it keeps the gate, and the comment above it keeps
its wording, because for a ceremony the gate is right: ChillDKG runs over NIP-17
because it has to -- its participants are not yet a Marmot group, and its whole
purpose is to produce the key one would be keyed on -- so a room that already
has an MLS tree is not a room a ceremony can run in. The two buttons stand next
to each other and answer different questions; only one of them is about the
transport the group already has.
**Not gated on whether the room can sign.** A plain DM now shows Proposals too,
and opening it says "This group has not been asked to sign anything yet."
Gating on `FrostSigningRepository.canSign` would mean threading that repository
into `ChatRoomDetailScreen` for one button's visibility, and the Shared Key
button it sits beside is not gated either -- a room with no ceremony behind it
still offers to hold one. An empty list is a truthful answer to a tap; an
absent button is no answer at all to a member wondering where the proposals
went, which is the complaint this commit starts from.
**One row is not the queue.** Tapping a signing line in the transcript went
straight to that session, and the list was reached only when the line predated
chat rows naming their session. That is right while the reader has one decision
outstanding and wrong the moment they have two: the second proposal is not in
the transcript beside the first -- it may be pages up, or have arrived while
they were reading -- so answering the one they tapped and leaving looks exactly
like being done. A group proposing a chapter and the translation that depends
on it is two sessions at once, which is the case `ProposalListScreen` exists
for; the transcript was still handing over one of them and calling it the
answer.
**Only for a row that is itself waiting.** `hidesOtherDecisions` asks two things
rather than one: that the tapped session is waiting on this member, and that it
is not the only one. A line about something the group has already signed still
opens that session directly. A reader who taps history is asking to see what was
signed, and meeting that with the queue would be substituting a general answer
for a specific request -- the same fault as the one being fixed, pointing the
other way.
**Where the answer comes from.** `ChatMessageListViewModel` observes
`observeSessionsForChatRoom` and keeps the ids of the sessions
`FrostSigningManager.isAwaitingApproval` calls pending. That is the same
question `ProposalListUIState.waitingForYou` asks, deliberately, so the
transcript and the list cannot come to different views about which proposals
still have a decision in them. Observed rather than read once, because a
proposal arrives while a room is open as often as before it is opened, and one
answered on another device stops being owed with nothing happening here at all.
Held as state rather than queried at the tap: a navigation callback is not
suspend, and there is nowhere inside one to put a query. The read happens in
the click lambda rather than during composition, so a proposal arriving moves
where the next tap goes without recomposing the transcript to do it.
That took the repository through `MantraNavHost` -> `ChatRoomMessagingScreen`
-> `ChatMessageListViewModel.factory`; the screen's preview takes
`NO_OP_FROST_SIGNING_REPOSITORY`, which already answers with an empty list.
**Not covered, deliberately.** `ChatRoomMessagingScreen` calls
`chatMessageListViewModel.initiate()` inside `key(true) { }` rather than a
`LaunchedEffect`, so it re-runs on recomposition and launches a fresh collector
each time. The three observers already there carry that exposure;
`observeProposalsAwaitingYou` joins them rather than diverging from them, and
since each collector writes the same value from the same Room flow the cost is
duplicate collection, not a wrong answer. Fixing it changes how this screen
starts all of its work, which is every observer's business rather than this
one's.
Nothing counts the outstanding decisions anywhere a reader can see them before
tapping. A signing line still says only whether the line it sits on has been
answered, and the room list says nothing. A badge wants this count somewhere it
outlives one open room, which is its own change.
No tests. Both changes are navigation decisions taken in composables --
`ChatRoomDetailScreen`'s visibility gate and `ChatRoomMessagingScreen`'s route
choice -- and there is no UI test harness here to press a button in; the one
piece with a seam, `isAwaitingApproval`, is already what the proposal list is
tested through.
Verified: :composeApp:compileDebugKotlinAndroid succeeds; 808 tests pass, 511
jvm and 297 android, unchanged from before the change. That a Marmot room now
offers the list and that a second waiting proposal redirects the tap are read
from the code, not asserted -- both want the app on a device with a group that
has a key.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -367,33 +367,38 @@ fun ChatRoomDetailScreen(
|
||||
Text("Shared Key")
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Next to the key, because that is what they sign with.
|
||||
// The transcript carries a proposal past as it happens;
|
||||
// this is where a member goes to find one that has
|
||||
// scrolled away, or to see what the group has signed.
|
||||
item {
|
||||
TextButton(
|
||||
onClick = {
|
||||
onNavigateToRoute.invoke(
|
||||
ProposalListRoute(
|
||||
activeUserPublicKey = activeUserPublicKey,
|
||||
chatRoomId = chatRoomId
|
||||
)
|
||||
// Every room, not only the one that holds the key. A
|
||||
// ceremony needs a NIP-17 group, but a signing message is
|
||||
// a marmot inner event -- see `FrostSigningManager.broadcast`
|
||||
// -- so the room a group actually proposes in is the MLS
|
||||
// one, which is the last place this should be missing from.
|
||||
//
|
||||
// The transcript carries a proposal past as it happens;
|
||||
// this is where a member goes to find one that has scrolled
|
||||
// away, or to see what the group has signed.
|
||||
item {
|
||||
TextButton(
|
||||
onClick = {
|
||||
onNavigateToRoute.invoke(
|
||||
ProposalListRoute(
|
||||
activeUserPublicKey = activeUserPublicKey,
|
||||
chatRoomId = chatRoomId
|
||||
)
|
||||
}
|
||||
) {
|
||||
Icon(
|
||||
Icons.Default.Draw,
|
||||
contentDescription = "Proposals"
|
||||
)
|
||||
|
||||
Spacer(
|
||||
modifier = Modifier.width(10.dp)
|
||||
)
|
||||
|
||||
Text("Proposals")
|
||||
}
|
||||
) {
|
||||
Icon(
|
||||
Icons.Default.Draw,
|
||||
contentDescription = "Proposals"
|
||||
)
|
||||
|
||||
Spacer(
|
||||
modifier = Modifier.width(10.dp)
|
||||
)
|
||||
|
||||
Text("Proposals")
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -44,6 +44,7 @@ import androidx.lifecycle.viewmodel.compose.viewModel
|
||||
import press.mantra.compose.database.model.ChatRoom
|
||||
import press.mantra.compose.database.model.intermdiate.LocalChatRoom
|
||||
import press.mantra.compose.repository.ChatRepository
|
||||
import press.mantra.compose.repository.FrostSigningRepository
|
||||
import press.mantra.compose.repository.NostrRepository
|
||||
import press.mantra.compose.ui.composable.navigation.routes.ChatRoomDetailRoute
|
||||
import press.mantra.compose.ui.composable.navigation.routes.DkgRitualRoute
|
||||
@@ -65,6 +66,7 @@ fun ChatRoomMessagingScreen(
|
||||
initialChatRoomMessagingUIState: ChatRoomMessagingUIState = ChatRoomMessagingUIState.Loading,
|
||||
nostrRepository: NostrRepository,
|
||||
chatRepository: ChatRepository,
|
||||
frostSigningRepository: FrostSigningRepository,
|
||||
onNavigateToRouteAndPopUpInclusive: (Route) -> Unit,
|
||||
onNavigateToRoute: (Route) -> Unit,
|
||||
) {
|
||||
@@ -100,7 +102,8 @@ fun ChatRoomMessagingScreen(
|
||||
factory = press.mantra.compose.ui.view.model.ChatMessageListViewModel.factory(
|
||||
localChatRoom = chatRoomDetailUIState.localChatRoom,
|
||||
nostrRepository = nostrRepository,
|
||||
chatRepository = chatRepository
|
||||
chatRepository = chatRepository,
|
||||
frostSigningRepository = frostSigningRepository
|
||||
)
|
||||
)
|
||||
|
||||
@@ -149,13 +152,21 @@ fun ChatRoomMessagingScreen(
|
||||
)
|
||||
},
|
||||
onOpenSigning = { sessionId ->
|
||||
// A line written before chat rows named their
|
||||
// session cannot say which proposal it is about,
|
||||
// and the room may have several. The list is the
|
||||
// honest answer: it shows all of them with their
|
||||
// own state, rather than guessing at one.
|
||||
// Two reasons to open the queue instead of the one
|
||||
// proposal. A line written before chat rows named
|
||||
// their session cannot say which proposal it is
|
||||
// about, and the room may have several -- the list
|
||||
// is the honest answer there, showing all of them
|
||||
// with their own state rather than guessing at one.
|
||||
//
|
||||
// The other is that the proposal they tapped is
|
||||
// not the only one waiting on them, in which case
|
||||
// it is not the whole of what is being asked and a
|
||||
// screen showing only it would say it was.
|
||||
onNavigateToRoute.invoke(
|
||||
if (sessionId == null) {
|
||||
if (sessionId == null ||
|
||||
chatMessageListViewModel.hidesOtherDecisions(sessionId)
|
||||
) {
|
||||
ProposalListRoute(
|
||||
activeUserPublicKey = activeUserPublicKey,
|
||||
chatRoomId = chatRoomId
|
||||
@@ -440,6 +451,7 @@ private fun ChatRoomMessagingScreenPreview() {
|
||||
),
|
||||
nostrRepository = NostrRepository.NO_OP_NOSTR_REPOSITORY,
|
||||
chatRepository = ChatRepository.NO_OP_CHAT_REPOSITORY,
|
||||
frostSigningRepository = FrostSigningRepository.NO_OP_FROST_SIGNING_REPOSITORY,
|
||||
onNavigateToRouteAndPopUpInclusive = {},
|
||||
onNavigateToRoute = {}
|
||||
)
|
||||
|
||||
@@ -746,6 +746,7 @@ fun MantraNavHost(
|
||||
relayHint = route.relayHint,
|
||||
nostrRepository = databaseNostrRepository,
|
||||
chatRepository = databaseChatRepository,
|
||||
frostSigningRepository = databaseFrostSigningRepository,
|
||||
onNavigateToRouteAndPopUpInclusive = { chatRoomDetailRoute ->
|
||||
navController.navigate(
|
||||
route = chatRoomDetailRoute
|
||||
|
||||
@@ -73,9 +73,11 @@ import press.mantra.compose.database.model.intermdiate.LocalChatRoom
|
||||
import press.mantra.compose.database.model.types.SynchronizationFilter
|
||||
import press.mantra.compose.extensions.shortened
|
||||
import press.mantra.compose.extensions.toFormattedTimeAndDateString
|
||||
import press.mantra.compose.managers.FrostSigningManager
|
||||
import press.mantra.compose.nostr.Nip17Filters
|
||||
import press.mantra.compose.nostr.Relays
|
||||
import press.mantra.compose.repository.ChatRepository
|
||||
import press.mantra.compose.repository.FrostSigningRepository
|
||||
import press.mantra.compose.repository.NostrRepository
|
||||
import press.mantra.compose.ui.composable.widgets.profile.ProfileColor
|
||||
import press.mantra.compose.ui.view.state.ChatMessageListUIState
|
||||
@@ -91,7 +93,8 @@ class ChatMessageListViewModel(
|
||||
initialChatMessageListUIState: ChatMessageListUIState,
|
||||
val localChatRoom: LocalChatRoom,
|
||||
val nostrRepository: NostrRepository,
|
||||
val chatRepository: ChatRepository
|
||||
val chatRepository: ChatRepository,
|
||||
val frostSigningRepository: FrostSigningRepository
|
||||
): ViewModel() {
|
||||
val logger = Logger.withTag(TAG)
|
||||
var chatMessageListUIState: ChatMessageListUIState by mutableStateOf(initialChatMessageListUIState)
|
||||
@@ -114,6 +117,33 @@ class ChatMessageListViewModel(
|
||||
var openMessageActionsFor: Long? by mutableStateOf(null)
|
||||
private set
|
||||
|
||||
/**
|
||||
* The sessions still waiting on this member to agree to sign, by id.
|
||||
*
|
||||
* Held rather than asked for, because it is read at the moment of a tap --
|
||||
* see [hidesOtherDecisions] -- and that is not a moment that can wait on a
|
||||
* query.
|
||||
*/
|
||||
var proposalsAwaitingYou: Set<String> by mutableStateOf(emptySet())
|
||||
private set
|
||||
|
||||
/**
|
||||
* Whether opening [sessionId] by itself would hide other decisions the room
|
||||
* is waiting on this member for.
|
||||
*
|
||||
* A signing line names one proposal, and opening it is right while that is
|
||||
* the only thing being asked of them. It stops being right the moment there
|
||||
* are two: the second is not in the transcript beside the first -- it may be
|
||||
* pages up, or have arrived while they were reading -- so answering the one
|
||||
* they tapped and leaving would look exactly like being done.
|
||||
*
|
||||
* Only for a proposal that is itself waiting on them. A line about something
|
||||
* the group has already signed is history, and a reader who taps one is
|
||||
* asking to see that, not to be handed the queue.
|
||||
*/
|
||||
fun hidesOtherDecisions(sessionId: String): Boolean =
|
||||
sessionId in proposalsAwaitingYou && proposalsAwaitingYou.size > 1
|
||||
|
||||
fun startDirectMessage(participant: Participant) {
|
||||
logger.d("Arming a direct message to ${participant.participantPublicKey}")
|
||||
directMessageRecipient = participant
|
||||
@@ -157,9 +187,31 @@ class ChatMessageListViewModel(
|
||||
logger.d("init")
|
||||
scheduleSynchronization()
|
||||
observeChatRoomFeed()
|
||||
observeProposalsAwaitingYou()
|
||||
askForGroupHistory()
|
||||
}
|
||||
|
||||
/**
|
||||
* Watches which of the room's proposals are waiting on this member.
|
||||
*
|
||||
* Observed rather than read once: a proposal arrives while the room is
|
||||
* open as often as before it is opened, and one answered on another device
|
||||
* stops being owed without anything happening here. Asked of the manager,
|
||||
* the way the proposal list asks, so the two cannot disagree about which
|
||||
* proposals still have a decision in them.
|
||||
*/
|
||||
private fun observeProposalsAwaitingYou() {
|
||||
viewModelScope.launch(Dispatchers.IO) {
|
||||
frostSigningRepository
|
||||
.observeSessionsForChatRoom(localChatRoom.chatRoom.id)
|
||||
.collect { sessions ->
|
||||
proposalsAwaitingYou = sessions.filter {
|
||||
FrostSigningManager.isAwaitingApproval(it.session, it.items)
|
||||
}.map { it.session.id }.toSet()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Ask the group for its signed record, if this device holds none of it.
|
||||
*
|
||||
@@ -643,14 +695,16 @@ class ChatMessageListViewModel(
|
||||
initialChatMessageListUIState: ChatMessageListUIState = ChatMessageListUIState.Loading,
|
||||
localChatRoom: LocalChatRoom,
|
||||
nostrRepository: NostrRepository,
|
||||
chatRepository: ChatRepository
|
||||
chatRepository: ChatRepository,
|
||||
frostSigningRepository: FrostSigningRepository
|
||||
): ViewModelProvider.Factory = viewModelFactory {
|
||||
initializer {
|
||||
ChatMessageListViewModel(
|
||||
initialChatMessageListUIState = initialChatMessageListUIState,
|
||||
localChatRoom = localChatRoom,
|
||||
nostrRepository = nostrRepository,
|
||||
chatRepository = chatRepository
|
||||
chatRepository = chatRepository,
|
||||
frostSigningRepository = frostSigningRepository
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user