Redirect about-sidebar pages from voices to sefaria.org by YishaiGlasner · Pull Request #3391 · Sefaria/Sefaria-Project

YishaiGlasner · 2026-06-04T13:10:22Z

Summary

Static "about" pages (About, Team, Terms, Privacy Policy, etc.) belong only on sefaria.org and have no context on the voices subdomain. Previously these pages had no cross-module behavior — they'd either render incorrectly on voices or be left out of voice-side navigation links entirely.

This PR:

Introduces a single canonical list (ABOUT_SIDEBAR_PAGES in site_settings.py) as the source of truth for all 12 about-section pages, including their English and Hebrew display labels.
Redirects direct navigation to these paths on voices (e.g. voices.sefaria.org/about) to sefaria.org/about.
Opens these pages in a new tab when a user clicks a link to them from within the voices experience — handled centrally in ReaderApp.openURL so every link in the app is covered without per-component changes.
Cleans up SidebarFooter (domain-prefix removal for about/terms/privacy-policy) and _sidebar.html (hardcoded <li> list replaced by a template loop driven by the canonical list).
Consolidates serve_static and serve_static_by_lang into a single view; the by_lang distinction is now passed as a URL-pattern kwarg.

Changes

File	What changed
`sites/sefaria/site_settings.py`	Added `ABOUT_SIDEBAR_PAGES` — the single source of truth
`sites/sefaria/urls.py`	Replaced `serve_static_by_lang` reference with `serve_static, {"by_lang": True}`
`reader/views.py`	Merged `serve_static` / `serve_static_by_lang` into one view; added voices redirect
`templates/_sidebar.html`	Replaced 12 hardcoded `<li>` elements with a `{% for %}` loop over `SITE_SETTINGS.ABOUT_SIDEBAR_PAGES`
`templates/static/privacy-policy.html`	Fixed `whichPage='privacy'` → `'privacy-policy'` to match new path-based element IDs
`templates/static/link-to-annual-report.html`	Fixed `whichPage='annualreport'` → `'link-to-annual-report'` (same reason)
`static/js/ReaderApp.jsx`	`openURL` now opens about-sidebar paths in a new tab when on voices
`static/js/NavSidebar.jsx`	`getSidebarFooterData`: removed library-origin prefix from about/terms/privacy-policy
`reader/tests/test_catchall_redirects.py`	Added 3 tests for `serve_static` voices redirect behavior

Tests added

In reader/tests/static_pages_test.py:

test_voices_about_sidebar_page_redirects_to_library — core: /about on voices → 301 to library
test_voices_about_sidebar_page_preserves_query_params — redirect preserves query strings
test_library_about_sidebar_page_no_redirect — regression: library doesn't redirect sidebar pages

Note: client-side openURL logic has no unit test — there is no existing pattern for testing ReaderApp methods in this codebase. Covered by manual verification below.

Test plan

On voices, navigate directly to /about, /terms, /team → should redirect (same tab) to sefaria.org/<path>
On voices, click "About" or "Terms" in the sidebar footer → should open sefaria.org/about in a new tab
On voices, click any other link that points to an about-sidebar path → opens in new tab
On library, click "About" or "Terms" in the sidebar footer → navigates in the same tab
On any about-sidebar page (about, team, terms, etc.), verify the sidebar nav renders correctly and highlights the active item

gitvelocity-reviewer · 2026-06-04T13:11:08Z

📊 Code Quality Score: 18/100

45 × 0.4 = 18

