Skip to content
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

Fix admin actions visibility for non admin mods #1412

Merged
merged 2 commits into from
Feb 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 44 additions & 29 deletions app/src/main/java/com/jerboa/feat/ModActions.kt
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
package com.jerboa.feat

import it.vercruysse.lemmyapi.v0x19.datatypes.CommunityModeratorView
import android.content.Context
import com.jerboa.MainActivity
import com.jerboa.db.entity.Account
import com.jerboa.findActivity
import it.vercruysse.lemmyapi.v0x19.datatypes.CommunityId
import it.vercruysse.lemmyapi.v0x19.datatypes.PersonId
import it.vercruysse.lemmyapi.v0x19.datatypes.PersonView
import java.time.Instant
Expand All @@ -12,29 +16,25 @@ import java.time.temporal.ChronoUnit
fun canMod(
creatorId: PersonId,
admins: List<PersonView>?,
moderators: List<CommunityModeratorView>?,
myId: PersonId?,
moderators: List<PersonId>?,
myId: PersonId,
onSelf: Boolean = false,
): Boolean {
return if (myId !== null) {
// You can do moderator actions only on the mods added after you.
val adminIds = admins?.map { a -> a.person.id }.orEmpty()
val modIds = moderators?.map { m -> m.moderator.id }.orEmpty()

val adminsThenMods = adminIds.toMutableList()
adminsThenMods.addAll(modIds)

val myIndex = adminsThenMods.indexOf(myId)
if (myIndex == -1) {
false
} else {
// onSelf +1 on mod actions not for yourself, IE ban, remove, etc
val subList = adminsThenMods.subList(0, myIndex.plus(if (onSelf) 0 else 1))

!subList.contains(creatorId)
}
} else {
// You can do moderator actions only on the mods added after you.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the moderator hierarchy not apply to admins? i.e. Any admin can moderate any admin?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does apply to other admins. Check L28 and L35, it adds admins, then mods for the ordered hierarchy.

Copy link
Collaborator Author

@MV-GH MV-GH Feb 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not about the code, but rather does Lemmy also have the hierarchy for admin. (so yes I assume)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep admins also have a hierarchy.

val adminIds = admins?.map { a -> a.person.id }.orEmpty()
val modIds = moderators.orEmpty()

val adminsThenMods = adminIds.toMutableList()
adminsThenMods.addAll(modIds)

val myIndex = adminsThenMods.indexOf(myId)
return if (myIndex == -1) {
false
} else {
// onSelf +1 on mod actions not for yourself, IE ban, remove, etc
val subList = adminsThenMods.subList(0, myIndex.plus(if (onSelf) 0 else 1))

!subList.contains(creatorId)
}
}

Expand All @@ -45,15 +45,30 @@ fun futureDaysToUnixTime(days: Long?): Long? {
}

fun amMod(
moderators: List<CommunityModeratorView>?,
myId: PersonId?,
moderators: List<PersonId>?,
myId: PersonId,
): Boolean {
return moderators?.map { it.moderator.id }?.contains(myId) ?: false
return moderators?.contains(myId) ?: false
}

fun amAdmin(
admins: List<PersonView>?,
myId: PersonId?,
): Boolean {
return admins?.map { it.person.id }?.contains(myId) ?: false
/**
* In screens with posts from different communities we don't have access to moderators of those communities
* So that means that non admin mods can't moderate those posts from that screen
*
* So this is QoL were we simulate the mods of the community
* It is not completely accurate as it doesn't take into account the hierarchy of mods
*/
Copy link
Member

@dessalines dessalines Feb 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This really shows that the canMod logic needs to be added on the back-end: LemmyNet/lemmy#4365

Once I eventually get some time to work on that, we'd be able to remove the mod list from almost every response, and stuff like this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing I forgot, how does the self option play into this? Which mod actions can you do on yourself? like lockpost? How would that work with canMod from server side?

Currently we don't do that correctly either. canMod should probably return which actions can be done?

Copy link
Member

@dessalines dessalines Feb 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just looked through the lemmy-ui code, and its only distinguish that you can do to yourself, and anyone lower than you. We could probably change that to just isModOrAdmin && myComment , and remove the onSelf attribute.

Copy link
Collaborator Author

@MV-GH MV-GH Feb 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only distinguish? Can top admin not pin his own comment? Or lock/feature his own posts?

fun simulateModerators(
ctx: Context,
account: Account,
forCommunity: CommunityId,
): List<PersonId> {
if (account.isMod) {
val siteVM = (ctx.findActivity() as MainActivity).siteViewModel
val canModerate = siteVM.moderatedCommunities().orEmpty().contains(forCommunity)
if (canModerate) {
return listOf(account.id)
}
}
return emptyList()
}
9 changes: 9 additions & 0 deletions app/src/main/java/com/jerboa/model/SiteViewModel.kt
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import com.jerboa.db.entity.isAnon
import com.jerboa.db.repository.AccountRepository
import com.jerboa.jerboaApplication
import it.vercruysse.lemmyapi.v0x19.datatypes.CommunityFollowerView
import it.vercruysse.lemmyapi.v0x19.datatypes.CommunityId
import it.vercruysse.lemmyapi.v0x19.datatypes.GetReportCount
import it.vercruysse.lemmyapi.v0x19.datatypes.GetReportCountResponse
import it.vercruysse.lemmyapi.v0x19.datatypes.GetSiteResponse
Expand Down Expand Up @@ -209,6 +210,13 @@ class SiteViewModel(private val accountRepository: AccountRepository) : ViewMode
}
}

