-
Notifications
You must be signed in to change notification settings - Fork 111
feat: newsletter page for pro users #155 #158
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
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. |
WalkthroughIntroduces a complete newsletter feature for pro users, including markdown-based content management, API routes to fetch and list newsletters with in-memory caching, client-side pages for browsing and reading individual newsletters with filtering/pagination, and new UI components supporting the newsletter interface alongside subscription-aware access control. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Client as NewslettersPage
participant API as /api/newsletters
participant FS as Filesystem
participant Cache as Memory Cache
User->>Client: Load /dashboard/newsletters
Client->>Client: Check subscription status
alt Subscription Loading
Client->>User: Show skeleton loaders
else Paid User
Client->>API: GET /api/newsletters
API->>Cache: Check cache
alt Cache Hit
Cache-->>API: Return cached data
else Cache Miss
API->>FS: Read newsletters/*.md
API->>API: Parse YAML front matter
API->>API: Extract slug, title, date, excerpt
API->>Cache: Store in cache (1min TTL)
end
API-->>Client: Return newsletters array
Client->>Client: Apply filters (search, time range)
Client->>Client: Paginate results
Client->>User: Render newsletter list
User->>User: Click newsletter card
else Non-Paid User
Client->>User: Show PremiumUpgradePrompt
end
User->>Client: Navigate to /dashboard/newsletters/[slug]
Client->>API: GET /api/newsletters/[slug]
API->>Cache: Check cache by slug
alt Cache Hit
Cache-->>API: Return cached data
else Cache Miss
API->>FS: Read newsletters/[slug].md
API->>API: Parse YAML front matter
API->>API: Convert markdown → HTML (marked)
API->>Cache: Store in cache (1min TTL)
end
API-->>Client: Return {title, date, readTime, content}
Client->>Client: Render content via dangerouslySetInnerHTML
Client->>User: Display newsletter with styling
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related issues
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
| <div | ||
| className="newsletter-content" | ||
| dangerouslySetInnerHTML={{ __html: newsletter.content }} | ||
| /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify server-side HTML sanitization to prevent XSS.
Using dangerouslySetInnerHTML without visible sanitization is a security risk. Ensure the API endpoint /api/newsletters/${slug} properly sanitizes the markdown content during server-side rendering to prevent XSS attacks.
Run the following script to examine the API route's markdown processing and sanitization:
🏁 Script executed:
#!/bin/bash
# Description: Verify markdown processing and HTML sanitization in the newsletter API route
# Search for the API route file and its markdown processing
rg -n -A 10 -B 2 "newsletters/\[slug\]/route" --type ts
# Look for markdown parsing and sanitization libraries
rg -n "remark|rehype|marked|markdown-it|sanitize|DOMPurify" --type ts --type jsonLength of output: 808
I need to examine the full API route file and verify the sanitization approach. Let me gather more information:
Now let me examine the full API route file and the frontend component:
Unfortunately, I cannot complete the verification without access to the full codebase files. However, based on my research findings:
The marked library's sanitize option is deprecated and should NOT be used as it cannot be considered secure. Instead, a sanitize library like DOMPurify is recommended. Markdown doesn't provide security benefits by default, and marked by default doesn't sanitize or escape HTML content found within Markdown text when making a conversion.
Verify HTML sanitization in API route and frontend component.
The security concern raised is valid. OWASP recommends DOMPurify for HTML sanitization, and the current implementation using dangerouslySetInnerHTML with markdown-parsed content requires explicit sanitization:
- Check if the API route (
apps/web/src/app/api/newsletters/[slug]/route.ts) applies sanitization aftermarked.parse()- either using DOMPurify, sanitize-html, or another library. - Verify the frontend component (
page.tsxlines 141-144) doesn't sanitize the HTML beforedangerouslySetInnerHTML. If no sanitization exists in either location, implement sanitization using DOMPurify or another security-focused library.
🏁 Script executed:
#!/bin/bash
# Read the full API route file
cat apps/web/src/app/api/newsletters/\[slug\]/route.ts
# Also search for sanitization in the route file and related frontend component
rg -n "sanitize|DOMPurify|escape" apps/web/src/app/api/newsletters/ --type ts
# Search for where newsletter content is used on the frontend
rg -n "dangerouslySetInnerHTML" apps/web/src/app/\(main\)/dashboard/newsletters/\[slug\]/page.tsx -B5 -A5Length of output: 258
Let me search for the actual file locations:
---
🏁 Script executed:
```shell
#!/bin/bash
# Find newsletter-related files
fd -t f "page.tsx" | grep -i newsletter
fd -t f "route.ts" | grep -i newsletter
# Search for any files containing dangerouslySetInnerHTML
rg -l "dangerouslySetInnerHTML" --type tsx
# Search for marked usage
rg -l "marked" --type ts --type tsx
Length of output: 263
---
🏁 Script executed:
```shell
#!/bin/bash
# Read the API route file
echo "=== API ROUTE FILE ==="
cat "apps/web/src/app/api/newsletters/[slug]/route.ts"
echo -e "\n=== PAGE COMPONENT FILE ==="
cat "apps/web/src/app/(main)/dashboard/newsletters/[slug]/page.tsx"
echo -e "\n=== SEARCH FOR SANITIZATION ==="
rg -n "sanitize|DOMPurify|escape|xss" apps/web/src/app/ -g "*.ts" -g "*.tsx"
Length of output: 6984
Implement HTML sanitization to prevent XSS attacks.
The security concern is valid. The API route (apps/web/src/app/api/newsletters/[slug]/route.ts line 41) parses markdown with marked.parse() without any sanitization, and the frontend component (page.tsx lines 141-144) renders this unsanitized HTML with dangerouslySetInnerHTML. This creates an XSS vulnerability where malicious JavaScript embedded in markdown content would execute.
Recommended fix:
- Add
DOMPurifyorsanitize-htmlto sanitize the HTML output frommarked.parse()in the API route before returning it to the client.
Alternatively, sanitize on the frontend before rendering, though server-side sanitization is the recommended security practice.
🧰 Tools
🪛 ast-grep (0.39.9)
[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)
[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)
| <div className="p-3 bg-gradient-to-br from-[#a472ea]/20 to-[#7150e7]/20 rounded-full"> | ||
| <LockClosedIcon className="size-10 text-[#a472ea]" /> | ||
| </div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add aria-hidden to decorative icon.
The lock icon is decorative and should be hidden from screen readers to avoid announcing redundant information.
Apply this diff:
- <LockClosedIcon className="size-10 text-[#a472ea]" />
+ <LockClosedIcon className="size-10 text-[#a472ea]" aria-hidden="true" />📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div className="p-3 bg-gradient-to-br from-[#a472ea]/20 to-[#7150e7]/20 rounded-full"> | |
| <LockClosedIcon className="size-10 text-[#a472ea]" /> | |
| </div> | |
| <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" /> | |
| </div> |
🤖 Prompt for AI Agents
In apps/web/src/components/newsletters/PremiumUpgradePrompt.tsx around lines 14
to 16, the LockClosedIcon is purely decorative and should be hidden from
assistive tech — add aria-hidden="true" to the LockClosedIcon element
(optionally also add focusable="false" for SVGs) so screen readers ignore it
while leaving the visual intact.
| <PrimaryButton | ||
| onClick={() => router.push("/pricing")} | ||
| classname="w-full px-6" | ||
| > | ||
| <SparklesIcon className="size-4" /> | ||
| Unlock Premium | ||
| </PrimaryButton> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add aria-hidden to decorative icon in button.
The sparkles icon is decorative and should be hidden from screen readers since the button text "Unlock Premium" already conveys the action.
Apply this diff:
<PrimaryButton
onClick={() => router.push("/pricing")}
classname="w-full px-6"
>
- <SparklesIcon className="size-4" />
+ <SparklesIcon className="size-4" aria-hidden="true" />
Unlock Premium
</PrimaryButton>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <PrimaryButton | |
| onClick={() => router.push("/pricing")} | |
| classname="w-full px-6" | |
| > | |
| <SparklesIcon className="size-4" /> | |
| Unlock Premium | |
| </PrimaryButton> | |
| <PrimaryButton | |
| onClick={() => router.push("/pricing")} | |
| classname="w-full px-6" | |
| > | |
| <SparklesIcon className="size-4" aria-hidden="true" /> | |
| Unlock Premium | |
| </PrimaryButton> |
🤖 Prompt for AI Agents
In apps/web/src/components/newsletters/PremiumUpgradePrompt.tsx around lines 27
to 33, the SparklesIcon inside the PrimaryButton is decorative and should be
hidden from assistive tech; add the aria-hidden="true" attribute to the
SparklesIcon element (e.g., SparklesIcon aria-hidden="true") so screen readers
skip the icon while leaving the visible text unchanged.
| function InputGroupAddon({ | ||
| className, | ||
| align = "inline-start", | ||
| ...props | ||
| }: React.ComponentProps<"div"> & VariantProps<typeof inputGroupAddonVariants>) { | ||
| return ( | ||
| <div | ||
| role="group" | ||
| data-slot="input-group-addon" | ||
| data-align={align} | ||
| className={cn(inputGroupAddonVariants({ align }), className)} | ||
| onClick={(e) => { | ||
| if ((e.target as HTMLElement).closest("button")) { | ||
| return | ||
| } | ||
| e.currentTarget.parentElement?.querySelector("input")?.focus() | ||
| }} | ||
| {...props} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addon focus logic ignores textarea-based groups
InputGroupAddon currently focuses the first input in the parent:
e.currentTarget.parentElement?.querySelector("input")?.focus()This works for InputGroupInput but fails when using InputGroupTextarea, since there is no input element. Both controls already expose data-slot="input-group-control", so you can target that instead and support both input and textarea:
- e.currentTarget.parentElement?.querySelector("input")?.focus()
+ e.currentTarget.parentElement
+ ?.querySelector<HTMLElement>('[data-slot="input-group-control"]')
+ ?.focus()This also future‑proofs the addon for any other control types that use the same slot.
🤖 Prompt for AI Agents
In apps/web/src/components/ui/input-group.tsx around lines 60 to 77, the addon
click handler currently focuses only the first <input> which breaks
textarea-based groups; change the selector to target the shared slot attribute
instead (query the parent for the element with data-slot="input-group-control")
and call focus() on that element if present, ensuring you safely handle
possibly-null results and cast the found Element to an HTMLElement (or use a
type guard) before invoking focus so both inputs and textareas (and future
controls using the slot) will receive focus.
| switch (timeFilter) { | ||
| case "day": | ||
| return newsletterDate >= today; | ||
| case "week": | ||
| const weekAgo = new Date(today); | ||
| weekAgo.setDate(weekAgo.getDate() - 7); | ||
| return newsletterDate >= weekAgo; | ||
| case "month": | ||
| const monthAgo = new Date(today); | ||
| monthAgo.setMonth(monthAgo.getMonth() - 1); | ||
| return newsletterDate >= monthAgo; | ||
| default: | ||
| return true; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wrap variable declarations in blocks within switch cases.
Variable declarations weekAgo and monthAgo inside switch cases should be wrapped in blocks to prevent scope leakage to other cases.
As per static analysis hints
Apply this diff:
switch (timeFilter) {
case "day":
return newsletterDate >= today;
- case "week":
+ case "week": {
const weekAgo = new Date(today);
weekAgo.setDate(weekAgo.getDate() - 7);
return newsletterDate >= weekAgo;
- case "month":
+ }
+ case "month": {
const monthAgo = new Date(today);
monthAgo.setMonth(monthAgo.getMonth() - 1);
return newsletterDate >= monthAgo;
+ }
default:
return true;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| switch (timeFilter) { | |
| case "day": | |
| return newsletterDate >= today; | |
| case "week": | |
| const weekAgo = new Date(today); | |
| weekAgo.setDate(weekAgo.getDate() - 7); | |
| return newsletterDate >= weekAgo; | |
| case "month": | |
| const monthAgo = new Date(today); | |
| monthAgo.setMonth(monthAgo.getMonth() - 1); | |
| return newsletterDate >= monthAgo; | |
| default: | |
| return true; | |
| } | |
| switch (timeFilter) { | |
| case "day": | |
| return newsletterDate >= today; | |
| case "week": { | |
| const weekAgo = new Date(today); | |
| weekAgo.setDate(weekAgo.getDate() - 7); | |
| return newsletterDate >= weekAgo; | |
| } | |
| case "month": { | |
| const monthAgo = new Date(today); | |
| monthAgo.setMonth(monthAgo.getMonth() - 1); | |
| return newsletterDate >= monthAgo; | |
| } | |
| default: | |
| return true; | |
| } |
🧰 Tools
🪛 Biome (2.1.2)
[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)
🤖 Prompt for AI Agents
In apps/web/src/hooks/useNewsletterFilters.ts around lines 31 to 44, the switch
cases declare weekAgo and monthAgo without block scopes which may leak their
bindings; wrap the "week" case body in { ... } and the "month" case body in {
... } so each const is scoped to its case, keeping the existing logic (compute
weekAgo/monthAgo from today and return comparison) intact and leave default and
"day" cases unchanged.
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-14_15-43-27.mp4
2025-11-15_13-36-29.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.