mirror of
https://github.com/civitai/civitai.git
synced 2026-09-20 22:08:18 +08:00
fix(remix-gallery): scope the gallery to the viewer, not to the image it hangs beside (#4497)
* fix(remix-gallery): scope the gallery to the viewer, not to the image it hangs beside Reported by a user 2026-08-29: a feed card showed "1 remix" and the remix gallery on that image's own detail page showed the empty state. Both numbers came from the same rows and the same SQL fragment. `ImageDetail2` wraps its whole sidebar in a `BrowsingLevelProvider` set to `image.nsfwLevel`, and `useBrowsingLevelDebounced` resolves `forced ?? override ?? user` — so the gallery ran at the HOST image's rating. The row filter is `(i."nsfwLevel" & <levels>) != 0`, and an entry rated above its host cannot intersect it, so it was dropped for every viewer including the owner who approved it and was paid for it. The feed card has no override, so it counted the same row. Measured on prod: 161 of 488 approved entries invisible that way, 160 of them paid, 10,550 Buzz. The reported pair was a PG13 entry on a PG host. The gallery now reads a new `useViewerBrowsingLevelDebounced`, which resolves `forced ?? user`. Where no page set an override — every feed card — the two resolutions return the same number, which is what makes the gallery and the flyout agree. `forced` stays first, and that is why this is a hook rather than a read of `useBrowsingSettings`. It carries the domain cap that mirrors the server middleware: anonymous is PG anywhere, logged-in on the green domain is PG+PG-13. Skipping the page override must not skip that. Both resolutions move into `resolve-browsing-level.ts` as pure functions, because the hooks are React and a hook test lands in the `component` project, which runs in no CI job. The precedence is now tested where CI can see it, and there is a source guard on the call site as well — the bug was a hook name at one call site, and testing the resolver alone would have proved nothing about it. Not touched: `ImageRemixOfDetails` reads the same overridden value by design, and that one is about the image itself rather than about other people's images. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(remix-gallery): enforce the creator's content band on read, and pin the hooks Two findings from the review round, both real. The content rule was enforced at submit, at approve and on the decline sweep - never on read. It did not need to be, because the detail page scoped the gallery to the host image's rating and an entry above the host could not intersect it. Removing that scoping removed the enforcement with it, and nothing replaced it. A host re-rated down after approval would then keep rendering entries above the band its owner set, for the week approval is locked. `hostContentBand` sits beside `minorHostCeiling` in the shared visibility fragment, so the batched count inherits it and cannot disagree with the gallery it opens - which is the bug class this whole file exists to prevent. It resolves the rule across all three placement scopes, because settings merge PER KEY and an image with no row of its own inherits its owner's rule. Measured on the replica: 280 of 575 approved entries resolve to `any` that way, 80 of them sit above their host, and every one of those must keep rendering. Running the exact predicate against prod hides ZERO rows today - this closes a latent hole rather than changing what anyone sees. Second: the resolver tests pinned a function that was never where the bug lived. Three mutations survived the whole suite - deleting the `forced` mapping from the viewer hook (optional field, so typecheck stays green, and the domain cap silently stops applying), swapping `override` and `user` in the page hook, and aliasing one hook to the other, which restores the original bug outright. `browsing-level-hooks.test.ts` renders both hooks under a hand-built context and catches all three; each now fails with a message naming what broke. Also corrected three comments this branch made false: ImageRemixOfDetails said the gallery card beside it reads the same overridden value, getServerBrowsingLevel said its client twin was the page-scoped hook, and a docblock claimed a test proved the gallery and the flyout agree - which is a claim about two React trees that no test here touches. The batch provider now records why its count is page-scoped while the gallery is viewer-scoped, rather than leaving two call sites of one concept silently disagreeing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(remix-gallery): scope the band assertions so the mutations actually fail The re-review found the new SQL case asserted the clauses and nothing between them. Four mutations passed it, and I confirmed each by running it: host lookup repointed at the entry -> band always true, inert, 8/8 green OR between the arms flipped to AND -> hides the 80 `any` rows, 8/8 green escape hatch inverted to != 'any' -> 8/8 green COALESCE scopes resolved user-first -> precedence backwards, 8/8 green Two of those survived my FIRST attempt at fixing them, which is the part worth recording. `FROM "Image" h WHERE h.id = pl."targetId"` is also rendered by `minorHostCeiling`, so asserting it proved nothing about the band - the same trap the `acceptableMinor` case in this file already documents. And `!= 'any'` ends with `= 'any'`, so the substring matched the inversion it was meant to catch. Both assertions are now anchored to a slice only the band can produce. All four mutations now fail, one test each, each naming what broke. Also: the fallback rule is a bound parameter that no string assertion can see, so it is asserted by value with a fixture guard beside it - flipping the product default to `any` would otherwise disarm the band for every host without a stored rule, silently and green. The context docblock no longer implies the hook test covers the cap's VALUE. It covers the ORDER. The value is computed in the provider from `canViewNsfw` and `currentUser`, and supplying the context by hand is exactly what hides that from the test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Justin Maier
@@ -3,7 +3,11 @@ import React, { createContext, useContext, useState } from 'react';
|
||||
import { useCurrentUser } from '~/hooks/useCurrentUser';
|
||||
import { useBrowsingSettings } from '~/providers/BrowserSettingsProvider';
|
||||
import { useFeatureFlags } from '~/providers/FeatureFlagsProvider';
|
||||
import { NsfwLevel } from '~/server/common/enums';
|
||||
import {
|
||||
BROWSING_LEVEL_FALLBACK,
|
||||
resolvePageBrowsingLevel,
|
||||
resolveViewerBrowsingLevel,
|
||||
} from '~/components/BrowsingLevel/resolve-browsing-level';
|
||||
import {
|
||||
nsfwBrowsingLevelsFlag,
|
||||
publicBrowsingLevelsFlag,
|
||||
@@ -19,7 +23,19 @@ type BrowsingModeProviderState = {
|
||||
blurLevels: number;
|
||||
};
|
||||
|
||||
const BrowsingModeOverrideCtx = createContext<
|
||||
/**
|
||||
* Exported for `__tests__/browsing-level-hooks.test.ts`, which renders the two
|
||||
* hooks under a hand-built value. The precedence they encode is a safety rule —
|
||||
* the domain cap must beat both the page override and the viewer's preference —
|
||||
* and the ORDER of that precedence lives in how each hook MAPS this context
|
||||
* onto the resolver, which no test of the resolver alone can reach.
|
||||
*
|
||||
* ⚠️ What that test does NOT cover: the cap's VALUE, computed below from
|
||||
* `features.canViewNsfw` and `currentUser`. Supplying the context by hand is
|
||||
* what makes the hooks testable at all, and it is exactly what makes that
|
||||
* computation invisible to them.
|
||||
*/
|
||||
export const BrowsingModeOverrideCtx = createContext<
|
||||
BrowsingModeProviderState & {
|
||||
setBrowsingLevelOverride?: React.Dispatch<React.SetStateAction<number | undefined>>;
|
||||
setForcedBrowsingLevel?: React.Dispatch<React.SetStateAction<number | undefined>>;
|
||||
@@ -86,9 +102,44 @@ export function BrowsingLevelProvider({
|
||||
export function useBrowsingLevelDebounced() {
|
||||
const { forcedBrowsingLevel, browsingLevelOverride, userBrowsingLevel } =
|
||||
useBrowsingLevelContext();
|
||||
const browsingLevel = forcedBrowsingLevel ?? browsingLevelOverride ?? userBrowsingLevel;
|
||||
const browsingLevel = resolvePageBrowsingLevel({
|
||||
forced: forcedBrowsingLevel,
|
||||
override: browsingLevelOverride,
|
||||
user: userBrowsingLevel,
|
||||
});
|
||||
const [debounced] = useDebouncedValue(browsingLevel, 500);
|
||||
return debounced ? debounced : NsfwLevel.PG;
|
||||
return debounced ? debounced : BROWSING_LEVEL_FALLBACK;
|
||||
}
|
||||
|
||||
/**
|
||||
* The viewer's own browsing level, ignoring any per-page override.
|
||||
*
|
||||
* `useBrowsingLevelDebounced` resolves `forcedBrowsingLevel ?? browsingLevelOverride
|
||||
* ?? userBrowsingLevel`. The middle term is what a page sets when it wants its
|
||||
* subtree read at some OTHER level — the image detail page passes the image's own
|
||||
* rating, so everything in its sidebar is scoped to that image rather than to the
|
||||
* person looking at it.
|
||||
*
|
||||
* That is right for the image and wrong for a list of OTHER people's images
|
||||
* beside it: an entry rated above the host can never intersect the host's own
|
||||
* level, so it is dropped for every viewer including the owner who approved it.
|
||||
* Measured on prod 2026-08-29: 161 of 488 approved remix-gallery entries were
|
||||
* invisible that way, 160 of them paid.
|
||||
*
|
||||
* 🔴 `forcedBrowsingLevel` is still honoured, and that is the whole reason this
|
||||
* is a separate hook rather than a call to `useBrowsingSettings`. It carries the
|
||||
* DOMAIN cap — anonymous anywhere is PG, logged-in on the green domain is
|
||||
* PG+PG-13 — which mirrors the server middleware and is not a preference anyone
|
||||
* may opt out of. Only the page-level override is skipped.
|
||||
*/
|
||||
export function useViewerBrowsingLevelDebounced() {
|
||||
const { forcedBrowsingLevel, userBrowsingLevel } = useBrowsingLevelContext();
|
||||
const browsingLevel = resolveViewerBrowsingLevel({
|
||||
forced: forcedBrowsingLevel,
|
||||
user: userBrowsingLevel,
|
||||
});
|
||||
const [debounced] = useDebouncedValue(browsingLevel, 500);
|
||||
return debounced ? debounced : BROWSING_LEVEL_FALLBACK;
|
||||
}
|
||||
|
||||
export function BrowsingLevelProviderOptional({
|
||||
|
||||
@@ -0,0 +1,114 @@
|
||||
// @vitest-environment happy-dom
|
||||
import { act, createElement } from 'react';
|
||||
import { createRoot } from 'react-dom/client';
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import {
|
||||
BrowsingModeOverrideCtx,
|
||||
useBrowsingLevelDebounced,
|
||||
useViewerBrowsingLevelDebounced,
|
||||
} from '~/components/BrowsingLevel/BrowsingLevelProvider';
|
||||
import { NsfwLevel } from '~/server/common/enums';
|
||||
import {
|
||||
allBrowsingLevelsFlag,
|
||||
sfwBrowsingLevelsFlag,
|
||||
} from '~/shared/constants/browsingLevel.constants';
|
||||
|
||||
/**
|
||||
* The two hooks, tested where the bug can actually live: in how each one MAPS
|
||||
* the provider's three values onto the resolver.
|
||||
*
|
||||
* 🔴 `resolve-browsing-level.test.ts` pins the resolver, and the resolver was
|
||||
* never where anything went wrong. Three mutations survive that file entirely:
|
||||
*
|
||||
* 1. Delete `forced: forcedBrowsingLevel,` from `useViewerBrowsingLevelDebounced`.
|
||||
* The field is optional, so typecheck stays green; the resolver is untouched,
|
||||
* so its tests stay green. The domain cap silently stops applying and a
|
||||
* logged-in green-domain viewer is served their saved `allBrowsingLevels`.
|
||||
* 2. Swap the `override` and `user` keys in `useBrowsingLevelDebounced`. Both
|
||||
* are numbers, so it typechecks, and the viewer's preference then beats every
|
||||
* page override in the app.
|
||||
* 3. `export const useViewerBrowsingLevelDebounced = useBrowsingLevelDebounced`
|
||||
* — the "these two look duplicated, consolidate them" tidy-up, which restores
|
||||
* the original bug with every other test still green.
|
||||
*
|
||||
* The two cases below catch all three. Neither asserts a hook's internals; they
|
||||
* assert the answer each hook gives for a context a real page can produce.
|
||||
*
|
||||
* No fake timers: Mantine's `useDebouncedValue` seeds its state with the value
|
||||
* and only starts debouncing after a mounted-ref effect, so the first render
|
||||
* already returns the resolved value.
|
||||
*/
|
||||
(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true;
|
||||
|
||||
type Ctx = {
|
||||
forcedBrowsingLevel?: number;
|
||||
browsingLevelOverride?: number;
|
||||
userBrowsingLevel: number;
|
||||
};
|
||||
|
||||
function renderBoth(ctx: Ctx) {
|
||||
const result = { page: 0, viewer: 0 };
|
||||
const container = document.createElement('div');
|
||||
const root = createRoot(container);
|
||||
|
||||
function Probe() {
|
||||
result.page = useBrowsingLevelDebounced();
|
||||
result.viewer = useViewerBrowsingLevelDebounced();
|
||||
return null;
|
||||
}
|
||||
|
||||
act(() => {
|
||||
root.render(
|
||||
createElement(
|
||||
BrowsingModeOverrideCtx.Provider,
|
||||
{ value: { ...ctx, blurLevels: 0 } },
|
||||
createElement(Probe)
|
||||
)
|
||||
);
|
||||
});
|
||||
act(() => root.unmount());
|
||||
return result;
|
||||
}
|
||||
|
||||
describe('browsing level hooks', () => {
|
||||
/**
|
||||
* 🔴 THE SAFETY CASE. `forcedBrowsingLevel` is a cap nobody may opt out of —
|
||||
* the domain ceiling mirroring `applyDomainFeature`, and the minor-safe cap
|
||||
* that three galleries write into the same slot. It must beat BOTH the page
|
||||
* override and the viewer's saved preference, in BOTH hooks.
|
||||
*
|
||||
* If this fails, do not adjust the expectation. Someone is being served
|
||||
* content past a ceiling that is not theirs to lift.
|
||||
*/
|
||||
it('puts the forced cap above everything, in both hooks', () => {
|
||||
const { page, viewer } = renderBoth({
|
||||
forcedBrowsingLevel: sfwBrowsingLevelsFlag,
|
||||
browsingLevelOverride: NsfwLevel.XXX,
|
||||
userBrowsingLevel: allBrowsingLevelsFlag,
|
||||
});
|
||||
|
||||
expect(page, 'the page hook must not let an override lift the cap').toBe(sfwBrowsingLevelsFlag);
|
||||
expect(viewer, 'the viewer hook must not let a saved preference lift the cap').toBe(
|
||||
sfwBrowsingLevelsFlag
|
||||
);
|
||||
});
|
||||
|
||||
/**
|
||||
* The two hooks must DISAGREE here, and that disagreement is the fix.
|
||||
*
|
||||
* A page override scopes its subtree — the image detail page sets the host
|
||||
* image's rating. That is right for the image and wrong for a list of other
|
||||
* people's images beside it, which is why the gallery reads the viewer hook.
|
||||
* Collapsing the two hooks into one — by aliasing, or by re-adding `override`
|
||||
* to the viewer resolution — fails here.
|
||||
*/
|
||||
it('lets a page override scope the page hook, and only the page hook', () => {
|
||||
const { page, viewer } = renderBoth({
|
||||
browsingLevelOverride: NsfwLevel.PG,
|
||||
userBrowsingLevel: allBrowsingLevelsFlag,
|
||||
});
|
||||
|
||||
expect(page, 'a page override must still scope the page hook').toBe(NsfwLevel.PG);
|
||||
expect(viewer, 'the viewer hook must ignore the page override').toBe(allBrowsingLevelsFlag);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,154 @@
|
||||
import { readFileSync } from 'fs';
|
||||
import path from 'path';
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import {
|
||||
resolvePageBrowsingLevel,
|
||||
resolveViewerBrowsingLevel,
|
||||
} from '~/components/BrowsingLevel/resolve-browsing-level';
|
||||
import { NsfwLevel } from '~/server/common/enums';
|
||||
import {
|
||||
allBrowsingLevelsFlag,
|
||||
publicBrowsingLevelsFlag,
|
||||
sfwBrowsingLevelsFlag,
|
||||
} from '~/shared/constants/browsingLevel.constants';
|
||||
|
||||
describe('resolvePageBrowsingLevel', () => {
|
||||
it('lets a page override the viewer for its own subtree', () => {
|
||||
expect(resolvePageBrowsingLevel({ override: NsfwLevel.PG, user: allBrowsingLevelsFlag })).toBe(
|
||||
NsfwLevel.PG
|
||||
);
|
||||
});
|
||||
|
||||
it('puts the domain cap above the page override', () => {
|
||||
expect(
|
||||
resolvePageBrowsingLevel({
|
||||
forced: publicBrowsingLevelsFlag,
|
||||
override: NsfwLevel.XXX,
|
||||
user: allBrowsingLevelsFlag,
|
||||
})
|
||||
).toBe(publicBrowsingLevelsFlag);
|
||||
});
|
||||
|
||||
it('falls back to the viewer when no page asked for anything', () => {
|
||||
expect(resolvePageBrowsingLevel({ user: sfwBrowsingLevelsFlag })).toBe(sfwBrowsingLevelsFlag);
|
||||
});
|
||||
});
|
||||
|
||||
describe('resolveViewerBrowsingLevel', () => {
|
||||
/**
|
||||
* The reason this function exists. `ImageDetail2` wraps its sidebar in a
|
||||
* provider set to the host image's rating, and anything in there reading the
|
||||
* page level is scoped to that image — including a list of OTHER people's
|
||||
* images, whose entries then cannot intersect it and vanish. Measured on prod
|
||||
* 2026-08-29: 161 of 488 approved remix-gallery entries invisible that way,
|
||||
* 160 of them paid.
|
||||
*
|
||||
* Asserted as "the override changes nothing" rather than against a number, so
|
||||
* this fails for anyone who routes this back through the page resolution.
|
||||
*/
|
||||
it('ignores a page override — that is the whole point of it', () => {
|
||||
const user = allBrowsingLevelsFlag;
|
||||
|
||||
expect(resolveViewerBrowsingLevel({ user })).toBe(user);
|
||||
// Same call with an override in scope must give the same answer. The typed
|
||||
// signature omits `override`, so this is what a caller passing the full
|
||||
// context object would produce.
|
||||
expect(resolveViewerBrowsingLevel({ user, ...({ override: NsfwLevel.PG } as object) })).toBe(
|
||||
user
|
||||
);
|
||||
});
|
||||
|
||||
/**
|
||||
* 🔴 THE SAFETY PROPERTY. Named for the decision so the next person to
|
||||
* "simplify" this to `user` alone has to delete a test that says why not.
|
||||
*
|
||||
* `forced` is the domain cap and it mirrors the server middleware: anonymous
|
||||
* is PG anywhere, and a logged-in viewer on the green domain is PG+PG-13
|
||||
* regardless of what they saved in their settings. Skipping the PAGE override
|
||||
* must not also skip that.
|
||||
*
|
||||
* If this ever fails, do not adjust the expectation — a signed-in viewer whose
|
||||
* preference is "everything" is being served everything on a domain that is
|
||||
* not allowed to serve it.
|
||||
*/
|
||||
it('keeps the domain cap above the viewer preference', () => {
|
||||
expect(
|
||||
resolveViewerBrowsingLevel({ forced: sfwBrowsingLevelsFlag, user: allBrowsingLevelsFlag }),
|
||||
'the green-domain cap must beat a saved preference of allBrowsingLevels'
|
||||
).toBe(sfwBrowsingLevelsFlag);
|
||||
|
||||
expect(
|
||||
resolveViewerBrowsingLevel({ forced: publicBrowsingLevelsFlag, user: allBrowsingLevelsFlag }),
|
||||
'the anonymous cap must beat a saved preference of allBrowsingLevels'
|
||||
).toBe(publicBrowsingLevelsFlag);
|
||||
});
|
||||
|
||||
/**
|
||||
* The two resolutions agree wherever no page set an override.
|
||||
*
|
||||
* ⚠️ Narrower than it looks, said plainly so nobody cites it as more: with
|
||||
* `override` omitted, `resolvePageBrowsingLevel` reduces to this function's
|
||||
* own body, so two of these rows compare an implementation against a copy of
|
||||
* itself. The 'green domain' row is the one that earns its place — it fails a
|
||||
* one-sided `user ?? forced`.
|
||||
*
|
||||
* It says NOTHING about the gallery and the feed flyout agreeing. That is a
|
||||
* claim about two React trees and two providers, and no test here touches
|
||||
* either; `browsing-level-hooks.test.ts` is as close as this gets.
|
||||
*/
|
||||
it.each([
|
||||
['anonymous', { forced: publicBrowsingLevelsFlag, user: publicBrowsingLevelsFlag }],
|
||||
['green domain', { forced: sfwBrowsingLevelsFlag, user: allBrowsingLevelsFlag }],
|
||||
['uncapped', { user: allBrowsingLevelsFlag }],
|
||||
])('agrees with the page resolution when nothing overrode it (%s)', (_label, inputs) => {
|
||||
expect(resolveViewerBrowsingLevel(inputs)).toBe(resolvePageBrowsingLevel(inputs));
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* The call site, not the function.
|
||||
*
|
||||
* Everything above tests a pure resolver that `RemixGalleryCard` merely has to
|
||||
* CHOOSE to use. The bug was never in the resolver — it was one hook name at one
|
||||
* call site, and unit-testing the resolver would have proved nothing about it.
|
||||
* The hooks are React, so a render test lands in the `component` project, which
|
||||
* runs in no CI job; this reads the source instead.
|
||||
*
|
||||
* ⚠️ What this does and does not catch, stated because a source guard flatters
|
||||
* itself: it catches reverting to `useBrowsingLevelDebounced`, which is the
|
||||
* regression that actually happened. It would NOT catch someone reading
|
||||
* `useBrowsingLevelContext()` and resolving the level by hand. It pins a
|
||||
* spelling, and the spelling it pins is the likely one.
|
||||
*/
|
||||
describe('the remix gallery reads the viewer, not the page', () => {
|
||||
const cardPath = path.resolve(__dirname, '..', '..', 'RemixGallery', 'RemixGalleryCard.tsx');
|
||||
// Throws rather than passing if the file moves or is renamed. A guard whose
|
||||
// subject has vanished must go red, not quietly become vacuous.
|
||||
const source = readFileSync(cardPath, 'utf-8');
|
||||
|
||||
it('is reading the file it thinks it is', () => {
|
||||
expect(source, 'wrong file — this guard is pointed at nothing').toContain('RemixGalleryCard');
|
||||
});
|
||||
|
||||
it('uses the viewer hook', () => {
|
||||
expect(source).toContain('useViewerBrowsingLevelDebounced()');
|
||||
});
|
||||
|
||||
/**
|
||||
* 🔴 `ImageDetail2` wraps this card in a provider set to the HOST image's
|
||||
* rating. The ordinary hook inherits that, which scopes a list of other
|
||||
* people's images to the rating of the image they hang beside — entries above
|
||||
* the host cannot intersect it and vanish for every viewer, including the owner
|
||||
* who approved and was paid for them. Measured on prod 2026-08-29: 161 of 488
|
||||
* approved entries invisible that way, 160 of them paid.
|
||||
*
|
||||
* Do not "simplify" this back. The domain cap is still honoured by the viewer
|
||||
* hook; skipping the page override is the entire point.
|
||||
*/
|
||||
it('does NOT use the page-scoped hook', () => {
|
||||
expect(
|
||||
source.includes('useBrowsingLevelDebounced()'),
|
||||
'RemixGalleryCard must not inherit the detail page browsing-level override'
|
||||
).toBe(false);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,51 @@
|
||||
import { NsfwLevel } from '~/server/common/enums';
|
||||
|
||||
/**
|
||||
* Which browsing level a read should run at, given the three the provider holds.
|
||||
*
|
||||
* Pure and in its own module so the precedence can be tested without rendering
|
||||
* anything — the hooks that use it are the thinnest possible wrappers over these
|
||||
* two functions.
|
||||
*
|
||||
* The three inputs are not interchangeable:
|
||||
*
|
||||
* - `forced` is the DOMAIN cap, mirroring the server middleware: anonymous is PG
|
||||
* anywhere, logged-in on the green domain is PG+PG-13. It is not a preference
|
||||
* and nobody may opt out of it, so it wins in both resolutions below.
|
||||
* - `override` is a page saying "read my subtree at some OTHER level". The image
|
||||
* detail page sets it to the image's own rating.
|
||||
* - `user` is the viewer's saved preference.
|
||||
*/
|
||||
type BrowsingLevelInputs = {
|
||||
forced?: number;
|
||||
override?: number;
|
||||
user?: number;
|
||||
};
|
||||
|
||||
/** The level a page asked for, falling back to the viewer's own. */
|
||||
export function resolvePageBrowsingLevel({ forced, override, user }: BrowsingLevelInputs) {
|
||||
return forced ?? override ?? user;
|
||||
}
|
||||
|
||||
/**
|
||||
* The viewer's own level, ignoring any page override.
|
||||
*
|
||||
* 🔴 `forced` is still first, and that ordering is the safety property worth
|
||||
* protecting: skipping the page override must not also skip the domain cap.
|
||||
* Collapsing this to `user` alone would serve a logged-in viewer's saved
|
||||
* preference on the green domain, which the server middleware forbids.
|
||||
*
|
||||
* Used where a page's subtree contains OTHER people's images — a list scoped to
|
||||
* the rating of the image it hangs beside drops entries that can never intersect
|
||||
* it. Measured on prod 2026-08-29: 161 of 488 approved remix-gallery entries
|
||||
* were invisible that way, 160 of them paid.
|
||||
*/
|
||||
export function resolveViewerBrowsingLevel({
|
||||
forced,
|
||||
user,
|
||||
}: Omit<BrowsingLevelInputs, 'override'>) {
|
||||
return forced ?? user;
|
||||
}
|
||||
|
||||
/** What both hooks fall back to once debouncing has settled on nothing. */
|
||||
export const BROWSING_LEVEL_FALLBACK = NsfwLevel.PG;
|
||||
@@ -71,10 +71,15 @@ function sourceAspectRatio(image: { width?: number | null; height?: number | nul
|
||||
* `<BrowsingLevelProvider browsingLevel={image.nsfwLevel}>`, so the strict
|
||||
* `Flags.intersects` test below compares the source against the REMIX's level.
|
||||
* The domain cap still wins (`forcedBrowsingLevel` takes priority) and
|
||||
* `ImageGuard2` still blurs from the viewer's own level, and `RemixGalleryCard`
|
||||
* beside this reads the same overridden value — so this is the established
|
||||
* detail-page pattern, not something this card invents. Do not describe it as
|
||||
* the viewer-specific gate; it is not one.
|
||||
* `ImageGuard2` still blurs from the viewer's own level. Do not describe this
|
||||
* as the viewer-specific gate; it is not one.
|
||||
*
|
||||
* ⚠️ `RemixGalleryCard` beside this NO LONGER reads the overridden value — it
|
||||
* was moved to the viewer's level deliberately, because it lists OTHER
|
||||
* people's images and scoping those to this image's rating hid entries from
|
||||
* the viewers entitled to see them. This card is different: its subject IS
|
||||
* this image's own provenance, so the host's rating is the coherent scope.
|
||||
* The two are meant to differ; do not harmonise them by copying either.
|
||||
*/
|
||||
export const ImageRemixOfDetails = ({ imageId }: { imageId: number }) => {
|
||||
const { data: generationData } = trpc.image.getGenerationData.useQuery({ id: imageId });
|
||||
|
||||
@@ -43,6 +43,16 @@ export function RemixGalleryBatchProvider({
|
||||
// filled in `allBrowsingLevelsFlag` and the thumbnails, which render as bare
|
||||
// `EdgeMedia` with no ImageGuard, came back at every level the viewer had
|
||||
// turned off.
|
||||
//
|
||||
// ⚠️ The PAGE level, while `RemixGalleryCard` reads the VIEWER's. They agree
|
||||
// wherever no page sets an override, which is every ordinary feed. Where one
|
||||
// does — the site root, the home blocks, a collection — this count is scoped
|
||||
// to the page and the gallery it opens is scoped to the viewer, so the two can
|
||||
// disagree. Deliberate for now rather than overlooked: this is a count on a
|
||||
// card inside a page-scoped feed, and widening it would show a number for
|
||||
// content the page narrowed on purpose. Raised with Justin 2026-08-30; if he
|
||||
// decides the count should follow the viewer instead, the change is this one
|
||||
// line and the reasoning above is what has to be re-argued.
|
||||
const browsingLevel = useBrowsingLevelDebounced();
|
||||
|
||||
// The sticker batch's chunker, not a third copy of it. Its own tests pin the
|
||||
|
||||
@@ -38,7 +38,7 @@ import {
|
||||
trimToWholeRows,
|
||||
type RemixGalleryItem,
|
||||
} from '~/components/RemixGallery/remix-gallery.utils';
|
||||
import { useBrowsingLevelDebounced } from '~/components/BrowsingLevel/BrowsingLevelProvider';
|
||||
import { useViewerBrowsingLevelDebounced } from '~/components/BrowsingLevel/BrowsingLevelProvider';
|
||||
import { useCurrentUser } from '~/hooks/useCurrentUser';
|
||||
import { useFeatureFlags } from '~/providers/FeatureFlagsProvider';
|
||||
import { Currency } from '~/shared/utils/prisma/enums';
|
||||
@@ -156,7 +156,16 @@ export function RemixGalleryCard({ imageId }: { imageId: number }) {
|
||||
// level has to travel with the request. It is also what resolves their
|
||||
// content addons server-side, so omitting it silently disables `disableMinor`
|
||||
// and their blocked-tag list as well as the level filter itself.
|
||||
const browsingLevel = useBrowsingLevelDebounced();
|
||||
//
|
||||
// 🔴 The VIEWER's level, not `useBrowsingLevelDebounced`. `ImageDetail2` wraps
|
||||
// this whole sidebar in a `BrowsingLevelProvider` set to the host image's own
|
||||
// rating, and the ordinary hook would inherit that — scoping a list of other
|
||||
// people's images to the rating of the image they are attached to. An entry
|
||||
// above the host can never intersect it, so it was dropped for every viewer
|
||||
// including the owner who approved and was paid for it, while the feed card's
|
||||
// count (no override there) still counted it. That is the mismatch reported on
|
||||
// 2026-08-29. Domain caps still apply; only the page override is skipped.
|
||||
const browsingLevel = useViewerBrowsingLevelDebounced();
|
||||
|
||||
const { data: visibility } = trpc.placement.getRemixGalleryVisibility.useQuery(
|
||||
{ imageId, browsingLevel },
|
||||
|
||||
@@ -2,6 +2,7 @@ import { beforeEach, describe, expect, it } from 'vitest';
|
||||
import { dbMock } from '~/__tests__/mocks/db.mock';
|
||||
import { nsfwBrowsingLevelsFlag } from '~/shared/constants/browsingLevel.constants';
|
||||
import { getRemixGalleryCardSummaries } from '~/server/services/remix-gallery.service';
|
||||
import { REMIX_GALLERY_DEFAULT_CONTENT_RULE } from '~/shared/utils/remix-gallery';
|
||||
|
||||
/**
|
||||
* The gates on a remix-gallery entry, pinned because production data cannot pin
|
||||
@@ -98,6 +99,84 @@ describe('remix gallery entry visibility', () => {
|
||||
expect(sql).toContain('i."nsfwLevel"');
|
||||
});
|
||||
|
||||
/**
|
||||
* 🔴 The creator's content rule, enforced on READ.
|
||||
*
|
||||
* It used to be enforced by accident: the detail page scoped the gallery to
|
||||
* the HOST image's rating, so an entry above the host could not intersect the
|
||||
* level. That scoping was removed because it also hid entries from viewers
|
||||
* entitled to see them — 161 of 488 approved on prod, 160 paid — and nothing
|
||||
* replaced it. A host re-rated down after approval would then keep rendering
|
||||
* entries above the band its owner set, for the week approval is locked.
|
||||
*
|
||||
* ⚠️ Like the rest of this file, this pins the SPELLING of a clause and not
|
||||
* its effect: prod has ZERO approved entries that the band would hide today
|
||||
* (measured 2026-08-30 by running this exact predicate against the replica),
|
||||
* so no data-driven check can distinguish present from absent. The nonzero
|
||||
* control that the predicate is not a constant is the other direction — 280
|
||||
* of 575 approved entries resolve to `any`, and 80 of those sit above their
|
||||
* host and must keep rendering.
|
||||
*/
|
||||
it('applies the host content band, correlated to the host and joined with AND', async () => {
|
||||
const sql = await render();
|
||||
// Whitespace-normalised so the assertions below can span the fragment rather
|
||||
// than match clauses in isolation. Every survivor review found was a change
|
||||
// BETWEEN clauses that each individual `toContain` waved through.
|
||||
const flat = sql.replace(/\s+/g, ' ');
|
||||
|
||||
// The two arms AND the OR between them, as one contiguous slice. `AND` in
|
||||
// place of that `OR` leaves both arms intact and hides the 80 above-host
|
||||
// entries whose creators opted into `any` - re-creating the regression this
|
||||
// fragment was added after. It also rules out `!= 'any'`, which the bare
|
||||
// `= 'any'` substring does not.
|
||||
// 🔴 The `)` before the `=` is load-bearing: `!= 'any'` ENDS with `= 'any'`,
|
||||
// so a bare match on that substring passes with the escape hatch inverted —
|
||||
// measured, it did.
|
||||
expect(flat).toContain(`) = 'any' OR i."nsfwLevel" <=`);
|
||||
|
||||
// Correlated to THIS row's host, not to the entry and not to a bound id.
|
||||
// `WHERE h.id = i.id` renders `i."nsfwLevel" <= i."nsfwLevel"`, always true,
|
||||
// and the band goes inert while every clause assertion stays green. The
|
||||
// minor-ceiling case above measured that exact failure; this inherits the
|
||||
// lesson rather than re-discovering it.
|
||||
// 🔴 Scoped to the BAND's own subquery, not to the bare correlation.
|
||||
// `minorHostCeiling` renders `FROM "Image" h WHERE h.id = pl."targetId"` too,
|
||||
// so the unscoped version passed with this comparison repointed at the entry
|
||||
// — measured, 8/8 green while the band was inert. Same trap the
|
||||
// `acceptableMinor` case above records.
|
||||
expect(flat, 'the level comparison must read this row’s host').toContain(
|
||||
`<= (SELECT h."nsfwLevel" FROM "Image" h WHERE h.id = pl."targetId")`
|
||||
);
|
||||
|
||||
// The band is ANDed onto the predicate. Leading with OR would make
|
||||
// `entryIsVisible` permissive in its entirety, and every `toContain` in this
|
||||
// file would still pass - none of them pins a conjunction.
|
||||
expect(flat).toContain('AND ( COALESCE(');
|
||||
|
||||
// Scope order IS precedence. `resolvePlacementSpace` merges settings per key
|
||||
// with the IMAGE most specific, so resolving user first lets an owner's
|
||||
// account-level `any` beat a per-image `atOrBelow` - backwards, and silent.
|
||||
const at = (scope: string) => {
|
||||
const index = flat.indexOf(`s."entityType" = '${scope}'`);
|
||||
expect(index, `the ${scope} scope must be resolved`).toBeGreaterThan(-1);
|
||||
return index;
|
||||
};
|
||||
expect(at('image'), 'image must be resolved before post').toBeLessThan(at('post'));
|
||||
expect(at('post'), 'post must be resolved before user').toBeLessThan(at('user'));
|
||||
|
||||
// Each scope correlated to the host as well, not to some other row.
|
||||
expect(flat).toContain(`s."entityId" = pl."targetId"`);
|
||||
expect(flat).toContain(`JOIN "Image" hp ON hp.id = pl."targetId"`);
|
||||
expect(flat).toContain(`JOIN "Image" hu ON hu.id = pl."targetId"`);
|
||||
|
||||
// The fallback is BOUND, so no string assertion can see it. Asserted by
|
||||
// value, with a fixture guard beside it: if the product ever flips the
|
||||
// default to `any`, the band silently stops applying to every host without a
|
||||
// stored rule, and a spelling-only test would stay green.
|
||||
expect(REMIX_GALLERY_DEFAULT_CONTENT_RULE, 'a default of any disarms the band').not.toBe('any');
|
||||
expect(boundValues(queryRaw.mock.calls[0])).toContain(REMIX_GALLERY_DEFAULT_CONTENT_RULE);
|
||||
});
|
||||
|
||||
it('excludes entries on private posts', async () => {
|
||||
expect(await render()).toContain('"availability"');
|
||||
});
|
||||
|
||||
@@ -39,6 +39,7 @@ import {
|
||||
isRemixGalleryPlacementData,
|
||||
REMIX_GALLERY_MAX_PENDING_PER_OWNER,
|
||||
REMIX_GALLERY_MAX_PINNED,
|
||||
REMIX_GALLERY_DEFAULT_CONTENT_RULE,
|
||||
REMIX_GALLERY_MINOR_HOST_MAX_LEVEL,
|
||||
REMIX_GALLERY_PAGE_SIZE,
|
||||
REMIX_GALLERY_QUEUE_LIMIT,
|
||||
@@ -1161,6 +1162,54 @@ const minorHostCeiling = (host: Prisma.Sql) => Prisma.sql`
|
||||
)
|
||||
)`;
|
||||
|
||||
/**
|
||||
* The display-side half of the creator's content rule.
|
||||
*
|
||||
* 🔴 This used to be enforced by accident and the accident was removed. The
|
||||
* detail page wrapped its sidebar in a provider set to the HOST image's rating,
|
||||
* so an entry above the host could not intersect the level and did not render.
|
||||
* That scoping was wrong for other reasons — it hid entries from the very viewer
|
||||
* who was allowed to see them, 161 of 488 approved on 2026-08-29 — and removing
|
||||
* it left the band enforced at submit, at approve and on the decline sweep, but
|
||||
* nowhere on read.
|
||||
*
|
||||
* Which matters because a host's rating can move AFTER entries are approved, and
|
||||
* approval is irreversible for a week (`REMIX_GALLERY_REMOVAL_LOCK_HOURS`). A
|
||||
* moderator re-rating a host down would otherwise leave R entries rendering on
|
||||
* what is now a PG page, past the band its owner set. Same argument as
|
||||
* `minorHostCeiling` above; that one got its display half and this one did not.
|
||||
*
|
||||
* The rule is resolved across all three placement scopes, image then post then
|
||||
* user, because `resolvePlacementSpace` merges `settings` PER KEY — an image
|
||||
* with no row of its own still inherits its owner's rule. Querying only the
|
||||
* image scope reports "no row, so the default" and is wrong: measured on prod
|
||||
* 2026-08-30, 280 of 548 approved entries resolve to `any` through a user-scope
|
||||
* row, and 80 of those sit above their host. Under `any` they are exactly what
|
||||
* the creator opted into, so they must keep rendering.
|
||||
*
|
||||
* Deliberately NOT applying the minor cap: `minorHostCeiling` already does, and
|
||||
* it applies under both rules. Repeating it here would be a second copy of a
|
||||
* ceiling that is allowed to change.
|
||||
*/
|
||||
const hostContentBand = (host: Prisma.Sql) => Prisma.sql`
|
||||
AND (
|
||||
COALESCE(
|
||||
(SELECT s.settings ->> 'contentRule' FROM "PlacementSpace" s
|
||||
WHERE s.surface = ${SURFACE} AND s."entityType" = 'image'
|
||||
AND s."entityId" = ${host}),
|
||||
(SELECT s.settings ->> 'contentRule' FROM "PlacementSpace" s
|
||||
JOIN "Image" hp ON hp.id = ${host}
|
||||
WHERE s.surface = ${SURFACE} AND s."entityType" = 'post'
|
||||
AND s."entityId" = hp."postId"),
|
||||
(SELECT s.settings ->> 'contentRule' FROM "PlacementSpace" s
|
||||
JOIN "Image" hu ON hu.id = ${host}
|
||||
WHERE s.surface = ${SURFACE} AND s."entityType" = 'user'
|
||||
AND s."entityId" = hu."userId"),
|
||||
${REMIX_GALLERY_DEFAULT_CONTENT_RULE}
|
||||
) = 'any'
|
||||
OR i."nsfwLevel" <= (SELECT h."nsfwLevel" FROM "Image" h WHERE h.id = ${host})
|
||||
)`;
|
||||
|
||||
/**
|
||||
* Which approved entries a viewer may see, as one fragment.
|
||||
*
|
||||
@@ -1222,7 +1271,7 @@ const entryIsVisible = (levels: number, host: Prisma.Sql) => Prisma.sql`
|
||||
AND (
|
||||
(i."nsfwLevel" & ${nsfwBrowsingLevelsFlag}) = 0
|
||||
OR NOT i."modelRestricted"
|
||||
)${minorHostCeiling(host)}`;
|
||||
)${minorHostCeiling(host)}${hostContentBand(host)}`;
|
||||
|
||||
export function parseGalleryCursor(cursor?: string | null) {
|
||||
if (!cursor) return null;
|
||||
|
||||
@@ -6,8 +6,15 @@ import {
|
||||
} from '~/shared/constants/browsingLevel.constants';
|
||||
|
||||
/**
|
||||
* SSR equivalent of the client's effective browsing level
|
||||
* (`BrowsingLevelProvider` → `useBrowsingLevelDebounced`). Use when SSR code
|
||||
* SSR equivalent of the client's effective browsing level.
|
||||
*
|
||||
* 🔴 It takes no page override, so its client twin is
|
||||
* `useViewerBrowsingLevelDebounced` (`forced ?? user`), NOT
|
||||
* `useBrowsingLevelDebounced` (`forced ?? override ?? user`). The "keep in sync"
|
||||
* below means that one. Syncing it against the page-scoped hook would teach SSR
|
||||
* to reproduce an override it has no way to know about.
|
||||
*
|
||||
* Use when SSR code
|
||||
* must reproduce a browsing-level-dependent client query key — e.g. resolving
|
||||
* the browsing-settings addons to prefetch `image.getInfinite` so a feed/carousel
|
||||
* hydrates without a layout shift.
|
||||
|
||||
Reference in New Issue
Block a user