mirror of
https://github.com/civitai/civitai.git
synced 2026-09-20 22:08:18 +08:00
feat(apps): surface Build apps as an /apps/* subnav tab, collapse the dropdown to one entry (#4668)
* feat(apps): surface Build apps as an /apps/* subnav tab, collapse the dropdown to one entry
The user-menu dropdown carried two adjacent App-Blocks rows — "Build apps" ->
/apps/get-started and "Apps" -> /apps. A moderator holds both flags, so they saw
two near-identical rows for one product.
Now there is ONE row. Its href is /apps with store access and /apps/get-started
without: a get-started-only viewer cannot load /apps at all (its
getServerSideProps runs resolveAppsPageAccess, which returns notFound), so
pointing them there would be a menu entry into a 404. "Build apps" is instead a
tab in the shared /apps/* subnav, ahead of Marketplace.
The subnav's whole-bar gate widens from hasAppsStoreAccess(features) to
hasAppsStoreAccess(features) || features.appBlocksGetStarted. This is required,
not optional: on the old gate the container returns null for exactly the cohort
the new tab exists for, so the tab would be invisible to them on the one page
they can load. Safe on the first paint for the same reason already written out
for appBlocksAuthor — appBlocksGetStarted is SSR-seeded into pageProps.flags,
frozen by useState in FeatureFlagsProvider, and not toggleable, so the server and
first client renders compute the same boolean.
Visibility side effect, intended: "Build apps" is unconditional, so the always-on
tab set goes 1 -> 2 and the "< 2 tabs" collapse no longer hides the bar for a
non-author with no installs. Those viewers now get the subnav on all 13 /apps/*
routes. The collapse branch is kept but is no longer reachable through the
container, and the tests that used to cover it say so rather than pretending.
The app-block chrome menu deliberately does NOT mirror the new row: it opens over
a RUNNING app, and it has no feature-flag plumbing, so mirroring a kill-switched
page would keep offering it after the switch. That exclusion is now asserted as a
set in chromeNavAlignsWithSubNav.test.ts rather than left as silence.
Known and unchanged by this commit: the Marketplace tab is unconditional, so a
viewer admitted by the get-started term alone sees a tab that /apps answers with
notFound. Not reachable today (the flag is staged mod-only and a moderator holds
the store flags), but the trigger is a Flipt toggle, not a deploy. Gating
Marketplace is not the fix — it drops that viewer to one tab and the collapse
hides the whole bar again. Recorded on both the subnav entry and get-started.tsx.
* fix(apps): gate the Build apps tab on appBlocksGetStarted, not on nothing
The tab was `visible: () => true` while the whole-bar gate is an OR
(`hasAppsStoreAccess(features) || features.appBlocksGetStarted`). A viewer with
store access but WITHOUT the get-started flag therefore passed the gate and was
offered a "Build apps" tab whose page answers 404 -- `resolveGetStartedAccess`
returns `{ notFound: true }` and the client body renders `<NotFound/>`.
Two consequences, both real:
1. A non-mod `app-dev-testers` member holds `appBlocks` (so `hasAppsStoreAccess`)
but not `appBlocksGetStarted`, so they were offered a 404. Before this branch
they had no route to that page at all -- the dropdown row was correctly gated
on `appsNav.getStarted`.
2. Flipping `app-blocks-get-started` OFF in Flipt -- the flag's stated purpose as
a kill switch -- no longer removed the nav entry, because every mod holds
`appBlocks` and the tab was unconditional. Section 4 of `IframeHost.tsx` cites
exactly that hazard as the reason to EXCLUDE the route from the app-block
chrome nav; the subnav was doing the thing that argument forbids.
The fix follows the pattern already established for `isAuthor`:
`AppsNavContext` gains `canGetStarted`, the container derives it from
`features.appBlocksGetStarted`, and the tab reads `visible: (_s, c) =>
c.canGetStarted`. The flag is verified NOT `toggleable` (absent from
`computeUserFeatureFlagsOverlay`; defined at feature-flags.service.ts:561), so it
is SSR-frozen and safe outside the `useIsClient` deferral -- the same argument
the branch already makes for the bar gate.
`canGetStarted` is deliberately NOT session-scoped, unlike `isAuthor`:
`resolveGetStartedAccess` reads the flag and consults no user, so folding it into
the `currentUser` branch would hide the tab from a logged-out viewer the page
would serve, and (Marketplace being their only other tab) the `< 2` collapse
would then hide the whole bar from them.
Resulting behaviour, each pinned by a test:
- store access only -> Marketplace alone, `< 2` collapse, no bar
= exactly main's behaviour, no new regression
- appBlocksGetStarted only -> Build apps + Marketplace, bar renders
- both -> both tabs
- logged out, no flag -> no bar; logged out WITH the flag -> Build apps
Because the collapse is reachable again, the test blocks the previous round had
to relabel as "pinning the view, not a cohort" are relabelled back to what they
are: coverage of a live cohort. Mutation-checked -- deleting `links.length < 2`
turns 6 of them red.
Also in this commit:
* chromeNavAlignsWithSubNav.test.ts: the exclusion ledger's parse controls sat on
the live counts (`toBe(4)` / `>= 8`), so they STOLE the ledger's failure.
Adding a chrome entry failed on `expect(inChrome.size, 'parsed no items out of
the chrome platform nav').toBe(4)` -- a message that states the opposite of
what happened -- and the `toEqual` ledger never ran, leaving its SHRINK
direction unproven. All three count controls are now `>= 1` and the ledgers own
their sets. Re-mutated both ways: GROW and SHRINK now each die on the ledger's
own assertion and message.
* The two `/apps/*/edit.tsx` notes and the `get-started.tsx` referent they point
at contradicted each other after this branch. Repaired against the code: the
empty-band case is closed for `/apps/get-started` and still open for the other
pages, and the tab enumeration now includes the get-started capability.
* AppsPageLayout.geometry.browser.test.tsx: dropped the computed-but-unasserted
`tabWidth`, put every element lookup behind a `required()` helper that fails
with the actual diagnosis instead of a bare TypeError, and rewrote the stale
header note from the post-fix code.
* preview-apps-marketplace.spec.ts documented "first tab is Marketplace
(AppsSubNav.tsx:55)" -- both halves wrong. Now names the row it keys on and
says why order is irrelevant to the assertion.
* hooks.tsx: recorded the label/destination decision for the get-started-only
cohort ("Apps" + plug glyph, navigating to onboarding) rather than leaving it
undocumented, with the trigger to re-decide it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SzWdpMmKwGdvb2K3eSua8h
* docs(apps): the hydration note now covers all three SSR-frozen inputs
It said "Both inputs" and enumerated only `appBlocksAuthor` and
`currentUser.isModerator`; the context object now derives a third,
`canGetStarted`, from `features.appBlocksGetStarted`. Same argument, same
evidence (not `toggleable`, so the client overlay cannot move it) — the comment
just did not say so.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SzWdpMmKwGdvb2K3eSua8h
* test(apps): own parseAllChromeLinks' route set with a ledger; correct two decision records
Round 3. Round 2 relaxed three count controls in
chromeNavAlignsWithSubNav.test.ts from exact counts to
toBeGreaterThanOrEqual(1). Two of those were right — their sets are still
owned by a toEqual elsewhere (the exact 4-item platform nav; the exact
excluded set). The third was not: nothing owned parseAllChromeLinks' set,
and its comment claimed "same reasoning as the two above", which was false.
The gap was live, not theoretical. Rules (a) and (b) in that test are
per-link, so a new chrome item pointing at a route SUB_NAV_LINKS already
carries, wearing that row's own glyph, satisfies both. Measured on the
pre-fix tree: adding a "Build apps" ChromeSurfaceItem for /apps/get-started
to the ⋮ overflow — outside the platform-nav slice the other ledgers
enumerate — passed all 8 tests. That is exactly the hazard the DELIBERATE
SUBSET note in IframeHost.tsx argues disqualifies the route: this surface
has no feature-flag plumbing, so it would keep advertising a page that
answers notFound once appBlocksGetStarted goes down.
Fix: add (d), a toEqual ledger over the whole literal-href set, sorted (so a
reorder cannot report a route change that did not happen) and keeping
duplicates (/apps/installed legitimately appears twice). Placed after
(a)/(b)/(c) so the more specific rules keep their own messages. toBe(5) is
deliberately NOT restored — it misdirected, telling a maintainer who added a
legitimate destination that the scanner had broken.
Mutation matrix (unit tier, isolated to one hunk each):
overflow-grow (add /apps/get-started to the ⋮ overflow)
pre-fix : 8 passed — SURVIVED
post-fix: 1 failed | 7 passed — dies on (d)'s own message,
"the set of routes the app-block chrome links to has changed…:
expected [ '/apps', '/apps/get-started', …(4) ] to deeply
equal [ '/apps', '/apps/installed', …(3) ]"
(a) and (b) pass, so (d) is the only thing that catches it.
platform-shrink-review (delete the Review item)
(d) fires with its own message (…(2) vs …(3)). Three other tests also
fire, which is correct — a platform-nav item is owned by them too.
overflow-shrink (delete "Manage apps")
dies on (c), which precedes (d) in the same test. Recorded honestly:
every EXISTING link is already owned by some assertion for SHRINK, so
(d)'s unique contribution is GROW outside the platform-nav slice.
Re-confirmed, no regression, both pre-existing ledgers still die on their
own toEqual:
exclusion ledger GROW (new SUB_NAV_LINKS row) → 1 failed | 7 passed
exclusion ledger SHRINK (add get-started to the platform nav) → fires
expected-glyphs GROW (same mutant) → fires
expected-glyphs SHRINK (delete Review) → fires
Also corrected two decision records that claimed more than the repo can
establish:
- The comment above the relaxed floor no longer says "same reasoning as the
two above". It now says this floor has NO sibling toEqual to inherit its
set from, and points at (d).
- hooks.tsx: the re-decide trigger was narrower than the branch's
reachability. The branch is `!marketplace && getStarted`, so the store
flags NARROWING (app-blocks-enabled / app-listings turned off in Flipt
while get-started stays on) reaches it just as widening get-started does —
every moderator would then see a row labelled "Apps" with
IconPlugConnected navigating to developer onboarding. Both directions are
now named. The stated ground was also a live-Flipt claim presented as
derived from `availability`; since getFeatureFlags returns Flipt's answer
before evaluating roles, `availability` is only the Flipt-down fallback, so
the cohort's emptiness is observable only in live Flipt. Said so. All four
App-Blocks flags verified Flipt-backed (feature-flags.service.ts:510, 520,
561, 571). Comment-only; no behaviour change.
Verification (branch worktree, instruments validated before each zero):
- vitest --project unit <this file> → 8 passed; whole AppLayout+guard → 32 passed
- vitest --project component (unfiltered) → 217 files, 2403 tests, all passed
- node scripts/typecheck.mjs → OK — 0 type errors in 81s. Positive control:
the identical command minutes earlier reported 2848 errors off a Prisma
client generated 2025-12-08; `prisma generate` under the flake's engine
paths takes it to 0.
- eslint on both files → 0. Positive control: src/utils/zod-helpers.ts
appended reports 6 problems. Array-quoted, file count read back as 2.
- prettier --check → clean. Negative control: a misformatted scratch file
reports "Code style issues found".
- Merged tree vs current origin/main (d6e5c4eec0, which moved #4666 into
IframeHost.tsx — the very file this ledger scans): the guard, including
(d), passes against the merged sources. gh reports MERGEABLE / CLEAN.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SzWdpMmKwGdvb2K3eSua8h
* test(apps): tighten (d)`s note — (a)/(b) do catch a WRONG added link
The topic sentence said (a) and (b) "structurally cannot see an ADDITION",
then immediately qualified it to the case that matters. The unqualified half
was broader than the truth: (a) does catch an invented route, and (b) a store
route drawn with the wrong glyph. What they cannot see is an addition that is
itself well-formed — a route SUB_NAV_LINKS already carries, under that row`s
own icon — which is the gap (d) exists for. Says that now.
Comment-only. vitest --project unit <this file> -> 8 passed; eslint 0;
prettier clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SzWdpMmKwGdvb2K3eSua8h
* test(apps): widen the chrome-link parser so (d) owns the set its message claims
F1. `parseAllChromeLinks` matched `<ChromeSurfaceItem …>text</…>` and dropped
anything lacking BOTH a literal href AND a `leftSection={<IconX`. Two ordinary
shapes walked through, both measured SURVIVING on 136fffd647 (8 passed):
P1 `<ActionIcon component={Link} href="/apps/get-started" …>` in the overflow
— the shape the chrome ALREADY uses for its `/apps` back-link;
P2 `<ChromeSurfaceItem href="/apps/get-started">Build apps</ChromeSurfaceItem>`
with no `leftSection` — legal, `ChromeSurface.tsx` types it optional, and
the very element (d) claims to enumerate.
Either is an ungated door into a flag-gated route with every test green. The
parser now scans TAGS rather than one tag name and treats the glyph as optional
metadata rather than a condition of inclusion, so (d) owns the whole literal-href
set: 7 sites, adding the compact back chevron and the breadcrumb crumb. Rule (b)
is scoped to links that HAVE a `leftSection` glyph and says so — an element with
no glyph cannot be drawing the route with the wrong one. Both shapes are pinned
as parser fixtures, plus a negative control that an href nested in another
element's attributes is not attributed to the outer tag.
Post-fix both die on (d)'s own message; (a)/(b)/(c) pass, so (d) is the only
thing that catches them.
F2. (d)'s message and comment claimed the whole chrome "has no feature-flag
plumbing at all — the only condition anywhere on it is `isModerator`". That was
true of the platform-nav SLICE and is false of the surface (d) governs:
`chromeBody()` spans through `ChromeDesktopLeadingGroup`, which renders
`<ChromeReviewMenuItem>` gating on `hasAppsStoreAccess(useOptionalFeatureFlags())`
and again through `useCanReviewListing` -> `resolveClientStoreScope`. Rewritten
from the code: every literal-href item is unconditional except the
moderator-gated `/apps/review`, and the surface CAN read flags — so the ledger
now hands a maintainer three options (exclude, or add it GATED the way that item
is) instead of foreclosing the one the repo already demonstrates.
F3. `hooks.tsx` named two of the three flags behind `marketplace`.
`hasAppsStoreAccess` is `appListings || appBlocks || appListingsPublicExternal`,
so it takes ALL THREE going off to reach the branch — a mod holding
`app-listings-public-external` alone keeps `marketplace === true`. Also scoped
the `availability` sentence: it is the Flipt-DOWN fallback for the ROLE terms
only; env/region/server-colour terms run BEFORE Flipt and Flipt cannot override
them. Accurate for a `['mod']` flag, which is the case at hand.
Comment-only in `hooks.tsx`; no behaviour change anywhere.
* docs(apps): say "gated by no flag", not "rendered unconditionally", in the chrome route ledger
Round-4 audit finding F-1. The ledger's comment and its assertion message both
claimed every literal-href item in the chrome is "rendered UNCONDITIONALLY"
except the moderator-gated /apps/review. Two of the seven are in fact
conditional: the compact back chevron renders only under `compact`
(IframeHost.tsx:762, `compact = isPage && geometry.compact`), and the breadcrumb
crumb only under `isPage` (:1048). Those are the two sites the previous round
added to the set.
The inference the sentence supports is unaffected — neither condition is a
feature flag, and the only useFeatureFlags() call in the file sits outside
chromeBody() — so no detection and no maintainer decision changes. But the
sentence as written was false, and this is the fourth draft of it to overstate
in the same direction: each round rewrote it while fixing the previous round,
and each rewrite widened a scope the code had not widened.
So the correction is the narrow one the code supports ("gated by a FEATURE
FLAG"), and the comment now records the two layout-conditional sites by name
plus the instruction not to reach for "unconditionally" again. Recording the
dead drafts is deliberate: it is what stops a fifth being derived.
Verified: guard 8/8; eslint 0 on the changed file (6-problem positive control on
src/utils/zod-helpers.ts in the same run); prettier clean, with a deliberately
misformatted control watched to fail first.
Ends the audit ladder. Rounds 2-4 changed 0 executable payload lines; the
remaining findings are prose about prose, so further rounds would audit the
ladder rather than the PR.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SzWdpMmKwGdvb2K3eSua8h
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Zachary Lowden
@@ -568,6 +568,18 @@ export function AppBlockChrome({
|
||||
which reads BOTH tables and fails when they drift — including when the
|
||||
subnav changes and this menu does not.
|
||||
|
||||
🔴 THIS SECTION IS A DELIBERATE SUBSET, NOT A MIRROR. The subnav also
|
||||
carries `/apps/get-started` ("Build apps"), `/apps/submit`,
|
||||
`/apps/invites` and `/apps/revenue`, and none of them belong in a menu
|
||||
that opens over a RUNNING app. `/apps/get-started` additionally sits
|
||||
behind the `appBlocksGetStarted` kill switch, which this section has no
|
||||
way to read — every item here is unconditional except Review. (The SUBNAV
|
||||
does read it: its row is `visible: (_s, c) => c.canGetStarted`. That is the
|
||||
difference, and it is the whole argument — a surface that cannot honour a
|
||||
kill switch must not advertise the route it switches off.) That excluded
|
||||
SET is itself asserted by the same guard, so adding a subnav row fails
|
||||
there until someone decides which side it lands on.
|
||||
|
||||
The LABELS are deliberately NOT all identical: the subnav's tabs sit under
|
||||
an "Apps" heading and can afford one-word labels ("Installed", "Review"),
|
||||
whereas these items stand alone over a running app and need the noun
|
||||
|
||||
@@ -200,29 +200,112 @@ function chromeBody(): string {
|
||||
return nextExport === -1 ? rest : rest.slice(0, nextExport);
|
||||
}
|
||||
|
||||
function parseAllChromeLinks(src: string): NavEntry[] {
|
||||
return [...src.matchAll(/<ChromeSurfaceItem\b([\s\S]*?)<\/ChromeSurfaceItem>/g)]
|
||||
.map((m) => {
|
||||
const block = m[1];
|
||||
const href = /href="([^"]+)"/.exec(block)?.[1];
|
||||
const icon = /leftSection=\{<(Icon\w+)/.exec(block)?.[1];
|
||||
if (!href || !icon) return null;
|
||||
let depth = 0;
|
||||
let tagEnd = -1;
|
||||
for (let i = 0; i < block.length; i += 1) {
|
||||
const ch = block[i];
|
||||
if (ch === '{') depth += 1;
|
||||
else if (ch === '}') depth -= 1;
|
||||
else if (ch === '>' && depth === 0) {
|
||||
tagEnd = i;
|
||||
break;
|
||||
}
|
||||
/** A link site in the chrome. `label` and `icon` are OPTIONAL because a real link in
|
||||
* this file may carry neither — see `parseAllChromeLinks`. */
|
||||
type ChromeLink = { tag: string; href: string; label: string | null; icon: string | null };
|
||||
|
||||
/** Drop every `{…}` expression from an attribute list, so `href="…"` is matched only
|
||||
* when it is a LITERAL attribute of this tag. Two things this rules out: a
|
||||
* `href={`/apps/run/${r.blockId}`}` template (not a platform route), and a nested
|
||||
* `someProp={<Foo href="…"/>}` being attributed to the OUTER element. */
|
||||
function stripAttrExpressions(attrs: string): string {
|
||||
let out = '';
|
||||
let depth = 0;
|
||||
for (const ch of attrs) {
|
||||
if (ch === '{') depth += 1;
|
||||
else if (ch === '}') depth -= 1;
|
||||
else if (depth === 0) out += ch;
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
/**
|
||||
* EVERY element in the chrome that carries a LITERAL `href="…"` — whatever tag it is
|
||||
* written as, and whether or not it carries a `leftSection` glyph.
|
||||
*
|
||||
* 🔴 ROUND 4 WIDENED THIS, BECAUSE (d)'s MESSAGE CLAIMED A SET THIS PARSER DID NOT OWN.
|
||||
* The previous version matched `<ChromeSurfaceItem …>text</ChromeSurfaceItem>` and then
|
||||
* dropped anything without BOTH a literal href AND a `leftSection={<IconX`. Two ordinary
|
||||
* shapes walked straight through it, both measured surviving as mutants:
|
||||
*
|
||||
* • `<ActionIcon component={Link} href="/apps/get-started" …>` in the ⋮ overflow — and
|
||||
* that is not an exotic shape, it is the one the chrome ALREADY uses for its `/apps`
|
||||
* back-link (the compact-mode chevron);
|
||||
* • `<ChromeSurfaceItem href="/apps/get-started">Build apps</ChromeSurfaceItem>` with no
|
||||
* `leftSection` — which compiles, because `ChromeSurface.tsx` types that prop
|
||||
* `leftSection?: ReactNode`, and which is the very element (d) claims to enumerate.
|
||||
*
|
||||
* Either one is a door out of a running app into a flag-gated route, offered
|
||||
* unconditionally, with all 8 tests green. So the parser now scans TAGS rather than one
|
||||
* tag name, and treats the glyph as optional metadata rather than a condition of
|
||||
* inclusion. What that costs is stated where it is paid: rule (b) below can only compare
|
||||
* a glyph an element actually HAS, so it now skips the icon-less links that (d) still
|
||||
* counts.
|
||||
*
|
||||
* 🔴 `icon` IS THE `leftSection` GLYPH ONLY — deliberately not "any Icon inside the
|
||||
* element". The back chevron renders `<IconChevronLeft/>` as its CHILD; that is a
|
||||
* directional affordance, not this route's glyph, and scoring it as one would make the
|
||||
* same-route rule red on correct code. The rule (b) enforces is about the picture in a
|
||||
* menu ROW, which is what `leftSection` is.
|
||||
*
|
||||
* Mechanics: an opening tag ends at the first `>` at brace depth 0 (`leftSection={<Icon
|
||||
* … />}` contains a `>` that is not the end of the tag), and scanning resumes AFTER that
|
||||
* `>` — so a nested `<Icon…>` inside the attribute region is never itself opened as an
|
||||
* element. The label is the element's text with nested tags removed, falling back to
|
||||
* `aria-label` (the back chevron has no text at all, and a null label would make (a)/(b)'s
|
||||
* failure messages name nothing). `</tag>` is matched by name, which is exact here because
|
||||
* none of these tags nest inside themselves; if one ever does, the LABEL is what goes
|
||||
* wrong, never the href set (d) owns.
|
||||
*/
|
||||
function parseAllChromeLinks(src: string): ChromeLink[] {
|
||||
const out: ChromeLink[] = [];
|
||||
let i = 0;
|
||||
while (i < src.length) {
|
||||
if (src[i] !== '<' || !/[A-Za-z]/.test(src[i + 1] ?? '')) {
|
||||
i += 1;
|
||||
continue;
|
||||
}
|
||||
const tag = /^[A-Za-z][\w.]*/.exec(src.slice(i + 1))?.[0];
|
||||
if (!tag) {
|
||||
i += 1;
|
||||
continue;
|
||||
}
|
||||
let depth = 0;
|
||||
let tagEnd = -1;
|
||||
for (let j = i + 1 + tag.length; j < src.length; j += 1) {
|
||||
const ch = src[j];
|
||||
if (ch === '{') depth += 1;
|
||||
else if (ch === '}') depth -= 1;
|
||||
else if (ch === '>' && depth === 0) {
|
||||
tagEnd = j;
|
||||
break;
|
||||
}
|
||||
if (tagEnd === -1) return null;
|
||||
const label = block.slice(tagEnd + 1).trim();
|
||||
return label ? { href, label, icon } : null;
|
||||
})
|
||||
.filter((e): e is NavEntry => e !== null);
|
||||
}
|
||||
if (tagEnd === -1) break;
|
||||
const attrs = src.slice(i + 1 + tag.length, tagEnd);
|
||||
const selfClosing = src[tagEnd - 1] === '/';
|
||||
i = tagEnd + 1;
|
||||
|
||||
const href = /\bhref="([^"]+)"/.exec(stripAttrExpressions(attrs))?.[1];
|
||||
if (!href) continue;
|
||||
const icon = /\bleftSection=\{<(Icon\w+)/.exec(attrs)?.[1] ?? null;
|
||||
|
||||
let label: string | null = null;
|
||||
if (!selfClosing) {
|
||||
const close = src.indexOf(`</${tag}>`, tagEnd);
|
||||
if (close !== -1) {
|
||||
label =
|
||||
src
|
||||
.slice(tagEnd + 1, close)
|
||||
.replace(/<[^>]*>/g, '')
|
||||
.trim() || null;
|
||||
}
|
||||
}
|
||||
if (!label) label = /\baria-label="([^"]+)"/.exec(attrs)?.[1] ?? null;
|
||||
|
||||
out.push({ tag, href, label, icon });
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
describe('the app-block chrome platform nav agrees with the store subnav', () => {
|
||||
@@ -284,10 +367,14 @@ describe('the app-block chrome platform nav agrees with the store subnav', () =>
|
||||
const subnav = parseSubNav(code(read(SUBNAV)));
|
||||
const nav = parsePlatformNav(code(read(CHROME)));
|
||||
|
||||
// Both tables were actually read. A zero here is indistinguishable from a
|
||||
// parser wired to nothing, so it must never be the thing that makes this pass.
|
||||
expect(subnav.length, 'parsed no rows out of SUB_NAV_LINKS').toBeGreaterThanOrEqual(7);
|
||||
expect(nav.length, 'parsed no items out of the chrome platform nav').toBe(4);
|
||||
// 🔴 PARSE CONTROLS ONLY — deliberately far below the live counts. A zero is
|
||||
// indistinguishable from a parser wired to nothing, so it must never be the thing
|
||||
// that makes this pass; but a control pinned ON the live count STEALS the failure
|
||||
// from the rule below, reporting a parse problem for what is really a nav change.
|
||||
// The exact chrome set is owned by the `expected glyphs` test; the exact excluded
|
||||
// set by the ledger.
|
||||
expect(subnav.length, 'parsed no rows out of SUB_NAV_LINKS').toBeGreaterThanOrEqual(1);
|
||||
expect(nav.length, 'parsed no items out of the chrome platform nav').toBeGreaterThanOrEqual(1);
|
||||
|
||||
const bySubNavHref = new Map(subnav.map((e) => [e.href, e]));
|
||||
|
||||
@@ -307,6 +394,70 @@ describe('the app-block chrome platform nav agrees with the store subnav', () =>
|
||||
}
|
||||
});
|
||||
|
||||
/**
|
||||
* 🔴 THE EXCLUSION LEDGER — the half this guard could not previously express.
|
||||
*
|
||||
* The rule above is ONE-DIRECTIONAL: every chrome destination must exist in the
|
||||
* subnav, never the reverse. That is correct — the chrome is deliberately a strict
|
||||
* SUBSET (Create / Invites / Revenue have always been subnav-only) — but it means a
|
||||
* new `SUB_NAV_LINKS` row is scored identically whether its absence from the chrome
|
||||
* was a decision or an oversight. Both readings pass, silently, which is the exact
|
||||
* shape of an unpinned decision.
|
||||
*
|
||||
* `/apps/get-started` ("Build apps") is the row that made this worth writing. It was
|
||||
* EXCLUDED from the chrome on purpose, for two reasons:
|
||||
*
|
||||
* 1. AUDIENCE. This menu opens over a RUNNING app — the viewer is consuming an app,
|
||||
* not looking for a developer-onboarding page. The four shared destinations are
|
||||
* the store's consumption + ownership surfaces; a "start building" call to action
|
||||
* is a store-front concern.
|
||||
* 2. GATING. The chrome's platform nav has no feature-flag plumbing at all (its only
|
||||
* condition is `isModerator`), while `/apps/get-started` is governed by the
|
||||
* `appBlocksGetStarted` KILL SWITCH. Mirroring the entry here would render it
|
||||
* unconditionally and defeat the switch — the chrome would keep offering a page
|
||||
* that has been turned off. The subnav's own row IS gated on that flag
|
||||
* (`visible: (_s, c) => c.canGetStarted`), which is what makes this a difference
|
||||
* between the two surfaces rather than an inconsistency.
|
||||
*
|
||||
* So this asserts the excluded SET, and fails when it GROWS (a new subnav row nobody
|
||||
* decided about) or SHRINKS (an entry added to the chrome without updating the note).
|
||||
* It is the deliberate alternative to loosening the guard.
|
||||
*/
|
||||
it('🔴 the subnav rows deliberately ABSENT from the chrome are exactly these', () => {
|
||||
const subnav = parseSubNav(code(read(SUBNAV)));
|
||||
const nav = parsePlatformNav(code(read(CHROME)));
|
||||
const inChrome = new Set(nav.map((e) => e.href));
|
||||
|
||||
// 🔴 PARSE CONTROLS, NOT LEDGERS — and the floors are deliberately far below the
|
||||
// live counts (8 and 4). Pinned ON those counts they MASK the ledger they were
|
||||
// meant to protect: adding a chrome item made `toBe(4)` fail first, with the
|
||||
// message "parsed no items out of the chrome platform nav: expected 5 to be 4" —
|
||||
// which states the opposite of what happened — and the `toEqual` below never ran,
|
||||
// leaving the ledger's SHRINK direction unproven. A zero on either side would make
|
||||
// the difference below trivially "everything" or "nothing", and that is all these
|
||||
// two are here to rule out; the SET is the `toEqual`'s to own, in both directions.
|
||||
expect(subnav.length, 'parsed no rows out of SUB_NAV_LINKS').toBeGreaterThanOrEqual(1);
|
||||
expect(inChrome.size, 'parsed no items out of the chrome platform nav').toBeGreaterThanOrEqual(
|
||||
1
|
||||
);
|
||||
|
||||
expect(
|
||||
subnav.map((e) => e.href).filter((h) => !inChrome.has(h)),
|
||||
'a store subnav destination is missing from the app-block chrome. If that is ' +
|
||||
'deliberate, add it here WITH the reason; if it is not, add it to the chrome ' +
|
||||
'(the subnav is the source of truth, so the chrome follows).'
|
||||
).toEqual([
|
||||
// Developer onboarding — a store-front concern, and flag-gated by a kill switch
|
||||
// the chrome cannot honour. See the block comment above.
|
||||
'/apps/get-started',
|
||||
// Authoring + owner-management surfaces. Pre-existing exclusions: the chrome is
|
||||
// navigation for someone RUNNING an app, not managing one.
|
||||
'/apps/submit',
|
||||
'/apps/invites',
|
||||
'/apps/revenue',
|
||||
]);
|
||||
});
|
||||
|
||||
it('the four shared destinations are the expected ones, drawn with the expected glyphs', () => {
|
||||
// The relationship test above is the real guard; this one names the resolved
|
||||
// values so a failure reads as a diff rather than sending you to two files.
|
||||
@@ -357,7 +508,18 @@ describe('the app-block chrome platform nav agrees with the store subnav', () =>
|
||||
const links = parseAllChromeLinks(chromeBody());
|
||||
const bySubNavHref = new Map(subnav.map((e) => [e.href, e]));
|
||||
|
||||
expect(links.length, 'parsed no literal-href items out of the chrome').toBe(5);
|
||||
// 🔴 PARSE CONTROL ONLY — AND, UNLIKE THE OTHER TWO FLOORS IN THIS FILE, IT HAS NO
|
||||
// SIBLING `toEqual` ELSEWHERE TO INHERIT ITS SET FROM. The floors in the two tests
|
||||
// above are safe to relax because something else owns their sets outright (the exact
|
||||
// 4-item platform nav; the exact excluded set). Nothing owned THIS parser's set, so
|
||||
// relaxing this one alone deleted a live detection — see (d), which was added to
|
||||
// restore it. A zero here is still indistinguishable from a regex wired to nothing,
|
||||
// and ruling that out is all this floor does; pinned on the live count it would
|
||||
// instead steal the failure from the rules below and report a nav change as a parse
|
||||
// error.
|
||||
expect(links.length, 'parsed no literal-href items out of the chrome').toBeGreaterThanOrEqual(
|
||||
1
|
||||
);
|
||||
|
||||
// (a) Every route the chrome links to is a route the store actually has.
|
||||
for (const link of links) {
|
||||
@@ -370,7 +532,13 @@ describe('the app-block chrome platform nav agrees with the store subnav', () =>
|
||||
}
|
||||
|
||||
// (b) …and draws it with the store's glyph, wherever in the chrome it appears.
|
||||
for (const link of links) {
|
||||
//
|
||||
// 🔴 SCOPED TO THE LINKS THAT HAVE A `leftSection` GLYPH, which since round 4 is a
|
||||
// SUBSET of `links` rather than all of it. An element with no glyph cannot be drawing
|
||||
// the route with the WRONG one, so there is nothing here to compare; its presence in
|
||||
// the chrome is still owned outright by (d). The two live examples are the compact
|
||||
// back chevron and the breadcrumb crumb, neither of which is a menu row.
|
||||
for (const link of links.filter((l) => l.icon !== null)) {
|
||||
expect(
|
||||
link.icon,
|
||||
`same-route icon drift: "${link.label}" links to \`${link.href}\` with \`${link.icon}\`, ` +
|
||||
@@ -388,6 +556,75 @@ describe('the app-block chrome platform nav agrees with the store subnav', () =>
|
||||
'their icons, not about removing one of them (that would be a behaviour change).'
|
||||
).toEqual(['Installed apps', 'Manage apps']);
|
||||
expect(new Set(installed.map((l) => l.icon)).size).toBe(1);
|
||||
|
||||
// (d) 🔴 THE LEDGER — the SET of routes the chrome links to, owned outright, so this
|
||||
// fails when it GROWS or SHRINKS.
|
||||
//
|
||||
// (a) and (b) are PER-LINK, so they catch an addition only when the added link is
|
||||
// itself wrong — an invented route, or the store's route under the wrong glyph. A new
|
||||
// item pointing at a route `SUB_NAV_LINKS` already carries, wearing that row's own
|
||||
// glyph, satisfies both and is invisible to them. Nor does anything else here close
|
||||
// the gap — the `expected glyphs` test enumerates the PLATFORM-NAV slice only, and (c)
|
||||
// enumerates the two `/apps/installed` LABELS only. So an item added to the ⋮ overflow
|
||||
// was invisible to every assertion in this file. Measured on the PRE-(d) tree: adding
|
||||
// `<ChromeSurfaceItem href="/apps/get-started" leftSection={<IconCode …/>}>Build
|
||||
// apps</ChromeSurfaceItem>` to the overflow passed all 8 tests. (d) kills that one now.
|
||||
//
|
||||
// 🔴 AND (d) ALONE WAS STILL NOT ENOUGH, WHICH IS WHY THIS COMMENT IS NOT THE END OF
|
||||
// THE STORY. Round 4 measured two FURTHER shapes surviving with (d) in place, because
|
||||
// the PARSER feeding it required both a literal `href` and a `leftSection` glyph — an
|
||||
// `<ActionIcon component={Link} href>` and a `leftSection`-less `<ChromeSurfaceItem>`.
|
||||
// Both are ordinary; the first is the shape the chrome already uses for its `/apps`
|
||||
// back-link. The ledger is only ever as wide as `parseAllChromeLinks` — read its
|
||||
// header before trusting the sentence below.
|
||||
//
|
||||
// That is not bookkeeping. `/apps/get-started` is governed by the `appBlocksGetStarted`
|
||||
// kill switch, and NO literal-href item in this chrome is gated by a FEATURE FLAG
|
||||
// except the moderator-gated `/apps/review` — no flag decides whether a LINK here is
|
||||
// offered. (Two ARE conditional, on LAYOUT rather than a flag: the compact back
|
||||
// chevron renders only under `compact`, the breadcrumb crumb only under `isPage`.
|
||||
// Neither can be switched off in Flipt, which is what this argument turns on — so
|
||||
// say "gated by no flag", never "rendered unconditionally". Four earlier drafts of
|
||||
// this sentence overstated it in exactly that way.)
|
||||
// The DELIBERATE SUBSET note above the platform nav in
|
||||
// `IframeHost.tsx` is the argument that a surface which does not honour a kill switch
|
||||
// must not advertise the route it switches off, and the overflow is the same surface,
|
||||
// so the same argument governs it; only the enumeration stopped short.
|
||||
//
|
||||
// 🔴 THAT IS NOT "THIS SURFACE CANNOT READ FLAGS" — it can, and the repo demonstrates
|
||||
// it a few lines away. `<ChromeReviewMenuItem>` (`IframeHost.tsx`) calls
|
||||
// `useOptionalFeatureFlags()` and returns null without `hasAppsStoreAccess(features)`,
|
||||
// and its `useCanReviewListing` narrows again through `resolveClientStoreScope`. It
|
||||
// carries no `href`, which is why it is not in this set — and it is exactly the shape a
|
||||
// gated door WOULD take. So a maintainer this ledger stops has two honest options, not
|
||||
// one: exclude the route, or add it gated the way that item is. What is not an option
|
||||
// is adding it as a plain link.
|
||||
//
|
||||
// SORTED, so a re-ORDER cannot report a route change that did not happen — this rule
|
||||
// is about the SET, and the platform-nav slice's own `toEqual` is what governs order
|
||||
// there. DUPLICATES KEPT: `/apps` and `/apps/installed` each legitimately appear more
|
||||
// than once, and collapsing to a Set would hide an extra item hung on a listed route.
|
||||
expect(
|
||||
links.map((l) => l.href).sort(),
|
||||
'the set of routes the app-block chrome links to has changed. Adding one is a ' +
|
||||
'product decision rather than a detail: NO literal-href item in this chrome is ' +
|
||||
'gated by a FEATURE FLAG except the moderator-gated `/apps/review`, so a ' +
|
||||
'flag-gated destination added here as a plain link keeps being offered after its ' +
|
||||
'flag goes down (that is why `/apps/get-started` is excluded; see the DELIBERATE ' +
|
||||
'SUBSET note in `IframeHost.tsx`). The surface CAN read flags — ' +
|
||||
'`ChromeReviewMenuItem` gates itself on `hasAppsStoreAccess(useOptionalFeatureFlags())` ' +
|
||||
'— so "add it GATED, the way that item is" is a real third option alongside ' +
|
||||
'excluding it; adding it ungated is not. Removing one deletes a door out of a ' +
|
||||
'running app. Update this list deliberately, WITH the reason.'
|
||||
).toEqual([
|
||||
'/apps', // Marketplace — platform nav
|
||||
'/apps', // the compact back chevron — `<ActionIcon component={Link}>`, no leftSection
|
||||
'/apps', // the breadcrumb's first crumb — `<Anchor component={Link}>`, no leftSection
|
||||
'/apps/installed', // Installed apps — platform nav
|
||||
'/apps/installed', // Manage apps — ⋮ overflow; the pair (c) governs their labels
|
||||
'/apps/mine', // My apps — platform nav
|
||||
'/apps/review', // Review — platform nav, moderator-gated
|
||||
]);
|
||||
});
|
||||
|
||||
it('the whole-chrome parser sees BOTH sections — positive control', () => {
|
||||
@@ -408,6 +645,41 @@ describe('the app-block chrome platform nav agrees with the store subnav', () =>
|
||||
'<ChromeSurfaceItem href={`/apps/run/${r.blockId}`} leftSection={<IconApps />}>x</ChromeSurfaceItem>'
|
||||
)
|
||||
).toHaveLength(0);
|
||||
|
||||
// 🔴 THE TWO SHAPES ROUND 4 ADDED, AS FIXTURES. Both were measured SURVIVING as
|
||||
// mutants against the pre-round-4 parser (8 passed, `/apps/get-started` invisible to
|
||||
// every assertion in this file). A widened parser that silently stopped matching them
|
||||
// again would restore that hole while looking exactly like this — so the shapes are
|
||||
// pinned here, not merely described in the comment above.
|
||||
//
|
||||
// P1: the `<ActionIcon component={Link} href>` shape, which the chrome ALREADY uses
|
||||
// for its `/apps` back-link. No text child and no `leftSection`, so the label comes
|
||||
// from `aria-label` and the icon is null.
|
||||
expect(
|
||||
parseAllChromeLinks(
|
||||
'<ActionIcon component={Link} href="/apps/get-started" aria-label="Build apps">' +
|
||||
'<IconCode size={16} stroke={1.5} /></ActionIcon>'
|
||||
)
|
||||
).toEqual([{ tag: 'ActionIcon', href: '/apps/get-started', label: 'Build apps', icon: null }]);
|
||||
|
||||
// P2: a `ChromeSurfaceItem` with NO `leftSection` — legal, because the primitive types
|
||||
// it `leftSection?: ReactNode`. The old parser required the glyph for INCLUSION.
|
||||
expect(
|
||||
parseAllChromeLinks(
|
||||
'<ChromeSurfaceItem href="/apps/get-started">Build apps</ChromeSurfaceItem>'
|
||||
)
|
||||
).toEqual([
|
||||
{ tag: 'ChromeSurfaceItem', href: '/apps/get-started', label: 'Build apps', icon: null },
|
||||
]);
|
||||
|
||||
// …and the negative control for `stripAttrExpressions`: an href nested inside ANOTHER
|
||||
// element in the attribute region is that element's, not this one's. Without the
|
||||
// strip, the outer tag would be reported as linking to `/apps/get-started`.
|
||||
expect(
|
||||
parseAllChromeLinks(
|
||||
'<ChromeSurfaceItem leftSection={<Foo href="/apps/get-started" />}>x</ChromeSurfaceItem>'
|
||||
)
|
||||
).toEqual([]);
|
||||
});
|
||||
|
||||
it('the breadcrumb’s first crumb reads "Marketplace" and still links to /apps', () => {
|
||||
|
||||
@@ -0,0 +1,274 @@
|
||||
import fs from 'fs';
|
||||
import path from 'path';
|
||||
import ts from 'typescript';
|
||||
import { describe, expect, it } from 'vitest';
|
||||
|
||||
/**
|
||||
* 🔒 THE USER-MENU `/apps` ENTRY — ONE ROW, TWO DESTINATIONS.
|
||||
*
|
||||
* WHAT CHANGED. `useGetMenuItems` used to carry TWO adjacent App-Blocks rows:
|
||||
* "Build apps" → `/apps/get-started` (gated on `appsNav.getStarted`) and "Apps" →
|
||||
* `/apps` (gated on `appsNav.marketplace`). A moderator holds both flags, so they
|
||||
* saw two near-identical rows for one product. "Build apps" moved into the shared
|
||||
* `/apps/*` sub-nav (`SUB_NAV_LINKS` in `~/components/Apps/AppsSubNav`), leaving
|
||||
* ONE dropdown row whose href is chosen from the same two booleans:
|
||||
*
|
||||
* visible = marketplace || getStarted
|
||||
* href = marketplace ? '/apps' : '/apps/get-started'
|
||||
*
|
||||
* The fallback is not cosmetic. A viewer holding `appBlocksGetStarted` WITHOUT a
|
||||
* store flag cannot load `/apps` at all — its `getServerSideProps` runs
|
||||
* `resolveAppsPageAccess`, which returns `notFound`. Sending them at `/apps` would
|
||||
* be a menu entry into a 404.
|
||||
*
|
||||
* 🔴 WHY A SOURCE SCAN RATHER THAN A RENDER. `useGetMenuItems` is a heavy hook —
|
||||
* router, session, Mantine theme, tRPC — and the menu table is a 40-entry literal
|
||||
* inside it. Mounting it to observe two expressions would drag that whole graph
|
||||
* into the node `unit` project (the tier that BLOCKS) for no gain, and the browser
|
||||
* tier that could mount it is report-only. `appsStoreAccessCallSites.test.ts` names
|
||||
* this exact seam as untested: "that `useGetMenuItems` hands it the real
|
||||
* `useFeatureFlags()` object and wires `appsNav.marketplace` to the right menu
|
||||
* item". This closes the second half of it.
|
||||
*
|
||||
* 🔴 AND IT IS BEHAVIOURAL, NOT A SPELLING CHECK. The two expressions are EXTRACTED
|
||||
* from the real source and then EVALUATED against every combination of the two
|
||||
* booleans. A reworded-but-equivalent implementation passes; a wrong one fails. A
|
||||
* guard that merely grepped for the literal ternary would be satisfied by the text
|
||||
* and blind to `appsNav.getStarted ? '/apps' : …`.
|
||||
*/
|
||||
|
||||
const HOOKS = path.resolve(__dirname, 'hooks.tsx');
|
||||
|
||||
function read(file: string): string {
|
||||
// Prove the path before trusting any "no match" below: a scan of an absent file
|
||||
// finds zero entries, which would read as "exactly one" failing for the wrong
|
||||
// reason — or, for a `length >= 0` style check, as a clean pass.
|
||||
expect(fs.existsSync(file), `${file} does not exist`).toBe(true);
|
||||
return fs.readFileSync(file, 'utf8');
|
||||
}
|
||||
|
||||
type MenuEntry = {
|
||||
/** Source text of the `href:` initializer, e.g. `"appsNav.marketplace ? '/apps' : …"`. */
|
||||
href: string;
|
||||
/** Source text of the `visible:` initializer, or `null` when the row has none. */
|
||||
visible: string | null;
|
||||
/** Source text of the `label:` initializer, quotes included. */
|
||||
label: string | null;
|
||||
};
|
||||
|
||||
/**
|
||||
* Every object literal in `hooks.tsx` that has an `href` property, as source text.
|
||||
*
|
||||
* Uses the real TypeScript parser rather than a regex: the menu table nests object
|
||||
* literals inside array literals inside a hook body, several rows carry template
|
||||
* literals and ternaries, and a brace-counting scanner would drop rows silently —
|
||||
* the failure mode that makes a "no match" indistinguishable from a clean result.
|
||||
*/
|
||||
function parseMenuEntries(source: string, fileName = 'hooks.tsx'): MenuEntry[] {
|
||||
const sf = ts.createSourceFile(fileName, source, ts.ScriptTarget.Latest, true, ts.ScriptKind.TSX);
|
||||
const out: MenuEntry[] = [];
|
||||
|
||||
const prop = (obj: ts.ObjectLiteralExpression, name: string): string | null => {
|
||||
for (const member of obj.properties) {
|
||||
if (
|
||||
ts.isPropertyAssignment(member) &&
|
||||
(ts.isIdentifier(member.name) || ts.isStringLiteral(member.name)) &&
|
||||
member.name.text === name
|
||||
) {
|
||||
return member.initializer.getText(sf);
|
||||
}
|
||||
}
|
||||
return null;
|
||||
};
|
||||
|
||||
const visit = (node: ts.Node) => {
|
||||
if (ts.isObjectLiteralExpression(node)) {
|
||||
const href = prop(node, 'href');
|
||||
if (href !== null)
|
||||
out.push({ href, visible: prop(node, 'visible'), label: prop(node, 'label') });
|
||||
}
|
||||
node.forEachChild(visit);
|
||||
};
|
||||
visit(sf);
|
||||
return out;
|
||||
}
|
||||
|
||||
/** Rows whose href expression mentions an `/apps` route, in source order. */
|
||||
function appsEntries(entries: MenuEntry[]): MenuEntry[] {
|
||||
return entries.filter((e) => /['"`]\/apps(\/|['"`])/.test(e.href));
|
||||
}
|
||||
|
||||
/**
|
||||
* Evaluate an extracted expression against a stub `appsNav`.
|
||||
*
|
||||
* A `ReferenceError` here means the expression grew a dependency this test does not
|
||||
* model (e.g. it started reading `features` directly instead of the pure
|
||||
* `appsNavVisibility` helper), which is itself worth failing on — so the error is
|
||||
* re-thrown with the offending source rather than swallowed.
|
||||
*/
|
||||
function evaluate(expr: string, appsNav: { marketplace: boolean; getStarted: boolean }): unknown {
|
||||
try {
|
||||
return new Function('appsNav', `return (${expr});`)(appsNav);
|
||||
} catch (err) {
|
||||
throw new Error(
|
||||
`could not evaluate the extracted expression against a stub \`appsNav\`:\n ${expr}\n` +
|
||||
`If the entry now reads something other than \`appsNav\`, this test must be ` +
|
||||
`re-pointed — do not delete it.\n cause: ${String(err)}`
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
const CASES = [
|
||||
{ marketplace: true, getStarted: true },
|
||||
{ marketplace: true, getStarted: false },
|
||||
{ marketplace: false, getStarted: true },
|
||||
{ marketplace: false, getStarted: false },
|
||||
] as const;
|
||||
|
||||
describe('the extractor (validate the instrument before reading its verdict)', () => {
|
||||
it('🔴 POSITIVE CONTROL: it parses a table shaped like the real one', () => {
|
||||
const entries = parseMenuEntries(
|
||||
`
|
||||
const items = [
|
||||
{ href: '/user/vault', visible: features.vault, icon: IconCloudLock, label: 'My Vault' },
|
||||
{
|
||||
// a comment mentioning href: '/apps/decoy' which must not be parsed
|
||||
href: appsNav.marketplace ? '/apps' : '/apps/get-started',
|
||||
visible: appsNav.marketplace || appsNav.getStarted,
|
||||
icon: IconPlugConnected,
|
||||
label: 'Apps',
|
||||
},
|
||||
];
|
||||
`,
|
||||
'probe.tsx'
|
||||
);
|
||||
expect(entries).toHaveLength(2);
|
||||
expect(appsEntries(entries)).toHaveLength(1);
|
||||
expect(appsEntries(entries)[0].label).toBe(`'Apps'`);
|
||||
});
|
||||
|
||||
it('🔴 NEGATIVE CONTROL: it reports TWO when the table carries two /apps rows', () => {
|
||||
// The pre-change shape. Without this, "exactly one" could be satisfied by a
|
||||
// parser that only ever finds one thing.
|
||||
const entries = appsEntries(
|
||||
parseMenuEntries(
|
||||
`
|
||||
const items = [
|
||||
{ href: '/apps/get-started', visible: appsNav.getStarted, label: 'Build apps' },
|
||||
{ href: '/apps', visible: appsNav.marketplace, label: 'Apps' },
|
||||
];
|
||||
`,
|
||||
'probe.tsx'
|
||||
)
|
||||
);
|
||||
expect(entries).toHaveLength(2);
|
||||
expect(entries.map((e) => e.label)).toEqual([`'Build apps'`, `'Apps'`]);
|
||||
});
|
||||
|
||||
it('does NOT read a lookalike route as an /apps route', () => {
|
||||
// `/appsomething` and `/user/apps` are not the store. A `\/apps/`-prefix test
|
||||
// without the boundary would claim both.
|
||||
const entries = appsEntries(
|
||||
parseMenuEntries(
|
||||
`const items = [{ href: '/appsomething' }, { href: '/user/apps' }];`,
|
||||
'probe.tsx'
|
||||
)
|
||||
);
|
||||
expect(entries).toEqual([]);
|
||||
});
|
||||
|
||||
it('the evaluator really evaluates (it is not returning the source text)', () => {
|
||||
expect(
|
||||
evaluate(`appsNav.marketplace ? '/apps' : '/apps/get-started'`, {
|
||||
marketplace: true,
|
||||
getStarted: false,
|
||||
})
|
||||
).toBe('/apps');
|
||||
expect(
|
||||
evaluate(`appsNav.marketplace ? '/apps' : '/apps/get-started'`, {
|
||||
marketplace: false,
|
||||
getStarted: true,
|
||||
})
|
||||
).toBe('/apps/get-started');
|
||||
expect(
|
||||
evaluate(`appsNav.marketplace || appsNav.getStarted`, {
|
||||
marketplace: false,
|
||||
getStarted: false,
|
||||
})
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it('the real hooks.tsx yields a plausible number of menu rows', () => {
|
||||
// A floor well below the live count. Its job is to prove the walk reached the
|
||||
// real table, so that every count below is a fact about the code rather than
|
||||
// about a parser that returned nothing.
|
||||
expect(parseMenuEntries(read(HOOKS)).length).toBeGreaterThanOrEqual(20);
|
||||
});
|
||||
});
|
||||
|
||||
describe('🔒 the user menu offers exactly ONE /apps entry', () => {
|
||||
const entries = appsEntries(parseMenuEntries(read(HOOKS)));
|
||||
|
||||
it('🔴 exactly one — the two adjacent rows were consolidated', () => {
|
||||
expect(
|
||||
entries.map((e) => `${e.label} → ${e.href}`),
|
||||
'the user-menu dropdown must carry a single `/apps*` row. Two rows for one ' +
|
||||
'product is the duplication this change removed; adding a second one back ' +
|
||||
'needs a deliberate decision, not a merge.'
|
||||
).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('is labelled "Apps"', () => {
|
||||
expect(entries[0].label).toBe(`'Apps'`);
|
||||
});
|
||||
|
||||
it('🔴 href: /apps WITH store access, /apps/get-started WITHOUT', () => {
|
||||
const href = (nav: (typeof CASES)[number]) => evaluate(entries[0].href, nav);
|
||||
|
||||
// Store access wins regardless of the get-started flag — the marketplace is the
|
||||
// richer landing and "Build apps" is one click away in the sub-nav.
|
||||
expect(href({ marketplace: true, getStarted: true })).toBe('/apps');
|
||||
expect(href({ marketplace: true, getStarted: false })).toBe('/apps');
|
||||
|
||||
// 🔴 THE CASE THE FALLBACK EXISTS FOR. This viewer cannot load `/apps`:
|
||||
// `resolveAppsPageAccess` returns `notFound` without a store flag. A ternary
|
||||
// written the other way round, or a plain `'/apps'`, fails here.
|
||||
expect(href({ marketplace: false, getStarted: true })).toBe('/apps/get-started');
|
||||
});
|
||||
|
||||
it('🔴 visible: whenever EITHER destination is reachable, and never otherwise', () => {
|
||||
const visible = entries[0].visible;
|
||||
expect(
|
||||
visible,
|
||||
'the entry must carry a `visible` predicate — an absent one is always shown'
|
||||
).not.toBeNull();
|
||||
|
||||
for (const nav of CASES) {
|
||||
expect(
|
||||
evaluate(visible as string, nav),
|
||||
`visible for marketplace=${nav.marketplace} getStarted=${nav.getStarted}`
|
||||
).toBe(nav.marketplace || nav.getStarted);
|
||||
}
|
||||
});
|
||||
|
||||
it('the href it resolves to is one the same flags actually admit', () => {
|
||||
// The relationship, not the two expressions separately: for every combination in
|
||||
// which the row is shown, the destination must be one the viewer's flags permit.
|
||||
// This is what a pair of independently-correct-looking expressions can still get
|
||||
// wrong — visible on `||`, href hardcoded to `/apps`.
|
||||
for (const nav of CASES) {
|
||||
if (!evaluate(entries[0].visible as string, nav)) continue;
|
||||
const href = evaluate(entries[0].href, nav);
|
||||
const admitted = href === '/apps' ? nav.marketplace : nav.getStarted;
|
||||
expect(admitted, `the row links to ${String(href)} for a viewer who cannot load it`).toBe(
|
||||
true
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
it('the expired `newUntil` badge from the retired "Build apps" row is gone', () => {
|
||||
// The consolidated row keeps the marketplace row's own 2026-07-01 badge; the
|
||||
// 2026-08-01 one belonged to the row that no longer exists.
|
||||
expect(read(HOOKS)).not.toContain("newUntil: new Date('2026-08-01')");
|
||||
});
|
||||
});
|
||||
@@ -1,15 +1,22 @@
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import { appsNavVisibility } from '~/components/AppLayout/AppHeader/appsNavVisibility';
|
||||
|
||||
// Scope-A invariant: the PUBLIC "Build apps" → /apps/get-started nav entry is
|
||||
// visible whenever the public `appBlocksGetStarted` flag is on, INDEPENDENTLY of
|
||||
// the store flags.
|
||||
// Scope-A invariant: `getStarted` is true whenever the public `appBlocksGetStarted`
|
||||
// flag is on, INDEPENDENTLY of the store flags.
|
||||
//
|
||||
// #3907 invariant: the "Apps" → /apps entry is visible exactly when the STORE is
|
||||
// visible (`hasAppsStoreAccess` = appListings || appBlocks ||
|
||||
// appListingsPublicExternal). It is the ONLY in-product route to `/apps`, so a
|
||||
// cohort that can load the store but not see this entry has a store it cannot
|
||||
// find. It used to read `appBlocks` alone.
|
||||
// #3907 invariant: `marketplace` is true exactly when the STORE is visible
|
||||
// (`hasAppsStoreAccess` = appListings || appBlocks || appListingsPublicExternal).
|
||||
// It used to read `appBlocks` alone, which hid the store from cohorts that could
|
||||
// load it.
|
||||
//
|
||||
// 🔴 WHAT CONSUMES THEM CHANGED; WHAT THEY MEAN DID NOT. The user menu used to carry
|
||||
// one row per boolean ("Build apps" → /apps/get-started and "Apps" → /apps).
|
||||
// "Build apps" moved into the /apps/* sub-nav, so there is now ONE row whose
|
||||
// VISIBILITY is `marketplace || getStarted` and whose HREF is /apps when
|
||||
// `marketplace` and /apps/get-started otherwise — which is why `getStarted` is still
|
||||
// asserted here rather than deleted with the row it used to drive. That wiring is
|
||||
// pinned separately, against the real `hooks.tsx` source, in `appsMenuEntry.test.ts`;
|
||||
// this file remains the behavioural cover for the PREDICATE.
|
||||
describe('appsNavVisibility — public get-started vs store-gated marketplace', () => {
|
||||
it('shows the public get-started entry when appBlocksGetStarted is on', () => {
|
||||
const nav = appsNavVisibility({ appBlocksGetStarted: true, appBlocks: false });
|
||||
@@ -24,7 +31,7 @@ describe('appsNavVisibility — public get-started vs store-gated marketplace',
|
||||
expect(nav.marketplace).toBe(false);
|
||||
});
|
||||
|
||||
it('shows BOTH entries for a moderator (both flags on) — distinct labels, no collision', () => {
|
||||
it('resolves BOTH true for a moderator (both flags on)', () => {
|
||||
const nav = appsNavVisibility({ appBlocksGetStarted: true, appBlocks: true });
|
||||
expect(nav.getStarted).toBe(true);
|
||||
expect(nav.marketplace).toBe(true);
|
||||
|
||||
@@ -2,19 +2,26 @@ import type { AppsStoreFeatureFlags } from '~/shared/utils/app-blocks-access';
|
||||
import { hasAppsStoreAccess } from '~/shared/utils/app-blocks-access';
|
||||
|
||||
/**
|
||||
* Pure visibility logic for the two App Blocks nav entries in the user menu.
|
||||
* Pure visibility logic for the SINGLE App Blocks "Apps" entry in the user menu.
|
||||
*
|
||||
* Extracted out of `useGetMenuItems` (which is a heavy hook — router, session,
|
||||
* theme, tRPC) so the gating invariant is unit-testable in isolation:
|
||||
* theme, tRPC) so the gating invariant is unit-testable in isolation. It returns
|
||||
* TWO booleans because the one menu entry makes two decisions from them:
|
||||
*
|
||||
* - the PUBLIC "Build apps" → `/apps/get-started` entry is visible whenever the
|
||||
* public `appBlocksGetStarted` flag is on (everyone by default; Flipt kill
|
||||
* switch);
|
||||
* - the "Apps" → `/apps` entry is visible exactly when the STORE is visible —
|
||||
* {@link hasAppsStoreAccess}, i.e. `appListings || appBlocks ||
|
||||
* appListingsPublicExternal`. Its visibility is INDEPENDENT of the
|
||||
* - `getStarted` — the PUBLIC `/apps/get-started` landing page is reachable,
|
||||
* i.e. the public `appBlocksGetStarted` flag is on (Flipt kill switch);
|
||||
* - `marketplace` — the STORE is visible: {@link hasAppsStoreAccess}, i.e.
|
||||
* `appListings || appBlocks || appListingsPublicExternal`. INDEPENDENT of the
|
||||
* get-started flag.
|
||||
*
|
||||
* 🔴 BOTH ARE STILL LOAD-BEARING AFTER THE CONSOLIDATION. The menu used to carry
|
||||
* one entry per boolean ("Build apps" → `/apps/get-started` and "Apps" →
|
||||
* `/apps`); "Build apps" moved into the `/apps/*` sub-nav, so there is now one
|
||||
* entry whose VISIBILITY is `marketplace || getStarted` and whose HREF is `/apps`
|
||||
* when `marketplace` and `/apps/get-started` otherwise. Deleting `getStarted`
|
||||
* here would send the get-started-only cohort at `/apps`, which their flags
|
||||
* cannot load (`resolveAppsPageAccess` → `notFound`).
|
||||
*
|
||||
* 🔴 THE MARKETPLACE ENTRY USED TO READ `appBlocks` ALONE, and that is what
|
||||
* issue #3907 was. Until the W13 decoupling, `appBlocks` WAS store visibility,
|
||||
* so gating the menu item on it was correct. Afterwards the store grants on
|
||||
|
||||
@@ -9,7 +9,6 @@ import {
|
||||
IconBrush,
|
||||
IconChartHistogram,
|
||||
IconCloudLock,
|
||||
IconCode,
|
||||
IconCube,
|
||||
IconCrown,
|
||||
IconGift,
|
||||
@@ -219,29 +218,71 @@ export function useGetMenuItems(): UserMenuItemGroup[] {
|
||||
newUntil: new Date('2026-07-20'),
|
||||
},
|
||||
{
|
||||
// PUBLIC "App builders" get-started landing page (Scope A soft launch).
|
||||
// Gated on the separate public `appBlocksGetStarted` flag (kill switch),
|
||||
// NOT the mod-only `appBlocks` gate — this is the only `/apps/*` surface
|
||||
// visible to non-mods. Distinct label ("Build apps") from the mod-only
|
||||
// marketplace entry below so a moderator never sees two identical labels.
|
||||
// Visibility comes from the pure `appsNavVisibility` helper (unit-tested).
|
||||
href: '/apps/get-started',
|
||||
visible: appsNav.getStarted,
|
||||
icon: IconCode,
|
||||
color: theme.colors.blue[getPrimaryShade(theme, colorScheme ?? 'dark')],
|
||||
label: 'Build apps',
|
||||
newUntil: new Date('2026-08-01'),
|
||||
},
|
||||
{
|
||||
// App store + in-page AppsSubNav hub (installed, submit,
|
||||
// my-submissions, revenue, review). Visible exactly when the STORE is
|
||||
// — `hasAppsStoreAccess`, via `appsNavVisibility` (#3907): this is the
|
||||
// only in-product route to `/apps`, so gating it on `appBlocks` alone
|
||||
// hid the store from the catalog-only and external-only cohorts. The
|
||||
// sub-nav entries behind it keep their own gates. Labeled "Apps" so it
|
||||
// reads distinctly from the public "Build apps" entry above.
|
||||
href: '/apps',
|
||||
visible: appsNav.marketplace,
|
||||
// 🔴 ONE `/apps*` DROPDOWN ENTRY, TWO DESTINATIONS. This used to be two
|
||||
// adjacent entries — "Build apps" → `/apps/get-started` and "Apps" →
|
||||
// `/apps` — which meant a moderator (who holds both flags) saw two
|
||||
// near-identical rows for one product. "Build apps" now lives in the
|
||||
// shared `/apps/*` sub-nav (`SUB_NAV_LINKS` in `~/components/Apps/
|
||||
// AppsSubNav`) instead, so the dropdown carries a single door.
|
||||
//
|
||||
// WHICH door depends on what the viewer is entitled to, and the fallback
|
||||
// is NOT cosmetic:
|
||||
// • store access (`appsNav.marketplace` = `hasAppsStoreAccess`) → `/apps`.
|
||||
// The marketplace is the richer landing, and "Build apps" is one click
|
||||
// away in the sub-nav.
|
||||
// • get-started ONLY (`appsNav.getStarted`) → `/apps/get-started`.
|
||||
// Such a viewer CANNOT load `/apps` at all: its `getServerSideProps`
|
||||
// gates on `resolveAppsPageAccess`, which returns `notFound` without a
|
||||
// store flag. Sending them to `/apps` would be a menu entry into a 404,
|
||||
// so they land directly on the one page they are entitled to.
|
||||
//
|
||||
// Both booleans still come from the pure `appsNavVisibility` helper
|
||||
// (unit-tested), and `marketplace` still routes through the shared
|
||||
// `hasAppsStoreAccess` predicate (#3907) — gating it on `appBlocks` alone
|
||||
// hid the store from the catalog-only and external-only cohorts.
|
||||
//
|
||||
// 🔴 THE LABEL AND ICON DO *NOT* FOLLOW THE HREF, AND THAT IS A DECISION,
|
||||
// NOT AN OVERSIGHT. On the get-started-only branch this row reads "Apps"
|
||||
// with a plug glyph while navigating to developer onboarding. Accepted,
|
||||
// for three reasons: (1) "Apps" names the PRODUCT AREA, which both
|
||||
// destinations are inside — the row is a door into `/apps*`, not a
|
||||
// description of the landing page; (2) branching the label AND the icon
|
||||
// (`IconPlugConnected` vs the sub-nav's `IconCode`) re-splits one row into
|
||||
// two presentations, which is the duplication this consolidation removed;
|
||||
// (3) we believe the cohort is EMPTY today — but note what that rests on,
|
||||
// because it is NOT something this repo can settle. `appBlocksGetStarted`
|
||||
// declares `availability: ['mod']`, and for a flag whose availability carries
|
||||
// ROLE terms only, `availability` is the Flipt-DOWN fallback: `getFeatureFlags`
|
||||
// returns Flipt's answer BEFORE it evaluates roles ("Flipt overrides role
|
||||
// checks (both enable AND disable)", `~/server/services/feature-flags.service.ts`).
|
||||
// 🔴 That is scoped on purpose — `availability` ALSO carries env / region /
|
||||
// server-colour terms, and those run BEFORE Flipt and Flipt cannot override
|
||||
// them ("Server/domain restrictions always apply", same file). `['mod']`
|
||||
// carries no such term, so for THIS flag the role reading is the operative one;
|
||||
// do not generalise the sentence to a flag that names a colour domain.
|
||||
// So who actually holds get-started, and who actually holds the store flags, is
|
||||
// observable only in LIVE FLIPT. What the code DOES establish is that all four
|
||||
// App-Blocks flags are Flipt-backed runtime toggles, so this branch becomes
|
||||
// reachable with no PR and no deploy.
|
||||
//
|
||||
// 🔴 RE-DECIDE IT when a real viewer can take this branch, which happens from
|
||||
// BOTH directions — the condition is `!marketplace && getStarted`, so either
|
||||
// side moving is enough:
|
||||
// • get-started WIDENS — `app-blocks-get-started` rolled out past
|
||||
// moderators in Flipt, or `availability` changed in
|
||||
// `feature-flags.service.ts`; or
|
||||
// • the STORE flags NARROW — `marketplace` is `hasAppsStoreAccess`, which is
|
||||
// an OR over THREE flags (`app-listings` ‖ `app-blocks-enabled` ‖
|
||||
// `app-listings-public-external`; see `~/shared/utils/app-blocks-access`),
|
||||
// so it takes ALL THREE going off in Flipt, while get-started stays on, to
|
||||
// reach this branch. 🔴 Name all three: a moderator still holding any ONE of
|
||||
// them — the external-only tester cohort holds
|
||||
// `app-listings-public-external` alone — keeps `marketplace === true` and
|
||||
// does NOT take this branch. Whoever is left sees a row labelled "Apps" with
|
||||
// `IconPlugConnected` navigating to developer onboarding.
|
||||
// Until then a conditional label would be untestable-in-production copy.
|
||||
href: appsNav.marketplace ? '/apps' : '/apps/get-started',
|
||||
visible: appsNav.marketplace || appsNav.getStarted,
|
||||
icon: IconPlugConnected,
|
||||
color: theme.colors.blue[getPrimaryShade(theme, colorScheme ?? 'dark')],
|
||||
label: 'Apps',
|
||||
|
||||
@@ -42,12 +42,14 @@ import { page } from 'vitest/browser';
|
||||
import { renderWithProviders } from '../../../test/component-setup';
|
||||
import type * as TrpcMod from '~/utils/trpc';
|
||||
|
||||
// 🔴 The viewer MUST be one the sub-nav renders for. `AppsSubNav` now hides itself
|
||||
// entirely below two qualifying tabs, and the summary query is stubbed empty here, so
|
||||
// an anonymous / non-author viewer would render NO `<nav>` at all and `measure()` would
|
||||
// throw on a null lookup instead of measuring. An author (`appBlocksAuthor`) yields
|
||||
// Marketplace + Create — the same two-tab bar every number in the table above was
|
||||
// measured against, so the pixels are unchanged.
|
||||
// 🔴 The viewer MUST be one the sub-nav renders for. `AppsSubNav` hides itself entirely
|
||||
// below two qualifying tabs, and the summary query is stubbed empty here, so an
|
||||
// anonymous / non-author viewer would render NO `<nav>` at all and `measure()` would
|
||||
// fail its lookup guard instead of measuring. Only Marketplace is unconditional; the
|
||||
// author capability (`appBlocksAuthor`) adds Create, so this viewer yields exactly the
|
||||
// two-tab bar every number in the table above was measured against. `appBlocksGetStarted`
|
||||
// is deliberately NOT set — "Build apps" is gated on it, so leaving it off keeps the
|
||||
// rendered bar identical to the one those numbers came from.
|
||||
vi.mock('~/providers/FeatureFlagsProvider', () => ({
|
||||
useFeatureFlags: () => ({ appBlocks: true, appBlocksAuthor: true }),
|
||||
}));
|
||||
@@ -69,13 +71,44 @@ const pad = (el: Element, side: 'Top' | 'Bottom' | 'Left' | 'Right') =>
|
||||
|
||||
type Geometry = ReturnType<typeof measure>;
|
||||
|
||||
/**
|
||||
* Resolve an element this file's numbers are measured FROM, or fail with the reason.
|
||||
*
|
||||
* A bare `document.querySelector(...) as HTMLElement` hands `null` on to
|
||||
* `getBoundingClientRect()` / `getComputedStyle()`, which throws an anonymous
|
||||
* `TypeError` naming neither the selector nor the likely cause. Every lookup here can
|
||||
* legitimately go missing when the sub-nav's flag mocks stop admitting the viewer, so
|
||||
* that is the diagnosis worth printing.
|
||||
*/
|
||||
function required(selector: string): HTMLElement {
|
||||
const el = document.querySelector(selector) as HTMLElement | null;
|
||||
expect(
|
||||
el,
|
||||
`\`${selector}\` is not in the document. The geometry here is measured off the ` +
|
||||
'`/apps` sub-nav, which renders only for a viewer with ≥2 qualifying tabs — check ' +
|
||||
'the `useFeatureFlags` / `useCurrentUser` mocks above before re-baselining a number.'
|
||||
).not.toBeNull();
|
||||
return el as HTMLElement;
|
||||
}
|
||||
|
||||
function measure() {
|
||||
const nav = document.querySelector('nav[aria-label="App sections"]') as HTMLElement;
|
||||
const nav = required('nav[aria-label="App sections"]');
|
||||
const band = nav.parentElement as HTMLElement;
|
||||
const container = band.parentElement?.parentElement as HTMLElement;
|
||||
const firstTab = document.querySelector('[role="tab"]') as HTMLElement;
|
||||
const firstTab = required('[role="tab"]');
|
||||
// 🔴 THE WIDTH PIN MUST FOLLOW THE *MARKETPLACE* TAB, NOT "whichever tab is first".
|
||||
// 133.27px is a measurement of the string "Marketplace" plus its icon and inline
|
||||
// padding; it says nothing about any other label. `/apps/get-started` ("Build apps")
|
||||
// sits AHEAD of Marketplace in `SUB_NAV_LINKS`, so for any viewer holding
|
||||
// `appBlocksGetStarted` the `[role="tab"]` first-match is a DIFFERENT tab (122.39px)
|
||||
// and the assertion would have to be re-baselined to a number nobody had a reason
|
||||
// for. This file's mocks do not set that flag, so the two happen to coincide today —
|
||||
// selecting by href is what keeps that a coincidence rather than a dependency, and
|
||||
// keeps the pin failing for the reason it was written: a `padding` shorthand instead
|
||||
// of `paddingBlock` narrowing the inline axis.
|
||||
const marketplaceTab = required('[role="tab"][href="/apps"]');
|
||||
const title = document.querySelector('h2') as HTMLElement | null;
|
||||
const body = document.querySelector('[data-testid="body"]') as HTMLElement;
|
||||
const body = required('[data-testid="body"]');
|
||||
|
||||
const tabRect = firstTab.getBoundingClientRect();
|
||||
const containerTop = container.getBoundingClientRect().top;
|
||||
@@ -91,7 +124,11 @@ function measure() {
|
||||
tabHeight: Math.round(tabRect.height * 100) / 100,
|
||||
tabPadBlock: [pad(firstTab, 'Top'), pad(firstTab, 'Bottom')] as const,
|
||||
tabPadInline: [pad(firstTab, 'Left'), pad(firstTab, 'Right')] as const,
|
||||
tabWidth: Math.round(tabRect.width * 100) / 100,
|
||||
// NOTE: there is deliberately no `tabWidth` (the FIRST tab's width) here. It was
|
||||
// computed and never asserted, which reads as a pin that is not one. The inline-axis
|
||||
// regression this file is about is pinned on `marketplaceTabWidth` below, measured
|
||||
// off the tab the 133.27px baseline actually came from.
|
||||
marketplaceTabWidth: Math.round(marketplaceTab.getBoundingClientRect().width * 100) / 100,
|
||||
containerPadInline: [pad(container, 'Left'), pad(container, 'Right')] as const,
|
||||
containerPadTop: pad(container, 'Top'),
|
||||
containerPadBottom: pad(container, 'Bottom'),
|
||||
@@ -192,7 +229,9 @@ describe('/apps chrome vertical geometry', () => {
|
||||
expect(g.styleSheetLoaded).toBe(true);
|
||||
|
||||
expect(g.tabPadInline).toEqual([16, 16]);
|
||||
expect(g.tabWidth).toBeCloseTo(133.27, 1);
|
||||
// Measured on the MARKETPLACE tab specifically — see `measure()` for why the
|
||||
// first-tab lookup stopped being the right box.
|
||||
expect(g.marketplaceTabWidth).toBeCloseTo(133.27, 1);
|
||||
expect(g.containerPadInline).toEqual([16, 16]);
|
||||
// …while the block axis DID move.
|
||||
expect(g.tabPadBlock).toEqual([6, 6]);
|
||||
|
||||
@@ -20,7 +20,7 @@ import { renderWithProviders } from '../../../test/component-setup';
|
||||
// - `summary` — the `blocks.getNavSummary` booleans (client-only, revealed
|
||||
// post-mount).
|
||||
// - `context` — viewer CAPABILITIES resolved from SSR-seeded values
|
||||
// (`isAuthor`), safe on the first paint.
|
||||
// (`isAuthor`, `canGetStarted`), safe on the first paint.
|
||||
//
|
||||
// The sub-nav uses the Mantine **Tabs** LOOK but is wrapped in a real
|
||||
// `<nav aria-label="App sections">` so it's exposed as a navigation LANDMARK
|
||||
@@ -57,10 +57,20 @@ const ALL: AppsNavSummary = {
|
||||
hasPendingInvites: true,
|
||||
};
|
||||
|
||||
/** May author apps (mod, or a non-mod holding `appBlocksAuthor`). */
|
||||
const AUTHOR: AppsNavContext = { isAuthor: true };
|
||||
/**
|
||||
* May author apps (mod, or a non-mod holding `appBlocksAuthor`), and does NOT hold
|
||||
* `appBlocksGetStarted` — the live moderator/`app-dev-testers` shape today.
|
||||
*/
|
||||
const AUTHOR: AppsNavContext = { isAuthor: true, canGetStarted: false };
|
||||
/** The widened store-visibility tester cohort: sees the store, cannot author. */
|
||||
const NOT_AUTHOR: AppsNavContext = { isAuthor: false };
|
||||
const NOT_AUTHOR: AppsNavContext = { isAuthor: false, canGetStarted: false };
|
||||
/**
|
||||
* Holds `appBlocksGetStarted` (so `/apps/get-started` loads for them) but cannot
|
||||
* author — the cohort the "Build apps" tab exists for.
|
||||
*/
|
||||
const BUILDER: AppsNavContext = { isAuthor: false, canGetStarted: true };
|
||||
/** Both capabilities — the fullest tab set any viewer can resolve to. */
|
||||
const AUTHOR_BUILDER: AppsNavContext = { isAuthor: true, canGetStarted: true };
|
||||
|
||||
function tab(name: string) {
|
||||
return page.getByRole('tab', { name });
|
||||
@@ -128,7 +138,11 @@ describe('AppsSubNavView (conditional sub-nav tabs)', () => {
|
||||
|
||||
test('Installed shows ONLY when hasInstalls', async () => {
|
||||
renderWithProviders(
|
||||
<AppsSubNavView summary={{ ...NONE, hasInstalls: true }} context={AUTHOR} currentPath="/apps" />
|
||||
<AppsSubNavView
|
||||
summary={{ ...NONE, hasInstalls: true }}
|
||||
context={AUTHOR}
|
||||
currentPath="/apps"
|
||||
/>
|
||||
);
|
||||
await expect.element(tab('Installed')).toBeInTheDocument();
|
||||
// The other conditionals stay hidden.
|
||||
@@ -188,7 +202,11 @@ describe('AppsSubNavView (conditional sub-nav tabs)', () => {
|
||||
|
||||
test('Review shows ONLY when isReviewer', async () => {
|
||||
renderWithProviders(
|
||||
<AppsSubNavView summary={{ ...NONE, isReviewer: true }} context={AUTHOR} currentPath="/apps" />
|
||||
<AppsSubNavView
|
||||
summary={{ ...NONE, isReviewer: true }}
|
||||
context={AUTHOR}
|
||||
currentPath="/apps"
|
||||
/>
|
||||
);
|
||||
await expect.element(tab('Review')).toBeInTheDocument();
|
||||
expect(tab('Installed').elements()).toHaveLength(0);
|
||||
@@ -196,9 +214,12 @@ describe('AppsSubNavView (conditional sub-nav tabs)', () => {
|
||||
expect(tab('Revenue').elements()).toHaveLength(0);
|
||||
});
|
||||
|
||||
test('an all-true summary + author shows every tab', async () => {
|
||||
renderWithProviders(<AppsSubNavView summary={ALL} context={AUTHOR} currentPath="/apps" />);
|
||||
test('an all-true summary + author + get-started shows every tab', async () => {
|
||||
renderWithProviders(
|
||||
<AppsSubNavView summary={ALL} context={AUTHOR_BUILDER} currentPath="/apps" />
|
||||
);
|
||||
for (const name of [
|
||||
'Build apps',
|
||||
'Marketplace',
|
||||
'Create',
|
||||
'My apps',
|
||||
@@ -292,14 +313,27 @@ describe('AppsSubNavView (Create is gated on the author capability)', () => {
|
||||
});
|
||||
|
||||
/**
|
||||
* The <2-tab collapse. With `Create` conditional, a store-visible non-author
|
||||
* with no installs / submissions / approved apps and no reviewer bit qualifies
|
||||
* for Marketplace ALONE — a one-entry "navigation" that can only link to the
|
||||
* page you are on, while still costing the tab row + its bottom rule. The whole
|
||||
* bar (nav landmark included) is dropped below two tabs.
|
||||
* 🔴 THE <2-TAB COLLAPSE, AND THE GET-STARTED GATE THAT KEEPS IT REACHABLE.
|
||||
*
|
||||
* Marketplace is the ONLY unconditional row in `SUB_NAV_LINKS`. "Build apps" is
|
||||
* `visible: (_s, c) => c.canGetStarted`, mirroring the gate `resolveGetStartedAccess`
|
||||
* puts on `/apps/get-started`, so a store-visible non-author with an empty summary and
|
||||
* no `appBlocksGetStarted` still qualifies for Marketplace ALONE and the bar still
|
||||
* drops itself. These tests therefore cover a LIVE cohort, not merely a property of the
|
||||
* exported view.
|
||||
*
|
||||
* Both directions are asserted for the new row — absent without the capability, present
|
||||
* with it — because asserting only one of them passes on a tab that renders for nobody,
|
||||
* and asserting only the other passes on a tab that renders for everybody. The second
|
||||
* shape is the defect this block exists to catch: an unconditional "Build apps" offers
|
||||
* the `app-dev-testers` cohort (`appBlocks` yes, `appBlocksGetStarted` no) a tab whose
|
||||
* page answers `notFound`, and defeats the `app-blocks-get-started` kill switch for
|
||||
* every moderator.
|
||||
*/
|
||||
describe('AppsSubNavView (hides entirely below two tabs)', () => {
|
||||
test('renders NOTHING when only Marketplace qualifies', async () => {
|
||||
describe('AppsSubNavView (the collapse, and the get-started gate that keeps it live)', () => {
|
||||
test('🔴 a non-author with an EMPTY summary and no get-started renders NOTHING', async () => {
|
||||
// The `< 2` collapse: Marketplace alone. Making "Build apps" unconditional fails
|
||||
// HERE, on all three readers.
|
||||
renderWithProviders(
|
||||
<>
|
||||
<RenderBarrier />
|
||||
@@ -307,13 +341,53 @@ describe('AppsSubNavView (hides entirely below two tabs)', () => {
|
||||
</>
|
||||
);
|
||||
await awaitCommit(); // without this the assertions below cannot fail — see RENDER_BARRIER
|
||||
// No tabs, no tablist, and no navigation landmark — the component returned null.
|
||||
expect(tab('Marketplace').elements()).toHaveLength(0);
|
||||
expect(page.getByRole('tablist').elements()).toHaveLength(0);
|
||||
expect(page.getByRole('navigation', { name: 'App sections' }).elements()).toHaveLength(0);
|
||||
});
|
||||
|
||||
test('renders as soon as a SECOND tab qualifies (one install)', async () => {
|
||||
test('🔴 canGetStarted is what brings the bar back for that same viewer', async () => {
|
||||
// Identical summary and identical `isAuthor` — ONLY `canGetStarted` moves. That is
|
||||
// what makes the pair attribute the bar to the get-started capability rather than
|
||||
// to some other difference between two fixtures.
|
||||
renderWithProviders(<AppsSubNavView summary={NONE} context={BUILDER} currentPath="/apps" />);
|
||||
await expect
|
||||
.element(page.getByRole('navigation', { name: 'App sections' }))
|
||||
.toBeInTheDocument();
|
||||
await expect.element(tab('Build apps')).toBeInTheDocument();
|
||||
await expect.element(tab('Marketplace')).toBeInTheDocument();
|
||||
expect(page.getByRole('tab').elements()).toHaveLength(2);
|
||||
});
|
||||
|
||||
test('🔴 Build apps is ABSENT for a store-visible viewer WITHOUT the flag', async () => {
|
||||
// The cohort the defect would have 404'd. An author, so the bar clears the floor on
|
||||
// Marketplace + Create and this is an assertion about the TAB, not about the bar.
|
||||
renderWithProviders(<AppsSubNavView summary={NONE} context={AUTHOR} currentPath="/apps" />);
|
||||
await expect.element(tab('Marketplace')).toBeInTheDocument(); // positive control
|
||||
await expect.element(tab('Create')).toBeInTheDocument();
|
||||
expect(tab('Build apps').elements()).toHaveLength(0);
|
||||
});
|
||||
|
||||
test('the Build apps tab points at /apps/get-started', async () => {
|
||||
renderWithProviders(<AppsSubNavView summary={NONE} context={BUILDER} currentPath="/apps" />);
|
||||
await expect.element(tab('Build apps')).toBeInTheDocument();
|
||||
expect(tab('Build apps').element().getAttribute('href')).toBe('/apps/get-started');
|
||||
});
|
||||
|
||||
test('it leads the bar — Build apps is the FIRST tab when it renders', async () => {
|
||||
renderWithProviders(
|
||||
<AppsSubNavView summary={ALL} context={AUTHOR_BUILDER} currentPath="/apps" />
|
||||
);
|
||||
await expect.element(tab('Build apps')).toBeInTheDocument();
|
||||
expect(
|
||||
page
|
||||
.getByRole('tab')
|
||||
.elements()
|
||||
.map((el) => (el.textContent ?? '').trim())[0]
|
||||
).toBe('Build apps');
|
||||
});
|
||||
|
||||
test('a summary flag alone brings the bar back (one install, no get-started)', async () => {
|
||||
renderWithProviders(
|
||||
<AppsSubNavView
|
||||
summary={{ ...NONE, hasInstalls: true }}
|
||||
@@ -321,30 +395,32 @@ describe('AppsSubNavView (hides entirely below two tabs)', () => {
|
||||
currentPath="/apps"
|
||||
/>
|
||||
);
|
||||
await expect.element(page.getByRole('navigation', { name: 'App sections' })).toBeInTheDocument();
|
||||
await expect.element(tab('Marketplace')).toBeInTheDocument();
|
||||
await expect
|
||||
.element(page.getByRole('navigation', { name: 'App sections' }))
|
||||
.toBeInTheDocument();
|
||||
await expect.element(tab('Installed')).toBeInTheDocument();
|
||||
expect(page.getByRole('tab').elements()).toHaveLength(2);
|
||||
});
|
||||
|
||||
test('the second tab can be Create (author, otherwise-empty summary)', async () => {
|
||||
test('the author capability alone brings it back too (Create is the second tab)', async () => {
|
||||
renderWithProviders(<AppsSubNavView summary={NONE} context={AUTHOR} currentPath="/apps" />);
|
||||
await expect.element(page.getByRole('navigation', { name: 'App sections' })).toBeInTheDocument();
|
||||
await expect
|
||||
.element(page.getByRole('navigation', { name: 'App sections' }))
|
||||
.toBeInTheDocument();
|
||||
expect(page.getByRole('tab').elements()).toHaveLength(2);
|
||||
});
|
||||
|
||||
// The hide applies to EVERY viewer — there is no moderator carve-out. A mod
|
||||
// always resolves to `isAuthor: true` in the container, so the only way a mod
|
||||
// reaches one tab is `isAuthor: false`, which is what this drives directly.
|
||||
test('the collapse has no moderator carve-out (isAuthor=false ⇒ hidden, whoever the viewer is)', async () => {
|
||||
renderWithProviders(
|
||||
<>
|
||||
<RenderBarrier />
|
||||
<AppsSubNavView summary={NONE} context={NOT_AUTHOR} currentPath="/apps/installed" />
|
||||
</>
|
||||
);
|
||||
await awaitCommit();
|
||||
expect(page.getByRole('tablist').elements()).toHaveLength(0);
|
||||
// 🔴 NEGATIVE CONTROL for the counts above. Every assertion in this block is
|
||||
// "the bar IS there with N tabs", and a reader wired to nothing would satisfy
|
||||
// none of them — but a `visible` predicate that had silently become `() => true`
|
||||
// for a conditional tab would satisfy all of them too. This one fails in that
|
||||
// case: a non-author must still NOT get Create / My apps / Invites.
|
||||
test('NEGATIVE CONTROL: the conditional tabs are still conditional', async () => {
|
||||
renderWithProviders(<AppsSubNavView summary={ALL} context={BUILDER} currentPath="/apps" />);
|
||||
await expect.element(tab('Build apps')).toBeInTheDocument();
|
||||
expect(tab('Create').elements()).toHaveLength(0);
|
||||
expect(tab('My apps').elements()).toHaveLength(0);
|
||||
expect(tab('Invites').elements()).toHaveLength(0);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -454,8 +530,11 @@ describe('AppsSubNavView (each tab navigates to its route)', () => {
|
||||
// click follows. Asserting the href is the deterministic equivalent of "a
|
||||
// click navigates to the right route" for a link-based tab.
|
||||
test('every visible tab points its href at the matching /apps route', async () => {
|
||||
renderWithProviders(<AppsSubNavView summary={ALL} context={AUTHOR} currentPath="/apps" />);
|
||||
renderWithProviders(
|
||||
<AppsSubNavView summary={ALL} context={AUTHOR_BUILDER} currentPath="/apps" />
|
||||
);
|
||||
const cases: Array<[string, string]> = [
|
||||
['Build apps', '/apps/get-started'],
|
||||
['Marketplace', '/apps'],
|
||||
['Create', '/apps/submit'],
|
||||
// Collaborator surfaces: "My apps" is the ownership-OR-seat list
|
||||
@@ -603,8 +682,10 @@ describe('AppsSubNavView (collaborator tabs are gated on the author capability)'
|
||||
}
|
||||
|
||||
test('a NON-author invitee gets no sub-nav at all (1 qualifying tab ⇒ hidden)', async () => {
|
||||
// Marketplace alone survives, and main's `links.length < 2` rule then hides the bar —
|
||||
// so the two changes compose to the right end state rather than merely not crashing.
|
||||
// Marketplace alone survives the author gate and the `< 2` collapse then hides the
|
||||
// bar — so the two changes compose to the right end state rather than merely not
|
||||
// crashing. (This viewer holds no `appBlocksGetStarted`, so "Build apps" is gated
|
||||
// out too; the same fixture WITH that capability is covered in the collapse block.)
|
||||
renderWithProviders(
|
||||
<>
|
||||
<RenderBarrier />
|
||||
@@ -616,6 +697,7 @@ describe('AppsSubNavView (collaborator tabs are gated on the author capability)'
|
||||
</>
|
||||
);
|
||||
await awaitCommit();
|
||||
expect(tab('Invites').elements()).toHaveLength(0);
|
||||
expect(page.getByRole('navigation', { name: 'App sections' }).elements()).toHaveLength(0);
|
||||
});
|
||||
|
||||
|
||||
@@ -186,24 +186,34 @@ describe('AppsSubNav container — hydration-safe conditional tabs', () => {
|
||||
* equality is not an `[] === []` that any broken render would also satisfy.
|
||||
*/
|
||||
describe('AppsSubNav container — SSR tab set === first client render', () => {
|
||||
// 🔴 BOTH COHORTS HOLD `appBlocksGetStarted`, DELIBERATELY. The pairs below assert
|
||||
// SSR ≡ first-client-paint against a pinned literal, and an EMPTY literal is
|
||||
// satisfied by two renders that are equally broken — a non-author without this flag
|
||||
// qualifies for Marketplace alone and the `< 2` collapse would make both sides `[]`.
|
||||
// Adding the flag makes the pinned set non-empty on both sides, which is strictly
|
||||
// stronger. It also puts `context.canGetStarted` under the invariant: like
|
||||
// `isAuthor`, it is applied OUTSIDE the `useIsClient` deferral, so if it were ever
|
||||
// sourced from something client-only the "Build apps" tab would appear on one side
|
||||
// of a pair and not the other and these tests would fail. (The cohorts WITHOUT the
|
||||
// flag — where the collapse fires — are pinned in the last describe of this file and
|
||||
// in `AppsSubNav.storeGate.browser.test.tsx`.)
|
||||
const NON_AUTHOR = () => {
|
||||
// The widened store-visibility tester cohort, verified live: sees the store,
|
||||
// cannot author.
|
||||
mocks.flags = { appBlocks: true, appBlocksAuthor: false };
|
||||
mocks.flags = { appBlocks: true, appBlocksAuthor: false, appBlocksGetStarted: true };
|
||||
mocks.user = { id: 7, username: 'tester', isModerator: false };
|
||||
};
|
||||
const AUTHOR = () => {
|
||||
mocks.flags = { appBlocks: true, appBlocksAuthor: true };
|
||||
mocks.flags = { appBlocks: true, appBlocksAuthor: true, appBlocksGetStarted: true };
|
||||
mocks.user = { id: 7, username: 'author', isModerator: false };
|
||||
};
|
||||
|
||||
test('NON-AUTHOR, server render (no query data) → no bar at all', async () => {
|
||||
test('NON-AUTHOR, server render (no query data) → Build apps + Marketplace', async () => {
|
||||
NON_AUTHOR();
|
||||
mocks.isClient = false;
|
||||
mocks.navSummary = undefined;
|
||||
await renderSubNav();
|
||||
expect(renderedTabs()).toEqual([]);
|
||||
expect(page.getByRole('tablist').elements()).toHaveLength(0);
|
||||
await expect.element(tab('Marketplace')).toBeInTheDocument();
|
||||
expect(renderedTabs()).toEqual(['Build apps', 'Marketplace']);
|
||||
});
|
||||
|
||||
test('NON-AUTHOR, first client paint (query data PRESENT) → the identical set', async () => {
|
||||
@@ -211,17 +221,17 @@ describe('AppsSubNav container — SSR tab set === first client render', () => {
|
||||
mocks.isClient = false;
|
||||
mocks.navSummary = { ...ALL_TRUE_SUMMARY }; // the prod condition that broke hydration
|
||||
await renderSubNav();
|
||||
expect(renderedTabs()).toEqual([]);
|
||||
expect(page.getByRole('tablist').elements()).toHaveLength(0);
|
||||
await expect.element(tab('Marketplace')).toBeInTheDocument();
|
||||
expect(renderedTabs()).toEqual(['Build apps', 'Marketplace']);
|
||||
});
|
||||
|
||||
test('AUTHOR, server render (no query data) → Marketplace + Create', async () => {
|
||||
test('AUTHOR, server render (no query data) → Build apps + Marketplace + Create', async () => {
|
||||
AUTHOR();
|
||||
mocks.isClient = false;
|
||||
mocks.navSummary = undefined;
|
||||
await renderSubNav();
|
||||
await expect.element(tab('Marketplace')).toBeInTheDocument();
|
||||
expect(renderedTabs()).toEqual(['Marketplace', 'Create']);
|
||||
expect(renderedTabs()).toEqual(['Build apps', 'Marketplace', 'Create']);
|
||||
});
|
||||
|
||||
test('AUTHOR, first client paint (query data PRESENT) → the identical set', async () => {
|
||||
@@ -233,7 +243,7 @@ describe('AppsSubNav container — SSR tab set === first client render', () => {
|
||||
// Non-empty on BOTH sides of the pair: the author gate applied on the first
|
||||
// paint (Create is here pre-mount) while the summary gate did NOT (none of the
|
||||
// four conditional tabs leaked in).
|
||||
expect(renderedTabs()).toEqual(['Marketplace', 'Create']);
|
||||
expect(renderedTabs()).toEqual(['Build apps', 'Marketplace', 'Create']);
|
||||
});
|
||||
|
||||
test('POSITIVE CONTROL: the same reader DOES see the conditional tabs post-mount', async () => {
|
||||
@@ -244,12 +254,7 @@ describe('AppsSubNav container — SSR tab set === first client render', () => {
|
||||
mocks.navSummary = { ...ALL_TRUE_SUMMARY };
|
||||
await renderSubNav();
|
||||
await expect.element(tab('Installed')).toBeInTheDocument();
|
||||
expect(renderedTabs()).toEqual([
|
||||
'Marketplace',
|
||||
'Installed',
|
||||
'Revenue',
|
||||
'Review',
|
||||
]);
|
||||
expect(renderedTabs()).toEqual(['Build apps', 'Marketplace', 'Installed', 'Revenue', 'Review']);
|
||||
// Create is the one tab the author gate removes.
|
||||
expect(renderedTabs()).not.toContain('Create');
|
||||
});
|
||||
@@ -301,42 +306,77 @@ describe('AppsSubNav container — the Create tab keys off isAppDeveloper', () =
|
||||
mocks.flags = { appBlocks: true, appBlocksAuthor: true };
|
||||
mocks.user = null;
|
||||
mocks.navSummary = undefined; // the summary query is protected — no data for anon
|
||||
await renderSubNav(); // barrier awaited inside — absence below is a real observation
|
||||
// Create is absent because a logged-out viewer resolves to `isAuthor: false`; with
|
||||
// no `appBlocksGetStarted` either, Marketplace is all that qualifies and the `< 2`
|
||||
// collapse then removes the bar entirely.
|
||||
expect(tab('Create').elements()).toHaveLength(0);
|
||||
expect(renderedTabs()).toEqual([]);
|
||||
});
|
||||
|
||||
test('🔴 a logged-out viewer WITH appBlocksGetStarted does get Build apps (positive control)', async () => {
|
||||
// The control for the `toEqual([])` above — the same reader observes a bar as soon
|
||||
// as one qualifying tab is added, so the empty set there is a real absence. It also
|
||||
// pins the deliberate asymmetry in the container: `canGetStarted` is NOT
|
||||
// session-scoped (`resolveGetStartedAccess` reads the flag and nothing else), so
|
||||
// folding it into the `currentUser` branch fails HERE.
|
||||
mocks.flags = { appBlocks: true, appBlocksAuthor: true, appBlocksGetStarted: true };
|
||||
mocks.user = null;
|
||||
mocks.navSummary = undefined;
|
||||
await renderSubNav();
|
||||
// Marketplace alone ⇒ the whole bar is hidden.
|
||||
expect(page.getByRole('tablist').elements()).toHaveLength(0);
|
||||
await expect.element(tab('Build apps')).toBeInTheDocument();
|
||||
expect(renderedTabs()).toEqual(['Build apps', 'Marketplace']);
|
||||
expect(tab('Create').elements()).toHaveLength(0);
|
||||
});
|
||||
});
|
||||
|
||||
/** The <2-tab collapse, driven through the container. */
|
||||
/**
|
||||
* The <2-tab collapse, driven through the container. Still reachable, and by the live
|
||||
* `app-dev-testers` shape: a store-visible non-author with an empty summary and no
|
||||
* `appBlocksGetStarted` qualifies for Marketplace ALONE. "Build apps" is gated on
|
||||
* `context.canGetStarted`, so it does not lift this viewer over the floor — the third
|
||||
* test is the same fixture with only that flag moved, which is what attributes the
|
||||
* difference to the flag rather than to two unrelated fixtures.
|
||||
*/
|
||||
describe('AppsSubNav container — hides entirely below two tabs', () => {
|
||||
const EMPTY_SUMMARY = {
|
||||
hasInstalls: false,
|
||||
hasSubmissions: false,
|
||||
hasApprovedApps: false,
|
||||
isReviewer: false,
|
||||
};
|
||||
|
||||
test('a non-author with an empty summary renders no nav at all', async () => {
|
||||
mocks.isClient = true;
|
||||
mocks.flags = { appBlocks: true, appBlocksAuthor: false };
|
||||
mocks.user = { id: 7, username: 'tester', isModerator: false };
|
||||
mocks.navSummary = {
|
||||
hasInstalls: false,
|
||||
hasSubmissions: false,
|
||||
hasApprovedApps: false,
|
||||
isReviewer: false,
|
||||
};
|
||||
await renderSubNav();
|
||||
mocks.navSummary = { ...EMPTY_SUMMARY };
|
||||
await renderSubNav(); // barrier awaited inside — absence below is a real observation
|
||||
expect(page.getByRole('navigation', { name: 'App sections' }).elements()).toHaveLength(0);
|
||||
expect(page.getByRole('tablist').elements()).toHaveLength(0);
|
||||
expect(renderedTabs()).toEqual([]);
|
||||
});
|
||||
|
||||
test('one install is enough to bring the bar back', async () => {
|
||||
mocks.isClient = true;
|
||||
mocks.flags = { appBlocks: true, appBlocksAuthor: false };
|
||||
mocks.user = { id: 7, username: 'tester', isModerator: false };
|
||||
mocks.navSummary = {
|
||||
hasInstalls: true,
|
||||
hasSubmissions: false,
|
||||
hasApprovedApps: false,
|
||||
isReviewer: false,
|
||||
};
|
||||
mocks.navSummary = { ...EMPTY_SUMMARY, hasInstalls: true };
|
||||
await renderSubNav();
|
||||
await expect.element(page.getByRole('navigation', { name: 'App sections' })).toBeInTheDocument();
|
||||
await expect
|
||||
.element(page.getByRole('navigation', { name: 'App sections' }))
|
||||
.toBeInTheDocument();
|
||||
expect(renderedTabs()).toEqual(['Marketplace', 'Installed']);
|
||||
});
|
||||
|
||||
test('🔴 …and so is appBlocksGetStarted, on the OTHERWISE IDENTICAL viewer', async () => {
|
||||
mocks.isClient = true;
|
||||
mocks.flags = { appBlocks: true, appBlocksAuthor: false, appBlocksGetStarted: true };
|
||||
mocks.user = { id: 7, username: 'tester', isModerator: false };
|
||||
mocks.navSummary = { ...EMPTY_SUMMARY };
|
||||
await renderSubNav();
|
||||
await expect
|
||||
.element(page.getByRole('navigation', { name: 'App sections' }))
|
||||
.toBeInTheDocument();
|
||||
expect(renderedTabs()).toEqual(['Build apps', 'Marketplace']);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -191,15 +191,31 @@ describe('AppsSubNav — real SSR → hydrate', () => {
|
||||
expect(hydrationConsoleErrors().length).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
test('NON-AUTHOR (app-listings tester): no bar in the server HTML, and hydration is clean', async () => {
|
||||
mocks.flags = { appBlocks: true, appBlocksAuthor: false };
|
||||
test('NON-AUTHOR with appBlocksGetStarted: Build apps IS server-rendered (the gate applies pre-hydration) and hydration is clean', async () => {
|
||||
// 🔴 THE GET-STARTED HALF OF THE SAME OBSERVATION THE AUTHOR TEST BELOW MAKES.
|
||||
// `context.canGetStarted` is applied OUTSIDE the `useIsClient` deferral, which is
|
||||
// only correct because `appBlocksGetStarted` is SSR-seeded and frozen (not
|
||||
// `toggleable`). That claim is checkable exactly here: the tab must be present in
|
||||
// the SERVER HTML, and hydrating a client whose summary cache is already full must
|
||||
// still produce no mismatch.
|
||||
//
|
||||
// The flag also keeps this cohort's pinned set non-empty: without it a non-author
|
||||
// with no summary qualifies for Marketplace alone and the `< 2` collapse empties
|
||||
// the HTML, which is a much weaker thing to assert (see the storeGate suite for the
|
||||
// no-flag cohort).
|
||||
mocks.flags = { appBlocks: true, appBlocksAuthor: false, appBlocksGetStarted: true };
|
||||
mocks.user = { id: 7, username: 'tester', isModerator: false };
|
||||
|
||||
// SERVER: the protected summary query has not resolved.
|
||||
mocks.navSummary = undefined;
|
||||
const html = renderToString(subNav());
|
||||
// A non-author with no summary qualifies for Marketplace alone ⇒ nothing renders.
|
||||
expect(html).not.toContain('role="tab"');
|
||||
expect(html).toContain('/apps/get-started');
|
||||
expect(html).toContain('/apps"'); // the Marketplace anchor
|
||||
// …while the SUMMARY-driven tabs are absent even though the client cache below is
|
||||
// full, and Create is absent because this viewer is not an author.
|
||||
expect(html).not.toContain('/apps/submit');
|
||||
expect(html).not.toContain('/apps/installed');
|
||||
expect(html).not.toContain('/apps/review');
|
||||
|
||||
// CLIENT: the query data IS in the cache on the very first render — the exact
|
||||
// condition that bailed hydration before the `useIsClient` gate existed.
|
||||
@@ -247,13 +263,22 @@ describe('AppsSubNav — real SSR → hydrate', () => {
|
||||
expect(hydrationConsoleErrors()).toEqual([]);
|
||||
});
|
||||
|
||||
test('logged-out: the server renders no bar, and hydration is clean', async () => {
|
||||
mocks.flags = { appBlocks: true, appBlocksAuthor: true };
|
||||
test('logged-out: no SESSION-scoped tab in the server HTML, and hydration is clean', async () => {
|
||||
mocks.flags = { appBlocks: true, appBlocksAuthor: true, appBlocksGetStarted: true };
|
||||
mocks.user = null;
|
||||
mocks.navSummary = undefined;
|
||||
|
||||
const html = renderToString(subNav());
|
||||
expect(html).not.toContain('role="tab"');
|
||||
// 🔴 THE ASYMMETRY BETWEEN THE TWO CONTEXT FIELDS, ASSERTED. `isAuthor` is a
|
||||
// capability OF A USER, so an anon viewer resolves it to false and Create is absent
|
||||
// even though `appBlocksAuthor` reads true. `canGetStarted` mirrors
|
||||
// `resolveGetStartedAccess`, which consults no user at all, so Build apps IS
|
||||
// rendered — and it doubles as this test's positive control: without it the HTML
|
||||
// would be empty and `not.toContain('Create')` would pass on a component that
|
||||
// rendered nothing for any reason at all.
|
||||
expect(html).toContain('/apps/get-started');
|
||||
expect(html).not.toContain('/apps/submit');
|
||||
expect(html).not.toContain('Create');
|
||||
|
||||
await hydrateInto(html, subNav());
|
||||
|
||||
|
||||
@@ -171,6 +171,8 @@ describe('AppsSubNav — store-visibility gate matches resolveAppsPageAccess', (
|
||||
// With `appBlocks` on, the summary query is enabled, so the conditional tabs
|
||||
// resolve too — the full bar today's testers actually see.
|
||||
await expect.element(tab('Review')).toBeInTheDocument();
|
||||
// No `appBlocksGetStarted` in this flag set, so "Build apps" is gated out —
|
||||
// this IS today's live moderator/tester cohort.
|
||||
expect(renderedTabs()).toEqual([
|
||||
'Marketplace',
|
||||
'Create',
|
||||
@@ -213,33 +215,151 @@ describe('AppsSubNav — store-visibility gate matches resolveAppsPageAccess', (
|
||||
});
|
||||
|
||||
/**
|
||||
* The gate is NOT the only thing that can hide the bar, and conflating the two
|
||||
* would make the tests above pass for the wrong reason. `AppsSubNavView` also
|
||||
* hides itself below two qualifying tabs. An `appListings`-only NON-author
|
||||
* qualifies for Marketplace alone, so they get no bar — correctly, via the
|
||||
* COLLAPSE rule, not via the store gate. Pinned so a future reader doesn't
|
||||
* "fix" the collapse thinking it is gate drift.
|
||||
* 🔴 THE <2-TAB COLLAPSE, DRIVEN THROUGH THE CONTAINER — UNCHANGED FROM `main` FOR
|
||||
* EVERY COHORT THAT DOES NOT HOLD `appBlocksGetStarted`.
|
||||
*
|
||||
* An `appListings`-only non-author (and a logged-out viewer) qualifies for Marketplace
|
||||
* ALONE and the collapse drops the whole bar. "Build apps" is gated on
|
||||
* `context.canGetStarted`, so it does NOT lift these cohorts over the floor — which is
|
||||
* the point: they cannot load `/apps/get-started`, so a tab into it would be a tab into
|
||||
* a 404. Pinned here so a later widening of that predicate is visible in a diff rather
|
||||
* than discovered on staging.
|
||||
*/
|
||||
describe('AppsSubNav — the <2-tab collapse is a SEPARATE rule from the gate', () => {
|
||||
test('appListings-only NON-author → no bar, by the collapse (Marketplace alone)', async () => {
|
||||
describe('AppsSubNav — the collapse still hides the bar for a one-tab viewer', () => {
|
||||
test('🔴 appListings-only NON-author, no get-started → no bar at all', async () => {
|
||||
mocks.flags = { appListings: true, appBlocks: false, appBlocksAuthor: false };
|
||||
mocks.user = { id: 8, username: 'tester', isModerator: false };
|
||||
await renderSubNav();
|
||||
await renderSubNav(); // barrier awaited inside — absence below is a real observation
|
||||
|
||||
expect(page.getByRole('tablist').elements()).toHaveLength(0);
|
||||
// …and the SAME viewer WITH the author capability does get a bar, which is
|
||||
// what proves the hide above came from the collapse and not from the gate.
|
||||
expect(renderedTabs()).toEqual([]);
|
||||
expect(page.getByRole('navigation', { name: 'App sections' }).elements()).toHaveLength(0);
|
||||
// …and the SAME viewer WITH the author capability DOES get a bar, which is what
|
||||
// proves the absence above came from the predicates and not from a stuck render.
|
||||
mocks.flags = { appListings: true, appBlocks: false, appBlocksAuthor: true };
|
||||
await renderSubNav();
|
||||
await expect.element(tab('Create')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
test('a logged-out viewer gets no bar even with the store flag lit', async () => {
|
||||
test('🔴 …and the get-started capability is what lifts that same viewer over the floor', async () => {
|
||||
// Identical to the first arm above except for ONE flag. Making "Build apps"
|
||||
// unconditional makes the first arm fail; removing the tab makes this one fail.
|
||||
mocks.flags = {
|
||||
appListings: true,
|
||||
appBlocks: false,
|
||||
appBlocksAuthor: false,
|
||||
appBlocksGetStarted: true,
|
||||
};
|
||||
mocks.user = { id: 8, username: 'tester', isModerator: false };
|
||||
await renderSubNav();
|
||||
|
||||
await expect.element(tab('Build apps')).toBeInTheDocument();
|
||||
expect(renderedTabs()).toEqual(['Build apps', 'Marketplace']);
|
||||
});
|
||||
|
||||
test('a logged-out viewer with the store flag lit gets no bar (Marketplace alone)', async () => {
|
||||
mocks.flags = { appListings: true, appBlocks: false, appBlocksAuthor: true };
|
||||
mocks.user = null;
|
||||
await renderSubNav();
|
||||
|
||||
expect(page.getByRole('tablist').elements()).toHaveLength(0);
|
||||
// A logged-out viewer resolves to `NO_CAPABILITIES.isAuthor`, so Create is gone and
|
||||
// Marketplace is all that qualifies — the collapse then removes the bar.
|
||||
expect(renderedTabs()).toEqual([]);
|
||||
expect(tab('Create').elements()).toHaveLength(0);
|
||||
expect(page.getByRole('navigation', { name: 'App sections' }).elements()).toHaveLength(0);
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* 🔴 THE GET-STARTED HALF OF THE BAR GATE — `hasAppsStoreAccess(features) ||
|
||||
* features.appBlocksGetStarted`.
|
||||
*
|
||||
* WHY THE SECOND TERM EXISTS: the bar now carries a "Build apps" tab pointing at
|
||||
* `/apps/get-started`, and that page's own gate (`resolveGetStartedAccess`) is
|
||||
* `appBlocksGetStarted` ALONE — it does not consult any store flag. On a store-only
|
||||
* gate the container returns `null` for exactly the cohort the tab was added for, so
|
||||
* the tab would be invisible to them on the one page they can load. Reverting the gate
|
||||
* to `if (!hasAppsStoreAccess(features)) return null` fails the first test below.
|
||||
*
|
||||
* 🔴 THE GATE AND THE TAB READ THE SAME FLAG, and both halves are pinned below. The
|
||||
* gate is an OR, so it admits viewers who hold the STORE flags without
|
||||
* `appBlocksGetStarted`; the "Build apps" tab is `visible: (_s, c) => c.canGetStarted`,
|
||||
* so those viewers are admitted to the bar and NOT offered the tab. An unconditional
|
||||
* tab would hand that cohort — `app-dev-testers` today, every moderator after the kill
|
||||
* switch is thrown — a tab whose page answers `notFound`.
|
||||
*
|
||||
* The `enabled:` predicate on `getNavSummary` is deliberately NOT widened with it (see
|
||||
* the block at the end of this file) — for a get-started-only viewer it stays false,
|
||||
* which is both correct and free.
|
||||
*/
|
||||
describe('AppsSubNav — the bar gate admits the get-started cohort', () => {
|
||||
test('🔴 appBlocksGetStarted ONLY (no store flag) → the bar AND the Build apps tab render', async () => {
|
||||
mocks.flags = { appBlocksGetStarted: true, appListings: false, appBlocks: false };
|
||||
mocks.user = { id: 9, username: 'builder', isModerator: false };
|
||||
await renderSubNav();
|
||||
|
||||
await expect
|
||||
.element(page.getByRole('navigation', { name: 'App sections' }))
|
||||
.toBeInTheDocument();
|
||||
await expect.element(tab('Build apps')).toBeInTheDocument();
|
||||
expect(tab('Build apps').element().getAttribute('href')).toBe('/apps/get-started');
|
||||
// Non-author, no store flag, summary query disabled ⇒ exactly the two always-on tabs.
|
||||
expect(renderedTabs()).toEqual(['Build apps', 'Marketplace']);
|
||||
});
|
||||
|
||||
test('🔴 store access but NO appBlocksGetStarted → a bar with NO Build apps tab', async () => {
|
||||
// The pre-existing cohort, and the one the tab predicate exists for: they hold
|
||||
// `appBlocks`/`appListings` (so the OR gate admits them — removing the store term,
|
||||
// or turning the OR into an AND, fails here rather than only in the case above) but
|
||||
// `/apps/get-started` answers `notFound` for them, so the tab must be absent.
|
||||
// An AUTHOR, so the bar clears the `< 2` floor on Marketplace + Create and this is
|
||||
// an assertion about the TAB rather than about the bar.
|
||||
mocks.flags = {
|
||||
appListings: true,
|
||||
appBlocks: false,
|
||||
appBlocksAuthor: true,
|
||||
appBlocksGetStarted: false,
|
||||
};
|
||||
mocks.user = { id: 8, username: 'tester', isModerator: false };
|
||||
await renderSubNav();
|
||||
|
||||
await expect
|
||||
.element(page.getByRole('navigation', { name: 'App sections' }))
|
||||
.toBeInTheDocument();
|
||||
expect(renderedTabs()).toEqual(['Marketplace', 'Create']);
|
||||
expect(tab('Build apps').elements()).toHaveLength(0);
|
||||
});
|
||||
|
||||
test('BOTH the store flags and appBlocksGetStarted → Build apps AND Marketplace', async () => {
|
||||
// The cohort a Flipt widening produces, and the positive control for the pair
|
||||
// above: the same reader that saw no "Build apps" there does see one here.
|
||||
mocks.flags = { appListings: true, appBlocks: false, appBlocksGetStarted: true };
|
||||
mocks.user = { id: 8, username: 'tester', isModerator: false };
|
||||
await renderSubNav();
|
||||
|
||||
await expect.element(tab('Build apps')).toBeInTheDocument();
|
||||
expect(renderedTabs()).toEqual(['Build apps', 'Marketplace']);
|
||||
});
|
||||
|
||||
test('NEITHER store access NOR appBlocksGetStarted → no bar (the gate is still a gate)', async () => {
|
||||
mocks.flags = { appListings: false, appBlocks: false, appBlocksGetStarted: false };
|
||||
mocks.user = { id: 9, username: 'nobody', isModerator: false };
|
||||
await renderSubNav(); // barrier awaited inside — absence below is a real observation
|
||||
|
||||
expect(renderedTabs()).toEqual([]);
|
||||
expect(page.getByRole('navigation', { name: 'App sections' }).elements()).toHaveLength(0);
|
||||
});
|
||||
|
||||
test('the get-started term does NOT enable the summary query', async () => {
|
||||
// `getNavSummary` is `protectedProcedure.use(enforceAppBlocksFlag)`; widening the
|
||||
// bar gate must not widen the query gate, or every get-started-only viewer buys a
|
||||
// round-trip to a guaranteed all-false answer.
|
||||
mocks.flags = { appBlocksGetStarted: true, appListings: false, appBlocks: false };
|
||||
mocks.user = { id: 9, username: 'builder', isModerator: false };
|
||||
await renderSubNav();
|
||||
|
||||
await expect.element(tab('Build apps')).toBeInTheDocument();
|
||||
expect(mocks.navSummaryEnabled.length).toBeGreaterThan(0); // the hook did run
|
||||
expect(mocks.navSummaryEnabled.every((e) => e === false)).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -2,6 +2,7 @@ import { Box, rem, ScrollArea, Tabs } from '@mantine/core';
|
||||
import {
|
||||
IconApps,
|
||||
IconBuildingStore,
|
||||
IconCode,
|
||||
IconCurrencyDollar,
|
||||
IconGavel,
|
||||
IconMail,
|
||||
@@ -76,10 +77,18 @@ export type AppsNavContext = {
|
||||
* `appBlocksAuthor` capability widens it to the curated non-mod cohort.
|
||||
*/
|
||||
isAuthor: boolean;
|
||||
/**
|
||||
* May load `/apps/get-started` → show "Build apps". This is the page's OWN gate
|
||||
* turned into a viewer fact: `resolveGetStartedAccess` grants on
|
||||
* `features.appBlocksGetStarted` and consults nothing else, so this is
|
||||
* `!!features.appBlocksGetStarted` and is NOT session-scoped (contrast
|
||||
* {@link isAuthor}, which needs a `currentUser`).
|
||||
*/
|
||||
canGetStarted: boolean;
|
||||
};
|
||||
|
||||
/** No capabilities — the shape a logged-out / non-author viewer resolves to. */
|
||||
const NO_CAPABILITIES: AppsNavContext = { isAuthor: false };
|
||||
/** No capabilities — the shape a viewer with neither flag resolves to. */
|
||||
const NO_CAPABILITIES: AppsNavContext = { isAuthor: false, canGetStarted: false };
|
||||
|
||||
type SubNavLink = {
|
||||
href: string;
|
||||
@@ -92,7 +101,7 @@ type SubNavLink = {
|
||||
/**
|
||||
* Tab order = discovery → author → manage → revenue → moderate.
|
||||
*
|
||||
* Only **Marketplace** is unconditional. "Create" links at `/apps/submit`,
|
||||
* **Marketplace** is the only unconditional entry. "Create" links at `/apps/submit`,
|
||||
* whose `getServerSideProps` gates on `features.appBlocksAuthor` +
|
||||
* `isAppDeveloper` and otherwise returns `notFound` — so a store-visible
|
||||
* NON-author (the widened `app-listings` tester cohort: `app-listings=true`,
|
||||
@@ -112,11 +121,58 @@ type SubNavLink = {
|
||||
* would re-create the very drift this change removes, and whether authoring
|
||||
* requires the block runtime is a product decision. See #3906.
|
||||
*
|
||||
* With "Create" conditional the bar can collapse to a single entry, so
|
||||
* {@link AppsSubNavView} hides itself entirely below two tabs — a one-tab
|
||||
* "navigation" is chrome that navigates nowhere.
|
||||
* {@link AppsSubNavView} still hides itself entirely below two tabs — a one-tab
|
||||
* "navigation" is chrome that navigates nowhere. 🔴 That floor IS reachable through
|
||||
* the container, and by a live cohort: a store-visible non-author with no installs,
|
||||
* no `appBlocksGetStarted` and an empty summary qualifies for Marketplace ALONE and
|
||||
* gets no bar. So the collapse tests below are covering a real viewer, not merely
|
||||
* pinning a property of the exported view.
|
||||
*/
|
||||
const SUB_NAV_LINKS: SubNavLink[] = [
|
||||
/**
|
||||
* The PUBLIC "App builders" landing page. It leads the table because the order is
|
||||
* discovery → author → manage → revenue → moderate and this is the front door for
|
||||
* someone who has not built anything yet.
|
||||
*
|
||||
* 🔴 GATED ON `c.canGetStarted`, WHICH IS THIS PAGE'S OWN GATE, for the same reason
|
||||
* "Create" below is gated on `c.isAuthor`. `/apps/get-started`'s
|
||||
* `getServerSideProps` runs `resolveGetStartedAccess`, which returns `notFound`
|
||||
* unless `features.appBlocksGetStarted` is set (and the page's client body renders
|
||||
* `<NotFound/>` on the same condition). The whole-bar gate on the container is an
|
||||
* OR, so a viewer with store access but WITHOUT that flag passes it — an unconditional
|
||||
* predicate here would offer them a tab into a 404. `app-dev-testers` is exactly that
|
||||
* shape today: `appBlocks` yes, `appBlocksGetStarted` no.
|
||||
*
|
||||
* The second reason is the kill switch. `app-blocks-get-started` exists to drop this
|
||||
* page without a deploy; every moderator holds `appBlocks`, so an unconditional tab
|
||||
* would keep offering the page after the switch was thrown. That is precisely the
|
||||
* argument §4 of `~/components/AppBlocks/IframeHost` makes for keeping this route OUT
|
||||
* of the app-block chrome nav — a bar that cannot read the flag must not advertise
|
||||
* the route. This bar CAN read it, so it does.
|
||||
*
|
||||
* The flag reaches the table the way every other one does: as a resolved viewer fact
|
||||
* on {@link AppsNavContext}, not as a raw flag read inside the predicate. It is
|
||||
* hydration-safe for the same reason `isAuthor` is — `appBlocksGetStarted` is
|
||||
* SSR-seeded into `pageProps.flags` and is not `toggleable`, so it is frozen and
|
||||
* identical on the server render and the first client paint (see the container).
|
||||
*
|
||||
* 🔴 AND THE COROLLARY, WHICH THIS GATE DOES NOT CLOSE: Marketplace below is
|
||||
* `() => true`, so a viewer admitted by the get-started term ALONE gets a Marketplace
|
||||
* tab pointing at `/apps`, which `resolveAppsPageAccess` answers with `notFound` for
|
||||
* them. NOT reachable today (`appBlocksGetStarted` is staged mod-only and a moderator
|
||||
* holds the store flags too); it becomes reachable the moment `app-blocks-get-started`
|
||||
* is flipped public in Flipt WITHOUT `app-listings` — a runtime toggle, no deploy.
|
||||
* Gating Marketplace on `hasAppsStoreAccess` is NOT the fix: it would drop that viewer
|
||||
* to this tab alone and the `< 2` collapse would hide the bar, deleting the tab for the
|
||||
* only cohort it exists for. The real fix is a get-started-aware `/apps` landing, and
|
||||
* that is a product decision.
|
||||
*/
|
||||
{
|
||||
href: '/apps/get-started',
|
||||
label: 'Build apps',
|
||||
icon: IconCode,
|
||||
visible: (_s, c) => c.canGetStarted,
|
||||
},
|
||||
{ href: '/apps', label: 'Marketplace', icon: IconBuildingStore, visible: () => true },
|
||||
{
|
||||
href: '/apps/submit',
|
||||
@@ -317,10 +373,18 @@ export function AppsSubNavView({
|
||||
* entry — this is the second-level navigation).
|
||||
*
|
||||
* Gated on the SHARED store-visibility predicate `hasAppsStoreAccess(features)`
|
||||
* — the SAME rule `resolveAppsPageAccess` enforces in `getServerSideProps` — and
|
||||
* on a logged-in user (the summary query is a `protectedProcedure`; an anon
|
||||
* viewer resolves to `NO_CAPABILITIES` + an empty summary ⇒ Marketplace alone ⇒
|
||||
* the bar hides itself entirely).
|
||||
* — the SAME rule `resolveAppsPageAccess` enforces in `getServerSideProps` —
|
||||
* OR on `features.appBlocksGetStarted`, which is the gate
|
||||
* `resolveGetStartedAccess` enforces for `/apps/get-started`. The union is
|
||||
* deliberate: the bar now carries a "Build apps" tab pointing at that page, so
|
||||
* the bar must exist for everyone either of the two pages admits. See the gate
|
||||
* itself for the hydration argument and the residual Marketplace-404 exposure.
|
||||
*
|
||||
* 🔴 THE `<2` COLLAPSE STILL FIRES, AND FOR THE SAME COHORT AS BEFORE THIS TAB
|
||||
* EXISTED. "Build apps" is gated on `appBlocksGetStarted`, so a store-visible
|
||||
* non-author with no installs and without that flag still qualifies for Marketplace
|
||||
* ALONE and still gets no chrome — unchanged behaviour, not a new regression. The
|
||||
* tab set only GROWS, and only for someone the destination would actually serve.
|
||||
*
|
||||
* 🔴 THIS GATE USED TO READ `features.appBlocks` ALONE while the page it sits on
|
||||
* granted access on `appListings || appBlocks`. The two could therefore disagree:
|
||||
@@ -365,20 +429,44 @@ export function AppsSubNav() {
|
||||
staleTime: 60_000,
|
||||
});
|
||||
|
||||
if (!hasAppsStoreAccess(features)) return null;
|
||||
// 🔴 THE BAR GATE IS `store OR get-started`, AND THE SECOND TERM IS REQUIRED, NOT
|
||||
// OPTIONAL. `/apps/get-started` is now a TAB in this bar, and it exists for exactly
|
||||
// the cohort that holds `appBlocksGetStarted` WITHOUT store access. On the old
|
||||
// `hasAppsStoreAccess`-only gate the container returns `null` for that viewer, so
|
||||
// the tab would be invisible to the only people it was added for — the page would
|
||||
// render its chrome band empty, exactly as `pages/apps/get-started.tsx` describes.
|
||||
// The SAME flag also drives `context.canGetStarted` below, so the gate and the tab
|
||||
// cannot disagree: this term admits nobody the tab then refuses.
|
||||
//
|
||||
// 🔴 SAFE ON THE FIRST PAINT, and for the SAME reason already written out for
|
||||
// `appBlocksAuthor` below — stated here rather than left implied. `appBlocksGetStarted`
|
||||
// is resolved server-side in `_app`'s `getInitialProps`
|
||||
// (`getFeatureFlagsAsync({ user: session.user, … })`, Flipt included), serialized into
|
||||
// `pageProps.flags`, and FROZEN by `useState(initialFlags)` in `FeatureFlagsProvider`.
|
||||
// It is NOT a `toggleable: true` flag, so `computeUserFeatureFlagsOverlay` never emits
|
||||
// it and the client `user.getFeatureFlags` overlay cannot move it. Server render and
|
||||
// first client render therefore compute the same boolean, so applying it here cannot
|
||||
// produce the tab-set hydration mismatch that `useIsClient` exists to prevent for the
|
||||
// client-only `getNavSummary` data.
|
||||
//
|
||||
// Widening DISCOVERY only: `/apps` itself still gates on `resolveAppsPageAccess`, so a
|
||||
// get-started-only viewer who clicks Marketplace gets the page's own 404 — the same
|
||||
// answer they got before this bar rendered for them.
|
||||
if (!hasAppsStoreAccess(features) && !features.appBlocksGetStarted) return null;
|
||||
|
||||
const summary = isClient ? data ?? EMPTY_SUMMARY : EMPTY_SUMMARY;
|
||||
|
||||
// 🔴 NOT gated on `useIsClient()` — deliberately, and verified against the
|
||||
// incident above rather than assumed. Both inputs are SSR-seeded and FROZEN,
|
||||
// incident above rather than assumed. All THREE inputs are SSR-seeded and FROZEN,
|
||||
// so this value is byte-identical on the server render and the first client
|
||||
// render, which is the whole condition for hydration safety:
|
||||
// • `features.appBlocksAuthor` is resolved server-side in `_app`'s
|
||||
// • `features.appBlocksAuthor` and `features.appBlocksGetStarted` are both
|
||||
// resolved server-side in `_app`'s
|
||||
// `getInitialProps` (`getFeatureFlagsAsync({ user: session.user, … })`,
|
||||
// Flipt included), serialized into `pageProps.flags`, and frozen by
|
||||
// `useState(initialFlags)` in `FeatureFlagsProvider`. It is NOT a
|
||||
// `useState(initialFlags)` in `FeatureFlagsProvider`. NEITHER is a
|
||||
// `toggleable: true` flag, so `computeUserFeatureFlagsOverlay` never emits
|
||||
// it and the client `user.getFeatureFlags` overlay cannot move it.
|
||||
// them and the client `user.getFeatureFlags` overlay cannot move them.
|
||||
// • `currentUser.isModerator` rides `SessionProvider`'s `useState(initial)`,
|
||||
// seeded from the same SSR `pageProps.session`. When that seed is
|
||||
// `undefined` (auth cookie present, session unresolved) the SERVER also
|
||||
@@ -386,9 +474,21 @@ export function AppsSubNav() {
|
||||
// session only arrives in a LATER render, post-hydration.
|
||||
// Contrast `getNavSummary` above, which really is client-only and therefore
|
||||
// really does need the `isClient` deferral.
|
||||
const context: AppsNavContext = currentUser
|
||||
? { isAuthor: isAppDeveloper(currentUser, { appBlocksAuthor: features.appBlocksAuthor }) }
|
||||
: NO_CAPABILITIES;
|
||||
//
|
||||
// 🔴 `canGetStarted` is NOT session-scoped, and that asymmetry is deliberate.
|
||||
// `isAuthor` is a capability OF A USER (`isAppDeveloper` reads `isModerator`), so
|
||||
// it collapses to `false` without a session. `resolveGetStartedAccess` — the gate
|
||||
// on the page this tab points at — reads the flag and NOTHING else, so mirroring it
|
||||
// means reading the flag and nothing else. Folding it into the `currentUser` branch
|
||||
// would hide the tab from a logged-out viewer the PAGE would happily serve if
|
||||
// `app-blocks-get-started` were flipped public, and (Marketplace being their only
|
||||
// other tab) the `< 2` collapse would then hide the whole bar from them.
|
||||
const context: AppsNavContext = {
|
||||
isAuthor: currentUser
|
||||
? isAppDeveloper(currentUser, { appBlocksAuthor: features.appBlocksAuthor })
|
||||
: NO_CAPABILITIES.isAuthor,
|
||||
canGetStarted: !!features.appBlocksGetStarted,
|
||||
};
|
||||
|
||||
return <AppsSubNavView summary={summary} context={context} currentPath={router.pathname} />;
|
||||
}
|
||||
|
||||
@@ -37,10 +37,20 @@ import { describe, it, expect } from 'vitest';
|
||||
* `appsNavVisibility.ts` — has behavioural cover in the same unit project
|
||||
* (`AppHeader/appsNavVisibility.test.ts`, which asserts the external-only and
|
||||
* catalog-only cohorts resolve `marketplace: true`), so its own argument IS
|
||||
* checked. What no test covers, there or here: that `useGetMenuItems` hands it
|
||||
* the real `useFeatureFlags()` object and wires `appsNav.marketplace` to the
|
||||
* right menu item. That seam is untested for BOTH nav entries and predates this
|
||||
* ledger — do not read the behavioural cover as reaching it.
|
||||
* checked.
|
||||
*
|
||||
* ⚠️ THE SEAM THIS PARAGRAPH USED TO CALL UNTESTED IS NOW HALF-COVERED, and the
|
||||
* remaining half is named rather than implied. It said: "that `useGetMenuItems`
|
||||
* hands it the real `useFeatureFlags()` object and wires `appsNav.marketplace` to
|
||||
* the right menu item" was untested for both nav entries. The two entries are now
|
||||
* ONE (`/apps` with store access, `/apps/get-started` without), and
|
||||
* `AppHeader/appsMenuEntry.test.ts` — also in this blocking unit project — extracts
|
||||
* that row's `href` / `visible` expressions from the real `hooks.tsx` source and
|
||||
* EVALUATES them across every combination of the two booleans. So the WIRING half is
|
||||
* covered. What is still not: that the object `useGetMenuItems` passes into
|
||||
* `appsNavVisibility(...)` is the real `useFeatureFlags()` result rather than some
|
||||
* other object — a source scan cannot see that, and no render-level test mounts this
|
||||
* hook.
|
||||
*
|
||||
* 🔴 AND THE SCOPE THAT MATTERS IN CI: the component suites above are REPORT-ONLY
|
||||
* (`preview / component-tests`) and do not block a merge. This unit-project ledger
|
||||
|
||||
@@ -96,11 +96,13 @@ export default function AppEditPage() {
|
||||
gates on `appBlocks` ALONE,
|
||||
with no author requirement, while `AppsSubNav` hides itself entirely below TWO
|
||||
qualifying tabs. Only "Marketplace" is unconditional; every other tab needs an
|
||||
author capability, an install, an approved app, a pending invite or reviewer
|
||||
status. So a viewer granted `app-blocks-enabled` in Flipt who is not a moderator,
|
||||
not an author, and holds none of those — a seated collaborator on someone else's
|
||||
listing is the realistic shape — reaches this page, qualifies for one tab, and
|
||||
gets an EMPTY chrome band: the `Stack gap="xl"` above the body and nothing else.
|
||||
author capability, an install, an approved app, a pending invite, reviewer status,
|
||||
or the `appBlocksGetStarted` flag ("Build apps" is gated on it, mirroring
|
||||
`resolveGetStartedAccess`). So a viewer granted `app-blocks-enabled` in Flipt who
|
||||
is not a moderator, not an author, and holds none of those — a seated collaborator
|
||||
on someone else's listing is the realistic shape — reaches this page, qualifies for
|
||||
one tab, and gets an EMPTY chrome band: the `Stack gap="xl"` above the body and
|
||||
nothing else.
|
||||
|
||||
Not a live defect (pre-GA the flag resolves for mods, who are authors), and not a
|
||||
correctness problem when it does happen — it is 32px of dead space, not a broken
|
||||
|
||||
@@ -53,46 +53,39 @@ export default function AppsGetStartedPage() {
|
||||
deIndex
|
||||
/>
|
||||
{/*
|
||||
🔴 THIS PAGE'S GATE DOES NOT IMPLY THE SUB-NAV'S. This page
|
||||
gates on `appBlocksGetStarted` ALONE (see the docstring above), while
|
||||
`AppsSubNav` renders only for `hasAppsStoreAccess` (`appListings || appBlocks`)
|
||||
AND hides itself below two qualifying tabs. Those are independent flags, so a
|
||||
viewer can hold this page's flag and not the nav's — and then the chrome band
|
||||
renders empty (the sub-nav returns `null`), costing the `Stack gap="xl"` above
|
||||
the body and nothing else.
|
||||
THE GATE MISMATCH THIS NOTE USED TO DESCRIBE IS CLOSED *FOR THIS PAGE*.
|
||||
`AppsSubNav`'s whole-bar gate is now
|
||||
`hasAppsStoreAccess(features) || features.appBlocksGetStarted` — the union of the
|
||||
two pages' own gates — and the bar carries a "Build apps" tab pointing here,
|
||||
gated on that same flag (`context.canGetStarted`). So a viewer admitted to THIS
|
||||
page is admitted to the bar AND to the tab by construction, and that tab plus the
|
||||
unconditional Marketplace clear the `< 2` collapse without depending on
|
||||
`appBlocksAuthor` or any summary flag. The empty-band case the old TODO(launch)
|
||||
asked someone to resolve before a Flipt widening no longer exists here, and the
|
||||
widening no longer needs a paired code change.
|
||||
|
||||
🔴 THE CRITERION, NOT A COUNT — apply it, do not trust a total written here. An
|
||||
earlier version of this note said "one of three", and it was wrong: it stated the
|
||||
two-tab floor as part of the test and then counted only the pages whose FLAG
|
||||
mismatches, missing `/apps` and `/apps/store-preview/[slug]`, which gate on
|
||||
`hasAppsStoreAccess` — a predicate that likewise does not imply two tabs (see
|
||||
`AppsSubNav.tsx`, which names the reachable cohort outright: `app-listings=true`,
|
||||
`app-blocks-author=false`, non-author, no installs ⇒ Marketplace alone). That was
|
||||
the second hand-maintained integer in this change to go stale, so there is no
|
||||
third one. THE TEST: a page can show an empty band iff its gate does not
|
||||
guarantee the viewer ≥2 qualifying sub-nav tabs. Read the page's
|
||||
`getServerSideProps`, then read `SUB_NAV_LINKS` + `if (links.length < 2) return
|
||||
null` in `AppsSubNav.tsx`, and decide. `/apps/[appBlockId]/revenue` is the useful
|
||||
contrast — it gates on `appBlocksAuthor` + `isAppDeveloper`, which guarantees the
|
||||
Create tab and therefore clears the floor.
|
||||
🔴 IT IS NOT CLOSED FOR THE OTHER `/apps/*` PAGES, and the same tab gate is why:
|
||||
a viewer holding a STORE flag without `appBlocksGetStarted` gets no "Build apps"
|
||||
tab, so a non-author with no installs is still back to Marketplace alone and an
|
||||
empty band on `/apps/[appBlockId]/edit` etc. That is `main`'s behaviour, unchanged
|
||||
— see the notes on those two pages, which point back here.
|
||||
|
||||
NOT REACHABLE TODAY: `appBlocksGetStarted` is staged mod-only, and a moderator
|
||||
holds `appBlocks` (hence the store predicate) and is an `isAppDeveloper`, so the
|
||||
bar clears its floor with Marketplace + Create.
|
||||
|
||||
🔴 BUT THE TRIGGER IS A RUNTIME TOGGLE, NOT A DEPLOY. An earlier version of this
|
||||
note said it becomes reachable "the moment this flag widens to ['public'] — a
|
||||
one-line change in feature-flags.service.ts". That is wrong, and wrong in the
|
||||
dangerous direction: it pins the hazard to an event a reviewer would see in a
|
||||
diff. `appBlocksGetStarted` is `{ availability: ['mod'], fliptKey:
|
||||
'app-blocks-get-started' }`, and `getFeatureFlags` returns the Flipt answer
|
||||
BEFORE it ever evaluates `availability` ("Flipt overrides role checks (both
|
||||
enable AND disable)"). So `availability` is only the Flipt-DOWN fallback: this
|
||||
page can widen to the public by flipping `app-blocks-get-started` in Flipt, with
|
||||
no code change, no PR and no deploy. All four App-Blocks flags are shaped this
|
||||
way (`appBlocks`, `appListings`, `appBlocksAuthor`, `appBlocksGetStarted`).
|
||||
TODO(launch): before any Flipt widening, either widen the sub-nav's gate with it
|
||||
or keep this page off the shared chrome.
|
||||
🔴 WHAT IS STILL TRUE, AND IS THE REASON THIS COMMENT SURVIVES AT ALL: the
|
||||
Marketplace tab in that bar is unconditional, and `/apps` gates on
|
||||
`resolveAppsPageAccess`. A viewer holding `appBlocksGetStarted` WITHOUT a store
|
||||
flag therefore sees a tab that answers `notFound`. Not reachable today — the flag
|
||||
is staged mod-only and a moderator holds the store flags too — but the trigger is
|
||||
a RUNTIME TOGGLE, not a deploy: `appBlocksGetStarted` is
|
||||
`{ availability: ['mod'], fliptKey: 'app-blocks-get-started' }`, and
|
||||
`getFeatureFlags` returns the Flipt answer BEFORE it evaluates `availability`
|
||||
("Flipt overrides role checks (both enable AND disable)"), so `availability` is
|
||||
only the Flipt-DOWN fallback. All four App-Blocks flags are shaped this way
|
||||
(`appBlocks`, `appListings`, `appBlocksAuthor`, `appBlocksGetStarted`). Flipping
|
||||
`app-blocks-get-started` public without `app-listings` makes that 404 reachable
|
||||
with no PR and no deploy. Gating the Marketplace tab is NOT the fix — it drops
|
||||
such a viewer to one tab and the collapse hides the whole bar, deleting the
|
||||
"Build apps" tab for the only cohort it exists for. See the note on that entry in
|
||||
`~/components/Apps/AppsSubNav`.
|
||||
*/}
|
||||
<AppsPageLayout measure={APPS_PAGE_MEASURES['/apps/get-started']}>
|
||||
<GetStartedBody />
|
||||
|
||||
@@ -190,11 +190,13 @@ export default function AppListingEditPage() {
|
||||
gates on `appBlocks` ALONE,
|
||||
with no author requirement, while `AppsSubNav` hides itself entirely below TWO
|
||||
qualifying tabs. Only "Marketplace" is unconditional; every other tab needs an
|
||||
author capability, an install, an approved app, a pending invite or reviewer
|
||||
status. So a viewer granted `app-blocks-enabled` in Flipt who is not a moderator,
|
||||
not an author, and holds none of those — a seated collaborator on someone else's
|
||||
listing is the realistic shape — reaches this page, qualifies for one tab, and
|
||||
gets an EMPTY chrome band: the `Stack gap="xl"` above the body and nothing else.
|
||||
author capability, an install, an approved app, a pending invite, reviewer status,
|
||||
or the `appBlocksGetStarted` flag ("Build apps" is gated on it, mirroring
|
||||
`resolveGetStartedAccess`). So a viewer granted `app-blocks-enabled` in Flipt who
|
||||
is not a moderator, not an author, and holds none of those — a seated collaborator
|
||||
on someone else's listing is the realistic shape — reaches this page, qualifies for
|
||||
one tab, and gets an EMPTY chrome band: the `Stack gap="xl"` above the body and
|
||||
nothing else.
|
||||
|
||||
Not a live defect (pre-GA the flag resolves for mods, who are authors), and not a
|
||||
correctness problem when it does happen — it is 32px of dead space, not a broken
|
||||
|
||||
@@ -48,9 +48,14 @@ import { trpcQuery } from './preview-trpc';
|
||||
* redirects to `/apps/store-preview/<slug>` (or `notFound` when the app has no
|
||||
* approved listing). It no longer renders, so this spec asserts the REDIRECT
|
||||
* rather than a heading on it.
|
||||
* - Marketplace page (`/apps/index.tsx`) renders the `AppsSubNav` tabs bar — its
|
||||
* first tab is `{ href: '/apps', label: 'Marketplace' }` (AppsSubNav.tsx:55) —
|
||||
* plus a search `TextInput` (placeholder "Search by name or block id"). It no
|
||||
* - Marketplace page (`/apps/index.tsx`) renders the `AppsSubNav` tabs bar, whose
|
||||
* rows come from `SUB_NAV_LINKS` in `~/components/Apps/AppsSubNav`. The row this
|
||||
* spec keys on is `{ href: '/apps', label: 'Marketplace' }` — the only
|
||||
* UNCONDITIONAL one. It is NOT necessarily the first tab: `/apps/get-started`
|
||||
* ("Build apps") precedes it for any viewer holding `appBlocksGetStarted`, which a
|
||||
* mod does, so the assertion below selects the tab BY NAME and the order is
|
||||
* irrelevant to it. Plus a search `TextInput` (placeholder "Search by name or block
|
||||
* id"). It no
|
||||
* longer renders a `<Title>Civitai App Blocks</Title>`: the app-blocks nav
|
||||
* refactor (#2749/#2758) made `AppsPageLayout` DELIBERATELY OMIT the page title
|
||||
* on the marketplace surface ("omit for a header with just the tabs, e.g. the
|
||||
|
||||
Reference in New Issue
Block a user