fun moderatedCommunities(): List<CommunityId>? {
return when (val res = siteRes) {
is ApiState.Success -> res.data.my_user?.moderates?.map { it.community.id }
else -> null
}
}

fun enableDownvotes(): Boolean {
return when (val res = siteRes) {
is ApiState.Success -> res.data.site_view.local_site.enable_downvotes
Expand All @@ -231,6 +239,7 @@ class SiteViewModel(private val accountRepository: AccountRepository) : ViewMode
} ?: run {
defaultMode
}

else -> defaultMode
}
}
Expand Down
48 changes: 18 additions & 30 deletions app/src/main/java/com/jerboa/ui/components/comment/CommentNode.kt
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ import com.jerboa.feat.InstantScores
import com.jerboa.feat.SwipeToActionPreset
import com.jerboa.feat.SwipeToActionType
import com.jerboa.feat.VoteType
import com.jerboa.feat.amAdmin
import com.jerboa.feat.amMod
import com.jerboa.feat.canMod
import com.jerboa.feat.isReadyAndIfNotShowSimplifiedInfoToast
Expand All @@ -88,7 +87,6 @@ import it.vercruysse.lemmyapi.v0x19.datatypes.Comment
import it.vercruysse.lemmyapi.v0x19.datatypes.CommentId
import it.vercruysse.lemmyapi.v0x19.datatypes.CommentView
import it.vercruysse.lemmyapi.v0x19.datatypes.Community
import it.vercruysse.lemmyapi.v0x19.datatypes.CommunityModeratorView
import it.vercruysse.lemmyapi.v0x19.datatypes.Person
import it.vercruysse.lemmyapi.v0x19.datatypes.PersonId
import it.vercruysse.lemmyapi.v0x19.datatypes.PersonView
Expand Down Expand Up @@ -186,7 +184,7 @@ fun CommentBodyPreview() {
fun LazyListScope.commentNodeItem(
node: CommentNode,
admins: List<PersonView>,
moderators: List<CommunityModeratorView>?,
moderators: List<PersonId>?,
increaseLazyListIndexTracker: () -> Unit,
addToParentIndexes: () -> Unit,
isFlat: Boolean,
Expand Down Expand Up @@ -495,7 +493,7 @@ fun LazyListScope.commentNodeItem(
fun LazyListScope.missingCommentNodeItem(
node: MissingCommentNode,
admins: List<PersonView>,
moderators: List<CommunityModeratorView>?,
moderators: List<PersonId>?,
increaseLazyListIndexTracker: () -> Unit,
addToParentIndexes: () -> Unit,
isFlat: Boolean,
Expand Down Expand Up @@ -744,7 +742,7 @@ fun PostAndCommunityContextHeaderPreview() {
fun CommentFooterLine(
commentView: CommentView,
admins: List<PersonView>,
moderators: List<CommunityModeratorView>?,
moderators: List<PersonId>?,
enableDownVotes: Boolean,
instantScores: InstantScores,
onUpvoteClick: () -> Unit,
Expand All @@ -770,31 +768,21 @@ fun CommentFooterLine(
) {
var showMoreOptions by remember { mutableStateOf(false) }

val amAdmin =
remember(admins) {
amAdmin(
admins = admins,
myId = account.id,
)
}

val amMod =
remember {
amMod(
moderators = moderators,
myId = account.id,
)
}
val amMod = remember(moderators) {
amMod(
moderators = moderators,
myId = account.id,
)
}

val canMod =
remember(admins) {
canMod(
creatorId = commentView.comment.creator_id,
admins = admins,
moderators = moderators,
myId = account.id,
)
}
val canMod = remember(admins) {
canMod(
creatorId = commentView.comment.creator_id,
admins = admins,
moderators = moderators,
myId = account.id,
)
}

if (showMoreOptions) {
CommentOptionsDropdown(
Expand All @@ -815,7 +803,7 @@ fun CommentFooterLine(
isCreator = account.id == commentView.creator.id,
canMod = canMod,
amMod = amMod,
amAdmin = amAdmin,
amAdmin = account.isAdmin,
viewSource = viewSource,
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import com.jerboa.feat.SwipeToActionPreset
import it.vercruysse.lemmyapi.v0x19.datatypes.CommentId
import it.vercruysse.lemmyapi.v0x19.datatypes.CommentView
import it.vercruysse.lemmyapi.v0x19.datatypes.Community
import it.vercruysse.lemmyapi.v0x19.datatypes.CommunityModeratorView
import it.vercruysse.lemmyapi.v0x19.datatypes.Person
import it.vercruysse.lemmyapi.v0x19.datatypes.PersonId
import it.vercruysse.lemmyapi.v0x19.datatypes.PersonView
Expand All @@ -29,7 +28,7 @@ import it.vercruysse.lemmyapi.v0x19.datatypes.PostId
fun CommentNodes(
nodes: List<CommentNodeData>,
admins: List<PersonView>,
moderators: List<CommunityModeratorView>?,
moderators: List<PersonId>?,
increaseLazyListIndexTracker: () -> Unit,
addToParentIndexes: () -> Unit,
isFlat: Boolean,
Expand Down Expand Up @@ -123,7 +122,7 @@ fun CommentNodes(
fun LazyListScope.commentNodeItems(
nodes: List<CommentNodeData>,
admins: List<PersonView>,
moderators: List<CommunityModeratorView>?,
moderators: List<PersonId>?,
increaseLazyListIndexTracker: () -> Unit,
addToParentIndexes: () -> Unit,
isFlat: Boolean,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,6 @@ import com.jerboa.ui.components.common.BanFromCommunityPopupMenuItem
import com.jerboa.ui.components.common.BanPersonPopupMenuItem
import com.jerboa.ui.components.common.PopupMenuItem
import com.jerboa.util.cascade.CascadeCenteredDropdownMenu
import io.github.z4kn4fein.semver.toVersion
import it.vercruysse.lemmyapi.FeatureFlags
import it.vercruysse.lemmyapi.v0x19.datatypes.CommentId
import it.vercruysse.lemmyapi.v0x19.datatypes.CommentView
import it.vercruysse.lemmyapi.v0x19.datatypes.Person
Expand Down Expand Up @@ -215,7 +213,9 @@ fun CommentOptionsDropdown(
onRemoveClick(commentView)
},
)
BanPersonPopupMenuItem(commentView.creator, onDismissRequest, onBanPersonClick)
if (amAdmin) {
BanPersonPopupMenuItem(commentView.creator, onDismissRequest, onBanPersonClick)
}

// Only show ban from community button if its a local community
if (commentView.community.local) {
Expand Down Expand Up @@ -251,7 +251,7 @@ fun CommentOptionsDropdown(
}

// You can do these actions on mods above you
if (FeatureFlags(version = API.version.toVersion()).listAdminVotes()) {
if (amAdmin && API.getInstanceOrNull()?.FF?.listAdminVotes() == true) {
PopupMenuItem(
text = stringResource(R.string.view_votes),
icon = ImageVector.vectorResource(R.drawable.up_filled),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ import com.jerboa.ui.theme.SMALL_PADDING
import com.jerboa.ui.theme.XXL_PADDING
import com.jerboa.ui.theme.muted
import it.vercruysse.lemmyapi.v0x19.datatypes.Community
import it.vercruysse.lemmyapi.v0x19.datatypes.CommunityModeratorView
import it.vercruysse.lemmyapi.v0x19.datatypes.Person
import it.vercruysse.lemmyapi.v0x19.datatypes.PersonId
import it.vercruysse.lemmyapi.v0x19.datatypes.PersonMentionView
Expand Down Expand Up @@ -101,7 +100,7 @@ fun CommentMentionNodeHeaderPreview() {
fun CommentMentionNodeFooterLine(
personMentionView: PersonMentionView,
admins: List<PersonView>,
moderators: List<CommunityModeratorView>?,
moderators: List<PersonId>?,
onUpvoteClick: () -> Unit,
onDownvoteClick: () -> Unit,
onReplyClick: (personMentionView: PersonMentionView) -> Unit,
Expand Down Expand Up @@ -244,7 +243,7 @@ fun CommentMentionNodeFooterLine(
fun CommentMentionNode(
personMentionView: PersonMentionView,
admins: List<PersonView>,
moderators: List<CommunityModeratorView>?,
moderators: List<PersonId>?,
onUpvoteClick: (personMentionView: PersonMentionView) -> Unit,
onDownvoteClick: (personMentionView: PersonMentionView) -> Unit,
onReplyClick: (personMentionView: PersonMentionView) -> Unit,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ fun CommunityActivity(
PostListings(
posts = postsRes.data.posts,
admins = siteViewModel.admins(),
moderators = moderators,
moderators = remember(moderators) { moderators?.map { it.moderator.id } },
contentAboveListings = {
when (communityRes) {
is ApiState.Success -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,13 +193,13 @@ fun PersonProfileActivity(
}
is ApiState.Holder -> {
val person = profileRes.data.person_view.person
val canBan =
canMod(
creatorId = person.id,
admins = siteViewModel.admins(),
moderators = null,
myId = account.id,
)
val canBan = canMod(
creatorId = person.id,
admins = siteViewModel.admins(),
moderators = null,
myId = account.id,
)

PersonProfileHeader(
scrollBehavior = scrollBehavior,
personName =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ fun PostActivity(
is ApiState.Failure -> ApiErrorText(postRes.msg, padding)
is ApiState.Success -> {
val postView = postRes.data.post_view
val moderators = postRes.data.moderators
val moderators = remember(postRes) { postRes.data.moderators.map { it.moderator.id } }
dessalines marked this conversation as resolved.
Show resolved Hide resolved

if (!account.isAnon()) appState.addReturn(PostViewReturn.POST_VIEW, postView.copy(read = true))
LazyColumn(
Expand Down
Loading