feat(newsletters): added newsletter ui and functionality #155 #168
Conversation
|
@Ankit-69k 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. |
WalkthroughAdds a newsletter feature: Markdown newsletter content, parsing utilities, types, list and detail Next.js app routes, dashboard UI components (cards, grid, markdown viewer, input/select), subscription gating, sidebar route, and sample newsletter markdown files. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ListPage as Newsletter List Page
participant Utils as getMarkdownNewsletter
participant Container as NewsLetterContainer
participant DetailPage as Newsletter Detail Page
participant Subscription as useSubscription
User->>ListPage: GET /dashboard/newsletter
ListPage->>Utils: getAllNewsletters()
Utils-->>ListPage: NewsletterProps[]
ListPage->>Container: render(newsletters)
Container->>Container: filter by search/year/month
Container-->>User: render grid of NewsletterCard
User->>DetailPage: GET /dashboard/newsletter/:slug
DetailPage->>Utils: getNewsletter(slug)
Utils-->>DetailPage: NewsletterProps | null
DetailPage->>Subscription: check subscription status
alt has subscription
DetailPage-->>User: render NewsletterView (uses MarkdownViewer)
else no subscription
DetailPage-->>User: render PremiumException
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
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 |
Ankit-69k
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (5)
apps/web/src/constant/newsletters/november-2024-tutorial-series.md (2)
293-296: Add language identifier to the code block.The fenced code block should specify a language for proper syntax highlighting.
Apply this diff:
-``` +```gitignore .env .env.local--- `362-364`: **Consider using angle brackets for the API key placeholder.** The static analysis tool flagged this as a potential security issue, though it's clearly a placeholder. To make it more obvious and avoid future confusion, consider using angle bracket notation. Apply this diff: ```diff # Verify your API key -curl -H "Authorization: Bearer YOUR_API_KEY" \ +curl -H "Authorization: Bearer <YOUR_API_KEY>" \ https://api.opensox.com/v1/verifyapps/web/src/constant/newsletters/december-2024-year-review.md (1)
1-230: Content and front matter are correct; style nits are optionalFront matter shape and content structure look good for the renderer. The LanguageTool suggestions (en dash instead of hyphen in date ranges, more formal alternative to “amazing”) are purely stylistic — feel free to adopt them, but they aren’t required.
apps/web/src/constant/newsletters/october-2024-security-update-new.md (1)
295-314: Double-check markdown fences if markdownlint MD040 is failingStatic analysis reports MD040 (missing language on a fenced code block). In the snippet here all visible fences already specify a language, so this may be a line-numbering mismatch or another fence elsewhere in the file. If markdownlint is blocking CI, it’s worth scanning for any bare ``` fences and adding a language tag.
apps/web/src/utils/getMarkdownNewsletter.ts (1)
34-34: Improve excerpt fallback logic.The fallback excerpt always appends "..." even when the content is shorter than 150 characters, which can look awkward.
Apply this diff:
- excerpt: data.excerpt || content.slice(0, 150) + "...", + excerpt: data.excerpt || (content.length > 150 ? content.slice(0, 150) + "..." : content.trim()),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
apps/web/package.json(1 hunks)apps/web/src/app/(main)/dashboard/layout.tsx(1 hunks)apps/web/src/app/(main)/dashboard/newsletter/[slug]/page.tsx(1 hunks)apps/web/src/app/(main)/dashboard/newsletter/page.tsx(1 hunks)apps/web/src/components/dashboard/MarkdownViewer.tsx(1 hunks)apps/web/src/components/dashboard/NewsLetterCard.tsx(1 hunks)apps/web/src/components/dashboard/NewsLetterContainer.tsx(1 hunks)apps/web/src/components/dashboard/NewsletterView.tsx(1 hunks)apps/web/src/components/dashboard/PremiumException.tsx(1 hunks)apps/web/src/components/dashboard/Sidebar.tsx(2 hunks)apps/web/src/components/ui/input.tsx(1 hunks)apps/web/src/components/ui/select.tsx(1 hunks)apps/web/src/constant/newsletters/december-2024-year-review.md(1 hunks)apps/web/src/constant/newsletters/january-2025-product-updates.md(1 hunks)apps/web/src/constant/newsletters/november-2024-tutorial-series.md(1 hunks)apps/web/src/constant/newsletters/october-2024-security-update-new.md(1 hunks)apps/web/src/constant/newsletters/october-2024-security-update.md(1 hunks)apps/web/src/data/newsletter.ts(1 hunks)apps/web/src/types/newsletter.ts(1 hunks)apps/web/src/utils/getMarkdownNewsletter.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (11)
apps/web/src/components/dashboard/PremiumException.tsx (1)
apps/web/src/components/ui/card.tsx (5)
CardHeader(78-78)CardContent(82-82)props(35-41)props(8-17)props(67-73)
apps/web/src/app/(main)/dashboard/newsletter/page.tsx (2)
apps/web/src/utils/getMarkdownNewsletter.ts (1)
getAllNewsletters(8-45)apps/web/src/components/dashboard/NewsLetterContainer.tsx (1)
NewsLetterContainer(19-180)
apps/web/src/components/ui/input.tsx (1)
apps/web/src/lib/utils.ts (1)
cn(4-6)
apps/web/src/types/newsletter.ts (3)
apps/web/src/data/blogs.ts (1)
BlogPost(8-13)apps/web/src/store/useProjectTitleStore.ts (1)
ProjectTitleProps(3-6)apps/web/src/components/faq/faqData.ts (1)
FAQ(1-4)
apps/web/src/components/dashboard/NewsLetterContainer.tsx (5)
apps/web/src/types/newsletter.ts (1)
NewsletterProps(3-10)apps/web/src/hooks/useSubscription.ts (1)
useSubscription(11-75)apps/web/src/data/newsletter.ts (1)
newsletters(3-727)apps/web/src/components/dashboard/NewsLetterCard.tsx (2)
NewsletterGrid(53-59)NewsletterCard(10-50)apps/web/src/components/ui/ErrMsg.tsx (1)
ErrMsg(1-7)
apps/web/src/utils/getMarkdownNewsletter.ts (2)
apps/web/src/types/newsletter.ts (1)
NewsletterProps(3-10)apps/web/src/data/newsletter.ts (1)
newsletters(3-727)
apps/web/src/app/(main)/dashboard/newsletter/[slug]/page.tsx (2)
apps/web/src/utils/getMarkdownNewsletter.ts (1)
getNewsletter(47-70)apps/web/src/components/dashboard/NewsletterView.tsx (1)
NewsletterView(15-83)
apps/web/src/components/ui/select.tsx (1)
apps/web/src/lib/utils.ts (1)
cn(4-6)
apps/web/src/components/dashboard/NewsletterView.tsx (3)
apps/web/src/types/newsletter.ts (1)
NewsletterProps(3-10)apps/web/src/hooks/useSubscription.ts (1)
useSubscription(11-75)apps/web/src/components/dashboard/MarkdownViewer.tsx (1)
MarkdownViewer(11-114)
apps/web/src/data/newsletter.ts (1)
apps/web/src/types/newsletter.ts (1)
NewsletterProps(3-10)
apps/web/src/components/dashboard/NewsLetterCard.tsx (2)
apps/web/src/types/newsletter.ts (1)
NewsletterProps(3-10)apps/web/src/components/ui/card.tsx (7)
CardHeader(78-78)CardContent(82-82)props(67-73)props(23-29)props(59-61)props(8-17)props(35-41)
🪛 Gitleaks (8.29.0)
apps/web/src/constant/newsletters/october-2024-security-update.md
[high] 247-247: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
apps/web/src/constant/newsletters/october-2024-security-update-new.md
[high] 247-247: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
apps/web/src/constant/newsletters/november-2024-tutorial-series.md
[high] 363-363: Discovered a potential authorization token provided in a curl command header, which could compromise the curl accessed resource.
(curl-auth-header)
🪛 LanguageTool
apps/web/src/constant/newsletters/december-2024-year-review.md
[typographical] ~26-~26: Consider using an en dash here instead of a hyphen.
Context: ...estones ### Q1: Foundation Building January - March 2024 In the first quarter, we focuse...
(QB_NEW_EN_DASH_RULE_EN)
[typographical] ~54-~54: Consider using an en dash here instead of a hyphen.
Context: ...ved 99.9% uptime ### Q2: Scaling Up April - June 2024 The second quarter was all abou...
(QB_NEW_EN_DASH_RULE_EN)
[typographical] ~69-~69: Consider using an en dash here instead of a hyphen.
Context: ... 5x faster ### Q3: Enterprise Ready July - September 2024 We became enterprise-ready with...
(QB_NEW_EN_DASH_RULE_EN)
[typographical] ~96-~96: Consider using an en dash here instead of a hyphen.
Context: ...ed ``` ### Q4: Innovation Unleashed October - December 2024 The final quarter brought cutti...
(QB_NEW_EN_DASH_RULE_EN)
[style] ~205-~205: Consider using a more formal and expressive alternative to ‘amazing’.
Context: ...ns clear: Empower developers to build amazing products faster. In 2025, we're focu...
(AWESOME)
apps/web/src/constant/newsletters/january-2025-product-updates.md
[style] ~26-~26: Consider a different adjective to strengthen your wording.
Context: ... ### Advanced Analytics Dashboard Get deeper insights into your data with our new an...
(DEEP_PROFOUND)
[style] ~133-~133: Consider using a more formal and expressive alternative to ‘amazing’.
Context: ...art of the OpenSox community. Here's to an amazing 2025! The OpenSox Team 💜
(AWESOME)
[style] ~133-~133: Using many exclamation marks might seem excessive (in this case: 5 exclamation marks for a text that’s 3312 characters long)
Context: ...Sox community. Here's to an amazing 2025! The OpenSox Team 💜
(EN_EXCESSIVE_EXCLAMATION)
apps/web/src/constant/newsletters/november-2024-tutorial-series.md
[style] ~416-~416: Using many exclamation marks might seem excessive (in this case: 5 exclamation marks for a text that’s 2935 characters long)
Context: ... are happy to help! --- Happy coding! 💜 _Got stuck? Check out the complet...
(EN_EXCESSIVE_EXCLAMATION)
🪛 markdownlint-cli2 (0.18.1)
apps/web/src/constant/newsletters/october-2024-security-update.md
321-321: Bare URL used
(MD034, no-bare-urls)
397-397: Bare URL used
(MD034, no-bare-urls)
405-405: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
apps/web/src/constant/newsletters/october-2024-security-update-new.md
293-293: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
apps/web/src/constant/newsletters/november-2024-tutorial-series.md
293-293: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (14)
apps/web/src/app/(main)/dashboard/layout.tsx (1)
29-29: LGTM!This formatting change is purely cosmetic with no functional impact on the dashboard layout.
apps/web/src/components/dashboard/Sidebar.tsx (2)
20-20: LGTM!The NewspaperIcon import is correctly added from @heroicons/react.
39-43: LGTM!The Newsletter route is correctly added to the sidebar navigation, following the established pattern for other routes.
apps/web/src/components/ui/input.tsx (1)
5-22: LGTM!The Input component follows React best practices with proper ref forwarding, type safety, and composable className patterns. The implementation is clean and reusable.
apps/web/src/components/ui/select.tsx (1)
1-159: LGTM!The Select component is well-implemented using Radix UI primitives with proper accessibility, animations, and type safety. The implementation follows established patterns and best practices.
apps/web/src/components/dashboard/PremiumException.tsx (1)
6-51: LGTM!The PremiumException component is well-structured with clear messaging, proper accessibility, and consistent styling. The UI effectively communicates the premium feature gate with appropriate CTAs.
apps/web/package.json (1)
29-29: All three markdown dependencies are current and secure.Verification confirms:
- gray-matter 4.0.3 is the latest version, no known vulnerabilities
- react-markdown 10.1.0 is the latest version, no known vulnerabilities, compatible with React 18
- remark-gfm 4.0.1 is the latest version, no known vulnerabilities
All packages are actively maintained and free from reported security issues.
apps/web/src/components/dashboard/NewsletterView.tsx (1)
15-82: NewsletterView gating and layout look solidLoading, access control, and the main content layout are wired cleanly, and the MarkdownViewer integration is straightforward. No issues from a correctness or UX perspective in this component.
apps/web/src/components/dashboard/MarkdownViewer.tsx (1)
11-113: Markdown rendering and styling are well-structuredThe MarkdownViewer composition, including GFM support, custom element mappings, and link hardening, is clear and appropriate for newsletter content. No functional or safety concerns here.
apps/web/src/app/(main)/dashboard/newsletter/[slug]/page.tsx (1)
4-15: UsenotFound()instead of returning a fallback div for 404 handlingThe code structure is correct for Next 15 App Router—
paramsis properly typed as a Promise and correctly awaited. However, returning a bare HTML div for missing resources bypasses framework-level 404 handling. Import and callnotFound()fromnext/navigationinstead:+import { notFound } from "next/navigation"; import { NewsletterView } from "@/components/dashboard/NewsletterView"; import { getNewsletter } from "@/utils/getMarkdownNewsletter"; export default async function NewsletterViewPage({ params, }: { params: Promise<{ slug: string }>; }) { const { slug } = await params; const newsletter = getNewsletter(slug); - if (!newsletter) return <div className="text-white p-10">Not Found</div>; + if (!newsletter) { + notFound(); + } return <NewsletterView newsletter={newsletter} />; }In Next 15,
paramsis a Promise and should be awaited, andnotFound()fromnext/navigationis the recommended API for rendering 404s inside server components.Likely an incorrect or invalid review comment.
apps/web/src/constant/newsletters/october-2024-security-update.md (1)
246-250: Static analysis false positive - example code is intentional.The static analysis tool flagged line 247 as containing an API key. However, this is intentional example code demonstrating an anti-pattern (marked with "❌ NEVER do this"). The newsletter content correctly shows what developers should avoid.
apps/web/src/constant/newsletters/january-2025-product-updates.md (1)
1-135: LGTM! Newsletter content looks good.The static analysis flagged minor stylistic suggestions (word choice, exclamation marks), but these are appropriate for marketing/newsletter content and don't represent actual issues.
apps/web/src/components/dashboard/NewsLetterCard.tsx (1)
10-59: LGTM! Clean implementation with good accessibility practices.The components are well-structured with proper semantic HTML (using
<time>withdateTimeattribute), responsive design, and clean styling. The card interactions and grid layout are implemented correctly.apps/web/src/data/newsletter.ts (1)
3-727: Thedata/newsletter.tsfile is dead code and should be removed or clarified.Verification shows the file has zero imports in the codebase. The actual newsletter data comes from
apps/web/src/utils/getMarkdownNewsletter.tsvia thegetAllNewsletters()function, which reads markdown files fromsrc/constant/newsletters/.The type mismatch you flagged (entries 1 & 2 have
contentas objects; entry 3 as string, whileNewsletterProps.content: string) is real but has no runtime impact since this code is never executed.Action: Remove
apps/web/src/data/newsletter.tsif it's legacy/test data, or clarify its purpose in comments if it should be kept as reference/example code. If retained, updateNewsletterPropstype to support both object and string content, or normalize all entries to match the type definition.Likely an incorrect or invalid review comment.
| function NewsletterPage() { | ||
| const newsletters = getAllNewsletters(); | ||
| return ( | ||
| <div className=" "> | ||
| <NewsLetterContainer newsletters={newsletters} /> | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Make NewsletterPage an async Server Component.
Calling getAllNewsletters() synchronously in the component body performs blocking filesystem I/O. In Next.js 15 with the App Router, page components should be async when performing I/O operations to avoid blocking the rendering thread.
Apply this diff to make it an async Server Component:
-function NewsletterPage() {
- const newsletters = getAllNewsletters();
+async function NewsletterPage() {
+ const newsletters = await getAllNewsletters();
return (
<div className=" ">
<NewsLetterContainer newsletters={newsletters} />Note: This also requires getAllNewsletters() to be made async. Verify that apps/web/src/utils/getMarkdownNewsletter.ts exports an async version or update it accordingly.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/web/src/app/(main)/dashboard/newsletter/page.tsx around lines 6 to 13,
the component currently calls getAllNewsletters() synchronously which causes
blocking I/O; make the page an async Server Component by declaring the component
async and awaiting getAllNewsletters(), i.e., change the function to async, use
const newsletters = await getAllNewsletters(), and return the same JSX; also
ensure apps/web/src/utils/getMarkdownNewsletter.ts exports an async
getAllNewsletters (or update its implementation to return a Promise) so the
awaited call works without runtime errors.
| return { | ||
| id: slug, | ||
| slug, | ||
| title: data.title || slug, | ||
| date: data.date ? new Date(data.date) : new Date(), | ||
| excerpt: data.excerpt || content.slice(0, 150) + "...", | ||
| content, | ||
| } as NewsletterProps; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Apply the same date validation and excerpt improvements here.
The same date validation and excerpt fallback issues identified in getAllNewsletters (lines 29-36) also apply to this function. Please apply similar fixes to ensure consistency.
Apply this diff:
return {
id: slug,
slug,
title: data.title || slug,
- date: data.date ? new Date(data.date) : new Date(),
- excerpt: data.excerpt || content.slice(0, 150) + "...",
+ date: data.date && !isNaN(new Date(data.date).getTime())
+ ? new Date(data.date)
+ : new Date(),
+ excerpt: data.excerpt || (content.length > 150 ? content.slice(0, 150) + "..." : content.trim()),
content,
} as NewsletterProps;📝 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.
| return { | |
| id: slug, | |
| slug, | |
| title: data.title || slug, | |
| date: data.date ? new Date(data.date) : new Date(), | |
| excerpt: data.excerpt || content.slice(0, 150) + "...", | |
| content, | |
| } as NewsletterProps; | |
| return { | |
| id: slug, | |
| slug, | |
| title: data.title || slug, | |
| date: data.date && !isNaN(new Date(data.date).getTime()) | |
| ? new Date(data.date) | |
| : new Date(), | |
| excerpt: data.excerpt || (content.length > 150 ? content.slice(0, 150) + "..." : content.trim()), | |
| content, | |
| } as NewsletterProps; |
🤖 Prompt for AI Agents
In apps/web/src/utils/getMarkdownNewsletter.ts around lines 58 to 65, the
returned object uses a naive date and excerpt fallback; update it to mirror
getAllNewsletters fixes by: validate data.date with Date.parse (only create new
Date(data.date) when parse succeeds, otherwise leave date undefined/null), and
improve excerpt fallback by stripping markdown/HTML from content, trimming
whitespace, taking up to 150 characters without cutting mid-word (truncate at
the last space before the limit), and append "..." only when the content was
actually truncated; keep types consistent with NewsletterProps.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
apps/web/src/constant/newsletters/october-2024-security-update-new.md (2)
322-322: Optional: Wrap bare email addresses to satisfy markdown linting.Lines 322 and 398 contain bare email addresses (
security@opensox.com) that triggerMD034 (no-bare-urls)in markdownlint. While these render fine in most markdown viewers, you can wrap them in angle brackets for stricter linting compliance:- Instead, email us at: **security@opensox.com** + Instead, email us at: **<security@opensox.com>**and
- 📧 Email: security@opensox.com + 📧 Email: <security@opensox.com>This is purely stylistic and optional given the "chill" review stance.
Also applies to: 398-398
406-406: Optional: Style final metadata line as a comment or block-level element.Line 406 uses emphasis (
_Last updated: October 25, 2024_) which markdownlint flags asMD036 (no-emphasis-as-heading). Since this is footer metadata rather than semantic content, you could instead use an HTML comment or leave it as-is if your rendering pipeline ignores this lint rule:- _Last updated: October 25, 2024_ + <!-- Last updated: October 25, 2024 -->or simply remove the emphasis:
- _Last updated: October 25, 2024_ + Last updated: October 25, 2024apps/web/src/components/dashboard/NewsLetterContainer.tsx (2)
65-83: Robust grouping and date-based sortingThe grouping by
year-monthkey plus sorting using the first item’sdateper group avoids the earlier locale-dependent parsing issue and gives a deterministic newest‑first order. The type forgroupsis also sound and keepsitemsstrongly typed.If you want to decouple from the prop name, you could replace
items: typeof newsletterswithitems: NewsletterProps[]for slightly clearer intent.
94-153: UI structure, filters, and empty state are well put togetherThe header, search input, year/month selects, and grouped newsletter sections are laid out cleanly and should scale well across breakpoints. The empty state via
ErrMsgis a nice touch for discoverability when filters or search yield no results.If you expect very long lists, consider debouncing the search input (e.g., via a small custom hook) to avoid frequent recomputation on every keystroke, though it’s not required at current scale.
Also applies to: 156-179
apps/web/src/utils/getMarkdownNewsletter.ts (1)
29-39: Date validation properly addressed; minor excerpt improvement possible.The date validation with
isNaNcheck correctly addresses the previous review feedback. The excerpt generation on line 37 could be refined to avoid appending "..." when content is already shorter than 150 characters, but this is a minor concern.Optional improvement for excerpt generation:
- excerpt: data.excerpt || content.slice(0, 150) + "...", + excerpt: data.excerpt || (content.length > 150 ? content.slice(0, 150) + "..." : content.trim()),
📜 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 (6)
apps/web/package.json(2 hunks)apps/web/src/app/(main)/dashboard/newsletter/page.tsx(1 hunks)apps/web/src/components/dashboard/NewsLetterContainer.tsx(1 hunks)apps/web/src/constant/newsletters/october-2024-security-update-new.md(1 hunks)apps/web/src/types/newsletter.ts(1 hunks)apps/web/src/utils/getMarkdownNewsletter.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/web/package.json
- apps/web/src/types/newsletter.ts
- apps/web/src/app/(main)/dashboard/newsletter/page.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
apps/web/src/utils/getMarkdownNewsletter.ts (2)
apps/web/src/types/newsletter.ts (1)
NewsletterProps(1-8)apps/web/src/data/newsletter.ts (1)
newsletters(3-727)
apps/web/src/components/dashboard/NewsLetterContainer.tsx (4)
apps/web/src/types/newsletter.ts (1)
NewsletterProps(1-8)apps/web/src/hooks/useSubscription.ts (1)
useSubscription(11-75)apps/web/src/components/dashboard/NewsLetterCard.tsx (2)
NewsletterGrid(53-59)NewsletterCard(10-50)apps/web/src/components/ui/ErrMsg.tsx (1)
ErrMsg(1-7)
🪛 markdownlint-cli2 (0.18.1)
apps/web/src/constant/newsletters/october-2024-security-update-new.md
322-322: Bare URL used
(MD034, no-bare-urls)
398-398: Bare URL used
(MD034, no-bare-urls)
406-406: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🔇 Additional comments (8)
apps/web/src/constant/newsletters/october-2024-security-update-new.md (2)
245-261: ✅ API key hardcoding issue resolved.The past review flagged hardcoded secret patterns triggering Gitleaks. The current examples now use neutral placeholders (
"YOUR_PRODUCTION_API_KEY_HERE"and"YOUR_SECRET_API_KEY_HERE") instead, which should satisfy secret scanners while maintaining the anti-pattern vs. recommended pattern contrast. This effectively closes the prior concern.
1-405: Newsletter content is comprehensive, well-structured, and actionable.The markdown follows the expected format (front-matter with title/date/excerpt, rich content with code examples, resources). Security guidance demonstrates clear before/after patterns, practical CVE remediation, and organizational best practices. The checklist and compliance updates add significant value. Content integrates well with the newsletter system architecture.
apps/web/src/components/dashboard/NewsLetterContainer.tsx (2)
45-63: Filtering logic and memoization look solidThe
availableYearsandfilteredNewslettersmemos are wired correctly withnewslettersin the dependency arrays, and the filter conditions for year, month, and search term are coherent and type-safe with the"all" | numberstates. This should behave correctly even as the newsletters list changes.
85-93: Subscription gating and loading UX are handled cleanlyEarly-returning on
subscriptionLoadingand then on!isPaidUserkeeps the main render path simple and avoids rendering gated UI unnecessarily. The loader andPremiumExceptionseparation is straightforward and easy to follow.apps/web/src/utils/getMarkdownNewsletter.ts (4)
1-6: LGTM: Imports and path setup are correct.The imports and newsletter path configuration are appropriate for this server-side utility.
11-14: Good defensive error handling.The directory existence check and safe fallback to an empty array provide robust error handling.
61-74: Date validation properly implemented; excerpt handling is improved.The date validation correctly addresses previous review feedback. The excerpt generation here (lines 69-71) properly checks content length before appending "..." which is better than the implementation in
getAllNewsletters.
1-79: The concern in the review comment is based on a misunderstanding of the codebase.Based on my verification:
NewsletterProps.contentis defined asstringtype, not asJSON | HTML | Markdown- The
MarkdownViewercomponent explicitly expectscontent: stringand renders it as markdown usingReactMarkdownwithremarkGfmplugin- The new functions in
getMarkdownNewsletter.tscorrectly returncontentas plain markdown strings, which matches the type definition and is compatible with the componentThe existing newsletter data in
newsletter.tscontains Tiptap/ProseMirror JSON objects assigned tocontent, which is actually a type violation in the existing code (not an issue introduced by your changes). However, the new markdown-based functions are properly aligned with the type definition and the rendering infrastructure.There is no content format compatibility issue to resolve.
|
@Ankit-69k thanks for the submission! Unfortunately, we are moving with a different submission this time, so we won't be able to accept it. Still, you are welcome to make contributions! 🙏 |
2 participants
Fixes #155 — [bounty-to-hire]: create UI for newsletter
What does this PR do?
This PR implements a complete markdown-based newsletter system that allows us to publish, display, and manage newsletters directly from markdown files. This feature is only for the pro user
- core features
- minimal formatting features
How to Add Newsletter ?
Recording for the list view of newletter with filters
Screencast.From.2025-11-16.01-31-53.mp4
Recording for the newletter view
Screencast.From.2025-11-16.01-32-30.mp4
Summary by CodeRabbit