Skip to content

refactor(app): reorganise routes into (public) and (internal) groups#85

Open
natashaannn wants to merge 7 commits into
mainfrom
refactor/p8-route-groups
Open

refactor(app): reorganise routes into (public) and (internal) groups#85
natashaannn wants to merge 7 commits into
mainfrom
refactor/p8-route-groups

Conversation

@natashaannn

@natashaannn natashaannn commented Jun 16, 2026

Copy link
Copy Markdown
Member

Summary

  • Reorganised `app/` routes into `(public)/` and `(internal)/` Next.js route groups — zero URL
    impact, since group names are stripped from paths by the App Router.
  • Renamed `/transcribe` → `/get-youtube-captions` to accurately describe the feature (fetches
    YouTube auto-generated captions via `youtube-transcript`, not the internal Whisper pipeline).
  • Deleted `app/login/` and removed all Sign In/Sign Out UI from `Header.tsx` and the landing
    page, as the fake demo-credential auth system is being replaced by the subscriber email gate
    (Issue feat(auth): add modular Resend subscriber email gate for carousel and get-youtube-captions #77).
  • Updated `CLAUDE.md` Key Source Files table to reflect the new route group paths.
  • Pinned `js-yaml` to `^4.2.0` via `overrides` to clear 18 moderate audit findings in Jest's
    transitive dep chain (`@istanbuljs/load-nyc-config`) — pre-existing on `main`, unrelated to
    this PR's feature work.
  • Added e2e smoke tests for `/carousel`, `/get-youtube-captions`, and `/about`; removed stale
    login-redirect references from `e2e/smoke.test.ts`.
  • Added `Header.test.tsx` — smoke render test asserting `/get-youtube-captions` is present,
    `/transcribe` is absent, and `/login` is absent from nav links.

How to review

  • `app/(public)/` and `app/(internal)/` — verify the correct routes landed in each group:
    public routes (`/`, `/about`, `/carousel`, `/get-youtube-captions`) in `(public)/`;
    internal routes (`/editor`, `/camera`) in `(internal)/`. No logic changes, import paths only.
  • `app/components/Header.tsx` — diff should show: removal of `useAuth`/`useRouter` imports,
    removal of Sign In/Sign Out `` blocks (both desktop and drawer), and the
    `/transcribe` → `/get-youtube-captions` nav link update. No other changes.
  • `app/(public)/page.tsx` — landing page diff should show `/transcribe` href updated to
    `/get-youtube-captions` and any `/login` anchor removed. `/auto-carousel` link is
    intentionally left for Issue chore(auto-carousel): remove auto-carousel API route, component, and all references #81.
  • `app/components/Header.test.tsx` — 4 tests: smoke render, renamed route present,
    `/transcribe` absent, `/login` absent. `/auto-carousel` is intentionally not asserted here
    (Issue chore(auto-carousel): remove auto-carousel API route, component, and all references #81's responsibility).
  • `e2e/smoke.test.ts` — 3 new route tests added; stale "redirects to login" description and
    comment removed.
  • `CLAUDE.md` — Key Source Files paths updated from `app/editor/` and `app/camera/` to
    `app/(internal)/editor/` and `app/(internal)/camera/`.
  • `package.json` — only the `"js-yaml": "^4.2.0"` override was added; no new deps.

Test plan

  • `npm test` passes — 346 tests, 20 suites (node + react projects)
  • `npm run build` passes — all 13 routes registered; `/login` absent; `/get-youtube-captions` present
  • `npm run test:e2e` passes — 6/6 smoke tests pass
  • `npm audit` — cleared to moderate+ threshold after js-yaml override

Manual verification (completed):

  • [UI-BROWSER] `/` renders; Header has no Sign In/Sign Out buttons; no `/login` link on page
  • [UI-BROWSER] `/carousel`, `/get-youtube-captions`, `/about` all render correctly
  • [UI-BROWSER] `/editor` and `/camera` render identically to before
  • [UI-BROWSER] `/login` returns Next.js 404
  • [UI-BROWSER] Header "Transcription" nav link navigates to `/get-youtube-captions`

Closes #76

natashaannn and others added 6 commits June 16, 2026 12:16
Introduces Next.js App Router route groups to formalise the boundary
between externally-exposed and internal-only pages. Route group names
are stripped from URLs so no redirects or existing links break.

Public routes (carousel, landing, about) move into app/(public)/.
Internal routes (editor, camera) move into app/(internal)/.
The transcribe/ directory is renamed to get-youtube-captions/ to
accurately describe what it does (YouTube auto-captions via the
youtube-transcript package, not Whisper-based transcription).

Deletes app/login/ as the fake demo-credential auth system is being
replaced by the subscriber email gate (Issue #77).

Fixes relative import paths in all moved files (one extra level now
that each sits under a route group folder). Suppresses pre-existing
ESLint violations in Timeline.tsx (react-hooks/refs stable-ref
pattern), TimelineScrollbar.tsx, useTimelineNav.ts, and camera/page.tsx
that were not previously linted because those files were not staged.

AC: file moves complete, login deleted, route group structure in place
Renames /transcribe → /get-youtube-captions in Header.tsx, the
landing page, and the about page to match the directory rename.

Removes the Sign In / Sign Out buttons from Header.tsx (both desktop
and mobile drawer) and their associated useRouter/useAuth imports.
The /login route has been deleted; auth will be replaced by the
subscriber email gate in Issue #77.

AC: no nav link points to /login; all /transcribe hrefs updated
Updates app/camera, app/editor paths to reflect their new locations
under app/(internal)/ after the route group reorganisation.
…audit finding

The nested js-yaml inside @istanbuljs/load-nyc-config was pinned at <=4.1.1
(GHSA-h67p-54hq-rp68). This override forces it to 4.2.0, clearing the 18
moderate vulnerabilities that blocked the pre-push hook.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
Adds /carousel, /get-youtube-captions, and /about smoke tests to match
the issue's additional test scenarios. Removes the stale login-redirect
comment from the root test (the /login route has been deleted).
Asserts /get-youtube-captions is present, /transcribe is absent, and
/login is absent — covering the rename and deletion done in this issue.
Auto-carousel nav removal is out of scope here (tracked in issue #81).

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>


- Make scripts/config/paths.test.ts cross-platform by normalizing
  expected paths with path.normalize() (fixes 26 failures on Windows)
- Update docs/review-findings/2026-06-16-refactor-p8-route-groups.md
  to reflect resolved W1/W2/B3 findings and passing test plan results

Branch was rebased onto origin/main during review session.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refactor(app): reorganise routes into (public) and (internal) Next.js route groups

2 participants