Merge pull request #989 from PR0M3TH3AN/codex/update-video-modal-comment-handling

Allow comments without definition address

js/nostr/commentEvents.js

@@ -223,7 +223,7 @@ function normalizeCommentTarget(targetInput = {}, overrides = {}) {
     target.participantRelay,
   );
 
-  if (!videoEventId || !videoDefinitionAddress) {
+  if (!videoEventId) {
     return null;
   }
 

js/services/commentThreadService.js

@@ -141,7 +141,7 @@ export default class CommentThreadService {
     this.parentCommentId = normalizeString(parentCommentId);
     this.activeRelays = Array.isArray(relays) ? [...relays] : null;
 
-    if (!this.videoEventId || !this.videoAddressPointer) {
+    if (!this.videoEventId) {
       this.emitError(
         new Error("Unable to resolve video pointer for comment thread."),
       );
@@ -158,10 +158,13 @@ export default class CommentThreadService {
     const fetchLimit = toPositiveInteger(limit, this.defaultLimit);
     const target = {
       videoEventId: this.videoEventId,
-      videoDefinitionAddress: this.videoAddressPointer,
       parentCommentId: this.parentCommentId,
     };
 
+    if (this.videoAddressPointer) {
+      target.videoDefinitionAddress = this.videoAddressPointer;
+    }
+
     let events = [];
     try {
       events = await this.fetchVideoComments(target, {

js/ui/videoModalCommentController.js

@@ -132,7 +132,7 @@ export default class VideoModalCommentController {
     const videoEventId = normalizeString(video.id);
     const videoDefinitionAddress = buildVideoAddressPointer(video);
 
-    if (!videoEventId || !videoDefinitionAddress) {
+    if (!videoEventId) {
       this.resetModalCommentState({ hide: false });
       this.videoModal.setCommentStatus?.(
         "Comments are unavailable for this video.",
@@ -142,7 +142,7 @@ export default class VideoModalCommentController {
 
     this.modalCommentState = {
       videoEventId,
-      videoDefinitionAddress,
+      videoDefinitionAddress: videoDefinitionAddress || null,
       parentCommentId: null,
     };
     this.modalCommentProfiles = new Map();
@@ -487,7 +487,7 @@ export default class VideoModalCommentController {
     const videoEventId = normalizeString(video.id);
     const videoDefinitionAddress = buildVideoAddressPointer(video);
 
-    if (!videoEventId || !videoDefinitionAddress) {
+    if (!videoEventId) {
       this.callbacks.showError("Comments are unavailable for this video.");
       return;
     }
@@ -499,13 +499,18 @@ export default class VideoModalCommentController {
       reason: "submitting",
     });
 
+    const publishPayload = {
+      videoEventId,
+      parentCommentId,
+    };
+
+    if (videoDefinitionAddress) {
+      publishPayload.videoDefinitionAddress = videoDefinitionAddress;
+    }
+
     const publishPromise = Promise.resolve(
       this.services.publishComment(
-        {
-          videoEventId,
-          videoDefinitionAddress,
-          parentCommentId,
-        },
+        publishPayload,
         {
           content: text,
         },

tests/comment-event-builder.test.mjs

@@ -1,9 +1,16 @@
 import test from "node:test";
 import assert from "node:assert/strict";
 
-import { buildCommentEvent } from "../js/nostrEventSchemas.js";
+import {
+  buildCommentEvent,
+  getNostrEventSchema,
+  NOTE_TYPES,
+} from "../js/nostrEventSchemas.js";
 
-test("buildCommentEvent produces required tags for top-level comments", () => {
+const VIDEO_COMMENT_KIND =
+  getNostrEventSchema(NOTE_TYPES.VIDEO_COMMENT)?.kind ?? 1111;
+
+test("buildCommentEvent references the video definition when available", () => {
   const event = buildCommentEvent({
     pubkey: "commenter",
     created_at: 1700000200,
@@ -14,10 +21,7 @@ test("buildCommentEvent produces required tags for top-level comments", () => {
     content: "Great \ud800video!",
   });
 
-  assert.equal(event.kind, 1);
-
-  const eventTags = event.tags.filter((tag) => Array.isArray(tag) && tag[0] === "e");
-  assert.deepEqual(eventTags, [["e", "video-event-id", "wss://relay.example"]]);
+  assert.equal(event.kind, VIDEO_COMMENT_KIND);
 
   const addressTags = event.tags.filter((tag) => Array.isArray(tag) && tag[0] === "a");
   assert.deepEqual(addressTags, [["a", "30078:deadbeefcafebabe:clip-1"]]);
@@ -39,13 +43,10 @@ test("buildCommentEvent includes parent pointers for threaded replies", () => {
     content: "Replying now",
   });
 
-  assert.equal(event.kind, 1);
+  assert.equal(event.kind, VIDEO_COMMENT_KIND);
 
   const eventTags = event.tags.filter((tag) => Array.isArray(tag) && tag[0] === "e");
-  assert.deepEqual(eventTags, [
-    ["e", "video-event-id"],
-    ["e", "parent-comment-id"],
-  ]);
+  assert.deepEqual(eventTags, [["e", "parent-comment-id"]]);
 
   const addressTags = event.tags.filter((tag) => Array.isArray(tag) && tag[0] === "a");
   assert.deepEqual(addressTags, [["a", "30078:deadbeefcafebabe:clip-1"]]);
@@ -70,9 +71,8 @@ test("buildCommentEvent normalizes optional relays and preserves additional tags
     content: " Appreciated! \ud800",
   });
 
-  assert.equal(event.kind, 1);
+  assert.equal(event.kind, VIDEO_COMMENT_KIND);
   assert.deepStrictEqual(event.tags, [
-    ["e", "event123", "wss://comments.main"],
     ["a", "30078:deadbeefcafebabe:clip-2", "wss://video.def"],
     ["e", "root-comment", "wss://parent"],
     ["p", "cafecafe", "wss://profile"],
@@ -81,3 +81,22 @@ test("buildCommentEvent normalizes optional relays and preserves additional tags
   ]);
   assert.equal(event.content, " Appreciated! ");
 });
+
+test("buildCommentEvent falls back to the video event when no definition address is provided", () => {
+  const event = buildCommentEvent({
+    pubkey: "commenter",
+    created_at: 1700000400,
+    videoEventId: "legacy-event", 
+    videoEventRelay: "wss://legacy.example", 
+    threadParticipantPubkey: "legacy-pubkey",
+    content: "Legacy thread support",
+  });
+
+  assert.equal(event.kind, VIDEO_COMMENT_KIND);
+
+  const eventTags = event.tags.filter((tag) => Array.isArray(tag) && tag[0] === "e");
+  assert.deepEqual(eventTags, [["e", "legacy-event", "wss://legacy.example"]]);
+
+  const participantTags = event.tags.filter((tag) => Array.isArray(tag) && tag[0] === "p");
+  assert.deepEqual(participantTags, [["p", "legacy-pubkey"]]);
+});

tests/comment-thread-service.test.mjs

@@ -205,6 +205,72 @@ test(
   },
 );
 
+test(
+  "CommentThreadService loadThread falls back to event id when address is missing",
+  async () => {
+    const video = {
+      ...createBaseVideo(),
+      tags: [["alt", "ignored"]],
+    };
+
+    const fetchTargets = [];
+    const fetchOptions = [];
+    const subscribeTargets = [];
+    const subscribeOptions = [];
+    const errors = [];
+
+    const service = new CommentThreadService({
+      fetchVideoComments: async (target, options) => {
+        fetchTargets.push(target);
+        fetchOptions.push(options);
+        return [];
+      },
+      subscribeVideoComments: (target, options = {}) => {
+        subscribeTargets.push(target);
+        subscribeOptions.push(options);
+        return () => {};
+      },
+    });
+
+    service.setCallbacks({
+      onError: (error) => errors.push(error),
+    });
+
+    const snapshot = await service.loadThread({ video });
+
+    assert.equal(fetchTargets.length, 1, "fetch should run even without an address");
+    assert.deepStrictEqual(fetchTargets[0], {
+      videoEventId: "video123",
+      parentCommentId: "",
+    });
+    assert.equal(
+      "videoDefinitionAddress" in fetchTargets[0],
+      false,
+      "videoDefinitionAddress should be omitted when unavailable",
+    );
+
+    assert.deepStrictEqual(fetchOptions[0], {
+      limit: service.defaultLimit,
+      relays: null,
+    });
+
+    assert.equal(
+      subscribeTargets.length,
+      1,
+      "subscribe should run even without an address",
+    );
+    assert.deepStrictEqual(subscribeTargets[0], fetchTargets[0]);
+    assert.equal(
+      typeof subscribeOptions[0]?.onEvent,
+      "function",
+      "subscription handler should be provided",
+    );
+
+    assert.equal(errors.length, 0, "missing address should not emit errors");
+    assert.equal(snapshot.videoEventId, "video123");
+  },
+);
+
 test(
   "CommentThreadService teardown cancels hydration timers",
   async () => {

tests/nostr/comment-events.test.mjs

@@ -186,6 +186,50 @@ test("publishComment falls back to session signer when active signer is missing"
   assert.equal(publishCalls.length, 1, "publish should still occur once");
 });
 
+test("publishComment accepts legacy targets with only an event id", async () => {
+  const { client, publishCalls } = createMockClient();
+
+  let signCalls = 0;
+  const signEventWithPrivateKey = (event) => {
+    signCalls += 1;
+    return { ...event, id: `signed-${signCalls}` };
+  };
+
+  const result = await publishComment(
+    client,
+    {
+      videoEventId: "legacy-event-id",
+      parentCommentId: "parent-legacy",
+    },
+    { content: "Legacy comment" },
+    {
+      resolveActiveSigner: () => null,
+      shouldRequestExtensionPermissions: () => false,
+      signEventWithPrivateKey,
+      DEFAULT_NIP07_PERMISSION_METHODS: [],
+    },
+  );
+
+  assert.equal(result.ok, true, "publishing should succeed without a definition address");
+  assert.equal(signCalls, 1, "session signer should be used to sign the event");
+  assert.equal(publishCalls.length, 1, "event should be published exactly once");
+
+  const publishedEvent = publishCalls[0]?.event;
+  const tags = buildCommentEventTags(publishedEvent);
+  const eTags = tags.filter((tag) => Array.isArray(tag) && tag[0] === "e");
+  const aTags = tags.filter((tag) => Array.isArray(tag) && tag[0] === "a");
+
+  assert.deepEqual(
+    eTags,
+    [
+      ["e", "legacy-event-id"],
+      ["e", "parent-legacy"],
+    ],
+    "legacy publish should target the video event id and parent comment",
+  );
+  assert.deepEqual(aTags, [], "no address tag should be emitted when unavailable");
+});
+
 test("listVideoComments builds filters that prefer #a pointers", async () => {
   const matchingEventLatest = {
     id: "comment-2",
@@ -279,6 +323,45 @@ test("listVideoComments builds filters that prefer #a pointers", async () => {
   assert.equal(legacyFilter.limit, 10, "limit option should propagate to legacy filter");
 });
 
+test("listVideoComments supports legacy targets without a definition address", async () => {
+  const legacyEvent = {
+    id: "legacy-comment",
+    kind: COMMENT_EVENT_KIND,
+    tags: [["e", "legacy-video"], ["e", "legacy-parent"]],
+  };
+
+  const { client, pool } = createMockClient();
+
+  let receivedFilters = null;
+  pool.list = async (relays, filters) => {
+    receivedFilters = { relays, filters };
+    return [[legacyEvent]];
+  };
+
+  const events = await listVideoComments(client, { videoEventId: "legacy-video" });
+
+  assert.ok(Array.isArray(events), "listing should still return an array");
+  assert.equal(events.length, 1, "legacy matching comment should be returned");
+  assert.equal(events[0].id, "legacy-comment", "returned event should match the pool payload");
+
+  assert.ok(receivedFilters, "pool.list should be invoked with filters");
+  assert.deepEqual(
+    receivedFilters.relays,
+    client.relays,
+    "legacy listing should fall back to client relays when none are provided",
+  );
+  assert.deepEqual(
+    receivedFilters.filters,
+    [
+      {
+        kinds: [COMMENT_EVENT_KIND],
+        "#e": ["legacy-video"],
+      },
+    ],
+    "legacy listing should target the video event id via #e",
+  );
+});
+
 test("subscribeVideoComments forwards matching events and cleans up unsubscribe", async () => {
   const {
     client,

tests/video-modal-comments.test.mjs

@@ -2,6 +2,7 @@ import test from "node:test";
 import assert from "node:assert/strict";
 
 import { setupModal } from "./video-modal-accessibility.test.mjs";
+import VideoModalCommentController from "../js/ui/videoModalCommentController.js";
 
 const FOCUSABLE_SELECTOR = [
   "a[href]",
@@ -174,6 +175,101 @@ test("VideoModal comment composer updates messaging and dispatches events", asyn
   assert.strictEqual(submitDetail.triggerElement, modal.commentsSubmitButton);
 });
 
+test(
+  "VideoModalCommentController publishes comment using event id fallback",
+  async () => {
+    const publishCalls = [];
+    const statusMessages = [];
+    let resetCalled = false;
+    const composerStates = [];
+    const appendedEvents = [];
+
+    const videoModal = {
+      setCommentComposerState: (state) => {
+        composerStates.push({ ...state });
+      },
+      resetCommentComposer: () => {
+        resetCalled = true;
+      },
+      setCommentStatus: (message) => {
+        statusMessages.push(message);
+      },
+      appendComment: (event) => {
+        appendedEvents.push(event);
+      },
+    };
+
+    const processedEvents = [];
+
+    const controller = new VideoModalCommentController({
+      commentThreadService: {
+        setCallbacks: () => {},
+        processIncomingEvent: (event) => {
+          processedEvents.push(event);
+        },
+      },
+      videoModal,
+      auth: {
+        isLoggedIn: () => true,
+      },
+      services: {
+        publishComment: async (payload, eventData) => {
+          publishCalls.push([payload, eventData]);
+          return {
+            ok: true,
+            event: {
+              id: "legacy-comment",
+              pubkey: "pk1",
+              content: eventData.content,
+              tags: [],
+            },
+          };
+        },
+      },
+      utils: {
+        normalizeHexPubkey: (value) => value?.toLowerCase?.() || value,
+      },
+    });
+
+    controller.currentVideo = {
+      id: "legacy-video",
+      pubkey: "AUTHORPK",
+      enableComments: true,
+      kind: 30078,
+      tags: [],
+    };
+    controller.modalCommentState.videoEventId = "legacy-video";
+
+    await controller.handleVideoModalCommentSubmit({ text: "Legacy support" });
+
+    assert.equal(publishCalls.length, 1, "publishComment should be invoked");
+    const [payload, eventData] = publishCalls[0];
+    assert.deepStrictEqual(payload, {
+      videoEventId: "legacy-video",
+      parentCommentId: null,
+    });
+    assert.equal(
+      "videoDefinitionAddress" in payload,
+      false,
+      "payload should omit videoDefinitionAddress when unavailable",
+    );
+    assert.equal(eventData.content, "Legacy support");
+
+    assert.equal(processedEvents.length, 1, "event should be processed optimistically");
+    assert.equal(resetCalled, true, "composer should reset after publishing");
+    assert.ok(
+      statusMessages.includes("Comment posted."),
+      "success message should be surfaced",
+    );
+    assert.equal(
+      composerStates[composerStates.length - 1]?.disabled,
+      false,
+      "composer should be re-enabled after publishing",
+    );
+    assert.equal(appendedEvents.length, 0, "optimistic insert should avoid duplicate append");
+  },
+);
+
 test("VideoModal comment section exposes aria landmarks and participates in focus trap", async (t) => {
   const { window, document, modal, playerModal, trigger, cleanup } = await setupModal();
   t.after(cleanup);