-
Notifications
You must be signed in to change notification settings - Fork 139
add newsletter feature for premium users #157
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
- implemented newsletter list page with search and filtering - created individual newsletter reading pages - added 3 demo newsletters with rich content - included comprehensive documentation - premium-only access with subscription gate - added newsletter link to sidebar
|
@manojk0303 is attempting to deploy a commit to the AJEET PRATAP SINGH's projects Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThis pull request introduces a complete newsletter feature for premium subscribers. It adds data structure definitions, two new page components for listing and viewing individual newsletters with premium gating and content rendering, updates sidebar navigation with a newsletters link, and includes comprehensive feature documentation. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Sidebar
participant NewsletterList
participant Auth as useSubscription
participant NewsletterDetail
participant Data as newsletters.ts
User->>Sidebar: Browse app
Sidebar->>Auth: Check isPaidUser
alt User is paid
Auth-->>Sidebar: isPaidUser = true
Sidebar-->>User: Show Newsletters link
User->>NewsletterList: Click Newsletters
NewsletterList->>Auth: Check isPaidUser
Auth-->>NewsletterList: isPaidUser = true
NewsletterList->>Data: Fetch newsletter data
Data-->>NewsletterList: Return newsletters array
NewsletterList-->>User: Render filtered/grouped list
User->>NewsletterDetail: Click newsletter item
NewsletterDetail->>Auth: Check isPaidUser
Auth-->>NewsletterDetail: isPaidUser = true
NewsletterDetail->>Data: Fetch newsletter by slug
Data-->>NewsletterDetail: Return newsletter object
NewsletterDetail-->>User: Render detail with HTML content
else User is not paid
Auth-->>Sidebar: isPaidUser = false
Sidebar-->>User: Hide Newsletters link
User->>NewsletterList: Direct navigation to /newsletters
NewsletterList->>Auth: Check isPaidUser
Auth-->>NewsletterList: isPaidUser = false
NewsletterList-->>User: Show pricing prompt
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Areas requiring attention:
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
|
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: 2
🧹 Nitpick comments (4)
apps/web/src/components/dashboard/Sidebar.tsx (1)
20-27: Premium gating in Sidebar is correct; consider encoding it in route config instead of a path checkThe
useSubscriptionintegration andisPaidUsercheck correctly hide the Newsletters link for non‑paid users, consistent with the page-level gate.To reduce the brittle
route.path === "/newsletters"string check, consider adding apremiumOnly: trueflag on the route config and checking that instead duringSIDEBAR_ROUTES.map, so future path changes don’t silently break gating.Also applies to: 40-44, 57-57, 125-127
apps/web/src/data/newsletters.ts (1)
3-11: Newsletter type matches usage but is out of sync with the guide’sreadTimeexampleThe
Newsletterinterface lines up with how both pages consume the data (id/date/title/slug/excerpt/content/tags). Two small alignment points:
The guide shows
readTime: 6 // optional, but the interface doesn’t definereadTime. Decide whether you wantreadTime?: numberas part of the model (and potentially used in the UI) or remove it from the docs to avoid confusion for contributors adding entries.The
datecomment states"DD-MM-YY"; both pages assume this exact format when grouping and sorting. It’s worth keeping the comment accurate and stable since any format change will breaksplit("-")parsing in the page helpers.apps/web/src/app/(main)/newsletters/page.tsx (1)
249-254: Read‑time calculation is duplicated; consider extracting a shared helper
calculateReadTimehere duplicates the implementation inapps/web/src/app/(main)/newsletters/[slug]/page.tsx. To keep behavior consistent and avoid future drift, consider extracting this into a small shared utility (e.g.,@/lib/newsletters/calculateReadTime) and reusing it in both pages.apps/web/src/app/(main)/newsletters/[slug]/page.tsx (1)
235-239: Share the read‑time helper with the listing page to avoid divergence
calculateReadTimehere is identical in behavior to the helper inapps/web/src/app/(main)/newsletters/page.tsx. To keep the read‑time estimate consistent and maintainable, you can move this function into a shared util (for example@/lib/newsletters/calculateReadTime) and import it in both places.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
NEWSLETTER_GUIDE.md(1 hunks)apps/web/src/app/(main)/newsletters/[slug]/page.tsx(1 hunks)apps/web/src/app/(main)/newsletters/page.tsx(1 hunks)apps/web/src/components/dashboard/Sidebar.tsx(4 hunks)apps/web/src/data/newsletters.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
apps/web/src/app/(main)/newsletters/[slug]/page.tsx (2)
apps/web/src/hooks/useSubscription.ts (1)
useSubscription(11-75)apps/web/src/data/newsletters.ts (1)
newsletters(13-245)
apps/web/src/app/(main)/newsletters/page.tsx (2)
apps/web/src/hooks/useSubscription.ts (1)
useSubscription(11-75)apps/web/src/data/newsletters.ts (1)
newsletters(13-245)
apps/web/src/components/dashboard/Sidebar.tsx (1)
apps/web/src/hooks/useSubscription.ts (1)
useSubscription(11-75)
🪛 ast-grep (0.39.9)
apps/web/src/app/(main)/newsletters/[slug]/page.tsx
[warning] 114-114: 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)/newsletters/[slug]/page.tsx
[error] 115-115: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
🪛 markdownlint-cli2 (0.18.1)
NEWSLETTER_GUIDE.md
138-138: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (2)
apps/web/src/app/(main)/newsletters/[slug]/page.tsx (2)
10-17: Theuse(params)pattern is correct for Next.js 15+ App Router client components.In Next.js 15+, the App Router passes params as a Promise, and client-mode page components must unwrap that Promise using React's
use(params). This pattern is documented and recommended by Next.js.Your implementation follows the official guidance and is the proper way to handle route parameters in client components. No changes needed for this aspect of the code.
Likely an incorrect or invalid review comment.
112-116: Current usage ofdangerouslySetInnerHTMLis acceptable for trusted local data, but implement sanitization if content source changesThe review comment is accurate. Verification confirms:
- Current state: Newsletter content is 100% static, locally defined in
apps/web/src/data/newsletters.ts. No CMS integrations or dynamic content fetching detected.- Current safety: Using
dangerouslySetInnerHTMLwith this trusted, static data is acceptable.- Future-proofing guidance: Your warning is well-placed. Current React/Next.js 15 guidance recommends sanitizing HTML before rendering via
dangerouslySetInnerHTML—either using DOMPurify (client-side), sanitize-html (server-side), or rehype-sanitize (in build pipelines).- Recommended pattern: If content ever moves to a CMS or becomes user-supplied, sanitize as early as practical (server or build time) before marking content "trusted" for rendering.
No sanitization libraries are currently in dependencies, and no modifications to
newsletter.contentexist elsewhere in the codebase. The preventive guidance is sound.
| const filteredNewsletters = useMemo(() => { | ||
| let filtered = newsletters; | ||
|
|
||
| // filter by search | ||
| if (searchQuery) { | ||
| const query = searchQuery.toLowerCase(); | ||
| filtered = filtered.filter(n => | ||
| n.title.toLowerCase().includes(query) || | ||
| n.excerpt.toLowerCase().includes(query) || | ||
| n.tags.some(t => t.toLowerCase().includes(query)) | ||
| ); | ||
| } | ||
|
|
||
| // filter by month | ||
| if (selectedMonth !== "all") { | ||
| filtered = filtered.filter(n => { | ||
| const [day, month, year] = n.date.split("-"); | ||
| const monthYear = `${month}-20${year}`; | ||
| return monthYear === selectedMonth; | ||
| }); | ||
| } | ||
|
|
||
| // sort by date (latest first) | ||
| return filtered.sort((a, b) => { | ||
| const dateA = parseDate(a.date); | ||
| const dateB = parseDate(b.date); | ||
| return dateB.getTime() - dateA.getTime(); | ||
| }); | ||
| }, [searchQuery, selectedMonth]); |
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.
Avoid mutating the shared newsletters array when sorting
filteredNewsletters initializes with let filtered = newsletters; and then returns filtered.sort(...). Because arrays are reference types, this mutates the original newsletters array in place.
Safer pattern:
const filteredNewsletters = useMemo(() => {
let filtered = newsletters;
// ...search + month filters...
return [...filtered].sort((a, b) => {
const dateA = parseDate(a.date);
const dateB = parseDate(b.date);
return dateB.getTime() - dateA.getTime();
});
}, [searchQuery, selectedMonth]);This keeps newsletters immutable and avoids subtle ordering side-effects elsewhere.
🤖 Prompt for AI Agents
In apps/web/src/app/(main)/newsletters/page.tsx around lines 30 to 58, the code
assigns let filtered = newsletters and then calls filtered.sort(...), which
mutates the original newsletters array; to fix this, make sure to avoid in-place
mutation by sorting a copy (e.g., use [...filtered] or filtered.slice() before
sort) so the original newsletters remains immutable and return that sorted copy
from the useMemo.
| ```typescript | ||
| { | ||
| id: "4", // increment from last id | ||
| date: "DD-MM-YY", // e.g., "20-11-25" for nov 20, 2025 | ||
| title: "your newsletter title", | ||
| slug: "url-friendly-slug", // lowercase, hyphens only | ||
| excerpt: "a compelling 1-2 sentence description", | ||
| content: ```html | ||
| <h2>main section heading</h2> | ||
| <p>your paragraph content here. use <strong>bold text</strong> for emphasis.</p> | ||
|
|
||
| <img src="https://your-image-url.com/image.jpg" alt="descriptive alt text" /> | ||
|
|
||
| <p>add <a href="https://example.com">external links</a> for references.</p> | ||
|
|
||
| <h3>subsection heading</h3> | ||
| <ul> | ||
| <li><strong>list item title</strong> - description of the item</li> | ||
| <li><strong>another item</strong> - more details here</li> | ||
| </ul> | ||
| ```, | ||
| tags: ["relevant", "topic", "tags"], | ||
| readTime: 6 // optional: estimated minutes to read | ||
| } | ||
| ``` |
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.
Fix TS example and fenced block language to avoid copy‑paste issues and markdownlint errors
-
In the TypeScript snippet, `content: ```html ... ````, is not valid TS. It should be shown as a string/template literal, e.g.:
content: ` <h2>main section heading</h2> ... `,
and you can keep the separate HTML examples below for formatting guidance rather than nesting ```html inside the TS example.
-
The file-structure fenced block starting at Line 138 should specify a language (e.g.
```textor```bash) to satisfy MD040. -
The example includes
readTime: 6butNewsletterinapps/web/src/data/newsletters.tsdoesn’t define areadTimefield. Either addreadTime?: numberto the interface (and optionally use it), or remove it from the example so users don’t hit TS excess-property errors.
Also applies to: 138-145
newsletter feature implementation
overview
implemented a complete newsletter system for premium users on opensox.ai, featuring organized content by date, rich formatting support, and an elegant reading experience.
features implemented
core functionality
ui/ux enhancements
files added
demo video
https://www.loom.com/share/be672f86641947fd8a46891a46775577
documentation
included comprehensive
NEWSLETTER_GUIDE.mdwith:access control
useSubscriptionhooktesting checklist
implementation approach
how to add a newsletter
apps/web/src/data/newsletters.tsnewslettersarrayNEWSLETTER_GUIDE.mdfor detailed instructionsthis pr is ready for review. all functionality is complete, tested, and documented.
looking forward to your feedback! 🙏
Summary by CodeRabbit
New Features
Documentation