diff --git a/apps/web/src/app/decks/_components/columns/add-column/deck-add-column-notifications-settings.tsx b/apps/web/src/app/decks/_components/columns/add-column/deck-add-column-notifications-settings.tsx index 960d249c0b..94812deab7 100644 --- a/apps/web/src/app/decks/_components/columns/add-column/deck-add-column-notifications-settings.tsx +++ b/apps/web/src/app/decks/_components/columns/add-column/deck-add-column-notifications-settings.tsx @@ -1,22 +1,45 @@ -import React, { useContext, useState } from "react"; +import React, { useContext, useEffect, useState } from "react"; import { DeckGridContext } from "../../deck-manager"; import { DeckAddColumnSearchBox } from "./deck-add-column-search-box"; import { SettingsProps, UsernameDataItem } from "./common"; -import { ICONS, NOTIFICATION_CONTENT_TYPES } from "../../consts"; +import { ICONS, notificationContentTypesFor } from "../../consts"; import useLocalStorage from "react-use/lib/useLocalStorage"; import { Button } from "@ui/button"; import i18next from "i18next"; import { UserAvatar } from "@/features/shared"; import { PREFIX } from "@/utils/local-storage"; +import { useActiveAccount } from "@/core/hooks/use-active-account"; export const DeckAddColumnNotificationsSettings = ({ deckKey }: SettingsProps) => { const { add } = useContext(DeckGridContext); + const { activeUser } = useActiveAccount(); const [username, setUsername] = useState(""); const [tag, setTag] = useState(""); const [contentType, setContentType] = useState(null); const [recent, setRecent] = useLocalStorage(PREFIX + "_dnr", []); + // Favourites, bookmarks and scheduled posts are Ecency-only data and are served only to + // the account they belong to, so offering them for someone else would build a column + // that can never load. + const contentTypes = notificationContentTypesFor(username, activeUser?.username); + + // Picking a self-only type and then changing the username to someone else would + // otherwise leave that choice selected and let the column be created anyway. + // + // Skipped while the username is empty: clearing the field to re-pick an account passes + // through "" on the way, and treating that as a switch would drop a valid selection + // even when the same account is chosen again. + useEffect(() => { + if (!username) { + return; + } + + if (contentType && !contentTypes.some(({ type }) => type === contentType)) { + setContentType(null); + } + }, [username, contentTypes, contentType]); + return (
{i18next.t("decks.columns.add-username-text")}
@@ -51,7 +74,7 @@ export const DeckAddColumnNotificationsSettings = ({ deckKey }: SettingsProps) = <>
{i18next.t("decks.filters")}
- {NOTIFICATION_CONTENT_TYPES.map(({ title, type }) => ( + {contentTypes.map(({ title, type }) => (
svg]:size-8 " + (contentType === type ? "selected" : "")} key={title} diff --git a/apps/web/src/app/decks/_components/columns/deck-notifications-column.tsx b/apps/web/src/app/decks/_components/columns/deck-notifications-column.tsx index 697e753351..0d5048348b 100644 --- a/apps/web/src/app/decks/_components/columns/deck-notifications-column.tsx +++ b/apps/web/src/app/decks/_components/columns/deck-notifications-column.tsx @@ -1,9 +1,14 @@ -import React, { useCallback, useContext, useEffect, useState } from "react"; +import React, { useCallback, useContext, useEffect, useMemo, useState } from "react"; import { ShortListItemSkeleton } from "./deck-items"; import { GenericDeckWithDataColumn } from "./generic-deck-with-data-column"; import { UserDeckGridItem } from "../types"; import { DraggableProvidedDragHandleProps } from "@hello-pangea/dnd"; -import { NOTIFICATION_CONTENT_TYPES, notificationsTitles } from "../consts"; +import { + effectiveNotificationContentType, + notificationContentTypesFor, + notificationsTitles, + shouldPersistContentTypeCorrection +} from "../consts"; import { DeckGridContext } from "../deck-manager"; import { DeckPostViewer } from "./content-viewer"; import { DeckLoginOverlayPlaceholder } from "./deck-login-overlay-placeholder"; @@ -39,20 +44,59 @@ export const DeckNotificationsColumn = ({ id, settings, draggable }: Props) => { const [isFirstLoaded, setIsFirstLoaded] = useState(false); const [hasNextPage, setHasNextPage] = useState(true); - const { updateColumnIntervalMs } = useContext(DeckGridContext); + const { updateColumnIntervalMs, updateColumnSpecificSettings } = useContext(DeckGridContext); const prevSettings = usePrevious(settings); + const allowedContentTypes = useMemo( + () => notificationContentTypesFor(settings.username, activeUser?.username), + [settings.username, activeUser?.username] + ); + + // Used for fetching straight away, before the correction below has been persisted. + const effectiveContentType = useMemo( + () => + effectiveNotificationContentType( + settings.contentType, + settings.username, + activeUser?.username + ), + [settings.contentType, settings.username, activeUser?.username] + ); + + // Persist the correction so the stored value, the header subtitle and the selector all + // agree, and so it survives a reload. Guarded on the active account being KNOWN: the + // store starts empty and ClientInit restores the user after mount, so writing during + // that first render would erase a valid self-only filter on an ordinary reload. + useEffect(() => { + if ( + shouldPersistContentTypeCorrection( + settings.contentType, + settings.username, + activeUser?.username + ) + ) { + updateColumnSpecificSettings(id, { contentType: effectiveContentType }); + } + }, [ + settings.contentType, + settings.username, + activeUser?.username, + effectiveContentType, + id, + updateColumnSpecificSettings + ]); + const fetchData = useCallback( async (since?: ApiNotification) => { if (data.length) { setIsReloading(true); } - const isAll = settings.contentType === "all"; + const isAll = effectiveContentType === "all"; try { const response = await getNotifications( getAccessToken(activeUser!.username), - isAll ? null : (settings.contentType as NotificationFilter), + isAll ? null : (effectiveContentType as NotificationFilter), since?.id, settings.username ); @@ -72,7 +116,7 @@ export const DeckNotificationsColumn = ({ id, settings, draggable }: Props) => { setIsFirstLoaded(true); } }, - [activeUser, data, settings.contentType, settings.username] + [activeUser, data, effectiveContentType, settings.username] ); useEffect(() => { @@ -106,7 +150,7 @@ export const DeckNotificationsColumn = ({ id, settings, draggable }: Props) => { setUpdateIntervalMs: (v) => updateColumnIntervalMs(id, v), additionalSettings: ( diff --git a/apps/web/src/app/decks/_components/consts/content-types.ts b/apps/web/src/app/decks/_components/consts/content-types.ts index cc0d10c441..79685ec466 100644 --- a/apps/web/src/app/decks/_components/consts/content-types.ts +++ b/apps/web/src/app/decks/_components/consts/content-types.ts @@ -97,3 +97,90 @@ export const NOTIFICATION_CONTENT_TYPES = [ type: "scheduled_published" } ]; + +/** + * Notification content types that are Ecency-only rather than chain-derived, so they are + * served solely to the account they belong to. + * + * nfavorites and nbookmarks reveal who a user has favorited and what they saved, and + * scheduled_published is Ecency scheduling metadata. vision-api downgrades a request for + * another account's notifications to scope=public, which makes enotify withhold these, so + * a column built for someone else with one of these types would render permanently empty. + * They are hidden in the picker instead. + * + * `all` and `transfers` stay available: they still return their chain-derived subset. + */ +export const SELF_ONLY_NOTIFICATION_CONTENT_TYPES = [ + "nfavorites", + "nbookmarks", + "scheduled_published" +]; + +/** + * The notification content types offered for a given column target. + * + * Used by BOTH the add-column picker and the settings of an existing column. Filtering + * only the picker left the settings able to switch a cross-account column onto a + * restricted type, which recreated the empty-column problem the filtering exists to + * prevent. + */ +export function notificationContentTypesFor( + targetUsername: string | undefined, + activeUsername: string | undefined +) { + const isSelf = + !!activeUsername && + !!targetUsername && + targetUsername.toLowerCase() === activeUsername.toLowerCase(); + + return isSelf + ? NOTIFICATION_CONTENT_TYPES + : NOTIFICATION_CONTENT_TYPES.filter( + ({ type }) => !SELF_ONLY_NOTIFICATION_CONTENT_TYPES.includes(type) + ); +} + +/** + * The content type a column should actually use, falling back to "all" when the stored + * one is not available for its target. + * + * A column persists its contentType, so one created before its target became + * cross-account, or created while signed in as a different account, can still hold a + * self-only type. Filtering the selector does not change a stored value, so without this + * the column keeps fetching a filter that returns nothing and sits permanently empty + * with its own current value missing from the selector. + */ +export function effectiveNotificationContentType( + contentType: string, + targetUsername: string | undefined, + activeUsername: string | undefined +) { + const allowed = notificationContentTypesFor(targetUsername, activeUsername); + return allowed.some(({ type }) => type === contentType) ? contentType : "all"; +} + +/** + * Whether a column's stored contentType should be corrected on disk. + * + * Separate from effectiveNotificationContentType because the two answer different + * questions. What to FETCH is safe to decide immediately, and "all" is the right + * temporary answer while signed out. What to PERSIST is not: the global store starts + * with no active user and ClientInit restores it after mount, so during that first + * render every column looks cross-account. Writing then would erase a valid self-only + * filter on an ordinary page reload. + * + * So: never persist until the active account is known. + */ +export function shouldPersistContentTypeCorrection( + contentType: string, + targetUsername: string | undefined, + activeUsername: string | undefined +) { + if (!activeUsername) { + return false; + } + + return ( + effectiveNotificationContentType(contentType, targetUsername, activeUsername) !== contentType + ); +} diff --git a/apps/web/src/specs/app/decks/notification-content-types.spec.ts b/apps/web/src/specs/app/decks/notification-content-types.spec.ts new file mode 100644 index 0000000000..bf5ff7f72c --- /dev/null +++ b/apps/web/src/specs/app/decks/notification-content-types.spec.ts @@ -0,0 +1,174 @@ +import { describe, expect, it } from "vitest"; +import { + effectiveNotificationContentType, + shouldPersistContentTypeCorrection, + NOTIFICATION_CONTENT_TYPES, + notificationContentTypesFor, + SELF_ONLY_NOTIFICATION_CONTENT_TYPES +} from "@/app/decks/_components/consts"; + +/** + * A Decks notifications column can be built for any account, because notifications are + * largely public data. The exception is Ecency-only activity: vision-api downgrades a + * cross-account request to scope=public and enotify then withholds those types, so a + * column built for someone else with one of them renders permanently empty. + * + * This pins which types that is. It has to stay in step with + * PUBLIC_ACTIVITY_MAIN_TYPES in enotify (constants.py); the two live in different + * repositories, so drift here is silent and shows up only as an empty column. + */ +describe("Decks self-only notification content types", () => { + it("withholds exactly the Ecency-only types", () => { + expect([...SELF_ONLY_NOTIFICATION_CONTENT_TYPES].sort()).toEqual([ + "nbookmarks", + "nfavorites", + "scheduled_published" + ]); + }); + + it("keeps every chain-derived type available cross-account", () => { + // These map to main types enotify serves under scope=public. + for (const type of [ + "rvotes", + "mentions", + "follows", + "replies", + "reblogs", + "payouts", + "delegations" + ]) { + expect(SELF_ONLY_NOTIFICATION_CONTENT_TYPES).not.toContain(type); + } + }); + + it("keeps `all` and `transfers` available, since both still return a subset", () => { + // Neither is withheld wholesale: `all` returns the public types and `transfers` + // returns chain transfers while enotify drops the Ecency Points rows from it. + expect(SELF_ONLY_NOTIFICATION_CONTENT_TYPES).not.toContain("all"); + expect(SELF_ONLY_NOTIFICATION_CONTENT_TYPES).not.toContain("transfers"); + }); + + it("only names types the picker actually offers", () => { + // A stale entry here would silently protect nothing. + const offered = NOTIFICATION_CONTENT_TYPES.map(({ type }) => type); + const unknown = SELF_ONLY_NOTIFICATION_CONTENT_TYPES.filter((t) => !offered.includes(t)); + expect(unknown).toEqual([]); + }); +}); + +/** + * Both the add-column picker and an existing column's settings resolve their options + * through this, so a cross-account column cannot be switched onto a restricted type + * after the fact. + */ +describe("notificationContentTypesFor", () => { + const names = (list: { type: string }[]) => list.map(({ type }) => type); + + it("offers everything for your own account, case-insensitively", () => { + for (const target of ["good-karma", "Good-Karma", "GOOD-KARMA"]) { + expect(names(notificationContentTypesFor(target, "good-karma"))).toEqual( + names(NOTIFICATION_CONTENT_TYPES) + ); + } + }); + + it("withholds the self-only types for another account", () => { + const offered = names(notificationContentTypesFor("someone-else", "good-karma")); + + for (const type of SELF_ONLY_NOTIFICATION_CONTENT_TYPES) { + expect(offered).not.toContain(type); + } + expect(offered).toContain("all"); + expect(offered).toContain("transfers"); + }); + + it("withholds them when signed out or with no target chosen", () => { + // Neither an unknown viewer nor an unset target can be shown to be self. + for (const [target, active] of [ + ["someone", undefined], + [undefined, "good-karma"], + ["", "good-karma"], + [undefined, undefined] + ] as [string | undefined, string | undefined][]) { + const offered = names(notificationContentTypesFor(target, active)); + expect(offered).not.toContain("nfavorites"); + } + }); +}); + +/** + * Filtering the selector does not change an already-stored value. A column created before + * its target became cross-account, or created while signed in as another account, keeps + * its persisted contentType and would otherwise fetch a filter that returns nothing. + */ +describe("effectiveNotificationContentType", () => { + it("keeps a stored type that is still allowed", () => { + expect(effectiveNotificationContentType("rvotes", "someone-else", "good-karma")).toBe( + "rvotes" + ); + expect(effectiveNotificationContentType("nfavorites", "good-karma", "good-karma")).toBe( + "nfavorites" + ); + expect(effectiveNotificationContentType("all", "someone-else", "good-karma")).toBe("all"); + }); + + it("falls back to `all` for a stored self-only type on another account", () => { + for (const type of SELF_ONLY_NOTIFICATION_CONTENT_TYPES) { + expect(effectiveNotificationContentType(type, "someone-else", "good-karma")).toBe("all"); + } + }); + + it("falls back when the ACTIVE account changes, not just the target", () => { + // A self column holding nfavorites becomes cross-account the moment someone else + // signs in, which is the case a target-only check would miss. + expect(effectiveNotificationContentType("nfavorites", "good-karma", "good-karma")).toBe( + "nfavorites" + ); + expect(effectiveNotificationContentType("nfavorites", "good-karma", "someone-else")).toBe( + "all" + ); + // Signed out entirely. + expect(effectiveNotificationContentType("nfavorites", "good-karma", undefined)).toBe("all"); + }); + + it("falls back for a type that no longer exists at all", () => { + expect(effectiveNotificationContentType("retired-type", "good-karma", "good-karma")).toBe( + "all" + ); + }); +}); + +/** + * The lifecycle half. What to FETCH can be decided immediately, but what to PERSIST + * cannot: the global store starts with no active user and ClientInit restores it after + * mount, so during that first render every column looks cross-account. + */ +describe("shouldPersistContentTypeCorrection", () => { + it("never persists while the active account is unknown", () => { + // The regression: a locally stored self column using nfavorites would otherwise be + // rewritten to `all` on an ordinary reload, before the account was restored. + for (const type of SELF_ONLY_NOTIFICATION_CONTENT_TYPES) { + expect(shouldPersistContentTypeCorrection(type, "good-karma", undefined)).toBe(false); + expect(shouldPersistContentTypeCorrection(type, "good-karma", "")).toBe(false); + } + }); + + it("undefined then the owning username leaves the stored filter intact", () => { + // Explicitly the sequence a page reload performs. + expect(shouldPersistContentTypeCorrection("nfavorites", "good-karma", undefined)).toBe(false); + expect(shouldPersistContentTypeCorrection("nfavorites", "good-karma", "good-karma")).toBe( + false + ); + }); + + it("persists once a DIFFERENT account is known to be active", () => { + expect(shouldPersistContentTypeCorrection("nfavorites", "good-karma", "someone-else")).toBe( + true + ); + }); + + it("does not persist when the stored type is already allowed", () => { + expect(shouldPersistContentTypeCorrection("rvotes", "someone-else", "good-karma")).toBe(false); + expect(shouldPersistContentTypeCorrection("all", "someone-else", "good-karma")).toBe(false); + }); +});