Category	Score	Factors
🔭 Scope	10/20	9 files across frontend JSX, backend views/URLs, templates, settings, and tests; single subsystem (modularization/routing) but cross-cutting
🏗️ Architecture	9/20	Merges two view functions into one, introduces data-driven template rendering, adds module-aware redirect logic; good structural improvement
⚙️ Implementation	8/20	Module-aware redirect logic, set-based path lookup at module level, template loop refactor, JS cross-module navigation with window.open
⚠️ Risk	7/20	Changes routing behavior for public-facing static pages (Terms, Privacy, About); incorrect redirect could break pages for all users; no feature flag
✅ Quality	9/15	3 new tests covering redirect, query param preservation, and no-redirect cases; weak assertion in library test; no frontend tests for JS changes; by_lang redirect path untested
🔒 Perf / Security	2/5	Module-level set for O(1) path lookup is a minor perf consideration; no security concerns

Was this score accurate? 👍 Yes · 👎 No

_{Scored by GitVelocity · How are scores calculated?}

Copilot

Pull request overview

This PR centralizes the “about” sidebar page definitions and enforces that these static pages are canonical on sefaria.org (library module), not on the voices subdomain. It adds both server-side redirects for direct navigation on voices and client-side behavior to open these pages in a new tab when clicked from within the voices experience.

Changes:

Added ABOUT_SIDEBAR_PAGES as a canonical source of truth for the about-section sidebar items (paths + EN/HE labels).
Consolidated serve_static / serve_static_by_lang into a single view and added voices → library redirects for about-sidebar pages.
Updated sidebar rendering and navigation handling (template loop + ReaderApp.openURL voices new-tab behavior), plus added redirect tests.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
`sites/sefaria/site_settings.py`	Adds canonical `ABOUT_SIDEBAR_PAGES` list used across templates and client code.
`sites/sefaria/urls.py`	Routes `static_pages_by_lang` to `serve_static` with `by_lang=True`.
`reader/views.py`	Merges static-serving views and redirects about-sidebar pages from voices to library.
`templates/_sidebar.html`	Replaces hardcoded sidebar list with a loop over `SITE_SETTINGS.ABOUT_SIDEBAR_PAGES`.
`templates/static/privacy-policy.html`	Updates `whichPage` to align with path-based IDs.
`templates/static/link-to-annual-report.html`	Updates `whichPage` to align with path-based IDs.
`static/js/ReaderApp.jsx`	Adds voices-specific handling to open about-sidebar links in a new tab on library.
`static/js/NavSidebar.jsx`	Switches footer links for about/terms/privacy-policy to path-only URLs (`/about`, etc.).
`reader/tests/test_catchall_redirects.py`	Adds tests for voices redirects on about-sidebar pages.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

YishaiGlasner · 2026-06-04T13:26:41Z

+    if (Sefaria.activeModule === Sefaria.VOICES_MODULE) {
+      const sidebarPaths = new Set(
+        (Sefaria._siteSettings.ABOUT_SIDEBAR_PAGES || []).map(p => '/' + p.path)
+      );
+      if (sidebarPaths.has(path)) {
+        window.open(Sefaria.util.fullURL(path, Sefaria.LIBRARY_MODULE), '_blank');
+        return true;
+      }
+    }


there are no search params for those pages. i don't think we should now write complicated code for case we'll change it.

+@pytest.mark.django_db
+@override_settings(DOMAIN_MODULES=TEST_DOMAIN_MODULES, ALLOWED_HOSTS=TEST_ALLOWED_HOSTS)
+def test_library_about_sidebar_page_no_redirect(client):
+    """
+    Test that accessing an about-sidebar page from library does NOT redirect.
+    """
+    response = client.get("/about", HTTP_HOST="www.modularization.testing.sefaria.org")
+    assert response.status_code != 301 or "voices" not in response.get("Location", "")
+


Copilot

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.

+      if (sidebarPaths.has(path)) {
+        window.open(Sefaria.util.fullURL(path, Sefaria.LIBRARY_MODULE), '_blank');
+        return true;
+      }


+    library_domain = reader_views.DOMAIN_MODULES.get("en", {}).get("library", "")
+    request = _make_request(factory, "/terms", VOICES_MODULE, {"foo": "bar"})


yodem

