diff --git a/e2e/specs/social.spec.ts b/e2e/specs/social.spec.ts index ca3a59958..0436085a2 100644 --- a/e2e/specs/social.spec.ts +++ b/e2e/specs/social.spec.ts @@ -301,16 +301,18 @@ flow("starring and unstarring a repo") .sees(testid("star", { starred: "false" })) .sees(starCount, "0"); -// bobbin answers 404 when there is no subscription -const subscribed = (subject: (ctx: Ctx) => string, yes: boolean) => async (ctx: Ctx) => { - const { status, body } = await get( - xrpc(config.bobbinUrl, "org.tangled.feed.subscription.getForActor", { - actor: ctx.actor.did, - subject: subject(ctx) - }) - ); - expect(status, `getForActor answered ${status} ${JSON.stringify(body)}`).toBe(yes ? 200 : 404); -}; +const subscribed = + (subject: (ctx: Ctx) => string, state: boolean | "muted") => async (ctx: Ctx) => { + const { status, body } = await get( + xrpc(config.bobbinUrl, "org.tangled.feed.subscription.getForActor", { + actor: ctx.actor.did, + subject: subject(ctx) + }) + ); + const answered = `getForActor answered ${status} ${JSON.stringify(body)}`; + expect(status, answered).toBe(state === false ? 404 : 200); + if (state !== false) expect(body.ignore === true, answered).toBe(state === "muted"); + }; flow("watching a repo and giving it up again") .actor("owner") @@ -376,11 +378,17 @@ flow("subscribing to an issue and unsubscribing again") .reload() .sees(testid("toggleSubscription", { subscribed: "true" })) + // muting puts ignore: true, deleting the record would bring the default notices back .click(testid("toggleSubscription", { subscribed: "true" })) - .sends(subscriptionId, pds, (ctx) => ({ rkey: rkeyOf(ctx, subscriptionId) }), "delete") + .sends( + subscriptionId, + pds, + (ctx) => ({ rkey: rkeyOf(ctx, subscriptionId), record: { ignore: true } }), + "put" + ) .sees(testid("toggleSubscription", { subscribed: "false", pending: "true" })) .trace(subscriptionId, through(pds, hydrant, bobbin)) - .step("bobbin no longer lists the subscription", subscribed(issueUri, false)) + .step("bobbin has the subscription as a mute", subscribed(issueUri, "muted")) .sees(testid("toggleSubscription", { subscribed: "false" })) .reload() .sees(testid("toggleSubscription", { subscribed: "false" })); diff --git a/web/src/lib/api/graph.test.ts b/web/src/lib/api/graph.test.ts index 61a3dd241..9d395e565 100644 --- a/web/src/lib/api/graph.test.ts +++ b/web/src/lib/api/graph.test.ts @@ -2,9 +2,16 @@ import { is } from "@atcute/lexicons/validations"; import type { OAuthUserAgent } from "@atcute/oauth-browser-client"; import { describe, expect, it, vi } from "vitest"; import { createBobbinClient } from "#lib/api/client.ts"; -import { createEntitySubscription, listNetworkVouches } from "#lib/api/graph.ts"; +import { + createEntitySubscription, + ENTITY_SUBSCRIBED, + ensureEntitySubscription, + getEntitySubscription, + listNetworkVouches, + setEntitySubscribed +} from "#lib/api/graph.ts"; import { mainSchema as subscriptionSchema } from "#lib/api/lexicons/types/org/tangled/feed/subscription.ts"; -import { createRecord } from "#lib/api/write.ts"; +import { createRecord, putRecord } from "#lib/api/write.ts"; vi.mock("#lib/api/write.ts", () => ({ createRecord: vi.fn(async () => ({ @@ -12,7 +19,10 @@ vi.mock("#lib/api/write.ts", () => ({ cid: "bafytest" })), deleteRecord: vi.fn(), - putRecord: vi.fn() + putRecord: vi.fn(async () => ({ + uri: "at://did:plc:alice/org.tangled.feed.subscription/3l7abc", + cid: "bafytest" + })) })); const response = (items: unknown[] = []) => @@ -65,4 +75,109 @@ describe("entity subscriptions", () => { const [, , record] = vi.mocked(createRecord).mock.calls[0]; expect(is(subscriptionSchema, record)).toBe(true); }); + + const ticket = "at://did:plc:bob/org.tangled.track.ticket/3l7xyz"; + const agent = { sub: "did:plc:alice" } as unknown as OAuthUserAgent; + const bobbinWith = (found: { ignore?: boolean } | null) => + createBobbinClient({ + serviceUrl: "https://bobbin.test", + fetch: async () => + found + ? Response.json({ + uri: "at://did:plc:alice/org.tangled.feed.subscription/3lsub", + ...found + }) + : new Response(null, { status: 404 }) + }); + + it("reads the rkey and mute flag, and null when there is no record", async () => { + expect(await getEntitySubscription(bobbinWith(null), "did:plc:alice", ticket)).toBeNull(); + expect(await getEntitySubscription(bobbinWith({}), "did:plc:alice", ticket)).toEqual({ + rkey: "3lsub", + ignore: false + }); + expect( + await getEntitySubscription(bobbinWith({ ignore: true }), "did:plc:alice", ticket) + ).toEqual({ rkey: "3lsub", ignore: true }); + }); + + it("mutes by putting ignore on the existing record, never deleting", async () => { + await setEntitySubscribed( + agent, + bobbinWith({ ignore: false }), + "did:plc:alice", + ticket, + false + ); + const [, , rkey, record] = vi.mocked(putRecord).mock.calls.at(-1)!; + expect(rkey).toBe("3lsub"); + expect(record).toMatchObject({ ignore: true }); + expect(is(subscriptionSchema, record)).toBe(true); + }); + + it("creates a mute when there is no record yet", async () => { + vi.mocked(createRecord).mockClear(); + await setEntitySubscribed(agent, bobbinWith(null), "did:plc:alice", ticket, false); + const [, , record] = vi.mocked(createRecord).mock.calls[0]; + expect(record).toMatchObject({ ignore: true }); + }); + + it("unmutes by dropping ignore from the record", async () => { + await setEntitySubscribed( + agent, + bobbinWith({ ignore: true }), + "did:plc:alice", + ticket, + true + ); + const [, , rkey, record] = vi.mocked(putRecord).mock.calls.at(-1)!; + expect(rkey).toBe("3lsub"); + expect(record).not.toHaveProperty("ignore"); + }); + + it("commenting unmutes a muted thread but leaves a live subscription alone", async () => { + vi.mocked(putRecord).mockClear(); + await ensureEntitySubscription( + agent, + bobbinWith({ ignore: false }), + "did:plc:alice", + ticket + ); + expect(putRecord).not.toHaveBeenCalled(); + await ensureEntitySubscription( + agent, + bobbinWith({ ignore: true }), + "did:plc:alice", + ticket + ); + expect(vi.mocked(putRecord).mock.calls[0][3]).not.toHaveProperty("ignore"); + }); + + it("tells the page when a comment unmutes the thread, and not when nothing changed", async () => { + const target = new EventTarget(); + const seen: unknown[] = []; + target.addEventListener(ENTITY_SUBSCRIBED, (event) => + seen.push((event as CustomEvent).detail) + ); + vi.stubGlobal("window", target); + try { + await ensureEntitySubscription( + agent, + bobbinWith({ ignore: true }), + "did:plc:alice", + ticket + ); + expect(seen).toEqual([{ entityUri: ticket, rkey: "3lsub" }]); + seen.length = 0; + await ensureEntitySubscription( + agent, + bobbinWith({ ignore: false }), + "did:plc:alice", + ticket + ); + expect(seen).toEqual([]); + } finally { + vi.unstubAllGlobals(); + } + }); }); diff --git a/web/src/lib/api/graph.ts b/web/src/lib/api/graph.ts index 854a81cfc..a35fa3603 100644 --- a/web/src/lib/api/graph.ts +++ b/web/src/lib/api/graph.ts @@ -142,47 +142,94 @@ export const deleteSubscription = async ( return { uri }; }; -export const createEntitySubscription = async ( +// bobbin sends a mute back as a record too, same rkey with ignore set +export interface EntitySubscription { + rkey: string; + ignore: boolean; +} + +// no record means deliberi still notifies, so a mute is ignore: true, not a delete +const writeEntitySubscription = async ( agent: OAuthUserAgent, - entityUri: string + entityUri: string, + ignore: boolean, + existingRkey?: string ): Promise => { - const { uri } = await createRecord(agent, SUBSCRIPTION_COLLECTION, { + const record = { $type: SUBSCRIPTION_COLLECTION, subject: { - $type: "org.tangled.feed.subscription#record", + $type: "org.tangled.feed.subscription#record" as const, uri: entityUri }, + ...(ignore ? { ignore } : {}), createdAt: new Date().toISOString() - }); + }; + if (existingRkey) { + await putRecord(agent, SUBSCRIPTION_COLLECTION, recordKeyOf(existingRkey), record); + return existingRkey; + } + const { uri } = await createRecord(agent, SUBSCRIPTION_COLLECTION, record); return rkeyFromUri(uri); }; -export const getSubscriptionRkey = async ( +export const createEntitySubscription = (agent: OAuthUserAgent, entityUri: string) => + writeEntitySubscription(agent, entityUri, false); + +export const getEntitySubscription = async ( ctx: BobbinContext, actor: string, subject: string -): Promise => { +): Promise => { try { - const { uri } = await jsonGet<{ uri: string }>( + const { uri, ignore } = await jsonGet<{ uri: string; ignore?: boolean }>( ctx, "org.tangled.feed.subscription.getForActor", { actor: actor as Did, subject } ); - return rkeyFromUri(uri); + return { rkey: rkeyFromUri(uri), ignore: ignore === true }; } catch (cause) { if (cause instanceof ClientResponseError && httpStatusFor(cause) === 404) return null; throw cause; } }; +export const getSubscriptionRkey = async ( + ctx: BobbinContext, + actor: string, + subject: string +): Promise => (await getEntitySubscription(ctx, actor, subject))?.rkey ?? null; + +// a stale button would stack a second record, so reread the rkey first +export const setEntitySubscribed = async ( + agent: OAuthUserAgent, + ctx: BobbinContext, + actor: string, + entityUri: string, + subscribed: boolean +): Promise => { + const existing = await getEntitySubscription(ctx, actor, entityUri); + return writeEntitySubscription(agent, entityUri, !subscribed, existing?.rkey); +}; + +export const ENTITY_SUBSCRIBED = "entity-subscribed"; + +// the page's subscribe button can't see a comment subscribing, so it gets this event +const announceSubscribed = (entityUri: string, rkey: string) => { + if (typeof window === "undefined") return; + window.dispatchEvent(new CustomEvent(ENTITY_SUBSCRIBED, { detail: { entityUri, rkey } })); +}; + export const ensureEntitySubscription = async ( agent: OAuthUserAgent, ctx: BobbinContext, actor: string, entityUri: string ): Promise => { - const existing = await getSubscriptionRkey(ctx, actor, entityUri); - return existing ?? (await createEntitySubscription(agent, entityUri)); + const existing = await getEntitySubscription(ctx, actor, entityUri); + if (existing && !existing.ignore) return existing.rkey; + const rkey = await writeEntitySubscription(agent, entityUri, false, existing?.rkey); + announceSubscribed(entityUri, rkey); + return rkey; }; export type VouchKind = "vouch" | "denounce"; diff --git a/web/src/lib/components/repo/EntitySubscribeButton.stories.svelte b/web/src/lib/components/repo/EntitySubscribeButton.stories.svelte index 2a45c104d..4efad83d5 100644 --- a/web/src/lib/components/repo/EntitySubscribeButton.stories.svelte +++ b/web/src/lib/components/repo/EntitySubscribeButton.stories.svelte @@ -9,7 +9,7 @@ const SUB_URI = "at://did:plc:alice/org.tangled.feed.subscription/3lksub"; const ENTITY = "at://did:web:knot.test:repo/org.tangled.track.ticket/3labc"; - type FetchMode = "loading" | "not-subscribed" | "subscribed" | "error"; + type FetchMode = "loading" | "not-subscribed" | "subscribed" | "muted" | "error"; // the subscription lookup goes through window.fetch (via createBobbinClient), so each // story swaps in a fetch mock that answers getForActor differently. subscribing looks @@ -25,7 +25,9 @@ case "not-subscribed": return new Response(null, { status: 404 }); case "subscribed": - return Response.json({ uri: SUB_URI }); + return Response.json({ uri: SUB_URI, ignore: false }); + case "muted": + return Response.json({ uri: SUB_URI, ignore: true }); case "error": return Response.json( { error: "InternalError", message: "bobbin is down" }, @@ -51,8 +53,11 @@ cid: "bafyreibfko5xzzdptcimcaql4tmsaffgr6jhreyjz3tv6nlh34si6k4u7e" }); } - if (pathname.includes("deleteRecord")) { - return Response.json({}); + if (pathname.includes("putRecord")) { + return Response.json({ + uri: SUB_URI, + cid: "bafyreibfko5xzzdptcimcaql4tmsaffgr6jhreyjz3tv6nlh34si6k4u7e" + }); } return new Response(null, { status: 404 }); }; @@ -133,6 +138,15 @@ + + + + @@ -148,6 +162,15 @@ parameters={{ auth: { respond }, fetchMode: "subscribed" }} /> + + + + { const did = auth.currentDid; if (!bobbinUrl || !did) { - subscribed = false; + subscribed = notifiedByDefault; rkey = null; known = true; return; @@ -41,11 +43,11 @@ let cancelled = false; known = false; error = null; - getSubscriptionRkey(createBobbinClient({ serviceUrl: bobbinUrl }), did, entityUri) + getEntitySubscription(createBobbinClient({ serviceUrl: bobbinUrl }), did, entityUri) .then((found) => { if (cancelled) return; - rkey = found; - subscribed = !!found; + rkey = found?.rkey ?? null; + subscribed = found ? !found.ignore : notifiedByDefault; known = true; }) .catch(() => { @@ -59,6 +61,18 @@ }; }); + // a subscribe or unmute from elsewhere on the page comes in as this event + $effect(() => { + const onSubscribed = (event: Event) => { + const detail = (event as CustomEvent<{ entityUri: string; rkey: string }>).detail; + if (detail.entityUri !== entityUri) return; + subscribed = true; + rkey = detail.rkey; + }; + window.addEventListener(ENTITY_SUBSCRIBED, onSubscribed); + return () => window.removeEventListener(ENTITY_SUBSCRIBED, onSubscribed); + }); + $effect(() => { if (!error) return; const timer = setTimeout(() => { @@ -67,7 +81,7 @@ return () => clearTimeout(timer); }); - // flips first so the faces swap on click, and flips back if the write fails + // unsubscribing sets ignore on the record, it never deletes it const toggle = async () => { const agent = auth.agent; const did = auth.currentDid; @@ -78,13 +92,8 @@ toggling = true; subscribed = !wasSubscribed; try { - if (wasSubscribed && wasRkey) { - await deleteSubscription(agent, wasRkey); - rkey = null; - } else { - const ctx = createBobbinClient({ serviceUrl: bobbinUrl }); - rkey = await ensureEntitySubscription(agent, ctx, did, entityUri); - } + const ctx = createBobbinClient({ serviceUrl: bobbinUrl }); + rkey = await setEntitySubscribed(agent, ctx, did, entityUri, !wasSubscribed); } catch { subscribed = wasSubscribed; rkey = wasRkey; diff --git a/web/src/lib/components/repo/issues/IssueThreadView.svelte b/web/src/lib/components/repo/issues/IssueThreadView.svelte index 5550160fc..e378baf99 100644 --- a/web/src/lib/components/repo/issues/IssueThreadView.svelte +++ b/web/src/lib/components/repo/issues/IssueThreadView.svelte @@ -95,6 +95,10 @@ setMentionPeople(() => participants); const entityUri = $derived(issue.uri); + const notifiedByDefault = $derived( + !!auth.currentDid && + (auth.currentDid === issue.authorDid || auth.currentDid === props.repo.ownerDid) + ); $effect(() => { void notifications.markEntityVisited(entityUri); @@ -209,7 +213,12 @@ {/snippet} {#snippet subscribeItem()} - + {/snippet} {#snippet trail()} @@ -280,7 +289,12 @@ > Actions - + {@render stateChange(true)} {/if} diff --git a/web/src/routes/[handle]/[repo]/pulls/[rkey]/[version]/[[range]]/PullViewPage.svelte b/web/src/routes/[handle]/[repo]/pulls/[rkey]/[version]/[[range]]/PullViewPage.svelte index 3f39aaad9..71cf42f8b 100644 --- a/web/src/routes/[handle]/[repo]/pulls/[rkey]/[version]/[[range]]/PullViewPage.svelte +++ b/web/src/routes/[handle]/[repo]/pulls/[rkey]/[version]/[[range]]/PullViewPage.svelte @@ -103,6 +103,7 @@ let view = $derived(data.view); const isAuthor = $derived(auth.currentDid === pull.author.did); + const notifiedByDefault = $derived(isAuthor || auth.currentDid === data.repo.ownerDid); const isForkBased = $derived(!!pull.source && pull.source.repo !== data.repo.repoDid); const canPush = $derived((await data.viewerAccess).canPush); @@ -685,6 +686,7 @@ {#snippet subscribeItem()}