diff --git a/src/__tests__/ssr-utils.test.ts b/src/__tests__/ssr-utils.test.ts index cd14ebc2c..9f0f8697c 100644 --- a/src/__tests__/ssr-utils.test.ts +++ b/src/__tests__/ssr-utils.test.ts @@ -296,6 +296,82 @@ describe('updateUserStateSSR', () => { expect(props.dehydratedAppState).not.toHaveProperty('searchMode'); }); + describe('fromADS param handling', () => { + test('seeds ASTROPHYSICS from fromADS=true with no cookie', async () => { + const context = getMockContext({}, { fromADS: 'true' }, '/'); + const result = await updateUserStateSSR(context, { props: {} }); + if (!('props' in result)) { + throw new Error('Expected props'); + } + const props = result.props as SSRPropsWithState; + expect(props.dehydratedAppState).toEqual(expect.objectContaining({ mode: AppMode.ASTROPHYSICS })); + }); + + test('fromADS=true takes priority over a different prefs cookie mode', async () => { + const prefs = { mode: 'HELIOPHYSICS' }; + const cookie = `scix_prefs=${encodeURIComponent(JSON.stringify(prefs))}`; + const context = getMockContext({}, { fromADS: 'true' }, '/', undefined, cookie); + const result = await updateUserStateSSR(context, { props: {} }); + if (!('props' in result)) { + throw new Error('Expected props'); + } + const props = result.props as SSRPropsWithState; + expect(props.dehydratedAppState).toEqual(expect.objectContaining({ mode: AppMode.ASTROPHYSICS })); + }); + + test('forceMode still takes priority over fromADS', async () => { + const context = getMockContext({}, { forceMode: 'heliophysics', fromADS: 'true' }, '/'); + const result = await updateUserStateSSR(context, { props: {} }); + if (!('props' in result)) { + throw new Error('Expected props'); + } + const props = result.props as SSRPropsWithState; + expect(props.dehydratedAppState).toEqual(expect.objectContaining({ mode: AppMode.HELIOPHYSICS })); + }); + + test('an unmappable forceMode leaves fromADS in charge', async () => { + const context = getMockContext({}, { forceMode: 'banana', fromADS: 'true' }, '/'); + const result = await updateUserStateSSR(context, { props: {} }); + if (!('props' in result)) { + throw new Error('Expected props'); + } + const props = result.props as SSRPropsWithState; + expect(props.dehydratedAppState).toEqual(expect.objectContaining({ mode: AppMode.ASTROPHYSICS })); + }); + + test('fromADS=true does not enable ADS compatibility search mode', async () => { + const context = getMockContext({}, { fromADS: 'true' }, '/'); + const result = await updateUserStateSSR(context, { props: {} }); + if (!('props' in result)) { + throw new Error('Expected props'); + } + const props = result.props as SSRPropsWithState; + expect(props.dehydratedAppState).not.toHaveProperty('searchMode'); + }); + + test('ignores fromADS outside the home page, keeping the persisted discipline', async () => { + const prefs = { mode: 'HELIOPHYSICS' }; + const cookie = `scix_prefs=${encodeURIComponent(JSON.stringify(prefs))}`; + const context = getMockContext({}, { fromADS: 'true' }, '/search', undefined, cookie); + const result = await updateUserStateSSR(context, { props: {} }); + if (!('props' in result)) { + throw new Error('Expected props'); + } + const props = result.props as SSRPropsWithState; + expect(props.dehydratedAppState).toEqual(expect.objectContaining({ mode: AppMode.HELIOPHYSICS })); + }); + + test('ignores a fromADS value that is not exactly true', async () => { + const context = getMockContext({}, { fromADS: 'false' }, '/'); + const result = await updateUserStateSSR(context, { props: {} }); + if (!('props' in result)) { + throw new Error('Expected props'); + } + const props = result.props as SSRPropsWithState; + expect(props.dehydratedAppState).not.toHaveProperty('mode'); + }); + }); + // composeNextGSSP and injectSessionGSSP both call this, so one test here // covers auth tagging for every SSR page. describe('sentry auth segmentation', () => { diff --git a/src/components/ClassicForm/ClassicForm.test.tsx b/src/components/ClassicForm/ClassicForm.test.tsx index 8e0aa7dc5..d7bb34bf1 100644 --- a/src/components/ClassicForm/ClassicForm.test.tsx +++ b/src/components/ClassicForm/ClassicForm.test.tsx @@ -31,4 +31,16 @@ describe('ClassicForm', () => { expect(arg.search).toContain('d=astrophysics'); expect(arg.search).not.toContain('general'); }); + + test('does not force ADS Compatibility mode on submit', async () => { + const { getByText, user } = render(, { + initialStore: { mode: AppMode.ASTROPHYSICS }, + }); + + await user.click(getByText('Search')); + + await waitFor(() => expect(router.push).toHaveBeenCalledTimes(1)); + const arg = router.push.mock.calls[0][0] as { pathname: string; search: string }; + expect(arg.search).not.toContain('ads_compat'); + }); }); diff --git a/src/components/ClassicForm/ClassicForm.tsx b/src/components/ClassicForm/ClassicForm.tsx index f70de81f9..7dae977d1 100644 --- a/src/components/ClassicForm/ClassicForm.tsx +++ b/src/components/ClassicForm/ClassicForm.tsx @@ -43,7 +43,6 @@ import { Sort } from '@/components/Sort'; import { Expandable } from '@/components/Expandable'; import { SimpleCopyButton } from '@/components/CopyButton'; import { normalizeSolrSort } from '@/utils/common/search'; -import { ADS_COMPAT_URL_PARAM } from '@/utils/common/searchMode'; import { SolrSort, SolrSortField } from '@/api/models'; const propTypes = { @@ -94,7 +93,6 @@ export const ClassicForm = (props: IClassicFormProps) => { } const search = getSearchQuery(params, { mode: AppMode.ASTROPHYSICS }); const urlParams = new URLSearchParams(search.startsWith('?') ? search.slice(1) : search); - urlParams.set(ADS_COMPAT_URL_PARAM, '1'); void router.push({ pathname: '/search', search: '?' + urlParams.toString() }); } catch (e) { setQueryError((e as Error)?.message); diff --git a/src/middleware.ts b/src/middleware.ts index f32e84c2c..e7d83b5fb 100644 --- a/src/middleware.ts +++ b/src/middleware.ts @@ -466,23 +466,33 @@ export async function middleware(req: NextRequest) { return response; } - // Legacy ADS app referrer handling - redirect to /?fromADS=true and set scix_prefs cookie - // so updateUserStateSSR seeds mode/searchMode without URL pollution. - // Guard includes fromADS to prevent a redirect loop: some browsers preserve the Referer - // header across same-origin redirects, which would re-trigger this block on the follow-up GET. + // scix_prefs cookie (not a URL param) lets updateUserStateSSR seed mode + // without URL pollution. + // The fromADS param guards a redirect loop: some browsers replay Referer + // across same-origin redirects, re-triggering this block on the follow-up GET. if (path === '/' && !req.nextUrl.searchParams.has('forceMode') && !req.nextUrl.searchParams.has('fromADS')) { if (isFromLegacyApp(referer)) { const url = new URL('/', req.url); url.searchParams.set('fromADS', 'true'); log.info({ referer, duration: Date.now() - startTime }, 'Legacy ADS referrer redirect'); const response = NextResponse.redirect(url); - setPrefsCookie(response, req, { mode: 'ASTROPHYSICS', searchMode: 'ADS_COMPAT' }); + setPrefsCookie(response, req, { mode: 'ASTROPHYSICS' }); return response; } } const res = NextResponse.next(); + // Handles /?fromADS=true reached directly (shared link, no legacy + // referer) — the redirect block above skips once fromADS is present. + if ( + path === '/' && + req.nextUrl.searchParams.get('fromADS') === 'true' && + !mapDisciplineParamToAppMode(req.nextUrl.searchParams.get('forceMode') ?? undefined) + ) { + setPrefsCookie(res, req, { mode: 'ASTROPHYSICS' }); + } + // Emit analytics void emitAnalytics(req); diff --git a/src/middlewares/__tests__/middleware.routes.integration.test.ts b/src/middlewares/__tests__/middleware.routes.integration.test.ts index 1db217f41..869687b39 100644 --- a/src/middlewares/__tests__/middleware.routes.integration.test.ts +++ b/src/middlewares/__tests__/middleware.routes.integration.test.ts @@ -509,7 +509,7 @@ describe('middleware route integration', () => { expect(prefs!.searchMode).toBeUndefined(); }); - test('legacy ADS referrer redirects to /?fromADS=true and seeds ADS_COMPAT cookie', async () => { + test('legacy ADS referrer redirects to /?fromADS=true and seeds ASTROPHYSICS only', async () => { const req = makeReq('https://example.com/', { headers: { referer: 'https://ui.adsabs.harvard.edu/search' }, }); @@ -519,6 +519,22 @@ describe('middleware route integration', () => { const prefs = getPrefsCookie(res); expect(prefs).not.toBeNull(); expect(prefs!.mode).toBe('ASTROPHYSICS'); + expect(prefs!.searchMode).toBeUndefined(); + }); + + test('legacy ADS referrer preserves a user-chosen ADS_COMPAT prefs cookie', async () => { + const existingCookie = JSON.stringify({ mode: 'GENERAL', searchMode: 'ADS_COMPAT' }); + const req = makeReq('https://example.com/', { + headers: { + referer: 'https://ui.adsabs.harvard.edu/search', + cookie: `scix_prefs=${existingCookie}`, + }, + }); + const res = (await middleware(req)) as NextResponse; + expect(res.status).toBe(307); + const prefs = getPrefsCookie(res); + expect(prefs).not.toBeNull(); + expect(prefs!.mode).toBe('ASTROPHYSICS'); expect(prefs!.searchMode).toBe('ADS_COMPAT'); }); @@ -530,6 +546,48 @@ describe('middleware route integration', () => { expect(res.headers.get('location')).toBeNull(); }); + test('fromADS param seeds ASTROPHYSICS without a legacy referrer (shared link)', async () => { + const req = makeReq('https://example.com/?fromADS=true'); + const res = (await middleware(req)) as NextResponse; + expect(res.headers.get('location')).toBeNull(); + const prefs = getPrefsCookie(res); + expect(prefs).not.toBeNull(); + expect(prefs!.mode).toBe('ASTROPHYSICS'); + expect(prefs!.searchMode).toBeUndefined(); + }); + + test('fromADS param overrides a different persisted discipline', async () => { + const existingCookie = JSON.stringify({ mode: 'HELIOPHYSICS' }); + const req = makeReq('https://example.com/?fromADS=true', { + headers: { cookie: `scix_prefs=${existingCookie}` }, + }); + const res = (await middleware(req)) as NextResponse; + const prefs = getPrefsCookie(res); + expect(prefs).not.toBeNull(); + expect(prefs!.mode).toBe('ASTROPHYSICS'); + }); + + test('a valid forceMode suppresses the fromADS cookie stamp', async () => { + const req = makeReq('https://example.com/?fromADS=true&forceMode=heliophysics'); + const res = (await middleware(req)) as NextResponse; + expect(res.headers.get('location')).toBeNull(); + expect(getPrefsCookie(res)).toBeNull(); + }); + + test('an unmappable forceMode leaves the fromADS cookie stamp in place', async () => { + const req = makeReq('https://example.com/?fromADS=true&forceMode=banana'); + const res = (await middleware(req)) as NextResponse; + const prefs = getPrefsCookie(res); + expect(prefs).not.toBeNull(); + expect(prefs!.mode).toBe('ASTROPHYSICS'); + }); + + test('fromADS param does not stamp a cookie when the value is not true', async () => { + const req = makeReq('https://example.com/?fromADS=false'); + const res = (await middleware(req)) as NextResponse; + expect(getPrefsCookie(res)).toBeNull(); + }); + test('does not redirect when forceMode param already present on root', async () => { const req = makeReq('https://example.com/?forceMode=astrophysics', { headers: { referer: 'https://ui.adsabs.harvard.edu/search' }, diff --git a/src/pages/index.tsx b/src/pages/index.tsx index a1cee0f9f..5816e20c6 100644 --- a/src/pages/index.tsx +++ b/src/pages/index.tsx @@ -14,7 +14,6 @@ import { StatLabel, StatNumber, Text, - useToast, VisuallyHidden, } from '@chakra-ui/react'; @@ -73,23 +72,6 @@ const HomePage: NextPage = () => { const isClient = useIsClient(); const { persistCurrentForm } = useLandingFormPreference(); const [searchMode, setSearchMode] = useSearchMode(); - const toast = useToast(); - - // Show toast when middleware has auto-set ADS_COMPAT (cookie/GSSP path) - useEffect(() => { - if (router.query.fromADS !== 'true') { - return; - } - toast({ - status: 'info', - duration: 10000, - isClosable: true, - position: 'top', - title: 'ADS Compatibility mode enabled', - description: - "Looks like you came from ADS — we've switched to ADS Compatibility mode automatically. You can change this using the Search mode menu.", - }); - }, [router.query.fromADS, toast]); // start tour if first time useTour(); diff --git a/src/ssr-utils.ts b/src/ssr-utils.ts index 0fa9cf7be..7dcea7ffe 100644 --- a/src/ssr-utils.ts +++ b/src/ssr-utils.ts @@ -37,11 +37,14 @@ export const updateUserStateSSR: IncomingGSSP = async (ctx, prevResult) => { // URL discipline param (d) only applies on /search const urlMode = pathname === '/search' ? mapDisciplineParamToAppMode(ctx.query?.d) : null; + const fromADSParam = ctx.query?.fromADS; + const fromADS = pathname === '/' && (Array.isArray(fromADSParam) ? fromADSParam[0] : fromADSParam) === 'true'; + // Prefs cookie — persisted user preference (written by middleware on first ADS visit) const prefs = readPrefsCookie(ctx.req.headers.cookie); const cookieMode = prefs.mode && VALID_APP_MODES.has(prefs.mode as AppMode) ? (prefs.mode as AppMode) : undefined; - // Priority: URL forceMode > URL d param > prefs cookie - const resolvedMode = forceMode ?? urlMode ?? cookieMode; + // Priority: URL forceMode > URL d param > fromADS (home page only) > prefs cookie + const resolvedMode = forceMode ?? urlMode ?? (fromADS ? AppMode.ASTROPHYSICS : undefined) ?? cookieMode; // ADS_COMPAT is only meaningful in the ASTROPHYSICS discipline context. // If the resolved mode is anything else, suppress the cookie search mode so