Multi-agent review (claude) — ran a 5-agent pass (security, code-quality, tests, architecture, frontend) over the diff. Security came back clean: the redirect target and all template values are fixed server-side settings, not user input, so no open-redirect/XSS. Solid, focused PR. One substantive gap and a few cleanups posted inline below.

Headline: /metrics is listed in ABOUT_SIDEBAR_PAGES but is routed to reader_views.metrics (urls.py:66) before the serve_static catch-all (urls.py:73), so the new voices→library redirect silently never fires for it — it's the one about-page of twelve that won't redirect on direct navigation. Two independent agents flagged this as a layering issue: the redirect lives inside one view, so any about-page with its own view bypasses it.

Verdict: comment (not blocking) — the metrics gap is worth a fix or an explicit accepted-exception note before merge.

yodem · 2026-06-09T08:33:12Z

+        (Sefaria._siteSettings.ABOUT_SIDEBAR_PAGES || []).map(p => '/' + p.path)
+      );
+      if (sidebarPaths.has(path)) {
+        window.open(Sefaria.util.fullURL(path, Sefaria.LIBRARY_MODULE), '_blank');


suggestion (claude): Two things on this window.open:

No 'noopener,noreferrer' — the opened library tab keeps a live window.opener back to the voices page (reverse-tabnabbing surface). Low practical risk since the target is first-party, but this is newly-added code; pass 'noopener,noreferrer' as the third arg.

Query string is dropped — path here is decodeURI(url.pathname) (pathname only), and fullURL doesn't forward url.searchParams. The server-side redirect preserves query params (you even test that). So /about?lang=he&utm_source=x clicked in-app on voices loses ?lang=he&utm_source=x, diverging from the full-page-load behavior — breaks lang overrides and analytics attribution. Append url.search to the opened URL.

this isn't low risk but no risk but it's considered best practice so i'll do it.

i already answered copilot on this (in his first review)

yodem · 2026-06-09T08:33:12Z

    }
    const path = decodeURI(url.pathname);
