feat: implement newsletters dashboard with filtering and detail views #159
Conversation
- Added main newsletters component with search and month filtering. - Created individual newsletter pages displaying detailed content. - Implemented reusable components for newsletter cards, filters, and empty states. - Introduced utility functions for filtering and grouping newsletters by month. - Added sample newsletter data for testing and display purposes.
|
@prudvinani 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 newsletters feature for the dashboard, including pages and components for browsing newsletters with search and month filtering, a detail page for viewing individual newsletters, utility functions for grouping and date formatting, sample newsletter data with rich content support, new UI components (Input and Select), sidebar navigation integration, and newsletter type definitions. Changes
Sequence DiagramsequenceDiagram
actor User
participant NewslettersPage as Newsletters Page
participant Filters as NewsletterFilters
participant List as NewsletterList
participant Card as NewsletterCard
participant Detail as Newsletter Detail
User->>NewslettersPage: Visit /newsletters
NewslettersPage->>NewslettersPage: Load all newsletters
rect rgb(200, 220, 240)
Note over NewslettersPage,Card: Browsing & Filtering Flow
NewslettersPage->>Filters: Render with initial state
User->>Filters: Search or select month
Filters->>NewslettersPage: onSearchChange / onMonthChange
NewslettersPage->>NewslettersPage: filterNewsletters(query, month)
NewslettersPage->>List: Pass filtered newsletters
List->>List: Group by month & sort
List->>Card: Render NewsletterCard per item
end
rect rgb(240, 220, 200)
Note over Card,Detail: Detail View Flow
User->>Card: Click newsletter card
Card->>Detail: Navigate to /newsletters/[id]
Detail->>Detail: Load newsletter content
Detail->>Detail: formatNewsletterDate()
Detail->>Detail: Render NewsletterContent
Detail->>User: Display formatted newsletter
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
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 |
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (11)
apps/web/src/app/(main)/dashboard/newsletters/page.tsx (2)
8-8: Consider removing the type assertion if data conforms to Newsletter[].The type assertion
as Newsletter[]may be unnecessary if thenewslettersdata already matches theNewsletter[]type. TypeScript should infer the type correctly if the data structure is aligned.If the assertion is needed due to type mismatch, verify the data structure in
data/newsletters.tsaligns with theNewsletterinterface.
7-9: Consider removing the wrapper div.The wrapper
<div>doesn't add semantic value or styling. You can return theNewsletterscomponent directly.export default function NewslettersPage() { - return ( - <div> - <Newsletters newsletters={newsletters as Newsletter[]} /> - </div> - ); + return <Newsletters newsletters={newsletters as Newsletter[]} />; }apps/web/src/app/(main)/dashboard/newsletters/components/NewsletterList.tsx (1)
16-24: Consider reusing NewsletterEmptyState component.The empty state message "No newsletters yet. Check back soon!" is duplicated here and in the
NewsletterEmptyStatecomponent. For consistency and maintainability, consider reusing theNewsletterEmptyStatecomponent.if (newsletters.length === 0) { - return ( - <div className="text-center py-12"> - <p className="text-muted-foreground"> - No newsletters yet. Check back soon! - </p> - </div> - ); + return <NewsletterEmptyState hasActiveFilters={false} onClearFilters={() => {}} />; }However, verify if this component's empty state should differ from the filter-driven empty state in the parent component.
apps/web/src/types/newsletter.ts (2)
36-36: Consider the date field type.The
datefield is typed asstring. While this is suitable for serialization and JSON compatibility, consider whether you need:
- A more specific type like
${number}-${number}-${number}for ISO date strings- Date parsing validation utilities if consuming this from external sources
- Documentation about the expected format (appears to be "YYYY-MM-DD" based on the sample data)
The current approach is acceptable for static data and JSON serialization. This is just a consideration for future enhancements if date validation or stronger typing becomes necessary.
9-11: Consider supporting additional heading levels.The heading type currently supports levels 1-3. Consider whether you need to support h4-h6 (
level: 1 | 2 | 3 | 4 | 5 | 6) for more granular content hierarchy in future newsletters.If the current levels (1-3) cover all use cases, the current implementation is fine.
apps/web/src/app/(main)/dashboard/newsletters/[id]/page.tsx (1)
101-101: Remove redundant type assertion.The
contentproperty is already typed asNewsletterContentItem[]in theNewsletterinterface, making theas NewsletterContentItem[]assertion unnecessary.Apply this diff:
- <NewsletterContent content={newsletter.content as NewsletterContentItem[]} /> + <NewsletterContent content={newsletter.content} />apps/web/src/app/(main)/dashboard/newsletters/utils/newsletter.filters.ts (2)
9-20: Remove unnecessary optional chaining on required properties.Based on the
NewsletterContentItemtype definition, thecontent,text, andurlproperties are required (not optional) for their respective item types. The optional chaining operators on lines 11, 15, and 16 are unnecessary.Apply this diff:
const matchesContent = newsletter.content?.some((item: NewsletterContentItem) => { if (item.type === "paragraph" || item.type === "heading" || item.type === "bold") { - return item.content?.toLowerCase().includes(query); + return item.content.toLowerCase().includes(query); } if (item.type === "link") { return ( - item.text?.toLowerCase().includes(query) || - item.url?.toLowerCase().includes(query) + item.text.toLowerCase().includes(query) || + item.url.toLowerCase().includes(query) ); } return false; });
22-22: Remove redundant|| falsein boolean expression.Since both
matchesBasicFieldsandmatchesContentare already boolean values, the|| falseis redundant.Apply this diff:
- return matchesBasicFields || matchesContent || false; + return matchesBasicFields || matchesContent;apps/web/src/app/(main)/dashboard/newsletters/components/NewsletterCard.tsx (1)
18-18: Replace hardcoded color with a theme variable.The hardcoded color
#693dabshould be replaced with a CSS variable or Tailwind theme color for better maintainability and theme consistency.Consider using a semantic color like
hover:border-primaryor defining this color in your Tailwind theme configuration:- <Card className="overflow-hidden hover:shadow-lg transition-all duration-300 border-border hover:border-[#693dab] cursor-pointer"> + <Card className="overflow-hidden hover:shadow-lg transition-all duration-300 border-border hover:border-primary cursor-pointer">apps/web/src/app/(main)/dashboard/newsletters/components/NewsletterContent.tsx (2)
5-5: Remove unused import.The
Imagecomponent fromnext/imageis imported but never used in this file.Apply this diff:
import { NewsletterContentItem } from "@/types/newsletter"; import Link from "next/link"; -import Image from "next/image";
23-37: Wrap variable declarations in block scope.Variable declarations in switch cases should be wrapped in blocks to prevent them from being erroneously accessible in other cases.
Based on learnings
Apply this diff:
case "heading": + { const HeadingTag = `h${item.level}` as keyof JSX.IntrinsicElements; const headingClasses = { }; return ( <HeadingTag key={index} className={headingClasses[item.level]} > {item.content} </HeadingTag> ); + }Apply the same pattern to the
"list"case on line 71.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
apps/web/src/app/(main)/dashboard/newsletters/Content.tsx(1 hunks)apps/web/src/app/(main)/dashboard/newsletters/[id]/page.tsx(1 hunks)apps/web/src/app/(main)/dashboard/newsletters/components/NewsletterCard.tsx(1 hunks)apps/web/src/app/(main)/dashboard/newsletters/components/NewsletterContent.tsx(1 hunks)apps/web/src/app/(main)/dashboard/newsletters/components/NewsletterEmptyState.tsx(1 hunks)apps/web/src/app/(main)/dashboard/newsletters/components/NewsletterFilters.tsx(1 hunks)apps/web/src/app/(main)/dashboard/newsletters/components/NewsletterList.tsx(1 hunks)apps/web/src/app/(main)/dashboard/newsletters/data/newsletters.ts(1 hunks)apps/web/src/app/(main)/dashboard/newsletters/page.tsx(1 hunks)apps/web/src/app/(main)/dashboard/newsletters/utils/newsletter.filters.ts(1 hunks)apps/web/src/app/(main)/dashboard/newsletters/utils/newsletter.utils.ts(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/types/index.ts(1 hunks)apps/web/src/types/newsletter.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (11)
apps/web/src/app/(main)/dashboard/newsletters/components/NewsletterCard.tsx (2)
apps/web/src/types/newsletter.ts (1)
Newsletter(33-42)apps/web/src/app/(main)/dashboard/newsletters/utils/newsletter.utils.ts (1)
formatNewsletterDate(51-57)
apps/web/src/app/(main)/dashboard/newsletters/utils/newsletter.filters.ts (1)
apps/web/src/types/newsletter.ts (2)
Newsletter(33-42)NewsletterContentItem(3-31)
apps/web/src/app/(main)/dashboard/newsletters/components/NewsletterList.tsx (3)
apps/web/src/types/newsletter.ts (1)
Newsletter(33-42)apps/web/src/app/(main)/dashboard/newsletters/utils/newsletter.utils.ts (2)
groupByMonth(3-26)sortMonthKeys(28-34)apps/web/src/app/(main)/dashboard/newsletters/components/NewsletterCard.tsx (1)
NewsletterCard(13-64)
apps/web/src/app/(main)/dashboard/newsletters/components/NewsletterContent.tsx (1)
apps/web/src/types/newsletter.ts (1)
NewsletterContentItem(3-31)
apps/web/src/app/(main)/dashboard/newsletters/page.tsx (3)
apps/web/src/app/(main)/dashboard/newsletters/Content.tsx (1)
Newsletters(16-73)apps/web/src/app/(main)/dashboard/newsletters/data/newsletters.ts (1)
newsletters(2-212)apps/web/src/types/newsletter.ts (1)
Newsletter(33-42)
apps/web/src/app/(main)/dashboard/newsletters/[id]/page.tsx (4)
apps/web/src/app/(main)/dashboard/newsletters/data/newsletters.ts (1)
newsletters(2-212)apps/web/src/app/(main)/dashboard/newsletters/utils/newsletter.utils.ts (1)
formatNewsletterDate(51-57)apps/web/src/app/(main)/dashboard/newsletters/components/NewsletterContent.tsx (1)
NewsletterContent(11-114)apps/web/src/types/newsletter.ts (1)
NewsletterContentItem(3-31)
apps/web/src/app/(main)/dashboard/newsletters/Content.tsx (6)
apps/web/src/types/newsletter.ts (1)
Newsletter(33-42)apps/web/src/app/(main)/dashboard/newsletters/utils/newsletter.utils.ts (1)
getAvailableMonths(37-48)apps/web/src/app/(main)/dashboard/newsletters/utils/newsletter.filters.ts (1)
filterNewsletters(40-59)apps/web/src/app/(main)/dashboard/newsletters/components/NewsletterFilters.tsx (1)
NewsletterFilters(24-82)apps/web/src/app/(main)/dashboard/newsletters/components/NewsletterEmptyState.tsx (1)
NewsletterEmptyState(10-28)apps/web/src/app/(main)/dashboard/newsletters/components/NewsletterList.tsx (1)
NewsletterList(12-44)
apps/web/src/app/(main)/dashboard/newsletters/utils/newsletter.utils.ts (2)
apps/web/src/types/newsletter.ts (1)
Newsletter(33-42)apps/web/src/app/(main)/dashboard/newsletters/data/newsletters.ts (1)
newsletters(2-212)
apps/web/src/components/ui/input.tsx (1)
apps/web/src/lib/utils.ts (1)
cn(4-6)
apps/web/src/components/ui/select.tsx (1)
apps/web/src/lib/utils.ts (1)
cn(4-6)
apps/web/src/app/(main)/dashboard/newsletters/components/NewsletterFilters.tsx (2)
apps/web/src/components/ui/input.tsx (1)
Input(22-22)apps/web/src/components/ui/select.tsx (5)
Select(149-149)SelectTrigger(152-152)SelectValue(151-151)SelectContent(153-153)SelectItem(155-155)
🪛 Biome (2.1.2)
apps/web/src/app/(main)/dashboard/newsletters/components/NewsletterContent.tsx
[error] 24-24: 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] 25-29: 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] 72-72: 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)
🔇 Additional comments (10)
apps/web/src/components/ui/select.tsx (1)
1-160: LGTM! Well-structured Radix UI Select wrapper.The component follows React best practices with proper forwardRef usage, TypeScript typing, and displayName assignments. The className composition using the
cnutility is consistent, and all Radix UI primitives are correctly wrapped and exported.apps/web/src/types/index.ts (1)
3-3: LGTM! Consistent barrel export pattern.The newsletter types re-export follows the established pattern and properly centralizes type access.
apps/web/src/app/(main)/dashboard/newsletters/components/NewsletterEmptyState.tsx (1)
1-29: LGTM! Clean empty state implementation.The component correctly handles both filtered and unfiltered empty states with appropriate messaging and user actions.
apps/web/src/components/ui/input.tsx (1)
1-22: LGTM! Well-implemented Input component.The component follows React best practices with proper forwardRef usage, TypeScript typing, and comprehensive styling. The use of
cnutility for className composition is consistent with other UI components.apps/web/src/app/(main)/dashboard/newsletters/components/NewsletterList.tsx (1)
12-44: LGTM! Clean implementation of grouped newsletter display.The component properly groups newsletters by month, sorts them in descending order, and renders them in an organized structure. The use of utility functions keeps the logic clean and maintainable.
apps/web/src/app/(main)/dashboard/newsletters/data/newsletters.ts (1)
1-212: LGTM! Comprehensive sample newsletter data.The sample data is well-structured with rich content blocks covering various types (paragraphs, headings, links, lists). This provides good test coverage for the UI components.
apps/web/src/types/newsletter.ts (1)
1-42: LGTM! Well-designed type definitions.The discriminated union for
NewsletterContentItemprovides type-safe content blocks, and theNewsletterinterface is comprehensive with optional fields for flexibility. The types support both static and dynamic image sources appropriately.apps/web/src/app/(main)/dashboard/newsletters/components/NewsletterFilters.tsx (1)
24-82: LGTM!The
NewsletterFilterscomponent is well-implemented with clean state management, responsive design, and good user experience. The active filter detection and clear filters functionality work correctly.apps/web/src/app/(main)/dashboard/newsletters/Content.tsx (1)
16-73: LGTM!The
Newsletterscomponent is well-structured with appropriate use ofuseMemofor performance optimization. The filtering logic is correctly implemented with proper state management and conditional rendering.apps/web/src/app/(main)/dashboard/newsletters/utils/newsletter.utils.ts (1)
3-26: Utility functions are well-structured.The
groupByMonth,getAvailableMonths, andformatNewsletterDatefunctions are correctly implemented with appropriate logic for organizing and formatting newsletter data. The only concern is withsortMonthKeys(noted separately).Also applies to: 37-48, 51-57
apps/web/src/app/(main)/dashboard/newsletters/components/NewsletterContent.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/web/next.config.js (1)
9-12: Consider adding pathname restrictions for tighter security.The remote pattern configuration is correct and enables loading newsletter images from Unsplash. However, the pattern is broad and allows any image from
images.unsplash.com.If your newsletter images follow a predictable URL structure, consider adding a
pathnamefield to restrict which images can be loaded.Example with pathname restriction:
{ protocol: "https", hostname: "images.unsplash.com", + pathname: "/photo-*/**", },Operational considerations:
- External images introduce latency and dependency on Unsplash availability
- Consider implementing fallback images for resilience
- Monitor Unsplash rate limits if serving high traffic
- Next.js will cache optimized images, but first load still depends on external service
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/next.config.js(1 hunks)apps/web/package.json(1 hunks)
🔇 Additional comments (1)
apps/web/package.json (1)
18-18: @radix-ui/react-select version is current and secure.The added version 2.2.6 is the latest available, and Next.js 15 supports React 18, which matches your project configuration. No security vulnerabilities were found. The dependency addition is safe to proceed with.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
apps/web/src/app/(main)/dashboard/newsletters/utils/newsletter.utils.ts (1)
28-37: Strengthen date parsing with validation and whitespace handling.The current implementation improves on the past review but still has gaps: it doesn't validate the split result, handle extra whitespace, or guard against invalid month names.
Consider a more robust approach using month name mapping:
+const MONTH_MAP: { [key: string]: number } = { + january: 0, february: 1, march: 2, april: 3, may: 4, june: 5, + july: 6, august: 7, september: 8, october: 9, november: 10, december: 11 +}; + export const sortMonthKeys = (keys: string[]): string[] => { return keys.sort((a, b) => { - // Parse month and year separately for reliable date parsing - const [monthA, yearA] = a.split(" "); - const [monthB, yearB] = b.split(" "); - const dateA = new Date(`${monthA} 1, ${yearA}`); - const dateB = new Date(`${monthB} 1, ${yearB}`); - return dateB.getTime() - dateA.getTime(); + const [monthA, yearA] = a.trim().split(/\s+/); + const [monthB, yearB] = b.trim().split(/\s+/); + + if (!monthA || !yearA || !monthB || !yearB) { + return 0; // Handle malformed keys + } + + const monthIndexA = MONTH_MAP[monthA.toLowerCase()]; + const monthIndexB = MONTH_MAP[monthB.toLowerCase()]; + const numA = parseInt(yearA) * 12 + (monthIndexA ?? 0); + const numB = parseInt(yearB) * 12 + (monthIndexB ?? 0); + + return numB - numA; }); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web/src/app/(main)/dashboard/newsletters/data/newsletters.ts(1 hunks)apps/web/src/app/(main)/dashboard/newsletters/utils/newsletter.utils.ts(1 hunks)apps/web/src/components/dashboard/Sidebar.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/src/app/(main)/dashboard/newsletters/data/newsletters.ts
🧰 Additional context used
🧬 Code graph analysis (1)
apps/web/src/app/(main)/dashboard/newsletters/utils/newsletter.utils.ts (2)
apps/web/src/types/newsletter.ts (1)
Newsletter(33-42)apps/web/src/app/(main)/dashboard/newsletters/data/newsletters.ts (1)
newsletters(2-212)
🔇 Additional comments (3)
apps/web/src/components/dashboard/Sidebar.tsx (3)
20-20: LGTM: Icon import is consistent with existing patterns.The
NewspaperIconimport from@heroicons/react/24/outlinecorrectly matches the icon library used for all other sidebar icons, resolving the previous concern about mixing icon libraries.
39-43: LGTM: Newsletters route correctly implemented and previous issues resolved.The newsletters route addition is well-structured and consistent with existing sidebar routes. The label is correctly spelled as "Newsletters" (one word), and the icon usage follows the established pattern with
className="size-5".
26-26: Import is used but appears unrelated to the newsletters feature scope—request manual verification of whether this should be in this PR.The
useFilterStoreimport on line 26 is actively used on lines 55 and 80 for the "Find Projects" functionality (viahandleFindProjects()), not for the newsletters feature. While the import is not dead code, the original concern about scope is valid: if this is a new addition to the codebase, it should clarify whether Find Projects filtering functionality belongs in a newsletters PR or if it's a separate feature being added concurrently.The line is marked as changed, which may indicate a new import, but the git history check failed and couldn't confirm whether this existed before.
- Implemented premium access control for newsletters, displaying a loading state and a premium gate for unpaid users. - Added caching mechanism for newsletters data to improve performance. - Introduced new content types for newsletters, including code snippets and tables. - Created a dedicated component for the premium access gate with upgrade options.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/web/src/app/(main)/dashboard/newsletters/components/NewsletterContent.tsx (1)
71-98: Fix inverted alignment logic.Line 72 has a critical logic error:
const isRightAligned = item.align === "left";assignstruewhen alignment is"left", which is backwards. This causes lists to display with incorrect alignment.Apply this diff to fix:
case "list": + { - const isRightAligned = item.align === "left"; + const isRightAligned = item.align === "right"; if (isRightAligned) { return ( <ul key={index} className="my-4 space-y-2 list-none text-right"> {item.items.map((listItem, itemIndex) => ( <li key={itemIndex} className="text-foreground/90 flex items-center justify-end gap-2" > <span>{listItem}</span> <span className="text-foreground/60">•</span> </li> ))} </ul> ); } return ( <ul key={index} className="my-4 space-y-2 list-disc list-inside"> {item.items.map((listItem, itemIndex) => ( <li key={itemIndex} className="text-foreground/90"> {listItem} </li> ))} </ul> ); + }Also wraps the case in a block to satisfy the linter warning about switch declarations.
🧹 Nitpick comments (10)
apps/web/src/app/(main)/dashboard/newsletters/utils/newsletter.filters.ts (2)
9-29: Optional: Simplify the return statement.Line 28's
|| falseis redundant defensive coding sincematchesBasicFieldsis already boolean. However, it guards againstmatchesContentbeing undefined whencontentis empty.If preferred, simplify to:
- return matchesBasicFields || matchesContent || false; + return matchesBasicFields || (matchesContent ?? false);
37-49: Add date validation to prevent "Invalid Date" comparisons.If
newsletter.dateis malformed,new Date(newsletter.date)will produce an invalid date, andtoLocaleDateStringwill return"Invalid Date", which will never matchselectedMonthbut won't surface the error.Add validation:
const matchesMonthFilter = ( newsletter: Newsletter, selectedMonth: string ): boolean => { if (selectedMonth === "all") return true; const date = new Date(newsletter.date); + if (isNaN(date.getTime())) { + console.warn(`Invalid date for newsletter ${newsletter.id}: ${newsletter.date}`); + return false; + } const monthYear = date.toLocaleDateString("en-US", { month: "long", year: "numeric", }); return monthYear === selectedMonth; };apps/web/src/app/(main)/dashboard/newsletters/page.tsx (2)
12-18: Consider enhancing the loading state.The loading state is very basic. For a better user experience, consider using a proper loading skeleton or spinner component that matches your design system.
Example improvement:
if (isLoading) { return ( - <div className="min-h-screen bg-background flex items-center justify-center"> - <div className="text-foreground">Loading...</div> + <div className="min-h-screen bg-background flex items-center justify-center"> + <div className="flex flex-col items-center gap-4"> + <div className="h-8 w-8 animate-spin rounded-full border-4 border-ox-purple border-t-transparent"></div> + <p className="text-muted-foreground">Loading newsletters...</p> + </div> </div> ); }
24-28: Remove unnecessary type assertion.Line 26 uses
as Newsletter[]to cast the newsletters data, but the importednewslettersconstant should already have the correct type from its definition. Type assertions can hide type mismatches.Apply this diff:
return ( <div> - <Newsletters newsletters={newsletters as Newsletter[]} /> + <Newsletters newsletters={newsletters} /> </div> );If TypeScript complains after removing the assertion, the root cause is likely a type mismatch that should be fixed at the source rather than suppressed with a cast.
apps/web/src/app/(main)/dashboard/newsletters/[id]/page.tsx (2)
16-50: LGTM with optional optimization note.The subscription gating and not-found handling are implemented correctly. The linear search on line 20 is acceptable for small datasets, but consider building a lookup map if the newsletter count grows significantly.
Optional: For larger datasets, pre-compute a lookup:
const newsletterMap = new Map(newsletters.map(n => [n.id, n])); const newsletter = newsletterMap.get(id);
114-117: Remove unnecessary type assertion.Line 116 casts
newsletter.contenttoNewsletterContentItem[], but it should already have this type from the Newsletter interface. Type assertions can hide type mismatches.Apply this diff:
<div className="prose prose-lg max-w-none font-sans"> - <NewsletterContent content={newsletter.content as NewsletterContentItem[]} /> + <NewsletterContent content={newsletter.content} /> </div>apps/web/src/app/(main)/dashboard/newsletters/components/NewsletterContent.tsx (2)
23-37: Wrap case declarations in blocks to satisfy linter.The linter flags that
HeadingTagandheadingClassesdeclarations can leak to other switch cases. While not a functional bug here (the case returns), wrapping in a block is best practice.Apply this diff:
case "heading": + { const HeadingTag = `h${item.level}` as keyof JSX.IntrinsicElements; const headingClasses = { }; return ( <HeadingTag key={index} className={headingClasses[item.level]} > {item.content} </HeadingTag> ); + }
60-69: Consider using Next.js Image for optimization.The image case uses a native
<img>tag. While this works, Next.js Image provides automatic optimization, lazy loading, and better performance.However, since newsletter images use external URLs (per the data file), the current approach is acceptable. If you want to use Next.js Image, you'll need to configure
remotePatternsinnext.config.js.apps/web/src/app/(main)/dashboard/newsletters/utils/newsletter.utils.ts (2)
60-71: Add date validation to prevent "Invalid Date" in month filters.Unlike
groupByMonth, this function doesn't validate dates before formatting. Invalidnewsletter.datevalues will produce"Invalid Date"strings that could appear in the month filter dropdown.Apply this diff to add validation:
export const getAvailableMonths = (newsletters: Newsletter[]): string[] => { - const months = newsletters.map((n) => { + const months = newsletters.flatMap((n) => { const date = new Date(n.date); + if (isNaN(date.getTime())) { + return []; // Skip invalid dates + } return date.toLocaleDateString("en-US", { month: "long", year: "numeric", }); }); const uniqueMonths = Array.from(new Set(months)); return sortMonthKeys(uniqueMonths); };Using
flatMapwithreturn []skips invalid dates, while valid dates return an array with one element that gets flattened.
79-85: Add validation to prevent displaying "Invalid Date" to users.This function is used to display dates in the UI. Without validation, malformed
dateStringvalues will produce"Invalid Date"that gets shown to users.Apply this diff to add validation with a user-friendly fallback:
export const formatNewsletterDate = (dateString: string): string => { - return new Date(dateString).toLocaleDateString("en-US", { + const date = new Date(dateString); + if (isNaN(date.getTime())) { + return "Date unavailable"; + } + return date.toLocaleDateString("en-US", { month: "long", day: "numeric", year: "numeric", }); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/web/src/app/(main)/dashboard/newsletters/[id]/page.tsx(1 hunks)apps/web/src/app/(main)/dashboard/newsletters/components/NewsletterContent.tsx(1 hunks)apps/web/src/app/(main)/dashboard/newsletters/components/NewsletterPremiumGate.tsx(1 hunks)apps/web/src/app/(main)/dashboard/newsletters/page.tsx(1 hunks)apps/web/src/app/(main)/dashboard/newsletters/utils/newsletter.cache.ts(1 hunks)apps/web/src/app/(main)/dashboard/newsletters/utils/newsletter.filters.ts(1 hunks)apps/web/src/app/(main)/dashboard/newsletters/utils/newsletter.utils.ts(1 hunks)apps/web/src/types/newsletter.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/src/types/newsletter.ts
🧰 Additional context used
🧬 Code graph analysis (7)
apps/web/src/app/(main)/dashboard/newsletters/utils/newsletter.cache.ts (2)
apps/web/src/types/newsletter.ts (1)
Newsletter(43-52)apps/web/src/app/(main)/dashboard/newsletters/data/newsletters.ts (1)
newsletters(2-212)
apps/web/src/app/(main)/dashboard/newsletters/components/NewsletterPremiumGate.tsx (1)
apps/web/src/components/ui/card.tsx (4)
CardHeader(78-78)CardTitle(80-80)CardDescription(81-81)CardContent(82-82)
apps/web/src/app/(main)/dashboard/newsletters/components/NewsletterContent.tsx (1)
apps/web/src/types/newsletter.ts (1)
NewsletterContentItem(3-41)
apps/web/src/app/(main)/dashboard/newsletters/[id]/page.tsx (6)
apps/web/src/hooks/useSubscription.ts (1)
useSubscription(11-75)apps/web/src/app/(main)/dashboard/newsletters/data/newsletters.ts (1)
newsletters(2-212)apps/web/src/app/(main)/dashboard/newsletters/components/NewsletterPremiumGate.tsx (1)
NewsletterPremiumGate(9-66)apps/web/src/app/(main)/dashboard/newsletters/utils/newsletter.utils.ts (1)
formatNewsletterDate(79-85)apps/web/src/app/(main)/dashboard/newsletters/components/NewsletterContent.tsx (1)
NewsletterContent(11-155)apps/web/src/types/newsletter.ts (1)
NewsletterContentItem(3-41)
apps/web/src/app/(main)/dashboard/newsletters/page.tsx (5)
apps/web/src/hooks/useSubscription.ts (1)
useSubscription(11-75)apps/web/src/app/(main)/dashboard/newsletters/components/NewsletterPremiumGate.tsx (1)
NewsletterPremiumGate(9-66)apps/web/src/app/(main)/dashboard/newsletters/Content.tsx (1)
Newsletters(16-73)apps/web/src/app/(main)/dashboard/newsletters/data/newsletters.ts (1)
newsletters(2-212)apps/web/src/types/newsletter.ts (1)
Newsletter(43-52)
apps/web/src/app/(main)/dashboard/newsletters/utils/newsletter.filters.ts (1)
apps/web/src/types/newsletter.ts (2)
Newsletter(43-52)NewsletterContentItem(3-41)
apps/web/src/app/(main)/dashboard/newsletters/utils/newsletter.utils.ts (2)
apps/web/src/types/newsletter.ts (1)
Newsletter(43-52)apps/web/src/app/(main)/dashboard/newsletters/data/newsletters.ts (1)
newsletters(2-212)
🪛 Biome (2.1.2)
apps/web/src/app/(main)/dashboard/newsletters/components/NewsletterContent.tsx
[error] 24-24: 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] 25-29: 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] 72-72: 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)
🔇 Additional comments (6)
apps/web/src/app/(main)/dashboard/newsletters/components/NewsletterPremiumGate.tsx (1)
9-66: LGTM!The premium gate component is well-structured with clear messaging and appropriate CTAs for both upgrading and returning to the dashboard.
apps/web/src/app/(main)/dashboard/newsletters/utils/newsletter.filters.ts (1)
59-78: LGTM!The filtering logic correctly chains search and month filters, with appropriate handling of empty search queries.
apps/web/src/app/(main)/dashboard/newsletters/[id]/page.tsx (1)
67-84: LGTM!The conditional rendering for cover images correctly handles both string URLs and Next.js StaticImageData objects. This flexible approach supports both external URLs and imported images.
apps/web/src/app/(main)/dashboard/newsletters/components/NewsletterContent.tsx (1)
46-58: LGTM!The link rendering correctly includes
target="_blank"withrel="noopener noreferrer"for security when opening external links.apps/web/src/app/(main)/dashboard/newsletters/utils/newsletter.utils.ts (2)
8-35: LGTM!The date validation added on lines 13-16 correctly addresses the previous review concern about handling invalid dates. Invalid newsletters are logged and skipped, preventing "Invalid Date" strings from appearing as group keys.
43-52: LGTM!The reliable date parsing approach (lines 46-49) addresses the previous review concern by splitting month/year components and explicitly adding a day. This ensures consistent parsing across JavaScript engines.
apps/web/src/app/(main)/dashboard/newsletters/utils/newsletter.cache.ts
Outdated
Show resolved
Hide resolved
- Replaced <img> tags with <Image> components for better optimization in newsletter pages and cards. - Adjusted image classes to use 'object-contain' for improved layout. - Removed the newsletter caching utility as it is no longer needed.
- Removed premium access checks and loading states from newsletters and individual newsletter pages. - Updated NewsletterContent component to convert URLs in text to clickable links. - Enhanced list items to support clickable links and adjusted alignment logic. - Removed the NewsletterPremiumGate component as it is no longer needed.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
apps/web/src/app/(main)/dashboard/newsletters/components/NewsletterContent.tsx (1)
19-42: Wrap switch case variable declarations in blocks.Multiple case clauses declare variables without block scope, which allows them to be erroneously accessed by other cases. While the current code doesn't exhibit this issue, wrapping each case body in blocks is a JavaScript/TypeScript best practice that prevents potential scope-related bugs.
Apply this diff to wrap the affected cases in blocks:
switch (item.type) { - case "paragraph": + case "paragraph": { // Convert URLs in text to clickable links const parts = item.content.split(urlRegex); return ( <p key={index} className="text-foreground/90 leading-relaxed"> {parts.map((part, partIndex) => { if (part.match(urlRegex)) { return ( <Link key={partIndex} href={part} target="_blank" rel="noopener noreferrer" className="text-blue-500 hover:text-blue-600 hover:underline font-medium" > {part} </Link> ); } return <span key={partIndex}>{part}</span>; })} </p> ); + } - case "heading": + case "heading": { const HeadingTag = `h${item.level}` as keyof JSX.IntrinsicElements; const headingClasses = { }; return ( <HeadingTag key={index} className={headingClasses[item.level]} > {item.content} </HeadingTag> ); + } - case "list": + case "list": { const isRightAligned = item.align === "right"; const renderListItem = (listItem: string, itemIndex: number) => { const parts = listItem.split(urlRegex); return parts.map((part, partIndex) => { if (part.match(urlRegex)) { return ( <Link key={partIndex} href={part} target="_blank" rel="noopener noreferrer" className="text-blue-500 hover:text-blue-600 hover:underline font-medium" > {part} </Link> ); } return <span key={partIndex}>{part}</span>; }); }; if (isRightAligned) { return ( <ul key={index} className="my-4 space-y-2 list-none text-right"> {item.items.map((listItem, itemIndex) => ( <li key={itemIndex} className="text-foreground/90 flex items-center justify-end gap-2" > <span>{renderListItem(listItem, itemIndex)}</span> <span className="text-foreground/60">•</span> </li> ))} </ul> ); } return ( <ul key={index} className="my-4 space-y-2 list-disc list-inside"> {item.items.map((listItem, itemIndex) => ( <li key={itemIndex} className="text-foreground/90"> {renderListItem(listItem, itemIndex)} </li> ))} </ul> ); + }Also applies to: 44-58, 94-141
apps/web/src/app/(main)/dashboard/newsletters/page.tsx (3)
8-12: Consider removing the redundant wrapper div.The outer
<div>serves no purpose since theNewsletterscomponent already provides its own wrapper.export default function NewslettersPage() { - return ( - <div> - <Newsletters newsletters={newsletters as Newsletter[]} /> - </div> - ); + return <Newsletters newsletters={newsletters as Newsletter[]} />; }
1-1: Consider removing the "use client" directive.This page component doesn't use any client-side hooks or browser APIs—it only passes props to the
Newsletterscomponent. In Next.js 15, server components are the default and offer better performance. TheNewsletterscomponent imported fromContent.tsx(verified to have"use client"on line 1) will continue to work as a client component regardless of whether the parent page is a server or client component.Remove the directive:
-"use client"; - import { Newsletter } from "@/types/newsletter"; import Newsletters from "./Content"; import { newsletters } from "./data/newsletters";
10-10: Add type annotation to thenewslettersexport to eliminate the type cast.The data structure in
newsletters.tsperfectly matches theNewsletterinterface—the cast is unnecessary. Add explicit type annotation to the export inapps/web/src/app/(main)/dashboard/newsletters/data/newsletters.ts:export const newsletters: Newsletter[] = [ // ... data ];Then remove the cast in
page.tsx:<Newsletters newsletters={newsletters} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/web/src/app/(main)/dashboard/newsletters/[id]/page.tsx(1 hunks)apps/web/src/app/(main)/dashboard/newsletters/components/NewsletterCard.tsx(1 hunks)apps/web/src/app/(main)/dashboard/newsletters/components/NewsletterContent.tsx(1 hunks)apps/web/src/app/(main)/dashboard/newsletters/data/newsletters.ts(1 hunks)apps/web/src/app/(main)/dashboard/newsletters/page.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/web/src/app/(main)/dashboard/newsletters/components/NewsletterCard.tsx
- apps/web/src/app/(main)/dashboard/newsletters/[id]/page.tsx
- apps/web/src/app/(main)/dashboard/newsletters/data/newsletters.ts
🧰 Additional context used
🧬 Code graph analysis (2)
apps/web/src/app/(main)/dashboard/newsletters/page.tsx (3)
apps/web/src/app/(main)/dashboard/newsletters/Content.tsx (1)
Newsletters(16-73)apps/web/src/app/(main)/dashboard/newsletters/data/newsletters.ts (1)
newsletters(2-224)apps/web/src/types/newsletter.ts (1)
Newsletter(43-52)
apps/web/src/app/(main)/dashboard/newsletters/components/NewsletterContent.tsx (1)
apps/web/src/types/newsletter.ts (1)
NewsletterContentItem(3-41)
🪛 Biome (2.1.2)
apps/web/src/app/(main)/dashboard/newsletters/components/NewsletterContent.tsx
[error] 21-21: 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] 45-45: 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] 46-50: 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] 95-95: 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] 97-115: 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)
🔇 Additional comments (1)
apps/web/src/app/(main)/dashboard/newsletters/components/NewsletterContent.tsx (1)
94-141: Previous alignment logic issue has been fixed.The inverted alignment logic flagged in the previous review has been corrected. Line 95 now properly assigns
isRightAligned = item.align === "right", ensuring right-aligned lists display correctly with the bullet on the right side.
1 participant
Fix : #155
opensox.mp4
Summary by CodeRabbit
Release Notes
New Features