Skip to content

Commit cde0194

Browse files
AndyScherzingerAndyScherzinger
committed
test(moderators): Cover the owner promotion preconditions
The rules deciding whether ownership can be handed over were private helpers in the Compose sheet, so nothing tested them. They are the kind of thing that rots quietly: a conversation type added to one of the sets, or a precondition the server tightens, changes who sees a destructive action with nothing to catch it. Move them to ParticipantRoleUtils, next to roleOf(), and test them there. canChangeOwnership() is the gate; canBePromotedToOwner() and canBeDemotedFromOwner() add the target's own rank. The sheet keeps only the question of which row to draw. The tests walk each dimension of the matrix rather than sampling it, so a new conversation or object type has to be classified deliberately instead of inheriting whatever the enum ordering gives it: every rankless conversation type, every object type with an implicit owner, every actor type that can never hold the rank, and every participant rank on both sides of promotable. Assisted-by: Claude Code:claude-opus-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
1 parent 17671f4 commit cde0194

3 files changed

Lines changed: 298 additions & 44 deletions

File tree

‎app/src/main/java/com/nextcloud/talk/conversationinfo/ui/ParticipantOperationsSheet.kt‎

Lines changed: 9 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -43,12 +43,10 @@ import com.nextcloud.talk.R
4343
import com.nextcloud.talk.conversationinfo.model.ParticipantModel
4444
import com.nextcloud.talk.models.domain.ConversationModel
4545
import com.nextcloud.talk.models.json.capabilities.SpreedCapability
46-
import com.nextcloud.talk.models.json.conversations.ConversationEnums
4746
import com.nextcloud.talk.models.json.participants.Participant
4847
import com.nextcloud.talk.utils.CapabilitiesUtil
4948
import com.nextcloud.talk.utils.ParticipantRole
5049
import com.nextcloud.talk.utils.ParticipantRoleUtils
51-
import com.nextcloud.talk.utils.SpreedFeatures
5250

5351
private data class RemoveOption(@DrawableRes val iconRes: Int, val label: String)
5452

@@ -63,37 +61,6 @@ private data class ParticipantOpsVisibility(
6361
val showBan: Boolean
6462
)
6563

