Skip to content

Conversation

@nygrenh
Copy link
Member

@nygrenh nygrenh commented Jan 8, 2026

Summary by CodeRabbit

  • Chores
    • Reduced server-side work for unsigned visitors to speed initial page loads and avoid redundant authentication checks; consolidated auth info into page props for more efficient page initialization.
  • Tests
    • Extended server response cache duration from 1 hour to 5 hours to improve cache effectiveness and reduce repeated backend queries.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Jan 8, 2026

📝 Walkthrough

Walkthrough

Adds an early authentication check in Next.js SSR to short-circuit data fetching for unsigned users and consolidate auth values (signedIn/admin/accessToken), and updates a Redis GraphQL cache test TTL from 1 hour to 5 hours.

Changes

Cohort / File(s) Summary
Authentication / SSR
frontend/lib/with-apollo-client/index.tsx, frontend/pages/_app.tsx
Move auth checks to start of getInitialProps, short-circuit SSR for unsigned users by returning minimal pageProps (currentUser: null) with apolloState and accessToken; avoid duplicate getAccessToken/auth recomputation; propagate signedIn, admin, accessToken in returned props.
Cache Test TTL
backend/middlewares/__test__/expressGraphqlCache.test.ts
Change Redis EX TTL in GraphQL cache MISS path from 3600 (1h) to 18000 (5h) in test expectations.

Sequence Diagram(s)

sequenceDiagram
  participant Browser
  participant NextJS as Next.js (getInitialProps)
  participant Auth as Auth helpers (isSignedIn/getAccessToken)
  participant Apollo as Apollo init

  Browser->>NextJS: request page (SSR)
  NextJS->>Auth: isSignedIn(ctx)
  alt user not signed in
    Auth-->>NextJS: false
    NextJS->>Apollo: initialize with apolloState
    NextJS-->>Browser: return minimal pageProps (currentUser:null, apolloState, accessToken)
  else user signed in
    Auth-->>NextJS: true + accessToken
    NextJS->>Apollo: populate SSR queries with accessToken
    NextJS->>Component: call Component.getInitialProps(ctx)
    NextJS-->>Browser: return populated pageProps + apolloState + accessToken
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰 twitches whiskers
Early checks, a nimble hop,
No extra fetches, no extra stop.
Access token carried through the night,
Cache runs longer, everything's light. 🥕

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main objective: skipping getInitialProps work for unauthenticated users by adding early exit logic in the authentication flow.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3cf002c and de336f3.

📒 Files selected for processing (1)
  • backend/middlewares/__test__/expressGraphqlCache.test.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Analyze (javascript)
  • GitHub Check: build-deploy
🔇 Additional comments (1)
backend/middlewares/__test__/expressGraphqlCache.test.ts (1)

130-130: Implementation correctly updated to match test expectation. The middleware at backend/middlewares/expressGraphqlCache.ts line 9 defines CACHE_EXPIRE_TIME_SECONDS = 60 * 60 * 5 (5 hours) and uses it consistently throughout, matching the test expectation on line 130.

The TTL change from 1 hour to 5 hours is unrelated to the PR's stated objective about authentication checks. While the middleware does include authentication-aware logic (skipping cache for authorized requests), consider documenting why the cache TTL was increased in this PR or moving it to a separate change for better traceability.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link

codecov bot commented Jan 8, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 66.31%. Comparing base (9076b01) to head (de336f3).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1303      +/-   ##
==========================================
+ Coverage   62.59%   66.31%   +3.71%     
==========================================
  Files         153      154       +1     
  Lines        6422     6460      +38     
  Branches     1579     1591      +12     
==========================================
+ Hits         4020     4284     +264     
+ Misses       2244     2037     -207     
+ Partials      158      139      -19     
Flag Coverage Δ
backend 66.31% <ø> (+3.71%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@nygrenh nygrenh merged commit c15df0c into master Jan 9, 2026
7 checks passed
@nygrenh nygrenh deleted the no-ssr-for-logged-out branch January 9, 2026 07:43
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.

2 participants