feat: implement newsletter page for pro users #179
Conversation
|
@Sandesh-Upadhyay is attempting to deploy a commit to the AJEET PRATAP SINGH's projects Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThis PR implements a complete newsletter feature for authenticated pro users, including new pages, components, and data models. Backend changes add lazy initialization for Razorpay and improve database connection error handling. Middleware is extended to protect newsletter routes. Extensive documentation guides setup, testing, and deployment. A minor Sidebar styling fix and development script are included. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Browser
participant Middleware
participant NextAuth
participant NewsletterPage as Newsletter<br/>Page Component
participant Subscription
participant NewsletterData
User->>Browser: Request /newsletters
Browser->>Middleware: Check protected path
alt Test Mode Enabled
Middleware->>Browser: Pass through
else Test Mode Disabled
Middleware->>NextAuth: Verify auth token
alt Token Missing
NextAuth->>Browser: Redirect to /login
else Token Present
Middleware->>Browser: Pass through
end
end
Note over NewsletterPage: Load Page
NewsletterPage->>NextAuth: Get session
NewsletterPage->>Subscription: Check subscription status
par Wait for Data
NextAuth-->>NewsletterPage: Session loaded
Subscription-->>NewsletterPage: Subscription status loaded
end
alt Unauthenticated
NewsletterPage->>Browser: Redirect to /login
else Not Pro User
NewsletterPage->>Browser: Redirect to /pricing
else Pro User
NewsletterPage->>NewsletterData: Fetch newsletters by date
NewsletterData-->>NewsletterPage: Grouped newsletters
NewsletterPage->>Browser: Render page with newsletters
end
sequenceDiagram
participant Code
participant RazorpayModule
participant RazorpayProxy as rz_instance<br/>(Proxy)
participant Init as getRazorpayInstance()
participant RazorpayAPI as Razorpay API
Code->>RazorpayProxy: Access rz_instance.method()
RazorpayProxy->>Init: Check if initialized
alt First Access (Not Initialized)
Init->>Init: Validate env vars
Init->>RazorpayAPI: new Razorpay(key, secret)
RazorpayAPI-->>Init: Instance created
Init-->>RazorpayProxy: Cached instance
else Subsequent Access
Init-->>RazorpayProxy: Return cached instance
end
RazorpayProxy->>RazorpayAPI: Delegate method call (bound)
RazorpayAPI-->>RazorpayProxy: Response
RazorpayProxy-->>Code: Return result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related issues
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
|
|
1 similar comment
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (4)
SETUP_INSTRUCTIONS.md (1)
17-55: Setup docs look good; consider small polish for clarity.The setup instructions are clear and use safe placeholders for secrets. A couple of non-blocking nits you may want to address:
- Capitalize “Google OAuth” for consistency and readability (e.g., in the note about using dummy values).
- In the “quick test without full setup” / “newsletter page redirects” sections, consider explicitly pointing to
TESTING_MODE.mdso readers discover the dedicated test-mode flow instead of only manually commenting out auth checks.Also applies to: 102-124
apps/web/src/lib/auth/config.ts (1)
6-16: Dynamic provider setup is solid; tighten typing and restrict fallbacks to dev/test.The overall approach (building
providersfrom env and adding asecret) looks good; a couple of refinements would make this more robust:
- Add an explicit type to
providersto keep NextAuth typings intact instead of falling back toany[], e.g.:-import type { NextAuthOptions } from "next-auth"; +import type { NextAuthOptions } from "next-auth"; … -// build providers array conditionally based on available env vars -const providers = []; +// build providers array conditionally based on available env vars +const providers: NextAuthOptions["providers"] = [];
- The dummy Google provider fallback is useful for local/test mode, but in a misconfigured production environment it will silently give you a non-working login page instead of failing fast. Consider gating this on environment, for example:
-export const authConfig: NextAuthOptions = { - providers: providers.length > 0 ? providers : [ - // fallback: create a dummy provider to prevent NextAuth errors - // this won't work for actual auth but allows the server to start - GoogleProvider({ - clientId: "dummy", - clientSecret: "dummy", - }), - ], +export const authConfig: NextAuthOptions = { + providers: + providers.length > 0 + ? providers + : process.env.NODE_ENV !== "production" + ? [ + // fallback: dev-only dummy provider to prevent NextAuth errors + GoogleProvider({ + clientId: "dummy", + clientSecret: "dummy", + }), + ] + : (() => { + throw new Error("No auth providers configured in production."); + })(),
- Similarly, the
secret: process.env.NEXTAUTH_SECRET || "development-secret-change-in-production"default is convenient for local dev, but you probably want to enforceNEXTAUTH_SECRETbeing set in production (even if that’s only via runtime checks elsewhere), and maybe add a short comment to that effect.Also applies to: 18-28, 30-38, 88-88
apps/web/src/data/newsletters.ts (2)
4-10: Consider using a type alias for the union type.While TypeScript allows interfaces for object types,
NewsletterContentrepresents a discriminated union. Using atypealias is more idiomatic for unions.-export interface NewsletterContent { +export type NewsletterContent = { type: "text" | "heading" | "link" | "image" | "bold"; content: string; href?: string; // for links alt?: string; // for images level?: number; // for headings -} +};This is a minor style preference; the current implementation works correctly.
259-292: Consider locale consistency for reliable sorting.The function uses
toLocaleString("default", { month: "long" })(line 266) to get month names, then reconstructs dates from these names for sorting (line 286-287). The "default" locale is system-dependent and could theoretically vary, though in practice it's usually consistent within a deployment.For more robust sorting, consider using numeric year and month:
export function getNewslettersByDate() { const grouped: Record<string, Newsletter[]> = {}; newsletters.forEach((newsletter) => { const date = new Date(newsletter.date); const year = date.getFullYear(); - const month = date.toLocaleString("default", { month: "long" }); - const key = `${year}-${month}`; + const monthNum = date.getMonth(); + const monthName = date.toLocaleString("default", { month: "long" }); + const key = `${year}-${String(monthNum).padStart(2, '0')}-${monthName}`; if (!grouped[key]) { grouped[key] = []; } grouped[key].push(newsletter); }); // ... sorting logic can use numeric prefixThen parse the numeric prefix for sorting. However, the current implementation should work fine in most environments.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (21)
NEXT_STEPS.md(1 hunks)QUICK_START.md(1 hunks)RUN_AND_PUSH_GUIDE.md(1 hunks)SETUP_INSTRUCTIONS.md(1 hunks)TESTING_MODE.md(1 hunks)TROUBLESHOOTING.md(1 hunks)apps/api/src/clients/razorpay.ts(1 hunks)apps/api/src/index.ts(3 hunks)apps/api/src/prisma.ts(1 hunks)apps/web/NEWSLETTER_DOCUMENTATION.md(1 hunks)apps/web/PR_DESCRIPTION.md(1 hunks)apps/web/src/app/(main)/newsletters/[id]/page.tsx(1 hunks)apps/web/src/app/(main)/newsletters/page.tsx(1 hunks)apps/web/src/components/dashboard/Sidebar.tsx(2 hunks)apps/web/src/components/newsletters/NewsletterCard.tsx(1 hunks)apps/web/src/components/newsletters/NewsletterContent.tsx(1 hunks)apps/web/src/components/newsletters/NewsletterHeader.tsx(1 hunks)apps/web/src/data/newsletters.ts(1 hunks)apps/web/src/lib/auth/config.ts(2 hunks)apps/web/src/middleware.ts(2 hunks)start-dev.bat(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
apps/web/src/components/dashboard/Sidebar.tsx (1)
apps/web/src/components/sidebar/SidebarItem.tsx (1)
SidebarItem(13-32)
apps/web/src/app/(main)/newsletters/page.tsx (4)
apps/web/src/hooks/useSubscription.ts (1)
useSubscription(11-77)apps/web/src/data/newsletters.ts (2)
getNewslettersByDate(260-292)newsletters(19-257)apps/web/src/components/newsletters/NewsletterHeader.tsx (1)
NewsletterHeader(5-32)apps/web/src/components/newsletters/NewsletterCard.tsx (1)
NewsletterCard(10-43)
apps/web/src/data/newsletters.ts (1)
apps/web/src/components/newsletters/NewsletterContent.tsx (1)
NewsletterContent(9-110)
apps/web/src/components/newsletters/NewsletterCard.tsx (1)
apps/web/src/data/newsletters.ts (1)
Newsletter(12-17)
apps/web/src/app/(main)/newsletters/[id]/page.tsx (4)
apps/web/src/hooks/useSubscription.ts (1)
useSubscription(11-77)apps/web/src/data/newsletters.ts (2)
getNewsletterById(295-297)NewsletterContent(4-10)apps/web/src/components/newsletters/NewsletterHeader.tsx (1)
NewsletterHeader(5-32)apps/web/src/components/newsletters/NewsletterContent.tsx (1)
NewsletterContent(9-110)
apps/web/src/components/newsletters/NewsletterContent.tsx (1)
apps/web/src/data/newsletters.ts (1)
NewsletterContent(4-10)
🪛 Biome (2.1.2)
apps/web/src/components/newsletters/NewsletterContent.tsx
[error] 61-61: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🪛 Gitleaks (8.29.0)
QUICK_START.md
[high] 11-11: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 27-27: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 LanguageTool
TROUBLESHOOTING.md
[style] ~5-~5: Consider an alternative verb to strengthen your wording.
Context: ...g guide ## prisma client error if you see this error: ``` Error: @prisma/client d...
(IF_YOU_HAVE_THIS_PROBLEM)
QUICK_START.md
[style] ~58-~58: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...ing without full authentication if you want to see the newsletter page ui without sett...
(REP_WANT_TO_VB)
SETUP_INSTRUCTIONS.md
[uncategorized] ~53-~53: The name of this company should be capitalized.
Context: ...tter page, you can use dummy values for google oauth (the page will still load) - gene...
(GOOGLE_PRODUCTS)
🪛 markdownlint-cli2 (0.18.1)
RUN_AND_PUSH_GUIDE.md
14-14: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
117-117: Bare URL used
(MD034, no-bare-urls)
132-132: Bare URL used
(MD034, no-bare-urls)
apps/web/PR_DESCRIPTION.md
36-36: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (25)
apps/web/src/components/dashboard/Sidebar.tsx (1)
75-75: Good catch on the typo fix!The CSS class correction from
bg-ox-sidebatobg-ox-sidebarensures consistent background styling for the mobile header, matching the rest of the sidebar component.apps/api/src/clients/razorpay.ts (1)
3-46: Lazy Razorpay singleton + Proxy accessor look correct; just confirm the new failure mode is acceptable.The refactor to a lazily initialized singleton plus
rz_instanceProxy is clean and keeps the public surface compatible:
- Env validation inside
getRazorpayInstancewith clear error messages is good; and binding functions in the Proxy preservesthisfor methods on the root Razorpay instance.- Access patterns like
rz_instance.orders.create(...)should continue to work sinceordersis returned as the underlying object after the first access.The main behavioral change is that missing
RAZORPAY_KEY_ID/RAZORPAY_KEY_SECRETwill now cause an error only on first use (e.g., first payment call) instead of at process startup. If you preferred fail-fast semantics in production, you might want a small health check or startup hook that callsgetRazorpayInstance()so misconfiguration is caught early, while still enjoying lazy init in tests.apps/api/src/prisma.ts (1)
112-120: LGTM! Environment-specific error handling improves developer experience.The environment-specific error handling is well-designed: production exits on database failure (preventing operation with broken DB), while development logs warnings and continues (enabling UI testing without full setup). This aligns well with the non-blocking startup approach in apps/api/src/index.ts.
apps/web/src/middleware.ts (1)
18-18: LGTM! Correctly protects newsletter routes.The addition of
/newslettersto the protected paths array is appropriate for the pro-user newsletter feature.apps/web/src/components/newsletters/NewsletterContent.tsx (1)
13-103: Well-structured paragraph grouping logic.The approach of accumulating inline elements and flushing them into paragraphs when encountering block elements (headings, images) is sound and creates clean HTML output.
start-dev.bat (1)
1-15: LGTM! Useful Windows development helper.The batch script correctly navigates the directory structure and launches both servers in separate windows with appropriate timing. This is a helpful addition for Windows developers.
apps/api/src/index.ts (2)
257-260: LGTM! Non-blocking database connection improves resilience.The non-blocking database connection approach works well with the environment-specific error handling in apps/api/src/prisma.ts. In development, the server can start for UI testing even without a database. In production, the process will exit on connection failure (handled in prisma.ts).
20-21: Enhanced logging improves developer experience.The added startup messages and emojis make the server state clearer during development.
Also applies to: 284-285
TROUBLESHOOTING.md (1)
1-47: Clear and helpful troubleshooting guide.The documentation covers common development issues with actionable solutions. The structure and content are well-organized.
RUN_AND_PUSH_GUIDE.md (1)
1-222: Comprehensive and well-structured guide.The documentation provides clear, step-by-step instructions for running the application and pushing changes. The git workflow section is particularly helpful.
apps/web/src/components/newsletters/NewsletterCard.tsx (2)
1-8: LGTM!The imports and interface definition are correct. Type safety is properly maintained with the Newsletter type import.
18-42: LGTM!The preview text extraction logic properly handles edge cases with optional chaining and a sensible fallback. The card component is well-structured with appropriate hover effects and accessibility.
apps/web/src/components/newsletters/NewsletterHeader.tsx (1)
1-32: LGTM!The header component is well-structured with consistent styling and proper use of Next.js Link for client-side navigation. The layout and hover effects align with the app's design system.
apps/web/src/app/(main)/newsletters/[id]/page.tsx (6)
12-18: LGTM!The component setup properly retrieves all necessary data including session, routing parameters, subscription status, and the newsletter. The optional chaining on
params?.idand conditional newsletter lookup handle edge cases appropriately.
20-21: LGTM!The test mode implementation using an environment variable is a clean approach that allows easy toggling between production and test behaviors without code changes.
23-45: LGTM!The authentication and authorization flow is well-structured:
- Test mode bypass is properly positioned first
- Loading states are checked before redirects to prevent premature navigation
- Redirect hierarchy (unauthenticated → login, non-pro → pricing) is logical
- Dependency array is complete
47-62: LGTM!The loading state logic is well-designed. Including authenticated non-pro users in the loading check (line 52) prevents content flash before the redirect in the useEffect, which is good UX.
64-67: LGTM!The authorization guard properly prevents content rendering for unauthorized users (unless in test mode), ensuring that the component returns null while the useEffect redirects.
69-88: LGTM!The not-found UI provides clear feedback with a helpful back link, maintaining good UX when a newsletter ID doesn't exist.
apps/web/src/app/(main)/newsletters/page.tsx (3)
21-43: LGTM!The authentication and authorization flow matches the detail page pattern and is correctly implemented with test mode bypass, loading checks, and proper redirect hierarchy.
45-60: LGTM!The loading state logic is consistent with the detail page and properly prevents content flash before redirects.
62-109: LGTM!The content rendering logic is well-structured:
- Proper authorization guard
- Clear empty state
- Newsletters grouped by month/year with appropriate headings
- NewsletterCard components properly keyed and mapped
- The key format parsing (line 85) correctly matches the format from
getNewslettersByDate()apps/web/NEWSLETTER_DOCUMENTATION.md (1)
1-192: LGTM!The documentation is comprehensive and well-structured:
- Clear step-by-step instructions for adding newsletters
- All content types properly documented with examples
- Examples match the TypeScript interface definitions
- Important guidelines and format requirements are clearly stated
- Testing instructions are helpful
This provides excellent guidance for future contributors.
apps/web/src/data/newsletters.ts (2)
19-257: LGTM!The sample newsletter data is well-structured:
- Follows the documented format and naming conventions
- Demonstrates all content types (text, heading, link, bold)
- Dates are properly formatted as YYYY-MM-DD
- Content maintains the lowercase style per project requirements
- Provides good examples for future contributors
294-297: LGTM!The lookup function is straightforward and correctly typed. Linear search is appropriate for the small, static dataset.
| const date = new Date(newsletter.date); | ||
| const formattedDate = date.toLocaleDateString("en-US", { | ||
| year: "numeric", | ||
| month: "long", | ||
| day: "numeric", | ||
| }); | ||
|
|
||
| return ( | ||
| <main className="min-h-screen w-full bg-[#101010] text-white"> | ||
| <NewsletterHeader /> | ||
| <div className="max-w-[2000px] mx-auto px-6 py-12"> | ||
| <div className="max-w-3xl mx-auto"> | ||
| <Link | ||
| href="/newsletters" | ||
| className="text-[#9455f4] hover:text-white underline transition-colors mb-6 inline-block" | ||
| > | ||
| ← back to newsletters | ||
| </Link> | ||
|
|
||
| <article className="mt-8"> | ||
| <header className="mb-8"> | ||
| <h1 className="text-4xl font-bold text-white mb-4"> | ||
| {newsletter.title} | ||
| </h1> | ||
| <time className="text-gray-400 font-mono text-sm"> | ||
| {formattedDate} | ||
| </time> | ||
| </header> | ||
|
|
||
| <div className="prose prose-invert max-w-none"> | ||
| <NewsletterContent content={newsletter.content} /> | ||
| </div> | ||
| </article> | ||
| </div> | ||
| </div> | ||
| </main> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Add error handling for invalid date formats.
The date parsing (lines 90-95) assumes a valid YYYY-MM-DD format without validation, similar to NewsletterCard. Invalid dates could result in displaying "Invalid Date" to users.
Consider adding validation:
const date = new Date(newsletter.date);
+ const isValidDate = !isNaN(date.getTime());
const formattedDate = date.toLocaleDateString("en-US", {
year: "numeric",
month: "long",
day: "numeric",
});Then conditionally render the date or omit it if invalid. Otherwise, the article structure and rendering logic are excellent with proper semantic HTML elements.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const date = new Date(newsletter.date); | |
| const formattedDate = date.toLocaleDateString("en-US", { | |
| year: "numeric", | |
| month: "long", | |
| day: "numeric", | |
| }); | |
| return ( | |
| <main className="min-h-screen w-full bg-[#101010] text-white"> | |
| <NewsletterHeader /> | |
| <div className="max-w-[2000px] mx-auto px-6 py-12"> | |
| <div className="max-w-3xl mx-auto"> | |
| <Link | |
| href="/newsletters" | |
| className="text-[#9455f4] hover:text-white underline transition-colors mb-6 inline-block" | |
| > | |
| ← back to newsletters | |
| </Link> | |
| <article className="mt-8"> | |
| <header className="mb-8"> | |
| <h1 className="text-4xl font-bold text-white mb-4"> | |
| {newsletter.title} | |
| </h1> | |
| <time className="text-gray-400 font-mono text-sm"> | |
| {formattedDate} | |
| </time> | |
| </header> | |
| <div className="prose prose-invert max-w-none"> | |
| <NewsletterContent content={newsletter.content} /> | |
| </div> | |
| </article> | |
| </div> | |
| </div> | |
| </main> | |
| ); | |
| } | |
| const date = new Date(newsletter.date); | |
| const isValidDate = !isNaN(date.getTime()); | |
| const formattedDate = date.toLocaleDateString("en-US", { | |
| year: "numeric", | |
| month: "long", | |
| day: "numeric", | |
| }); | |
| return ( | |
| <main className="min-h-screen w-full bg-[#101010] text-white"> | |
| <NewsletterHeader /> | |
| <div className="max-w-[2000px] mx-auto px-6 py-12"> | |
| <div className="max-w-3xl mx-auto"> | |
| <Link | |
| href="/newsletters" | |
| className="text-[#9455f4] hover:text-white underline transition-colors mb-6 inline-block" | |
| > | |
| ← back to newsletters | |
| </Link> | |
| <article className="mt-8"> | |
| <header className="mb-8"> | |
| <h1 className="text-4xl font-bold text-white mb-4"> | |
| {newsletter.title} | |
| </h1> | |
| <time className="text-gray-400 font-mono text-sm"> | |
| {formattedDate} | |
| </time> | |
| </header> | |
| <div className="prose prose-invert max-w-none"> | |
| <NewsletterContent content={newsletter.content} /> | |
| </div> | |
| </article> | |
| </div> | |
| </div> | |
| </main> | |
| ); | |
| } |
🤖 Prompt for AI Agents
In apps/web/src/app/(main)/newsletters/[id]/page.tsx around lines 90 to 127 the
code unconditionally constructs and formats a Date from newsletter.date which
can produce "Invalid Date" for bad input; change the logic to validate the date
after parsing (e.g., create the Date, check !isNaN(date.getTime()) or use
Date.parse) and only compute formattedDate and render the <time> element when
the date is valid, otherwise omit the time (or render an empty/fallback string);
update the JSX to conditionally render the <time> node based on that validation.
There was a problem hiding this comment.
Hi @apsinghdev 👋,
I have completed the newsletter page UI implementation for issue #155 and submitted this PR.
Merge conflicts are resolved and the UI is working properly.
Kindly review it whenever you get time.
Happy to make changes if required. Thanks!
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| // test mode: allow access without auth for testing (set NEXT_PUBLIC_ENABLE_TEST_MODE=true in .env.local) | ||
| // if no providers are configured, we're in test mode | ||
| const testMode = process.env.NEXT_PUBLIC_ENABLE_TEST_MODE === "true"; |
There was a problem hiding this comment.
Remove misleading comment.
The comment on line 18 states "if no providers are configured, we're in test mode", but the code actually checks the NEXT_PUBLIC_ENABLE_TEST_MODE environment variable. This comment doesn't match the implementation and could confuse future developers.
Apply this diff:
// test mode: allow access without auth for testing (set NEXT_PUBLIC_ENABLE_TEST_MODE=true in .env.local)
- // if no providers are configured, we're in test mode
const testMode = process.env.NEXT_PUBLIC_ENABLE_TEST_MODE === "true";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // test mode: allow access without auth for testing (set NEXT_PUBLIC_ENABLE_TEST_MODE=true in .env.local) | |
| // if no providers are configured, we're in test mode | |
| const testMode = process.env.NEXT_PUBLIC_ENABLE_TEST_MODE === "true"; | |
| // test mode: allow access without auth for testing (set NEXT_PUBLIC_ENABLE_TEST_MODE=true in .env.local) | |
| const testMode = process.env.NEXT_PUBLIC_ENABLE_TEST_MODE === "true"; |
🤖 Prompt for AI Agents
In apps/web/src/app/(main)/newsletters/page.tsx around lines 17 to 19, the
inline comment incorrectly claims "if no providers are configured, we're in test
mode" while the code actually checks NEXT_PUBLIC_ENABLE_TEST_MODE; update or
remove the misleading comment so it accurately reflects implementation (e.g.,
say the value is controlled by NEXT_PUBLIC_ENABLE_TEST_MODE or just remove the
second sentence), keeping the remaining comment that notes test mode allows
access without auth.
| const date = new Date(newsletter.date); | ||
| const formattedDate = date.toLocaleDateString("en-US", { | ||
| year: "numeric", | ||
| month: "long", | ||
| day: "numeric", | ||
| }); |
There was a problem hiding this comment.
Add error handling for invalid date formats.
The date parsing assumes a valid YYYY-MM-DD format without validation. Invalid dates could result in "Invalid Date" being displayed to users.
Consider adding validation:
export default function NewsletterCard({ newsletter }: NewsletterCardProps) {
const date = new Date(newsletter.date);
+ const isValidDate = !isNaN(date.getTime());
const formattedDate = date.toLocaleDateString("en-US", {
year: "numeric",
month: "long",
day: "numeric",
});Then use isValidDate to conditionally render the date or a fallback.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/web/src/components/newsletters/NewsletterCard.tsx around lines 11 to 16,
the code blindly constructs a Date from newsletter.date and formats it, which
can produce "Invalid Date" for bad input; add a validation step (e.g., check
that new Date(newsletter.date) is valid via !isNaN(date.getTime()) or an
isValidDate helper that enforces the expected YYYY-MM-DD pattern and range),
then only call toLocaleDateString when valid, otherwise render a safe fallback
like "Unknown date" (or omit the date) so users never see "Invalid Date".
| case "heading": | ||
| flushParagraph(); | ||
| const HeadingTag = `h${item.level || 2}` as keyof JSX.IntrinsicElements; | ||
| elements.push( | ||
| <HeadingTag | ||
| key={index} | ||
| className={`font-bold text-white mt-8 mb-4 ${ | ||
| item.level === 1 | ||
| ? "text-3xl" | ||
| : item.level === 2 | ||
| ? "text-2xl" | ||
| : item.level === 3 | ||
| ? "text-xl" | ||
| : "text-lg" | ||
| }`} | ||
| > | ||
| {item.content} | ||
| </HeadingTag> | ||
| ); | ||
| break; |
There was a problem hiding this comment.
Fix variable declaration scope in switch statement.
The HeadingTag variable declared at line 61 is accessible to other switch cases due to variable hoisting, which can cause errors.
Apply this diff to wrap the declaration in a block:
switch (item.type) {
- case "heading":
+ case "heading": {
flushParagraph();
const HeadingTag = `h${item.level || 2}` as keyof JSX.IntrinsicElements;
elements.push(
<HeadingTag
key={index}
className={`font-bold text-white mt-8 mb-4 ${
item.level === 1
? "text-3xl"
: item.level === 2
? "text-2xl"
: item.level === 3
? "text-xl"
: "text-lg"
}`}
>
{item.content}
</HeadingTag>
);
break;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| case "heading": | |
| flushParagraph(); | |
| const HeadingTag = `h${item.level || 2}` as keyof JSX.IntrinsicElements; | |
| elements.push( | |
| <HeadingTag | |
| key={index} | |
| className={`font-bold text-white mt-8 mb-4 ${ | |
| item.level === 1 | |
| ? "text-3xl" | |
| : item.level === 2 | |
| ? "text-2xl" | |
| : item.level === 3 | |
| ? "text-xl" | |
| : "text-lg" | |
| }`} | |
| > | |
| {item.content} | |
| </HeadingTag> | |
| ); | |
| break; | |
| case "heading": { | |
| flushParagraph(); | |
| const HeadingTag = `h${item.level || 2}` as keyof JSX.IntrinsicElements; | |
| elements.push( | |
| <HeadingTag | |
| key={index} | |
| className={`font-bold text-white mt-8 mb-4 ${ | |
| item.level === 1 | |
| ? "text-3xl" | |
| : item.level === 2 | |
| ? "text-2xl" | |
| : item.level === 3 | |
| ? "text-xl" | |
| : "text-lg" | |
| }`} | |
| > | |
| {item.content} | |
| </HeadingTag> | |
| ); | |
| break; | |
| } |
🧰 Tools
🪛 Biome (2.1.2)
[error] 61-61: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🤖 Prompt for AI Agents
In apps/web/src/components/newsletters/NewsletterContent.tsx around lines 59 to
78, the const HeadingTag declared in the "heading" switch case is not
block-scoped to that case and may leak into other cases; wrap the case body in
its own block (add { ... } after case "heading":) so flushParagraph() remains
before the block and the HeadingTag and related variables are scoped inside the
block, then keep the break; inside that block.
| // skip auth check if NEXTAUTH_SECRET is not configured or test mode is enabled | ||
| if (!process.env.NEXTAUTH_SECRET || process.env.NEXT_PUBLIC_ENABLE_TEST_MODE === "true") { | ||
| return NextResponse.next(); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Test mode bypass creates deployment risk.
The test mode bypass (NEXT_PUBLIC_ENABLE_TEST_MODE === "true") completely disables authentication. Since NEXT_PUBLIC_* variables are exposed to the client bundle, this creates a risk if accidentally deployed to production.
Consider adding safeguards:
// skip auth check if NEXTAUTH_SECRET is not configured or test mode is enabled
- if (!process.env.NEXTAUTH_SECRET || process.env.NEXT_PUBLIC_ENABLE_TEST_MODE === "true") {
+ if (!process.env.NEXTAUTH_SECRET) {
+ return NextResponse.next();
+ }
+ if (process.env.NEXT_PUBLIC_ENABLE_TEST_MODE === "true" && process.env.NODE_ENV !== "production") {
return NextResponse.next();
}Run this script to verify test mode isn't enabled in production configs:
🏁 Script executed:
#!/bin/bash
# Check for test mode in production configs
rg -n "NEXT_PUBLIC_ENABLE_TEST_MODE.*true" --type-add 'env:*.env*' --type env -g '!*.local' -g '!*.development'Length of output: 0
🏁 Script executed:
#!/bin/bash
# 1. Find all references to NEXT_PUBLIC_ENABLE_TEST_MODE to understand usage
echo "=== References to NEXT_PUBLIC_ENABLE_TEST_MODE ==="
rg -n "NEXT_PUBLIC_ENABLE_TEST_MODE" -g '!node_modules' -C2
echo ""
echo "=== Check for build-time safeguards or warnings ==="
# Look for build config files that might enforce production safety
fd -e "next.config" -e "tsconfig" -e ".env.production" 2>/dev/null | head -20
echo ""
echo "=== Check for any validation or warning logic ==="
rg -n "NODE_ENV.*production|process.env.NODE_ENV" apps/web/src/middleware.ts -C3
echo ""
echo "=== Look for TESTING_MODE.md documentation ==="
fd -i "TESTING_MODE" -o -i "testing.*guide"Length of output: 1987
🏁 Script executed:
#!/bin/bash
# 2. Verify what happens when test mode is disabled (normal production scenario)
echo "=== Checking middleware logic for auth enforcement ==="
rg -n "middleware" apps/web/src/ --type=ts -C5 | head -50
echo ""
echo "=== Look for environment validation at startup ==="
rg -n "process.env" apps/web/src/ --type=ts -g 'middleware*' | head -30Length of output: 1798
🏁 Script executed:
#!/bin/bash
# Check for production environment validation in build config and deployment
echo "=== Checking for NODE_ENV validation in build files ==="
fd -e "next.config.js" -e "next.config.mjs" -e "next.config.ts" 2>/dev/null
for file in $(fd -e "next.config.*" 2>/dev/null); do
echo "File: $file"
rg -n "NODE_ENV|NEXT_PUBLIC_ENABLE_TEST_MODE" "$file" || echo " (no matches)"
done
echo ""
echo "=== Look for package.json build scripts and deployment config ==="
if [ -f "package.json" ]; then
rg -n "NODE_ENV|build|start" package.json | head -20
fi
echo ""
echo "=== Check full middleware.ts logic for auth enforcement ==="
cat apps/web/src/middleware.tsLength of output: 1060
🏁 Script executed:
#!/bin/bash
# Verify current state: check if NODE_ENV check would be effective
echo "=== Review middleware.ts for test mode handling ==="
rg -n "test" apps/web/src/middleware.ts -i
echo ""
echo "=== Check if there are any guards against prod deployments with test mode ==="
rg -n "console.warn|console.error|throw.*Error" apps/web/src/middleware.tsLength of output: 309
Test mode bypass creates unprotected production deployment risk.
The middleware completely disables authentication for protected paths (/dashboard, /newsletters) when NEXT_PUBLIC_ENABLE_TEST_MODE === "true". Since NEXT_PUBLIC_* variables are embedded in the client bundle, accidental production deployment with this enabled would entirely bypass auth with no warnings or runtime guards.
Add NODE_ENV check to prevent test mode in production:
// skip auth check if NEXTAUTH_SECRET is not configured or test mode is enabled
- if (!process.env.NEXTAUTH_SECRET || process.env.NEXT_PUBLIC_ENABLE_TEST_MODE === "true") {
+ if (!process.env.NEXTAUTH_SECRET) {
+ return NextResponse.next();
+ }
+ if (process.env.NEXT_PUBLIC_ENABLE_TEST_MODE === "true" && process.env.NODE_ENV !== "production") {
return NextResponse.next();
}Alternatively, remove test mode override entirely and use dedicated test environment configuration.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // skip auth check if NEXTAUTH_SECRET is not configured or test mode is enabled | |
| if (!process.env.NEXTAUTH_SECRET || process.env.NEXT_PUBLIC_ENABLE_TEST_MODE === "true") { | |
| return NextResponse.next(); | |
| } | |
| // skip auth check if NEXTAUTH_SECRET is not configured or test mode is enabled | |
| if (!process.env.NEXTAUTH_SECRET) { | |
| return NextResponse.next(); | |
| } | |
| if (process.env.NEXT_PUBLIC_ENABLE_TEST_MODE === "true" && process.env.NODE_ENV !== "production") { | |
| return NextResponse.next(); | |
| } |
🤖 Prompt for AI Agents
In apps/web/src/middleware.ts around lines 5 to 8, the middleware currently
bypasses auth whenever NEXT_PUBLIC_ENABLE_TEST_MODE === "true", which is unsafe
because NEXT_PUBLIC_* is client-exposed; update the condition so test-mode
bypass only applies in non-production (e.g., add an explicit check
process.env.NODE_ENV !== "production" before allowing the bypass), or remove the
NEXT_PUBLIC_ENABLE_TEST_MODE override entirely and rely on server-only envs or a
dedicated test environment; ensure the change prevents any client-exposed flag
from disabling auth in production and add a short comment explaining why.
| ### step 1: check your changes | ||
| ```bash | ||
| git status | ||
| ``` | ||
| you should see: | ||
| - new newsletter files | ||
| - documentation files | ||
| - modified pnpm-lock.yaml | ||
| ### step 2: create a feature branch | ||
| ```bash | ||
| git checkout -b feature/newsletter-page | ||
| ``` | ||
| ### step 3: stage your changes | ||
| ```bash | ||
| git add apps/web/src/app/(main)/newsletters/ | ||
| git add apps/web/src/components/newsletters/ | ||
| git add apps/web/src/data/newsletters.ts | ||
| git add apps/web/NEWSLETTER_DOCUMENTATION.md | ||
| git add apps/web/PR_DESCRIPTION.md | ||
| git add pnpm-lock.yaml | ||
| ``` | ||
| or add everything: | ||
| ```bash | ||
| git add . | ||
| ``` | ||
| **important:** don't commit the `.env` files (they're in .gitignore) | ||
| ### step 4: commit | ||
| ```bash | ||
| git commit -m "feat: add newsletter page for pro users | ||
|
|
||
| - implement newsletter listing page with date organization | ||
| - add newsletter detail page with rich content support | ||
| - support text, links, images, bold text, and headings | ||
| - add pro user protection and authentication | ||
| - include 3 sample newsletters | ||
| - add documentation for adding new newsletters | ||
|
|
||
| closes #155" | ||
| ``` |
There was a problem hiding this comment.
Fix invalid git commit example and avoid brittle line-number references.
- The
git commit -mexample uses a multi-line message with an opening quote but no closing quote. As written, this command will fail in the shell. Prefer a single-line message (orgit committo open an editor) and keep the detailed bullets only in the docs/PR description, e.g.:
-```bash
-git commit -m "feat: add newsletter page for pro users
-
-- implement newsletter listing page with date organization
-- add newsletter detail page with rich content support
-- support text, links, images, bold text, and headings
-- add pro user protection and authentication
-- include 3 sample newsletters
-- add documentation for adding new newsletters
-
-closes #155"
-```
+```bash
+git commit -m "feat: add newsletter page for pro users"
+```
+
+You can include the detailed bullets and `closes #155` in the PR description instead.- The instructions that say “comment out lines 17–35” and “change line 50 to
if (false) {” inpage.tsxare fragile; line numbers will drift as the file changes. It’s safer to reference the specificuseEffectthat handles auth redirects and the condition guarding the newsletter content in prose (e.g., “comment out the auth redirectuseEffectand temporarily force the content condition tofalse”) rather than hard-coded line numbers.
Also applies to: 46-60
🤖 Prompt for AI Agents
In NEXT_STEPS.md around lines 64–112, fix the broken multi-line git commit
example by replacing it with a single-line git commit instruction (or instruct
to run git commit to open an editor) and advise putting the detailed bullets and
"closes #155" into the PR description instead; also remove brittle hard-coded
line-number instructions (e.g., “comment out lines 17–35” and “change line 50…”)
and replace them with a prose reference to the specific code elements to edit —
namely, say to comment out the auth-redirect useEffect and temporarily force the
newsletter content guard condition to false (or explain how to locate that
useEffect and condition) so the guidance remains stable as the file changes
(also apply the same replacement for the 46–60 range).
| ```env | ||
| DATABASE_URL="postgresql://USER:PASSWORD@localhost:5432/opensox?schema=public" | ||
| JWT_SECRET="3bb3238092da53f8ba9d1e02e15efe8ec84341252a11eff1b28ff742c292224e" | ||
| PORT=8080 | ||
| CORS_ORIGINS=http://localhost:3000 | ||
| NODE_ENV=development | ||
| ``` |
There was a problem hiding this comment.
Replace realistic-looking secrets with placeholders and fix the commit example.
- The example values for
JWT_SECRETandNEXTAUTH_SECRETare long random hex strings and have already been flagged by Gitleaks as potential secrets. Even if they are dummy, committing realistic-looking secrets will keep triggering leak scanners and encourages copy-pasting the same secret everywhere. Align this guide withSETUP_INSTRUCTIONS.mdby using descriptive placeholders and instructing users to generate their own secret, e.g.:
-```env
-DATABASE_URL="postgresql://USER:PASSWORD@localhost:5432/opensox?schema=public"
-JWT_SECRET="3bb3238092da53f8ba9d1e02e15efe8ec84341252a11eff1b28ff742c292224e"
-PORT=8080
-CORS_ORIGINS=http://localhost:3000
-NODE_ENV=development
-```
+```env
+DATABASE_URL="postgresql://USER:PASSWORD@localhost:5432/opensox?schema=public"
+JWT_SECRET="replace-with-a-strong-random-secret"
+PORT=8080
+CORS_ORIGINS=http://localhost:3000
+NODE_ENV=development
+```
…
-```env
-NEXT_PUBLIC_API_URL="http://localhost:8080"
-GOOGLE_CLIENT_ID="dummy-client-id"
-GOOGLE_CLIENT_SECRET="dummy-secret"
-NEXTAUTH_SECRET="3bb3238092da53f8ba9d1e02e15efe8ec84341252a11eff1b28ff742c292224e"
-NEXTAUTH_URL="http://localhost:3000"
-```
+```env
+NEXT_PUBLIC_API_URL="http://localhost:8080"
+GOOGLE_CLIENT_ID="dummy-client-id"
+GOOGLE_CLIENT_SECRET="dummy-secret"
+NEXTAUTH_SECRET="replace-with-a-strong-random-secret"
+NEXTAUTH_URL="http://localhost:3000"
+```Also add a short note showing how to generate the secret (e.g., using node -e "console.log(require('crypto').randomBytes(32).toString('hex'))"), like you did in SETUP_INSTRUCTIONS.md.
- The
git commit -mexample here has the same issue as inNEXT_STEPS.md: multi-line message inside an unterminated quoted string. Consider simplifying to a single-line-m(and move the detailed bullets into the PR description), for example:
-# commit
-git commit -m "feat: add newsletter page for pro users
-…
-closes #155"
+# commit
+git commit -m "feat: add newsletter page for pro users"If you need a multi-paragraph commit message, recommend running git commit without -m so the editor can be used instead.
Also applies to: 23-29, 76-85
🧰 Tools
🪛 Gitleaks (8.29.0)
[high] 11-11: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
| from the root directory (`opensox`): | ||
|
|
||
| ```bash | ||
| pnpm install | ||
| ``` |
There was a problem hiding this comment.
Add language identifier to fenced code block.
Specify the language for better rendering and tooling support.
Apply this diff:
from the root directory (`opensox`):
-```
+```bash
pnpm install
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.18.1)</summary>
14-14: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
In RUN_AND_PUSH_GUIDE.md around lines 14 to 18, the fenced code block for the
install command is missing a language identifier; update the opening fence to
specify "bash" (i.e., change the triple backticks to start with ```bash) so the
block becomes a bash-highlighted code block.
</details>
<!-- This is an auto-generated comment by CodeRabbit -->
| # testing mode - authentication bypassed | ||
|
|
||
| ## current status | ||
|
|
||
| authentication has been **temporarily disabled** on the newsletter pages for ui testing. this allows you to view the newsletter page without setting up oauth credentials. | ||
|
|
||
| ## what's changed | ||
|
|
||
| the following files have been modified to bypass authentication: | ||
| - `apps/web/src/app/(main)/newsletters/page.tsx` | ||
| - `apps/web/src/app/(main)/newsletters/[id]/page.tsx` | ||
|
|
||
| the authentication checks are commented out with notes indicating they're for testing only. | ||
|
|
||
| ## how to test | ||
|
|
||
| 1. make sure the dev server is running: `pnpm dev` | ||
| 2. open your browser and go to: `http://localhost:3000/newsletters` | ||
| 3. you should now see the newsletter listing page without needing to log in | ||
| 4. click on any newsletter to view the full content | ||
|
|
||
| ## before pushing to github | ||
|
|
||
| **important:** you must re-enable authentication before creating your pull request! | ||
|
|
||
| to re-enable: | ||
| 1. uncomment all the authentication code in both newsletter page files | ||
| 2. remove the "temporarily disabled" comments | ||
| 3. test that authentication works properly | ||
| 4. then commit and push | ||
|
|
||
| ## re-enabling authentication | ||
|
|
||
| uncomment the following sections in both files: | ||
| - the `useEffect` hook that handles redirects | ||
| - the loading state check | ||
| - the authentication check before rendering content | ||
|
|
||
| make sure to test that: | ||
| - unauthenticated users are redirected to login | ||
| - authenticated non-pro users are redirected to pricing | ||
| - only pro users can see the newsletter content | ||
|
|
There was a problem hiding this comment.
Clarify test mode implementation in documentation.
The documentation states that "authentication has been temporarily disabled" and requires manual uncommenting to re-enable. However, the actual implementation uses the NEXT_PUBLIC_ENABLE_TEST_MODE environment variable to control test mode, which is a cleaner approach.
Consider updating this documentation to:
- Clarify that test mode is controlled via the
NEXT_PUBLIC_ENABLE_TEST_MODEenvironment variable - Explain that authentication is bypassed when this variable is set to
"true", not permanently commented out - Update the "before pushing" section to mention setting the env var to
"false"or removing it
This will prevent confusion about the actual implementation and testing workflow.
🤖 Prompt for AI Agents
In TESTING_MODE.md around lines 1 to 43, the doc currently says authentication
is "temporarily disabled" and instructs manual uncommenting, but the code
actually uses the NEXT_PUBLIC_ENABLE_TEST_MODE env var; update the doc to state
that test mode is controlled by NEXT_PUBLIC_ENABLE_TEST_MODE and that setting it
to "true" bypasses auth, replace the manual uncomment instructions with guidance
to set the env var to "false" (or remove it) before pushing, and add a short
note on how to toggle and verify authentication (start dev server and confirm
unauthenticated users are redirected when the env var is not "true").
|
Hi @apsinghdev 👋, I have completed the newsletter page UI and created this PR for issue #155. Please take a look whenever you get time. |
|
@Sandesh-Upadhyay hey, can you pls attach a screen recording of your changes? |
|
@Sandesh-Upadhyay thanks for the submission! Unfortunately, we are moving with a different submission this time, so we won't be able to accept it. Still, you are welcome to make contributions! 🙏 |
2 participants
this pr implements a newsletter page for pro users on opensox.ai. the newsletter page displays newsletters as blog posts with rich content support, organized by date (month and year).
core features:
organization by date: newsletters automatically organized by month and year, latest first
rich content support: text, links, images, bold text, and headings (h1-h6)
content management: easy to add newsletters through code in src/data/newsletters.ts
newsletter listing: all newsletters displayed in one place, ordered by date (latest on top)
readability: clean design with proper typography and spacing
minimal formatting: bold text and headings as required
pro user protection:
newsletter page only accessible to authenticated pro users
non-pro users redirected to pricing page
unauthenticated users redirected to login page
test mode available for ui testing without auth
sample data:
three dummy newsletters included:
"welcome to opensox ai newsletter" (november 2025)
"building in public: lessons learned" (november 2025)
"launching opensox ai pro" (october 2025)
documentation:
NEWSLETTER_DOCUMENTATION.md: how to add newsletters
PR_DESCRIPTION.md: detailed pr description
technical implementation:
next.js 15 with app router
tailwind css for styling
custom content renderer that groups inline elements into paragraphs
automatic date organization with grouping and sorting by month/year
middleware and page-level pro user protection
files added:
src/data/newsletters.ts: newsletter data structure and sample data
src/app/(main)/newsletters/page.tsx: newsletter listing page
src/app/(main)/newsletters/[id]/page.tsx: individual newsletter detail page
src/components/newsletters/NewsletterHeader.tsx: header component
src/components/newsletters/NewsletterCard.tsx: newsletter card component
src/components/newsletters/NewsletterContent.tsx: rich content renderer
files modified:
src/middleware.ts: added newsletters to protected paths, test mode support
src/lib/auth/config.ts: conditional provider loading, test mode support
src/components/dashboard/Sidebar.tsx: added newsletter link to sidebar
implements bounty requirements for newsletter page feature.
Summary by CodeRabbit
New Features
Newsletter section to browse and read newsletters (pro-only), with list and detail pages, cards, headers, and rich content rendering
Sidebar link for quick access to newsletters
Documentation
Added setup, quick-start, run & push, testing, troubleshooting, PR guidance, and newsletter authoring docs
Bug Fixes
Mobile sidebar styling corrected
Chores
Dev helper to start servers on Windows (development convenience)
Summary by CodeRabbit
New Features
Bug Fixes
Documentation