66-
/**
67-
* Conversation types and object types in which the owner rank can be handed out, mirroring the
68-
* server. Everything else binds the conversation to an object that assumes a single owner.
69-
*/
70-
private val ownerChangeConversationTypes = setOf(
71-
ConversationEnums.ConversationType.ROOM_GROUP_CALL,
72-
ConversationEnums.ConversationType.ROOM_PUBLIC_CALL
73-
)
74-
75-
private val ownerChangeObjectTypes = setOf(
76-
ConversationEnums.ObjectType.DEFAULT,
77-
ConversationEnums.ObjectType.CLASSIFIED,
78-
ConversationEnums.ObjectType.INSTANT_MEETING
79-
)
80-
81-
/**
82-
* Only an owner can change ownership, only a user can hold it, and only in conversations that are
83-
* not bound to an object with an implicit owner. The server enforces all of this again.
84-
*/
85-
private fun canChangeOwnership(
86-
participant: Participant,
87-
conversation: ConversationModel?,
88-
spreedCapabilities: SpreedCapability?
89-
): Boolean =
90-
CapabilitiesUtil.hasSpreedFeatureCapability(spreedCapabilities, SpreedFeatures.PROMOTE_DEMOTE_OWNER) &&
91-
conversation != null &&
92-
conversation.participantType == Participant.ParticipantType.OWNER &&
93-
conversation.type in ownerChangeConversationTypes &&
94-
conversation.objectType in ownerChangeObjectTypes &&
95-
participant.calculatedActorType == Participant.ActorType.USERS
96-
9764
@Composable
9865
@Suppress("LongMethod")
9966
private fun computeVisibility(
@@ -104,7 +71,7 @@ private fun computeVisibility(
10471
val participant = model.participant
10572
val pin = participant.attendeePin?.takeIf { it.isNotEmpty() }
10673
val deleteIcon = R.drawable.ic_delete_grey600_24dp
107-
val ownership = canChangeOwnership(participant, conversation, spreedCapabilities)
74+
val canDemoteFromOwner = ParticipantRoleUtils.canBeDemotedFromOwner(participant, conversation, spreedCapabilities)
10875
val isOwner = participant.type == Participant.ParticipantType.OWNER
10976

11077
return when {
@@ -114,7 +81,7 @@ private fun computeVisibility(
11481
showDemote = false,
11582
showPromoteToOwner = false,
11683
// An owner may step down, but only as far as moderator, to avoid locking themselves out
117-
showDemoteOwnerToModerator = ownership && isOwner,
84+
showDemoteOwnerToModerator = canDemoteFromOwner,
11885
showDemoteOwnerToUser = false,
11986
remove = pin?.let {
12087
RemoveOption(R.drawable.ic_lock_grey600_24px, stringResource(R.string.nc_attendee_pin, it))
@@ -149,8 +116,8 @@ private fun computeVisibility(
149116
showPromote = false,
150117
showDemote = false,
151118
showPromoteToOwner = false,
152-
showDemoteOwnerToModerator = ownership,
153-
showDemoteOwnerToUser = ownership,
119+
showDemoteOwnerToModerator = canDemoteFromOwner,
120+
showDemoteOwnerToUser = canDemoteFromOwner,
154121
remove = null,
155122
showBan = false
156123
)
@@ -161,7 +128,11 @@ private fun computeVisibility(
161128
participant.type == Participant.ParticipantType.GUEST,
162129
showDemote = participant.type == Participant.ParticipantType.MODERATOR ||
163130
participant.type == Participant.ParticipantType.GUEST_MODERATOR,
164-
showPromoteToOwner = ownership && participant.type in promotableToOwner,
131+
showPromoteToOwner = ParticipantRoleUtils.canBePromotedToOwner(
132+
participant,
133+
conversation,
134+
spreedCapabilities
135+
),
165136
showDemoteOwnerToModerator = false,
166137
showDemoteOwnerToUser = false,
167138
remove = RemoveOption(deleteIcon, stringResource(R.string.nc_remove_participant)),
@@ -170,12 +141,6 @@ private fun computeVisibility(
170141
}
171142
}
172143

173-
private val promotableToOwner = setOf(
174-
Participant.ParticipantType.USER,
175-
Participant.ParticipantType.USER_FOLLOWING_LINK,
176-
Participant.ParticipantType.MODERATOR
177-
)
178-
179144
@Composable
180145
fun ParticipantOperationsContent(
181146
model: ParticipantModel,

‎app/src/main/java/com/nextcloud/talk/utils/ParticipantRoleUtils.kt‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ package com.nextcloud.talk.utils
99
import androidx.annotation.DrawableRes
1010
import androidx.annotation.StringRes
1111
import com.nextcloud.talk.R
12+
import com.nextcloud.talk.models.domain.ConversationModel
13+
import com.nextcloud.talk.models.json.capabilities.SpreedCapability
1214
import com.nextcloud.talk.models.json.conversations.ConversationEnums
1315
import com.nextcloud.talk.models.json.participants.Participant
1416

@@ -66,4 +68,69 @@ object ParticipantRoleUtils {
6668
ParticipantRole.MODERATOR -> R.drawable.outline_shield_24
6769
ParticipantRole.NONE -> null
6870
}
71+
72+
/**
73+
* Conversation types in which the owner rank can be handed out. One-to-one conversations already
74+
* have two owners by design, and note-to-self and changelog conversations only ever have one
75+
* participant.
76+
*/
77+
private val OWNER_CHANGE_CONVERSATION_TYPES = setOf(
78+
ConversationEnums.ConversationType.ROOM_GROUP_CALL,
79+
ConversationEnums.ConversationType.ROOM_PUBLIC_CALL
80+
)
81+
82+
/**
83+
* Object types in which the owner rank can be handed out. Everything else binds the conversation
84+
* to an object that already implies an owner, such as a share, a calendar event or a parent
85+
* conversation.
86+
*
87+
* The server also allows its `classified_persist` and `extended_conversation` object types, which
88+
* [ConversationEnums.ObjectType] does not model; those decode to
89+
* [ConversationEnums.ObjectType.DEFAULT] here.
90+
*/
91+
private val OWNER_CHANGE_OBJECT_TYPES = setOf(
92+
ConversationEnums.ObjectType.DEFAULT,
93+
ConversationEnums.ObjectType.CLASSIFIED,
94+
ConversationEnums.ObjectType.INSTANT_MEETING
95+
)
96+
97+
/** `USER_FOLLOWING_LINK` is this client's name for the server's `USER_SELF_JOINED`. */
98+
private val PROMOTABLE_TO_OWNER = setOf(
99+
Participant.ParticipantType.USER,
100+
Participant.ParticipantType.USER_FOLLOWING_LINK,
101+
Participant.ParticipantType.MODERATOR
102+
)
103+
104+
/**
105+
* Whether ownership can be handed over at all: only an owner may do it, only a user may hold it,
106+
* and only in a conversation that is not bound to an object with an implicit owner. Mirrors
107+
* `ParticipantService::updateParticipantTypeByModerator`, which enforces all of it again.
108+
*/
109+
fun canChangeOwnership(
110+
participant: Participant,
111+
conversation: ConversationModel?,
112+
spreedCapabilities: SpreedCapability?
113+
): Boolean =
114+
CapabilitiesUtil.hasSpreedFeatureCapability(spreedCapabilities, SpreedFeatures.PROMOTE_DEMOTE_OWNER) &&
115+
conversation != null &&
116+
conversation.participantType == Participant.ParticipantType.OWNER &&
117+
conversation.type in OWNER_CHANGE_CONVERSATION_TYPES &&
118+
conversation.objectType in OWNER_CHANGE_OBJECT_TYPES &&
119+
participant.calculatedActorType == Participant.ActorType.USERS
120+
121+
fun canBePromotedToOwner(
122+
participant: Participant,
123+
conversation: ConversationModel?,
124+
spreedCapabilities: SpreedCapability?
125+
): Boolean =
126+
canChangeOwnership(participant, conversation, spreedCapabilities) &&
127+
participant.type in PROMOTABLE_TO_OWNER
128+
129+
fun canBeDemotedFromOwner(
130+
participant: Participant,
131+
conversation: ConversationModel?,
132+
spreedCapabilities: SpreedCapability?
133+
): Boolean =
134+
canChangeOwnership(participant, conversation, spreedCapabilities) &&
135+
participant.type == Participant.ParticipantType.OWNER
69136
}

0 commit comments

Comments
 (0)