Implementation of NewsLetter #161
Conversation
|
@codebyNJ 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. |
WalkthroughImplements a complete newsletter feature for pro users with data structures defining newsletter content types, new pages for listing and displaying individual newsletters with pro-only access control, rich content rendering component, shader-based visual header component, sidebar integration, and comprehensive documentation on implementation and usage. Changes
Sequence DiagramsequenceDiagram
participant User
participant Browser
participant NewslettersPage
participant useSubscription
participant PricingPage
participant NewsletterDetail
User->>Browser: Navigate to /dashboard/newsletters
Browser->>NewslettersPage: Load page
NewslettersPage->>useSubscription: Check subscription status
alt User is Pro/Paid
useSubscription-->>NewslettersPage: isPaidUser = true
NewslettersPage->>NewslettersPage: Render newsletter list<br/>(filtered, sorted by date)
NewslettersPage-->>Browser: Show newsletters
User->>Browser: Click newsletter card
Browser->>NewsletterDetail: Load /dashboard/newsletters/[id]
NewsletterDetail->>useSubscription: Verify subscription
useSubscription-->>NewsletterDetail: isPaidUser = true
NewsletterDetail->>NewsletterDetail: Render header image<br/>+ FaultyTerminal + content
NewsletterDetail-->>Browser: Display newsletter detail
else User is not Pro
useSubscription-->>NewslettersPage: isPaidUser = false
NewslettersPage->>PricingPage: Redirect to /pricing
PricingPage-->>Browser: Show pricing page
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related issues
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✅ 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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/web/src/app/(main)/(landing)/page.tsx (1)
8-8: Remove unused import.The Navbar is imported but no longer used in this file (line 18 has its usage commented out). Remove this import to keep the code clean.
-import Navbar from '@/components/landing-sections/navbar'apps/web/src/components/dashboard/Sidebar.tsx (1)
29-45: Gate the Newsletters sidebar item behind subscription to match the documented behaviorRight now the “Newsletters” item is rendered for everyone, even though:
- You’re already calling
useSubscription()and readingisPaidUser.NEWSLETTER_GUIDE.mdexplicitly says “The newsletter link in the sidebar only appears for pro users.”This is just a UI mismatch (users get redirected later), but it’s easy to align behavior with the docs by gating the route inside the map:
- {SIDEBAR_ROUTES.map((route) => { - const isActive = pathname === route.path || pathname.startsWith(`${route.path}/`); + {SIDEBAR_ROUTES.map((route) => { + if (route.path === "/dashboard/newsletters" && !isPaidUser) { + return null; + } + + const isActive = + pathname === route.path || + pathname.startsWith(`${route.path}/`); const activeClass = isActive ? "text-ox-purple" : "text-ox-white"; return ( <Link href={route.path} className={activeClass} key={route.path}> <SidebarItem itemName={route.label} icon={route.icon} collapsed={isCollapsed} /> </Link> ); })}(Feel free to add a
requiresProflag on the route definition instead of hard‑coding the path if you prefer.)Also,
getSidebarLinkClassNameis now unused and could be dropped in a later tidy‑up.Also applies to: 57-57, 125-137
🧹 Nitpick comments (10)
apps/api/package.json (1)
11-11: Consider extracting the inline script to a separate file for maintainability.While the inline Node script works, the complexity makes it difficult to read and modify. Extracting this to a separate
scripts/postinstall.jsfile would improve maintainability and allow for better error logging if needed.// scripts/postinstall.js const fs = require('fs'); const { execSync } = require('child_process'); try { if (fs.existsSync('prisma/schema.prisma')) { execSync('prisma generate', { stdio: 'inherit' }); } } catch (e) { // Silent fail to match original behavior }Then in package.json:
-"postinstall": "node -e \"try { if (require('fs').existsSync('prisma/schema.prisma')) require('child_process').execSync('prisma generate', {stdio: 'inherit'}); } catch(e) {}\"" +"postinstall": "node scripts/postinstall.js"apps/web/src/app/(main)/blogs/page.tsx (3)
16-17: Question the necessity of UnifiedPost abstraction.The
UnifiedPosttype currently only wraps blog posts with a discriminator. This adds complexity without an obvious immediate benefit. Is this preparing to merge newsletters with blogs in the future? If not, consider simplifying by usingBlogPostdirectly.If the intent is to eventually show newsletters alongside blogs on this page, please document that intent in a comment or the PR description.
41-45: Consider extracting date parsing logic to a shared utility.The
parseDatefunction parses the "DD-MM-YY" date format. If this format is used elsewhere in the codebase (e.g., newsletter dates), consider extracting it to a shared utility function to avoid duplication.Create a utility file
apps/web/src/lib/dateUtils.ts:export function parseBlogDate(dateStr: string): Date { const [day, month, year] = dateStr.split("-").map(Number); return new Date(2000 + year, month - 1, day); }Then use it in the component:
+import { parseBlogDate } from "@/lib/dateUtils"; + // Sort by date return filtered.sort((a, b) => { - const parseDate = (dateStr: string) => { - const [day, month, year] = dateStr.split("-").map(Number); - return new Date(2000 + year, month - 1, day); - }; - return parseDate(b.date).getTime() - parseDate(a.date).getTime(); + return parseBlogDate(b.date).getTime() - parseBlogDate(a.date).getTime(); });
22-31: Simplified mapping adds unnecessary overhead.The
allPostsarray currently just mapsblogsinto the same structure with an addedtypefield. Since you're only using blogs here, this adds cognitive overhead without functional benefit.apps/web/package.json (1)
33-33: Newogldependency – confirm types and bundle impactAdding
oglto back theFaultyTerminalcomponent makes sense. Please just double‑check:
- That you either have TypeScript typings for
oglor a small ambientdeclare module "ogl";somewhere to keep TS happy.- That the bundle impact is acceptable; if this WebGL effect stays confined to the newsletter detail header, a future enhancement could be to lazy‑load it to keep the core dashboard bundle lighter.
Separately (not caused by this PR),
next: "15.5.3"is currently paired withreact/react-dom18.2.0; Next 15’s docs expect React 19+. Worth verifying the overall app’s React version upgrade plan in a follow‑up.apps/web/src/data/NEWSLETTER_GUIDE.md (1)
60-79: Inline content example doesn’t match the renderer’s paragraph groupingWith the current
NewsletterContentlogic, this example:{ type: "heading", level: 1, content: "Welcome!" }, { type: "paragraph", content: "This is a paragraph with " }, { type: "bold", content: "bold text" }, { type: "text", content: " and a " }, { type: "link", text: "link", url: "https://opensox.ai" }, { type: "text", content: "." },will render as two separate paragraphs (the
paragraphitem becomes its own block; subsequent inline items are grouped into another block), not a single line of text.To match how the renderer actually works and avoid confusing future authors, consider updating the example like:
- { type: "paragraph", content: "This is a paragraph with " }, + { type: "text", content: "This is a paragraph with " },and/or explicitly stating that:
paragraphis for standalone block paragraphs, andtext/bold/linkare intended to be combined within the same paragraph (typically ended with an emptyparagraphitem for spacing).Also applies to: 83-86
apps/web/src/app/(main)/dashboard/newsletters/page.tsx (3)
55-66: Search helper is correct but non‑exhaustive for future content types
getNewsletterTextContentcorrectly pulls text from current block types and is used consistently in the search filter. However, if newNewsletterContentTypevariants are added later (e.g., lists, code blocks), they’ll silently be excluded from search results.Consider either:
- Centralizing “to plain text” logic next to the
NewsletterContentTypedefinition, or- Using a
switchwith an exhaustive check (and a fallback that logs when a new type is unhandled),so new content variants automatically participate in search.
Also applies to: 68-82
46-52: Month filter options are global, not per year
availableMonthsis computed from all newsletters once, and the month chips are rendered independently of the selected year. If your data eventually spans multiple years, picking a month plus a year with no newsletters will show a “No newsletters found” state even though that month might exist in other years.If you intend the month filter to be contextual to the selected year, consider recomputing available months based on
selectedYear(or showing only months that have data for the chosen year).Also applies to: 301-335
339-347: Avoid recomputing the total results count twiceThe total results count is computed via
filteredAndGroupedNewsletters.reduce(...)twice in the same expression. It’s minor, but you can simplify and avoid duplicate work:- {(searchQuery || selectedYear !== null || selectedMonth !== null) && ( - <div className="text-sm text-[#9b9a97]"> - {filteredAndGroupedNewsletters.reduce( - (sum, group) => sum + group.newsletters.length, - 0 - )}{" "} - newsletter{filteredAndGroupedNewsletters.reduce((sum, group) => sum + group.newsletters.length, 0) !== 1 ? "s" : ""} found - </div> - )} + {(searchQuery || selectedYear !== null || selectedMonth !== null) && (() => { + const total = filteredAndGroupedNewsletters.reduce( + (sum, group) => sum + group.newsletters.length, + 0 + ); + return ( + <div className="text-sm text-[#9b9a97]"> + {total} newsletter{total !== 1 ? "s" : ""} found + </div> + ); + })()}This keeps the JSX clearer and slightly reduces repeated work.
apps/web/src/data/newsletters.ts (1)
28-99: Seed data looks good; double‑check external assets and future extensibilityThe two sample newsletters cover headings, paragraphs, bold text, links, and images, which is a good functional test set for the renderer and search. A couple of minor checks:
- All
datevalues use the documentedYYYY-MM-DDformat and appear consistent with the archive sorting logic in the page.- The
headerImageandimage.srcfields all point to Unsplash; as noted in the page review, confirm these domains are whitelisted in your Next.js image config.- If you plan to actively use the optional
categoryfield in the UI later, consider adding at least one example here so downstream components have real data to exercise.Nothing blocking, just small data/UX considerations.
📜 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 (18)
NEWSLETTER_IMPLEMENTATION.md(1 hunks)apps/api/package.json(1 hunks)apps/web/next.config.js(1 hunks)apps/web/package.json(1 hunks)apps/web/src/app/(main)/(landing)/layout.tsx(1 hunks)apps/web/src/app/(main)/(landing)/page.tsx(1 hunks)apps/web/src/app/(main)/blogs/page.tsx(2 hunks)apps/web/src/app/(main)/dashboard/newsletters/[id]/page.tsx(1 hunks)apps/web/src/app/(main)/dashboard/newsletters/page.tsx(1 hunks)apps/web/src/components/blogs/BlogHeader.tsx(1 hunks)apps/web/src/components/dashboard/Sidebar.tsx(4 hunks)apps/web/src/components/landing-sections/Hero.tsx(1 hunks)apps/web/src/components/landing-sections/navbar.tsx(6 hunks)apps/web/src/components/newsletters/NewsletterContent.tsx(1 hunks)apps/web/src/components/ui/FaultyTerminal.tsx(1 hunks)apps/web/src/data/NEWSLETTER_GUIDE.md(1 hunks)apps/web/src/data/newsletters.ts(1 hunks)apps/web/src/middleware.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
apps/web/src/components/newsletters/NewsletterContent.tsx (1)
apps/web/src/data/newsletters.ts (1)
NewsletterContentType(11-17)
apps/web/src/components/landing-sections/Hero.tsx (1)
apps/web/src/components/icons/icons.tsx (1)
Terminal(1-5)
apps/web/src/components/dashboard/Sidebar.tsx (1)
apps/web/src/hooks/useSubscription.ts (1)
useSubscription(11-75)
apps/web/src/app/(main)/dashboard/newsletters/page.tsx (4)
apps/web/src/data/newsletters.ts (2)
NewsletterPost(19-26)newsletters(28-99)apps/web/src/hooks/useSubscription.ts (1)
useSubscription(11-75)apps/web/src/components/ui/SpinnerElm.tsx (1)
SpinnerElm(5-17)apps/web/src/components/ui/FaultyTerminal.tsx (1)
FaultyTerminal(244-424)
apps/web/src/components/landing-sections/navbar.tsx (1)
apps/web/src/lib/utils.ts (1)
cn(4-6)
apps/web/src/app/(main)/dashboard/newsletters/[id]/page.tsx (4)
apps/web/src/hooks/useSubscription.ts (1)
useSubscription(11-75)apps/web/src/data/newsletters.ts (1)
newsletters(28-99)apps/web/src/components/ui/FaultyTerminal.tsx (1)
FaultyTerminal(244-424)apps/web/src/components/newsletters/NewsletterContent.tsx (1)
NewsletterContent(11-165)
apps/web/src/app/(main)/blogs/page.tsx (1)
apps/web/src/data/blogs.ts (2)
BlogTag(1-6)blogs(15-70)
🔇 Additional comments (17)
apps/api/package.json (2)
11-11: Cross-platform postinstall script improvement.The change replaces Unix-specific shell syntax with a Node.js approach, making the postinstall script portable across Windows, macOS, and Linux. The
{stdio: 'inherit'}option correctly preserves output visibility.
7-12: Clarify the relationship between this infrastructure change and the newsletter PR.This postinstall script change appears orthogonal to the newsletter feature PR objectives (design/implementation of newsletter UI, content management, etc.). Please clarify whether this change is a prerequisite for the newsletter feature or should be submitted as a separate PR for better change isolation and review focus.
apps/web/next.config.js (1)
9-12: LGTM! Unsplash domain added for image optimization.This correctly allows the Next.js Image component to optimize and serve images from Unsplash, which aligns with the newsletter feature's need for external images.
apps/web/src/components/blogs/BlogHeader.tsx (2)
7-7: LGTM! Improved header styling with sticky positioning and blur effect.The sticky header with backdrop blur and semi-transparent background provides better visual hierarchy and modern UI polish.
11-11: LGTM! Consistent font weight for navigation links.Adding
font-mediumto both navigation links improves visual consistency.Also applies to: 17-17
NEWSLETTER_IMPLEMENTATION.md (1)
1-147: Comprehensive documentation provided.The documentation thoroughly covers the newsletter feature implementation, including architecture, usage, testing checklist, and future enhancements. This fulfills the PR objective requirement for "documentation on how to add a blog."
apps/web/src/app/(main)/blogs/page.tsx (1)
76-77: Minor: Updated message text is more generic.Changing from "No blog posts found." to "No posts found." is more generic, which aligns with the UnifiedPost abstraction if you plan to add other content types later.
apps/web/src/components/landing-sections/navbar.tsx (3)
32-34: Navbar now always visible (scroll-hide removed).The scroll-responsive hiding behavior has been removed for non-pricing pages. The navbar now remains visible at all times. This is a UX change that may improve navigation discoverability but removes the immersive full-screen scroll experience.
If this change was intentional, consider documenting the reasoning. If not, the previous scroll-hide logic should be restored.
49-56: LGTM! Modern glassmorphism styling with improved entrance animation.The updated styling (rounded corners, semi-transparent background with blur) provides a modern, polished appearance. The extended entrance animation with vertical offset creates a smoother initial render.
80-90: LGTM! Improved link styling with transitions.The white/white-90 color scheme with transition-colors provides better contrast and smooth hover interactions.
apps/web/src/components/landing-sections/Hero.tsx (1)
8-87: Hero layout and fixed Navbar integration look goodThe fixed Navbar, hero spacing, background image layering, and CTA setup are all consistent and should behave well across breakpoints. No changes needed here.
apps/web/src/components/newsletters/NewsletterContent.tsx (1)
11-163: Newsletter content grouping and rendering look solidThe
useMemoblock grouping plusrenderInlineContentcleanly support headings, images, and inline text/bold/link segments, and the Notion‑style typography/spacing reads well. I don’t see any functional issues here.apps/web/src/components/ui/FaultyTerminal.tsx (1)
244-399: WebGL lifecycle and FaultyTerminal component structure look well-managedThe component sets up the renderer/program/mesh inside
useEffect, wires resize and mouse handlers, and fully cleans up (RAF, observer, DOM node, and GL context) on unmount. Given how stateful WebGL is, this is a solid pattern, and I don’t see correctness issues in the React lifecycle handling.Also applies to: 421-423
apps/web/src/app/(main)/dashboard/newsletters/page.tsx (3)
22-36: Access control flow is sound for pro-only gatingThe combination of subscription-based
isLoadinghandling, early spinner return, redirect effect, and the finalif (!isPaidUser) return nullensures non‑paid users never see newsletter content and get pushed to/pricing. This matches the “pro users only” requirement fromuseSubscription(apps/web/src/hooks/useSubscription.ts) and looks correct.Also applies to: 138-148
155-183: Overall archive UI, grouping, and card layout look strongThe combination of the header visual (
FaultyTerminalbanner), year/month grouping, date‑sorted sections, and card layout with header images and clear titles delivers a readable, polished newsletter archive that aligns well with the “Notion‑like, readability‑focused” brief. The grouping and sorting logic (latest first per requirements) also matches the NewsletterPost dates correctly.Also applies to: 352-441
393-408: Confirm remote image domains are configured for Next.jsCard headers use
next/imagewith Unsplash URLs fromnewsletter.headerImage. Ensure the Next.js image configuration (likely in the root or app-specificnext.config.*) includesimages.unsplash.com(and any other domains you plan to use), otherwise images will fail to load at runtime.apps/web/src/data/newsletters.ts (1)
1-27: Type definitions and inline documentation are clear and extensible
NewsletterContentTypeandNewsletterPostcleanly model the rich content you need (headings, bold, links, images, paragraphs), and the top comment concisely documents how to add new newsletters, including date format. This should be easy for the team to extend without over‑engineering.
| @@ -1,10 +1,10 @@ | |||
| import Navbar from '@/components/landing-sections/navbar' | |||
| // import Navbar from '@/components/landing-sections/navbar' | |||
There was a problem hiding this comment.
🧩 Analysis chain
Consider removing commented code or clarifying intent.
The Navbar import and rendering are commented out rather than removed. According to the AI summary, the Navbar has been moved to the Hero component, which is an unusual architectural choice since layouts typically house persistent navigation elements.
If this is a temporary change for testing, it should not be merged. If intentional, please remove the commented code and document why the Navbar belongs in the Hero component rather than at the layout level.
Also applies to: 7-7
I'll verify this review comment by examining the codebase structure and confirming the Navbar's actual location and usage.
🏁 Script executed:
#!/bin/bash
# Check the full content of the layout file to see commented code
cat "apps/web/src/app/(main)/(landing)/layout.tsx"Length of output: 288
🏁 Script executed:
#!/bin/bash
# Search for Navbar usage in Hero component
find . -name "hero*" -type f | grep -i hero | head -20Length of output: 0
🏁 Script executed:
#!/bin/bash
# Search for imports and usage of Navbar in the codebase
rg -n "Navbar" --type=tsx --type=ts -C 2 | head -50Length of output: 27
🏁 Script executed:
#!/bin/bash
# Check if there are other layouts or pages using the landing layout
rg -n "landing.*layout\|useLayout.*landing" --type=tsx --type=ts | head -20Length of output: 27
🏁 Script executed:
#!/bin/bash
# Look for any recent comments or documentation about Navbar changes
rg -n "Navbar.*moved\|Hero.*Navbar\|moved.*Hero" --type=tsx --type=ts -C 2Length of output: 27
I need to correct the ripgrep syntax and search more broadly for the Hero component and Navbar usage.
---
🏁 Script executed:
```shell
#!/bin/bash
# Search for Navbar imports/usages with correct syntax
rg -n "Navbar" -g "*.tsx" -g "*.ts" -C 2 | head -60
Length of output: 3953
🏁 Script executed:
#!/bin/bash
# Find Hero component file(s)
fd -e tsx -e ts -i "hero" Length of output: 49
🏁 Script executed:
#!/bin/bash
# Check landing-sections directory for components
ls -la "apps/web/src/components/landing-sections/" 2>/dev/null || echo "Directory not found"Length of output: 749
🏁 Script executed:
#!/bin/bash
# Search for all imports from navbar component
rg "from.*navbar|import.*[Nn]avbar" -g "*.tsx" -g "*.ts" -C 3Length of output: 1563
🏁 Script executed:
#!/bin/bash
# Check directory structure of components
find "apps/web/src/components" -type f \( -name "*.tsx" -o -name "*.ts" \) | grep -i "hero\|navbar" | head -20Length of output: 101
Remove commented code and clean up unused Navbar import.
The Navbar has been intentionally moved to the Hero component where it's rendered as a fixed/floating element—an appropriate architectural choice for landing-page-only navigation. However, commented code should be removed rather than left in the codebase.
Please remove:
- layout.tsx lines 1 and 7 (commented Navbar import and usage)
- page.tsx line 8 (unused Navbar import)
- page.tsx line 18 (commented Navbar usage)
🤖 Prompt for AI Agents
In apps/web/src/app/(main)/(landing)/layout.tsx lines 1 and 7 and
apps/web/src/app/(main)/(landing)/page.tsx lines 8 and 18, remove the commented
Navbar import and commented usage plus the now-unused Navbar import in page.tsx;
delete the commented line "// import Navbar from
'@/components/landing-sections/navbar'" and the commented JSX usage in
layout.tsx, and remove the unused import statement in page.tsx so the codebase
is free of dead/commented Navbar references (Hero continues to render the
Navbar).
| return ( | ||
| <main className='min-h-screen w-full bg-[#101010] text-white font-sans overflow-hidden relative'> | ||
| <Navbar /> | ||
| {/* <Navbar /> */} |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove commented code.
Either restore the Navbar rendering or remove the commented line. Commented code should not be committed to version control.
- {/* <Navbar /> */}📝 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.
| {/* <Navbar /> */} |
🤖 Prompt for AI Agents
In apps/web/src/app/(main)/(landing)/page.tsx around line 18, there is a
commented-out Navbar JSX: remove the commented code or re-enable it;
specifically either restore the Navbar component call (<Navbar />) if it should
render on the page, or delete the commented line entirely so no commented code
remains in the committed file. Ensure imports remain consistent (remove unused
Navbar import if you delete the line, or keep/add the import if you re-enable
it).
| "use client"; | ||
|
|
||
| import { use, useMemo } from "react"; | ||
| import { newsletters } from "@/data/newsletters"; |
There was a problem hiding this comment.
🧩 Analysis chain
Don’t use React use() for params in this client page; read params synchronously
This page is marked "use client", but it treats params as a Promise and uses the experimental use() API:
- With the current setup (
react/react-dom18.2.0),useis not exported from"react", so this won’t type‑check or run. - For client pages in the app router, Next passes
paramssynchronously; you don’t need (and shouldn’t use)use()here.
You can simplify and fix this by making params synchronous and dropping use entirely:
-"use client";
-
-import { use, useMemo } from "react";
+"use client";
+
+import { useMemo } from "react";
export default function NewsletterDetailPage({
params,
}: {
- params: Promise<{ id: string }>;
+ params: { id: string };
}) {
- const { id } = use(params);
+ const { id } = params;
const { isPaidUser, isLoading } = useSubscription();(Optionally, you can also simplify the date memo to depend on newsletter instead of newsletter?.date, but that’s non‑blocking.)
Everything else in the page—subscription gating, not‑found handling, header visual, and use of NewsletterContent—looks good.
Also applies to: 15-21, 33-40
I'll verify the review comment by checking the React version, investigating use() availability, and inspecting the actual file content.
Now I need to check the actual package.json and file content to verify which Next.js version is being used:
🏁 Script executed:
#!/bin/bash
# Check package.json for React and Next.js versions
cat package.json | grep -E '"(react|next)"'Length of output: 0
🏁 Script executed:
#!/bin/bash
# Inspect the actual file
cat "apps/web/src/app/(main)/dashboard/newsletters/[id]/page.tsx"Length of output: 6876
🏁 Script executed:
#!/bin/bash
# Check for any use of useParams hook
grep -r "useParams" "apps/web/src/app/(main)/dashboard/newsletters/[id]/" 2>/dev/null || echo "No useParams found"Length of output: 18
🏁 Script executed:
#!/bin/bash
# Find all page.tsx files to understand the pattern
find "apps/web/src/app" -name "page.tsx" -type f | head -5Length of output: 223
🏁 Script executed:
#!/bin/bash
# Check another similar page to see how params are handled
find "apps/web/src/app" -name "[id]" -o -name "[*]" -type d | head -3 | xargs -I {} find {} -name "page.tsx" -exec grep -l "use\|params" {} \; 2>/dev/null | head -3Length of output: 0
🏁 Script executed:
#!/bin/bash
# Look at similar client component pages with params
rg -A5 "use client.*params.*Promise" apps/web/src/app --type ts --type tsx -C2 | head -20Length of output: 27
Let me search for the package.json and verify the React/Next.js versions:
🏁 Script executed:
#!/bin/bash
# Look for package.json files
find . -maxdepth 2 -name "package.json" -type fLength of output: 14
🏁 Script executed:
#!/bin/bash
# Check root or apps level for React/Next versions
if [ -f "package.json" ]; then cat package.json | grep -A 50 "dependencies"; fiLength of output: 0
🏁 Script executed:
#!/bin/bash
# Check apps/web package.json
if [ -f "apps/web/package.json" ]; then cat "apps/web/package.json" | grep -A 50 "dependencies"; fiLength of output: 1327
🏁 Script executed:
#!/bin/bash
# Look for other dynamic pages to see if they also use Promise params
rg -n "params.*Promise" apps/web/src/app --type js --type ts -C2 | head -30Length of output: 399
🏁 Script executed:
#!/bin/bash
# Alternatively search for params pattern in pages
rg -n "params:" apps/web/src/app -C1 | grep -A 1 -B 1 "Promise\|params:" | head -40Length of output: 233
🏁 Script executed:
#!/bin/bash
# Check if use() is used elsewhere in the codebase
rg -n "from.*react.*use" apps/web/src --type js --type ts | head -10Length of output: 0
🌐 Web query:
What version of React introduced the `use()` hook?
💡 Result:
The use() API was introduced in React 19. [1][2]
Sources:
[1] React 19 release post (react.dev).
[2] React 19 upgrade guide (react.dev).
🌐 Web query:
In Next.js, do client components receive params as Promise or synchronously?
💡 Result:
Client components receive resolved (synchronous) values — not a Promise. The App Router's page/server component may receive params as a Promise (you await them on the server), but when you pass params down as props to a "use client" component they are serialized and delivered as normal values. [1][2]
Sources:
[1] Next.js App Router examples showing page params typed as Promise and awaited in a Server Component.
[2] Next.js docs on Server/Client Components and passing data (props) from Server → Client.
Remove use() import and treat params as synchronous—this code will crash with React 18.2.0
This page is marked "use client", but it incorrectly treats params as a Promise and uses the use() hook:
The problem:
- Your codebase uses React 18.2.0, but
use()was not introduced until React 19—the import will fail at runtime withTypeError: use is not a function - Next.js 15.5.3 client components receive
paramssynchronously as normal props, not as a Promise; only server components receive Promise-typed params - This page is the only one in the app using this incorrect pattern
Fix:
-import { use, useMemo } from "react";
+import { useMemo } from "react";
export default function NewsletterDetailPage({
params,
}: {
- params: Promise<{ id: string }>;
+ params: { id: string };
}) {
- const { id } = use(params);
+ const { id } = params;Everything else—subscription gating, routing, and layout—is correct.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/web/src/app/(main)/dashboard/newsletters/[id]/page.tsx around lines 1 to
4, the component is a client component but imports and calls React's use() and
treats params as a Promise which will fail on React 18.2.0 and Next.js client
components; remove the use import and any calls to use(), accept params
synchronously from the component props (function signature should read ( {
params } ) ), treat params.id as a plain string, look up the newsletter
synchronously from newsletters (no await), and remove any async/Promise handling
related to params so the page runs as a normal client component.
| // TEMPORARILY DISABLED FOR UI DEVELOPMENT - REMOVE THIS COMMENT AND UNCOMMENT BELOW WHEN DONE | ||
| return NextResponse.next(); |
There was a problem hiding this comment.
CRITICAL: Authentication is completely disabled.
The middleware immediately returns NextResponse.next() without any authentication checks, bypassing all protected route security. This means anyone can access the /dashboard routes without logging in, completely defeating the pro-user newsletter restrictions described in the PR objectives.
This temporary development workaround must be reverted before merging to main.
Apply this diff to restore authentication:
- // TEMPORARILY DISABLED FOR UI DEVELOPMENT - REMOVE THIS COMMENT AND UNCOMMENT BELOW WHEN DONE
- return NextResponse.next();
-
- /* ORIGINAL AUTH CODE - UNCOMMENT WHEN DONE WITH UI WORK
const adaptedReq = {
headers: req.headers,
cookies: req.cookies,
};
const token = await getToken({
req: adaptedReq as any,
secret: process.env.NEXTAUTH_SECRET,
});
const protectedPaths = ["/dashboard"];
if (protectedPaths.some((path) => req.nextUrl.pathname.startsWith(path))) {
if (!token) {
const signInUrl = new URL("/login", req.url);
return NextResponse.redirect(signInUrl);
}
}
return NextResponse.next();
- */Committable suggestion skipped: line range outside the PR's diff.
| 4. **Pro User Protection** | ||
| - Newsletter pages are only accessible to pro users | ||
| - Non-pro users are automatically redirected to pricing page | ||
| - Newsletter link in sidebar only appears for pro users |
There was a problem hiding this comment.
Documentation contradicts middleware implementation.
The documentation states that "Newsletter pages are only accessible to pro users" and "Non-pro users are automatically redirected to pricing page," but the middleware.ts file has authentication completely disabled. This means the documented security feature is not actually functioning.
Ensure the authentication middleware is re-enabled before merging, or update this documentation to reflect the actual implementation.
🤖 Prompt for AI Agents
In NEWSLETTER_IMPLEMENTATION.md around lines 33 to 36, the doc claims
"Newsletter pages are only accessible to pro users" and "Non-pro users are
automatically redirected to pricing page" but the middleware was disabled;
either re-enable the authentication/authorization middleware in middleware.ts so
it checks user subscription status and redirects non-pro users to the pricing
page (and gate the sidebar link rendering behind the same pro-check), or update
these lines to accurately reflect the current behavior; after making the change,
run relevant route/middleware tests and update any frontend conditional that
renders the newsletter sidebar link to match the enforced policy.
1 participant
Recording.2025-11-15.013830.mp4
fix: #155
Added newsletters dashboard.
Focused more towards the UI so the overall product feel would be uplifted.
Implement search and filter functionality.
Added sample newsletters for testing
.
Summary by CodeRabbit
Release Notes
New Features
Style
UI/UX