From ff780bcb543950d6bfba4dc727035cbdb53b389d Mon Sep 17 00:00:00 2001 From: davotoula Date: Tue, 12 May 2026 18:30:52 +0200 Subject: [PATCH] Code review: - consolidate thread-root resolution - consolidate mute-state writes --- .../vitorpamplona/amethyst/model/Account.kt | 12 ++--- .../amethyst/ui/dal/FilterByListParams.kt | 5 +- .../dal/NotificationFeedFilter.kt | 7 +-- .../settings/dal/MutedThreadsFeedFilter.kt | 2 +- .../amethyst/commons/model/Note.kt | 9 ++-- .../commons/model/NoteIsHiddenForTest.kt | 53 ++----------------- .../quartz/nip10Notes/BaseThreadedEvent.kt | 3 ++ 7 files changed, 23 insertions(+), 68 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt index d3b1cfa56..2f7f2877a 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt @@ -167,11 +167,11 @@ import com.vitorpamplona.quartz.nip03Timestamp.OtsResolver import com.vitorpamplona.quartz.nip04Dm.PrivateDMCache import com.vitorpamplona.quartz.nip04Dm.messages.PrivateDmEvent import com.vitorpamplona.quartz.nip09Deletions.DeletionEvent -import com.vitorpamplona.quartz.nip10Notes.BaseThreadedEvent import com.vitorpamplona.quartz.nip10Notes.TextNoteEvent import com.vitorpamplona.quartz.nip10Notes.content.findHashtags import com.vitorpamplona.quartz.nip10Notes.content.findNostrUris import com.vitorpamplona.quartz.nip10Notes.content.findURLs +import com.vitorpamplona.quartz.nip10Notes.threadRootIdOrSelf import com.vitorpamplona.quartz.nip17Dm.NIP17Factory import com.vitorpamplona.quartz.nip17Dm.base.NIP17Group import com.vitorpamplona.quartz.nip17Dm.files.ChatMessageEncryptedFileHeaderEvent @@ -2838,17 +2838,16 @@ class Account( } suspend fun muteThread(rootHex: HexKey) { + if (isThreadMuted(rootHex)) return sendMyPublicAndPrivateOutbox(muteList.hideThread(rootHex)) } suspend fun unmuteThread(rootHex: HexKey) { + if (!isThreadMuted(rootHex)) return muteList.showThread(rootHex)?.let { sendMyPublicAndPrivateOutbox(it) } } - fun resolveThreadRoot(note: Note): HexKey { - val ev = note.event - return (ev as? BaseThreadedEvent)?.root()?.eventId ?: note.idHex - } + fun resolveThreadRoot(note: Note): HexKey = note.event?.threadRootIdOrSelf() ?: note.idHex fun isThreadMuted(rootHex: HexKey): Boolean = hiddenUsers.flow.value.isThreadMuted(rootHex) @@ -2986,7 +2985,8 @@ class Account( } override fun isAcceptable(note: Note): Boolean { - if (isThreadMuted(resolveThreadRoot(note))) return false + val mutedThreads = hiddenUsers.flow.value.mutedThreads + if (mutedThreads.isNotEmpty() && mutedThreads.contains(resolveThreadRoot(note))) return false return note.author?.let { isAcceptable(it) } ?: true && // if user hasn't hided this author isAcceptableDirect(note) && diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/dal/FilterByListParams.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/dal/FilterByListParams.kt index bb37bd3a2..c1ef9f4c4 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/dal/FilterByListParams.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/dal/FilterByListParams.kt @@ -31,7 +31,7 @@ import com.vitorpamplona.quartz.nip01Core.core.Event import com.vitorpamplona.quartz.nip01Core.core.HexKey import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl import com.vitorpamplona.quartz.nip01Core.tags.hashtags.countHashtags -import com.vitorpamplona.quartz.nip10Notes.BaseThreadedEvent +import com.vitorpamplona.quartz.nip10Notes.threadRootIdOrSelf import com.vitorpamplona.quartz.nip51Lists.muteList.MuteListEvent import com.vitorpamplona.quartz.nip51Lists.peopleList.PeopleListEvent import com.vitorpamplona.quartz.utils.TimeUtils @@ -46,8 +46,7 @@ class FilterByListParams( fun isNotInMutedThread(noteEvent: Event): Boolean { if (hiddenLists.mutedThreads.isEmpty()) return true - val rootId = (noteEvent as? BaseThreadedEvent)?.root()?.eventId ?: noteEvent.id - return !hiddenLists.mutedThreads.contains(rootId) + return !hiddenLists.mutedThreads.contains(noteEvent.threadRootIdOrSelf()) } fun isNotInTheFuture(noteEvent: Event) = noteEvent.createdAt <= now diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/notifications/dal/NotificationFeedFilter.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/notifications/dal/NotificationFeedFilter.kt index 324088747..c24217b76 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/notifications/dal/NotificationFeedFilter.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/notifications/dal/NotificationFeedFilter.kt @@ -225,11 +225,8 @@ class NotificationFeedFilter( } } - // Drop reactions/zaps/reposts whose target post is in a muted thread. - // The inner-note renderer would otherwise show a misleading - // "This post was hidden because it mentions your hidden users or words" - // placeholder for muted-thread targets (regression from Task 6's - // Note.isHiddenFor extension). + // Reactions/zaps/reposts target a note via `replyTo`, not via thread-root tags, + // so isNotInMutedThread on the wrapper event misses them. if (noteEvent is ReactionEvent || noteEvent is LnZapEvent || noteEvent is RepostEvent || noteEvent is GenericRepostEvent ) { diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/settings/dal/MutedThreadsFeedFilter.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/settings/dal/MutedThreadsFeedFilter.kt index 594ce96c6..616b91b5f 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/settings/dal/MutedThreadsFeedFilter.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/settings/dal/MutedThreadsFeedFilter.kt @@ -34,6 +34,6 @@ class MutedThreadsFeedFilter( override fun feed(): List = account.hiddenUsers.flow.value.mutedThreads - .mapNotNull { LocalCache.getNoteIfExists(it) ?: LocalCache.getOrCreateNote(it) } + .map { LocalCache.getOrCreateNote(it) } .sortedByDescending { it.createdAt() } } diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/Note.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/Note.kt index 6f1ef34c5..1a2790d08 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/Note.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/Note.kt @@ -39,6 +39,7 @@ import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl import com.vitorpamplona.quartz.nip01Core.tags.hashtags.anyHashTag import com.vitorpamplona.quartz.nip01Core.tags.publishedAt.PublishedAtProvider import com.vitorpamplona.quartz.nip10Notes.BaseThreadedEvent +import com.vitorpamplona.quartz.nip10Notes.threadRootIdOrSelf import com.vitorpamplona.quartz.nip18Reposts.GenericRepostEvent import com.vitorpamplona.quartz.nip18Reposts.RepostEvent import com.vitorpamplona.quartz.nip19Bech32.entities.NAddress @@ -860,10 +861,10 @@ open class Note( return true } - // if this note belongs to a muted thread (by NIP-10 root id) - if (accountChoices.mutedThreads.isNotEmpty()) { - val rootId = (thisEvent as? BaseThreadedEvent)?.root()?.eventId ?: idHex - if (accountChoices.mutedThreads.contains(rootId)) return true + if (accountChoices.mutedThreads.isNotEmpty() && + accountChoices.mutedThreads.contains(thisEvent.threadRootIdOrSelf()) + ) { + return true } // if the post is sensitive and the user doesn't want to see sensitive content diff --git a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/model/NoteIsHiddenForTest.kt b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/model/NoteIsHiddenForTest.kt index 5f54c096a..b8c137800 100644 --- a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/model/NoteIsHiddenForTest.kt +++ b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/model/NoteIsHiddenForTest.kt @@ -25,22 +25,11 @@ import kotlin.test.Test import kotlin.test.assertFalse import kotlin.test.assertTrue -/** - * Unit tests for Note.isHiddenFor() focusing on the mutedThreads check. - * - * Fixture approach: - * - TextNoteEvent is constructed directly via its primary constructor (no signing needed — - * the sig field is not validated in isHiddenFor). - * - A Note is created with the event's id, then note.event is set. - * - LiveHiddenUsers is a plain data class — no Android plumbing required. - */ class NoteIsHiddenForTest { - // 64-char hex constants private val rootId = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" private val replyId = "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" private val authorPubKey = "cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc" - // Minimal LiveHiddenUsers with all filters off (used as a baseline) private val noHidden = LiveHiddenUsers( showSensitiveContent = null, @@ -50,12 +39,6 @@ class NoteIsHiddenForTest { mutedThreads = emptySet(), ) - /** - * Build a TextNoteEvent. - * - * @param id The event id (64 hex chars). - * @param eTags Raw `e` tag arrays included in the event's tags. - */ private fun textNoteEvent( id: String, pubKey: String = authorPubKey, @@ -69,21 +52,12 @@ class NoteIsHiddenForTest { sig = "sig", ) - /** A marked `e` tag array with marker="root" at index 3. */ private fun rootETag(eventId: String) = arrayOf("e", eventId, "", "root") - // ------------------------------------------------------------------------- - @Test fun reply_inMutedThread_isHidden() { - // A reply whose NIP-10 root points to `rootId`, which is muted. - val event = - textNoteEvent( - id = replyId, - eTags = arrayOf(rootETag(rootId)), - ) + val event = textNoteEvent(id = replyId, eTags = arrayOf(rootETag(rootId))) val note = Note(replyId).also { it.event = event } - val choices = noHidden.copy(mutedThreads = setOf(rootId)) assertTrue(note.isHiddenFor(choices), "Reply inside a muted thread must be hidden") @@ -91,14 +65,8 @@ class NoteIsHiddenForTest { @Test fun topLevelNote_ownIdMuted_isHidden() { - // A top-level note (no e-tags) whose own id is in mutedThreads. - val event = - textNoteEvent( - id = rootId, - eTags = emptyArray(), - ) + val event = textNoteEvent(id = rootId, eTags = emptyArray()) val note = Note(rootId).also { it.event = event } - val choices = noHidden.copy(mutedThreads = setOf(rootId)) assertTrue(note.isHiddenFor(choices), "Top-level note whose id is muted must be hidden") @@ -106,16 +74,9 @@ class NoteIsHiddenForTest { @Test fun note_inUnmutedThread_isNotHidden() { - // A reply note whose root is NOT in mutedThreads, author not hidden. val otherRoot = "dddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd" - val event = - textNoteEvent( - id = replyId, - eTags = arrayOf(rootETag(otherRoot)), - ) + val event = textNoteEvent(id = replyId, eTags = arrayOf(rootETag(otherRoot))) val note = Note(replyId).also { it.event = event } - - // rootId is muted, but this note's root is otherRoot val choices = noHidden.copy(mutedThreads = setOf(rootId)) assertFalse(note.isHiddenFor(choices), "Reply in an un-muted thread must not be hidden") @@ -123,15 +84,9 @@ class NoteIsHiddenForTest { @Test fun authorHidden_isHidden_regression() { - // Regression guard: author-hidden notes must still return true. val event = textNoteEvent(id = replyId) val note = Note(replyId).also { it.event = event } - - // Put the author's pubKey hashCode into hiddenUsersHashCodes - val choices = - noHidden.copy( - hiddenUsersHashCodes = setOf(authorPubKey.hashCode()), - ) + val choices = noHidden.copy(hiddenUsersHashCodes = setOf(authorPubKey.hashCode())) assertTrue(note.isHiddenFor(choices), "Note whose author is hidden must still be hidden") } diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip10Notes/BaseThreadedEvent.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip10Notes/BaseThreadedEvent.kt index 56e92115c..bec989c89 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip10Notes/BaseThreadedEvent.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip10Notes/BaseThreadedEvent.kt @@ -22,6 +22,7 @@ package com.vitorpamplona.quartz.nip10Notes import androidx.compose.runtime.Immutable import com.vitorpamplona.quartz.experimental.nipsOnNostr.NipTextEvent +import com.vitorpamplona.quartz.nip01Core.core.Event import com.vitorpamplona.quartz.nip01Core.core.HexKey import com.vitorpamplona.quartz.nip01Core.tags.aTag.taggedATags import com.vitorpamplona.quartz.nip01Core.tags.people.taggedUsers @@ -30,6 +31,8 @@ import com.vitorpamplona.quartz.nip54Wiki.WikiNoteEvent import com.vitorpamplona.quartz.nip72ModCommunities.definition.CommunityDefinitionEvent import com.vitorpamplona.quartz.utils.lastNotNullOfOrNull +fun Event.threadRootIdOrSelf(): HexKey = (this as? BaseThreadedEvent)?.root()?.eventId ?: id + @Immutable open class BaseThreadedEvent( id: HexKey,