From 4a9267ca3cd8763fc2e9283ab91f7f7f5ef15768 Mon Sep 17 00:00:00 2001 From: feruzm Date: Tue, 1 Sep 2026 15:58:30 +0000 Subject: [PATCH 1/4] fix(decks): hide self-only notification types when viewing another account Companion to ecency/enotify-py#21 and the vision-api scope change. A Decks notifications column can be built for any account, because notifications are largely public data and that is why the feature exists. That stays. The exception is Ecency-only activity. vision-api now downgrades a cross-account request to scope=public and enotify withholds those types, so a column built for someone else with favourites, bookmarks or scheduled posts would render permanently empty with no explanation. The picker no longer offers them once the chosen username is not the signed-in user. `all` and `transfers` stay available for any account: neither is withheld wholesale. `all` returns the public types, and enotify drops only the Ecency Points rows from `transfers` while chain and Hive-Engine transfers remain. Picking a self-only type and then changing the username to someone else would have left that choice selected and let the column be created anyway, so the selection is cleared when it stops being offered. The spec pins which types are withheld, that the chain-derived ones stay available, and that every name listed is one the picker actually offers, since a stale entry would silently protect nothing. It has to stay in step with PUBLIC_ACTIVITY_MAIN_TYPES in enotify: the two live in different repositories, so drift is silent and surfaces only as an empty column. typecheck clean, lint clean, 373 files / 3685 tests pass. --- apps/web/cand.txt | 29 ++++++++++ ...deck-add-column-notifications-settings.tsx | 30 +++++++++-- .../decks/_components/consts/content-types.ts | 18 +++++++ .../decks/notification-content-types.spec.ts | 54 +++++++++++++++++++ 4 files changed, 128 insertions(+), 3 deletions(-) create mode 100644 apps/web/cand.txt create mode 100644 apps/web/src/specs/app/decks/notification-content-types.spec.ts diff --git a/apps/web/cand.txt b/apps/web/cand.txt new file mode 100644 index 0000000000..a36324417f --- /dev/null +++ b/apps/web/cand.txt @@ -0,0 +1,29 @@ +/@anime.indo.net/harley-f84fae1031806 +/@aquagelas/i-once-wanted-to-be +/@awulonualex/what-a-mess +/@brianoflondon/anyone-can-start-receiving-value-4-value-streaming-sats-for-your-podcast +/@claudio83/first-accumulation-strategies-towards-the-next-set +/@culgin/why-are-banks-so-profitable +/@daily.prompt/19-january-2024-mariannewests-freewrite-writing-prompt-day-2256--flat +/@educ.leb/manchester-united-after-the-decisive-double-of-pogba-mourinho-sends-messages +/@elizabeths14/pesadillas-es-mejor-que-suene-contigo-esp-eng +/@gamarm/un-hada-real-en-las-selvas-del-amazonas-colombia +/@greengalletti/lincoln-rhyme-hunt-for-the-bone-collector-2020-is-a-tv-remake-based-on-the-book-by-jeffery-deaver +/@hrichakar/5e133036 +/@joey-fancy/umv-vl-2020-star-trek-first-contact-day +/@knotwork/gravitational-wave-mechanics-and-circuitry-is-means-so-can-do +/@krolestwo/glosowanie-ustawa-o-skladaniu-wnioskow +/@marry.ulhikmah/use-your-brain-in-your-life +/@midlet/collections-a-feature-suggestion-for-peakd +/@myeong/re-yeckingo1-r9e4u6 +/@mysticnamuha/projection-becomes-the-medium-or-the-lens-through-which-we-view-ourselves-and-the-world-from +/@nodzz/episode-4-or-genie +/@queroveromundo/rss +/@ramsayubolt/online-gaming-ad5b6148a3bfe +/@reysa/rss +/@ruffatotmeee/loh-215-plan-and-priorities-next-year-to-heal-a-trauma-and-to-bring-peace-in-my-head +/@stokjockey/lets-take-a-look-at-another-silver-art-bar-from-1973-from-the-mount-everest-mint-mintage-of-12-671 +/@tobetada/welcome-to-the-love-the-clouds-contest-172 +/@very86/review-tv-orange-is-the-new-black-spoiler-alert +/@vicnzia/parte-i-conoce-las-posturas-del-yoga-chakrasana-o-la-postura-de-la-rueda-part-i-know-the-yoga-postures-chakrasana-or-the-wh +/@zimri7/re-silentnightowl-2026419t181911386z 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..19e3f7bf63 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,46 @@ -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, + NOTIFICATION_CONTENT_TYPES, + SELF_ONLY_NOTIFICATION_CONTENT_TYPES +} 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. Hive names are lowercase, but compare defensively. + const isSelf = !!activeUser && username.toLowerCase() === activeUser.username.toLowerCase(); + const contentTypes = isSelf + ? NOTIFICATION_CONTENT_TYPES + : NOTIFICATION_CONTENT_TYPES.filter( + ({ type }) => !SELF_ONLY_NOTIFICATION_CONTENT_TYPES.includes(type) + ); + + // 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. + useEffect(() => { + if (contentType && !contentTypes.some(({ type }) => type === contentType)) { + setContentType(null); + } + }, [contentTypes, contentType]); + return (
{i18next.t("decks.columns.add-username-text")}
@@ -51,7 +75,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/consts/content-types.ts b/apps/web/src/app/decks/_components/consts/content-types.ts index cc0d10c441..05ee70f321 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,21 @@ 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" +]; 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..61a4cb23a9 --- /dev/null +++ b/apps/web/src/specs/app/decks/notification-content-types.spec.ts @@ -0,0 +1,54 @@ +import { describe, expect, it } from "vitest"; +import { + NOTIFICATION_CONTENT_TYPES, + 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([]); + }); +}); From ce2d914e72fa6f6eb5a12b40b949cfb87c5f9aff Mon Sep 17 00:00:00 2001 From: feruzm Date: Tue, 1 Sep 2026 16:54:15 +0000 Subject: [PATCH 2/4] Filter existing column settings too, and keep the selection across a re-pick Two review findings, both on the same root cause: the filtering lived inside one component instead of being shared. Codex and Qodo both caught that only the add-column picker was filtered. DeckNotificationsColumn still handed the full list to DeckContentTypeColumnSettings, so an existing cross-account column could be switched onto nfavorites, nbookmarks or scheduled_published afterwards, recreating exactly the empty column the filtering exists to prevent. Both call sites now resolve their options through notificationContentTypesFor(). CodeRabbit caught that clearing the username to re-pick an account passes through an empty string, and the effect treated that as a switch and dropped a valid selection even when the same account was chosen again. The effect now skips while the username is empty. Spec covers the helper directly: everything offered for your own account and case-insensitively so, the self-only types withheld for another account while `all` and `transfers` remain, and withheld when signed out or with no target, since neither an unknown viewer nor an unset target can be shown to be self. typecheck clean, lint clean, 373 files / 3688 tests pass. --- ...deck-add-column-notifications-settings.tsx | 25 ++++++----- .../columns/deck-notifications-column.tsx | 4 +- .../decks/_components/consts/content-types.ts | 24 +++++++++++ .../decks/notification-content-types.spec.ts | 41 +++++++++++++++++++ 4 files changed, 79 insertions(+), 15 deletions(-) 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 19e3f7bf63..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 @@ -2,11 +2,7 @@ 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, - SELF_ONLY_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"; @@ -25,21 +21,24 @@ export const DeckAddColumnNotificationsSettings = ({ deckKey }: SettingsProps) = // 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. Hive names are lowercase, but compare defensively. - const isSelf = !!activeUser && username.toLowerCase() === activeUser.username.toLowerCase(); - const contentTypes = isSelf - ? NOTIFICATION_CONTENT_TYPES - : NOTIFICATION_CONTENT_TYPES.filter( - ({ type }) => !SELF_ONLY_NOTIFICATION_CONTENT_TYPES.includes(type) - ); + // 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); } - }, [contentTypes, contentType]); + }, [username, contentTypes, contentType]); return (
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..067fd9cced 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 @@ -3,7 +3,7 @@ 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 { notificationContentTypesFor, notificationsTitles } from "../consts"; import { DeckGridContext } from "../deck-manager"; import { DeckPostViewer } from "./content-viewer"; import { DeckLoginOverlayPlaceholder } from "./deck-login-overlay-placeholder"; @@ -106,7 +106,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 05ee70f321..960092fefd 100644 --- a/apps/web/src/app/decks/_components/consts/content-types.ts +++ b/apps/web/src/app/decks/_components/consts/content-types.ts @@ -115,3 +115,27 @@ export const SELF_ONLY_NOTIFICATION_CONTENT_TYPES = [ "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) + ); +} 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 index 61a4cb23a9..cbdf52835e 100644 --- a/apps/web/src/specs/app/decks/notification-content-types.spec.ts +++ b/apps/web/src/specs/app/decks/notification-content-types.spec.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from "vitest"; import { NOTIFICATION_CONTENT_TYPES, + notificationContentTypesFor, SELF_ONLY_NOTIFICATION_CONTENT_TYPES } from "@/app/decks/_components/consts"; @@ -52,3 +53,43 @@ describe("Decks self-only notification content types", () => { 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"); + } + }); +}); From 0818808e718bf3795af931dd5dfc94a83167a4f0 Mon Sep 17 00:00:00 2001 From: feruzm Date: Tue, 1 Sep 2026 18:13:50 +0000 Subject: [PATCH 3/4] Normalize a persisted content type that is no longer allowed, drop cand.txt Review was right that filtering the selector was not enough. A column persists its contentType, so one created before its target became cross-account, or created while signed in as a different account, keeps a self-only type. The selector no longer offers it, but the stored value is unchanged, so the column kept fetching a filter that returns nothing and sat permanently empty with its own current value missing from the dropdown. effectiveNotificationContentType() falls back to "all" when the stored type is not available for the column's target. It is used twice: for fetching, so the very first load is already correct, and to persist the correction, so the stored value, the header subtitle and the selector agree and it survives a reload. It depends on the ACTIVE user as well as the target, which is the half a target-only check would miss: signing in as someone else is what turns a self column into a cross-account one, and signing out does the same. Mutation-verified: removing the fallback fails three cases, including the account-switch one. Also removes apps/web/cand.txt. That was an untracked scratch file already in the working tree, and a `git add -A` in the earlier commit swept it in. It has nothing to do with this change. typecheck clean, lint clean, 373 files / 3692 tests pass. --- apps/web/cand.txt | 29 ------------- .../columns/deck-notifications-column.tsx | 43 ++++++++++++++++--- .../decks/_components/consts/content-types.ts | 19 ++++++++ .../decks/notification-content-types.spec.ts | 43 +++++++++++++++++++ 4 files changed, 98 insertions(+), 36 deletions(-) delete mode 100644 apps/web/cand.txt diff --git a/apps/web/cand.txt b/apps/web/cand.txt deleted file mode 100644 index a36324417f..0000000000 --- a/apps/web/cand.txt +++ /dev/null @@ -1,29 +0,0 @@ -/@anime.indo.net/harley-f84fae1031806 -/@aquagelas/i-once-wanted-to-be -/@awulonualex/what-a-mess -/@brianoflondon/anyone-can-start-receiving-value-4-value-streaming-sats-for-your-podcast -/@claudio83/first-accumulation-strategies-towards-the-next-set -/@culgin/why-are-banks-so-profitable -/@daily.prompt/19-january-2024-mariannewests-freewrite-writing-prompt-day-2256--flat -/@educ.leb/manchester-united-after-the-decisive-double-of-pogba-mourinho-sends-messages -/@elizabeths14/pesadillas-es-mejor-que-suene-contigo-esp-eng -/@gamarm/un-hada-real-en-las-selvas-del-amazonas-colombia -/@greengalletti/lincoln-rhyme-hunt-for-the-bone-collector-2020-is-a-tv-remake-based-on-the-book-by-jeffery-deaver -/@hrichakar/5e133036 -/@joey-fancy/umv-vl-2020-star-trek-first-contact-day -/@knotwork/gravitational-wave-mechanics-and-circuitry-is-means-so-can-do -/@krolestwo/glosowanie-ustawa-o-skladaniu-wnioskow -/@marry.ulhikmah/use-your-brain-in-your-life -/@midlet/collections-a-feature-suggestion-for-peakd -/@myeong/re-yeckingo1-r9e4u6 -/@mysticnamuha/projection-becomes-the-medium-or-the-lens-through-which-we-view-ourselves-and-the-world-from -/@nodzz/episode-4-or-genie -/@queroveromundo/rss -/@ramsayubolt/online-gaming-ad5b6148a3bfe -/@reysa/rss -/@ruffatotmeee/loh-215-plan-and-priorities-next-year-to-heal-a-trauma-and-to-bring-peace-in-my-head -/@stokjockey/lets-take-a-look-at-another-silver-art-bar-from-1973-from-the-mount-everest-mint-mintage-of-12-671 -/@tobetada/welcome-to-the-love-the-clouds-contest-172 -/@very86/review-tv-orange-is-the-new-black-spoiler-alert -/@vicnzia/parte-i-conoce-las-posturas-del-yoga-chakrasana-o-la-postura-de-la-rueda-part-i-know-the-yoga-postures-chakrasana-or-the-wh -/@zimri7/re-silentnightowl-2026419t181911386z 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 067fd9cced..19764a3042 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,13 @@ -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 { notificationContentTypesFor, notificationsTitles } from "../consts"; +import { + effectiveNotificationContentType, + notificationContentTypesFor, + notificationsTitles +} from "../consts"; import { DeckGridContext } from "../deck-manager"; import { DeckPostViewer } from "./content-viewer"; import { DeckLoginOverlayPlaceholder } from "./deck-login-overlay-placeholder"; @@ -39,20 +43,45 @@ 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. Depends on the active user too, since signing in + // as a different account is what turns a self column into a cross-account one. + useEffect(() => { + if (effectiveContentType !== settings.contentType) { + updateColumnSpecificSettings(id, { contentType: effectiveContentType }); + } + }, [effectiveContentType, settings.contentType, 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 +101,7 @@ export const DeckNotificationsColumn = ({ id, settings, draggable }: Props) => { setIsFirstLoaded(true); } }, - [activeUser, data, settings.contentType, settings.username] + [activeUser, data, effectiveContentType, settings.username] ); useEffect(() => { @@ -106,7 +135,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 960092fefd..b283958869 100644 --- a/apps/web/src/app/decks/_components/consts/content-types.ts +++ b/apps/web/src/app/decks/_components/consts/content-types.ts @@ -139,3 +139,22 @@ export function notificationContentTypesFor( ({ 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"; +} 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 index cbdf52835e..3cd981eeea 100644 --- a/apps/web/src/specs/app/decks/notification-content-types.spec.ts +++ b/apps/web/src/specs/app/decks/notification-content-types.spec.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from "vitest"; import { + effectiveNotificationContentType, NOTIFICATION_CONTENT_TYPES, notificationContentTypesFor, SELF_ONLY_NOTIFICATION_CONTENT_TYPES @@ -93,3 +94,45 @@ describe("notificationContentTypesFor", () => { } }); }); + +/** + * 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" + ); + }); +}); From ea6e76feb4c5270522005a097044ba202d694991 Mon Sep 17 00:00:00 2001 From: feruzm Date: Tue, 1 Sep 2026 18:34:54 +0000 Subject: [PATCH 4/4] Do not persist a content-type correction before the account is known Review caught a regression in my own previous fix. The global store starts with no active user and ClientInit restores it after mount, so during that first render every column looks cross-account. The persist effect fired then, so a locally stored self column using nfavorites lost its filter on an ordinary page reload, before the account it belongs to had been restored. Fetching and persisting answer different questions, so they are now separate. What to FETCH is safe to decide immediately, and "all" is the right temporary answer while signed out. What to PERSIST waits until an active username is known, because writing on a guess is destructive and the guess is wrong on every reload. shouldPersistContentTypeCorrection() holds that rule so the lifecycle is testable without mounting the store. Tests cover the exact sequence a reload performs, undefined then the owning username, leaving the stored filter intact throughout, plus persisting once a different account is genuinely active. Mutation-verified: removing the uninitialized guard fails both lifecycle cases. typecheck clean, lint clean, 373 files / 3696 tests pass. --- .../columns/deck-notifications-column.tsx | 25 ++++++++++--- .../decks/_components/consts/content-types.ts | 26 ++++++++++++++ .../decks/notification-content-types.spec.ts | 36 +++++++++++++++++++ 3 files changed, 82 insertions(+), 5 deletions(-) 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 19764a3042..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 @@ -6,7 +6,8 @@ import { DraggableProvidedDragHandleProps } from "@hello-pangea/dnd"; import { effectiveNotificationContentType, notificationContentTypesFor, - notificationsTitles + notificationsTitles, + shouldPersistContentTypeCorrection } from "../consts"; import { DeckGridContext } from "../deck-manager"; import { DeckPostViewer } from "./content-viewer"; @@ -63,13 +64,27 @@ export const DeckNotificationsColumn = ({ id, settings, draggable }: Props) => { ); // Persist the correction so the stored value, the header subtitle and the selector all - // agree, and so it survives a reload. Depends on the active user too, since signing in - // as a different account is what turns a self column into a cross-account one. + // 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 (effectiveContentType !== settings.contentType) { + if ( + shouldPersistContentTypeCorrection( + settings.contentType, + settings.username, + activeUser?.username + ) + ) { updateColumnSpecificSettings(id, { contentType: effectiveContentType }); } - }, [effectiveContentType, settings.contentType, id, updateColumnSpecificSettings]); + }, [ + settings.contentType, + settings.username, + activeUser?.username, + effectiveContentType, + id, + updateColumnSpecificSettings + ]); const fetchData = useCallback( async (since?: ApiNotification) => { 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 b283958869..79685ec466 100644 --- a/apps/web/src/app/decks/_components/consts/content-types.ts +++ b/apps/web/src/app/decks/_components/consts/content-types.ts @@ -158,3 +158,29 @@ export function effectiveNotificationContentType( 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 index 3cd981eeea..bf5ff7f72c 100644 --- a/apps/web/src/specs/app/decks/notification-content-types.spec.ts +++ b/apps/web/src/specs/app/decks/notification-content-types.spec.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from "vitest"; import { effectiveNotificationContentType, + shouldPersistContentTypeCorrection, NOTIFICATION_CONTENT_TYPES, notificationContentTypesFor, SELF_ONLY_NOTIFICATION_CONTENT_TYPES @@ -136,3 +137,38 @@ describe("effectiveNotificationContentType", () => { ); }); }); + +/** + * 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); + }); +});