SUMMA-19 and SUMMA-21: Rebased #121
Conversation
WalkthroughThis update introduces new service abstractions for search and subscription API interactions in the frontend, refactors related components to use these services, and standardizes error handling. The backend search controller and service are refactored for clarity, consistency, and improved error handling. Minor import path and API version updates are included, and some component and method names are renamed for clarity. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Navbar
participant SearchService
participant BackendAPI
User->>Navbar: Enter search query
Navbar->>SearchService: searchUsers(query, startingPoint)
SearchService->>BackendAPI: GET /search/users?searchText=...&userStartingPoint=...
BackendAPI-->>SearchService: { users, nextStartingPoint }
SearchService-->>Navbar: { users, nextStartingPoint } or Error
Navbar->>SearchService: searchContents(query)
SearchService->>BackendAPI: GET /search/contents?searchText=...
BackendAPI-->>SearchService: { contents, nextStartingPoint }
SearchService-->>Navbar: { contents, nextStartingPoint } or Error
Navbar-->>User: Display search results
sequenceDiagram
participant User
participant ProPage
participant SubscriptionService
participant BackendAPI
User->>ProPage: Click "Subscribe" or "Check Status"
ProPage->>SubscriptionService: getSubscriptionStatus()
SubscriptionService->>BackendAPI: GET /subscription/status
BackendAPI-->>SubscriptionService: { status, ... }
SubscriptionService-->>ProPage: status or Error
alt User clicks "Subscribe"
ProPage->>SubscriptionService: createSubscriptionSession()
SubscriptionService->>BackendAPI: POST /subscription/session
BackendAPI-->>SubscriptionService: { url }
SubscriptionService-->>ProPage: url or Error
ProPage-->>User: Redirect to Stripe Checkout
end
alt User clicks "Cancel Subscription"
ProPage->>SubscriptionService: cancelSubscription()
SubscriptionService->>BackendAPI: POST /subscription/cancel
BackendAPI-->>SubscriptionService: { message, willEndOn }
SubscriptionService-->>ProPage: message or Error
ProPage-->>User: Show cancellation result
end
Poem
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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: 4
🔭 Outside diff range comments (1)
frontend/src/pages/pro/ManageSubscription.tsx (1)
27-45: 🛠️ Refactor suggestion
fetchSubscriptionStatusmissingtry / finallyleavesloadingspinner forever on unhandled throw.
SubscriptionService.getSubscriptionStatusshould swallow Axios errors and returnError, but a regression could re-throw.
Guard against that and always clearloading.- const subscriptionStatus = await SubscriptionService.getSubscriptionStatus( - forceRefresh - ); - … - setLoading(false); + try { + const subscriptionStatus = + await SubscriptionService.getSubscriptionStatus(forceRefresh); + if (subscriptionStatus instanceof Error) { + setError(subscriptionStatus.message); + } else { + if (subscriptionStatus.status === "canceled") { + setCancelSuccess(false); + } + setSubscription(subscriptionStatus); + } + } catch (err) { + setError("Unexpected error while fetching subscription status."); + } finally { + setLoading(false); + }
🧹 Nitpick comments (11)
frontend/src/components/search/SearchListContent.tsx (2)
69-73: Avoid blocking alerts in async code paths.Using
alert("Already Fetching!")will interrupt the user’s flow, feels jarring on web, and is not accessible.
Prefer a non-blocking notification (toast / inline message) or silent return.- alert("Already Fetching!"); + // TODO: surface this information via a toast or inline message
74-113: Sharedfetchingflag can block parallel user & content requests.
fetchUserDataguards itself withfetching, butfetchContentDatasets the same flag totruewithout the guard.
IffetchUserDatais running,fetchContentDatastarts, but subsequent user-initiated “fetch more” clicks will bail out becausefetchingis stilltrue.
Consider a per-endpoint flag (fetchingUsers,fetchingContent) or just removing the guard since back-to-back clicks are already throttled by the disabled button.frontend/src/components/Navbar.tsx (2)
121-125: Run searches concurrently for better UX.The user & content searches are independent; firing them sequentially doubles the perceived latency.
A small refactor withPromise.allkeeps the same error-handling style but halves waiting time.- const userSearchResults = await SearchService.searchUsers(trimmedQuery); - const contentSearchResults = await SearchService.searchContents(trimmedQuery); + const [userSearchResults, contentSearchResults] = await Promise.all([ + SearchService.searchUsers(trimmedQuery), + SearchService.searchContents(trimmedQuery), + ]);
127-141: Reduce duplicated error-handling boilerplate.Both result blocks follow the pattern “if instanceof Error → log, else → setState”.
Wrapping this in a helper would make the function shorter and easier to reason about.No code diff provided to avoid over-refactor noise.
frontend/src/pages/pro/ManageSubscription.tsx (2)
48-52: Early-exit guard duplicates navigation logic.
handleCancelSubscriptioncopies the auth check present in the parent route guard (useEffect).
If the user somehow gets here unauthenticated, the UI already redirects.
Consider removing the duplication or extracting a shared helper so the behaviour can’t drift.
64-66:currentSubscriptionmay hold stale data after async await.Between storing
currentSubscriptionand using it inside thesetSubscriptionstate updater, another state update could have fired, making the snapshot obsolete.
Capture it inside the functional update instead:- const currentSubscription = subscription ? { ...subscription } : null; - … - setSubscription((prev) => { - const base = prev ?? currentSubscription; + setSubscription((prev) => { + const base = prev ?? (subscription ? { ...subscription } : null);frontend/src/services/SearchService.ts (3)
6-50: Refactor class to use standalone functionsThe class implementation is clear and functions well, but the static analysis tool correctly points out that this class only contains static methods. This pattern can be simplified.
Consider refactoring to use standalone functions instead of a class with static methods:
-export class SearchService { +/** + * searchUsers(query: string) -> Promise<User[] | Error> + * + * @description + * Searches for users based on the provided query string. + * + * @param query - The search query string. + * @returns A promise that resolves to an array of User objects or an Error. + */ +export async function searchUsers( + searchText: string, + userStartingPoint: string | null = null +): Promise<{ users: User[]; nextStartingPoint: string } | Error> { + try { + const response = await axios.get(`${apiURL}/search/users`, { + params: { searchText, userStartingPoint }, + }); + + return response.data; + } catch (error) { + if (error instanceof Error) { + return new Error("Failed to fetch user data: " + error.message); + } + return new Error("Failed to fetch user data: Unknown error"); + } +} +export async function searchContents( + searchText: string +): Promise<{ contents: Content[]; nextStartingPoint: string } | Error> { + try { + const response = await axios.get(`${apiURL}/search/contents`, { + params: { searchText }, + }); + + return response.data; + } catch (error) { + if (error instanceof Error) { + return new Error("Failed to fetch content data: " + error.message); + } + return new Error("Failed to fetch content data: Unknown error"); + } +} - /** - * searchUsers(query: string) -> Promise<User[] | Error> - * - * @description - * Searches for users based on the provided query string. - * - * @param query - The search query string. - * @returns A promise that resolves to an array of User objects or an Error. - */ - static async searchUsers( - searchText: string, - userStartingPoint: string | null = null - ): Promise<{ users: User[]; nextStartingPoint: string } | Error> { - try { - const response = await axios.get(`${apiURL}/search/users`, { - params: { searchText, userStartingPoint }, - }); - - return response.data; - } catch (error) { - if (error instanceof Error) { - return new Error("Failed to fetch user data: " + error.message); - } - return new Error("Failed to fetch user data: Unknown error"); - } - } - - static async searchContents( - searchText: string - ): Promise<{ contents: Content[]; nextStartingPoint: string } | Error> { - try { - const response = await axios.get(`${apiURL}/search/contents`, { - params: { searchText }, - }); - - return response.data; - } catch (error) { - if (error instanceof Error) { - return new Error("Failed to fetch content data: " + error.message); - } - return new Error("Failed to fetch content data: Unknown error"); - } - } -}🧰 Tools
🪛 Biome (1.9.4)
[error] 6-50: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
16-32: Consider using the axios.isAxiosError type guardFor more precise error handling, use axios's built-in type guard for Axios errors.
static async searchUsers( searchText: string, userStartingPoint: string | null = null ): Promise<{ users: User[]; nextStartingPoint: string } | Error> { try { const response = await axios.get(`${apiURL}/search/users`, { params: { searchText, userStartingPoint }, }); return response.data; } catch (error: unknown) { - if (error instanceof Error) { + if (axios.isAxiosError(error)) { + return new Error( + `Failed to fetch user data: ${ + error.response?.data?.message || error.message + }` + ); - return new Error("Failed to fetch user data: " + error.message); } return new Error("Failed to fetch user data: Unknown error"); } }
34-49: Add missing JSDoc commentThe
searchContentsmethod is missing JSDoc documentation unlike thesearchUsersmethod.Add JSDoc documentation for consistency:
+ /** + * searchContents(query: string) -> Promise<Content[] | Error> + * + * @description + * Searches for content based on the provided query string. + * + * @param searchText - The search query string. + * @returns A promise that resolves to an array of Content objects or an Error. + */ static async searchContents( searchText: string ): Promise<{ contents: Content[]; nextStartingPoint: string } | Error> { try { const response = await axios.get(`${apiURL}/search/contents`, { params: { searchText }, }); return response.data; } catch (error) { if (error instanceof Error) { return new Error("Failed to fetch content data: " + error.message); } return new Error("Failed to fetch content data: Unknown error"); } }frontend/src/services/SubscriptionService.ts (2)
5-104: Refactor class to use standalone functionsThe static analysis tool correctly points out that this class only contains static methods. This pattern can be simplified.
Consider refactoring to use standalone functions instead of a class with static methods:
-export class SubscriptionService { +/** + * Creates a subscription session for the user. + * + * @returns A promise that resolves to the response containing the session URL. + * @returns {Promise<{url: string}>} The checkout session URL on success + * @returns {Error} Error object with message if the request fails + */ +export async function createSubscriptionSession(): Promise<{ url: string } | Error> { + try { + const response = await axios.post( + `${apiURL}/subscription/create-checkout-session`, + {}, + { + withCredentials: true, + } + ); + + return response.data; + } catch (error: unknown) { + if (axios.isAxiosError(error)) { + return new Error( + `Failed to create subscription session: ${ + error.response?.data?.message || error.message + }` + ); + } + return new Error("Failed to create subscription session: Unknown error"); + } +} +/** + * Cancels the user's subscription. + * + * @returns A promise that resolves to a response containing the cancellation message and end date. + * @returns {Promise<{message: string, willEndOn: Date}>} The cancellation message and end date on success + * @returns {Error} Error object with message if the request fails + */ +export async function cancelSubscription(): Promise< + | { + message: string; + willEndOn: Date; + } + | Error +> { + try { + const response = await axios.post( + `${apiURL}/subscription/cancel`, + {}, + { + withCredentials: true, + } + ); + + return response.data; + } catch (error: unknown) { + if (axios.isAxiosError(error)) { + return new Error( + `Failed to cancel subscription: ${ + error.response?.data?.message || error.message + }` + ); + } + return new Error("Failed to cancel subscription: Unknown error"); + } +} +/** + * Retrieves the subscription status for the user. + * + * @param forceRefresh - Whether to force a refresh of the subscription status. + * @returns A promise that resolves to the subscription status or an error. + * @returns {Promise<SubscriptionStatus | Error>} The subscription status on success + * @returns {Error} Error object with message if the request fails + */ +export async function getSubscriptionStatus( + forceRefresh: boolean = false +): Promise<SubscriptionStatus | Error> { + try { + const url = forceRefresh + ? `${apiURL}/subscription/status?forceRefresh=true&t=${new Date().getTime()}` + : `${apiURL}/subscription/status`; + + const response = await axios.get(url, { + withCredentials: true, + }); + + return response.data; + } catch (error: unknown) { + if (axios.isAxiosError(error)) { + return new Error( + `Failed to get subscription status: ${ + error.response?.data?.message || error.message + }` + ); + } + return new Error("Failed to get subscription status: Unknown error"); + } +} - /** - * Creates a subscription session for the user. - * - * @returns A promise that resolves to the response containing the session URL. - * @returns {Promise<{url: string}>} The checkout session URL on success - * @returns {Error} Error object with message if the request fails - */ - static async createSubscriptionSession(): Promise<{ url: string } | Error> { - try { - const response = await axios.post( - `${apiURL}/subscription/create-checkout-session`, - {}, - { - withCredentials: true, - } - ); - - return response.data; - } catch (error: unknown) { - if (axios.isAxiosError(error)) { - return new Error( - `Failed to create subscription session: ${ - error.response?.data?.message || error.message - }` - ); - } - return new Error("Failed to create subscription session: Unknown error"); - } - } - - /** - * Cancels the user's subscription. - * - * @returns A promise that resolves to a response containing the cancellation message and end date. - * @returns {Promise<{message: string, willEndOn: Date}>} The cancellation message and end date on success - * @returns {Error} Error object with message if the request fails - */ - static async cancelSubscription(): Promise< - | { - message: string; - willEndOn: Date; - } - | Error - > { - try { - const response = await axios.post( - `${apiURL}/subscription/cancel`, - {}, - { - withCredentials: true, - } - ); - - return response.data; - } catch (error: unknown) { - if (axios.isAxiosError(error)) { - return new Error( - `Failed to cancel subscription: ${ - error.response?.data?.message || error.message - }` - ); - } - return new Error("Failed to cancel subscription: Unknown error"); - } - } - - /** - * Retrieves the subscription status for the user. - * - * @param forceRefresh - Whether to force a refresh of the subscription status. - * @returns A promise that resolves to the subscription status or an error. - * @returns {Promise<SubscriptionStatus | Error>} The subscription status on success - * @returns {Error} Error object with message if the request fails - */ - static async getSubscriptionStatus( - forceRefresh: boolean = false - ): Promise<SubscriptionStatus | Error> { - try { - const url = forceRefresh - ? `${apiURL}/subscription/status?forceRefresh=true&t=${new Date().getTime()}` - : `${apiURL}/subscription/status`; - - const response = await axios.get(url, { - withCredentials: true, - }); - - return response.data; - } catch (error: unknown) { - if (axios.isAxiosError(error)) { - return new Error( - `Failed to get subscription status: ${ - error.response?.data?.message || error.message - }` - ); - } - return new Error("Failed to get subscription status: Unknown error"); - } - } -}🧰 Tools
🪛 Biome (1.9.4)
[error] 5-104: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
84-87: Improve URL construction with URLSearchParamsThe manual string concatenation for URL parameters could be more robust using URLSearchParams.
try { - const url = forceRefresh - ? `${apiURL}/subscription/status?forceRefresh=true&t=${new Date().getTime()}` - : `${apiURL}/subscription/status`; + const url = new URL(`${apiURL}/subscription/status`); + if (forceRefresh) { + url.searchParams.append('forceRefresh', 'true'); + url.searchParams.append('t', new Date().getTime().toString()); + } const response = await axios.get(url, { withCredentials: true, });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
backend/src/modules/search/controllers/search.controller.ts(1 hunks)backend/src/modules/search/routes/search.routes.ts(1 hunks)backend/src/modules/search/services/search.service.ts(4 hunks)frontend/src/App.tsx(2 hunks)frontend/src/components/Navbar.tsx(3 hunks)frontend/src/components/feed/ContentList.tsx(1 hunks)frontend/src/components/search/SearchListContent.tsx(4 hunks)frontend/src/hooks/AuthProvider.tsx(2 hunks)frontend/src/pages/content/ContentView.tsx(1 hunks)frontend/src/pages/pro/ManageSubscription.tsx(4 hunks)frontend/src/pages/pro/ProDetails.tsx(2 hunks)frontend/src/pages/pro/SubscribePro.tsx(3 hunks)frontend/src/services/SearchService.ts(1 hunks)frontend/src/services/SubscriptionService.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (8)
backend/src/modules/search/routes/search.routes.ts (1)
backend/src/modules/search/controllers/search.controller.ts (1)
SearchController(6-63)
frontend/src/App.tsx (1)
frontend/src/hooks/AuthProvider.tsx (1)
AuthProvider(10-95)
frontend/src/pages/pro/ProDetails.tsx (1)
frontend/src/services/SubscriptionService.ts (1)
SubscriptionService(5-104)
frontend/src/pages/pro/ManageSubscription.tsx (1)
frontend/src/services/SubscriptionService.ts (1)
SubscriptionService(5-104)
backend/src/modules/search/services/search.service.ts (3)
backend/src/shared/utils/logger.ts (1)
logger(14-36)backend/src/shared/config/firebase.config.ts (1)
db(31-31)backend/src/modules/user/models/user.model.ts (1)
User(1-33)
frontend/src/components/Navbar.tsx (1)
frontend/src/services/SubscriptionService.ts (1)
SubscriptionService(5-104)
frontend/src/services/SubscriptionService.ts (2)
frontend/src/scripts/api.ts (1)
apiURL(8-8)frontend/src/models/SubscriptionStatus.ts (1)
SubscriptionStatus(1-7)
frontend/src/services/SearchService.ts (1)
frontend/src/scripts/api.ts (1)
apiURL(8-8)
🪛 Biome (1.9.4)
backend/src/modules/search/services/search.service.ts
[error] 99-99: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
frontend/src/services/SubscriptionService.ts
[error] 5-104: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
frontend/src/services/SearchService.ts
[error] 6-50: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
🔇 Additional comments (20)
frontend/src/pages/content/ContentView.tsx (1)
24-24: Properly restructured helper function import path.The import path for
normalizeContentDateshas been correctly updated from"../../services/contentHelper"to"../../utils/contentHelper", which aligns with best practices of keeping helper functions in a utils directory rather than services.frontend/src/components/feed/ContentList.tsx (1)
6-6: Consistent import path update for helper function.This change properly aligns with the same import path update made in ContentView.tsx, ensuring consistency across the codebase by moving helper functions from services to utils.
frontend/src/hooks/AuthProvider.tsx (2)
8-8: Added navigation capability to AuthProvider.The useNavigate hook has been properly imported and initialized to enable programmatic navigation within the authentication flow.
Also applies to: 16-17
31-31: Improved logout UX with automatic redirection.Adding navigation to the login page after successful logout improves the user experience by ensuring users are directed to an appropriate page after ending their session.
backend/src/modules/search/routes/search.routes.ts (1)
6-7: Standardized API endpoint URLs and aligned handler names.The changes correctly:
- Remove trailing slashes from route paths for consistency
- Update the handler for the content endpoint to use
searchContentswhich matches the controller method nameThese improvements ensure better maintainability and alignment between routes and controller methods.
frontend/src/App.tsx (1)
58-59: Restructured component hierarchy for proper hook usage.The order of
RouterandAuthProvidercomponents has been swapped to support the use of navigation hooks withinAuthProvider. This change is necessary becauseAuthProvidernow usesuseNavigatefor handling logout redirects, which requires it to be a child of theRoutercomponent.Also applies to: 106-107
frontend/src/pages/pro/ProDetails.tsx (2)
5-5: Added SubscriptionService import to centralize API interactions.Good refactoring choice to replace direct API calls with a dedicated service abstraction.
22-31: Improved error handling with SubscriptionService implementation.The refactoring properly uses the new service layer for subscription status checks and implements a cleaner error handling pattern by checking if the returned value is an
Errorinstance rather than using try-catch blocks.frontend/src/pages/pro/SubscribePro.tsx (3)
4-4: Added SubscriptionService import to centralize API interactions.Good refactoring choice to replace direct API calls with a dedicated service abstraction.
38-56: Improved subscription status check with service abstraction.The refactored code properly delegates the API interaction to the SubscriptionService and handles error cases appropriately. The type checking against
Errorinstances provides a clear control flow.
67-76: Enhanced subscription session creation with service abstraction.The implementation correctly uses the SubscriptionService for creating a subscription session and handles errors appropriately. The error handling pattern is consistent with other refactored components.
backend/src/modules/search/controllers/search.controller.ts (2)
7-35: Improved documentation and error handling for searchUsers.The addition of detailed JSDoc comments and explicit type handling for query parameters enhances code readability and type safety. The error handling now distinguishes between
Errorinstances and other error types, providing more detailed logging.
37-62: Renamed searchContent to searchContents for consistency.The method has been appropriately renamed to follow consistent naming conventions (plural for collections). The implementation includes improved error handling with detailed logging and proper type handling for query parameters.
backend/src/modules/search/services/search.service.ts (7)
19-21: LGTM: Improved environment variable handlingThe explicit type casting of environment variables to string enhances type safety.
Also applies to: 24-29
53-66: Improved code organization and query constructionThe refactored query construction with a base query and conditional
startAfteris cleaner and avoids code duplication.
70-79: Better type safety and structured return objectAdding explicit typing for users as
User[]and returning a structured object improves type safety and API consistency.
93-96: Efficient early return patternAdding an early return for empty search text is a good optimization that avoids unnecessary processing.
127-130: Improved type safety with type guardThe use of a type guard in the filter function ensures type safety by properly narrowing the type.
132-135: Consistent return structureThe structured return with
contentsandnextStartingPointaligns with frontend expectations for easier integration.
137-138: Enhanced error loggingThe improved error logging provides better traceability for debugging search failures.
| if (uniqueContents && uniqueContents.length > 0) { | ||
| setContentReturned((prev) => [...prev, ...uniqueContents]); | ||
| } else { | ||
| setUserDisabled(true); // Disable the button if no unique users are found | ||
| } |
There was a problem hiding this comment.
Wrong state variable updated – breaks “fetch more” logic for content.
Inside fetchContentData the code disables the “fetch more” button by calling setUserDisabled(true) – that’s the flag that controls USER pagination, not CONTENT pagination. As a result, the user button may be disabled unexpectedly while the content button is never disabled.
- setUserDisabled(true); // Disable the button if no unique users are found
+ // Disable the content "Fetch more" button (add a dedicated piece of state)
+ // setContentDisabled(true);Consider introducing a separate contentDisabled state or re-using numberOfContentsToDisplay to decide when to hide the button.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In frontend/src/components/search/SearchListContent.tsx around lines 147 to 151,
the code incorrectly calls setUserDisabled(true) to disable the "fetch more"
button for content pagination, which controls user pagination instead. To fix
this, introduce a separate state variable like contentDisabled or use
numberOfContentsToDisplay to manage the content fetch button's disabled state,
and update that state here instead of setUserDisabled.
| const [usersReturned, setUsersReturned] = useState<User[]>([]); | ||
| const [userDisabled, setUserDisabled] = useState(true); | ||
| const [contentReturned, setContentReturned] = useState<Content[]>([]); | ||
|
|
||
| const [userStartingPoint, setUserStartingPoint] = useState<string | null>( | ||
| null | ||
| ); | ||
|
|
||
| const [contentReturned, setContentReturned] = useState<Content[]>([]); | ||
| // const [contentStartingPoint, setContentStartingPoint] = useState< |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Reset pagination & internal state when the query changes.
usersReturned, contentReturned, userStartingPoint, and numberOfContentsToDisplay keep their old values when the user types a brand-new query.
This means the very first backend request for a new term will start from the previous userStartingPoint, skipping early results and potentially showing stale data.
useEffect(() => {
+ // When the query itself changes, clear all accumulated state
+ setUsersReturned([]);
+ setContentReturned([]);
+ setUserStartingPoint(null);
+ setNumberOfContentsToDisplay(5);Adding the reset in the same useEffect (before the fetch calls) makes the component behave like a fresh search every time.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In frontend/src/components/search/SearchListContent.tsx around lines 24 to 31,
the state variables usersReturned, contentReturned, userStartingPoint, and
numberOfContentsToDisplay are not reset when the search query changes, causing
stale or skipped results. To fix this, add a reset for these state variables at
the start of the useEffect that triggers on query changes, setting usersReturned
and contentReturned to empty arrays, userStartingPoint to null, and
numberOfContentsToDisplay to its initial value, ensuring each new query starts
fresh.
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
frontend/src/components/search/SearchListResults.tsx (1)
192-200:⚠️ Potential issue“Fetch more Content” button never appears for service-fetched data
The visibility check relies on the prop
contentSearchResults, but after the first request all content lives in the localcontentReturnedstate.
When the component is mounted without pre-loaded props, the button is never rendered, preventing pagination.-{contentSearchResults && - contentSearchResults.length > numberOfContentsToDisplay && ( +{contentReturned.length > numberOfContentsToDisplay && (Aligning the condition with
contentReturned(or introducing acontentDisabledflag similar to users) restores the expected behaviour.
🧹 Nitpick comments (2)
frontend/src/components/search/SearchListResults.tsx (2)
69-73: Avoid blocking UI withalertfor concurrent fetch protectionUsing
alert("Already Fetching!")freezes the tab and provides a jarring UX. Simply return silently or show a non-blocking toast/snackbar.- if (fetching) { - alert("Already Fetching!"); - return; - } + if (fetching) return; // or display a non-modal notificationThis keeps the interface responsive while still preventing duplicate requests.
37-38: Singlefetchingflag serialises user & content requestsBecause
fetchingis shared, callingfetchUserDataandfetchContentDataback-to-back means the second call is blocked until the first finishes.
For slow networks this delays data arrival unnecessarily.Introduce independent flags, e.g.
fetchingUsersandfetchingContents, or track ongoing promises per resource so both requests can run concurrently without race conditions.Also applies to: 69-75, 122-129
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
backend/src/modules/search/routes/search.routes.ts(1 hunks)backend/src/modules/subscription/services/stripe.service.ts(1 hunks)frontend/src/components/Navbar.tsx(3 hunks)frontend/src/components/search/SearchListResults.tsx(5 hunks)
✅ Files skipped from review due to trivial changes (1)
- backend/src/modules/subscription/services/stripe.service.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- backend/src/modules/search/routes/search.routes.ts
- frontend/src/components/Navbar.tsx
| useEffect(() => { | ||
| if (userSearchResults) { | ||
| setUsersReturned(userSearchResults); | ||
| } else if (param) { | ||
| } else if (query) { | ||
| fetchUserData(); | ||
| } | ||
|
|
||
| if (contentSearchResults) { | ||
| setContentReturned(contentSearchResults); | ||
| } else if (param) { | ||
| } else if (query) { | ||
| fetchContentData(); | ||
| } | ||
| }, [userSearchResults, contentSearchResults, param]); | ||
| }, [userSearchResults, contentSearchResults, query]); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
State is not reset when the query string changes
When query changes, the previous usersReturned, contentReturned, and pagination pointers persist, mixing results from different searches.
useEffect(() => {
+ // Reset state on query change
+ setUsersReturned([]);
+ setContentReturned([]);
+ setUserStartingPoint(null);
+ setUserDisabled(true);
+
if (userSearchResults) {
…Resetting prevents stale data leakage and ensures correct pagination for each distinct query.
📝 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.
| useEffect(() => { | |
| if (userSearchResults) { | |
| setUsersReturned(userSearchResults); | |
| } else if (param) { | |
| } else if (query) { | |
| fetchUserData(); | |
| } | |
| if (contentSearchResults) { | |
| setContentReturned(contentSearchResults); | |
| } else if (param) { | |
| } else if (query) { | |
| fetchContentData(); | |
| } | |
| }, [userSearchResults, contentSearchResults, param]); | |
| }, [userSearchResults, contentSearchResults, query]); | |
| useEffect(() => { | |
| // Reset state on query change | |
| setUsersReturned([]); | |
| setContentReturned([]); | |
| setUserStartingPoint(null); | |
| setUserDisabled(true); | |
| if (userSearchResults) { | |
| setUsersReturned(userSearchResults); | |
| } else if (query) { | |
| fetchUserData(); | |
| } | |
| if (contentSearchResults) { | |
| setContentReturned(contentSearchResults); | |
| } else if (query) { | |
| fetchContentData(); | |
| } | |
| }, [userSearchResults, contentSearchResults, query]); |
🤖 Prompt for AI Agents
In frontend/src/components/search/SearchListResults.tsx around lines 44 to 56,
the state variables usersReturned, contentReturned, and pagination pointers are
not reset when the query string changes, causing stale data from previous
searches to persist. To fix this, add logic inside the useEffect hook to reset
usersReturned, contentReturned, and pagination states to their initial empty or
default values whenever the query changes before fetching new data. This ensures
fresh results and correct pagination for each new query.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
frontend/src/components/search/SearchListResults.tsx (1)
42-55: State is still not reset whenquerychanges – stale results leak between searches
This issue was flagged in a previous review and remains unresolved.
Before fetching for a new query, clearusersReturned,contentReturned, pagination pointers, and disable flags to avoid mixing results from different queries.
🧹 Nitpick comments (3)
frontend/src/components/search/SearchListResults.tsx (3)
164-169: Use stable keys instead of array indexReact keys should be stable across re-renders to prevent unnecessary mounting/unmounting.
Useuser.uid(already available) instead of the array index.- usersReturned.map((user: User, index) => ( - <div key={index} className='searchItem'> + usersReturned.map((user: User) => ( + <div key={user.uid} className='searchItem'>
180-184: Same key issue for content items- contentReturned.map((content: Content, index) => ( - <div key={index} className='searchItem'> + contentReturned.map((content: Content) => ( + <div key={content.uid} className='searchItem'>
65-152: Consider extracting shared fetch logic to reduce duplication and race conditionsBoth
fetchUserDataandfetchContentDatashare ~90 % identical logic (fetch → dedupe → update state → toggle disabled flag).
Duplicated asynchronous code makes maintenance harder and increases the chance of subtle drift.Suggestion:
- Create a generic helper that accepts:
• fetchFn (SearchService method)
• existingItems
• setItems / setStartingPoint / setDisabled- Debounce or abort in-flight requests when
querychanges to avoid race conditions (e.g.,AbortControlleror a localcancelledflag).This keeps the component lean and makes future behaviour changes (e.g., new pagination scheme) one-liner updates.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Lite
📒 Files selected for processing (2)
frontend/src/components/search/SearchListResults.tsx(1 hunks)frontend/src/services/SearchService.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/services/SearchService.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
frontend/src/components/search/SearchListResults.tsx (2)
frontend/src/services/SearchService.ts (1)
SearchService(6-51)frontend/src/components/search/ContentSearchResult.tsx (1)
ContentSearchResult(4-29)
Summary by CodeRabbit