mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-10-06 11:48:24 +00:00
fix: decide top-level community posts from the parent, not the kind tag alone
isTopLevelCommunityPost() fell back to the root kind whenever no parent kind (`k`) was present. That fallback exists for bridged posts that carry only the uppercase root set, but it also swallowed nested replies that omit `k` -- those name a parent *event*, so they answer a post inside the community, not the community, and would have lost their parent card to a community card. Decide from what the comment points at instead: an explicit `k` is authoritative; failing that, a parent address that is a community means top level and a parent event means it is not; only a comment naming no parent at all falls back to the root kind. Also pin two things the earlier commits changed but did not cover: - communityAddress() against its pre-rewrite implementation as a reference oracle across eight tag shapes. Seven call sites outside this branch depend on it being unchanged, and nothing asserted that. - isCommunityDefinition() and the lastOrNull selection RenderRepost now shares, which is the logic that changed there. The composable itself would need an instrumented test; the predicate is where the bug lived. All three suites mutation-checked: reverting the predicate or reversing the address scan order fails them.
This commit is contained in:
+32
@@ -26,8 +26,10 @@ import com.vitorpamplona.quartz.nip01Core.core.Address
|
||||
import com.vitorpamplona.quartz.nip01Core.core.Event
|
||||
import com.vitorpamplona.quartz.nip22Comments.CommentEvent
|
||||
import kotlin.test.Test
|
||||
import kotlin.test.assertFalse
|
||||
import kotlin.test.assertNull
|
||||
import kotlin.test.assertSame
|
||||
import kotlin.test.assertTrue
|
||||
|
||||
class ParentNoteTest {
|
||||
private val communityOwner = "9ca0bd7450742d6a20319c0e3d4c679c9e046a9dc70e8ef55c2905e24052340b"
|
||||
@@ -140,6 +142,36 @@ class ParentNoteTest {
|
||||
assertSame(parent, replyingDirectlyTo(note, cache))
|
||||
}
|
||||
|
||||
/**
|
||||
* The predicate `RenderRepost` now shares (`note.replyTo?.lastOrNull { !it.isCommunityDefinition() }`).
|
||||
* The composable itself needs an instrumented test, but the logic that changed is this
|
||||
* predicate: it has to recognise the community from the address, with no event loaded.
|
||||
*/
|
||||
@Test
|
||||
fun communityIsRecognisedWithoutItsDefinitionEvent() {
|
||||
val uncached = AddressableNote(moviesAddress)
|
||||
assertNull(uncached.event, "precondition: the definition has not arrived")
|
||||
assertTrue(uncached.isCommunityDefinition(), "an uncached community must still be excluded")
|
||||
|
||||
val article = AddressableNote(Address(30023, communityOwner, "some-article"))
|
||||
assertFalse(article.isCommunityDefinition(), "other addressable kinds must not be excluded")
|
||||
|
||||
val plainNote = Note("55".repeat(32))
|
||||
assertFalse(plainNote.isCommunityDefinition(), "a note with no event and no address is not a community")
|
||||
}
|
||||
|
||||
/** The selection `RenderRepost` performs: skip the community, take the real boosted note. */
|
||||
@Test
|
||||
fun repostSelectionSkipsAnUncachedCommunityAndTakesTheRealNote() {
|
||||
val boosted = Note("66".repeat(32))
|
||||
val replyTo = listOf(AddressableNote(moviesAddress), boosted)
|
||||
|
||||
assertSame(boosted, replyTo.lastOrNull { !it.isCommunityDefinition() })
|
||||
|
||||
// Community-only: nothing to render, rather than the empty shell that produced the blank.
|
||||
assertNull(listOf(AddressableNote(moviesAddress)).lastOrNull { !it.isCommunityDefinition() })
|
||||
}
|
||||
|
||||
/** A post in a channel is rendered by the channel header, not as a parent note. */
|
||||
@Test
|
||||
fun noteInsideAChannelIsNotOfferedAsAParent() {
|
||||
|
||||
@@ -149,6 +149,9 @@ class CommentEvent(
|
||||
|
||||
fun directReplies() = tags.filter { ReplyIdentifierTag.match(it) || ReplyAddressTag.match(it) || ReplyEventTag.match(it) }
|
||||
|
||||
/** Whether a parent is named at all, without materialising [directReplies]. */
|
||||
fun hasDirectReplies() = tags.fastAny { ReplyIdentifierTag.match(it) || ReplyAddressTag.match(it) || ReplyEventTag.match(it) }
|
||||
|
||||
fun directKinds() = tags.filter(ReplyKindTag::match)
|
||||
|
||||
/** Whether a parent kind (`k`) is declared at all, without materialising [directKinds]. */
|
||||
|
||||
+12
-6
@@ -65,12 +65,18 @@ fun CommentEvent.isCommunityScoped() = hasRootScopeKind(CommunityDefinitionEvent
|
||||
* parent post's kind for the latter. Bridges (e.g. mostr) sometimes emit only the uppercase set,
|
||||
* hence the fallback to the root kind when no `k` is present.
|
||||
*/
|
||||
fun CommentEvent.isTopLevelCommunityPost() =
|
||||
if (hasDirectKinds()) {
|
||||
hasReplyScopeKind(CommunityDefinitionEvent.KIND_STR)
|
||||
} else {
|
||||
hasRootScopeKind(CommunityDefinitionEvent.KIND_STR)
|
||||
}
|
||||
fun CommentEvent.isTopLevelCommunityPost(): Boolean {
|
||||
// An explicit parent kind (`k`) is authoritative: it names what this comment answers.
|
||||
if (hasDirectKinds()) return hasReplyScopeKind(CommunityDefinitionEvent.KIND_STR)
|
||||
|
||||
// No `k`. Decide from what the comment actually points at rather than guessing: a parent
|
||||
// *address* that is a community means top level; a parent *event* means it answers a post
|
||||
// inside the community, not the community itself.
|
||||
if (hasDirectReplies()) return replyAddressIds().any { Address.isOfKind(it, CommunityDefinitionEvent.KIND_STR) }
|
||||
|
||||
// Nothing names a parent at all -- the shape bridges emit, with only the uppercase root set.
|
||||
return hasRootScopeKind(CommunityDefinitionEvent.KIND_STR)
|
||||
}
|
||||
|
||||
/**
|
||||
* The community this comment is a top-level post in, or null when it is a reply to another post
|
||||
|
||||
+82
@@ -22,6 +22,7 @@ package com.vitorpamplona.quartz.nip72ModCommunities
|
||||
|
||||
import com.vitorpamplona.quartz.nip01Core.core.Event
|
||||
import com.vitorpamplona.quartz.nip22Comments.CommentEvent
|
||||
import com.vitorpamplona.quartz.nip72ModCommunities.definition.CommunityDefinitionEvent
|
||||
import kotlin.test.Test
|
||||
import kotlin.test.assertEquals
|
||||
import kotlin.test.assertFalse
|
||||
@@ -107,6 +108,87 @@ class CommunityCommentScopeTest {
|
||||
assertTrue(event.tagsWithoutCitations().contains(parentId))
|
||||
}
|
||||
|
||||
/**
|
||||
* A nested reply that omits the parent kind (`k`) entirely -- malformed, but emitted in the
|
||||
* wild. It must not be mistaken for a top-level post just because the root kind says 34550:
|
||||
* it names a parent *event*, so it answers a post inside the community, not the community.
|
||||
*/
|
||||
@Test
|
||||
fun nestedReplyWithoutAParentKindIsNotATopLevelPost() {
|
||||
val event =
|
||||
comment(
|
||||
arrayOf("A", movies),
|
||||
arrayOf("K", "34550"),
|
||||
arrayOf("e", "33".repeat(32)),
|
||||
)
|
||||
|
||||
assertTrue(event.isCommunityScoped())
|
||||
assertFalse(event.isTopLevelCommunityPost())
|
||||
}
|
||||
|
||||
/** The mirror case: no `k`, but the parent *address* is the community, so it is top level. */
|
||||
@Test
|
||||
fun postNamingTheCommunityAsItsParentAddressIsATopLevelPost() {
|
||||
val event =
|
||||
comment(
|
||||
arrayOf("A", movies),
|
||||
arrayOf("a", movies),
|
||||
arrayOf("K", "34550"),
|
||||
)
|
||||
|
||||
assertTrue(event.isTopLevelCommunityPost())
|
||||
}
|
||||
|
||||
/** A reply whose parent address is some other addressable event is not a top-level post. */
|
||||
@Test
|
||||
fun replyToANonCommunityAddressIsNotATopLevelPost() {
|
||||
val event =
|
||||
comment(
|
||||
arrayOf("A", movies),
|
||||
arrayOf("a", "30023:$communityOwner:some-article"),
|
||||
arrayOf("K", "34550"),
|
||||
)
|
||||
|
||||
assertFalse(event.isTopLevelCommunityPost())
|
||||
}
|
||||
|
||||
/**
|
||||
* [communityAddress] was rewritten to stop at the first community root address instead of
|
||||
* parsing every `A` tag into a list. This pins it against the original algorithm as a
|
||||
* reference oracle -- seven call sites outside this PR depend on it being unchanged.
|
||||
*/
|
||||
@Test
|
||||
fun communityAddressMatchesTheOriginalAlgorithmOnEveryShape() {
|
||||
val other = "30023:$communityOwner:some-article"
|
||||
val secondCommunity = "34550:$communityOwner:films"
|
||||
|
||||
val shapes =
|
||||
listOf(
|
||||
"no tags at all" to comment(),
|
||||
"one community root" to comment(arrayOf("A", movies)),
|
||||
"non-community root only" to comment(arrayOf("A", other)),
|
||||
"non-community first, community second" to comment(arrayOf("A", other), arrayOf("A", movies)),
|
||||
"two communities picks the first" to comment(arrayOf("A", movies), arrayOf("A", secondCommunity)),
|
||||
"unparseable root" to comment(arrayOf("A", "not-an-address"), arrayOf("A", movies)),
|
||||
"reply address only, no root" to comment(arrayOf("a", movies)),
|
||||
"root without a value" to comment(arrayOf("A")),
|
||||
)
|
||||
|
||||
shapes.forEach { (name, event) ->
|
||||
assertEquals(
|
||||
originalCommunityAddress(event)?.toValue(),
|
||||
(event as Event).communityAddress()?.toValue(),
|
||||
"communityAddress diverged from the original algorithm for: $name",
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
/** The pre-rewrite implementation, kept here purely as the oracle for the test above. */
|
||||
private fun originalCommunityAddress(event: CommentEvent) =
|
||||
event.rootAddress().firstNotNullOfOrNull {
|
||||
if (it.kind == CommunityDefinitionEvent.KIND) it else null
|
||||
}
|
||||
|
||||
@Test
|
||||
fun commentOnSomethingOtherThanACommunityIsNotATopLevelPost() {
|
||||
val event =
|
||||
|
||||
Reference in New Issue
Block a user