feat: newsletter page for pro users #155 #158
Conversation
|
@huamanraj 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 premium newsletter system: listing and detail pages, protected API routes with in-memory caching and markdown rendering, UI components for filtering/pagination and skeletons, newsletter styling and sample content, a git submodule for premium content, sidebar pro-section integration, and related tooling/dep updates. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Client as Newsletters<br/>(list/detail)
participant Sub as useSubscription
participant API as /api/newsletters<br/>(server)
participant Cache as In-Memory<br/>Cache
participant FS as FileSystem<br/>(newsletters-premium)
participant Parser as Markdown<br/>Parser
User->>Client: Open newsletters page or article
Client->>Sub: Check subscription status
Sub-->>Client: Loading → resolved
alt Not Authenticated
Client-->>User: Redirect to /login
else Not Premium
Client-->>User: Show PremiumUpgradePrompt
else Premium
Client->>API: GET /api/newsletters or /api/newsletters/{slug}
API->>Cache: Lookup key
alt Cache hit
Cache-->>API: Return cached payload
else Cache miss
API->>FS: Read markdown files / {slug}.md
FS-->>API: File contents
API->>Parser: gray-matter + marked -> HTML + meta
Parser-->>API: Parsed data
API->>Cache: Store result (TTL env-based)
end
API-->>Client: JSON payload (list or article HTML)
Client-->>User: Render list / article (dangerouslySetInnerHTML)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (9)
apps/web/tailwind.config.ts (1)
122-122: Remove the deprecated@tailwindcss/line-clampplugin.The project already uses Tailwind v3.4.1, which includes native
line-clamputilities. The plugin is unnecessary and can be safely removed. Native utilities are already in use in the codebase (e.g.,line-clamp-2atapps/web/src/app/(main)/dashboard/newsletters/page.tsx:150).- plugins: [require("tailwindcss-animate"), require("@tailwindcss/line-clamp")], + plugins: [require("tailwindcss-animate")],Also remove
"@tailwindcss/line-clamp": "^0.4.4"fromapps/web/package.jsondevDependencies.apps/web/src/styles/newsletter.css (1)
1-117: Scoped markdown styles look solid; consider minor tweak to link breakingThe stylesheet is well-scoped under
.newsletter-contentand covers headings, lists, code blocks, images, and tables cleanly—good fit for markdown rendering. As a minor optional tweak, you might revisit the combination ofbreak-allon.newsletter-content awith the more nuanced wrapping rules onp > aandli > ato avoid overly aggressive breaking for shorter inline links.apps/web/src/app/api/newsletters/route.ts (1)
6-54: Listing route logic is correct; consider typing the payload and clarifying error statusThe list endpoint reads markdown, extracts front matter, sorts by date, and caches for 1 minute—this is a clean and efficient implementation for the requirements.
Two non-blocking improvements you might consider:
- Define a
NewsletterMetatype/interface and use it instead ofany[]forcachedNewslettersand the mapped objects to tighten type safety.- In the catch block, returning
NextResponse.json([])with the default 200 can hide server-side failures; returning a{ error }payload with a 5xx status (or at least logging more structured metadata) would make failures easier to detect and distinguish from “no newsletters yet”.apps/web/src/app/(main)/dashboard/newsletters/page.tsx (1)
72-82: Newsletter fetch collapses network/server errors into an empty stateThe fetch logic is straightforward and only runs once the user is confirmed paid, which is good. Right now, though, any failure (non-2xx response, JSON parse issue, network error) just sets
loadingtofalse, which makes the UI show “No newsletters available yet.” This conflates “no content” with “something broke.”Consider:
- Checking
res.okbeforeres.json()and handling non-2xx separately.- Tracking an
errorstate so you can show a distinct “Failed to load newsletters” message rather than the empty state.This is non-blocking but will make debugging and UX clearer.
apps/web/src/app/api/newsletters/[slug]/route.ts (2)
17-27: Slug handler signature works but Promise typing onparamsis unnecessaryAt runtime
paramsis a plain object, soawait paramsjust returns the same object and everything behaves as expected. However, the typeparams: Promise<{ slug: string }>is misleading and doesn’t match Next’s usualcontext: { params: { slug: string } }shape.You can simplify to:
export async function GET( _request: Request, { params }: { params: { slug: string } } ) { const { slug } = params; // ... }This keeps the behavior identical but aligns with the framework’s expectations and avoids confusion.
32-56: Differentiate missing newsletters from server errorsThe core happy-path logic (reading the markdown file, parsing front matter, caching, and returning JSON) looks good. One small concern is that both “file not found” and unexpected I/O/parsing errors return a 404 with
{ error: "Newsletter not found" }.To make debugging and monitoring easier, consider:
- Keeping the 404 for the explicit
!fs.existsSync(filePath)case, and- Returning a 5xx status (e.g., 500) with a different error message from the catch block when something else goes wrong.
This preserves the external API contract for missing slugs while making genuine server failures visible.
apps/web/src/app/(main)/dashboard/newsletters/[slug]/page.tsx (3)
89-106: Data fetch flow is sound; optionally resetloadingon slug changeThe effect correctly:
- Waits until
subscriptionLoadingis false andisPaidUseris true.- Fetches the newsletter for the current
slug.- Handles both not-found and generic errors.
Given this is a dynamic
[slug]route, if Next ever reuses the same component instance across slug changes, you might want to setsetLoading(true)at the top of the effect whenslugchanges so the skeleton appears between navigations. Today, in most app-router navigations the page remounts, so this is more of an optional UX polish than a bug.
150-159: Back navigation label suggests list but usesrouter.back()The “Back to newsletters” button uses
router.back(), which depends on browser history. If users land directly on a newsletter (e.g., deep link), “back” might not actually take them to/dashboard/newsletters, despite the label.To make behavior match the label, consider replacing
router.back()withrouter.push("/dashboard/newsletters").
161-187: Sanitize HTML before usingdangerouslySetInnerHTMLRendering the newsletter body with
dangerouslySetInnerHTMLis expected when consuming HTML from the/api/newsletters/[slug]endpoint, but it bypasses React’s XSS protections. Even though the markdown currently comes from trusted repository files, this path would become risky if content ever originates from user input or external sources.Consider one of these patterns:
- Sanitize on the server in the API route (e.g., run the
marked.parseoutput through a sanitizer likesanitize-htmlbefore returning it), or- Sanitize on the client (e.g., with DOMPurify) before passing it into
dangerouslySetInnerHTML.That keeps your markdown flexibility while aligning with the static analysis warning and hardening against future changes in the content pipeline.
📜 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 (13)
apps/web/package.json(3 hunks)apps/web/src/app/(main)/dashboard/newsletters/[slug]/page.tsx(1 hunks)apps/web/src/app/(main)/dashboard/newsletters/page.tsx(1 hunks)apps/web/src/app/api/newsletters/[slug]/route.ts(1 hunks)apps/web/src/app/api/newsletters/route.ts(1 hunks)apps/web/src/app/globals.css(1 hunks)apps/web/src/components/dashboard/Sidebar.tsx(2 hunks)apps/web/src/components/ui/skeleton.tsx(1 hunks)apps/web/src/content/newsletters/2024-01-welcome.md(1 hunks)apps/web/src/content/newsletters/2024-02-new-features.md(1 hunks)apps/web/src/content/newsletters/2024-03-success-stories.md(1 hunks)apps/web/src/styles/newsletter.css(1 hunks)apps/web/tailwind.config.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
apps/web/src/app/(main)/dashboard/newsletters/page.tsx (1)
apps/web/src/hooks/useSubscription.ts (1)
useSubscription(11-75)
apps/web/src/app/(main)/dashboard/newsletters/[slug]/page.tsx (1)
apps/web/src/hooks/useSubscription.ts (1)
useSubscription(11-75)
apps/web/src/app/api/newsletters/[slug]/route.ts (1)
apps/web/src/app/api/newsletters/route.ts (1)
GET(11-55)
apps/web/src/components/ui/skeleton.tsx (1)
apps/web/src/lib/utils.ts (1)
cn(4-6)
apps/web/src/components/dashboard/Sidebar.tsx (2)
apps/web/src/components/sidebar/SidebarItem.tsx (1)
SidebarItem(12-28)apps/web/src/components/ui/badge.tsx (1)
Badge(36-36)
apps/web/src/app/api/newsletters/route.ts (1)
apps/web/src/app/api/newsletters/[slug]/route.ts (1)
GET(17-57)
🪛 ast-grep (0.39.9)
apps/web/src/app/(main)/dashboard/newsletters/[slug]/page.tsx
[warning] 185-185: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🪛 Biome (2.1.2)
apps/web/src/app/(main)/dashboard/newsletters/[slug]/page.tsx
[error] 186-186: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
🪛 LanguageTool
apps/web/src/content/newsletters/2024-03-success-stories.md
[uncategorized] ~38-~38: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...elationships. ## Upcoming Events 📅 Open Source Workshop - March 20th Learn best pr...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[style] ~56-~56: Using many exclamation marks might seem excessive (in this case: 3 exclamation marks for a text that’s 1444 characters long)
Context: ... --- Keep contributing, keep growing! [Share your story](mailto:hi@opensox...
(EN_EXCESSIVE_EXCLAMATION)
apps/web/src/content/newsletters/2024-02-new-features.md
[style] ~36-~36: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...bold text**. this is italic text. this is bold and nested italic. this...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~37-~37: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...this is bold and nested italic. this is a [link to opensox](https://opensox....
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[grammar] ~149-~149: Use a hyphen to join words.
Context: ...tros, community messages, and deep write ups. verifying its behavior now will sav...
(QB_NEW_EN_HYPHEN)
[grammar] ~149-~149: Use a hyphen to join words.
Context: ...er in production environments where user generated content appears. --- ## link...
(QB_NEW_EN_HYPHEN)
apps/web/src/content/newsletters/2024-01-welcome.md
[style] ~30-~30: Using many exclamation marks might seem excessive (in this case: 3 exclamation marks for a text that’s 704 characters long)
Context: ...hare your journey. Happy Contributing! The Opensox Team
(EN_EXCESSIVE_EXCLAMATION)
🪛 markdownlint-cli2 (0.18.1)
apps/web/src/content/newsletters/2024-02-new-features.md
164-164: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🔇 Additional comments (7)
apps/web/src/components/dashboard/Sidebar.tsx (1)
137-156: LGTM! Clean implementation of the Newsletters navigation.The Newsletters navigation entry is well-integrated with the existing sidebar structure. The PRO badge positioning using absolute positioning with translate centering is appropriate, and the conditional rendering based on sidebar collapse state ensures a clean UI.
apps/web/src/components/ui/skeleton.tsx (1)
1-15: LGTM! Well-implemented Skeleton component.The Skeleton component follows best practices with proper prop spreading, className composition via
cn, and semantic use of Tailwind utilities for the pulse animation.apps/web/src/content/newsletters/2024-01-welcome.md (1)
1-31: LGTM! Well-structured welcome newsletter content.The newsletter content is well-organized with proper front matter, clear sections, and appropriate markdown formatting. The content effectively introduces Opensox AI and provides clear next steps for users.
apps/web/src/content/newsletters/2024-02-new-features.md (1)
1-165: LGTM! Comprehensive formatting test file.This test newsletter effectively covers a wide range of markdown features and edge cases (headings, lists, code blocks, tables, inline HTML, special characters, long paragraphs). The content serves its stated purpose as a "stress test" for the newsletter rendering system.
apps/web/src/content/newsletters/2024-03-success-stories.md (1)
1-58: LGTM! Engaging success stories newsletter.The newsletter is well-structured with compelling content, including featured stories, statistics, actionable tips, and upcoming events. The front matter format is consistent with other newsletters, and the content effectively promotes community engagement.
apps/web/package.json (1)
18-18: All package versions exist and resolve correctly; verify security advisories locally with npm audit.Version validation results:
@radix-ui/react-slot@^1.2.3✓ (resolves to 1.2.3–1.2.4)gray-matter@^4.0.3✓ (resolves to 4.0.3)marked@^17.0.0✓ (resolves to 17.0.0)@tailwindcss/line-clamp@^0.4.3✓ (resolves to 0.4.3–0.4.4)Security audit could not be completed in the sandbox environment due to missing lockfile. Run
npm auditlocally in your repository to verify no known vulnerabilities exist before merging.apps/web/src/app/(main)/dashboard/newsletters/page.tsx (1)
1-4: The CSS import is compatible with your Next.js setup and does not require changes.Your project uses Next.js 15.5.3 with the App Router, which explicitly allows global CSS imports from any component inside the
app/tree, including client components ("use client"). Global styles can be imported from any layout, page, or component inside app/ (including colocated client components). This pattern is already established in your rootapp/layout.tsx, which directly importsglobals.css. The import in the newsletters page is valid and consistent with your framework configuration.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (7)
apps/web/src/components/newsletters/PremiumUpgradePrompt.tsx (1)
23-25: Consider slightly larger text for better readability.The description uses
text-xswhich may be too small for comfortable reading, especially for longer content. Considertext-smfor improved readability.- <p className="text-xs text-zinc-400 mb-6 leading-relaxed"> + <p className="text-sm text-zinc-400 mb-6 leading-relaxed"> Stay ahead in the open source world. Get curated insights on jobs, funding news, trending projects, upcoming trends, and expert tips. </p>apps/web/src/content/newsletters/2024-01-welcome.md (1)
1-6: Consider aligning slug year with front‑matter dateThe file name suggests January 2024, while
dateis2025-01-15. If you’re using filenames for archives/URLs and front matter for display/sorting, this mismatch can be confusing. Consider updating either the slug or thedateso they reflect the same month/year, for consistency across newsletters.apps/web/src/content/newsletters/2024-02-new-features.md (1)
1-165: Verify markdown renderer handles all “stress-test” constructsThis file is great as a rendering stress test (nested lists, tables, multiple fenced code blocks, inline HTML, long paragraphs, odd characters). Two things to double‑check in your renderer configuration:
- That raw HTML (
<img>,<div style=...>) is allowed and not escaped or stripped, if you intend to rely on it.- That mismatched fence lengths (```ts … ````) are accepted by your markdown library and don’t break subsequent parsing.
If either of these isn’t guaranteed, you may want to normalize fences and/or avoid inline HTML here.
apps/web/src/components/ui/input.tsx (1)
1-22: Optionally defaulttypeto"text"for clarityThe component works fine as-is since browsers default missing
typeto"text", but you could make that intent explicit:-const Input = React.forwardRef<HTMLInputElement, React.ComponentProps<"input">>( - ({ className, type, ...props }, ref) => { +const Input = React.forwardRef<HTMLInputElement, React.ComponentProps<"input">>( + ({ className, type = "text", ...props }, ref) => {This avoids ever rendering an input with an undefined
typeprop and documents the default behavior.apps/web/src/components/newsletters/NewsletterFilters.tsx (1)
19-24: Consider moving the filters array outside the component.The
filtersarray is recreated on every render. Since it's static, moving it outside the component function would be a minor performance optimization.Apply this diff:
+const filters: { value: TimeFilter; label: string }[] = [ + { value: "all", label: "All Time" }, + { value: "day", label: "Today" }, + { value: "week", label: "This Week" }, + { value: "month", label: "This Month" }, +]; + export function NewsletterFilters({ searchQuery, onSearchChange, timeFilter, onTimeFilterChange, }: NewsletterFiltersProps) { - const filters: { value: TimeFilter; label: string }[] = [ - { value: "all", label: "All Time" }, - { value: "day", label: "Today" }, - { value: "week", label: "This Week" }, - { value: "month", label: "This Month" }, - ];apps/web/src/app/(main)/dashboard/newsletters/[slug]/page.tsx (2)
18-39: Consider reusing the existing NewsletterSkeleton component.A local
NewsletterSkeletoncomponent is defined here, but there's already aNewsletterSkeletoncomponent atapps/web/src/components/newsletters/NewsletterSkeleton.tsx. If the skeleton layouts need to differ between list and detail views, consider renaming one to clarify intent (e.g.,NewsletterDetailSkeleton). Otherwise, reuse the existing component to reduce duplication.
49-51: Clarify or remove the "testing mode" comment.The comment suggests subscription gating is disabled for testing, but the code still checks
subscriptionLoading. If this comment is outdated or the subscription check will be fully enabled later, please clarify or remove the comment to avoid confusion.
📜 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)
apps/web/package.json(2 hunks)apps/web/src/app/(main)/dashboard/newsletters/[slug]/page.tsx(1 hunks)apps/web/src/app/(main)/dashboard/newsletters/page.tsx(1 hunks)apps/web/src/app/globals.css(1 hunks)apps/web/src/components/newsletters/NewsletterCard.tsx(1 hunks)apps/web/src/components/newsletters/NewsletterFilters.tsx(1 hunks)apps/web/src/components/newsletters/NewsletterList.tsx(1 hunks)apps/web/src/components/newsletters/NewsletterPagination.tsx(1 hunks)apps/web/src/components/newsletters/NewsletterSkeleton.tsx(1 hunks)apps/web/src/components/newsletters/PremiumUpgradePrompt.tsx(1 hunks)apps/web/src/components/ui/input-group.tsx(1 hunks)apps/web/src/components/ui/input.tsx(1 hunks)apps/web/src/components/ui/pagination.tsx(1 hunks)apps/web/src/components/ui/textarea.tsx(1 hunks)apps/web/src/content/newsletters/2024-01-welcome.md(1 hunks)apps/web/src/content/newsletters/2024-02-new-features.md(1 hunks)apps/web/src/content/newsletters/2024-03-success-stories.md(1 hunks)apps/web/src/hooks/useNewsletterFilters.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/web/src/app/(main)/dashboard/newsletters/page.tsx
- apps/web/package.json
🧰 Additional context used
🧬 Code graph analysis (11)
apps/web/src/components/newsletters/NewsletterSkeleton.tsx (2)
apps/web/src/components/ui/skeleton.tsx (1)
Skeleton(15-15)apps/web/src/components/ui/Client-tweet.tsx (2)
props(63-83)props(54-61)
apps/web/src/components/newsletters/PremiumUpgradePrompt.tsx (1)
apps/web/src/components/checkout/CheckoutWrapper.tsx (1)
CheckoutWrapper(8-33)
apps/web/src/hooks/useNewsletterFilters.ts (2)
apps/web/src/components/newsletters/NewsletterCard.tsx (1)
Newsletter(4-10)apps/web/src/components/newsletters/NewsletterFilters.tsx (1)
TimeFilter(4-4)
apps/web/src/components/ui/pagination.tsx (2)
apps/web/src/lib/utils.ts (1)
cn(4-6)apps/web/src/components/ui/button.tsx (2)
ButtonProps(37-41)buttonVariants(57-57)
apps/web/src/app/(main)/dashboard/newsletters/[slug]/page.tsx (2)
apps/web/src/components/newsletters/NewsletterSkeleton.tsx (1)
NewsletterSkeleton(3-15)apps/web/src/hooks/useSubscription.ts (1)
useSubscription(11-75)
apps/web/src/components/newsletters/NewsletterFilters.tsx (1)
apps/web/src/components/ui/input-group.tsx (3)
InputGroup(164-164)InputGroupAddon(165-165)InputGroupInput(168-168)
apps/web/src/components/ui/input-group.tsx (1)
apps/web/src/lib/utils.ts (1)
cn(4-6)
apps/web/src/components/newsletters/NewsletterList.tsx (2)
apps/web/src/components/newsletters/NewsletterCard.tsx (2)
Newsletter(4-10)NewsletterCard(16-48)apps/web/src/components/newsletters/NewsletterSkeleton.tsx (1)
NewsletterSkeleton(3-15)
apps/web/src/components/ui/input.tsx (1)
apps/web/src/lib/utils.ts (1)
cn(4-6)
apps/web/src/components/ui/textarea.tsx (1)
apps/web/src/lib/utils.ts (1)
cn(4-6)
apps/web/src/components/newsletters/NewsletterPagination.tsx (1)
apps/web/src/components/ui/pagination.tsx (7)
Pagination(110-110)PaginationContent(111-111)PaginationItem(113-113)PaginationPrevious(114-114)PaginationLink(112-112)PaginationEllipsis(116-116)PaginationNext(115-115)
🪛 ast-grep (0.39.9)
apps/web/src/app/(main)/dashboard/newsletters/[slug]/page.tsx
[warning] 142-142: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🪛 Biome (2.1.2)
apps/web/src/hooks/useNewsletterFilters.ts
[error] 35-35: 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)
[error] 39-39: 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)
apps/web/src/app/(main)/dashboard/newsletters/[slug]/page.tsx
[error] 143-143: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
🪛 LanguageTool
apps/web/src/content/newsletters/2024-02-new-features.md
[style] ~36-~36: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...bold text**. this is italic text. this is bold and nested italic. this...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~37-~37: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...this is bold and nested italic. this is a [link to opensox](https://opensox....
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[grammar] ~149-~149: Use a hyphen to join words.
Context: ...tros, community messages, and deep write ups. verifying its behavior now will sav...
(QB_NEW_EN_HYPHEN)
[grammar] ~149-~149: Use a hyphen to join words.
Context: ...er in production environments where user generated content appears. --- ## link...
(QB_NEW_EN_HYPHEN)
apps/web/src/content/newsletters/2024-01-welcome.md
[style] ~30-~30: Using many exclamation marks might seem excessive (in this case: 3 exclamation marks for a text that’s 704 characters long)
Context: ...hare your journey. Happy Contributing! The Opensox Team
(EN_EXCESSIVE_EXCLAMATION)
apps/web/src/content/newsletters/2024-03-success-stories.md
[uncategorized] ~38-~38: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...elationships. ## Upcoming Events 📅 Open Source Workshop - March 20th Learn best pr...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[style] ~56-~56: Using many exclamation marks might seem excessive (in this case: 3 exclamation marks for a text that’s 1444 characters long)
Context: ... --- Keep contributing, keep growing! [Share your story](mailto:hi@opensox...
(EN_EXCESSIVE_EXCLAMATION)
🪛 markdownlint-cli2 (0.18.1)
apps/web/src/content/newsletters/2024-02-new-features.md
164-164: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🔇 Additional comments (9)
apps/web/src/app/globals.css (1)
72-75: ✅ CSS syntax error resolved.The previous critical issue with the unmatched closing brace has been corrected. The braces are now properly matched: the
@layer base { ... }block closes at line 70, and thehtml { scroll-behavior: smooth; }rule is correctly positioned outside the layer with its own closing brace at line 74. The trailing newline is good practice. CSS should now compile without errors.apps/web/src/components/newsletters/PremiumUpgradePrompt.tsx (1)
1-9: LGTM! Correct Next.js App Router setup.The imports and component initialization follow the correct Next.js 15 patterns with the "use client" directive and appropriate use of
next/navigation.apps/web/src/components/ui/textarea.tsx (1)
1-22: Textarea implementation looks solidForwardRef wiring, prop typing, and class merging all look correct and consistent with the rest of the UI primitives.
apps/web/src/content/newsletters/2024-03-success-stories.md (1)
1-58: Newsletter content is structurally soundFront matter matches the expected shape and the markdown uses only standard constructs your renderer should handle well (headings, lists, quote, links). No structural issues from a code/processing standpoint.
apps/web/src/components/newsletters/NewsletterSkeleton.tsx (1)
1-15: LGTM!The skeleton loading component is well-structured and provides appropriate visual placeholders for newsletter content during loading states.
apps/web/src/components/newsletters/NewsletterCard.tsx (1)
1-48: LGTM!The NewsletterCard component is well-structured with proper type definitions, responsive design, and good accessibility through semantic HTML. The hover states and icon usage enhance the user experience.
apps/web/src/components/newsletters/NewsletterPagination.tsx (1)
1-89: LGTM!The pagination component is well-implemented with proper edge case handling, ellipsis support for large page counts, and appropriate disabled states at boundaries. The logic for calculating visible page numbers is correct and maintainable.
apps/web/src/components/newsletters/NewsletterList.tsx (1)
1-40: LGTM!The component provides a clean separation of loading, empty, and success states with appropriate messaging for users. The use of
newsletter.slugas the key is correct for list rendering.apps/web/src/components/ui/pagination.tsx (1)
1-117: LGTM!Excellent implementation of accessible pagination UI primitives. The components properly use
forwardReffor ref forwarding, include appropriate ARIA attributes for accessibility (aria-label, aria-current, aria-hidden, sr-only), and set displayName for better debugging. The composition pattern allows flexible reuse across the application.
|
@apsinghdev updated with new ui design! excited for the results! |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/components/dashboard/Sidebar.tsx (1)
25-25: Fix lucide-react Chevron icon import—non-standard suffixed names will cause compilation errorYour codebase does not use lucide-react's suffixed export bundle or custom module aliases. In the default configuration, lucide-react 0.456.0 exports unsuffixed icon names only (
ChevronLeft,ChevronRight), not the suffixed variants (ChevronLeftIcon,ChevronRightIcon). The current import and usage will fail at compile/runtime.Update line 25 and all usages:
-import { ChevronLeftIcon, ChevronRightIcon } from 'lucide-react'; +import { ChevronLeft, ChevronRight } from 'lucide-react';Then replace component names at lines 114, 120, and 230.
♻️ Duplicate comments (1)
apps/web/src/app/(main)/dashboard/newsletters/[slug]/page.tsx (1)
193-208: dangerouslySetInnerHTML still requires HTML sanitization (XSS risk)
newsletter.contentis injected viadangerouslySetInnerHTMLwithout any visible sanitization in this file, and past review notes indicate the API route usesmarked.parse()without sanitizing the output. This leaves you open to XSS if any newsletter markdown contains embedded HTML/JS (even from internal contributors).Recommended:
- Sanitize the HTML on the server in
apps/web/src/app/api/newsletters/[slug]/route.tsright after markdown parsing (e.g., withDOMPurifyin a JSDOM context orsanitize-html), and only then sendcontentto the client.- Keep
dangerouslySetInnerHTMLonly for that already-sanitized HTML.Because this was already raised and appears unresolved, consider this a blocking security fix before shipping.
🧹 Nitpick comments (6)
apps/web/src/components/dashboard/Sidebar.tsx (1)
27-49: Inline route rendering looks good; consider preserving active-route stylingThe new
SIDEBAR_ROUTESentry for/dashboard/newsletterswithisPro: trueand the inline rendering withOpensoxProBadgelook consistent and clear.If the previous
SidebarItemabstraction handled active-route highlighting or accessibility roles, consider reintroducing an active-state indicator in this custom block (e.g., comparingroute.pathto the current pathname and adjusting classes) so navigation remains consistent with other parts of the app.Also applies to: 119-136
apps/web/src/components/newsletters/NewsletterCard.tsx (1)
3-38: Solid card implementation; optional semantic tweaksThe
NewsletterCardandNewslettertype are clean and idiomatic; layout and responsive behavior look good.If you want minor semantic improvements later, you could:
- Wrap the card content in an
<article>for better document structure.- Render the date via
<time dateTime={newsletter.date}>for machine-readability.apps/web/src/hooks/useNewsletterFilters.ts (1)
5-34: Filtering logic matches requirements; watch date parsing robustnessThe hook cleanly handles search across title/description/excerpt and month-based filtering, with appropriate memoization dependencies.
Given
new Date(newsletter.date)is used, just ensure the API/frontmatter always provides ISO-like dates (e.g.,YYYY-MM-DD) so parsing is reliable across environments; otherwise you might want to normalize or validate the date before deriving the month.apps/web/src/components/newsletters/NewsletterList.tsx (1)
10-36: List state handling is good; simplify key usageLoading, empty, and populated states are well handled and messages are clear.
Since
Newsletter.idis a required string in the shared interface, you can simplify the key to avoid index fallback:- {newsletters.map((newsletter, index) => ( - <NewsletterCard key={newsletter.id || `newsletter-${index}`} newsletter={newsletter} /> - ))} + {newsletters.map((newsletter) => ( + <NewsletterCard key={newsletter.id} newsletter={newsletter} /> + ))}apps/web/src/app/(main)/dashboard/newsletters/page.tsx (1)
16-37: Nice filtering/pagination flow; consider skipping fetch for non-Pro usersThe state setup, use of
useNewsletterFilters, sorting viauseMemo, and page-based slicing all look solid and match the UX requirements.Since non-paid users are immediately short-circuited to
<PremiumUpgradePrompt />, you could optionally avoid fetching newsletter data when!isPaidUserto save the extra request:- useEffect(() => { - if (subscriptionLoading) return; - - fetch("/api/newsletters") + useEffect(() => { + if (subscriptionLoading || !isPaidUser) return; + + fetch("/api/newsletters") .then((res) => res.json()) .then((data) => { setNewsletters(data); setLoading(false); }) .catch(() => setLoading(false)); - }, [subscriptionLoading]); + }, [subscriptionLoading, isPaidUser]);Not required, but it tightens the paywall semantics and avoids unnecessary work for free users.
Also applies to: 39-56, 63-112
apps/web/src/components/newsletters/NewsletterFilters.tsx (1)
3-67: Filter UI is clean; consider small accessibility enhancementThe filter component and types are well-structured, and wiring to the parent callbacks is clear.
For a small accessibility boost later, you might add
aria-labelattributes (or visible<label>elements) for the month and sort<select>elements so screen readers have explicit descriptions beyond the option text.
📜 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 (11)
apps/web/package.json(2 hunks)apps/web/src/app/(main)/dashboard/newsletters/[slug]/page.tsx(1 hunks)apps/web/src/app/(main)/dashboard/newsletters/page.tsx(1 hunks)apps/web/src/app/api/newsletters/route.ts(1 hunks)apps/web/src/components/dashboard/Sidebar.tsx(3 hunks)apps/web/src/components/newsletters/NewsletterCard.tsx(1 hunks)apps/web/src/components/newsletters/NewsletterFilters.tsx(1 hunks)apps/web/src/components/newsletters/NewsletterList.tsx(1 hunks)apps/web/src/components/newsletters/NewsletterSkeleton.tsx(1 hunks)apps/web/src/hooks/useNewsletterFilters.ts(1 hunks)apps/web/src/styles/newsletter.css(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/web/src/components/newsletters/NewsletterSkeleton.tsx
- apps/web/src/styles/newsletter.css
- apps/web/src/app/api/newsletters/route.ts
🧰 Additional context used
🧬 Code graph analysis (5)
apps/web/src/app/(main)/dashboard/newsletters/page.tsx (8)
apps/web/src/components/newsletters/NewsletterCard.tsx (1)
Newsletter(3-11)apps/web/src/components/newsletters/NewsletterFilters.tsx (3)
TimeFilter(3-3)SortFilter(4-4)NewsletterFilters(15-68)apps/web/src/hooks/useSubscription.ts (1)
useSubscription(11-77)apps/web/src/hooks/useNewsletterFilters.ts (1)
useNewsletterFilters(5-35)apps/web/src/components/newsletters/NewsletterSkeleton.tsx (1)
NewsletterSkeleton(3-19)apps/web/src/components/newsletters/PremiumUpgradePrompt.tsx (1)
PremiumUpgradePrompt(7-37)apps/web/src/components/newsletters/NewsletterList.tsx (1)
NewsletterList(10-38)apps/web/src/components/newsletters/NewsletterPagination.tsx (1)
NewsletterPagination(17-89)
apps/web/src/components/newsletters/NewsletterList.tsx (2)
apps/web/src/components/newsletters/NewsletterCard.tsx (2)
Newsletter(3-11)NewsletterCard(13-39)apps/web/src/components/newsletters/NewsletterSkeleton.tsx (1)
NewsletterSkeleton(3-19)
apps/web/src/hooks/useNewsletterFilters.ts (2)
apps/web/src/components/newsletters/NewsletterCard.tsx (1)
Newsletter(3-11)apps/web/src/components/newsletters/NewsletterFilters.tsx (1)
TimeFilter(3-3)
apps/web/src/components/dashboard/Sidebar.tsx (1)
apps/web/src/components/sheet/OpensoxProBadge.tsx (1)
OpensoxProBadge(8-28)
apps/web/src/app/(main)/dashboard/newsletters/[slug]/page.tsx (3)
apps/web/src/components/newsletters/NewsletterSkeleton.tsx (1)
NewsletterSkeleton(3-19)apps/web/src/hooks/useSubscription.ts (1)
useSubscription(11-77)apps/web/src/components/newsletters/PremiumUpgradePrompt.tsx (1)
PremiumUpgradePrompt(7-37)
🪛 ast-grep (0.39.9)
apps/web/src/app/(main)/dashboard/newsletters/[slug]/page.tsx
[warning] 207-207: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🪛 Biome (2.1.2)
apps/web/src/app/(main)/dashboard/newsletters/[slug]/page.tsx
[error] 208-208: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
🔇 Additional comments (3)
apps/web/package.json (3)
32-32: marked v17 is safe to add with no configuration needed.marked v17 does not require any Next.js-specific configuration or special setup, and the codebase has Next.js 15.5.3 and React 18.2.0 which are compatible. No usage of marked is currently present in the codebase, so no breaking changes apply here.
30-30: No action required—gray-matter includes built-in TypeScript support.Gray-matter provides TypeScript definitions through its own
gray-matter.d.tsfile, so @types/gray-matter is not needed. The npm registry query confirms@types/gray-matterdoesn't exist as a separate package. The current usage pattern in your newsletter routes (import matter from "gray-matter") is correct and properly typed.
18-18: All dependency updates and additions are compatible—no issues found.Verification confirms:
- @radix-ui/react-slot v1.2.3 is backward compatible with no API-breaking changes, and its usage in
apps/web/src/components/ui/button.tsx(importingSlot) is compatible.- gray-matter includes bundled TypeScript declarations, so no additional
@typespackage is required; usage inapps/web/src/app/api/newsletters/[slug]/route.tsis standard.- marked v17.0.0 ships its own TypeScript types (making @types/marked deprecated); usage in the same file uses only basic
setOptions()andparse()calls, unaffected by list/token renderer breaking changes.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/web/src/app/(main)/dashboard/newsletters/[slug]/page.tsx (1)
215-231: Critical: XSS vulnerability - HTML must be sanitized before rendering.The newsletter content is rendered with
dangerouslySetInnerHTMLwithout any sanitization. The API route atapps/web/src/app/api/newsletters/[slug]/route.ts(line 72) usesmarked.parse()which does not sanitize HTML by default. Malicious JavaScript embedded in markdown content will execute in users' browsers.Required fix: Add server-side HTML sanitization in the API route using
DOMPurify(viaisomorphic-dompurify) orsanitize-htmlimmediately aftermarked.parse()and before returning the content.Apply this diff to the API route (
apps/web/src/app/api/newsletters/[slug]/route.ts):+import DOMPurify from 'isomorphic-dompurify'; // Render markdown (supports headings, links, lists, code blocks, images, tables, etc.) const htmlContent = marked.parse(content); +const sanitizedContent = DOMPurify.sanitize(htmlContent); const result = { title: data.title || "Untitled", date: data.date || new Date().toISOString(), readTime: data.readTime || "5 min read", - content: htmlContent, + content: sanitizedContent, };Install the dependency:
npm install isomorphic-dompurify
🧹 Nitpick comments (1)
apps/web/src/app/api/newsletters/route.ts (1)
9-12: Module-level cache limitations in serverless environments.The in-memory cache using module-level variables won't persist across serverless function invocations or scale horizontally. Each container/instance maintains its own cache, leading to:
- Cache misses on cold starts
- Inconsistent cache state across instances
- No cache sharing in multi-instance deployments
For production, consider Redis, Vercel KV, or Next.js's built-in caching mechanisms for consistent cache behavior.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/web/src/app/(main)/dashboard/newsletters/[slug]/page.tsx(1 hunks)apps/web/src/app/(main)/dashboard/newsletters/page.tsx(1 hunks)apps/web/src/app/api/newsletters/[slug]/route.ts(1 hunks)apps/web/src/app/api/newsletters/route.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
apps/web/src/app/api/newsletters/route.ts (3)
apps/web/src/app/api/newsletters/[slug]/route.ts (1)
GET(20-88)apps/web/src/lib/auth/config.ts (2)
session(43-49)authConfig(6-69)apps/web/src/lib/trpc-server.ts (1)
serverTrpc(8-18)
apps/web/src/app/(main)/dashboard/newsletters/[slug]/page.tsx (3)
apps/web/src/components/newsletters/NewsletterSkeleton.tsx (1)
NewsletterSkeleton(3-19)apps/web/src/hooks/useSubscription.ts (1)
useSubscription(11-77)apps/web/src/components/newsletters/PremiumUpgradePrompt.tsx (1)
PremiumUpgradePrompt(7-37)
apps/web/src/app/(main)/dashboard/newsletters/page.tsx (8)
apps/web/src/components/newsletters/NewsletterCard.tsx (1)
Newsletter(3-11)apps/web/src/components/newsletters/NewsletterFilters.tsx (3)
TimeFilter(3-3)SortFilter(4-4)NewsletterFilters(15-68)apps/web/src/hooks/useSubscription.ts (1)
useSubscription(11-77)apps/web/src/hooks/useNewsletterFilters.ts (1)
useNewsletterFilters(5-35)apps/web/src/components/newsletters/NewsletterSkeleton.tsx (1)
NewsletterSkeleton(3-19)apps/web/src/components/newsletters/PremiumUpgradePrompt.tsx (1)
PremiumUpgradePrompt(7-37)apps/web/src/components/newsletters/NewsletterList.tsx (1)
NewsletterList(10-38)apps/web/src/components/newsletters/NewsletterPagination.tsx (1)
NewsletterPagination(17-89)
apps/web/src/app/api/newsletters/[slug]/route.ts (3)
apps/web/src/app/api/newsletters/route.ts (1)
GET(14-87)apps/web/src/lib/auth/config.ts (2)
session(43-49)authConfig(6-69)apps/web/src/lib/trpc-server.ts (1)
serverTrpc(8-18)
🪛 ast-grep (0.39.9)
apps/web/src/app/(main)/dashboard/newsletters/[slug]/page.tsx
[warning] 229-229: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🪛 Biome (2.1.2)
apps/web/src/app/(main)/dashboard/newsletters/[slug]/page.tsx
[error] 230-230: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
🔇 Additional comments (4)
apps/web/src/app/api/newsletters/route.ts (1)
14-41: Authentication and subscription checks properly implemented.Good implementation of the security requirements. The route now correctly:
- Authenticates users via NextAuth session
- Verifies premium subscription status via TRPC
- Returns appropriate HTTP status codes (401, 403, 500)
This addresses the critical security concern from the previous review about missing access controls.
apps/web/src/app/(main)/dashboard/newsletters/page.tsx (1)
28-56: LGTM - API security concerns from previous review have been addressed.The client-side error handling correctly processes 401/403 responses from the API. The corresponding API routes now implement proper authentication and subscription checks, so the security concerns raised in the previous review have been resolved.
apps/web/src/app/(main)/dashboard/newsletters/[slug]/page.tsx (1)
93-126: API security properly enforced server-side.The fetch logic correctly handles authentication (401) and authorization (403) responses. The underlying API route now implements proper session and subscription checks, resolving the security concerns from the previous review.
apps/web/src/app/api/newsletters/[slug]/route.ts (1)
20-50: Authentication and subscription verification correctly implemented.Excellent implementation of access controls:
- Session-based authentication via NextAuth
- Premium subscription verification via TRPC
- Proper error responses (401, 403, 500)
This resolves the critical security gaps flagged in the previous review.
|
@huamanraj nice work! can you pls rebase this branch on top of latest changes? |
huamanraj
force-pushed
the
feat_newsletter
branch
from
15e1064 to
22cf36d
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
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/components/dashboard/Sidebar.tsx (2)
31-53: Avoid duplicate “Newsletters” item and nested Link usage for Pro badgeRight now:
SIDEBAR_ROUTESdefines a/dashboard/newslettersitem (Lines 47-52).- The sidebar body also renders a separate explicit Newsletters
<Link>further down (Lines 154-173).This will show two Newsletters entries in the sidebar. In addition, within the
SIDEBAR_ROUTES.map(...)block you renderroute.isPro && <OpensoxProBadge ... />, butOpensoxProBadgeitself wraps its contents in aLinkto/pricing, so you end up with a<Link>to the route containing another<Link>to/pricing, which is invalid HTML and can break click behavior and accessibility.A straightforward fix is to keep the explicit Newsletters block (which already has a PRO badge) and make the mapped routes plain links with no nested
OpensoxProBadge:const SIDEBAR_ROUTES = [ { path: "/dashboard/home", label: "Home", icon: <HomeIcon className="size-5" />, }, { path: "/dashboard/projects", label: "OSS Projects", icon: <FolderIcon className="size-5" />, }, { path: "/dashboard/sheet", label: "OSS Sheet", icon: <DocumentTextIcon className="size-5" />, }, - { - path: "/dashboard/newsletters", - label: "Newsletters", - icon: <NewspaperIcon className="size-5" />, - isPro: true, - }, ]; @@ <div className="flex items-center gap-1"> <h1 className="text-xs font-medium text-[#c8c8c8] group-hover:text-white transition-colors"> {route.label} </h1> - {route.isPro && ( - <OpensoxProBadge className="px-1.5 py-0.5 scale-75" /> - )} </div>This removes the duplicate Newsletters entry and eliminates the nested link issue. If you still want a generic “Pro” indicator for mapped routes later, consider a non-link badge (e.g. using the
Badgecomponent) instead ofOpensoxProBadgeinside these links.Also applies to: 124-143
9-21: Fix undefinedhandleFindProjectsand missingMagnifyingGlassIconimportBoth issues identified in the review comment are confirmed:
handleFindProjectsis referenced on line 149 but never defined anywhere in Sidebar.tsxMagnifyingGlassIconis used on line 150 but not imported from@heroicons/react/24/outlineBoth will cause compile-time errors. The suggested fix is valid: add the missing icon import and inline the navigation handler using the
routerinstance.import { XMarkIcon, HomeIcon, FolderIcon, ArrowRightOnRectangleIcon, SparklesIcon, StarIcon, DocumentTextIcon, Cog6ToothIcon, + MagnifyingGlassIcon, HeartIcon, EnvelopeIcon, NewspaperIcon, } from "@heroicons/react/24/outline"; @@ - <SidebarItem - itemName="Find projects" - onclick={handleFindProjects} - icon={<MagnifyingGlassIcon className="size-5" />} - collapsed={isCollapsed} - /> + <SidebarItem + itemName="Find projects" + onclick={() => router.push("/dashboard/projects")} + icon={<MagnifyingGlassIcon className="size-5" />} + collapsed={isCollapsed} + />
♻️ Duplicate comments (3)
apps/web/src/components/newsletters/PremiumUpgradePrompt.tsx (1)
11-33: Tighten accessibility and confirmPrimaryButtonclass prop
- The
LockClosedIconandSparklesIconare decorative; they should be hidden from screen readers witharia-hidden="true"(and optionallyfocusable="false"on the SVG) so they don’t get announced redundantly.- The
PrimaryButtonreceives aclassnameprop. Unless that component explicitly expectsclassname, this likely should beclassNameor your classes may be ignored.Example fix:
- <div className="p-3 bg-gradient-to-br from-[#a472ea]/20 to-[#7150e7]/20 rounded-full"> - <LockClosedIcon className="size-10 text-[#a472ea]" /> + <div className="p-3 bg-gradient-to-br from-[#a472ea]/20 to-[#7150e7]/20 rounded-full"> + <LockClosedIcon className="size-10 text-[#a472ea]" aria-hidden="true" focusable="false" /> </div> ... - <PrimaryButton - onClick={() => router.push("/pricing")} - classname="w-full px-6" - > - <SparklesIcon className="size-4" /> + <PrimaryButton + onClick={() => router.push("/pricing")} + className="w-full px-6" + > + <SparklesIcon className="size-4" aria-hidden="true" focusable="false" /> Unlock Premium </PrimaryButton>apps/web/src/components/ui/input-group.tsx (1)
60-76: Use the shared control slot instead of hard‑coding<input>for addon focus
InputGroupAddoncurrently focuses only the first<input>in the parent:e.currentTarget.parentElement?.querySelector("input")?.focus()This breaks for
InputGroupTextarea(and any future control) even though all controls already exposedata-slot="input-group-control".Update the selector to target the shared slot instead:
e.currentTarget.parentElement ?.querySelector<HTMLElement>('[data-slot="input-group-control"]') ?.focus()This keeps the behavior working for both inputs and textareas and makes the addon more future‑proof.
apps/web/src/app/(main)/dashboard/newsletters/[slug]/page.tsx (1)
215-230: Ensure newsletter HTML is sanitized before usingdangerouslySetInnerHTML
dangerouslySetInnerHTMLis used to rendernewsletter.content. If that HTML is not sanitized on the server (in/api/newsletters/[slug]), this remains an XSS risk, as earlier reviews already noted.Please confirm that the API now sanitizes the markdown output (e.g., via DOMPurify or
sanitize-html) before sending it to the client; if not, this should be treated as a blocking security issue and fixed server-side rather than relying on raw markdown/HTML.
🧹 Nitpick comments (4)
apps/web/src/components/ui/input.tsx (1)
5-19: Input implementation is solid; consider defaultingtypeThe forwardRef wrapper and class merging look good. You might want to default
typeto"text"so callers don’t accidentally render an input without a type:- ({ className, type, ...props }, ref) => { + ({ className, type = "text", ...props }, ref) => {apps/web/src/components/newsletters/NewsletterList.tsx (1)
10-36: Newsletter list states and rendering look correctLoading, empty, and populated states are handled cleanly, and sorting/typing are delegated appropriately. Using
newsletter.idas the primary key is correct; the index fallback is fine given current API guarantees.apps/web/src/styles/newsletter.css (1)
1-195: Style rules are comprehensive; consider consolidating duplicate selectorsThe markdown/newsletter styling is thorough and should produce a good reading experience. Several selectors (e.g.,
.newsletter-content h2,.newsletter-content img,.newsletter-content ul, ol, table cell styles) are defined in two places, with later rules refining earlier ones. It works due to cascade order, but merging these into single blocks per selector would reduce cognitive load and the risk of accidental overrides later.apps/web/src/app/(main)/dashboard/newsletters/[slug]/page.tsx (1)
143-146: Avoid callingrouter.pushduring render; move redirect into an effectHere
router.push('/login')is executed directly in the render phase whenerror === 'unauthorized'. This introduces a side effect during render and may be triggered multiple times.Prefer moving the redirect into a
useEffect:useEffect(() => { if (error === "unauthorized") { router.push("/login"); } }, [error, router]); if (error === "unauthorized") { return null; }This keeps rendering side‑effect free while preserving behavior.
📜 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 (23)
apps/web/package.json(3 hunks)apps/web/src/app/(main)/dashboard/newsletters/[slug]/page.tsx(1 hunks)apps/web/src/app/(main)/dashboard/newsletters/page.tsx(1 hunks)apps/web/src/app/api/newsletters/[slug]/route.ts(1 hunks)apps/web/src/app/api/newsletters/route.ts(1 hunks)apps/web/src/app/globals.css(1 hunks)apps/web/src/components/dashboard/Sidebar.tsx(3 hunks)apps/web/src/components/newsletters/NewsletterCard.tsx(1 hunks)apps/web/src/components/newsletters/NewsletterFilters.tsx(1 hunks)apps/web/src/components/newsletters/NewsletterList.tsx(1 hunks)apps/web/src/components/newsletters/NewsletterPagination.tsx(1 hunks)apps/web/src/components/newsletters/NewsletterSkeleton.tsx(1 hunks)apps/web/src/components/newsletters/PremiumUpgradePrompt.tsx(1 hunks)apps/web/src/components/ui/input-group.tsx(1 hunks)apps/web/src/components/ui/input.tsx(1 hunks)apps/web/src/components/ui/pagination.tsx(1 hunks)apps/web/src/components/ui/skeleton.tsx(1 hunks)apps/web/src/components/ui/textarea.tsx(1 hunks)apps/web/src/content/newsletters/2024-01-welcome.md(1 hunks)apps/web/src/content/newsletters/2024-02-new-features.md(1 hunks)apps/web/src/content/newsletters/2024-03-success-stories.md(1 hunks)apps/web/src/hooks/useNewsletterFilters.ts(1 hunks)apps/web/src/styles/newsletter.css(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (9)
- apps/web/src/hooks/useNewsletterFilters.ts
- apps/web/src/components/ui/textarea.tsx
- apps/web/src/components/newsletters/NewsletterSkeleton.tsx
- apps/web/package.json
- apps/web/src/app/api/newsletters/[slug]/route.ts
- apps/web/src/components/newsletters/NewsletterCard.tsx
- apps/web/src/app/globals.css
- apps/web/src/components/newsletters/NewsletterPagination.tsx
- apps/web/src/components/ui/skeleton.tsx
🧰 Additional context used
🧬 Code graph analysis (8)
apps/web/src/components/ui/input.tsx (1)
apps/web/src/lib/utils.ts (1)
cn(4-6)
apps/web/src/components/ui/input-group.tsx (1)
apps/web/src/lib/utils.ts (1)
cn(4-6)
apps/web/src/app/(main)/dashboard/newsletters/[slug]/page.tsx (3)
apps/web/src/components/newsletters/NewsletterSkeleton.tsx (1)
NewsletterSkeleton(3-19)apps/web/src/hooks/useSubscription.ts (1)
useSubscription(11-77)apps/web/src/components/newsletters/PremiumUpgradePrompt.tsx (1)
PremiumUpgradePrompt(7-37)
apps/web/src/app/(main)/dashboard/newsletters/page.tsx (8)
apps/web/src/components/newsletters/NewsletterCard.tsx (1)
Newsletter(3-11)apps/web/src/components/newsletters/NewsletterFilters.tsx (3)
TimeFilter(3-3)SortFilter(4-4)NewsletterFilters(15-68)apps/web/src/hooks/useSubscription.ts (1)
useSubscription(11-77)apps/web/src/hooks/useNewsletterFilters.ts (1)
useNewsletterFilters(5-35)apps/web/src/components/newsletters/NewsletterSkeleton.tsx (1)
NewsletterSkeleton(3-19)apps/web/src/components/newsletters/PremiumUpgradePrompt.tsx (1)
PremiumUpgradePrompt(7-37)apps/web/src/components/newsletters/NewsletterList.tsx (1)
NewsletterList(10-38)apps/web/src/components/newsletters/NewsletterPagination.tsx (1)
NewsletterPagination(17-89)
apps/web/src/components/dashboard/Sidebar.tsx (2)
apps/web/src/components/sheet/OpensoxProBadge.tsx (1)
OpensoxProBadge(8-28)apps/web/src/components/sidebar/SidebarItem.tsx (1)
SidebarItem(13-42)
apps/web/src/components/ui/pagination.tsx (2)
apps/web/src/lib/utils.ts (1)
cn(4-6)apps/web/src/components/ui/button.tsx (2)
ButtonProps(37-41)buttonVariants(57-57)
apps/web/src/components/newsletters/NewsletterList.tsx (2)
apps/web/src/components/newsletters/NewsletterCard.tsx (2)
Newsletter(3-11)NewsletterCard(13-39)apps/web/src/components/newsletters/NewsletterSkeleton.tsx (1)
NewsletterSkeleton(3-19)
apps/web/src/app/api/newsletters/route.ts (3)
apps/web/src/app/api/newsletters/[slug]/route.ts (1)
GET(20-88)apps/web/src/lib/auth/config.ts (2)
session(43-49)authConfig(6-69)apps/web/src/lib/trpc-server.ts (1)
serverTrpc(8-18)
🪛 ast-grep (0.40.0)
apps/web/src/app/(main)/dashboard/newsletters/[slug]/page.tsx
[warning] 229-229: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🪛 Biome (2.1.2)
apps/web/src/app/(main)/dashboard/newsletters/[slug]/page.tsx
[error] 230-230: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
🪛 LanguageTool
apps/web/src/content/newsletters/2024-01-welcome.md
[style] ~30-~30: Using many exclamation marks might seem excessive (in this case: 3 exclamation marks for a text that’s 704 characters long)
Context: ...hare your journey. Happy Contributing! The Opensox Team
(EN_EXCESSIVE_EXCLAMATION)
apps/web/src/content/newsletters/2024-02-new-features.md
[style] ~36-~36: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...bold text**. this is italic text. this is bold and nested italic. this...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~37-~37: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...this is bold and nested italic. this is a [link to opensox](https://opensox....
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[uncategorized] ~149-~149: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ... later in production environments where user generated content appears. --- ## links with ti...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
apps/web/src/content/newsletters/2024-03-success-stories.md
[uncategorized] ~38-~38: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...elationships. ## Upcoming Events 📅 Open Source Workshop - March 20th Learn best pr...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[style] ~56-~56: Using many exclamation marks might seem excessive (in this case: 3 exclamation marks for a text that’s 1444 characters long)
Context: ... --- Keep contributing, keep growing! [Share your story](mailto:hi@opensox...
(EN_EXCESSIVE_EXCLAMATION)
🪛 markdownlint-cli2 (0.18.1)
apps/web/src/content/newsletters/2024-02-new-features.md
164-164: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🔇 Additional comments (6)
apps/web/src/content/newsletters/2024-03-success-stories.md (1)
1-58: Newsletter front matter and structure look goodFront matter matches the API expectations and the body exercises headings, lists, and links nicely for the renderer. No code-side issues here.
apps/web/src/content/newsletters/2024-01-welcome.md (1)
1-31: Welcome newsletter content aligns with the API contractFront matter keys and overall markdown structure are consistent with the newsletter loader and provide a clear introductory article. No changes needed from a code/integration perspective.
apps/web/src/content/newsletters/2024-02-new-features.md (1)
1-165: Formatting stress-test newsletter is well structuredThis file is a good coverage test for the markdown→HTML pipeline (code blocks, tables, inline HTML, long paragraphs). Front matter matches what the API expects, so it should integrate cleanly.
apps/web/src/app/(main)/dashboard/newsletters/page.tsx (1)
3-3: Verify that importingnewsletter.csshere complies with Next.js global CSS rules
newsletter.cssis imported directly in this page. If it’s a global stylesheet (plain.css, not a module), Next’s app router usually requires importing it from the rootapp/layout.tsxrather than from nested pages. Please confirm this doesn’t cause a build-time “global CSS cannot be imported” error and, if needed, move the import to the root layout or convert it to a CSS module.apps/web/src/components/newsletters/NewsletterFilters.tsx (1)
3-66: Newsletter filters wiring and types look solidTime and sort filters, plus search input, are correctly typed and wired to their callbacks, and the month values align with
useNewsletterFiltersexpectations. Conditional rendering of the sort select keeps the API flexible without over-complicating the UI.apps/web/src/components/dashboard/Sidebar.tsx (1)
154-173: Incorrect path suggestion—use/dashboard/newslettersnot/newslettersThe review correctly identifies that
getSidebarLinkClassNameandpathnameare undefined (line 156 will cause a compile error), but the suggested fix path is wrong.The newsletters page is at
apps/web/src/app/(main)/dashboard/newsletters/page.tsx. The(main)route group doesn't affect the URL path in Next.js App Router, so the actual route is/dashboard/newsletters, not/newsletters. Using/newsletterswould result in a 404.The fix should remove the undefined
classNameexpression and keep the correct href path:<Link - href="/dashboard/newsletters" - className={getSidebarLinkClassName(pathname, "/dashboard/newsletters")} + href="/dashboard/newsletters" >Likely an incorrect or invalid review comment.
done @apsinghdev |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
apps/web/src/app/api/newsletters/route.ts (2)
10-10: Replaceany[]with a proper type for type safety.Define a Newsletter type to ensure type safety for the cached data.
Apply this diff to improve type safety:
+interface Newsletter { + id: string; + title: string; + date: string; + excerpt: string; + readTime: string; + description: string; +} + // Cache newsletters in memory for faster subsequent loads -let cachedNewsletters: any[] | null = null; +let cachedNewsletters: Newsletter[] | null = null; let lastCacheTime = 0; const CACHE_DURATION = 60000; // 1 minute cache
60-76: Consider graceful handling of individual file parsing errors.If a single newsletter file has malformed front matter, the entire request will fail. Consider wrapping the map operation with try-catch to skip problematic files and log warnings.
Apply this diff to handle individual file errors gracefully:
const newsletters = files .filter((file) => file.endsWith(".md")) - .map((file) => { + .flatMap((file) => { + try { const filePath = path.join(newslettersDir, file); const fileContent = fs.readFileSync(filePath, "utf8"); const { data } = matter(fileContent); return { id: file.replace(".md", ""), title: data.title || "Untitled", date: data.date || new Date().toISOString(), excerpt: data.excerpt || "", readTime: data.readTime || "5 min read", description: data.description || data.excerpt || "", }; + } catch (error) { + console.warn(`Failed to parse newsletter file ${file}:`, error); + return []; // Skip this file + } }) .sort((a, b) => new Date(b.date).getTime() - new Date(a.date).getTime());
📜 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 (4)
apps/web/src/app/(main)/dashboard/newsletters/page.tsx(1 hunks)apps/web/src/app/api/newsletters/route.ts(1 hunks)apps/web/src/components/dashboard/Sidebar.tsx(3 hunks)apps/web/src/styles/newsletter.css(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/web/src/styles/newsletter.css
- apps/web/src/app/(main)/dashboard/newsletters/page.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
apps/web/src/components/dashboard/Sidebar.tsx (1)
apps/web/src/components/sheet/OpensoxProBadge.tsx (1)
OpensoxProBadge(8-28)
apps/web/src/app/api/newsletters/route.ts (2)
apps/web/src/app/api/newsletters/[slug]/route.ts (1)
GET(20-88)apps/web/src/lib/trpc-server.ts (1)
serverTrpc(8-18)
🔇 Additional comments (1)
apps/web/src/components/dashboard/Sidebar.tsx (1)
47-52: LGTM! Newsletter route properly configured.The newsletters route is correctly added with the
isPro: trueflag, which properly gates this feature for pro users as intended in the PR objectives.
|
@huamanraj one humble request - please don't mark the coderabit reviews as
for example you've marked this review as resolved without any reason or fix: and it's throwing error locally: 
remember, our goal isn't to just push the code anyhow. our goal is to build a solid product that thousands of people use every day. 🙏 |
Hey @apsinghdev , |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
apps/web/src/app/api/newsletters/[slug]/route.ts (1)
69-80: Critical: sanitize markdown HTML to prevent XSS
marked.parse(Line 73) returns raw HTML and does not sanitize by default. Returning this directly ascontent(Line 79) and rendering it on the client (viadangerouslySetInnerHTML) allows arbitrary script injection from newsletter markdown.You should sanitize the HTML server‑side before returning it, e.g. with
isomorphic-dompurify:-import { marked } from "marked"; +import { marked } from "marked"; +import DOMPurify from "isomorphic-dompurify"; @@ - // Render markdown (supports headings, links, lists, code blocks, images, tables, etc.) - const htmlContent = marked.parse(content); + // Render markdown (supports headings, links, lists, code blocks, images, tables, etc.) + const htmlContent = marked.parse(content); + const sanitizedContent = DOMPurify.sanitize(htmlContent, { + ALLOWED_TAGS: [ + "p", + "br", + "strong", + "em", + "u", + "h1", + "h2", + "h3", + "h4", + "h5", + "h6", + "ul", + "ol", + "li", + "a", + "code", + "pre", + "blockquote", + "img", + "table", + "thead", + "tbody", + "tr", + "th", + "td", + ], + ALLOWED_ATTR: ["href", "src", "alt", "title", "class"], + }); @@ - const result = { + const result = { title: data.title || "Untitled", date: data.date || new Date().toISOString(), readTime: data.readTime || "5 min read", - content: htmlContent, + content: sanitizedContent, };And ensure
isomorphic-dompurifyis added to your dependencies (e.g.,npm install isomorphic-dompurifyinapps/web).
🧹 Nitpick comments (2)
apps/web/src/lib/trpc-server.ts (1)
9-19: Optional: DRY the TRPC base URLThe TRPC URL string is duplicated in both
serverTrpcandcreateAuthenticatedClient. Extracting it into a shared constant reduces drift risk if the base URL ever changes.-import { createTRPCProxyClient, httpBatchLink } from "@trpc/client"; +import { createTRPCProxyClient, httpBatchLink } from "@trpc/client"; import superjson from "superjson"; import type { AppRouter } from "../../../api/src/routers/_app"; import type { Session } from "next-auth"; +const TRPC_URL = + (process.env.NEXT_PUBLIC_API_URL || "http://localhost:4000") + "/trpc"; + /** * Server-side tRPC client for use in NextAuth callbacks and server components */ export const serverTrpc = createTRPCProxyClient<AppRouter>({ links: [ httpBatchLink({ transformer: superjson, - url: `${process.env.NEXT_PUBLIC_API_URL || "http://localhost:4000"}/trpc`, + url: TRPC_URL, @@ export function createAuthenticatedClient(session: Session) { return createTRPCProxyClient<AppRouter>({ links: [ httpBatchLink({ transformer: superjson, - url: `${process.env.NEXT_PUBLIC_API_URL || "http://localhost:4000"}/trpc`, + url: TRPC_URL,Also applies to: 25-30
apps/web/src/app/api/newsletters/[slug]/route.ts (1)
20-23: Optional: simplify route signature forparamsThe GET handler currently types
paramsasPromise<{ slug: string }>and then awaits it. Next.js route handlers typically receiveparamsas a plain object, so this is a bit misleading even thoughawaiton a non‑promise is a no‑op at runtime.You can simplify and better match Next.js conventions:
-export async function GET( - _request: Request, - { params }: { params: Promise<{ slug: string }> } -) { +export async function GET( + _request: Request, + { params }: { params: { slug: string } } +) { @@ - const { slug } = await params; + const { slug } = params;Also applies to: 53-59
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web/src/app/api/newsletters/[slug]/route.ts(1 hunks)apps/web/src/app/api/newsletters/route.ts(1 hunks)apps/web/src/lib/trpc-server.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
apps/web/src/lib/trpc-server.ts (4)
apps/web/src/lib/auth/config.ts (2)
session(43-49)jwt(51-64)apps/web/src/providers/trpc-provider.tsx (4)
TRPCProvider(11-53)baseUrl(25-46)headers(34-42)QueryClient(14-21)apps/api/src/trpc.ts (1)
authHeader(10-36)apps/web/src/types/next-auth.d.ts (1)
Session(4-6)
apps/web/src/app/api/newsletters/[slug]/route.ts (3)
apps/web/src/app/api/newsletters/route.ts (1)
GET(14-91)apps/web/src/lib/auth/config.ts (2)
session(43-49)authConfig(6-69)apps/web/src/lib/trpc-server.ts (1)
createAuthenticatedClient(24-38)
apps/web/src/app/api/newsletters/route.ts (3)
apps/web/src/app/api/newsletters/[slug]/route.ts (1)
GET(20-89)apps/web/src/lib/auth/config.ts (2)
session(43-49)authConfig(6-69)apps/web/src/lib/trpc-server.ts (1)
createAuthenticatedClient(24-38)
🔇 Additional comments (2)
apps/web/src/app/api/newsletters/route.ts (1)
14-42: Auth gating and error responses look solidThe flow in this route—401 for no session, 403 for non‑paid users, and a 500 with an explicit error payload when subscription verification or file reading fails—is clean and matches the premium‑only requirement. Combined with the in‑memory cache, this should behave well once
createAuthenticatedClientis corrected to send the JWT access token.Also applies to: 53-90
apps/web/src/app/api/newsletters/[slug]/route.ts (1)
24-51: Auth, subscription gating, and per‑slug caching look goodThe 401/403/500 handling mirrors the list route, and the per‑slug Map cache with a 1‑minute TTL (Line 18) is a reasonable optimization for repeated reads of the same newsletter. Once
createAuthenticatedClientis fixed to send the JWT access token, this route’s auth and caching behavior should be solid.Also applies to: 57-88
|
still some error related to server side subscription check that i will fix! |
|
@huamanraj hey aman! is it done? some suggestions:
|
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
apps/web/src/components/dashboard/Sidebar.tsx (2)
142-145: Clean up excessive empty lines.This issue was previously flagged but remains unresolved. Four consecutive empty lines appear to be leftover from refactoring and should be removed for cleaner code.
124-138: Inconsistent rendering approach and missing accessibility features.This issue was previously flagged but remains unresolved. The custom div rendering for SIDEBAR_ROUTES items is inconsistent with the
SidebarItemcomponent usage elsewhere (lines 146-151, 168-173). Additionally, the div lacks semantic HTML and accessibility features like keyboard navigation and ARIA attributes.Per the previous review, consider either:
- Updating
SidebarItemto support theisProflag and using it consistently, or- Replacing the
<div>with a<button>element and adding proper accessibility attributes
🧹 Nitpick comments (2)
apps/web/src/app/api/newsletters/route.ts (2)
10-10: Consider improving type safety.Two areas reduce type safety:
- Line 10:
any[]for cached newsletters- Line 28: Type assertion
(trpc.user as any)While the
anyassertion on line 28 may be necessary ifAppRoutertypes don't properly expose the user router, consider defining a proper interface for newsletter metadata to replaceany[].Apply this diff to add a type and comment for clarity:
+// Newsletter metadata type +interface NewsletterMetadata { + id: string; + title: string; + date: string; + excerpt: string; + readTime: string; + description: string; +} + // Cache newsletters in memory for faster subsequent loads -let cachedNewsletters: any[] | null = null; +let cachedNewsletters: NewsletterMetadata[] | null = null; let lastCacheTime = 0; -const CACHE_DURATION = 60000; +const CACHE_DURATION = 60_000; // 1 minute cache (60 seconds)Also applies to: 28-28
54-56: Remove early return after directory creation.After creating the directory on line 55, the code immediately returns an empty array. While the newly created directory would indeed be empty, this early return is unnecessary and creates inconsistent control flow. Let the normal file reading logic (lines 59-77) handle the empty directory case.
Apply this diff:
if (!fs.existsSync(newslettersDir)) { fs.mkdirSync(newslettersDir, { recursive: true }); - return NextResponse.json([]); } const files = fs.readdirSync(newslettersDir);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web/src/app/api/newsletters/route.ts(1 hunks)apps/web/src/components/dashboard/Sidebar.tsx(3 hunks)apps/web/src/lib/trpc-server.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
apps/web/src/lib/trpc-server.ts (2)
apps/web/src/lib/auth/config.ts (1)
session(43-49)apps/api/src/routers/_app.ts (1)
AppRouter(26-26)
apps/web/src/components/dashboard/Sidebar.tsx (1)
apps/web/src/components/sheet/OpensoxProBadge.tsx (1)
OpensoxProBadge(8-28)
apps/web/src/app/api/newsletters/route.ts (3)
apps/web/src/app/api/newsletters/[slug]/route.ts (1)
GET(20-89)apps/web/src/lib/auth/config.ts (2)
session(43-49)authConfig(6-69)apps/web/src/lib/trpc-server.ts (1)
createAuthenticatedClient(24-42)
🔇 Additional comments (5)
apps/web/src/lib/trpc-server.ts (1)
21-42: LGTM! Authentication implementation is correct.The function correctly uses
session.accessTokenfor the Authorization header, addressing the previous critical issue. The implementation aligns with the NextAuth session callback pattern and safely handles missing tokens.apps/web/src/app/api/newsletters/route.ts (2)
25-42: LGTM! Subscription verification correctly implemented.The code now properly uses
createAuthenticatedClient(session)to create an authenticated TRPC client, fixing the previous critical runtime error. Error handling returns appropriate 500 status, addressing the past concern about error masking.
84-90: LGTM! Error handling properly implemented.The catch block now correctly returns a 500 status with an error message instead of masking failures with an empty array, addressing the previous critical review concern. This allows clients to distinguish between "no newsletters" and "failed to read newsletters."
apps/web/src/components/dashboard/Sidebar.tsx (2)
18-18: LGTM!The
NewspaperIconimport is correctly added and used in the newsletters route.
43-48: Subscription gating correctly implemented in newsletter pages.Verification confirms that both newsletter pages properly implement subscription checks using the
useSubscription()hook and display aPremiumUpgradePromptfor free users, as intended.
- Listing page (
apps/web/src/app/(main)/dashboard/newsletters/page.tsx): ChecksisPaidUserand shows upgrade prompt- Detail page (
apps/web/src/app/(main)/dashboard/newsletters/[slug]/page.tsx): Same gating pattern implementedBoth pages handle unauthorized access by redirecting to login and wait for subscription loading before rendering.
|
hey @apsinghdev ! now it's done! tested 
|
…nd improve newsletter UI styling
apsinghdev
force-pushed
the
feat_newsletter
branch
from
90b0563 to
79af1ea
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
apps/web/src/components/ui/input-group.tsx (1)
60-78: Addon focus should target the shared control slot, not only<input>elements
InputGroupAddoncurrently focuses only the firstinputin the parent, so clicking the addon forInputGroupTextarea(and any future non-inputcontrols) won’t transfer focus even though all controls exposedata-slot="input-group-control". Use that shared slot instead:onClick={(e) => { if ((e.target as HTMLElement).closest("button")) { return } - e.currentTarget.parentElement?.querySelector("input")?.focus() + e.currentTarget.parentElement + ?.querySelector<HTMLElement>('[data-slot="input-group-control"]') + ?.focus() }}This keeps the behavior consistent across input and textarea groups and future-proofs the addon.
apps/web/src/app/(main)/dashboard/newsletters/[slug]/page.tsx (1)
229-231: CRITICAL: XSS vulnerability—content rendered without sanitization.Line 230 renders HTML using
dangerouslySetInnerHTMLwith content fetched from the API. The API route (verified inapps/web/src/app/api/newsletters/[slug]/route.tsline 83) usesmarked.parse()without sanitization, creating an XSS vulnerability. Malicious JavaScript in markdown files would execute in users' browsers.Required fix: The API route must sanitize HTML output before sending it to the client (see my comment on that file). Alternatively, sanitize here on the client before rendering, though server-side sanitization is the recommended practice.
As per static analysis hints
apps/web/src/app/api/newsletters/[slug]/route.ts (1)
79-90: CRITICAL: XSS vulnerability remains unresolved—HTML not sanitized after markdown parsing.Line 83 parses markdown with
marked.parse()without any sanitization, and line 89 includes the raw HTML in the response. Themarkedlibrary does not sanitize HTML by default, creating an XSS vulnerability where malicious JavaScript embedded in markdown files would execute in users' browsers when rendered withdangerouslySetInnerHTML(line 230 in the page component).Required fix: Add HTML sanitization using
DOMPurifyorsanitize-htmlimmediately after parsing.Install the dependency:
npm install isomorphic-dompurifyApply this diff to sanitize the HTML:
import { marked } from "marked"; +import DOMPurify from "isomorphic-dompurify"; import { getServerSession } from "next-auth"; import { authConfig } from "@/lib/auth/config"; import { createAuthenticatedClient } from "@/lib/trpc-server"; // ... existing code ... const fileContent = fs.readFileSync(filePath, "utf8"); const { data, content } = matter(fileContent); // Render markdown (supports headings, links, lists, code blocks, images, tables, etc.) const htmlContent = marked.parse(content); + const sanitizedContent = DOMPurify.sanitize(htmlContent, { + ALLOWED_TAGS: ['p', 'br', 'strong', 'em', 'u', 'h1', 'h2', 'h3', 'h4', 'h5', 'h6', + 'ul', 'ol', 'li', 'a', 'code', 'pre', 'blockquote', 'img', + 'table', 'thead', 'tbody', 'tr', 'th', 'td', 'hr', 'del', 'sup', 'sub'], + ALLOWED_ATTR: ['href', 'src', 'alt', 'title', 'class'] + }); const result = { title: data.title || "Untitled", date: data.date || new Date().toISOString(), readTime: data.readTime || "5 min read", - content: htmlContent, + content: sanitizedContent, };apps/web/src/components/ui/pagination.tsx (2)
37-59: MAJOR: Keyboard accessibility issue—pagination links not focusable.
PaginationLinkrenders an<a>element (line 48), but callers (likeNewsletterPagination) pass onlyonClickwithout anhrefattribute. Anchors withouthrefare not keyboard-focusable in most browsers, making all pagination controls inaccessible to keyboard users.Required fix: Render a
<button>when nohrefis provided, or addrole="button"andtabIndex={0}to the<a>with keyboard event handling for Enter/Space keys.Apply this diff to render semantic buttons when no href is provided:
type PaginationLinkProps = { isActive?: boolean + href?: string } & Pick<ButtonProps, "size"> & - React.ComponentProps<"a"> + ( + | ({ href: string } & React.AnchorHTMLAttributes<HTMLAnchorElement>) + | ({ href?: never } & React.ButtonHTMLAttributes<HTMLButtonElement>) + ) const PaginationLink = ({ className, isActive, size = "icon", + href, ...props }: PaginationLinkProps) => ( - <a + {href ? ( + <a + href={href} + aria-current={isActive ? "page" : undefined} + className={cn( + buttonVariants({ + variant: isActive ? "outline" : "ghost", + size, + }), + className + )} + {...props} + /> + ) : ( + <button + type="button" - aria-current={isActive ? "page" : undefined} + aria-current={isActive ? "page" : undefined} - className={cn( + className={cn( - buttonVariants({ + buttonVariants({ - variant: isActive ? "outline" : "ghost", + variant: isActive ? "outline" : "ghost", - size, + size, - }), + }), - className + className - )} + )} - {...props} + {...props} - /> + /> + )} )Note: The diff above shows the concept; you may need to adjust the exact TypeScript union for props depending on your needs.
62-92: PaginationPrevious and PaginationNext need href or button semantics.These components extend
PaginationLink, so they inherit the keyboard accessibility issue. When callers pass onlyonClick(as inNewsletterPagination), these controls won't be keyboard-focusable.Once you fix
PaginationLinkto support button semantics when nohrefis provided, these components will automatically benefit from the fix.
🧹 Nitpick comments (5)
apps/web/scripts/init-submodules.sh (1)
1-16: Add validation and debugging for submodule initialization.To improve observability and reliability, consider adding checks for successful initialization and better logging for troubleshooting failed builds.
Apply this diff to add validation and debugging output:
#!/bin/bash # initialize git submodules during vercel build set -e # setup ssh for private submodule if [ -n "$GIT_SSH_KEY" ]; then + echo "Configuring SSH for private submodule access..." mkdir -p ~/.ssh printf '%s' "$GIT_SSH_KEY" > ~/.ssh/id_ed25519 chmod 600 ~/.ssh/id_ed25519 ssh-keyscan -t ed25519 github.com >> ~/.ssh/known_hosts 2>/dev/null || true fi # initialize and update submodules +echo "Initializing git submodules..." git submodule update --init --recursive --remote + +if [ $? -eq 0 ]; then + echo "✓ Submodules initialized successfully" +else + echo "✗ Failed to initialize submodules" >&2 + exit 1 +fiThis adds:
- Logging for SSH setup (clarity during build logs)
- Explicit success/failure indication after submodule initialization
- Stderr output for errors (easier to spot in build logs)
- Exit code propagation for proper build failure detection
apps/web/src/components/newsletters/NewsletterPagination.tsx (1)
58-61: Consider addingaria-disabledfor better screen reader support.The Previous and Next buttons use
pointer-events-noneandopacity-50for visual disabled states, but screen readers won't recognize them as disabled. Addingaria-disabled="true"when at boundaries would improve accessibility.Apply this diff:
<PaginationItem> <PaginationPrevious onClick={() => onPageChange(Math.max(1, currentPage - 1))} - className={currentPage === 1 ? "pointer-events-none opacity-50" : "cursor-pointer"} + className={currentPage === 1 ? "pointer-events-none opacity-50" : "cursor-pointer"} + aria-disabled={currentPage === 1} /> </PaginationItem> {/* ... */} <PaginationItem> <PaginationNext onClick={() => onPageChange(Math.min(totalPages, currentPage + 1))} - className={currentPage === totalPages ? "pointer-events-none opacity-50" : "cursor-pointer"} + className={currentPage === totalPages ? "pointer-events-none opacity-50" : "cursor-pointer"} + aria-disabled={currentPage === totalPages} /> </PaginationItem>Also applies to: 81-84
apps/web/src/components/dashboard/Sidebar.tsx (3)
72-76: Minor: tightenproSectionExpandedbehavior andaria-expanded.
proSectionExpandedis always used to setaria-expanded, even for non‑paid users where clicking the row navigates to/pricinginstead of expanding/collapsing content. That can be slightly misleading for assistive tech, since nothing actually expands/collapses for free users.You could make the semantics clearer by only exposing
aria-expandedfor paid users:aria-expanded={isPaidUser ? proSectionExpanded : undefined}Optionally, if you prefer the Pro section to start collapsed for paid users, you could also initialize with
useState(false)and rely on theuseEffectto auto‑expand when the user lands on a premium route.Also applies to: 77-90, 95-101
156-200: Reduce duplication between free and premium nav item markup.The JSX for free items (FREE_ROUTES) and premium sub‑items (PREMIUM_ROUTES inside the expanded Pro section) is nearly identical: same structure, same classes, same badge rendering, only the left padding and maybe some context differ.
To keep this maintainable, consider extracting a small shared component (e.g.,
SidebarNavItem) or reusingSidebarItemwith a richer props shape so that:
- Icon, label, active state, and optional badge are driven by props.
- Styling stays consistent between free and premium items.
- Future tweaks to hover/active styles or typography only need to be made in one place.
This would also make it easier to add more routes later without re‑copying JSX.
Also applies to: 288-327
209-277: Nested clickable elements in Pro header can cause confusing interactions.The Pro header row is clickable (
onClick={handleProSectionClick}withrole="button"), and inside it you renderOpensoxProBadge, which itself is a<Link href="/pricing">. That creates overlapping interactive regions:
- For paid users, clicking the badge may both toggle the section (via the parent
onClick) and navigate to/pricing.- For free users, both the parent click handler and the inner link navigate to
/pricing, which is redundant.This can feel janky and is not ideal for accessibility.
Consider one of these adjustments:
- Add a prop to
OpensoxProBadgeto render a non‑link variant (plain<span>/<div>) when used inside this header, letting the header row control the behavior; or- Keep
OpensoxProBadgeas a link but move the expand/collapse behavior to a separate control (e.g., only the chevron area toggles, while the badge/label link to pricing).
📜 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 (26)
.gitmodules(1 hunks)apps/web/.gitignore(1 hunks)apps/web/package.json(4 hunks)apps/web/scripts/init-submodules.sh(1 hunks)apps/web/src/app/(main)/dashboard/newsletters/[slug]/page.tsx(1 hunks)apps/web/src/app/(main)/dashboard/newsletters/page.tsx(1 hunks)apps/web/src/app/api/newsletters/[slug]/route.ts(1 hunks)apps/web/src/app/api/newsletters/route.ts(1 hunks)apps/web/src/app/globals.css(1 hunks)apps/web/src/components/dashboard/Sidebar.tsx(5 hunks)apps/web/src/components/newsletters/NewsletterCard.tsx(1 hunks)apps/web/src/components/newsletters/NewsletterFilters.tsx(1 hunks)apps/web/src/components/newsletters/NewsletterList.tsx(1 hunks)apps/web/src/components/newsletters/NewsletterPagination.tsx(1 hunks)apps/web/src/components/newsletters/NewsletterSkeleton.tsx(1 hunks)apps/web/src/components/newsletters/PremiumUpgradePrompt.tsx(1 hunks)apps/web/src/components/ui/input-group.tsx(1 hunks)apps/web/src/components/ui/input.tsx(1 hunks)apps/web/src/components/ui/pagination.tsx(1 hunks)apps/web/src/components/ui/skeleton.tsx(1 hunks)apps/web/src/components/ui/textarea.tsx(1 hunks)apps/web/src/content/newsletters-premium(1 hunks)apps/web/src/content/newsletters/2024-01-welcome.md(1 hunks)apps/web/src/hooks/useNewsletterFilters.ts(1 hunks)apps/web/src/lib/trpc-server.ts(2 hunks)apps/web/src/styles/newsletter.css(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- .gitmodules
- apps/web/src/content/newsletters-premium
- apps/web/.gitignore
🚧 Files skipped from review as they are similar to previous changes (13)
- apps/web/src/components/newsletters/NewsletterList.tsx
- apps/web/src/components/newsletters/PremiumUpgradePrompt.tsx
- apps/web/src/lib/trpc-server.ts
- apps/web/src/app/globals.css
- apps/web/src/components/ui/textarea.tsx
- apps/web/src/components/newsletters/NewsletterSkeleton.tsx
- apps/web/src/components/newsletters/NewsletterFilters.tsx
- apps/web/src/app/(main)/dashboard/newsletters/page.tsx
- apps/web/src/app/api/newsletters/route.ts
- apps/web/package.json
- apps/web/src/styles/newsletter.css
- apps/web/src/components/ui/skeleton.tsx
- apps/web/src/components/newsletters/NewsletterCard.tsx
🧰 Additional context used
🧬 Code graph analysis (7)
apps/web/src/hooks/useNewsletterFilters.ts (2)
apps/web/src/components/newsletters/NewsletterCard.tsx (1)
Newsletter(3-11)apps/web/src/components/newsletters/NewsletterFilters.tsx (1)
TimeFilter(3-3)
apps/web/src/components/newsletters/NewsletterPagination.tsx (1)
apps/web/src/components/ui/pagination.tsx (7)
Pagination(110-110)PaginationContent(111-111)PaginationItem(113-113)PaginationPrevious(114-114)PaginationLink(112-112)PaginationEllipsis(116-116)PaginationNext(115-115)
apps/web/src/components/ui/input.tsx (1)
apps/web/src/lib/utils.ts (1)
cn(4-6)
apps/web/src/components/ui/input-group.tsx (3)
apps/web/src/lib/utils.ts (1)
cn(4-6)apps/web/src/components/ui/input.tsx (1)
Input(22-22)apps/web/src/components/ui/textarea.tsx (1)
Textarea(22-22)
apps/web/src/app/api/newsletters/[slug]/route.ts (4)
apps/web/src/app/api/newsletters/route.ts (1)
GET(18-101)apps/web/src/lib/auth/config.ts (2)
session(43-49)authConfig(6-69)apps/web/src/lib/trpc.ts (1)
trpc(4-4)apps/web/src/lib/trpc-server.ts (1)
createAuthenticatedClient(24-42)
apps/web/src/components/dashboard/Sidebar.tsx (3)
apps/web/src/store/useShowSidebar.ts (1)
useShowSidebar(10-16)apps/web/src/hooks/useSubscription.ts (1)
useSubscription(11-77)apps/web/src/components/sheet/OpensoxProBadge.tsx (1)
OpensoxProBadge(8-28)
apps/web/src/components/ui/pagination.tsx (2)
apps/web/src/lib/utils.ts (1)
cn(4-6)apps/web/src/components/ui/button.tsx (2)
ButtonProps(37-41)buttonVariants(57-57)
🪛 ast-grep (0.40.0)
apps/web/src/app/(main)/dashboard/newsletters/[slug]/page.tsx
[warning] 229-229: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🪛 Biome (2.1.2)
apps/web/src/app/(main)/dashboard/newsletters/[slug]/page.tsx
[error] 230-230: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
🪛 LanguageTool
apps/web/src/content/newsletters/2024-01-welcome.md
[style] ~46-~46: Using many exclamation marks might seem excessive (in this case: 3 exclamation marks for a text that’s 1209 characters long)
Context: ...source world. --- Happy Contributing! The Opensox Team
(EN_EXCESSIVE_EXCLAMATION)
🔇 Additional comments (15)
apps/web/src/content/newsletters/2024-01-welcome.md (4)
5-5: Clarify readTime: hardcoded vs. calculated.The
readTimefield is hardcoded as "3 min read" in the front matter. Confirm whether:
- This is the intended pattern for sample/documentation (acceptable), or
- The API should dynamically calculate readTime based on word count (in which case this should be documented as an example value only)
24-24: Verify external image URL and consider providing a fallback.The Unsplash image link on Line 24 is an external dependency. If the URL becomes unavailable, the newsletter rendering could degrade. Consider documenting how broken image URLs are handled in the renderer, or provide local image alternatives.
42-42: Verify the/pricinglink target exists.The relative link on Line 42 points to
/pricing. Ensure this page is routed and accessible in your application, and that it clearly explains the premium upgrade benefits to encourage conversions.
1-47: Sample newsletter content structure is well-organized and demonstrates markdown features effectively.The newsletter showcases the supported features (headings, bold, lists, links, images, horizontal rules) in a professional, developer-focused format. The content flows logically from introduction through premium upgrade CTA, making it an excellent example for the documentation. The front matter is properly formatted and the content is engaging.
apps/web/src/components/ui/input.tsx (1)
1-22: Input wrapper is clean and idiomaticTyped
forwardRef, prop spreading, and class merging viacnall look correct and consistent with the rest of the UI kit; no changes needed here.apps/web/src/components/ui/input-group.tsx (1)
1-170: InputGroup composition and variant usage look solidThe grouping, data-slot usage, and cva-based variants across
InputGroup,InputGroupButton,InputGroupText,InputGroupInput, andInputGroupTextareaare cohesive and align well with the existing UI primitives; nothing blocking here.apps/web/src/hooks/useNewsletterFilters.ts (1)
5-34: LGTM! Clean filtering implementation.The hook correctly implements memoized filtering for search and month-based time filters. The search filter handles optional fields safely with optional chaining, and the month extraction logic using
toLocaleStringaligns well with theTimeFiltertype union.Note: The past review comment about switch case declarations is now outdated—the implementation has changed to use month name matching instead of date range logic, eliminating that concern entirely.
apps/web/src/components/newsletters/NewsletterPagination.tsx (1)
24-52: LGTM! Solid pagination logic with smart ellipsis placement.The
getPageNumbershelper correctly handles various edge cases:
- Shows all pages when totalPages ≤ 5
- Always displays first and last pages
- Computes a sliding window around the current page
- Inserts ellipses only when there are hidden ranges
The logic produces intuitive pagination controls for any page count.
apps/web/src/app/(main)/dashboard/newsletters/[slug]/page.tsx (2)
84-126: Good multi-layer access control on the frontend.The component correctly implements subscription-based gating:
- Waits for subscription status before fetching data
- Handles API auth responses (401 → login redirect, 403 → upgrade prompt)
- Client-side verification with
isPaidUseras a UX optimizationHowever, the ultimate security relies on the API endpoint enforcing authentication and authorization server-side (see comments on the API route file).
128-175: Excellent loading and error state handling.The progressive disclosure pattern is well-implemented:
- Skeleton during subscription check
- Upgrade prompt for non-paid users
- Login redirect for unauthenticated users
- Skeleton during data fetch
- Not-found state with navigation back
This provides clear feedback at every stage and prevents layout shifts.
apps/web/src/app/api/newsletters/[slug]/route.ts (2)
25-54: Excellent! Authentication and authorization properly implemented.The API route now correctly enforces access control:
- Authentication: Lines 26-33 verify session with NextAuth, returning 401 for unauthenticated requests
- Authorization: Lines 36-47 check subscription status via TRPC, returning 403 for non-paid users
This resolves the critical security concern from the previous review about bypassing the paywall via direct API access.
16-19: Well-designed caching strategy with environment-aware TTL.The per-slug cache with a Map and time-based invalidation is appropriate for this use case:
- 60 seconds in development for quick iteration
- 1 hour in production since newsletter content changes infrequently
This balances freshness with performance effectively.
Also applies to: 56-62, 92-92
apps/web/src/components/ui/pagination.tsx (1)
7-15: LGTM! Proper semantic markup and ARIA labels.The
Paginationnav wrapper correctly usesrole="navigation"andaria-label="pagination"for screen reader context. ThePaginationEllipsisappropriately usesaria-hiddensince ellipses are purely decorative with screen-reader-only fallback text.Also applies to: 94-107
apps/web/src/components/dashboard/Sidebar.tsx (2)
279-377: Premium gating, locked preview, and collapsed behavior look consistent.The Pro section behavior is cohesive:
- Paid users only see premium sub‑items when
isPaidUser && proSectionExpanded, with active styling that matches the free routes and supports nested paths viapathname.startsWith(...).- Free users get a locked preview list of premium routes that consistently routes to
/pricing, with keyboard support viarole="button",tabIndex, andonKeyDown.- The collapsed sidebar variant exposes a single Pro icon button that reuses
handleProSectionClick, so free users still get a pricing shortcut and paid users can toggle the section.This aligns well with the “pro‑gated + upsell” requirements from the PR.
Also applies to: 379-395, 397-402
3-21: Route path is correct—no action needed.The newsletter route path in
Sidebar.tsxline 65 (path: "/dashboard/newsletters") is correct. Page files are located atapps/web/src/app/(main)/dashboard/newsletters/, which routes to/dashboard/newslettersin Next.js App Router. This is confirmed by:
- Nested page at
/dashboard/newsletters/[slug]/page.tsx- NewsletterCard links to
/dashboard/newsletters/${newsletter.id}- Redirect logic in the newsletter detail page uses
/dashboard/newslettersThe configuration aligns with the actual routing structure and link usage throughout the codebase.
2 participants
fix #155
this pr adds a complete newsletter system for pro users. newsletters are stored as
markdown files(easy to write edit and render), and displayed as clean, readable blog-style posts withskeleton loading states. the system is simple, fast, and easy for the team to maintain also and a practical approach for scalability .design:
2025-11-17_17-35-39.mp4
2025-11-17_18-20-09.mp4
hey @apsinghdev this is my submissong for bounty and frontend role kept it simple for user and a better approach to scale as well as easy to write newsletter also! more focused on practicality! matching with the theme and also good ui!
i have also contributed in dashboard design prev and also i have good practical experience with next js high preformat applications in this bounty also hoping for positive results!
what this includes
updated files
sample data
how to add a new newsletter
you can use bold text, headings, links, images, lists, code blocks, and tables.
example:
the newsletter will automatically appear in the newsletters list, sorted by date.
that’s it. no additional configuration or code changes needed.
Summary by CodeRabbit
New Features
UI Improvements
✏️ Tip: You can customize this high-level summary in your review settings.