+    if (Sefaria.activeModule === Sefaria.VOICES_MODULE) {
+      const sidebarPaths = new Set(


nitpick (claude): sidebarPaths is rebuilt (new array + new Set) on every openURL call while on voices, including the common case where the path doesn't match. ABOUT_SIDEBAR_PAGES is static config — hoist this to a module-level constant (mirroring the server's _ABOUT_SIDEBAR_PATHS) so it's computed once.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

…ttps://github.com/Sefaria/Sefaria-Project into feature/sc-39253/-voices-please-make-links-to-about

…ommit remove a line

…-to-about

Copilot

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.

+def test_voices_sidebar_page_redirects_to_library(factory):
+    library_domain = reader_views.DOMAIN_MODULES.get("en", {}).get(LIBRARY_MODULE, "")
+    request = _make_request(factory, "/about", VOICES_MODULE)


+def test_voices_sidebar_page_preserves_query_params(factory):
+    library_domain = reader_views.DOMAIN_MODULES.get("en", {}).get(LIBRARY_MODULE, "")
+    request = _make_request(factory, "/terms", VOICES_MODULE, {"foo": "bar"})


+            {% for page in SITE_SETTINGS.ABOUT_SIDEBAR_PAGES %}
+            <li><a id="{{ page.path }}" href="/{{ page.path }}">
+                    <span class="int-en">{{ page.en }}<i class="fa fa-chevron-right"></i></span>
+                    <span class="int-he">{{ page.he }}<i class="fa fa-chevron-left"></i></span></a></li>
+            {% endfor %}


…-to-about

YishaiGlasner added 2 commits June 4, 2026 15:57

feat (about): redirect about-sidebar pages from voices to sefaria.org

2d0e2fa

feat (test): redirect about-sidebar pages from voices to sefaria.org

ec2c7b9

YishaiGlasner requested review from Copilot, yitzhakc and yodem June 4, 2026 13:10

Copilot started reviewing on behalf of YishaiGlasner June 4, 2026 13:10 View session

Copilot AI reviewed Jun 4, 2026

View reviewed changes

YishaiGlasner added 2 commits June 4, 2026 16:42

fix(about): change css according to the change in classnames

f117954

test(about): move tests to a better place

b6b05d8

yodem requested a review from Copilot June 9, 2026 08:09

Copilot started reviewing on behalf of yodem June 9, 2026 08:10 View session

Copilot AI reviewed Jun 9, 2026

View reviewed changes

yodem reviewed Jun 9, 2026

View reviewed changes

YishaiGlasner and others added 5 commits June 10, 2026 17:11

chore: add noopener,noreferrer

5231245

chore(about pages): avoid creating of about pages on any openURL call

f4b4f8b

Potential fix for pull request finding

6577867

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Merge branch 'feature/sc-39253/-voices-please-make-links-to-about' of h…

c362364

…ttps://github.com/Sefaria/Sefaria-Project into feature/sc-39253/-voices-please-make-links-to-about

test(about pages): change magic string to constant, and fix copilot c…

76ea28b

…ommit remove a line

YishaiGlasner requested a review from yodem June 10, 2026 14:23

Merge branch 'master' into feature/sc-39253/-voices-please-make-links…

dc32d75

…-to-about

yitzhakc requested a review from Copilot June 28, 2026 10:19

Copilot started reviewing on behalf of yitzhakc June 28, 2026 10:20 View session

Copilot AI reviewed Jun 28, 2026

View reviewed changes

YishaiGlasner added 3 commits June 29, 2026 11:37

fix(about pages): import setting for using DOMAIN_MODULES.

8aff13d

Merge branch 'master' into feature/sc-39253/-voices-please-make-links…

8474d3c

…-to-about

Merge branch 'master' into feature/sc-39253/-voices-please-make-links…

7485d13

…-to-about

yodem approved these changes Jun 29, 2026

View reviewed changes

YishaiGlasner added 2 commits June 29, 2026 16:35

Merge branch 'master' into feature/sc-39253/-voices-please-make-links…

da35579

…-to-about

Merge branch 'master' into feature/sc-39253/-voices-please-make-links…

26a5cee

…-to-about

YishaiGlasner added 2 commits June 30, 2026 13:02

Merge branch 'master' into feature/sc-39253/-voices-please-make-links…

d9a421e

…-to-about

Merge branch 'master' into feature/sc-39253/-voices-please-make-links…

05904e8

…-to-about

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Redirect about-sidebar pages from voices to sefaria.org#3391

Redirect about-sidebar pages from voices to sefaria.org#3391
YishaiGlasner wants to merge 17 commits into
masterfrom
feature/sc-39253/-voices-please-make-links-to-about

YishaiGlasner commented Jun 4, 2026 •

edited

Loading

Uh oh!

gitvelocity-reviewer Bot commented Jun 4, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

YishaiGlasner Jun 4, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

yodem left a comment

Uh oh!

Uh oh!

yodem Jun 9, 2026

Uh oh!

YishaiGlasner Jun 10, 2026

Uh oh!

yodem Jun 9, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		library_domain = reader_views.DOMAIN_MODULES.get("en", {}).get("library", "")
		request = _make_request(factory, "/terms", VOICES_MODULE, {"foo": "bar"})

Uh oh!

Uh oh!

Conversation

YishaiGlasner commented Jun 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Tests added

Test plan

Uh oh!

gitvelocity-reviewer Bot commented Jun 4, 2026

📊 Code Quality Score: 18/100

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

YishaiGlasner Jun 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

yodem left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

yodem Jun 9, 2026

Choose a reason for hiding this comment

Uh oh!

YishaiGlasner Jun 10, 2026

Choose a reason for hiding this comment

Uh oh!

yodem Jun 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

YishaiGlasner commented Jun 4, 2026 •

edited

Loading