fix(search): never auto-enable ADS compatibility mode - #944
Open
thostetler wants to merge 3 commits into
Open
thostetler wants to merge 3 commits into
thostetler wants to merge 3 commits into
Conversation
The legacy-ADS referrer path seeded searchMode=ADS_COMPAT into the prefs cookie and the classic form forced ads_compat=1 on every submit, so the mode turned on without the user asking. The landing page then announced it with a toast. Astrophysics discipline seeding and the fromADS redirect-loop guard stay. The mode itself is unchanged and still reachable from the Search mode menu or an explicit ads_compat=1 URL.
Bundle sizeShared by all pages: 629.6 kB (—) ⚪ No route changed by more than 1 kB. ✅ First load = polyfills + shared |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #944 +/- ##
========================================
+ Coverage 55.0% 55.1% +0.1%
========================================
Files 374 374
Lines 11462 11467 +5
Branches 2514 2518 +4
========================================
+ Hits 6303 6310 +7
+ Misses 4527 4525 -2
Partials 632 632
🚀 New features to boost your workflow:
|
Arriving from legacy ADS redirects to /?fromADS=true and stamps the discipline into the prefs cookie, but visiting that link directly did nothing: the redirect block skips once fromADS is present, so a forwarded link never seeded anything. The discipline is resolved server-side so the first render already has it, which is what selects the ADS variant of the site tour. Scoped to the home page, matching where the cookie is written, so the param cannot override a persisted discipline on other pages. Search mode is untouched.
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Middleware can persist Astrophysics even when an explicit valid forceMode selects another discipline.
Review effort: Balanced
Findings: 1
What changed in this PR
Removes automatic ADS Compatibility activation while retaining explicit user preferences and Astrophysics handoff.
Risk: Medium—fromADS can persist Astrophysics despite an explicit forceMode.
Changes:
- Stops legacy referrals and Classic Form submissions from enabling compatibility mode.
- Removes the automatic compatibility toast.
- Adds middleware and SSR regression coverage.
| File | Description |
|---|---|
src/ssr-utils.ts |
Resolves fromADS as Astrophysics only. |
src/pages/index.tsx |
Removes the compatibility-mode toast. |
src/middleware.ts |
Stops seeding ADS_COMPAT; retains discipline seeding. |
src/middlewares/__tests__/middleware.routes.integration.test.ts |
Tests referral cookie behavior. |
src/components/ClassicForm/ClassicForm.tsx |
Removes forced compatibility URL parameter. |
src/components/ClassicForm/ClassicForm.test.tsx |
Verifies compatibility is not forced. |
src/__tests__/ssr-utils.test.ts |
Tests fromADS precedence and scope. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+488
to
+490
| if (path === '/' && req.nextUrl.searchParams.get('fromADS') === 'true') { | ||
| setPrefsCookie(res, req, { mode: 'ASTROPHYSICS' }); | ||
| } |
fromADS's direct-hit cookie stamp ignored forceMode, so a page rendered under a forced discipline would silently revert to ASTROPHYSICS on the next navigation. Gate the stamp on forceMode not mapping to a discipline, matching updateUserStateSSR's own precedence.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ADS Compatibility search mode was turning on without the user asking: legacy-ADS referrers seeded it into the prefs cookie, the classic form forced it on every submit, and the landing page announced it with a toast. All three are removed; the mode is still reachable from the Search mode menu or an explicit ads_compat=1 URL.