Conversation
* feat: update my feeds tab on profile page * feat: update tabs to be routes * fix: double scrollbar * comment unused files in overview tab * fix: revert rsbuild * fmt * fix: add routegen to prettier ignore * clean up, server side --------- Co-authored-by: Elliot Braem <elliot@ejlbraem.com>
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
|
Warning Rate limit exceeded@elliotBraem has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 9 minutes and 12 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
WalkthroughThis update introduces a comprehensive overhaul of the user profile and feed management experience. It restructures the profile route hierarchy to support tabbed navigation, adds robust feed editing and creation flows with modular components, and implements new backend endpoints and schema changes for consistent feed ID handling and creator-based feed queries. The UI is refactored for improved usability and maintainability. Changes
Sequence Diagram(s)Editing a Feed (New Flow)sequenceDiagram
participant User
participant EditFeedComponent
participant useEditFeed (Hook)
participant API
User->>EditFeedComponent: Navigates to /edit/feed/:feedId
EditFeedComponent->>useEditFeed: Initializes with feedId
useEditFeed->>API: Fetch feed details
API-->>useEditFeed: Returns feed data
useEditFeed-->>EditFeedComponent: Provides feed data, config, handlers
User->>EditFeedComponent: Edits config/image, clicks Save/Delete
EditFeedComponent->>useEditFeed: Calls handler (save/delete)
useEditFeed->>API: Sends update/delete request
API-->>useEditFeed: Returns result
useEditFeed-->>EditFeedComponent: Updates state/UI
Profile Tabs NavigationsequenceDiagram
participant User
participant ProfileLayout
participant ProfileTabsAreaLayout
participant Router
User->>ProfileLayout: Navigates to /profile
ProfileLayout->>Router: Checks auth
Router-->>ProfileLayout: Authenticated
ProfileLayout->>ProfileTabsAreaLayout: Renders tab layout
User->>ProfileTabsAreaLayout: Clicks a tab
ProfileTabsAreaLayout->>Router: Navigates to tab route
Router-->>ProfileTabsAreaLayout: Loads tab content
Possibly related PRs
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
packages/core-services/src/services/feed.service.ts (1)
195-203: Inconsistent feed ID normalization in processFeed method.The
processFeedmethod doesn't apply lowercase normalization like other methods in the service. This could lead to inconsistent behavior when processing feeds.Apply this diff to ensure consistent feed ID handling:
- async processFeed(feedId: string, distributorsParam?: string) { - const feed = await this.feedRepository.findFeedById(feedId); + async processFeed(feedId: string, distributorsParam?: string) { + const lowerCaseFeedId = feedId.toLowerCase(); + const feed = await this.feedRepository.findFeedById(lowerCaseFeedId); if (!feed) { this.logger.error( - { feedId }, + { feedId: lowerCaseFeedId }, "FeedService: processFeed - Feed not found", ); - throw new NotFoundError("Feed", feedId); + throw new NotFoundError("Feed", lowerCaseFeedId); } - const feedConfig = await this.feedRepository.getFeedConfig(feedId); + const feedConfig = await this.feedRepository.getFeedConfig(lowerCaseFeedId); if (!feedConfig) { this.logger.error( - { feedId }, + { feedId: lowerCaseFeedId }, "FeedService: processFeed - Feed configuration not found in database", ); - throw new Error(`Feed configuration not found for feedId: ${feedId}`); + throw new Error(`Feed configuration not found for feedId: ${lowerCaseFeedId}`);
🧹 Nitpick comments (8)
apps/app/src/components/profile/overview/index.tsx (2)
2-5: Consider cleanup if these components are permanently removed.The commented-out imports suggest these components (
CurateCTA,TopBadges,UserStats) are being temporarily disabled. If this is a permanent change, consider removing the commented code entirely for better maintainability.If this is temporary for testing or gradual rollout, consider adding a TODO comment explaining the timeline for re-enabling these components.
10-14: Clean up commented JSX if components are permanently removed.The commented JSX blocks mirror the disabled imports above. For better code maintainability, consider either:
- Removing the commented code if this is a permanent change
- Adding explanatory comments about the timeline for re-enabling these features
apps/api/src/routes/api/users.ts (1)
338-373: Good API endpoint implementation with minor optimization suggestion.The endpoint correctly implements parameter validation, error handling, and response formatting. However, the response mapping on lines 353-356 explicitly spreads the feed object and then re-assigns the config property, which is redundant since the config is already part of the feed object.
Consider simplifying the response mapping:
- data: feeds.map((feed) => ({ - ...feed, - config: feed.config, - })), + data: feeds,apps/app/src/components/profile/my-feeds/FeedCard.tsx (1)
20-20: Track the TODO for tags system implementation.The TODO comment indicates missing functionality for extracting tags from feed sources.
Would you like me to help implement the tags extraction logic or open a new issue to track this task?
apps/app/src/components/profile/my-feeds/index.tsx (2)
30-52: Improve safety with optional chaining.The filtering logic is correct, but using optional chaining would make it more robust against unexpected data structures.
Apply this diff to use optional chaining:
const matchesSearch = !filters.term || feed.name.toLowerCase().includes(filters.term.toLowerCase()) || - (feed.description && - feed.description.toLowerCase().includes(filters.term.toLowerCase())); + feed.description?.toLowerCase().includes(filters.term.toLowerCase()); const isComplete = !!( - feed.config && - feed.config.name && - feed.config.description && - feed.config.sources && - feed.config.sources.length > 0 + feed.config?.name && + feed.config?.description && + feed.config?.sources?.length > 0 );
152-157: Simplify conditional rendering.The double check for
filteredFeedsis redundant since it's already checked in the parent condition.Apply this diff to simplify:
<div className="grid grid-cols-1 sm:grid-cols-2 md:grid-cols-3 gap-4 sm:gap-5"> - {filteredFeeds && - filteredFeeds.map((feed) => <FeedCard key={feed.id} feed={feed} />)} + {filteredFeeds.map((feed) => <FeedCard key={feed.id} feed={feed} />)} </div>apps/app/src/lib/api/feeds.ts (2)
201-211: Stats hook with pending curator count implementation.The hook structure is well-designed. The TODO comment indicates a missing API endpoint for curator count.
Would you like me to help implement the curator count API endpoint or create an issue to track this task?
188-211: Well-implemented statistical hooks with good performance practices.The hooks include proper stale time configuration (5 minutes) and error handling. The TODO comment for curator count should be tracked.
Do you want me to create an issue to track the TODO for adding the curator count API endpoint?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (28)
.prettierignore(1 hunks)apps/api/src/routes/api/feeds.ts(2 hunks)apps/api/src/routes/api/users.ts(2 hunks)apps/app/src/components/BasicInformationForm.tsx(0 hunks)apps/app/src/components/UserMenu.tsx(1 hunks)apps/app/src/components/profile/ProfileTabs.tsx(2 hunks)apps/app/src/components/profile/my-feeds/FeedCard.tsx(1 hunks)apps/app/src/components/profile/my-feeds/SearchForm.tsx(1 hunks)apps/app/src/components/profile/my-feeds/card.tsx(3 hunks)apps/app/src/components/profile/my-feeds/constants.ts(1 hunks)apps/app/src/components/profile/my-feeds/index.tsx(2 hunks)apps/app/src/components/profile/overview/index.tsx(1 hunks)apps/app/src/lib/api/feeds.ts(5 hunks)apps/app/src/lib/api/users.ts(2 hunks)apps/app/src/routeTree.gen.ts(3 hunks)apps/app/src/routes/_layout.tsx(1 hunks)apps/app/src/routes/_layout/create/feed/index.tsx(4 hunks)apps/app/src/routes/_layout/profile/_tabs/activity.tsx(1 hunks)apps/app/src/routes/_layout/profile/_tabs/my-feeds.tsx(1 hunks)apps/app/src/routes/_layout/profile/_tabs/overview.tsx(1 hunks)apps/app/src/routes/_layout/profile/_tabs/route.tsx(1 hunks)apps/app/src/routes/_layout/profile/index.tsx(1 hunks)packages/core-services/src/services/feed.service.ts(6 hunks)packages/shared-db/src/repositories/feed.repository.ts(1 hunks)packages/shared-db/src/schema/feeds.ts(3 hunks)packages/types/src/api/feeds.ts(1 hunks)packages/types/src/config.ts(1 hunks)pnpm-workspace.yaml(1 hunks)
💤 Files with no reviewable changes (1)
- apps/app/src/components/BasicInformationForm.tsx
🧰 Additional context used
🧬 Code Graph Analysis (11)
apps/app/src/components/profile/ProfileTabs.tsx (1)
apps/app/src/components/profile/my-feeds/index.tsx (1)
MyFeeds(20-160)
apps/app/src/routes/_layout/profile/_tabs/overview.tsx (6)
apps/app/src/routes/_layout.tsx (1)
Route(4-6)apps/app/src/routes/_layout/profile/_tabs/activity.tsx (1)
Route(4-6)apps/app/src/routes/_layout/profile/_tabs/my-feeds.tsx (1)
Route(4-6)apps/app/src/routes/_layout/profile/index.tsx (1)
Route(3-7)apps/app/src/routes/_layout/profile/_tabs/route.tsx (1)
Route(18-20)apps/app/src/components/profile/overview/index.tsx (1)
ProfileOverview(7-21)
apps/app/src/routes/_layout/profile/_tabs/my-feeds.tsx (6)
apps/app/src/routes/_layout.tsx (1)
Route(4-6)apps/app/src/routes/_layout/profile/_tabs/activity.tsx (1)
Route(4-6)apps/app/src/routes/_layout/profile/_tabs/overview.tsx (1)
Route(4-6)apps/app/src/routes/_layout/profile/index.tsx (1)
Route(3-7)apps/app/src/routes/_layout/profile/_tabs/route.tsx (1)
Route(18-20)apps/app/src/components/profile/my-feeds/index.tsx (1)
MyFeeds(20-160)
apps/app/src/routes/_layout/profile/_tabs/activity.tsx (4)
apps/app/src/routes/_layout/profile/_tabs/overview.tsx (1)
Route(4-6)apps/app/src/routes/_layout/profile/_tabs/my-feeds.tsx (1)
Route(4-6)apps/app/src/routes/_layout/profile/_tabs/route.tsx (1)
Route(18-20)apps/app/src/components/profile/activity/index.tsx (1)
ProfileActivity(76-103)
apps/api/src/routes/api/users.ts (4)
packages/types/src/api/users.ts (1)
UserNearAccountIdParamSchema(10-12)packages/core-services/src/services/feed.service.ts (1)
FeedService(27-311)packages/types/src/api/feeds.ts (1)
FeedsWrappedResponseSchema(59-60)packages/types/src/api/common.ts (1)
ApiErrorResponseSchema(25-32)
apps/api/src/routes/api/feeds.ts (2)
packages/utils/src/errors.ts (1)
ConflictError(218-222)packages/types/src/api/common.ts (1)
ApiErrorResponseSchema(25-32)
packages/shared-db/src/repositories/feed.repository.ts (2)
packages/shared-db/src/schema/feeds.ts (1)
SelectFeed(129-129)packages/shared-db/src/utils.ts (2)
withErrorHandling(108-140)executeWithRetry(82-98)
apps/app/src/routes/_layout/create/feed/index.tsx (2)
apps/app/src/lib/api-client.ts (1)
apiClient(83-83)packages/types/src/api/feeds.ts (1)
FeedWrappedResponse(54-54)
apps/app/src/routes/_layout/profile/_tabs/route.tsx (4)
apps/app/src/routes/_layout/profile/_tabs/activity.tsx (1)
Route(4-6)apps/app/src/routes/_layout/profile/index.tsx (1)
Route(3-7)apps/app/src/contexts/auth-context.tsx (1)
useAuth(28-34)apps/app/src/components/profile/ProfileHeader.tsx (1)
ProfileHeader(4-103)
apps/app/src/routes/_layout/profile/index.tsx (5)
apps/app/src/routes/_layout.tsx (1)
Route(4-6)apps/app/src/routes/_layout/profile/_tabs/activity.tsx (1)
Route(4-6)apps/app/src/routes/_layout/profile/_tabs/overview.tsx (1)
Route(4-6)apps/app/src/routes/_layout/profile/_tabs/my-feeds.tsx (1)
Route(4-6)apps/app/src/routes/_layout/profile/_tabs/route.tsx (1)
Route(18-20)
apps/app/src/lib/api/users.ts (2)
apps/app/src/hooks/api-client.ts (1)
useApiQuery(17-44)packages/types/src/api/feeds.ts (2)
FeedsWrappedResponse(61-61)FeedResponse(41-41)
🪛 Biome (1.9.4)
apps/app/src/components/profile/my-feeds/FeedCard.tsx
[error] 13-14: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
apps/app/src/components/profile/my-feeds/index.tsx
[error] 35-36: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 39-40: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 154-155: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (47)
.prettierignore (1)
8-8: LGTM: Properly excludes generated file from formatting.Adding the generated route tree file to
.prettierignoreis the correct approach to prevent formatting conflicts with auto-generated code.pnpm-workspace.yaml (1)
7-7: To confirm whethermsgpackr-extractis a workspace package (rather than an external dependency), let’s inspect the workspace config and check for a local folder or lockfile entries:#!/bin/bash # 1. Show the onlyBuiltDependencies section rg -A5 -B2 "onlyBuiltDependencies" pnpm-workspace.yaml # 2. Show declared workspace package patterns rg -A5 -B2 "packages:" pnpm-workspace.yaml # 3. Look for a local workspace directory named msgpackr-extract fd -t d msgpackr-extract # 4. Check lockfile for any reference if [ -f pnpm-lock.yaml ]; then rg -i "msgpackr-extract" pnpm-lock.yaml fiapps/app/src/components/UserMenu.tsx (1)
79-81: Verify mobile layout after making display name always visible.Removing the
hidden sm:blockclasses makes the user display name always visible, which improves user experience. However, please test on small screens to ensure this doesn't cause layout issues or text truncation in the user menu button.packages/types/src/api/feeds.ts (1)
29-29: Verify consistency of property name change across the codebase.Changing from
created_bytocreatedByfollows good camelCase naming conventions. However, this is a breaking change that must be consistent across the database schema, API endpoints, and all frontend code.#!/bin/bash # Description: Verify all usages of created_by have been updated to createdBy # Expected: Find any remaining snake_case usage that needs updating # Search for any remaining created_by usage rg "created_by" --type ts --type js -A 2 -B 2 # Search for database schema consistency rg "createdBy" --type ts --type js -A 2 -B 2 | grep -E "(schema|database|db)"apps/app/src/components/profile/ProfileTabs.tsx (3)
6-6: LGTM: Properly imports icon for My Feeds tab.The Newspaper icon is appropriately chosen for the "My Feeds" tab functionality.
11-11: LGTM: Correctly imports MyFeeds component.The MyFeeds component import is properly structured and aligns with the component shown in the relevant code snippets.
29-34: LGTM: Well-structured tab configuration for My Feeds.The tab configuration properly includes all required properties (value, label, icon, component) and follows the same pattern as other tabs. This directly addresses the PR objective of adding "my feeds" functionality.
packages/types/src/config.ts (1)
76-76: Good addition for feed ID consistency.The lowercase transformation ensures consistent feed ID handling across the system, preventing case-sensitivity issues.
Verify that existing feeds with uppercase characters in their IDs are properly migrated or handled:
#!/bin/bash # Check for any existing feeds with uppercase characters in IDs rg -i "feed.*id.*[A-Z]" --type json --type yaml --type ts --type jsapps/api/src/routes/api/feeds.ts (2)
10-14: Good addition for better error handling.Adding
ConflictErrorto the imports enables more specific error responses for feed creation conflicts.
91-100: Excellent improvement for conflict error handling.The specific handling of
ConflictErrorwith a 409 status code follows REST API best practices and provides clearer feedback when feed ID conflicts occur. This improves the user experience by giving more meaningful error responses instead of generic 500 errors.apps/app/src/routes/_layout/profile/_tabs/activity.tsx (1)
1-11: Clean implementation following established patterns.The route definition correctly follows the same pattern as other profile tab routes (
overview.tsx,my-feeds.tsx) and properly renders theProfileActivitycomponent. The structure is consistent with the profile tabs navigation system.apps/app/src/routes/_layout/profile/_tabs/overview.tsx (1)
1-11: Consistent implementation with other profile tab routes.The route definition properly follows the established pattern and correctly renders the
ProfileOverviewcomponent. This integrates well with the profile tabs navigation system where the main profile route redirects to/profile/overview.apps/app/src/routes/_layout/profile/_tabs/my-feeds.tsx (1)
1-11: LGTM! Clean route implementation following established patterns.The route definition correctly follows the pattern used by other profile tab routes and properly integrates with the nested route hierarchy. The import path and component rendering are appropriate.
apps/api/src/routes/api/users.ts (1)
1-5: Verify the import of FeedService.The import statement adds
FeedServicefrom the core services package. Ensure this service has thegetFeedsByCreatormethod available.#!/bin/bash # Description: Verify FeedService has getFeedsByCreator method # Expected: Method should exist in the service class ast-grep --pattern $'class FeedService { $$$ getFeedsByCreator($_) { $$$ } $$$ }'apps/app/src/routes/_layout.tsx (1)
10-15: Good layout simplification removing unnecessary nesting.The changes appropriately simplify the layout by:
- Switching from fixed height (
h-screen) to flexible height (min-h-screen h-full)- Removing redundant nested flex containers
- Maintaining proper spacing with padding classes
This creates a cleaner, more maintainable layout structure.
packages/shared-db/src/repositories/feed.repository.ts (1)
71-86: Excellent implementation following established repository patterns.The new
getFeedsByCreatormethod correctly implements:
- Proper error handling with
withErrorHandlingwrapper- Retry logic with
executeWithRetry- Appropriate database query using Drizzle ORM
- Consistent return type and error defaults
- Good documentation and context for debugging
The implementation aligns perfectly with other methods in this repository.
apps/app/src/routes/_layout/create/feed/index.tsx (3)
19-21: Good imports for async validation functionality.The added imports correctly support the new feed ID validation feature using React Query and the API client.
64-87: Excellent async validation implementation with good UX patterns.The
checkFeedIdfunction implements proper async validation:
- Uses React Query for efficient caching and data fetching
- Handles both success (feed exists) and error (feed doesn't exist) cases appropriately
- Sets clear error messages for users
- Uses
retry: falseto avoid unnecessary retries on 404 responsesThe implementation provides good user feedback without being overly aggressive.
155-166: I’ll check the feed creation form’s submit handler to see if it re-validates the ID:#!/bin/bash # Search for form submission handler in feed creation page rg -n "onSubmit" -A3 -B3 apps/app/src/routes/_layout/create/feed/index.tsx rg -n "handleSubmit" -A3 -B3 apps/app/src/routes/_layout/create/feed/index.tsx rg -n "checkFeedId" -A3 -B3 apps/app/src/routes/_layout/create/feed/index.tsxapps/app/src/components/profile/my-feeds/constants.ts (1)
1-16: Well-implemented filter constants with proper TypeScript typing.The use of
as constassertion and derived type union creates a type-safe, maintainable filter system. The constant array structure with value/label pairs provides good separation of concerns.packages/core-services/src/services/feed.service.ts (4)
15-19: Good addition of ConflictError for proper error handling.The import of
ConflictErrorsupports the duplicate feed check functionality added later in the file.
59-68: Consistent lowercase normalization and corrected property name.The lowercase normalization ensures consistent feed ID handling, and changing
created_bytocreatedByaligns with camelCase conventions.
97-100: Excellent duplicate prevention with proper error handling.The duplicate check before creation prevents conflicts and uses the appropriate
ConflictErrorfor HTTP 409 responses.
92-94: Verify the repository method exists.The new
getFeedsByCreatormethod needs a corresponding implementation in the repository layer.#!/bin/bash # Description: Verify that getFeedsByCreator method exists in the repository # Expected: Should find the method implementation in feed repository ast-grep --pattern $'getFeedsByCreator($$$) { $$$ }'apps/app/src/components/profile/my-feeds/SearchForm.tsx (1)
1-24: Clean and well-structured search form component.The component follows React best practices with controlled input, proper TypeScript interfaces, and responsive design. The embedded search icon provides good UX.
apps/app/src/components/profile/my-feeds/FeedCard.tsx (1)
9-36: Well-structured feed card component with proper data flow.The component effectively uses hooks for data fetching, handles missing data with appropriate fallbacks, and maintains clear separation of concerns.
apps/app/src/routes/_layout/profile/index.tsx (1)
4-6: Clean redirect implementation following TanStack Router patterns.The
beforeLoadhook with redirect is the correct approach for route-level redirects. This aligns well with the new nested profile route structure.#!/bin/bash # Description: Verify the redirect target route exists # Expected: Should find the overview route file fd "overview.tsx" apps/app/src/routes/_layout/profilepackages/shared-db/src/schema/feeds.ts (3)
28-46: LGTM! Good schema improvements.The refactoring to use the two-argument table function is cleaner, and the lowercase check constraint ensures consistent feed IDs across the system. The camelCase naming for
createdByaligns better with JavaScript conventions.
49-54: Relations updated correctly.The relations are properly updated to use the renamed
createdByfield.
111-120: Schema transformation ensures data consistency.The lowercase transformation for IDs is a good addition that enforces consistency at the application layer, complementing the database check constraint.
Please verify that existing feed creation code handles the property rename from
created_bytocreatedBy:#!/bin/bash # Description: Check for any remaining usage of the old 'created_by' property name # Search for potential uses of the old property name rg -A 3 "created_by" --type ts --type jsapps/app/src/components/profile/my-feeds/card.tsx (2)
6-31: Good navigation enhancement.Making the entire card clickable by wrapping it in a Link component improves user experience. The route pattern correctly uses the feed ID parameter.
46-53: Nice image error handling and accessibility improvement.The fallback image handler provides a better user experience when feed images fail to load, and the dynamic alt text improves accessibility.
apps/app/src/routes/_layout/profile/_tabs/route.tsx (2)
61-70: Good authentication gating.The component properly checks authentication status and provides a clear login prompt for unauthenticated users.
78-98: Well-implemented tab navigation.The tab system correctly syncs with the router state and provides a clean navigation experience with appropriate icons.
apps/app/src/components/profile/my-feeds/index.tsx (1)
28-64: Good data fetching and error handling.The component properly handles loading and error states, providing appropriate feedback to users.
apps/app/src/lib/api/users.ts (1)
31-40: Well-implemented API hook.The
useUserFeedshook follows the established pattern with proper TypeScript typing, conditional enabling, and data transformation.apps/app/src/lib/api/feeds.ts (10)
47-50: Good implementation of authenticated user feeds hook.The hook correctly handles authentication state and reuses the existing
useUserFeedshook efficiently.
52-70: Proper cache invalidation on feed creation.The implementation correctly invalidates both user-specific feeds and the general feeds list after successful creation.
72-91: Comprehensive cache invalidation on feed update.The invalidation strategy correctly covers all affected queries: the specific feed details, user feeds, and the general feeds list.
98-116: Consistent cache invalidation pattern for feed deletion.The implementation maintains consistency with other mutation hooks and properly invalidates all affected queries.
188-199: Efficient implementation for fetching content count.Good approach using
limit=1to minimize data transfer when only the count is needed. The 5-minute stale time is reasonable for this type of data.
1-1: LGTM: Imports are correctly added for new functionality.The new imports support the cache invalidation logic and user feed functionality introduced in this update.
Also applies to: 17-17
53-54: Excellent cache invalidation implementation.The cache invalidation logic properly invalidates both user-specific feeds and the general feeds list, ensuring data consistency after feed creation.
Also applies to: 62-67
73-74: Comprehensive cache invalidation for feed updates.Good implementation that invalidates feed details, user feeds, and the general feeds list. This ensures all related cached data stays consistent when a feed is updated.
Also applies to: 82-88
98-99: Complete cache invalidation for feed deletion.The cache invalidation correctly handles all relevant cache keys when a feed is deleted, maintaining data consistency across the application.
Also applies to: 107-113
47-50: ```bash
#!/bin/bashSearch for the implementation of useUserFeeds to inspect null handling
rg -n --context 5 "export function useUserFeeds" --glob '.ts'
rg -n --context 5 "function useUserFeeds" --glob '.ts'</details> <details> <summary>apps/app/src/routeTree.gen.ts (1)</summary> `1-10`: **Skip review of auto-generated file.** This file is automatically generated by TanStack Router and should not be manually edited or reviewed as stated in the header comments. </details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| const isComplete = !!( | ||
| feed.config && | ||
| feed.config.name && | ||
| feed.config.description && | ||
| feed.config.sources && | ||
| feed.config.sources.length > 0 | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Simplify the completion check using optional chaining.
The static analysis tool correctly suggests using optional chaining for cleaner, more readable code.
Apply this diff to improve readability:
- const isComplete = !!(
- feed.config &&
- feed.config.name &&
- feed.config.description &&
- feed.config.sources &&
- feed.config.sources.length > 0
- );
+ const isComplete = !!(
+ feed.config?.name &&
+ feed.config?.description &&
+ feed.config?.sources?.length > 0
+ );🧰 Tools
🪛 Biome (1.9.4)
[error] 13-14: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🤖 Prompt for AI Agents
In apps/app/src/components/profile/my-feeds/FeedCard.tsx around lines 12 to 18,
simplify the isComplete check by replacing the multiple logical AND checks with
optional chaining. Use optional chaining to access feed.config and its
properties safely, and check if sources array has length greater than zero in a
cleaner, more readable way.
* feat: add name and tag validation to create * feat: add no activity data in activity tab * add coming soon sections for profile page * feat: clicking on leaderboard feed hashtag will redirect to that feed * fix: keeps name on start when disable feed names collapse * fix: rsbuild * fix: add routegen to prettier ignore * fix: add ability to navigate to collapsed feeds in leaderboard * add ability to expand or collapse all * fix: rsbuild * adjustments * nitpicks --------- Co-authored-by: Elliot Braem <elliot@ejlbraem.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (2)
apps/app/src/components/profile/activity/index.tsx (1)
91-94: Hide filter button when no data is available.The filter button remains visible even when there's no activity data, which could confuse users about its functionality.
Make the filter button conditional:
- <Button variant="outline" size="sm" className="mt-2"> - Filter - <ChevronsUpDown size={14} className="ml-1" /> - </Button> + {hasData && ( + <Button variant="outline" size="sm" className="mt-2"> + Filter + <ChevronsUpDown size={14} className="ml-1" /> + </Button> + )}apps/app/src/components/leaderboard/LeaderboardFilters.tsx (1)
84-84: Inconsistent feed value handling between dropdowns.The feed dropdown (line 84) uses
search.feeddirectly, while the time dropdown (line 125) usessearch.feed.toLowerCase(). This inconsistency could lead to routing issues if feed IDs have different casing.Apply this diff to ensure consistent feed handling:
<Link key={index} to="/leaderboard" - search={{ feed: feed.value, timeframe: search.timeframe }} + search={{ feed: feed.value.toLowerCase(), timeframe: search.timeframe }} role="option"Also applies to: 125-125
🧹 Nitpick comments (3)
apps/app/src/components/profile/activity/index.tsx (1)
98-102: Enhance the empty state with better UX.The "no data" message provides basic feedback, but could be more informative and actionable for users.
Consider enhancing the empty state:
- {!hasData && ( - <div className="p-8 text-center text-gray-500"> - No activity data available - </div> - )} + {!hasData && ( + <div className="p-8 text-center"> + <div className="text-gray-500 mb-2">No activity data available</div> + <div className="text-sm text-gray-400"> + Start by submitting content or interacting with feeds to see your activity here. + </div> + </div> + )}apps/app/src/components/leaderboard/LeaderboardColumns.tsx (1)
92-105: LGTM! Good navigation enhancement with minor safety consideration.The Link wrapping makes feed submissions clickable and navigable, significantly improving UX. The routing pattern
/feed/$feedIdis consistent and appropriate.Consider adding safety checks for undefined feedId values:
<Link to={"/feed/$feedId"} params={{ feedId: feedSubmissions[0].feedId }} + className={!feedSubmissions[0].feedId ? "pointer-events-none" : ""} >Or add conditional rendering:
{feedSubmissions && feedSubmissions.length > 0 && ( + feedSubmissions[0].feedId && ( <Link to={"/feed/$feedId"} params={{ feedId: feedSubmissions[0].feedId }} >Also applies to: 128-137
apps/app/src/routes/_layout/create/feed/index.tsx (1)
66-92: Consider optimizing the React Query configuration.The current query setup could benefit from additional configuration for better performance and user experience.
const data = await queryClient.fetchQuery({ queryKey: ["feed-details", id], queryFn: () => apiClient.makeRequest<FeedWrappedResponse>("GET", `/feeds/${id}`), retry: false, + staleTime: 5 * 60 * 1000, // 5 minutes - avoid unnecessary refetches + gcTime: 10 * 60 * 1000, // 10 minutes - keep in cache longer });This prevents unnecessary API calls when users re-validate the same ID within the stale time window.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
apps/app/src/components/Leaderboard.tsx(3 hunks)apps/app/src/components/coming-soon.tsx(1 hunks)apps/app/src/components/leaderboard/LeaderboardColumns.tsx(3 hunks)apps/app/src/components/leaderboard/LeaderboardFilters.tsx(1 hunks)apps/app/src/components/leaderboard/LeaderboardTable.tsx(1 hunks)apps/app/src/components/profile/activity/index.tsx(2 hunks)apps/app/src/hooks/useLeaderboard.ts(3 hunks)apps/app/src/routes/_layout/create/feed/index.tsx(6 hunks)apps/app/src/routes/_layout/profile/settings/notifications.tsx(2 hunks)apps/app/src/routes/_layout/profile/settings/preferences.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- apps/app/src/components/leaderboard/LeaderboardTable.tsx
- apps/app/src/routes/_layout/profile/settings/notifications.tsx
- apps/app/src/routes/_layout/profile/settings/preferences.tsx
- apps/app/src/components/coming-soon.tsx
🧰 Additional context used
🧬 Code Graph Analysis (1)
apps/app/src/routes/_layout/create/feed/index.tsx (2)
apps/app/src/lib/api-client.ts (1)
apiClient(83-83)packages/types/src/api/feeds.ts (1)
FeedWrappedResponse(54-54)
🔇 Additional comments (12)
apps/app/src/components/profile/activity/index.tsx (1)
106-106: LGTM! Conditional pagination is correctly implemented.The conditional rendering ensures pagination controls only appear when there's data to paginate, which is the correct UX pattern.
apps/app/src/components/leaderboard/LeaderboardFilters.tsx (1)
49-49: LGTM! Clean styling adjustment.The padding/margin adjustments improve the vertical spacing of the filter controls.
apps/app/src/components/Leaderboard.tsx (2)
2-2: LGTM! Clean import and prop additions.The new imports and props are properly integrated with the hook pattern.
Also applies to: 37-38
69-86: LGTM! Well-implemented expand/collapse controls.The expand/collapse all buttons are properly:
- Conditionally rendered based on data availability
- Styled consistently with the existing UI
- Using appropriate icons for visual clarity
- Triggering the correct callback functions
apps/app/src/components/leaderboard/LeaderboardColumns.tsx (1)
5-5: LGTM! Proper routing import.The Link import enables navigation to feed detail pages.
apps/app/src/hooks/useLeaderboard.ts (3)
89-91: LGTM! Clean collapse all implementation.The callback properly clears the expanded rows state with appropriate memoization.
144-151: LGTM! Robust expand all implementation.The callback correctly:
- Handles the edge case where filtered data might be undefined
- Generates all row indices based on the filtered data length
- Uses proper memoization with dependency on filteredLeaderboardWithRanks
175-175: LGTM! Proper hook interface extension.The new callbacks and state are correctly exposed in the hook's return object.
Also applies to: 183-184
apps/app/src/routes/_layout/create/feed/index.tsx (4)
3-3: Import additions look good for the new async validation functionality.The new imports (
useState,useQueryClient,apiClient,FeedWrappedResponse) are appropriately added to support the feed ID validation feature.Also applies to: 20-22
46-47: State management setup is appropriate.The
queryClientandisValidatingIdstate are properly initialized for managing the async validation workflow.
160-171: Input field modifications enhance UX with lowercase normalization.The lowercase conversion on change and validation on blur provide good user experience. The implementation correctly maintains form field integration.
181-190: Button state management provides clear user feedback.The dynamic button text and disabled states during validation and submission give users appropriate feedback about the current operation state.
* feat: update the feed editor * feat: improve performance, and fix bugs * feat: revert local development change * add routegen to pretttier ignore * fix: resolve code rabbit comments * fix some nitpick comments * fix prettier and build config * formats * merge --------- Co-authored-by: Elliot Braem <elliot@ejlbraem.com>
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (16)
apps/app/src/routes/_layout/profile/_tabs/overview.tsx (1)
12-26: Consider extracting the ProfileOverview component and cleaning up commented code.The implementation is functional, but there are some improvements to consider:
- The
ProfileOverviewcomponent is defined inline within the route file. Consider extracting it to a separate component file for better maintainability.- The extensive commented-out code (lines 15-19) should either be removed or properly documented with TODO comments explaining the implementation timeline.
The responsive grid layout and component structure look good overall.
Consider this structure:
+// Create apps/app/src/components/profile/overview/ProfileOverview.tsx +export function ProfileOverview() { + return ( + <div className="flex flex-col items-stretch gap-4 sm:gap-6"> + <div className="grid grid-cols-1 md:grid-cols-2 gap-4 sm:gap-6"> + <ApproverFor /> + <CuratorFor /> + </div> + </div> + ); +} -function ProfileOverview() { - return ( - <div className="flex flex-col items-stretch gap-4 sm:gap-6"> - {/* <div className="grid grid-cols-1 md:grid-cols-2 gap-4 sm:gap-6"> - <UserStats /> - <TopBadges /> - </div> */} - {/* <CurateCTA /> */} - <div className="grid grid-cols-1 md:grid-cols-2 gap-4 sm:gap-6"> - <ApproverFor /> - <CuratorFor /> - </div> - </div> - ); -}apps/app/src/routes/_layout/profile.tsx (1)
14-23: Authentication gating implemented correctly with room for UI enhancement.The authentication logic is sound and properly prevents unauthorized access to profile pages. However, the login prompt UI could be enhanced for better user experience.
Consider improving the login prompt styling:
-<div className="min-h-screen mx-auto max-w-[1440px] flex flex-col gap-4 items-center justify-center"> - <h1>Please Login to Continue</h1> - <Button onClick={handleSignIn}>Login</Button> -</div> +<div className="min-h-screen mx-auto max-w-[1440px] flex flex-col gap-6 items-center justify-center text-center"> + <div className="space-y-2"> + <h1 className="text-2xl font-semibold">Please Login to Continue</h1> + <p className="text-neutral-600">You need to sign in to access your profile.</p> + </div> + <Button onClick={handleSignIn} size="lg">Sign In</Button> +</div>apps/app/src/components/feed/EditFeedLoadingState.tsx (1)
5-9: Improve type safety for feedData prop.The
feedDataprop is typed asunknownwhich doesn't provide type safety. Consider using a more specific type or generic approach.- interface EditFeedLoadingStateProps { + interface EditFeedLoadingStateProps<T = unknown> { isLoadingFeed: boolean; feedError: Error | null; - feedData: unknown; + feedData: T | null | undefined; } - export function EditFeedLoadingState({ + export function EditFeedLoadingState<T>({ isLoadingFeed, feedError, feedData, - }: EditFeedLoadingStateProps) { + }: EditFeedLoadingStateProps<T>) {apps/app/src/components/feed/EditFeedActions.tsx (1)
5-6: Remove unused prop from component interface.The
currentConfigprop is defined in the interface but never used in the component implementation.interface EditFeedActionsProps { - currentConfig: FeedConfig | null; onSaveChanges: () => Promise<void>; onDeleteFeed: () => Promise<void>; updateFeedMutation: { isPending: boolean; }; deleteFeedMutation: { isPending: boolean; }; } export function EditFeedActions({ onSaveChanges, onDeleteFeed, updateFeedMutation, deleteFeedMutation, }: EditFeedActionsProps) {Also applies to: 17-22
apps/app/src/routes/_layout/profile/_tabs/settings/_tabs.tsx (3)
40-45: Improve error handling for edge cases in tab detection.The fallback to "connections" is hardcoded, but what if the connections route doesn't exist or the user doesn't have access? Consider making the fallback more robust.
const getCurrentTab = () => { const currentItem = navigationItems.find( (item) => location.pathname === item.path, ); - return currentItem?.value || "connections"; + return currentItem?.value || navigationItems[0]?.value || "connections"; };
22-38: Consider extracting navigation configuration to improve maintainability.The navigation items are defined inline within the component. Consider extracting this configuration to improve maintainability and enable easier testing.
+const SETTINGS_NAVIGATION_ITEMS = [ + { + value: "connections", + path: "/profile/settings/connections", + label: "Connected Accounts", + }, + { + value: "notifications", + path: "/profile/settings/notifications", + label: "Notifications", + }, + { + value: "preferences", + path: "/profile/settings/preferences", + label: "Preferences", + }, +] as const; function SettingsTabsAreaLayout() { const location = useLocation(); const navigate = useNavigate(); - const navigationItems = [ - { - value: "connections", - path: "/profile/settings/connections", - label: "Connected Accounts", - }, - { - value: "notifications", - path: "/profile/settings/notifications", - label: "Notifications", - }, - { - value: "preferences", - path: "/profile/settings/preferences", - label: "Preferences", - }, - ]; + const navigationItems = SETTINGS_NAVIGATION_ITEMS;
68-70: Optimize TabsContent rendering.The
TabsContentcomponent is rendered withgetCurrentTab()which recalculates the current tab on every render. Consider memoizing this value or using the already calculated value.+ const currentTab = getCurrentTab(); + return ( <Tabs - value={getCurrentTab()} + value={currentTab} onValueChange={handleTabChange} className="w-full" > <TabsList className="w-full justify-start"> {navigationItems.map((item) => ( <TabsTrigger key={item.value} value={item.value}> {item.label} </TabsTrigger> ))} </TabsList> - <TabsContent value={getCurrentTab()} className="mt-6"> + <TabsContent value={currentTab} className="mt-6"> <Outlet /> </TabsContent> </Tabs> );apps/app/src/components/feed/EditFeedImageSection.tsx (1)
5-9: Remove unused feedId prop.The
feedIdprop is defined in the interface but never used in the component implementation.interface EditFeedImageSectionProps { currentConfig: FeedConfig | null; - feedId: string; onImageUploaded: (ipfsHash: string, ipfsUrl: string) => void; } export function EditFeedImageSection({ currentConfig, onImageUploaded, }: EditFeedImageSectionProps) {Also applies to: 11-14
apps/app/src/components/feed/form/SourcesSection.tsx (1)
126-138: Duplicate ID generation logic.The same ID generation logic is used here as in the SearchEntries component. Consider extracting this into a utility function to reduce duplication.
Create a utility function:
const generateSearchId = () => `search-${crypto.randomUUID()}`;Then use it in both locations:
- searchId: `search-${Date.now()}-${Math.random().toString(36).substr(2, 9)}`, + searchId: generateSearchId(),apps/app/src/components/feed/form/ModerationSection.tsx (1)
17-59: Consider refactoring to reduce code duplication.The
TwitterApproversFieldandTwitterBlacklistFieldcomponents share nearly identical logic. Consider creating a reusable component to reduce duplication.Create a generic component:
function TwitterListField({ control, name, label, placeholder }: { control: Control<FormValues>; name: "moderationApprovers" | "moderationBlacklist"; label: string; placeholder: string; }) { const { field, fieldState: { error } } = useController({ name, control }); const [localValue, setLocalValue] = useState(field.value?.twitter?.join(", ") || ""); useEffect(() => { const formValue = field.value?.twitter?.join(", ") || ""; setLocalValue(formValue); }, [field.value]); return ( <FormItem> <FormLabel>{label}</FormLabel> <FormControl> <Input placeholder={placeholder} value={localValue} onChange={(e) => setLocalValue(e.target.value)} onBlur={() => { const list = { ...field.value }; list.twitter = localValue .split(",") .map((s) => s.trim()) .filter(Boolean); field.onChange(list); }} /> </FormControl> {error && <FormMessage>{error.message}</FormMessage>} </FormItem> ); }apps/app/src/components/feed/form/RecapsSection.tsx (2)
40-45: Consider using a more robust ID generation method.The current ID generation using
Date.now() + Math.random()could potentially create collisions in rapid succession. Consider using a UUID library or crypto.randomUUID() for better uniqueness guarantees.- id: `recap-${Date.now()}-${Math.random().toString(36).substr(2, 9)}`, + id: `recap-${crypto.randomUUID()}`,
44-44: Consider making timezone configurable in the UI.The timezone is hardcoded to "UTC" but not exposed as a form field. Users might want to configure recaps in their local timezone or other specific timezones.
Add a timezone selector field to the form:
+ <FormField + control={control} + name={`recaps.${recapIndex}.timezone`} + render={({ field }) => ( + <FormItem> + <FormLabel>Timezone</FormLabel> + <FormControl> + <Input placeholder="UTC" {...field} /> + </FormControl> + <FormMessage /> + </FormItem> + )} + />apps/app/src/components/feed/EditFeedForm.tsx (3)
123-130: Remove or implement the commented recap field arrayThis commented code should either be implemented if recaps functionality is needed, or removed to keep the codebase clean.
190-193: Consider optimizing deep object comparisonsThe multiple
JSON.stringifycalls for deep comparison could impact performance with large configurations. Consider using a more efficient deep comparison utility like lodash'sisEqualor implementing a custom comparison that's tailored to your specific needs.+import { isEqual } from 'lodash-es'; // Line 190-193 -if ( - values.sources !== undefined && - JSON.stringify(values.sources) !== JSON.stringify(config.sources) -) { +if ( + values.sources !== undefined && + !isEqual(values.sources, config.sources) +) {Also applies to: 231-237, 268-271, 282-282
348-354: Clean up commented form sectionsMultiple form sections are commented out. If these features are planned for future implementation, consider adding TODO comments explaining the timeline. Otherwise, remove them to maintain code clarity.
Also applies to: 358-358
apps/app/src/hooks/use-edit-feed.ts (1)
125-125: Consider navigating to a more contextual route after deletionAfter successfully deleting a feed, navigating to the root path ("/") might not provide the best user experience. Consider navigating to the user's feeds list instead.
-navigate({ to: "/" }); +navigate({ to: "/profile/my-feeds" });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (36)
.prettierignore(1 hunks)apps/app/src/components/feed/EditFeedActions.tsx(1 hunks)apps/app/src/components/feed/EditFeedConfigSection.tsx(1 hunks)apps/app/src/components/feed/EditFeedForm.tsx(1 hunks)apps/app/src/components/feed/EditFeedHeader.tsx(1 hunks)apps/app/src/components/feed/EditFeedImageSection.tsx(1 hunks)apps/app/src/components/feed/EditFeedLoadingState.tsx(1 hunks)apps/app/src/components/feed/form/BasicFieldsSection.tsx(1 hunks)apps/app/src/components/feed/form/IngestionSection.tsx(1 hunks)apps/app/src/components/feed/form/ModerationSection.tsx(1 hunks)apps/app/src/components/feed/form/RecapsSection.tsx(1 hunks)apps/app/src/components/feed/form/SourcesSection.tsx(1 hunks)apps/app/src/components/feed/form/StreamSettingsSection.tsx(1 hunks)apps/app/src/components/feed/form/index.ts(1 hunks)apps/app/src/components/feed/form/types.ts(1 hunks)apps/app/src/components/profile/ProfileTabs.tsx(0 hunks)apps/app/src/components/profile/my-feeds/index.tsx(0 hunks)apps/app/src/components/profile/overview/index.tsx(0 hunks)apps/app/src/components/ui/tabs.tsx(2 hunks)apps/app/src/components/ui/textarea.tsx(1 hunks)apps/app/src/hooks/use-edit-feed.ts(5 hunks)apps/app/src/routeTree.gen.ts(2 hunks)apps/app/src/routes/_layout/edit/feed/$feedId.tsx(1 hunks)apps/app/src/routes/_layout/profile.tsx(1 hunks)apps/app/src/routes/_layout/profile/_tabs.tsx(1 hunks)apps/app/src/routes/_layout/profile/_tabs/activity.tsx(3 hunks)apps/app/src/routes/_layout/profile/_tabs/my-feeds.tsx(1 hunks)apps/app/src/routes/_layout/profile/_tabs/overview.tsx(1 hunks)apps/app/src/routes/_layout/profile/_tabs/settings/_tabs.tsx(1 hunks)apps/app/src/routes/_layout/profile/_tabs/settings/_tabs/connections.tsx(1 hunks)apps/app/src/routes/_layout/profile/_tabs/settings/_tabs/notifications.tsx(1 hunks)apps/app/src/routes/_layout/profile/_tabs/settings/_tabs/preferences.tsx(1 hunks)apps/app/src/routes/_layout/profile/_tabs/settings/index.tsx(1 hunks)apps/app/src/routes/_layout/profile/index.tsx(1 hunks)apps/app/src/routes/_layout/profile/settings/index.tsx(0 hunks)apps/app/src/routes/_layout/profile/settings/route.tsx(0 hunks)
💤 Files with no reviewable changes (5)
- apps/app/src/routes/_layout/profile/settings/index.tsx
- apps/app/src/components/profile/overview/index.tsx
- apps/app/src/components/profile/my-feeds/index.tsx
- apps/app/src/components/profile/ProfileTabs.tsx
- apps/app/src/routes/_layout/profile/settings/route.tsx
✅ Files skipped from review due to trivial changes (5)
- apps/app/src/routes/_layout/profile/_tabs/settings/_tabs/connections.tsx
- apps/app/src/routes/_layout/profile/_tabs/settings/_tabs/preferences.tsx
- apps/app/src/components/feed/form/index.ts
- apps/app/src/components/feed/EditFeedHeader.tsx
- apps/app/src/components/ui/textarea.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- .prettierignore
- apps/app/src/routes/_layout/profile/_tabs/activity.tsx
- apps/app/src/routes/_layout/profile/index.tsx
🧰 Additional context used
🧬 Code Graph Analysis (14)
apps/app/src/routes/_layout/profile/_tabs/settings/_tabs/notifications.tsx (4)
apps/app/src/routes/_layout/profile/_tabs/settings/_tabs.tsx (1)
Route(14-16)apps/app/src/routes/_layout/profile/_tabs/settings/_tabs/preferences.tsx (1)
Route(4-8)apps/app/src/routes/_layout/profile/_tabs/settings/_tabs/connections.tsx (1)
Route(7-11)apps/app/src/components/coming-soon.tsx (1)
ComingSoon(11-53)
apps/app/src/routes/_layout/profile/_tabs/settings/index.tsx (5)
apps/app/src/routes/_layout/profile.tsx (1)
Route(7-9)apps/app/src/routes/_layout/profile/_tabs.tsx (1)
Route(45-47)apps/app/src/routes/_layout/profile/_tabs/settings/_tabs.tsx (1)
Route(14-16)apps/app/src/routes/_layout/profile/_tabs/settings/_tabs/connections.tsx (1)
Route(7-11)apps/app/src/routes/_layout/profile/index.tsx (1)
Route(3-7)
apps/app/src/routes/_layout/profile/_tabs/overview.tsx (4)
apps/app/src/routes/_layout/profile/_tabs.tsx (1)
Route(45-47)apps/app/src/routes/_layout/profile/index.tsx (1)
Route(3-7)apps/app/src/components/profile/overview/ApproverFor.tsx (1)
ApproverFor(23-59)apps/app/src/components/profile/overview/CuratorFor.tsx (1)
CuratorFor(23-59)
apps/app/src/components/feed/EditFeedActions.tsx (1)
packages/types/src/config.ts (1)
FeedConfig(105-105)
apps/app/src/routes/_layout/profile/_tabs/settings/_tabs.tsx (8)
apps/app/src/routes/_layout/profile.tsx (1)
Route(7-9)apps/app/src/routes/_layout/profile/_tabs.tsx (1)
Route(45-47)apps/app/src/routes/_layout/profile/_tabs/settings/_tabs/notifications.tsx (1)
Route(4-8)apps/app/src/routes/_layout/profile/_tabs/settings/_tabs/preferences.tsx (1)
Route(4-8)apps/app/src/routes/_layout/profile/_tabs/settings/_tabs/connections.tsx (1)
Route(7-11)apps/app/src/routes/_layout/profile/index.tsx (1)
Route(3-7)apps/app/src/routes/_layout/profile/_tabs/settings/index.tsx (1)
Route(3-7)apps/app/src/components/ui/tabs.tsx (4)
Tabs(53-53)TabsList(53-53)TabsTrigger(53-53)TabsContent(53-53)
apps/app/src/components/feed/EditFeedImageSection.tsx (2)
packages/types/src/config.ts (1)
FeedConfig(105-105)apps/app/src/hooks/use-toast.ts (1)
toast(211-211)
apps/app/src/components/feed/form/ModerationSection.tsx (2)
apps/app/src/components/feed/form/index.ts (2)
FormValues(7-7)ModerationSection(5-5)apps/app/src/components/feed/form/types.ts (1)
FormValues(27-27)
apps/app/src/components/feed/form/types.ts (1)
packages/types/src/config.ts (4)
SourceConfigSchema(59-67)TransformConfigSchema(14-17)DistributorConfigSchema(20-24)RecapConfigSchema(34-43)
apps/app/src/components/feed/form/RecapsSection.tsx (1)
apps/app/src/components/feed/form/types.ts (1)
FormValues(27-27)
apps/app/src/routes/_layout/edit/feed/$feedId.tsx (6)
apps/app/src/hooks/use-edit-feed.ts (1)
useEditFeed(7-152)apps/app/src/components/feed/EditFeedLoadingState.tsx (1)
EditFeedLoadingState(11-46)apps/app/src/components/feed/EditFeedHeader.tsx (1)
EditFeedHeader(8-21)apps/app/src/components/feed/EditFeedImageSection.tsx (1)
EditFeedImageSection(11-36)apps/app/src/components/feed/EditFeedConfigSection.tsx (1)
EditFeedConfigSection(16-96)apps/app/src/components/feed/EditFeedActions.tsx (1)
EditFeedActions(17-60)
apps/app/src/routes/_layout/profile.tsx (2)
apps/app/src/contexts/auth-context.tsx (1)
useAuth(28-34)apps/app/src/components/profile/ProfileHeader.tsx (1)
ProfileHeader(4-103)
apps/app/src/components/feed/EditFeedConfigSection.tsx (3)
packages/types/src/config.ts (1)
FeedConfig(105-105)apps/app/src/components/feed/EditFeedForm.tsx (2)
EditFeedFormRef(28-30)EditFeedForm(32-364)apps/app/src/hooks/use-toast.ts (1)
toast(211-211)
apps/app/src/components/feed/form/StreamSettingsSection.tsx (1)
apps/app/src/components/feed/form/types.ts (1)
FormValues(27-27)
apps/app/src/components/feed/form/IngestionSection.tsx (1)
apps/app/src/components/feed/form/types.ts (1)
FormValues(27-27)
🪛 Biome (1.9.4)
apps/app/src/routes/_layout/profile/_tabs/my-feeds.tsx
[error] 47-48: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 51-52: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 166-167: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (21)
apps/app/src/routes/_layout/profile/_tabs/settings/index.tsx (1)
1-7: LGTM! Consistent redirect pattern implemented.The implementation follows the established pattern seen in other route files like
/profile/index.tsx. The redirect logic is clean and appropriately usesbeforeLoadto handle navigation.apps/app/src/routes/_layout/profile/_tabs/settings/_tabs/notifications.tsx (1)
4-24: Well-structured placeholder implementation with clear feature roadmap.The route definition follows the established pattern for nested settings tabs. The use of the
ComingSooncomponent is appropriate and provides users with clear expectations about upcoming notification features. The feature list is comprehensive and covers the key notification scenarios users would expect.apps/app/src/components/ui/tabs.tsx (2)
15-15: Updated tabs styling improves alignment and visual hierarchy.The styling changes shift from a compact, centered layout to a wider, left-aligned design with improved contrast. The change from
justify-centertojustify-startand the white background with border provide better visual separation and alignment for the new profile tabs layout.
30-30: Enhanced active tab styling with better contrast.The updated active tab styling (
data-[state=active]:bg-black data-[state=active]:text-white) provides excellent contrast and clearly indicates the selected tab. The increased font size (text-base) and adjusted padding improve readability and touch targets. Dark mode styles are properly maintained.apps/app/src/routes/_layout/profile.tsx (1)
25-35: Well-structured layout with proper responsive design and component composition.The authenticated layout is well-designed with:
- Consistent responsive spacing using utility classes
- Proper use of Container component for layout constraints
- Clean separation of ProfileHeader and nested route content via Outlet
- Good gap spacing that scales across screen sizes
The layout structure follows React Router best practices and provides a solid foundation for the nested profile routes.
apps/app/src/components/feed/EditFeedImageSection.tsx (1)
27-32: LGTM: Clean integration with ImageUpload component.The integration with the
ImageUploadcomponent looks good. The props are appropriately passed, including the initial image URL from the config and the upload success callback.apps/app/src/components/feed/form/types.ts (1)
1-28: Well-structured Zod schema definition.The
FeedConfigFormSchemais comprehensive and follows Zod best practices. The validation rules are appropriate, especially the minimum values for required fields and proper optional field handling. The schema provides good type safety for the form components.apps/app/src/routes/_layout/profile/_tabs/my-feeds.tsx (1)
32-77: Well-structured component with good UX patterns.The component demonstrates good practices with proper loading states, error handling, and user-friendly empty states. The filtering logic is comprehensive and the navigation integration is clean.
apps/app/src/routes/_layout/edit/feed/$feedId.tsx (1)
14-77: Excellent component architecture and separation of concerns.This component demonstrates excellent React patterns:
- Clean separation of business logic into the
useEditFeedhook- Proper component composition with specialized sub-components
- Good error boundary handling with
EditFeedLoadingState- Clear prop passing and state management
The structure is maintainable and follows React best practices effectively.
apps/app/src/components/feed/form/RecapsSection.tsx (1)
84-100: LGTM! Good cron expression handling.The cron schedule field provides helpful placeholder text and description. The implementation correctly handles the form field binding and validation messaging.
apps/app/src/components/feed/EditFeedConfigSection.tsx (2)
25-38: LGTM! Proper error handling for JSON operations.The error handling correctly catches JSON stringification errors, logs them for debugging, and provides user feedback through toast notifications. The dual callback pattern (onConfigChange + onJsonChange) maintains proper state synchronization.
78-92: LGTM! Clean conditional rendering with proper component isolation.The conditional rendering between JSON and form modes is well-implemented. The ref forwarding to EditFeedForm allows proper imperative control when needed.
apps/app/src/components/feed/form/BasicFieldsSection.tsx (3)
78-103: LGTM! Excellent number input handling.The polling interval field correctly handles number conversion and includes proper minimum validation (1000ms) with clear user guidance. The custom onChange handler properly converts string to number or undefined.
56-76: LGTM! Well-structured checkbox field with clear description.The enabled field follows React Hook Form best practices with proper controlled checkbox implementation and helpful user guidance about the feed's behavior when enabled.
20-22: LGTM! Proper component memoization.The component is correctly memoized with a display name, which will help with debugging and performance optimization.
apps/app/src/components/feed/form/IngestionSection.tsx (2)
49-69: LGTM! Excellent conditional field enabling pattern.The schedule input is properly disabled when ingestion is not enabled, using the
watchfunction to reactively control the field state. The cron expression example and description provide clear guidance to users.
28-47: LGTM! Clear checkbox implementation with helpful description.The ingestion enabled checkbox follows the established pattern with proper controlled state and provides clear user guidance about scheduled ingestion functionality.
apps/app/src/components/feed/form/StreamSettingsSection.tsx (3)
87-99: LGTM! Excellent JSON parsing error handling.The onBlur validation pattern correctly handles invalid JSON by reverting to the last valid state rather than losing user input. This prevents data loss while maintaining form consistency.
Also applies to: 142-154
71-75: LGTM! Proper useEffect synchronization for external form updates.The useEffect hooks correctly sync local JSON state when the form values change from external sources (like form resets), ensuring consistency between the JSON text input and form state.
Also applies to: 126-130
199-327: LGTM! Well-structured conditional rendering and dynamic arrays.The stream settings section properly shows/hides based on the enabled state, and the dynamic field arrays for transforms and distributors are well-implemented with proper add/remove functionality and clear labeling.
apps/app/src/routeTree.gen.ts (1)
1-859: Skip review for auto-generated fileThis file is automatically generated by TanStack Router and should not be manually modified as stated in the header comments.
| const router = useRouterState(); | ||
| const currentPath = router.location.pathname; | ||
| const activeTab = TABS.find((tab) => currentPath.startsWith(tab.to)); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Potential issue with active tab detection logic.
The startsWith method for active tab detection could cause incorrect tab highlighting when paths overlap. For example, if there were a /profile/overview-extended route, it would incorrectly match the "Overview" tab.
Consider using exact path matching or a more robust path comparison:
- const activeTab = TABS.find((tab) => currentPath.startsWith(tab.to));
+ const activeTab = TABS.find((tab) =>
+ currentPath === tab.to || currentPath.startsWith(tab.to + "/")
+ );📝 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.
| const router = useRouterState(); | |
| const currentPath = router.location.pathname; | |
| const activeTab = TABS.find((tab) => currentPath.startsWith(tab.to)); | |
| const router = useRouterState(); | |
| const currentPath = router.location.pathname; | |
| const activeTab = TABS.find((tab) => | |
| currentPath === tab.to || currentPath.startsWith(tab.to + "/") | |
| ); |
🤖 Prompt for AI Agents
In apps/app/src/routes/_layout/profile/_tabs.tsx around lines 50 to 52, the use
of startsWith for determining the active tab can cause incorrect tab
highlighting when paths overlap. To fix this, replace startsWith with an exact
path comparison or implement a more precise matching logic that distinguishes
between similar paths, ensuring only the correct tab is highlighted for the
current route.
| {TABS.map(({ to, label, icon: Icon }) => ( | ||
| <Link key={label} to={to}> | ||
| <TabsTrigger value={to}> | ||
| <Icon strokeWidth={1} size={20} /> | ||
| <span>{label}</span> | ||
| </TabsTrigger> | ||
| </Link> | ||
| ))} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Review the Link wrapper around TabsTrigger pattern.
Wrapping TabsTrigger with Link is an unconventional pattern that might interfere with the tabs component's built-in accessibility and keyboard navigation features. This could create nested interactive elements which is generally not recommended for accessibility.
Consider using the tabs component's built-in navigation handling:
- {TABS.map(({ to, label, icon: Icon }) => (
- <Link key={label} to={to}>
- <TabsTrigger value={to}>
- <Icon strokeWidth={1} size={20} />
- <span>{label}</span>
- </TabsTrigger>
- </Link>
- ))}
+ {TABS.map(({ to, label, icon: Icon }) => (
+ <TabsTrigger
+ key={label}
+ value={to}
+ onClick={() => navigate({ to })}
+ >
+ <Icon strokeWidth={1} size={20} />
+ <span>{label}</span>
+ </TabsTrigger>
+ ))}You'll need to import useNavigate and add the navigation logic.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/app/src/routes/_layout/profile/_tabs.tsx around lines 57 to 64, the
current pattern wraps TabsTrigger with a Link component, which can cause
accessibility and keyboard navigation issues due to nested interactive elements.
To fix this, remove the Link wrapper and instead import useNavigate from the
routing library. Use useNavigate to programmatically handle navigation inside
the TabsTrigger's onClick or onValueChange handler, ensuring the tabs component
manages navigation without nested links.
| if (isLoadingFeed) { | ||
| return ( | ||
| <div className="fixed inset-0 flex items-center justify-center z-50"> | ||
| <div className="flex flex-col items-center gap-4"> | ||
| <Loading /> | ||
| <p className="text-lg text-gray-600">Loading feed details...</p> | ||
| </div> | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider accessibility and UX issues with the fixed loading overlay.
The fixed positioning with z-50 creates a full-screen overlay that could cause focus management issues and doesn't follow typical loading state patterns. This implementation might interfere with other UI elements and doesn't provide a way to cancel the operation.
Consider a more contained loading approach:
- if (isLoadingFeed) {
- return (
- <div className="fixed inset-0 flex items-center justify-center z-50">
- <div className="flex flex-col items-center gap-4">
- <Loading />
- <p className="text-lg text-gray-600">Loading feed details...</p>
- </div>
- </div>
- );
- }
+ if (isLoadingFeed) {
+ return (
+ <div className="flex items-center justify-center min-h-[400px]">
+ <div className="flex flex-col items-center gap-4">
+ <Loading />
+ <p className="text-lg text-gray-600">Loading feed details...</p>
+ </div>
+ </div>
+ );
+ }📝 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.
| if (isLoadingFeed) { | |
| return ( | |
| <div className="fixed inset-0 flex items-center justify-center z-50"> | |
| <div className="flex flex-col items-center gap-4"> | |
| <Loading /> | |
| <p className="text-lg text-gray-600">Loading feed details...</p> | |
| </div> | |
| </div> | |
| ); | |
| } | |
| if (isLoadingFeed) { | |
| return ( | |
| <div className="flex items-center justify-center min-h-[400px]"> | |
| <div className="flex flex-col items-center gap-4"> | |
| <Loading /> | |
| <p className="text-lg text-gray-600">Loading feed details...</p> | |
| </div> | |
| </div> | |
| ); | |
| } |
🤖 Prompt for AI Agents
In apps/app/src/components/feed/EditFeedLoadingState.tsx around lines 16 to 25,
the current fixed full-screen loading overlay with z-50 can cause accessibility
and UX issues by trapping focus and blocking interaction without cancellation.
Refactor the loading state to use a more contained, inline loading indicator
within the feed details area instead of a fixed overlay. Remove the fixed
positioning and z-index, and ensure the loading UI does not block other UI
elements or trap keyboard focus, improving accessibility and user control.
| <Button | ||
| onClick={onSaveChanges} | ||
| disabled={updateFeedMutation.isPending} | ||
| className="flex-1 sm:flex-none" | ||
| size="lg" | ||
| > | ||
| {updateFeedMutation.isPending ? ( | ||
| <> | ||
| <Loading /> | ||
| <span className="ml-2">Saving...</span> | ||
| </> | ||
| ) : ( | ||
| "Save Changes" | ||
| )} | ||
| </Button> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding error handling and confirmation for destructive actions.
The save button should ideally have some form of validation, and the delete button should include a confirmation dialog to prevent accidental deletions.
Consider adding validation for the save action and confirmation for delete:
<Button
onClick={onSaveChanges}
disabled={updateFeedMutation.isPending}
className="flex-1 sm:flex-none"
size="lg"
>
{updateFeedMutation.isPending ? (
<>
<Loading />
<span className="ml-2">Saving...</span>
</>
) : (
"Save Changes"
)}
</Button>For the delete button, consider integrating a confirmation dialog or modal before executing the delete action.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/app/src/components/feed/EditFeedActions.tsx around lines 26 to 40, add
validation logic before allowing the save action to proceed, ensuring data
correctness. For the delete button, implement a confirmation dialog or modal
that prompts the user to confirm the deletion before executing the delete
mutation, preventing accidental destructive actions.
| const handleImageUploaded = (ipfsHash: string, ipfsUrl: string) => { | ||
| onImageUploaded(ipfsHash, ipfsUrl); | ||
| toast({ | ||
| title: "Image Updated", | ||
| description: "Image URL has been updated in the JSON config.", | ||
| }); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider error handling for image upload failures.
The current implementation only handles successful uploads. Consider adding error handling for upload failures to provide better user feedback.
const handleImageUploaded = (ipfsHash: string, ipfsUrl: string) => {
- onImageUploaded(ipfsHash, ipfsUrl);
- toast({
- title: "Image Updated",
- description: "Image URL has been updated in the JSON config.",
- });
+ try {
+ onImageUploaded(ipfsHash, ipfsUrl);
+ toast({
+ title: "Image Updated",
+ description: "Image URL has been updated in the JSON config.",
+ });
+ } catch (error) {
+ toast({
+ title: "Upload Failed",
+ description: "Failed to update the image. Please try again.",
+ variant: "destructive",
+ });
+ }
};Also consider adding an onUploadError callback to the ImageUpload component props if it supports error handling.
📝 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.
| const handleImageUploaded = (ipfsHash: string, ipfsUrl: string) => { | |
| onImageUploaded(ipfsHash, ipfsUrl); | |
| toast({ | |
| title: "Image Updated", | |
| description: "Image URL has been updated in the JSON config.", | |
| }); | |
| }; | |
| const handleImageUploaded = (ipfsHash: string, ipfsUrl: string) => { | |
| try { | |
| onImageUploaded(ipfsHash, ipfsUrl); | |
| toast({ | |
| title: "Image Updated", | |
| description: "Image URL has been updated in the JSON config.", | |
| }); | |
| } catch (error) { | |
| toast({ | |
| title: "Upload Failed", | |
| description: "Failed to update the image. Please try again.", | |
| variant: "destructive", | |
| }); | |
| } | |
| }; |
🤖 Prompt for AI Agents
In apps/app/src/components/feed/EditFeedImageSection.tsx around lines 15 to 21,
the handleImageUploaded function currently only handles successful image uploads
without any error handling. Add error handling logic to manage upload failures
by showing an error toast notification or message to the user. Additionally, if
the ImageUpload component supports an onUploadError callback prop, add this prop
and implement a handler function to capture and respond to upload errors
appropriately.
| <div className="grid grid-cols-1 sm:grid-cols-2 md:grid-cols-3 gap-4 sm:gap-5"> | ||
| {filteredFeeds && | ||
| filteredFeeds.map((feed) => <FeedCard key={feed.id} feed={feed} />)} | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Apply optional chaining for feed rendering.
Use optional chaining for the filteredFeeds array access as suggested by static analysis.
<div className="grid grid-cols-1 sm:grid-cols-2 md:grid-cols-3 gap-4 sm:gap-5">
- {filteredFeeds &&
- filteredFeeds.map((feed) => <FeedCard key={feed.id} feed={feed} />)}
+ {filteredFeeds?.map((feed) => <FeedCard key={feed.id} feed={feed} />)}
</div>📝 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="grid grid-cols-1 sm:grid-cols-2 md:grid-cols-3 gap-4 sm:gap-5"> | |
| {filteredFeeds && | |
| filteredFeeds.map((feed) => <FeedCard key={feed.id} feed={feed} />)} | |
| </div> | |
| )} | |
| <div className="grid grid-cols-1 sm:grid-cols-2 md:grid-cols-3 gap-4 sm:gap-5"> | |
| - {filteredFeeds && | |
| - filteredFeeds.map((feed) => <FeedCard key={feed.id} feed={feed} />)} | |
| + {filteredFeeds?.map((feed) => <FeedCard key={feed.id} feed={feed} />)} | |
| </div> | |
| )} |
🧰 Tools
🪛 Biome (1.9.4)
[error] 166-167: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🤖 Prompt for AI Agents
In apps/app/src/routes/_layout/profile/_tabs/my-feeds.tsx around lines 165 to
169, the rendering of filteredFeeds does not use optional chaining, which can
cause errors if filteredFeeds is undefined or null. Update the code to use
optional chaining when accessing filteredFeeds, changing filteredFeeds.map(...)
to filteredFeeds?.map(...) to safely handle cases where filteredFeeds might be
undefined.
| const filteredFeeds = useMemo(() => { | ||
| return feeds.filter((feed) => { | ||
| const matchesSearch = | ||
| !filters.term || | ||
| feed.name.toLowerCase().includes(filters.term.toLowerCase()) || | ||
| (feed.description && | ||
| feed.description.toLowerCase().includes(filters.term.toLowerCase())); | ||
|
|
||
| const isComplete = !!( | ||
| feed.config && | ||
| feed.config.name && | ||
| feed.config.description && | ||
| feed.config.sources && | ||
| feed.config.sources.length > 0 | ||
| ); | ||
|
|
||
| if (filters.value === "all") return matchesSearch; | ||
| if (filters.value === "completed") return matchesSearch && isComplete; | ||
| if (filters.value === "incomplete") return matchesSearch && !isComplete; | ||
|
|
||
| return matchesSearch; | ||
| }); | ||
| }, [feeds, filters]); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Apply optional chaining for safer property access.
The static analysis correctly identifies opportunities to use optional chaining for safer property access.
Apply these changes for safer code:
const matchesSearch =
!filters.term ||
feed.name.toLowerCase().includes(filters.term.toLowerCase()) ||
- (feed.description &&
- feed.description.toLowerCase().includes(filters.term.toLowerCase()));
+ feed.description?.toLowerCase().includes(filters.term.toLowerCase());
const isComplete = !!(
- feed.config &&
- feed.config.name &&
- feed.config.description &&
- feed.config.sources &&
- feed.config.sources.length > 0
+ feed.config?.name &&
+ feed.config?.description &&
+ feed.config?.sources?.length > 0
);Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Biome (1.9.4)
[error] 47-48: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 51-52: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🤖 Prompt for AI Agents
In apps/app/src/routes/_layout/profile/_tabs/my-feeds.tsx between lines 42 and
64, the code accesses nested properties like feed.config.name and
feed.config.sources without optional chaining, which can cause runtime errors if
any intermediate property is undefined. Update these property accesses to use
optional chaining (e.g., feed.config?.name, feed.config?.sources?.length) to
safely handle cases where feed.config or its nested properties might be
undefined.
| setTimeout(() => { | ||
| formRef.current?.updateFromConfig(); | ||
| }, 0); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace setTimeout with React scheduling.
Using setTimeout(0) is a code smell and can be unreliable. Consider using React's built-in scheduling mechanisms for better consistency.
- setTimeout(() => {
- formRef.current?.updateFromConfig();
- }, 0);
+ React.startTransition(() => {
+ formRef.current?.updateFromConfig();
+ });Or use useEffect with a dependency:
+ useEffect(() => {
+ if (!isJsonMode) {
+ formRef.current?.updateFromConfig();
+ }
+ }, [isJsonMode]);🤖 Prompt for AI Agents
In apps/app/src/components/feed/EditFeedConfigSection.tsx around lines 43 to 45,
replace the setTimeout call with React's scheduling by using useEffect to
trigger formRef.current?.updateFromConfig() when the relevant dependencies
change. Remove the setTimeout and instead place the update logic inside a
useEffect hook with appropriate dependencies to ensure consistent and reliable
execution within React's lifecycle.
| function TransformConfigField({ | ||
| control, | ||
| transformIndex, | ||
| }: { | ||
| control: Control<FormValues>; | ||
| transformIndex: number; | ||
| }) { | ||
| const { | ||
| field, | ||
| fieldState: { error }, | ||
| } = useController({ | ||
| name: `streamTransforms.${transformIndex}.config`, | ||
| control, | ||
| }); | ||
|
|
||
| const [localValue, setLocalValue] = useState( | ||
| field.value ? JSON.stringify(field.value, null, 2) : "{}", | ||
| ); | ||
|
|
||
| // Sync local value when field value changes from external sources (like form reset) | ||
| useEffect(() => { | ||
| const formValue = field.value ? JSON.stringify(field.value, null, 2) : "{}"; | ||
| setLocalValue(formValue); | ||
| }, [field.value]); | ||
|
|
||
| return ( | ||
| <FormItem> | ||
| <FormLabel>Transform Config (JSON)</FormLabel> | ||
| <FormControl> | ||
| <Textarea | ||
| placeholder='{"key": "value"}' | ||
| value={localValue} | ||
| onChange={(e) => { | ||
| setLocalValue(e.target.value); | ||
| }} | ||
| onBlur={() => { | ||
| try { | ||
| const parsed = JSON.parse(localValue || "{}"); | ||
| field.onChange(parsed); | ||
| } catch { | ||
| // If JSON is invalid, keep the current value and don't update | ||
| // This prevents losing data while typing | ||
| const formValue = field.value | ||
| ? JSON.stringify(field.value, null, 2) | ||
| : "{}"; | ||
| setLocalValue(formValue); | ||
| } | ||
| }} | ||
| /> | ||
| </FormControl> | ||
| {error && <FormMessage>{error.message}</FormMessage>} | ||
| </FormItem> | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider extracting a reusable JSONConfigField component.
The TransformConfigField and DistributorConfigField components have nearly identical logic. This code duplication could be eliminated by creating a generic JSONConfigField component.
+function JSONConfigField({
+ control,
+ name,
+ label = "Config (JSON)",
+ placeholder = '{"key": "value"}',
+}: {
+ control: Control<FormValues>;
+ name: string;
+ label?: string;
+ placeholder?: string;
+}) {
+ const {
+ field,
+ fieldState: { error },
+ } = useController({ name, control });
+
+ const [localValue, setLocalValue] = useState(
+ field.value ? JSON.stringify(field.value, null, 2) : "{}",
+ );
+
+ useEffect(() => {
+ const formValue = field.value ? JSON.stringify(field.value, null, 2) : "{}";
+ setLocalValue(formValue);
+ }, [field.value]);
+
+ return (
+ <FormItem>
+ <FormLabel>{label}</FormLabel>
+ <FormControl>
+ <Textarea
+ placeholder={placeholder}
+ value={localValue}
+ onChange={(e) => setLocalValue(e.target.value)}
+ onBlur={() => {
+ try {
+ const parsed = JSON.parse(localValue || "{}");
+ field.onChange(parsed);
+ } catch {
+ const formValue = field.value
+ ? JSON.stringify(field.value, null, 2)
+ : "{}";
+ setLocalValue(formValue);
+ }
+ }}
+ />
+ </FormControl>
+ {error && <FormMessage>{error.message}</FormMessage>}
+ </FormItem>
+ );
+}Then replace both components with:
- <TransformConfigField
- control={control}
- transformIndex={transformIndex}
- />
+ <JSONConfigField
+ control={control}
+ name={`streamTransforms.${transformIndex}.config`}
+ label="Transform Config (JSON)"
+ />Also applies to: 107-160
🤖 Prompt for AI Agents
In apps/app/src/components/feed/form/StreamSettingsSection.tsx around lines 52
to 105 and 107 to 160, the TransformConfigField and DistributorConfigField
components contain nearly identical logic for handling JSON configuration input.
To fix this, create a reusable generic JSONConfigField component that accepts
props for control and the field name path, encapsulating the JSON parsing,
stringifying, and error handling logic. Then replace both TransformConfigField
and DistributorConfigField with this new JSONConfigField component, passing the
appropriate field name paths to reduce code duplication and improve
maintainability.
| function EditFeedComponent() { | ||
| const { feedId } = Route.useParams(); | ||
| export function useEditFeed(feedId: string): { | ||
| feedData: unknown; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve type safety for feedData
The feedData is typed as unknown, which reduces type safety. Consider using a proper type definition based on the API response structure.
-feedData: unknown;
+feedData: { config: FeedConfig; [key: string]: any } | undefined;📝 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.
| feedData: unknown; | |
| - feedData: unknown; | |
| + feedData: { config: FeedConfig; [key: string]: any } | undefined; |
🤖 Prompt for AI Agents
In apps/app/src/hooks/use-edit-feed.ts at line 8, the feedData variable is
currently typed as unknown, which limits type safety. Replace the unknown type
with a specific type that matches the expected structure of the API response for
feedData. Define or import the appropriate interface or type that accurately
represents the feed data shape to improve type safety and enable better type
checking.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
apps/app/src/hooks/use-edit-feed.ts (1)
8-8: Improve type safety for feedData.The
feedDatais typed asunknown, which reduces type safety. This is the same issue identified in a previous review.
🧹 Nitpick comments (1)
apps/app/src/hooks/use-edit-feed.ts (1)
138-151: Consider memoizing the return object.The return object is recreated on every render, which could cause unnecessary re-renders in consuming components.
+import { useMemo } from "react"; // ... inside the hook +return useMemo(() => ({ feedData, isLoadingFeed, feedError, currentConfig, jsonString, updateFeedMutation, deleteFeedMutation, handleImageUploaded, handleJsonChange, handleConfigChange, handleSaveChanges, handleDeleteFeed, +}), [ + feedData, + isLoadingFeed, + feedError, + currentConfig, + jsonString, + updateFeedMutation, + deleteFeedMutation, + handleImageUploaded, + handleJsonChange, + handleConfigChange, + handleSaveChanges, + handleDeleteFeed, +]);
📜 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 (19)
apps/api/src/routes/api/processing.ts(1 hunks)apps/app/package.json(2 hunks)apps/app/src/components/FilterControls.tsx(3 hunks)apps/app/src/components/feed/EditFeedForm.tsx(1 hunks)apps/app/src/components/processing/ProcessingActions.tsx(1 hunks)apps/app/src/components/processing/ProcessingHistory.tsx(5 hunks)apps/app/src/components/processing/ProcessingStepDetails.tsx(3 hunks)apps/app/src/hooks/use-edit-feed.ts(6 hunks)apps/app/src/hooks/useLeaderboard.ts(6 hunks)apps/app/src/lib/api/processing.ts(1 hunks)apps/app/src/routes/_layout/create/feed/index.tsx(5 hunks)apps/app/src/routes/_layout/profile/_tabs/settings/_tabs.tsx(1 hunks)apps/app/src/store/feed-creation-store.ts(1 hunks)packages/core-services/src/service-provider.ts(2 hunks)packages/core-services/src/services/processing.service.ts(7 hunks)packages/shared-db/migrations/0021_ambitious_iron_man.sql(1 hunks)packages/shared-db/migrations/meta/0021_snapshot.json(1 hunks)packages/shared-db/migrations/meta/_journal.json(1 hunks)packages/shared-db/src/schema/processing.ts(3 hunks)
✅ Files skipped from review due to trivial changes (2)
- apps/app/src/store/feed-creation-store.ts
- packages/shared-db/migrations/meta/_journal.json
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/app/src/routes/_layout/profile/_tabs/settings/_tabs.tsx
- apps/app/src/routes/_layout/create/feed/index.tsx
- apps/app/src/hooks/useLeaderboard.ts
- apps/app/src/components/feed/EditFeedForm.tsx
🧰 Additional context used
🧬 Code Graph Analysis (2)
packages/core-services/src/service-provider.ts (1)
packages/shared-db/src/repositories/submission.repository.ts (1)
SubmissionRepository(30-561)
apps/api/src/routes/api/processing.ts (3)
packages/types/src/api/processing.ts (2)
JobIdParamSchema(38-40)ProcessingJobRetryResponseSchema(167-174)packages/utils/src/errors.ts (2)
NotFoundError(205-216)ServiceError(168-177)packages/types/src/api/common.ts (1)
ApiErrorResponseSchema(25-32)
🔇 Additional comments (25)
apps/app/package.json (1)
45-45: LGTM! Good modernization with lodash-es.The migration from
lodashtolodash-esimproves tree-shaking capabilities and aligns with modern ES module standards. The corresponding TypeScript type update ensures proper type support.Also applies to: 69-69
apps/app/src/components/FilterControls.tsx (4)
6-6: LGTM! Import aligns with package.json modernization.The import of
debouncefromlodash-esis consistent with the dependency migration in package.json.
51-62: Excellent debounce implementation.The refactoring to use lodash-es debounce is well-implemented:
useRefcorrectly prevents function recreation on re-renders- 300ms delay is appropriate for search input
- The implementation is more robust than manual timeout management
64-68: Clean and simplified search handler.The
handleSearchChangefunction is now much cleaner by delegating debounce logic to the lodash utility.
71-71: Proper debounce cleanup and cancellation.Good implementation of debounce cancellation on filter application and component unmount. This prevents potential memory leaks and ensures proper cleanup.
Also applies to: 90-90
apps/app/src/hooks/use-edit-feed.ts (5)
2-5: LGTM: Import updates are appropriate.The import changes correctly reflect the refactoring from component to hook, adding navigation and updated API imports.
77-81: LGTM: Toast error handling is consistent.The toast error handling follows the same pattern as other error cases in the hook.
100-100: LGTM: Mutation call is correct.The async mutation call properly awaits the result for error handling.
125-125: ```shell
#!/bin/bashSearch for route definitions related to 'my-feeds' and 'profile'
rg -n '"/profile/my-feeds"' apps/app/src
rg -n 'my-feeds' apps/app/src
rg -n '"/profile/' apps/app/src--- `85-88`: **Add input validation for handleConfigChange.** The function doesn't validate the config parameter, which could lead to runtime errors. ```diff const handleConfigChange = (config: FeedConfig) => { + if (!config || typeof config !== 'object') { + toast({ + title: "Invalid Configuration", + description: "Configuration must be a valid object.", + variant: "destructive", + }); + return; + } + setCurrentConfig(config); setJsonString(JSON.stringify(config, null, 2)); };Likely an incorrect or invalid review comment.
packages/core-services/src/service-provider.ts (2)
61-61: LGTM: Proper dependency injection patternThe SubmissionRepository instance is correctly instantiated with the database dependency, following the same pattern as other repositories in the constructor.
102-103: LGTM: Enhanced ProcessingService dependenciesThe addition of feedRepository and submissionRepository parameters to the ProcessingService constructor aligns with the enhanced processing capabilities mentioned in the AI summary. This provides the processing service with access to both feed and submission data needed for the new processing plan features.
packages/shared-db/migrations/0021_ambitious_iron_man.sql (2)
1-1: LGTM: Appropriate data type for processing plansAdding a JSONB column for the processing plan is the correct approach for storing the complex processing plan data structure. JSONB provides efficient storage and querying capabilities for the processing step sequences.
2-2: LGTM: Data integrity improvementThe lowercase check constraint on the feeds table enforces consistent feed ID formatting at the database level, which prevents data inconsistencies and aligns with the repository normalization mentioned in the AI summary.
apps/app/src/lib/api/processing.ts (2)
94-114: LGTM: Well-structured reprocess job hookThe
useReprocessJobhook follows the established pattern in the file with proper TypeScript typing, correct path construction, and consistent onSuccess placeholder. The implementation correctly handles the job reprocessing functionality.
116-136: LGTM: Comprehensive tweak and reprocess hookThe
useTweakAndReprocessStephook is properly implemented with correct parameter typing for bothstepIdandnewInput. The path template and return type are consistent with the backend API contract described in the AI summary.apps/app/src/components/processing/ProcessingHistory.tsx (2)
96-110: LGTM: Improved code organization with centralized actionsThe refactoring to use the
ProcessingActionscomponent improves maintainability by centralizing action logic. The explicit function body with styled JSX container provides better readability than inline JSX.
165-191: LGTM: Smart job lookup for step actionsThe addition of job lookup logic (line 169) is essential for providing proper context to the
ProcessingActionscomponent. The null check ensures the component handles cases where the job might not be found, preventing runtime errors.apps/app/src/components/processing/ProcessingStepDetails.tsx (3)
68-119: LGTM: Comprehensive metadata display enhancementThe enhanced dialog header provides valuable step information including status badge, type, stage, timestamps, duration, and step order. The grid layout and date formatting using
date-fnscreate a professional and informative UI.
30-38: LGTM: Proper state initialization patternThe
useEffecthook correctly initializes the editable input when the step changes, ensuring the textarea always reflects the current step's input data. The dependency array with[step]is appropriate for this use case.
128-150: LGTM: Well-implemented tweak-and-reprocess featureThe editable textarea with monospace font is perfect for JSON editing, and the button properly handles the loading state with a spinner. The mutation call passes the correct parameters, and closing the dialog on success provides good user experience.
apps/api/src/routes/api/processing.ts (1)
233-342: Well-structured API endpoints with proper validation and error handling.Both new endpoints follow the established patterns in the codebase with:
- Proper Zod validation for parameters and request bodies
- Consistent error handling for NotFoundError and ServiceError cases
- Structured API responses using defined schemas
- Appropriate logging for debugging
The implementation is clean and maintainable.
packages/shared-db/src/schema/processing.ts (1)
50-67: Clean schema implementation for processing plans.The ProcessingPlan abstraction is well-designed with:
- Clear type definitions for plan steps
- Proper typing for the JSONB column
- Optional plan field in the insert schema to maintain backward compatibility
The schema changes align well with the service layer implementation.
Also applies to: 131-131, 246-246
packages/core-services/src/services/processing.service.ts (1)
1-405: Excellent refactoring with the ProcessingPlan abstraction.The service implementation is well-structured with:
- Clean separation of concerns between plan building and execution
- Proper validation in retry methods checking for plan existence and step bounds
- Good error handling for JSON parsing in tweakAndReprocessStep
- Backward compatibility by building plans from existing configs
The explicit plan abstraction simplifies the processing flow and makes it more maintainable.
packages/shared-db/migrations/meta/0021_snapshot.json (1)
1601-1606: Migration snapshot correctly includes the plan column.The auto-generated snapshot properly captures the new
planJSONB column in theprocessing_jobstable, consistent with the schema changes.
| const handleImageUploaded = (_ipfsHash: string, ipfsUrl: string) => { | ||
| setCurrentConfig((prevConfig) => { | ||
| const newConfig = { | ||
| ...(prevConfig || { id: feedId, name: "", description: "" }), | ||
| image: ipfsUrl, | ||
| } as FeedConfig; | ||
| setJsonString(JSON.stringify(newConfig, null, 2)); | ||
| return newConfig; | ||
| }); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix type safety and improve error handling in handleImageUploaded.
The function has type safety issues and doesn't validate the input URL format.
-const handleImageUploaded = (_ipfsHash: string, ipfsUrl: string) => {
+const handleImageUploaded = (ipfsHash: string, ipfsUrl: string) => {
+ if (!ipfsUrl || typeof ipfsUrl !== 'string') {
+ toast({
+ title: "Invalid Image",
+ description: "Image URL is required and must be valid.",
+ variant: "destructive",
+ });
+ return;
+ }
+
setCurrentConfig((prevConfig) => {
- const newConfig = {
- ...(prevConfig || { id: feedId, name: "", description: "" }),
- image: ipfsUrl,
- } as FeedConfig;
+ if (!prevConfig) {
+ toast({
+ title: "Error",
+ description: "No feed configuration available to update.",
+ variant: "destructive",
+ });
+ return prevConfig;
+ }
+
+ const newConfig: FeedConfig = {
+ ...prevConfig,
+ image: ipfsUrl,
+ };
setJsonString(JSON.stringify(newConfig, null, 2));
return newConfig;
});
};Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/app/src/hooks/use-edit-feed.ts around lines 59 to 68, the
handleImageUploaded function lacks type safety and does not validate the ipfsUrl
input. Fix this by explicitly typing the function parameters and adding
validation logic to check if ipfsUrl is a valid URL before updating the state.
If the URL is invalid, handle the error gracefully, for example by logging an
error or ignoring the update. This ensures type safety and prevents invalid data
from being set.
| export function useEditFeed(feedId: string): { | ||
| feedData: unknown; | ||
| isLoadingFeed: boolean; | ||
| feedError: Error | null; | ||
| currentConfig: FeedConfig | null; | ||
| jsonString: string; | ||
| updateFeedMutation: ReturnType<typeof useUpdateFeed>; | ||
| deleteFeedMutation: ReturnType<typeof useDeleteFeed>; | ||
| handleImageUploaded: (ipfsHash: string, ipfsUrl: string) => void; | ||
| handleJsonChange: (newJsonString: string) => void; | ||
| handleConfigChange: (config: FeedConfig) => void; | ||
| handleSaveChanges: () => Promise<void>; | ||
| handleDeleteFeed: () => Promise<void>; | ||
| } { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider reducing the hook's public interface.
The hook exposes many internal implementation details (mutations, handlers, raw state). This creates tight coupling between the hook and its consumers, making future refactoring difficult.
Consider exposing a more focused interface:
export function useEditFeed(feedId: string): {
- feedData: unknown;
- isLoadingFeed: boolean;
- feedError: Error | null;
- currentConfig: FeedConfig | null;
- jsonString: string;
- updateFeedMutation: ReturnType<typeof useUpdateFeed>;
- deleteFeedMutation: ReturnType<typeof useDeleteFeed>;
+ feed: { data: unknown; isLoading: boolean; error: Error | null };
+ config: { current: FeedConfig | null; jsonString: string };
+ actions: {
handleImageUploaded: (ipfsHash: string, ipfsUrl: string) => void;
handleJsonChange: (newJsonString: string) => void;
handleConfigChange: (config: FeedConfig) => void;
handleSaveChanges: () => Promise<void>;
handleDeleteFeed: () => Promise<void>;
+ };
+ mutations: {
+ isUpdating: boolean;
+ isDeleting: boolean;
+ };
} {📝 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.
| export function useEditFeed(feedId: string): { | |
| feedData: unknown; | |
| isLoadingFeed: boolean; | |
| feedError: Error | null; | |
| currentConfig: FeedConfig | null; | |
| jsonString: string; | |
| updateFeedMutation: ReturnType<typeof useUpdateFeed>; | |
| deleteFeedMutation: ReturnType<typeof useDeleteFeed>; | |
| handleImageUploaded: (ipfsHash: string, ipfsUrl: string) => void; | |
| handleJsonChange: (newJsonString: string) => void; | |
| handleConfigChange: (config: FeedConfig) => void; | |
| handleSaveChanges: () => Promise<void>; | |
| handleDeleteFeed: () => Promise<void>; | |
| } { | |
| export function useEditFeed(feedId: string): { | |
| feed: { data: unknown; isLoading: boolean; error: Error | null }; | |
| config: { current: FeedConfig | null; jsonString: string }; | |
| actions: { | |
| handleImageUploaded: (ipfsHash: string, ipfsUrl: string) => void; | |
| handleJsonChange: (newJsonString: string) => void; | |
| handleConfigChange: (config: FeedConfig) => void; | |
| handleSaveChanges: () => Promise<void>; | |
| handleDeleteFeed: () => Promise<void>; | |
| }; | |
| mutations: { | |
| isUpdating: boolean; | |
| isDeleting: boolean; | |
| }; | |
| } { |
🤖 Prompt for AI Agents
In apps/app/src/hooks/use-edit-feed.ts around lines 7 to 20, the hook exposes
too many internal details like mutations, handlers, and raw state, causing tight
coupling with consumers. Refactor the hook to expose a simpler, more focused
interface by returning only the essential data and high-level actions needed by
consumers, hiding internal implementation details such as mutation objects and
individual handlers.
| onClick={() => | ||
| handleTweakAndReprocess(JSON.stringify(step.input, null, 2)) | ||
| } |
There was a problem hiding this comment.
Add validation for step.input before stringifying.
The code assumes step.input is always defined, but it could be null or undefined based on the ProcessingStep type definition.
Consider adding a validation check:
onClick={() =>
- handleTweakAndReprocess(JSON.stringify(step.input, null, 2))
+ handleTweakAndReprocess(
+ step.input ? JSON.stringify(step.input, null, 2) : "{}"
+ )
}📝 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.
| onClick={() => | |
| handleTweakAndReprocess(JSON.stringify(step.input, null, 2)) | |
| } | |
| onClick={() => | |
| handleTweakAndReprocess( | |
| step.input ? JSON.stringify(step.input, null, 2) : "{}" | |
| ) | |
| } |
🤖 Prompt for AI Agents
In apps/app/src/components/processing/ProcessingActions.tsx around lines 55 to
57, the code calls JSON.stringify on step.input without checking if step.input
is null or undefined, which can cause runtime errors. Add a validation check
before stringifying step.input to ensure it is defined; if it is null or
undefined, handle that case appropriately, such as passing an empty object or a
default value to JSON.stringify or preventing the function call.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/app/src/contexts/auth-context.tsx(4 hunks)
🔇 Additional comments (2)
apps/app/src/contexts/auth-context.tsx (2)
49-62: Verify the change from sessionStorage to localStorage is intentional.This change makes the authorization state persist across browser sessions. Users will remain authorized even after closing and reopening the browser, which is a significant behavioral change from the previous session-based approach.
Please confirm this persistence behavior aligns with your security requirements and user experience goals.
207-207: Good defensive programming!Explicitly resetting the authorization state ensures it's cleared even if the wallet sign-out operation fails.
| variant: "success", | ||
| duration: 1000, | ||
| }); | ||
| Promise.all([checkAuthorization(), initiateLogin(currentAccountId)]); |
There was a problem hiding this comment.
Add error handling for the Promise.all call.
The concurrent promises could fail independently. Consider wrapping in a try-catch or handling errors individually to prevent unhandled promise rejections.
- Promise.all([checkAuthorization(), initiateLogin(currentAccountId)]);
+ Promise.all([checkAuthorization(), initiateLogin(currentAccountId)])
+ .catch(error => {
+ console.error("Failed to complete authorization setup:", error);
+ });📝 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.
| Promise.all([checkAuthorization(), initiateLogin(currentAccountId)]); | |
| Promise.all([checkAuthorization(), initiateLogin(currentAccountId)]) | |
| .catch(error => { | |
| console.error("Failed to complete authorization setup:", error); | |
| }); |
🤖 Prompt for AI Agents
In apps/app/src/contexts/auth-context.tsx at line 126, the Promise.all call
lacks error handling, which can lead to unhandled promise rejections if any
promise fails. Wrap the Promise.all call inside a try-catch block to catch and
handle any errors that occur during the concurrent execution of
checkAuthorization() and initiateLogin(currentAccountId). This will ensure
proper error management and prevent unhandled rejections.
* Feat: add recent feeds * feat: add recent content to the main feed page * fmt * Update apps/app/src/hooks/use-rss-feed.ts Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * nitpicks --------- Co-authored-by: Elliot Braem <elliot@everything.dev> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Elliot Braem <elliot@ejlbraem.com>
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (2)
apps/app/src/components/rss-feed/RssFeedItem.tsx (1)
34-46: Remove unnecessary Fragment wrapper.The Fragment wrapper around the category badges is redundant since it contains only the map function output.
- {item.categories && item.categories.length > 0 && ( - <> - {item.categories.map((category, idx) => ( - <Badge - key={idx} - variant="outline" - className="cursor-pointer hover:bg-blue-50 transition-colors" - onClick={() => onCategoryClick(category)} - title={`Filter by ${category}`} - > - {category} - </Badge> - ))} - </> - )} + {item.categories && item.categories.length > 0 && + item.categories.map((category, idx) => ( + <Badge + key={idx} + variant="outline" + className="cursor-pointer hover:bg-blue-50 transition-colors" + onClick={() => onCategoryClick(category)} + title={`Filter by ${category}`} + > + {category} + </Badge> + )) + }apps/app/src/routes/_layout/feed/$feedId/_tabs/content.tsx (1)
122-126: Use useLayoutEffect from React import instead of React namespace.For consistency with other hooks in the file, use the direct import.
+import { useState, useMemo, useRef, useLayoutEffect } from "react"; // Setup window virtualization - React.useLayoutEffect(() => { + useLayoutEffect(() => { parentOffsetRef.current = parentRef.current?.offsetTop ?? 0; }, []);
📜 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 (12)
apps/app/package.json(3 hunks)apps/app/src/components/feed/FeedCard.tsx(1 hunks)apps/app/src/components/feed/RecentContent.tsx(1 hunks)apps/app/src/components/rss-feed/FilterBadges.tsx(1 hunks)apps/app/src/components/rss-feed/FilterControls.tsx(1 hunks)apps/app/src/components/rss-feed/RssFeedItem.tsx(1 hunks)apps/app/src/hooks/use-rss-feed.ts(1 hunks)apps/app/src/routeTree.gen.ts(2 hunks)apps/app/src/routes/_layout/feed/$feedId.tsx(2 hunks)apps/app/src/routes/_layout/feed/$feedId/_tabs.tsx(3 hunks)apps/app/src/routes/_layout/feed/$feedId/_tabs/content.tsx(1 hunks)apps/app/src/types/rss.ts(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- apps/app/package.json
- apps/app/src/types/rss.ts
🧰 Additional context used
🧬 Code Graph Analysis (3)
apps/app/src/routes/_layout/feed/$feedId.tsx (1)
apps/app/src/components/feed/RecentContent.tsx (1)
RecentContent(13-103)
apps/app/src/components/feed/FeedCard.tsx (2)
apps/app/src/components/rss-feed/RssFeedItem.tsx (1)
RssFeedItem(10-79)apps/app/src/types/rss.ts (1)
RssFeedItem(1-10)
apps/app/src/components/rss-feed/RssFeedItem.tsx (2)
apps/app/src/types/rss.ts (1)
RssFeedItem(1-10)apps/app/src/components/profile/my-feeds/card.tsx (1)
Card(18-89)
🪛 Biome (1.9.4)
apps/app/src/components/rss-feed/RssFeedItem.tsx
[error] 10-10: Shouldn't redeclare 'RssFeedItem'. Consider to delete it or rename it.
'RssFeedItem' is defined here:
(lint/suspicious/noRedeclare)
[error] 34-46: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
🔇 Additional comments (16)
apps/app/src/routes/_layout/feed/$feedId.tsx (2)
7-7: Clean import addition.The
RecentContentcomponent import is appropriately added with other component imports.
153-159: Well-integrated RecentContent component.The component is properly positioned above the outlet and receives appropriate props with sensible fallbacks. The spacing with
mb-6provides good visual separation from the tabs content below.apps/app/src/components/feed/FeedCard.tsx (3)
13-17: Secure external link handling.The click handler properly opens external links in a new tab with secure attributes to prevent security vulnerabilities.
26-37: Robust image error handling.The image error handling gracefully hides the image when it fails to load, preventing broken image displays.
39-52: Excellent flexbox layout design.The flex-grow structure ensures that the footer remains at the bottom of the card regardless of content length, creating a consistent visual layout.
apps/app/src/components/rss-feed/FilterBadges.tsx (3)
22-22: Appropriate early return for empty state.The early return when no filters are active prevents unnecessary DOM rendering and maintains clean conditional logic.
33-39: Well-designed interactive remove buttons.The remove buttons are properly nested within the badges with appropriate hover states and accessibility considerations.
41-50: Smart conditional "Clear all" functionality.The "Clear all" button only appears when there are multiple filters, providing a clean UX that avoids unnecessary UI elements.
apps/app/src/components/rss-feed/RssFeedItem.tsx (2)
15-26: Good image error handling implementation.The image error handling correctly hides the image when it fails to load, maintaining a clean layout.
51-63: Secure external link implementation.The conditional link rendering with proper security attributes (
noopener noreferrer) prevents security vulnerabilities when opening external links.apps/app/src/routes/_layout/feed/$feedId/_tabs.tsx (2)
18-22: Well-defined Content tab addition.The new Content tab is properly configured with route, label, and icon, following the existing pattern.
77-92: Excellent refactoring to controlled Tabs component.The refactoring from manual link rendering to a controlled Tabs component improves maintainability and provides better UI consistency.
apps/app/src/hooks/use-rss-feed.ts (1)
39-40: Add null safety checks for querySelector results.The
linkvariable shadows the outer scope variable and querySelector calls could return null.const items = Array.from(channel.querySelectorAll("item")).map((item) => { - const link = item.querySelector("link")?.textContent || ""; - const description = item.querySelector("description")?.textContent || ""; + const itemLink = item.querySelector("link")?.textContent || ""; + const itemDescription = item.querySelector("description")?.textContent || "";Also update all references to use
itemLinkanditemDescriptionin the rest of the function.Likely an incorrect or invalid review comment.
apps/app/src/components/feed/RecentContent.tsx (1)
26-30: Keep sorting in RecentContent.tsx – the hook doesn’t sort the dataThe
useRssFeedimplementation in apps/app/src/hooks/use-rss-feed.ts contains no.sortcalls, so your component’s sort is required to order items bypubDate. You can safely ignore the suggestion to remove it.Likely an incorrect or invalid review comment.
apps/app/src/routes/_layout/feed/$feedId/_tabs/content.tsx (1)
28-56: Optimize filtering and sorting with proper memoization.The current implementation might cause unnecessary recalculations. The dependency array should include all used values.
const filteredAndSortedData = useMemo(() => { let filtered = rssData; // Apply platform filter if (platformFilter !== "all") { filtered = filtered.filter((item) => item.platform === platformFilter); } // Apply category filter if (categoryFilter !== "all") { filtered = filtered.filter((item) => item.categories?.includes(categoryFilter), ); } // Apply sorting filtered = [...filtered].sort((a, b) => { const dateA = new Date(a.pubDate).getTime(); const dateB = new Date(b.pubDate).getTime(); if (sortBy === "recent") { return dateB - dateA; } else { return dateA - dateB; } }); return filtered; - }, [rssData, platformFilter, categoryFilter, sortBy]); + }, [rssData, platformFilter, categoryFilter, sortBy]);Also consider adding date parsing error handling:
+ const dateA = new Date(a.pubDate).getTime() || 0; + const dateB = new Date(b.pubDate).getTime() || 0;Likely an incorrect or invalid review comment.
apps/app/src/routeTree.gen.ts (1)
1-10: Auto-generated file - no changes needed.This file is automatically generated by TanStack Router and should not be modified manually as stated in the header.
| onCategoryClick: (category: string) => void; | ||
| } | ||
|
|
||
| export function RssFeedItem({ item, onCategoryClick }: RssFeedItemProps) { |
There was a problem hiding this comment.
Fix naming conflict with imported type.
The component name RssFeedItem conflicts with the imported type of the same name, which can cause confusion and potential issues.
Rename the component to avoid the conflict:
-export function RssFeedItem({ item, onCategoryClick }: RssFeedItemProps) {
+export function RssFeedItemComponent({ item, onCategoryClick }: RssFeedItemProps) {Or rename the type import:
-import type { RssFeedItem } from "@/types/rss";
+import type { RssFeedItem as RssFeedItemType } from "@/types/rss";
interface RssFeedItemProps {
- item: RssFeedItem;
+ item: RssFeedItemType;
onCategoryClick: (category: string) => void;
}📝 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.
| export function RssFeedItem({ item, onCategoryClick }: RssFeedItemProps) { | |
| -export function RssFeedItem({ item, onCategoryClick }: RssFeedItemProps) { | |
| +export function RssFeedItemComponent({ item, onCategoryClick }: RssFeedItemProps) { |
🧰 Tools
🪛 Biome (1.9.4)
[error] 10-10: Shouldn't redeclare 'RssFeedItem'. Consider to delete it or rename it.
'RssFeedItem' is defined here:
(lint/suspicious/noRedeclare)
🤖 Prompt for AI Agents
In apps/app/src/components/rss-feed/RssFeedItem.tsx at line 10, the component
name RssFeedItem conflicts with an imported type of the same name. To fix this,
rename either the component or the imported type to a distinct name to avoid
confusion and potential issues. For example, rename the component to
RssFeedItemComponent or rename the imported type to RssFeedItemType, and update
all references accordingly.
| // Find the current active tab based on the current pathname | ||
| const currentTab = TABS.find((tab) => | ||
| location.pathname.includes(tab.to.replace("$feedId", feedId)), | ||
| ); | ||
| const activeValue = currentTab?.label || TABS[0]?.label; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve active tab detection robustness.
The current active tab detection using includes() might match partial paths incorrectly, potentially causing tab highlighting issues.
Consider using a more precise matching approach:
- const currentTab = TABS.find((tab) =>
- location.pathname.includes(tab.to.replace("$feedId", feedId)),
- );
+ const currentTab = TABS.find((tab) => {
+ const tabPath = tab.to.replace("$feedId", feedId);
+ return location.pathname === tabPath || location.pathname.startsWith(tabPath + "/");
+ });This ensures exact path matching or proper sub-path matching without false positives.
📝 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.
| // Find the current active tab based on the current pathname | |
| const currentTab = TABS.find((tab) => | |
| location.pathname.includes(tab.to.replace("$feedId", feedId)), | |
| ); | |
| const activeValue = currentTab?.label || TABS[0]?.label; | |
| // Find the current active tab based on the current pathname | |
| const currentTab = TABS.find((tab) => { | |
| const tabPath = tab.to.replace("$feedId", feedId); | |
| return ( | |
| location.pathname === tabPath || | |
| location.pathname.startsWith(tabPath + "/") | |
| ); | |
| }); | |
| const activeValue = currentTab?.label || TABS[0]?.label; |
🤖 Prompt for AI Agents
In apps/app/src/routes/_layout/feed/$feedId/_tabs.tsx around lines 68 to 72, the
current active tab detection uses location.pathname.includes(), which can cause
incorrect partial matches. Replace this with a more precise matching method such
as checking if the pathname starts with the tab path or using exact equality for
better accuracy. Adjust the logic to ensure the active tab is correctly
identified without false positives by comparing the full path or using a robust
path matching function.
| if (!image && description) { | ||
| const imgMatch = description.match(/<img[^>]+src="([^"]+)"/); | ||
| if (imgMatch) { | ||
| image = imgMatch[1]; | ||
| } | ||
| } |
There was a problem hiding this comment.
Sanitize HTML content to prevent XSS vulnerabilities.
Extracting image URLs from HTML description using regex without sanitization could lead to XSS attacks if malicious content is injected into RSS feeds.
Use a proper HTML parser and sanitization library instead:
- if (!image && description) {
- const imgMatch = description.match(/<img[^>]+src="([^"]+)"/);
- if (imgMatch) {
- image = imgMatch[1];
- }
- }
+ if (!image && description) {
+ // Use DOMParser to safely extract image URL
+ const descDoc = parser.parseFromString(description, "text/html");
+ const imgElement = descDoc.querySelector("img");
+ if (imgElement) {
+ image = imgElement.getAttribute("src") || "";
+ }
+ }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/app/src/hooks/use-rss-feed.ts around lines 56 to 61, the code extracts
image URLs from the HTML description using regex without sanitizing the content,
which risks XSS vulnerabilities. Replace the regex extraction with a proper HTML
parser to safely parse the description, then sanitize the extracted image URL
using a trusted sanitization library before assigning it to the image variable.
| // Determine platform from link | ||
| let platform = "other"; | ||
| if (link.includes("twitter.com") || link.includes("x.com")) { | ||
| platform = "twitter"; | ||
| } else if (link.includes("youtube.com") || link.includes("youtu.be")) { | ||
| platform = "youtube"; | ||
| } else if (link.includes("github.com")) { | ||
| platform = "github"; | ||
| } else if (link.includes("reddit.com")) { | ||
| platform = "reddit"; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Extract platform detection to a configurable mapping.
The platform detection logic is hardcoded and not maintainable. This should be extracted to a configuration object.
+const PLATFORM_PATTERNS = {
+ twitter: ["twitter.com", "x.com"],
+ youtube: ["youtube.com", "youtu.be"],
+ github: ["github.com"],
+ reddit: ["reddit.com"],
+} as const;
+
+function detectPlatform(url: string): string {
+ for (const [platform, patterns] of Object.entries(PLATFORM_PATTERNS)) {
+ if (patterns.some(pattern => url.includes(pattern))) {
+ return platform;
+ }
+ }
+ return "other";
+}
// Determine platform from link
- let platform = "other";
- if (link.includes("twitter.com") || link.includes("x.com")) {
- platform = "twitter";
- } else if (link.includes("youtube.com") || link.includes("youtu.be")) {
- platform = "youtube";
- } else if (link.includes("github.com")) {
- platform = "github";
- } else if (link.includes("reddit.com")) {
- platform = "reddit";
- }
+ const platform = detectPlatform(itemLink);Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/app/src/hooks/use-rss-feed.ts around lines 63 to 73, the platform
detection logic is hardcoded with multiple if-else statements. Refactor this by
creating a configuration object that maps platform names to their identifying
URL substrings. Replace the if-else chain with a loop or method that checks the
link against this mapping to determine the platform, making it easier to
maintain and extend.
| async function fetchRssFeed(serviceUrl: string): Promise<RssFeedData> { | ||
| const response = await fetch(serviceUrl); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Add URL validation and implement server-side proxy for RSS feeds.
Fetching RSS feeds directly from the browser will fail due to CORS restrictions for most external RSS feeds. Additionally, allowing arbitrary URLs without validation poses a security risk.
Consider implementing a server-side proxy endpoint that:
- Validates and sanitizes the RSS feed URL
- Fetches the RSS content server-side to avoid CORS issues
- Caches the results to improve performance
Example approach:
-async function fetchRssFeed(serviceUrl: string): Promise<RssFeedData> {
- const response = await fetch(serviceUrl);
+async function fetchRssFeed(serviceUrl: string): Promise<RssFeedData> {
+ // Use a server-side proxy to avoid CORS and add security
+ const response = await fetch('/api/rss-proxy', {
+ method: 'POST',
+ headers: { 'Content-Type': 'application/json' },
+ body: JSON.stringify({ url: serviceUrl })
+ });🏁 Script executed:
#!/bin/bash
set -e
# Locate the use-rss-feed hook file and inspect its contents
file=$(fd use-rss-feed.ts -t f)
echo "Found file: $file"
echo "--- serviceUrl occurrences ---"
grep -n "serviceUrl" "$file" || true
echo "--- fetch occurrences ---"
grep -n "fetch" "$file" || true
echo "--- File content (lines 1-200) ---"
sed -n '1,200p' "$file"Length of output: 4814
Add server-side proxy with URL validation for RSS fetching
Fetching arbitrary RSS URLs directly in the browser will often fail due to CORS restrictions and exposes the app to unvalidated input. Instead, introduce a server-side proxy endpoint that:
- Validates and sanitizes the incoming
serviceUrl(e.g. enforce a domain whitelist or strict URL regex) - Fetches and parses the RSS feed server-side to avoid CORS issues
- Implements caching (in-memory or via an HTTP cache) to improve performance and reduce upstream load
In apps/app/src/hooks/use-rss-feed.ts, replace this block:
- async function fetchRssFeed(serviceUrl: string): Promise<RssFeedData> {
- const response = await fetch(serviceUrl);
+ async function fetchRssFeed(serviceUrl: string): Promise<RssFeedData> {
+ const response = await fetch("/api/rss-proxy", {
+ method: "POST",
+ headers: { "Content-Type": "application/json" },
+ body: JSON.stringify({ url: serviceUrl }),
+ });• Ensure /api/rss-proxy performs URL validation, server-side fetch, XML parsing, error handling, and appropriate caching.
• Update your front-end hook to handle any new error shapes returned by the proxy.
• Add tests for invalid URLs and proxy error responses.
📝 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.
| async function fetchRssFeed(serviceUrl: string): Promise<RssFeedData> { | |
| const response = await fetch(serviceUrl); | |
| async function fetchRssFeed(serviceUrl: string): Promise<RssFeedData> { | |
| const response = await fetch("/api/rss-proxy", { | |
| method: "POST", | |
| headers: { "Content-Type": "application/json" }, | |
| body: JSON.stringify({ url: serviceUrl }), | |
| }); | |
| // …rest of your existing parsing and error-handling logic | |
| } |
🤖 Prompt for AI Agents
In apps/app/src/hooks/use-rss-feed.ts around lines 12 to 13, replace the direct
client-side fetch of the RSS feed URL with a call to a new server-side proxy
endpoint at /api/rss-proxy. Implement the proxy to validate and sanitize the
incoming serviceUrl (using a domain whitelist or strict regex), fetch and parse
the RSS feed server-side to avoid CORS issues, handle errors properly, and add
caching to improve performance. Update the front-end hook to handle the proxy's
error responses accordingly. Finally, add tests covering invalid URLs and proxy
error scenarios to ensure robustness.
| // Don't render if no RSS feed or no data | ||
| if (!hasRssFeed || isLoading || isError || recentItems.length === 0) { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve user experience by showing loading and error states.
Currently, the component returns null for loading/error states, providing no feedback to users.
- // Don't render if no RSS feed or no data
- if (!hasRssFeed || isLoading || isError || recentItems.length === 0) {
- return null;
- }
+ if (!hasRssFeed) return null;
+
+ if (isLoading) {
+ return (
+ <div className="space-y-4">
+ <h3 className="leading-8 text-xl sm:text-[24px] font-semibold">
+ Recent Content
+ </h3>
+ <div className="grid grid-cols-1 sm:grid-cols-2 lg:grid-cols-3 gap-4">
+ {[...Array(3)].map((_, i) => (
+ <div key={i} className="h-32 bg-gray-200 animate-pulse rounded-lg" />
+ ))}
+ </div>
+ </div>
+ );
+ }
+
+ if (isError) {
+ return (
+ <div className="text-center text-gray-500 py-4">
+ Failed to load recent content
+ </div>
+ );
+ }
+
+ if (recentItems.length === 0) return null;📝 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.
| // Don't render if no RSS feed or no data | |
| if (!hasRssFeed || isLoading || isError || recentItems.length === 0) { | |
| return null; | |
| } | |
| if (!hasRssFeed) return null; | |
| if (isLoading) { | |
| return ( | |
| <div className="space-y-4"> | |
| <h3 className="leading-8 text-xl sm:text-[24px] font-semibold"> | |
| Recent Content | |
| </h3> | |
| <div className="grid grid-cols-1 sm:grid-cols-2 lg:grid-cols-3 gap-4"> | |
| {[...Array(3)].map((_, i) => ( | |
| <div key={i} className="h-32 bg-gray-200 animate-pulse rounded-lg" /> | |
| ))} | |
| </div> | |
| </div> | |
| ); | |
| } | |
| if (isError) { | |
| return ( | |
| <div className="text-center text-gray-500 py-4"> | |
| Failed to load recent content | |
| </div> | |
| ); | |
| } | |
| if (recentItems.length === 0) return null; |
🤖 Prompt for AI Agents
In apps/app/src/components/feed/RecentContent.tsx around lines 51 to 54, the
component returns null during loading or error states, which gives no user
feedback. Instead of returning null, update the code to render appropriate UI
elements indicating loading or error states, such as a spinner or error message,
to improve user experience by providing clear feedback.
| // Get recent items (latest 9 items, 3 pages of 3 items each) | ||
| const recentItems = useMemo(() => { | ||
| if (!rssData || rssData.length === 0) return []; | ||
|
|
||
| // Sort by date (most recent first) and take first 9 items | ||
| const sortedItems = [...rssData] | ||
| .sort( | ||
| (a, b) => new Date(b.pubDate).getTime() - new Date(a.pubDate).getTime(), | ||
| ) | ||
| .slice(0, 9); | ||
|
|
||
| // Group into pages of 3 | ||
| const pages = []; | ||
| for (let i = 0; i < sortedItems.length; i += 3) { | ||
| pages.push(sortedItems.slice(i, i + 3)); | ||
| } | ||
|
|
||
| return pages; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Extract magic numbers to configuration constants.
The pagination logic uses hardcoded values that should be configurable.
+const ITEMS_PER_PAGE = 3;
+const MAX_RECENT_ITEMS = 9;
+
export function RecentContent({
feedId,
feedName,
feedImage,
}: RecentContentProps) {
const [currentPage, setCurrentPage] = useState(0);
const { hasRssFeed, rssData, isLoading, isError } = useRssFeed(feedId);
// Get recent items (latest 9 items, 3 pages of 3 items each)
const recentItems = useMemo(() => {
if (!rssData || rssData.length === 0) return [];
// Sort by date (most recent first) and take first 9 items
const sortedItems = [...rssData]
.sort(
(a, b) => new Date(b.pubDate).getTime() - new Date(a.pubDate).getTime(),
)
- .slice(0, 9);
+ .slice(0, MAX_RECENT_ITEMS);
// Group into pages of 3
const pages = [];
- for (let i = 0; i < sortedItems.length; i += 3) {
- pages.push(sortedItems.slice(i, i + 3));
+ for (let i = 0; i < sortedItems.length; i += ITEMS_PER_PAGE) {
+ pages.push(sortedItems.slice(i, i + ITEMS_PER_PAGE));
}📝 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.
| // Get recent items (latest 9 items, 3 pages of 3 items each) | |
| const recentItems = useMemo(() => { | |
| if (!rssData || rssData.length === 0) return []; | |
| // Sort by date (most recent first) and take first 9 items | |
| const sortedItems = [...rssData] | |
| .sort( | |
| (a, b) => new Date(b.pubDate).getTime() - new Date(a.pubDate).getTime(), | |
| ) | |
| .slice(0, 9); | |
| // Group into pages of 3 | |
| const pages = []; | |
| for (let i = 0; i < sortedItems.length; i += 3) { | |
| pages.push(sortedItems.slice(i, i + 3)); | |
| } | |
| return pages; | |
| // Extracted configuration constants | |
| const ITEMS_PER_PAGE = 3; | |
| const MAX_RECENT_ITEMS = 9; | |
| export function RecentContent({ | |
| feedId, | |
| feedName, | |
| feedImage, | |
| }: RecentContentProps) { | |
| const [currentPage, setCurrentPage] = useState(0); | |
| const { hasRssFeed, rssData, isLoading, isError } = useRssFeed(feedId); | |
| // Get recent items (latest MAX_RECENT_ITEMS items, pages of ITEMS_PER_PAGE each) | |
| const recentItems = useMemo(() => { | |
| if (!rssData || rssData.length === 0) return []; | |
| // Sort by date (most recent first) and take first MAX_RECENT_ITEMS items | |
| const sortedItems = [...rssData] | |
| .sort( | |
| (a, b) => | |
| new Date(b.pubDate).getTime() - new Date(a.pubDate).getTime(), | |
| ) | |
| .slice(0, MAX_RECENT_ITEMS); | |
| // Group into pages of ITEMS_PER_PAGE | |
| const pages = []; | |
| for (let i = 0; i < sortedItems.length; i += ITEMS_PER_PAGE) { | |
| pages.push(sortedItems.slice(i, i + ITEMS_PER_PAGE)); | |
| } | |
| return pages; | |
| }, [rssData]); | |
| // …rest of component | |
| } |
🤖 Prompt for AI Agents
In apps/app/src/components/feed/RecentContent.tsx around lines 21 to 38, the
pagination logic uses hardcoded magic numbers for the number of items per page
and total items to display. Extract these numbers into clearly named
configuration constants at the top of the file or in a separate config file.
Replace the hardcoded values (9 for total items, 3 for items per page) with
these constants to improve maintainability and configurability.
| <SelectContent> | ||
| <SelectItem value="all">All Platforms</SelectItem> | ||
| <SelectItem value="twitter">Twitter</SelectItem> | ||
| <SelectItem value="youtube">YouTube</SelectItem> | ||
| <SelectItem value="github">GitHub</SelectItem> | ||
| <SelectItem value="reddit">Reddit</SelectItem> | ||
| <SelectItem value="other">Other</SelectItem> |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Ensure platform options are consistent with platform detection logic.
The platform options are hardcoded here but should match the platform detection logic in use-rss-feed.ts to maintain consistency.
Create a shared constants file for platform configuration:
// src/constants/platforms.ts
export const PLATFORMS = [
{ value: "all", label: "All Platforms" },
{ value: "twitter", label: "Twitter" },
{ value: "youtube", label: "YouTube" },
{ value: "github", label: "GitHub" },
{ value: "reddit", label: "Reddit" },
{ value: "other", label: "Other" },
] as const;
export type Platform = typeof PLATFORMS[number]["value"];Then update this component:
+import { PLATFORMS } from "@/constants/platforms";
<SelectContent>
- <SelectItem value="all">All Platforms</SelectItem>
- <SelectItem value="twitter">Twitter</SelectItem>
- <SelectItem value="youtube">YouTube</SelectItem>
- <SelectItem value="github">GitHub</SelectItem>
- <SelectItem value="reddit">Reddit</SelectItem>
- <SelectItem value="other">Other</SelectItem>
+ {PLATFORMS.map((platform) => (
+ <SelectItem key={platform.value} value={platform.value}>
+ {platform.label}
+ </SelectItem>
+ ))}
</SelectContent>🏁 Script executed:
#!/bin/bash
# Find the use-rss-feed.ts file
echo "Locating use-rss-feed.ts..."
fd use-rss-feed.ts
# Once located, search for platform detection logic within the file
echo "Searching for platform detection logic in use-rss-feed.ts..."
# Adjust path if fd finds the file under a nested directory
FILE_PATH=$(fd use-rss-feed.ts | head -n1)
if [ -n "$FILE_PATH" ]; then
rg -n "platform" "$FILE_PATH" -A3 -B3
else
echo "use-rss-feed.ts not found."
fiLength of output: 1152
Unify platform values via shared constants
To avoid duplication and ensure consistency between FilterControls.tsx and use-rss-feed.ts, extract your platform list into a shared module:
• Create src/constants/platforms.ts
export const PLATFORMS = [
{ value: "all", label: "All Platforms" },
{ value: "twitter",label: "Twitter" },
{ value: "youtube",label: "YouTube" },
{ value: "github", label: "GitHub" },
{ value: "reddit", label: "Reddit" },
{ value: "other", label: "Other" },
] as const;
export type Platform = typeof PLATFORMS[number]["value"];• Update FilterControls.tsx to consume the shared list
-import { SelectContent, SelectItem } from "@/components/ui/select";
+import { SelectContent, SelectItem } from "@/components/ui/select";
+import { PLATFORMS } from "@/constants/platforms";
<SelectContent>
- <SelectItem value="all">All Platforms</SelectItem>
- <SelectItem value="twitter">Twitter</SelectItem>
- <SelectItem value="youtube">YouTube</SelectItem>
- <SelectItem value="github">GitHub</SelectItem>
- <SelectItem value="reddit">Reddit</SelectItem>
- <SelectItem value="other">Other</SelectItem>
+ {PLATFORMS.map(({ value, label }) => (
+ <SelectItem key={value} value={value}>
+ {label}
+ </SelectItem>
+ ))}
</SelectContent>• (Optional) In use-rss-feed.ts, import Platform to type your platform variable and consider driving detection from the same constants for full alignment.
This refactor centralizes your platform definitions and removes brittle, duplicated strings.
🤖 Prompt for AI Agents
In apps/app/src/components/rss-feed/FilterControls.tsx around lines 61 to 67,
the platform values are hardcoded and duplicated elsewhere. To fix this, create
a new shared module at src/constants/platforms.ts exporting a constant array of
platform objects with value and label fields, plus a Platform type. Then update
FilterControls.tsx to import and use this shared PLATFORMS array to render the
SelectItem components dynamically. Optionally, update use-rss-feed.ts to import
the Platform type and use the shared constants for platform detection to ensure
consistency across the codebase.
| import { useState, useMemo, useRef } from "react"; | ||
| import * as React from "react"; |
There was a problem hiding this comment.
Remove duplicate React import.
There are two React imports which is unnecessary.
import { useState, useMemo, useRef } from "react";
-import * as React from "react";📝 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.
| import { useState, useMemo, useRef } from "react"; | |
| import * as React from "react"; | |
| import { useState, useMemo, useRef } from "react"; |
🤖 Prompt for AI Agents
In apps/app/src/routes/_layout/feed/$feedId/_tabs/content.tsx at lines 3 to 4,
there are two imports for React: one importing useState, useMemo, useRef from
"react" and another importing all as React. Remove the duplicate import by
keeping only one import statement that includes all necessary React hooks and
references, preferably the first one, and delete the second import line.
| // Setup window virtualization | ||
| React.useLayoutEffect(() => { | ||
| parentOffsetRef.current = parentRef.current?.offsetTop ?? 0; | ||
| }, []); | ||
|
|
There was a problem hiding this comment.
Add cleanup for window virtualizer to prevent memory leaks.
The virtualizer should be properly cleaned up when the component unmounts.
// Setup window virtualization
React.useLayoutEffect(() => {
parentOffsetRef.current = parentRef.current?.offsetTop ?? 0;
}, []);
+
+ // Cleanup virtualizer on unmount
+ React.useEffect(() => {
+ return () => {
+ virtualizer._willUpdate();
+ };
+ }, [virtualizer]);Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/app/src/routes/_layout/feed/$feedId/_tabs/content.tsx around lines 122
to 126, the React useLayoutEffect sets up window virtualization but lacks a
cleanup function. To fix this, add a cleanup function inside the useLayoutEffect
that properly disposes or cleans up the virtualizer resources when the component
unmounts to prevent memory leaks.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/core-services/src/services/processing.service.ts (2)
125-167: Well-structured step execution with comprehensive error handling.The refactored method properly manages step lifecycle and provides excellent error handling. Consider whether both
pluginNameandstepNameneed to be set to the same value (planStep.plugin) for consistency or if this could be simplified.
176-212: Consistent implementation with executeTransform.The method follows the same well-structured pattern for step execution. The same consideration about
pluginNameandstepNameredundancy applies here as well.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/core-services/src/services/processing.service.ts(7 hunks)
🔇 Additional comments (8)
packages/core-services/src/services/processing.service.ts (8)
2-11: LGTM! Well-structured imports for the new processing plan architecture.The new imports properly support the ProcessingPlan abstraction and additional repository dependencies needed for enhanced reprocessing capabilities.
35-35: Good addition of optional plan parameter.The optional
planparameter provides flexibility for custom processing workflows while maintaining backward compatibility.
45-46: Appropriate dependency injection for enhanced functionality.The new repository dependencies are properly injected to support the reprocessing features.
67-105: Excellent architectural improvement with explicit ProcessingPlan.The refactoring from implicit processing flow to explicit ProcessingPlan-based approach significantly improves:
- Processing transparency and auditability
- Retry capability with partial plan execution
- Job state persistence and recovery
The sequential plan execution logic is well-structured and maintains proper error handling.
244-296: Robust retry implementation leveraging ProcessingPlan architecture.The rewritten method provides comprehensive validation and proper partial plan execution. The logic correctly:
- Validates step and job state
- Creates partial plan from retry point
- Reuses existing process method
The complexity is justified by the robustness it provides.
298-331: Clean implementation of plan building logic.The method correctly constructs a ProcessingPlan from configuration, maintaining proper order:
- Global transforms first
- Distributor-specific transforms
- Distribution steps
The logic is clear and follows the expected processing flow.
333-362: Useful reprocessing feature with comprehensive validation.The method provides valuable functionality for reprocessing jobs with latest configuration. The implementation properly:
- Validates all required entities exist
- Uses current feed configuration
- Maintains proper job relationships
Excellent error handling and clear logic flow.
364-407: Valuable debugging feature with proper input validation.The method enables step input tweaking for debugging and testing. The implementation correctly:
- Validates step and job state
- Handles JSON parsing errors gracefully
- Creates appropriate partial plan
- Maintains job relationships
The feature adds significant value for development and troubleshooting workflows.
2 participants
Summary by CodeRabbit
New Features
Enhancements
Bug Fixes
Refactor
Chores