mirror of
https://github.com/civitai/civitai.git
synced 2026-09-20 22:08:18 +08:00
fix(apps): an off-site listing with an OAuth client no longer renders a dead CTA (#3585)
* fix(apps): an off-site listing with an OAuth client no longer renders a dead CTA Three approved, live, moderator-visible off-site listings rendered a dead primary action reading "Connecting this app will be available soon." — a disabled button with no href — so there was no way to open them from the store at all. Mechanism. `getDetailPrimaryAction`'s final fallthrough returned the connect STUB unconditionally, and `resolveOffsiteSubKind` decides the sub-kind on `connectClientId != null` alone. So linking an OAuth client was the ONLY thing that moved a listing off the working `Visit ↗` path. All four off-site listings were enumerated: the three with a client were dead; the one without worked. The stub's stated premise was false. Its docstring justified itself with "a complete OAuth authorize URL is NOT derivable from the public DTO", but these are confidential clients that own their own redirect_uri/state/PKCE and start the flow from their own site — the store never needed to build an authorize URL, only to get the viewer to the app. That destination is already on the public DTO as `externalUrl`, already https-guarded, and the external-link branch has always rendered it correctly. Fix: in both the detail and the card view-models, the DESTINATION decides the action, not the sub-kind. An off-site listing with an https `externalUrl` takes the existing, proven external-link path (`safeExternalHref` → `Visit ↗`) whichever sub-kind it is. The stub survives only for a connect listing with genuinely no destination, where it still fails safe (no href, no dead nav). Card/detail coherence: `getListingCta` gave every connect card "View details", pointing at a detail page whose affordance was the dead stub. It now mirrors the detail — direct `Visit ↗` when there is a destination, "View details" when there is not. No server, DTO or `resolveOffsiteSubKind` change; no new copy beyond what the external-link branch already used. * docs(apps): correct a false 'intent reversed' claim in the test commentary The comment above the new connect+destination case said this PR reverses a deliberately-pinned invariant. It does not, and an audit caught it. The replaced test passed `connectClientId: 'client-123'` and asserted `href` was undefined, which reads as 'even with a client_id, produce nothing'. But the fixture destructures `externalUrl = null` by default (line 51), so what it actually pinned was the destination-LESS case: connect + no address -> stub. That behaviour is unchanged by this PR — every assertion in the replaced test still holds, and they are now re-pinned over four absence shapes (null, '', http://, javascript:) instead of one, each carrying the client_id. What the new case adds is the shape nothing covered before: connect WITH a valid https address. So this is additive coverage, not a reversal. Left as permanent source commentary, an 'INTENT REVERSED' banner would have told the next reader that a safety property had been deliberately removed — the exact comment-overclaim class this codebase keeps getting bitten by, where the comment IS the control because a reviewer who reads it stops checking. Verified: appListingDetailView.test.ts 22 passed, rc=0. Four absence shapes confirmed present by reading the enumeration, not assumed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Zachary Lowden
@@ -220,11 +220,14 @@ describe('AppListingCard', () => {
|
||||
await expect.element(visit).toHaveAttribute('rel', 'noopener noreferrer');
|
||||
});
|
||||
|
||||
test('off-site connect → View details → unified detail (P2c)', async () => {
|
||||
// P2c: cards route to the unified detail; the Connect action itself lives on
|
||||
// the detail page (the connect flow needs a P2a authorize-URL DTO addition),
|
||||
// so the card's CTA is "View details", not an inert Connect button. The
|
||||
// former "Connect app" kind badge is gone — no longer asserted here.
|
||||
test('off-site connect with NO external target → View details → unified detail', async () => {
|
||||
// 🔴 `externalUrl: null` is what makes this the View-details case, NOT the
|
||||
// sub-kind. The comment here used to say cards route connect listings to the
|
||||
// detail because "the connect flow needs a P2a authorize-URL DTO addition" —
|
||||
// that premise was wrong and produced a dead end (the detail's Connect
|
||||
// affordance was an inert stub). A connect listing WITH an https
|
||||
// `externalUrl` now gets a direct Visit ↗; see the test below. The former
|
||||
// "Connect app" kind badge is gone — no longer asserted here.
|
||||
renderWithProviders(
|
||||
<AppListingCard
|
||||
card={base({
|
||||
@@ -238,6 +241,27 @@ describe('AppListingCard', () => {
|
||||
await expect.element(details).toHaveAttribute('href', '/apps/store-preview/my-app');
|
||||
});
|
||||
|
||||
test('🔴 off-site connect WITH an https externalUrl → Visit ↗ external anchor', async () => {
|
||||
// The rendered counterpart of the view-model reversal: a linked OAuth client
|
||||
// no longer strands the card on "View details". Asserted on the real anchor
|
||||
// (href + target + rel), not just the label, because the whole defect was an
|
||||
// affordance that looked present and went nowhere.
|
||||
renderWithProviders(
|
||||
<AppListingCard
|
||||
card={base({
|
||||
kind: 'offsite',
|
||||
name: 'Connect App',
|
||||
kindData: { kind: 'offsite', subKind: 'connect', externalUrl: 'https://connect.app' },
|
||||
})}
|
||||
/>
|
||||
);
|
||||
await expect.element(page.getByRole('link', { name: 'View details' })).not.toBeInTheDocument();
|
||||
const visit = page.getByRole('link', { name: 'Visit' });
|
||||
await expect.element(visit).toHaveAttribute('href', 'https://connect.app');
|
||||
await expect.element(visit).toHaveAttribute('target', '_blank');
|
||||
await expect.element(visit).toHaveAttribute('rel', 'noopener noreferrer');
|
||||
});
|
||||
|
||||
test('on-site page app WITHOUT canOpenPage → View details → unified detail (P2c)', async () => {
|
||||
renderWithProviders(<AppListingCard card={base({})} canOpenPage={false} />);
|
||||
const details = page.getByRole('link', { name: 'View details' });
|
||||
|
||||
@@ -483,8 +483,42 @@ describe('AppListingDetailBody', () => {
|
||||
// mutation and an info-branch mutation each be observed on the same run
|
||||
// instead of the first one short-circuiting the second.
|
||||
|
||||
test('🔴 a connect listing WITH an https externalUrl renders a real Visit ↗ anchor', async () => {
|
||||
// The rendered reproduction of the bug this change fixes: an approved
|
||||
// off-site listing with a linked OAuth client used to render an INERT
|
||||
// disabled "Connect" button reading "Connecting this app will be available
|
||||
// soon." — the viewer had no way to open the app at all. It now takes the
|
||||
// same `visit` branch as any other off-site listing.
|
||||
//
|
||||
// Asserted through `renderAndSettle`, which keys on the CTA's href: this
|
||||
// test cannot pass unless a real anchor to the app's address exists.
|
||||
const visitBtn = await renderAndSettle(
|
||||
<AppListingDetailBody
|
||||
detail={base({
|
||||
kind: 'offsite',
|
||||
kindData: {
|
||||
kind: 'offsite',
|
||||
subKind: 'connect',
|
||||
externalUrl: 'https://connect.app',
|
||||
connectClientId: 'oauth-client-1',
|
||||
},
|
||||
})}
|
||||
/>,
|
||||
'https://connect.app'
|
||||
);
|
||||
expect(visitBtn.textContent?.trim()).toBe('Visit');
|
||||
expect(visitBtn.getAttribute('target')).toBe('_blank');
|
||||
expect(visitBtn.getAttribute('rel')).toBe('noopener noreferrer');
|
||||
expect(glyphOf(visitBtn)).toBe('external-link');
|
||||
// The stub copy must be GONE from this state — it is the exact string a user
|
||||
// reported as a dead end.
|
||||
expect(document.body.textContent).not.toContain('Connecting this app will be available soon');
|
||||
});
|
||||
|
||||
test('the connect branch keeps its own glyph', async () => {
|
||||
// off-site OAuth → the `connect` stub (disabled button + note).
|
||||
// off-site OAuth with NO destination (`externalUrl: null`) → the `connect`
|
||||
// stub (disabled button + note). 🔴 The null url is what selects this branch
|
||||
// now, not the sub-kind — see the test above.
|
||||
await renderWithProviders(
|
||||
<AppListingDetailBody
|
||||
detail={base({
|
||||
|
||||
@@ -379,8 +379,15 @@ function PrimaryAction({ detail, canOpenPage }: { detail: ListingDetail; canOpen
|
||||
}
|
||||
|
||||
if (action.mode === 'connect') {
|
||||
// Honest stub — no derivable OAuth authorize URL from the public DTO. Inert
|
||||
// button + a note so the affordance is never a dead 404 link.
|
||||
// Honest stub for a connect listing with NO destination at all: inert button
|
||||
// + a note, so the affordance is never a dead 404 link.
|
||||
//
|
||||
// 🔴 This is NOT the ordinary connect case any more. A connect listing that
|
||||
// carries an https `externalUrl` — which is every one in production — takes
|
||||
// the `visit` branch above and renders a real `Visit ↗`. The mode used to be
|
||||
// returned unconditionally for the sub-kind, which made this inert button
|
||||
// the ONLY thing a viewer ever saw for an app with a linked OAuth client.
|
||||
// See `getDetailPrimaryAction`.
|
||||
const GlyphIcon = glyphFor('connect');
|
||||
return (
|
||||
<Stack gap={4}>
|
||||
|
||||
@@ -189,14 +189,44 @@ describe('getListingCta — off-site (P2c: View details → unified detail)', ()
|
||||
external: false,
|
||||
});
|
||||
});
|
||||
it('connect → View details → unified detail (Connect affordance lives on the detail page)', () => {
|
||||
expect(getListingCta(offsiteCard('connect', null), { canOpenPage: true })).toEqual({
|
||||
label: 'View details',
|
||||
action: 'detail',
|
||||
href: '/apps/store-preview/ext-app',
|
||||
external: false,
|
||||
/**
|
||||
* 🔴 INTENT CHANGED alongside the detail view-model. Connect used to route to
|
||||
* "View details" unconditionally, on the premise that the Connect affordance
|
||||
* lived on the detail page — but that affordance was a dead stub, so the card
|
||||
* sent the viewer to a page with no way to open the app. The detail now
|
||||
* renders a real `Visit ↗` whenever an off-site listing carries an https
|
||||
* `externalUrl`; the card matches, so the two cannot disagree about whether an
|
||||
* app is reachable.
|
||||
*/
|
||||
it('🔴 connect + https externalUrl → Visit ↗ (matches the detail, was View details)', () => {
|
||||
expect(
|
||||
getListingCta(offsiteCard('connect', 'https://connect.app'), { canOpenPage: true })
|
||||
).toEqual({
|
||||
label: 'Visit',
|
||||
action: 'visit',
|
||||
href: 'https://connect.app',
|
||||
external: true,
|
||||
});
|
||||
});
|
||||
it('🔴 connect and external-link with the SAME url produce the SAME CTA', () => {
|
||||
const url = 'https://same-target.app';
|
||||
expect(getListingCta(offsiteCard('connect', url), { canOpenPage: true })).toEqual(
|
||||
getListingCta(offsiteCard('external-link', url), { canOpenPage: true })
|
||||
);
|
||||
});
|
||||
it('connect with no usable target → View details → unified detail (fails safe)', () => {
|
||||
for (const externalUrl of [null, '', 'http://insecure.app', 'javascript:alert(1)']) {
|
||||
expect(
|
||||
getListingCta(offsiteCard('connect', externalUrl), { canOpenPage: true }),
|
||||
String(externalUrl)
|
||||
).toEqual({
|
||||
label: 'View details',
|
||||
action: 'detail',
|
||||
href: '/apps/store-preview/ext-app',
|
||||
external: false,
|
||||
});
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('getOwnerEditHref (owner Edit deep-link)', () => {
|
||||
|
||||
@@ -301,15 +301,112 @@ describe('getDetailPrimaryAction — off-site', () => {
|
||||
'info'
|
||||
);
|
||||
});
|
||||
it('connect → Connect stub (mode connect, no dead href, note set)', () => {
|
||||
const action = getDetailPrimaryAction(offsiteDetail('connect', { connectClientId: 'client-123' }), {
|
||||
canOpenPage: true,
|
||||
/**
|
||||
* COVERAGE ADDED, NOTHING REVERSED — and the distinction matters, because an
|
||||
* earlier version of this comment claimed the opposite and was wrong.
|
||||
*
|
||||
* The test replaced here passed `connectClientId: 'client-123'` and asserted
|
||||
* `href` was undefined. It read like "even with a client_id, produce
|
||||
* nothing", but the fixture defaults `externalUrl` to `null`, so what it
|
||||
* actually pinned was the destination-LESS case: connect + no address → stub.
|
||||
* That behaviour is UNCHANGED — every one of its assertions still holds, and
|
||||
* they are re-pinned below over four absence shapes instead of one.
|
||||
*
|
||||
* What this case adds is the shape nothing covered: connect WITH a valid
|
||||
* https address. `resolveOffsiteSubKind` flips the sub-kind on
|
||||
* `connectClientId != null` alone, so linking an OAuth client was the sole
|
||||
* cause of the dead CTA on three approved, live listings. The destination
|
||||
* decides now — the sub-kind does not.
|
||||
*/
|
||||
it('🔴 connect + https externalUrl → Visit ↗ (a client_id no longer kills the CTA)', () => {
|
||||
const action = getDetailPrimaryAction(
|
||||
offsiteDetail('connect', {
|
||||
connectClientId: 'client-123',
|
||||
externalUrl: 'https://connect.app',
|
||||
}),
|
||||
{ canOpenPage: true }
|
||||
);
|
||||
// Byte-identical to the external-link result for the same URL — the point of
|
||||
// the fix is that connect REUSES that path rather than growing a second one.
|
||||
expect(action).toEqual({
|
||||
label: 'Visit',
|
||||
mode: 'visit',
|
||||
href: 'https://connect.app',
|
||||
external: true,
|
||||
});
|
||||
expect(action.mode).toBe('connect');
|
||||
expect(action.label).toBe('Connect');
|
||||
expect(action.href).toBeUndefined();
|
||||
expect(action.external).toBe(false);
|
||||
expect(action.note).toBeTruthy();
|
||||
});
|
||||
|
||||
it('🔴 connect and external-link with the SAME url produce the SAME action', () => {
|
||||
// Structural restatement of the rule, independent of the literals above: if
|
||||
// a future change re-branches on sub-kind before the href guard, these two
|
||||
// diverge and this fails — even if it picks copy that satisfies the pins.
|
||||
const url = 'https://same-target.app';
|
||||
expect(
|
||||
getDetailPrimaryAction(
|
||||
offsiteDetail('connect', { connectClientId: 'c1', externalUrl: url }),
|
||||
{
|
||||
canOpenPage: true,
|
||||
}
|
||||
)
|
||||
).toEqual(
|
||||
getDetailPrimaryAction(offsiteDetail('external-link', { externalUrl: url }), {
|
||||
canOpenPage: true,
|
||||
})
|
||||
);
|
||||
});
|
||||
|
||||
it('🔴 connect with NO usable destination → the stub still fails safe', () => {
|
||||
// The stub must stay REACHABLE and must stay hrefless. Enumerated over every
|
||||
// way a destination can be absent, with the client_id present in each — so
|
||||
// this cannot pass by accident of the sub-kind being mis-resolved.
|
||||
// (`undefined` is deliberately absent: the DTO types `externalUrl` as
|
||||
// `string | null`, and the fixture's destructuring default would silently
|
||||
// rewrite it to `null` anyway — a row whose label lied about its input.)
|
||||
for (const externalUrl of [null, '', 'http://insecure.app', 'javascript:alert(1)']) {
|
||||
const key = `externalUrl=${String(externalUrl)}`;
|
||||
const action = getDetailPrimaryAction(
|
||||
offsiteDetail('connect', { connectClientId: 'client-123', externalUrl }),
|
||||
{ canOpenPage: true }
|
||||
);
|
||||
expect(action.mode, key).toBe('connect');
|
||||
expect(action.label, key).toBe('Connect');
|
||||
expect(action.href, key).toBeUndefined();
|
||||
expect(action.external, key).toBe(false);
|
||||
expect(action.note, key).toBeTruthy();
|
||||
}
|
||||
});
|
||||
|
||||
it('🔴 no off-site listing with an https target is ever left un-navigable', () => {
|
||||
// The off-site analogue of the on-site "no state strands the viewer" matrix.
|
||||
// Anti-vacuity is explicit: count the rows that MUST be navigable and assert
|
||||
// the count, so a change that stopped producing hrefs everywhere cannot make
|
||||
// this green by having nothing to check.
|
||||
const stranded: string[] = [];
|
||||
let navigable = 0;
|
||||
for (const subKind of ['connect', 'external-link'] as const) {
|
||||
for (const externalUrl of ['https://ok.app', 'http://insecure.app', null]) {
|
||||
for (const connectClientId of ['client-123', null]) {
|
||||
const key = `subKind=${subKind} url=${String(externalUrl)} client=${
|
||||
connectClientId ?? 'null'
|
||||
}`;
|
||||
const action = getDetailPrimaryAction(
|
||||
offsiteDetail(subKind, { externalUrl, connectClientId }),
|
||||
{ canOpenPage: true }
|
||||
);
|
||||
// `visit` is the only external mode and may only carry an https target.
|
||||
expect(action.external, key).toBe(action.mode === 'visit');
|
||||
if (action.mode === 'visit') {
|
||||
expect(action.href, key).toMatch(/^https:\/\//);
|
||||
navigable++;
|
||||
} else if (externalUrl?.startsWith('https://')) {
|
||||
stranded.push(key);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
expect(stranded).toEqual([]);
|
||||
// 2 sub-kinds × 2 client values, all with the https url → 4 navigable rows.
|
||||
expect(navigable).toBe(4);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -46,7 +46,9 @@ export const ACTION_GLYPH_ICONS: Record<PrimaryActionGlyph, Icon> = {
|
||||
launch: IconPlayerPlay,
|
||||
/** Leaves civitai.com in a new tab (`target="_blank"` + `rel="noopener noreferrer"`). */
|
||||
external: IconExternalLink,
|
||||
/** OAuth connect affordance (stubbed until the cutover wires it). */
|
||||
/** OAuth connect affordance — the inert stub for a connect listing with NO
|
||||
* destination. A connect listing WITH an https `externalUrl` takes the
|
||||
* `external` glyph via `visit`, like any other off-site app. */
|
||||
connect: IconPlugConnected,
|
||||
/** Informational — no launch, no navigation off-site, and no target to go to. */
|
||||
info: IconInfoCircle,
|
||||
@@ -101,8 +103,11 @@ export function detailActionGlyph(mode: DetailActionMode): PrimaryActionGlyph {
|
||||
* already ships for this action, on both the card and the recents rail.
|
||||
*
|
||||
* `AppListingCard` is the live caller. `getListingCta` still does not emit
|
||||
* `'connect'` (the off-site OAuth case routes to `'detail'`), so that arm remains
|
||||
* unreachable from live data; it is mapped for totality over the type.
|
||||
* `'connect'` at all: an off-site connect listing now takes the `'visit'` arm
|
||||
* when it carries an https `externalUrl` and `'detail'` when it does not, so the
|
||||
* `'connect'` arm remains unreachable from live data and is mapped for totality
|
||||
* over the type. (The DETAIL view-model does still emit `connect`, for a connect
|
||||
* listing with no destination — `detailActionGlyph`'s arm is live.)
|
||||
*/
|
||||
export function cardActionGlyph(action: ListingCtaAction): PrimaryActionGlyph {
|
||||
switch (action) {
|
||||
|
||||
@@ -21,13 +21,22 @@
|
||||
* there when the viewer can actually open it) — the direct primary action.
|
||||
* - on-site otherwise (no page, or page but no `appBlocksPages`) → **View
|
||||
* details** → `/apps/store-preview/<slug>` (the unified P2c detail).
|
||||
* - off-site external-link (https) → **Visit ↗** → external anchor (direct
|
||||
* primary action).
|
||||
* - off-site external-link (missing / non-https url) → **View details** →
|
||||
* the unified detail (the DTO already null-guards non-https — we re-guard;
|
||||
* the detail page shows the informational state).
|
||||
* - off-site connect → **View details** → the unified detail (the Connect
|
||||
* affordance lives on the detail page).
|
||||
* - off-site, EITHER sub-kind, with an https `externalUrl` → **Visit ↗** →
|
||||
* external anchor (direct primary action). 🔴 The sub-kind does NOT decide
|
||||
* this; the presence of a destination does — see below.
|
||||
* - off-site with no usable target (missing / non-https url, either sub-kind)
|
||||
* → **View details** → the unified detail (the DTO already null-guards
|
||||
* non-https — we re-guard; the detail page shows the informational state).
|
||||
*
|
||||
* 🔴 Connect used to route to "View details" UNCONDITIONALLY, on the premise
|
||||
* that "the Connect affordance lives on the detail page". That affordance was a
|
||||
* dead stub, so the card handed the viewer a detail page with nothing on it —
|
||||
* the card half of the same defect. The detail now renders a real `Visit ↗` for
|
||||
* any off-site listing carrying an https `externalUrl` (see
|
||||
* `appListingDetailView`), so the card matches it: a connect listing with a
|
||||
* destination gets the direct Visit, and only a listing with NO destination
|
||||
* falls back to the detail. Card and detail must not disagree about whether an
|
||||
* app is reachable.
|
||||
* Every CTA now has a working `href` (never actionless). The card ALSO links its
|
||||
* title to the detail (via `getListingDetailHref`) so the detail is reachable
|
||||
* even when the primary CTA is a direct Open / Visit.
|
||||
@@ -128,17 +137,17 @@ export function getListingCta(
|
||||
return { label: 'View details', action: 'detail', href: detailHref, external: false };
|
||||
}
|
||||
|
||||
// Off-site.
|
||||
if (card.kindData.subKind === 'external-link') {
|
||||
const href = safeExternalHref(card.kindData.externalUrl);
|
||||
if (href) {
|
||||
return { label: 'Visit', action: 'visit', href, external: true };
|
||||
}
|
||||
// No usable external target (missing / non-https) → the unified detail.
|
||||
return { label: 'View details', action: 'detail', href: detailHref, external: false };
|
||||
// Off-site — BOTH sub-kinds. The destination decides, not the sub-kind: a
|
||||
// connect app is reached at its own address exactly like a plain external
|
||||
// link (it starts its own OAuth flow from there). Mirrors
|
||||
// `getDetailPrimaryAction`; do NOT reintroduce a sub-kind test above this
|
||||
// line, or the card and the detail disagree again.
|
||||
const href = safeExternalHref(card.kindData.externalUrl);
|
||||
if (href) {
|
||||
return { label: 'Visit', action: 'visit', href, external: true };
|
||||
}
|
||||
|
||||
// Off-site connect (OAuth) — the Connect affordance lives on the detail page.
|
||||
// No usable external target (missing / non-https, either sub-kind) → the
|
||||
// unified detail, which shows the informational / connect-stub state.
|
||||
return { label: 'View details', action: 'detail', href: detailHref, external: false };
|
||||
}
|
||||
|
||||
|
||||
@@ -56,13 +56,31 @@
|
||||
* explicitly in the "no on-site state strands the viewer" matrix test — but
|
||||
* the branch is vacuous today: every approved on-site listing declares a
|
||||
* page, so nothing takes it.
|
||||
* - off-site external-link (https) → **Visit ↗** → external anchor.
|
||||
* - off-site external-link (missing / non-https) → **informational** (guarded
|
||||
* out; no target).
|
||||
* - off-site connect (OAuth) → **Connect** STUB: a complete OAuth authorize
|
||||
* URL is NOT derivable from the public DTO (needs redirect_uri /
|
||||
* response_type / scope), so the connect flow is an honest stub with a note
|
||||
* until the cutover wires it — no dead 404 nav.
|
||||
* - off-site, EITHER sub-kind, with an https `externalUrl` → **Visit ↗** →
|
||||
* external anchor. 🔴 The sub-kind does NOT decide this; the presence of a
|
||||
* destination does. See the connect note below.
|
||||
* - off-site external-link with no usable target (missing / non-https) →
|
||||
* **informational** (guarded out; no target).
|
||||
* - off-site connect (OAuth) with no usable target → **Connect** STUB with a
|
||||
* note — the only remaining state with nowhere to send the viewer.
|
||||
*
|
||||
* 🔴 THE CONNECT STUB USED TO BE UNCONDITIONAL, AND THAT WAS THE BUG. Every
|
||||
* off-site listing with a linked OAuth client rendered a dead
|
||||
* "Connecting this app will be available soon." affordance — no href, disabled
|
||||
* button — because `resolveOffsiteSubKind` routes on `connectClientId != null`
|
||||
* alone (`app-listing.service.ts`), so linking a client was the ONLY thing that
|
||||
* moved a listing off the working `Visit ↗` path. Three approved, live listings
|
||||
* were in that state; a fourth, with no client, worked.
|
||||
*
|
||||
* The stub's stated premise — "a complete OAuth authorize URL is NOT derivable
|
||||
* from the public DTO" — is true and irrelevant. These are CONFIDENTIAL clients
|
||||
* that own their own `redirect_uri` / `state` / PKCE and start the flow from
|
||||
* their own site; the store never needed to build an authorize URL, only to get
|
||||
* the viewer to the app. That destination is already on the public DTO as
|
||||
* `externalUrl`, https-guarded, and the external-link branch has always
|
||||
* rendered it correctly. So connect now reuses that same proven path rather
|
||||
* than growing a second one, and the stub survives only for a connect listing
|
||||
* with genuinely no destination — where it still fails safe.
|
||||
*/
|
||||
|
||||
import { safeExternalHref } from '~/components/Apps/appListingCardView';
|
||||
@@ -171,10 +189,17 @@ export function getDetailPrimaryAction(
|
||||
};
|
||||
}
|
||||
|
||||
// Off-site.
|
||||
// Off-site — BOTH sub-kinds. An off-site app lives at its own address, and
|
||||
// that address is the only thing this page can route to; a connect app then
|
||||
// runs its own confidential-client OAuth flow from there. So the destination,
|
||||
// not the sub-kind, decides the action: one https-guarded path
|
||||
// (`safeExternalHref`), shared with the external-link case that has always
|
||||
// worked. Do NOT reintroduce a sub-kind test above this line — that is
|
||||
// precisely what made a linked OAuth client the sole cause of a dead CTA.
|
||||
const href = safeExternalHref(kd.externalUrl);
|
||||
if (href) return { label: 'Visit', mode: 'visit', href, external: true };
|
||||
|
||||
if (kd.subKind === 'external-link') {
|
||||
const href = safeExternalHref(kd.externalUrl);
|
||||
if (href) return { label: 'Visit', mode: 'visit', href, external: true };
|
||||
return {
|
||||
label: 'Unavailable',
|
||||
mode: 'info',
|
||||
@@ -183,7 +208,8 @@ export function getDetailPrimaryAction(
|
||||
};
|
||||
}
|
||||
|
||||
// Off-site connect (OAuth) — honest stub (see docstring: no derivable authorize URL).
|
||||
// Off-site connect with NO usable destination — the honest stub, now reached
|
||||
// only in that genuinely-nowhere-to-go state. Fails safe: no dead nav.
|
||||
return {
|
||||
label: 'Connect',
|
||||
mode: 'connect',
|
||||
|
||||
Reference in New Issue
Block a user