Refactor API helper function param structure #2347
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
npm warn config production Use WalkthroughThis pull request introduces a systematic refactoring of the Changes
Possibly related PRs
Suggested Labels
Suggested Reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
sunilsabatp
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/features/user/PlanetCash/screens/Transactions.tsx (1)
66-71: Check for concurrency or paging edge casesUsing an object parameter is a cleaner approach. If multiple calls to
fetchTransactionscan happen concurrently (for instance, via repeated “Load More” clicks), consider whether the in-flight request or the updatedtransactionHistorymight lead to race conditions or UI inconsistencies.pages/sites/[slug]/[locale]/profile/history.tsx (1)
113-122: Potential duplication ofgetAuthenticatedRequestusage.
The code for fetching payment history is repeated with slightly different parameters. If further expansions are needed, consider extracting or unifying these calls to reduce duplication.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
pages/sites/[slug]/[locale]/profile/bulk-codes/[method]/[id].tsx(1 hunks)pages/sites/[slug]/[locale]/profile/history.tsx(3 hunks)pages/sites/[slug]/[locale]/profile/projects/[id].tsx(1 hunks)pages/sites/[slug]/[locale]/profile/recurrency.tsx(1 hunks)src/features/user/BulkCodes/components/ProjectSelector.tsx(1 hunks)src/features/user/ManagePayouts/index.tsx(1 hunks)src/features/user/ManageProjects/ProjectsContainer.tsx(1 hunks)src/features/user/ManageProjects/components/ProjectCertificates.tsx(1 hunks)src/features/user/ManageProjects/components/ProjectMedia.tsx(1 hunks)src/features/user/ManageProjects/components/ProjectSites.tsx(1 hunks)src/features/user/ManageProjects/components/ProjectSpending.tsx(1 hunks)src/features/user/ManageProjects/index.tsx(1 hunks)src/features/user/PlanetCash/index.tsx(1 hunks)src/features/user/PlanetCash/screens/Transactions.tsx(1 hunks)src/features/user/RegisterTrees/RegisterTreesWidget.tsx(1 hunks)src/features/user/Settings/ApiKey/ApiKeyForm.tsx(1 hunks)src/features/user/Settings/EditProfile/AddressManagement/index.tsx(1 hunks)src/features/user/TreeMapper/Analytics/index.tsx(1 hunks)src/features/user/TreeMapper/Import/components/PlantingLocation.tsx(2 hunks)src/features/user/TreeMapper/Import/index.tsx(1 hunks)src/features/user/TreeMapper/MySpecies/MySpeciesForm.tsx(1 hunks)src/features/user/TreeMapper/index.tsx(2 hunks)src/utils/apiRequests/api.ts(2 hunks)
🧰 Additional context used
📓 Learnings (1)
src/features/user/Settings/EditProfile/AddressManagement/index.tsx (1)
Learnt from: mohitb35
PR: Plant-for-the-Planet-org/planet-webapp#2286
File: src/features/user/Settings/EditProfile/AddressManagement/index.tsx:52-65
Timestamp: 2024-12-12T06:50:16.513Z
Learning: In future reviews, do not suggest adding a retry mechanism to the `updateUserAddresses` function in `src/features/user/Settings/EditProfile/AddressManagement/index.tsx`.
🔇 Additional comments (25)
src/features/user/TreeMapper/Analytics/index.tsx (1)
26-31: Great use of the new object-parameter structure.Switching from multiple arguments to a single requestOptions object is more robust. The typed parameter
<MapProject[]>is consistent with the returned data type.src/features/user/BulkCodes/components/ProjectSelector.tsx (1)
34-43: Neat inclusion of query parameters in the request object.Passing
legacyPriceFordynamically whenuser != nullis a good use of object spreading. Ensure that all downstream APIs handle these query parameters gracefully, particularly ifuser.idis unavailable or undefined.src/features/user/Settings/ApiKey/ApiKeyForm.tsx (1)
51-56: Clean and consistent refactor to use object-based parameters.The updated code is more readable and maintainable. The default values and error handling remain intact.
src/features/user/ManagePayouts/index.tsx (1)
79-84: Refactored API call aligns well with the new object structure.Ensuring that asynchronous state updates are properly handled keeps this approach safe. The
setIsDataLoading(false)after the request completes is a good choice to manage the UI state consistently.pages/sites/[slug]/[locale]/profile/recurrency.tsx (1)
55-60: Ensure robust handling of optional parametersConsolidating parameters into an object significantly improves readability and maintainability. Verify that
tenantConfig.id,token, andlogoutUserare always correctly set (e.g., when the user session expires or tenant info is missing). Consider adding fallback behavior or explicit error handling for when these fields are undefined or null.pages/sites/[slug]/[locale]/profile/projects/[id].tsx (1)
71-76: Validate function calls for potential null valuesThe new object-based parameter passing is consistent and clear. However, if
projectGUIDis undefined ortenantConfig.idis null, this request may fail. Ensure there is appropriate defensive code or conditional checks to handle such edge cases before making the request.src/features/user/TreeMapper/Import/index.tsx (1)
68-73: Confirm route parameters before sending requestsThe object-based call to
getAuthenticatedRequestis clear. However, ifrouter.query.locis null or malformed, the request could fail silently. Either add guards or verify that this query parameter is always defined before callingfetchPlantLocation.src/features/user/TreeMapper/MySpecies/MySpeciesForm.tsx (1)
54-59: Great job consolidating parameters into a single object
This refactor significantly improves readability and reduces the risk of confusion from multiple positional arguments.pages/sites/[slug]/[locale]/profile/bulk-codes/[method]/[id].tsx (1)
66-75: Kudos on adopting a descriptive object-based request
Using an object that includestenant,url,token, andlogoutUserclarifies what is being passed in, simplifying future maintenance.src/features/user/PlanetCash/index.tsx (1)
89-94: Object-based parameters enhance maintainability
Adopting a more declarative style forgetAuthenticatedRequesthelps centralize the logic for token handling and potential future additions of parameters or defaults.src/features/user/ManageProjects/ProjectsContainer.tsx (1)
115-120: Well-structured request reduces risk of misordered parameters
Switching to a single object for parameters ensures that the correct values are sent and can make future modifications more straightforward.pages/sites/[slug]/[locale]/profile/history.tsx (1)
96-101: No issues found.
The parameter refactoring forgetAuthenticatedRequestlooks clear and consistent.src/features/user/Settings/EditProfile/AddressManagement/index.tsx (1)
55-60: Looks good.
The object-parameter approach forgetAuthenticatedRequestaligns well with the PR objective.src/features/user/TreeMapper/index.tsx (2)
49-55: Consistent parameter object usage.
No issues found. The function call is consistent with the new parameter structure.
93-99: Code clarity improved.
Passing a version string as part of the object clarifies how the API version is specified.src/utils/apiRequests/api.ts (2)
10-19: Interface definition aligns with usage.
TheGetAuthRequestOptionsinterface clearly defines all needed properties. Good job adding optional fields with defaults.
88-96: Solid refactor to a single object parameter.
This refactoring enhances readability and maintainability, especially with sensible defaults forheader,queryParams, andversion.src/features/user/ManageProjects/index.tsx (1)
153-158: Refactor to object-based parameter passing is consistent and improves clarity.The updated approach successfully eliminates the reliance on positional arguments. Make sure all references to
getAuthenticatedRequestin this file and others have been updated to match this new signature.src/features/user/ManageProjects/components/ProjectCertificates.tsx (1)
136-141: Transition to an object parameter eliminates parameter ordering issues.This refactoring clearly associates each parameter with its purpose. Ensure that any references to the
CertificateScopeProjectstype orgetAuthenticatedRequestare consistent across the codebase.src/features/user/ManageProjects/components/ProjectMedia.tsx (1)
70-75: Enhanced readability by collecting parameters into one object.This change aligns well with the pull request objectives. Just verify downstream error handling remains intact with the updated parameter structure.
src/features/user/ManageProjects/components/ProjectSpending.tsx (1)
170-175: Object-based parameter usage fosters maintainability and clarity.The new approach eliminates confusion surrounding parameter order. Please confirm all usage sites for
fetchProjSpendingreflect this signature.src/features/user/RegisterTrees/RegisterTreesWidget.tsx (1)
219-224: Great usage of object parameters.Well-structured call to
getAuthenticatedRequestusing an object parameter for improved readability and maintainability.src/features/user/TreeMapper/Import/components/PlantingLocation.tsx (2)
216-221: Consider verifying the tenant property.Ensure
tenantis always defined intenantConfig?.id. IftenantConfigis sometimesnullorundefined, handle that gracefully to avoid runtime errors.
230-235: Consistent approach to fetching species.Fetching species through the new object-parameter approach is consistent with the refactor. Looks good!
src/features/user/ManageProjects/components/ProjectSites.tsx (1)
338-343: Refactored request with object arguments.The updated code is clearer and less prone to parameter ordering errors. Good job!
There was a problem hiding this comment.
Actionable comments posted: 5
🔭 Outside diff range comments (3)
src/features/user/Settings/DeleteProfile/DeleteProfileForm.tsx (1)
Line range hint
20-21: Fix state setter naming conventionsThe state setter functions should follow camelCase naming conventions. Currently, they're inconsistent with React best practices.
Apply these changes:
- const [isModalOpen, setisModalOpen] = React.useState(false); - const [canDeleteAccount, setcanDeleteAccount] = React.useState(false); + const [isModalOpen, setIsModalOpen] = React.useState(false); + const [canDeleteAccount, setCanDeleteAccount] = React.useState(false);Remember to update all occurrences of these setter functions throughout the component.
src/features/user/Settings/ImpersonateUser/SupportPin.tsx (1)
Line range hint
23-31: Enhance error handling and user feedback.The current error handling could be improved in several ways:
- The
return falsestatement is unreachable as the function is implicitly void- Users receive no feedback when errors occur
- Error logging should be more informative
Consider this improvement:
} catch (err) { - console.log(err); + console.error('Failed to update support pin:', err); + // Consider using a toast or alert to inform the user + throw err; // Let error boundary handle it }src/utils/apiRequests/api.ts (1)
Line range hint
79-350: Refactor common header construction logic.There's significant code duplication in header construction across all authenticated request functions. Consider extracting this into a utility function.
/** Constructs common headers for authenticated requests */ function constructAuthHeaders( tenant: string | undefined, token: string | null, lang: string, version = '1.0.3', additionalHeaders: Record<string, string> = {} ): Promise<Record<string, string>> { return { 'Content-Type': 'application/json', 'tenant-key': `${tenant}`, 'X-SESSION-ID': await getsessionId(), Authorization: `Bearer ${token}`, 'x-locale': lang, 'x-accept-versions': version, ...additionalHeaders, }; }This would simplify the request functions and ensure consistent header construction.
🧹 Nitpick comments (20)
src/features/user/Settings/DeleteProfile/DeleteProfileForm.tsx (1)
Line range hint
41-51: Consider enhancing error handling maintainabilityWhile the error handling is functional, it could be more maintainable by extracting error types into constants and using a more structured approach.
Consider refactoring like this:
// Add at the top of the file const ERROR_TYPES = { ACTIVE_SUBSCRIPTIONS: 'active_subscriptions' } as const; // Then in the error handling: const handleError = (error: SerializedError) => { switch (error.message) { case ERROR_TYPES.ACTIVE_SUBSCRIPTIONS: setIsModalOpen(true); break; default: return error; } }; // Usage in the try-catch: const _serializedErrors = serializedErrors .map(handleError) .filter((error): error is SerializedError => error !== undefined);This approach:
- Makes error types maintainable and reusable
- Reduces the risk of typos in error message strings
- Makes it easier to add new error cases
src/features/user/TreeMapper/Import/components/SampleTrees.tsx (1)
143-149: Ensure object-based parameters follow consistent typing and handletenantConfig?.idfallback.Great job refactoring the API call to use a single object parameter. This improves readability and reduces positional-parameter errors. However, consider handling the case where
tenantConfig?.idmight be undefined. A fallback or explicit check can prevent potential runtime issues.src/utils/maps/plantLocations.ts (1)
77-77: Confirm the version number against versioning standards.Hardcoding the version string as
'1.0.4'might be acceptable if it remains constant, but consider if a constant or configuration file reference better aligns with your versioning practices. That way, changes to the API version can be centralized.src/features/user/BulkCodes/forms/IssueCodesForm.tsx (1)
155-164: Successfully refactored the request parameters into a single object.This aligns well with the PR objective to enhance maintainability. It reduces the reliance on positional arguments and clarifies which parameters are being passed to the API request.
Consider ensuring that
tenantConfig?.idis consistently available and not undefined in edge scenarios. Also, adding inline documentation for fields liketenant,url,token, andheaderscould further improve readability.src/features/user/Account/PauseModal.tsx (1)
91-97: Consider adding a TypeScript interface for the request object.Although this is a notable improvement, strongly typing the request fields can provide further clarity and catch potential issues at compile time.
src/features/user/Widget/DonationLink/index.tsx (1)
26-31: Consider default values for optional parametersWhen the
tenantConfigorlocaleis undefined, the query parameters might lead to unintended behavior. If these props are optional, consider using a default value or adding validation before passing them to the query parameters.src/features/projectsV2/ProjectsContext.tsx (1)
207-208: Consider adding default query parameters for robust usage
Providing defaults (e.g., fallback currency or additional sensible defaults) within thequeryParamsobject could help prevent frequent null checks in downstream logic.pages/sites/[slug]/[locale]/home.tsx (1)
70-73: Avoid code duplication inloadLeaderboardandloadTenantScore
Both blocks callgetRequestwith nearly identical syntax, which might be a candidate for refactoring to a reusable helper function, if the logic and data structures are similar. This would further simplify future maintenance efforts.src/features/user/RegisterTrees/RegisterTreesWidget.tsx (2)
192-198: LGTM! Consider adding TypeScript interface for request options.The refactoring from positional parameters to an object parameter structure improves maintainability and readability. The implementation correctly includes all necessary parameters and maintains error handling.
Consider defining a TypeScript interface for the request options to enhance type safety:
interface AuthenticatedRequestOptions<T> { tenant: string; url: string; data: T; token: string; logoutUser: () => void; }
Line range hint
192-224: Excellent architectural improvement in API request handling.The consistent use of object parameters for API requests across both functions enhances maintainability and readability. This pattern makes it easier to:
- Add or remove parameters without breaking existing calls
- Understand the purpose of each parameter
- Maintain consistent error handling
Consider applying this pattern consistently across all API requests in the codebase to maintain uniformity.
pages/sites/[slug]/[locale]/projects-archive/index.tsx (1)
117-119: Consider de-duplicating tenant usage.The
tenantfield is used both at the top level (tenant: pageProps.tenantConfig.id) and again inqueryParams.tenant. If the backend requires duplication, that’s fine; otherwise, removing the extratenantfield could reduce clutter.src/features/user/Settings/ImpersonateUser/SupportPin.tsx (2)
18-23: LGTM! Consider adding default values for optional parameters.The refactoring to use an object parameter structure improves readability and maintainability. This aligns well with the PR objectives.
Consider defining default values for optional parameters as mentioned in the PR objectives. This could be done by creating an interface for the options:
interface AuthenticatedRequestOptions { tenant: string; url: string; token: string; logoutUser?: () => void; }
Line range hint
7-9: Consider enhancing TypeScript definitions.The
SupportPininterface could be more descriptive and potentially shared if used elsewhere in the application.Consider this enhancement:
export interface SupportPinResponse { supportPin: string; // Add other potential fields returned by the API }src/features/user/Profile/ForestProgress/TargetsModal.tsx (1)
81-87: Use a strongly typed parameter object for clarity and maintainabilityRefactoring these parameters into a single object aligns well with the PR goal. To further enhance type safety, consider declaring a dedicated interface or type alias that specifies each field (e.g.,
tenant,url,data,token,logoutUser) and its optionality. This reduces any ambiguity, making it easier for others to understand and use the function.src/features/user/Account/CancelModal.tsx (1)
85-91: Great refactor towards a more maintainable parameter structure.By consolidating parameters into an object, you’ve made the codebase more readable and minimized the risk of passing parameters in the wrong order. Consider the following to further improve this change:
Add a Typed Interface
Defining a dedicated interface for this parameter object (e.g.,PutRequestParams) can help developers quickly understand the allowed fields and their types. It will also reduce guesswork and potential runtime mistakes.Handle Optional Tenant
IftenantConfig?.idcan beundefined, consider adding a fallback or explicit error handling to avoid passing an unexpectedundefinedtenant toputAuthenticatedRequest.Otherwise, this update aligns nicely with the PR objective, and the usage is straightforward.
src/features/user/Profile/ProfileCard/RedeemModal/index.tsx (1)
53-59: Great use of the new parameters object approach, but consider additional null checks.The refactoring to a single object parameter is consistent with the PR's objective. A minor enhancement would be to ensure
tenantConfig?.idandtokenare defined before making the request, as missing values could cause unexpected behavior.src/utils/apiRequests/api.ts (3)
16-50: Enhance type safety and add documentation.The interfaces look well-structured, but there are a few improvements we can make:
- Replace
anytypes with more specific types for better type safety- Add JSDoc documentation to describe the purpose and usage of each interface
Here's how to improve the type safety:
interface PostAuthRequestOptions extends BaseRequestOptions { - data: any; + data: Record<string, unknown>; token: string | null; logoutUser: (value?: string | undefined) => void; headers?: Record<string, string>; } interface PutAuthRequestOptions extends BaseRequestOptions { - data?: any; + data?: Record<string, unknown>; token: string | null; logoutUser: (value?: string | undefined) => void; } interface PostRequestOptions extends BaseRequestOptions { - data: any; + data: Record<string, unknown>; }And add documentation:
/** Base interface for all request options */ interface BaseRequestOptions { tenant?: string | undefined; url: string; } /** Options for authenticated GET requests */ interface GetAuthRequestOptions extends BaseRequestOptions { token: string | null; logoutUser: (value?: string | undefined) => void; header?: Record<string, string> | null; queryParams?: { [key: string]: string }; version?: string; } // Add similar documentation for other interfaces🧰 Tools
🪛 GitHub Check: CodeFactor
[warning] 45-45: src/utils/apiRequests/api.ts#L45
Unexpected any. Specify a different type. (@typescript-eslint/no-explicit-any)
[warning] 35-35: src/utils/apiRequests/api.ts#L35
Unexpected any. Specify a different type. (@typescript-eslint/no-explicit-any)
[warning] 44-44: src/utils/apiRequests/api.ts#L44
Unexpected any. Specify a different type. (@typescript-eslint/no-explicit-any)
Line range hint
52-71: Address the TODO comment and improve error handling.The TODO comment indicates missing error handling for the
getAccountInfofunction.Would you like me to help implement proper error handling for the impersonation case? I can propose a solution that aligns with the error handling pattern used in other functions.
🧰 Tools
🪛 GitHub Check: CodeFactor
[warning] 45-45: src/utils/apiRequests/api.ts#L45
Unexpected any. Specify a different type. (@typescript-eslint/no-explicit-any)
[warning] 35-35: src/utils/apiRequests/api.ts#L35
Unexpected any. Specify a different type. (@typescript-eslint/no-explicit-any)
[warning] 44-44: src/utils/apiRequests/api.ts#L44
Unexpected any. Specify a different type. (@typescript-eslint/no-explicit-any)
[warning] 57-57: src/utils/apiRequests/api.ts#L57
Unexpected any. Specify a different type. (@typescript-eslint/no-explicit-any)
119-127: Verify version parameter consistency.The
versionparameter has a default value of '1.0.3', but this value is duplicated in the header construction. Consider using a constant for the default version.const DEFAULT_API_VERSION = '1.0.3'; export function getAuthenticatedRequest<T>({ tenant, url, token, logoutUser, header = null, queryParams = {}, version = DEFAULT_API_VERSION, }: GetAuthRequestOptions): Promise<T> {src/features/user/Settings/EditProfile/EditProfileForm.tsx (1)
165-171: Consider clarifying parameter naming and ensuring default values.By switching to a single object parameter, readability is improved. However, consider renaming
tenanttotenantIdfor clarity. Additionally, confirm that the functionputAuthenticatedRequesthas sensible defaults for any other optional parameters not provided here.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (61)
pages/sites/[slug]/[locale]/all.tsx(4 hunks)pages/sites/[slug]/[locale]/claim/[type]/[code].tsx(1 hunks)pages/sites/[slug]/[locale]/home.tsx(2 hunks)pages/sites/[slug]/[locale]/profile/redeem/[code].tsx(1 hunks)pages/sites/[slug]/[locale]/projects-archive/[p].tsx(1 hunks)pages/sites/[slug]/[locale]/projects-archive/index.tsx(1 hunks)pages/sites/[slug]/[locale]/s/[id].tsx(1 hunks)pages/sites/[slug]/[locale]/t/[profile].tsx(1 hunks)src/features/common/Layout/CurrencyContext.tsx(1 hunks)src/features/common/Layout/UserPropsContext.tsx(1 hunks)src/features/projects/screens/Projects.tsx(1 hunks)src/features/projectsV2/ProjectDetails/index.tsx(2 hunks)src/features/projectsV2/ProjectsContext.tsx(1 hunks)src/features/user/Account/CancelModal.tsx(1 hunks)src/features/user/Account/EditModal.tsx(1 hunks)src/features/user/Account/PauseModal.tsx(1 hunks)src/features/user/Account/ReactivateModal.tsx(1 hunks)src/features/user/Account/components/DownloadCodes.tsx(1 hunks)src/features/user/BulkCodes/forms/IssueCodesForm.tsx(1 hunks)src/features/user/BulkCodes/index.tsx(1 hunks)src/features/user/CompleteSignup/index.tsx(1 hunks)src/features/user/ManagePayouts/index.tsx(2 hunks)src/features/user/ManagePayouts/screens/AddBankAccount.tsx(1 hunks)src/features/user/ManagePayouts/screens/EditBankAccount.tsx(1 hunks)src/features/user/ManagePayouts/screens/PayoutScheduleForm.tsx(1 hunks)src/features/user/ManageProjects/components/BasicDetails.tsx(2 hunks)src/features/user/ManageProjects/components/DetailedAnalysis.tsx(1 hunks)src/features/user/ManageProjects/components/ProjectCertificates.tsx(3 hunks)src/features/user/ManageProjects/components/ProjectMedia.tsx(4 hunks)src/features/user/ManageProjects/components/ProjectSites.tsx(4 hunks)src/features/user/ManageProjects/components/ProjectSpending.tsx(3 hunks)src/features/user/ManageProjects/index.tsx(2 hunks)src/features/user/PlanetCash/components/CreateAccountForm.tsx(1 hunks)src/features/user/Profile/ForestProgress/TargetsModal.tsx(1 hunks)src/features/user/Profile/ProfileCard/RedeemModal/index.tsx(1 hunks)src/features/user/Profile/TpoProjects/index.tsx(1 hunks)src/features/user/RegisterTrees/RegisterTrees/UploadImages.tsx(2 hunks)src/features/user/RegisterTrees/RegisterTreesWidget.tsx(2 hunks)src/features/user/Settings/ApiKey/ApiKeyForm.tsx(2 hunks)src/features/user/Settings/DeleteProfile/DeleteProfileForm.tsx(1 hunks)src/features/user/Settings/EditProfile/AddressManagement/AddAddress.tsx(1 hunks)src/features/user/Settings/EditProfile/AddressManagement/DeleteAddress.tsx(1 hunks)src/features/user/Settings/EditProfile/AddressManagement/EditAddress.tsx(1 hunks)src/features/user/Settings/EditProfile/AddressManagement/UnsetBillingAddress.tsx(1 hunks)src/features/user/Settings/EditProfile/AddressManagement/UpdateAddressType.tsx(1 hunks)src/features/user/Settings/EditProfile/EditProfileForm.tsx(2 hunks)src/features/user/Settings/ImpersonateUser/ImpersonateUserForm.tsx(1 hunks)src/features/user/Settings/ImpersonateUser/SupportPin.tsx(1 hunks)src/features/user/TreeMapper/Import/components/PlantingLocation.tsx(3 hunks)src/features/user/TreeMapper/Import/components/SampleTrees.tsx(1 hunks)src/features/user/TreeMapper/MySpecies/MySpeciesForm.tsx(3 hunks)src/features/user/TreeMapper/MySpecies/SpeciesAutoComplete.tsx(1 hunks)src/features/user/Widget/DonationLink/index.tsx(1 hunks)src/features/user/Widget/EmbedModal.tsx(1 hunks)src/tenants/planet/LeaderBoard/components/Score.tsx(1 hunks)src/tenants/salesforce/Mangroves/components/ProjectGrid.tsx(1 hunks)src/tenants/salesforce/OceanforceCampaign/components/ContentSection.tsx(1 hunks)src/tenants/salesforce/VTOCampaign/components/ProjectGrid.tsx(1 hunks)src/tenants/salesforce/VTOCampaign2023/components/ProjectGrid.tsx(1 hunks)src/utils/apiRequests/api.ts(7 hunks)src/utils/maps/plantLocations.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (9)
- src/features/user/ManageProjects/index.tsx
- src/features/user/ManageProjects/components/ProjectMedia.tsx
- src/features/user/Settings/ApiKey/ApiKeyForm.tsx
- src/features/user/ManageProjects/components/ProjectSpending.tsx
- src/features/user/TreeMapper/Import/components/PlantingLocation.tsx
- src/features/user/ManagePayouts/index.tsx
- src/features/user/TreeMapper/MySpecies/MySpeciesForm.tsx
- src/features/user/ManageProjects/components/ProjectSites.tsx
- src/features/user/ManageProjects/components/ProjectCertificates.tsx
🧰 Additional context used
📓 Learnings (2)
src/features/user/Profile/TpoProjects/index.tsx (1)
Learnt from: mohitb35
PR: Plant-for-the-Planet-org/planet-webapp#2324
File: src/features/projectsV2/ProjectsContext.tsx:192-194
Timestamp: 2024-12-10T12:31:50.536Z
Learning: Within `src/features/projectsV2/ProjectsContext.tsx`, in the `loadProjects` function, the condition `projectsLocale === locale && projects !== null` is intentionally used to prevent unnecessary API calls by only fetching projects when the locale changes or if projects have not been fetched yet.
src/features/user/Settings/EditProfile/AddressManagement/UnsetBillingAddress.tsx (1)
Learnt from: mohitb35
PR: Plant-for-the-Planet-org/planet-webapp#2286
File: src/features/user/Settings/EditProfile/AddressManagement/UnsetBillingAddress.tsx:39-62
Timestamp: 2024-12-12T06:27:34.195Z
Learning: Avoid suggesting wrapping functions with `useCallback` when it results in unnecessary optimization, such as for the `unsetAddress` function in `src/features/user/Settings/EditProfile/AddressManagement/UnsetBillingAddress.tsx`.
🪛 GitHub Check: CodeFactor
src/utils/apiRequests/api.ts
[warning] 45-45: src/utils/apiRequests/api.ts#L45
Unexpected any. Specify a different type. (@typescript-eslint/no-explicit-any)
[warning] 35-35: src/utils/apiRequests/api.ts#L35
Unexpected any. Specify a different type. (@typescript-eslint/no-explicit-any)
[warning] 44-44: src/utils/apiRequests/api.ts#L44
Unexpected any. Specify a different type. (@typescript-eslint/no-explicit-any)
🔇 Additional comments (57)
src/features/common/Layout/UserPropsContext.tsx (1)
103-103: Verify tenant presence and error-handling.Passing both
tenantandtokenas part of a single object parameter is consistent with the refactor goals. However, consider verifying thattenantConfig?.idis not null or undefined before invokinggetAccountInfo, and handle potential errors more explicitly.src/features/user/Settings/ImpersonateUser/ImpersonateUserForm.tsx (1)
75-79: Good use of object parameters to improve maintainability.This refactor aligns with the PR’s objective of converting positional parameters into a single object. It enhances clarity and reduces the risk of mismatched arguments. However, ensure that the rest of the code correctly handles cases where
tenantConfig?.idmight beundefined, potentially returning an error from the API call.src/features/user/Settings/DeleteProfile/DeleteProfileForm.tsx (1)
29-34: Well-structured parameter refactoring!The refactored
deleteAuthenticatedRequestcall with named parameters improves code clarity and maintainability. This change aligns well with the PR objectives and TypeScript best practices.src/features/user/Profile/TpoProjects/index.tsx (2)
34-36: Ensure robust tenant handlingIf
tenantConfig?.idis undefined, the request might fail or hit a fallback tenant logic ingetRequest. Consider adding a default or an error-handling scenario to avoid unexpected failures.
37-40: Validate locale usagePassing the
localeinqueryParamsis correct, but consider ensuring consistent locale usage or fallback in caselocaleis undefined, especially if this code is called in multiple places such as SSR or client contexts.src/utils/maps/plantLocations.ts (2)
71-78: Great job migrating to an object parameter forgetRequest.Switching from multiple positional parameters to a single object significantly improves readability and maintainability. The code is much clearer when you see each property explicitly named (e.g.,
tenant,url, andversion). This also reduces the likelihood of mix-ups when more parameters are introduced in the future.
73-73: Verify the string interpolation in the URL.Ensure the string interpolation uses a backtick for correct syntax:
`/app/plantLocations/${project}`. A minor mistake, such as missing the backtick, could break the request URL.src/features/user/Account/PauseModal.tsx (2)
91-97: Well done transitioning to an object-based parameter structure.This refactoring aligns with the PR's objective to improve maintainability and readability by reducing the reliance on multiple positional parameters. The usage of descriptive object keys makes the code more self-documenting.
91-97: Verify tenant configuration fallback.You’re passing
tenantConfig?.idastenant, which could be undefined iftenantConfigis not present. Consider adding a fallback or validation if it’s a required property at runtime.src/features/user/Widget/DonationLink/index.tsx (1)
23-24: Great use of generics in the function callSpecifying
<MapProject[]>provides clear type safety for the returned data. This helps avoid type mismatches and makes the code more maintainable. Good job!src/features/projects/screens/Projects.tsx (1)
175-178: Adoption of the new single-object parameter structure appears correct.The refactored call to
getRequest<Tenant>correctly passes an object with named parameters (tenantandurl). This aligns well with the PR objectives to enhance maintainability and clarity. However, ensure that downstream logic handles cases whereres.topProjectsOnlymight be missing or undefined if the response schema changes.Use the following script to confirm that all
getRequestcalls in the repository conform to the single-object parameter structure:✅ Verification successful
Let me try a more specific pattern to catch potential non-object parameter usage.
Let me try one more search pattern to be thorough.
All getRequest calls follow the new single-object parameter structure
Based on the comprehensive search results, I can confirm that all
getRequestcalls in the codebase are using the new single-object parameter structure. Every instance found shows the correct usage with named parameters passed as an object, including:
- Required parameters like
tenantandurl- Optional parameters like
queryParamswhere neededThe implementation in the reviewed file (
src/features/projects/screens/Projects.tsx) aligns with this consistent pattern across the codebase.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that all getRequest calls use the new object param approach. ast-grep --pattern $'getRequest($_)'Length of output: 2005
Script:
#!/bin/bash # Search for getRequest calls that might not use object parameters ast-grep --pattern 'getRequest($_, $_)'Length of output: 41
Script:
#!/bin/bash # Search for all getRequest usages with ripgrep for more context rg "getRequest" -A 3Length of output: 18150
src/features/projectsV2/ProjectsContext.tsx (1)
198-201: Well-founded change to object-based parameters
This refactoring to pass parameters as an object better aligns with the PR’s objective of improving maintainability and readability. It helps guard against positional parameter mistakes and makes it easy to add new fields in the future. Good call!pages/sites/[slug]/[locale]/home.tsx (1)
55-58: Object parameter usage looks good forgetRequest
The transition to an object parameter forgetRequest<TenantScore>withtenantandurlkeys is clear and aligns with the PR objective to improve readability and maintainability. Nice work!src/features/user/RegisterTrees/RegisterTreesWidget.tsx (1)
219-224: LGTM! Verify API endpoint compatibility.The refactoring to use an object parameter structure is consistent with the overall pattern. The implementation maintains proper error handling and includes all necessary parameters.
Let's verify that the API endpoint is consistently used across the codebase:
✅ Verification successful
API endpoint usage is consistent across the codebase
The verification shows that both instances of the
/app/profile/projectsendpoint inRegisterTreesWidget.tsxandPlantingLocation.tsxuse identical parameter structure withgetAuthenticatedRequest:
- Both include
tenant,url,token, andlogoutUserparameters- Both follow the same object parameter pattern
- The endpoint path is consistently used as
/app/profile/projectsThe router paths in other files (
/profile/projects) are for frontend routing and not related to the API endpoint structure.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for consistent usage of the projects API endpoint # Expected: All calls should use the same endpoint structure # Search for project endpoint usage rg -A 2 "'/app/profile/projects'" # Search for alternative endpoint patterns that might need updating rg -A 2 "'*/projects'" rg -A 2 "'/projects'"Length of output: 2588
Script:
#!/bin/bash # Let's check the implementation in PlantingLocation.tsx to verify if it uses the same parameter structure ast-grep --pattern 'getAuthenticatedRequest<$_>({ $$$ })'Length of output: 61
Script:
#!/bin/bash # Let's check both files for the complete context of how the API is used rg -B 5 -A 5 '/app/profile/projects' src/features/user/TreeMapper/Import/components/PlantingLocation.tsx src/features/user/RegisterTrees/RegisterTreesWidget.tsxLength of output: 2103
pages/sites/[slug]/[locale]/t/[profile].tsx (1)
46-49: Transition to object param usage looks great!This switch to passing parameters as an object instead of multiple positional arguments aligns well with the refactoring goal. It makes the code more readable, maintainable, and less prone to bugs when new parameters need to be added. Make sure to verify that other calls in the codebase to
getRequesthave also been updated accordingly.✅ Verification successful
Let me search for more occurrences of
getRequestwith a different pattern to ensure we catch all usages.
All getRequest calls consistently use object parameters
Based on the comprehensive search results, all instances of
getRequestacross the codebase are already using the object parameter structure. Every call follows the same pattern, passing an object with properties liketenant,url, and optionalqueryParams. The change in the reviewed file aligns perfectly with the existing usage pattern throughout the codebase.Examples of consistent usage:
pages/sites/[slug]/[locale]/t/[profile].tsxsrc/tenants/salesforce/VTOCampaign/components/ProjectGrid.tsxsrc/tenants/salesforce/VTOCampaign2023/components/ProjectGrid.tsx🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verification: Search for all occurrences of getRequest calls, ensuring they utilize the object parameter structure. rg -A 3 'getRequest\s*\('Length of output: 814
Script:
#!/bin/bash # Search for getRequest using ast-grep to catch different patterns of usage ast-grep --pattern 'getRequest($$$)' # Also search with ripgrep including more context and different possible patterns rg -A 5 'getRequest[\s\(]'Length of output: 15386
pages/sites/[slug]/[locale]/s/[id].tsx (1)
41-44: Great job refactoring to an object-based parameter.This aligns well with the PR objective of enhancing maintainability and readability. Please verify that every call site has been updated to match this object-based approach across the codebase.
✅ Verification successful
Let me search for more instances of
getRequestto ensure we have a complete picture.
Let me try a different approach to search for all getRequest usages.
All getRequest calls consistently use the object-based parameter approach
Based on the search results, I can confirm that all instances of
getRequestacross the codebase are already using the object-based parameter approach. Every call follows the same pattern of passing an object with properties liketenant,url, and optionalqueryParams. The refactoring is consistent throughout the codebase.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Recursively search for getRequest usage to ensure it uses the object-based parameter approach everywhere. rg -A 3 "getRequest\s*\("Length of output: 814
Script:
#!/bin/bash # Search for getRequest with more context and include TypeScript/JavaScript files rg -t typescript -t javascript -t tsx -t jsx -C 5 "getRequest\s*[\(<]"Length of output: 107
Script:
#!/bin/bash # Search for getRequest in all files, showing more context rg --type-add 'ts:*.{ts,tsx}' --type-add 'js:*.{js,jsx}' -t ts -t js -C 5 "getRequest\s*[\(<]"Length of output: 21296
pages/sites/[slug]/[locale]/projects-archive/index.tsx (1)
116-126: Nicely done switching to an object parameter structure for the API request.This change significantly improves readability and maintainability by removing ambiguity with multiple positional parameters. The explicit naming of parameters in lines 117–119 (i.e.,
tenant,url, andqueryParams) is consistent with the refactoring goal. Good job!src/tenants/salesforce/VTOCampaign2023/components/ProjectGrid.tsx (1)
22-30: Confirm whethertenantConfig.idis intentionally duplicatedCurrently, the
tenantproperty appears both at the top level (line 22) and as part ofqueryParams(line 28). If the API specifically requires it in both places, then this duplication is fine. Otherwise, you might reduce confusion by removing the duplicate reference.src/features/user/Settings/ImpersonateUser/SupportPin.tsx (1)
18-23: Verify consistency with other refactored components.Let's ensure this refactoring is consistent with other components mentioned in the AI summary.
✅ Verification successful
The refactoring is consistent across all components
After analyzing the codebase, I can confirm that:
- The
putAuthenticatedRequestimplementation inapi.tsshows consistent parameter structure across all usages- All 13 instances of
putAuthenticatedRequestfound in the codebase follow the same pattern, using the required parameters:tenant,url,token, andlogoutUser- The
SupportPin.tsximplementation matches the standard pattern used throughout the codebase🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other putAuthenticatedRequest calls to verify consistent parameter structure rg "putAuthenticatedRequest[<].*[>]?\([{]" --type typescript # Search for related components mentioned in the AI summary rg -l "getAuthenticatedRequest|postAuthenticatedRequest" "pages/sites/*/profile/"Length of output: 244
Script:
#!/bin/bash # Search for other putAuthenticatedRequest calls to verify consistent parameter structure rg "putAuthenticatedRequest\s*[<].*[>]?\s*\([{]" # Search for related components in the entire codebase rg -l "getAuthenticatedRequest|postAuthenticatedRequest" # Find all TypeScript/JavaScript files that might contain authenticated requests fd "\.(ts|tsx|js|jsx)$" -x grep -l "AuthenticatedRequest"Length of output: 6651
Script:
#!/bin/bash # Let's check the implementation of putAuthenticatedRequest to verify the parameter structure cat src/utils/apiRequests/api.tsLength of output: 10251
src/features/user/Account/ReactivateModal.tsx (1)
43-49: Use optional chaining or default values for any optional parameters.This refactored object-based approach looks cleaner and more maintainable, aligning well with the PR’s objectives. Verify any optional parameters (e.g.,
tenant,token) are gracefully handled if undefined to prevent runtime errors. Otherwise, great job!src/features/common/Layout/CurrencyContext.tsx (1)
27-29: Well-done adopting the new object-based request parameter!This change aligns with the refactoring objective by using a named parameter object. It's clearer and less error-prone than multiple positional parameters. Please ensure consistency across the codebase for all
getRequestcalls.To confirm consistency, run the following script and look for calls to
getRequestthat still use positional parameters:✅ Verification successful
The previous script had a regex parsing error. Let's try with proper escaping of the parenthesis. If this doesn't provide enough context, we can try using ast-grep to find function calls more accurately.
Let's try using ast-grep to get a more accurate view of all
getRequestcalls in the codebase, as the previous ripgrep results might have missed some occurrences.
All
getRequestcalls consistently use object-based parametersBased on the verification results, all instances of
getRequestin the codebase (including the reviewed change inCurrencyContext.tsxand other occurrences inProjectGrid.tsxfiles) consistently use the object parameter pattern. Each call passes a configuration object with named properties likeurl,tenant, andqueryParams, which aligns with the refactoring objective.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that all calls to getRequest pass an object parameter rg -A 2 "getRequest("Length of output: 96
Script:
#!/bin/bash # Search for getRequest calls with proper escaping rg -A 2 "getRequest\("Length of output: 633
Script:
#!/bin/bash # Use ast-grep to find getRequest function calls with their full context ast-grep --pattern 'getRequest($$$)'Length of output: 2006
src/features/user/ManageProjects/components/BasicDetails.tsx (1)
387-393: Great consistency in thepostAuthenticatedRequestrefactor.The shift to an object parameter benefits maintainability and protects against parameter ordering errors. Ensure the TypeScript definitions properly match the new object signature (particularly optional properties like
tenantandlogoutUser) and verify that all calls across the codebase have been adjusted to reflect these new parameter names.✅ Verification successful
Let me check the TypeScript definition of
postAuthenticatedRequestto verify the parameter types.
Let me check the type definition of
PostAuthRequestOptionsto verify the optional properties.
Let me try a different pattern to find the type definition.
Let me check the BaseRequestOptions interface since PostAuthRequestOptions extends it.
All
postAuthenticatedRequestcalls are properly refactored and type-safeBased on the verification:
- The function signature uses a properly defined interface
PostAuthRequestOptionsthat extendsBaseRequestOptions- All required properties are properly typed:
tenantis correctly marked as optional (tenant?: string)urlis requireddatais requiredtokenis requiredlogoutUseris requiredheadersis optional- All calls across the codebase consistently use the object parameter pattern with the correct property names
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash #!/bin/bash # Verify the `postAuthenticatedRequest` usage throughout the repository # to check consistent refactoring and alignment with the updated object parameter structure. rg -A 3 'postAuthenticatedRequest'Length of output: 16252
Script:
#!/bin/bash # Check the TypeScript definition of postAuthenticatedRequest ast-grep --pattern 'export function postAuthenticatedRequest<$_>($_) { $$$ }'Length of output: 3028
Script:
#!/bin/bash # Find the PostAuthRequestOptions type definition ast-grep --pattern 'type PostAuthRequestOptions = { $$$ }'Length of output: 62
Script:
#!/bin/bash # Find the PostAuthRequestOptions interface/type definition rg "PostAuthRequestOptions" -A 10Length of output: 1485
Script:
#!/bin/bash # Find the BaseRequestOptions interface definition rg "interface BaseRequestOptions" -A 5Length of output: 321
pages/sites/[slug]/[locale]/all.tsx (5)
50-53: Adopting single-parameter object is consistent and clear.
Great work on switching to a single object parameter, which improves code readability and reduces the chance of passing arguments in the wrong order.
69-72: Consistent refactoring approach.
Using the same object-based approach here forgetRequestmaintains uniformity and reduces potential errors. Nicely done.
88-91: Consider a fallback forprocess.env.WEBHOOK_URL.
While referencing environment variables is fine, ensure there's a safe fallback (e.g., an empty string or a default URL) ifprocess.env.WEBHOOK_URLis undefined, to prevent runtime errors.
105-109: Improved prop formatting for clarity.
Breaking out props onto separate lines enhances readability, making it easier to see which props the<LeaderBoard>component receives. No further issues found here.
114-118: Maintain consistent usage across tenants.
Using the same structured approach for passing props in different tenant configurations helps keep the codebase consistent.src/features/user/Settings/EditProfile/AddressManagement/DeleteAddress.tsx (1)
40-45: Good job adopting the new object-based parameter structure.This refactor improves maintainability and readability by grouping related arguments into a single object. It aligns with the PR objective to replace multiple positional parameters with a more descriptive, typed structure.
src/features/user/ManagePayouts/screens/AddBankAccount.tsx (1)
38-44: Good adoption of object-parameter refactor.Using a single object for parameters, including
tenant,url,data,token, andlogoutUser, improves clarity and scalability. Be sure to verify thattenantConfig?.idmay beundefinedand handle that scenario gracefully if necessary.src/features/user/RegisterTrees/RegisterTrees/UploadImages.tsx (2)
85-90: 🛠️ Refactor suggestionAvoid in-place array updates.
Following the same principle, avoid mutating
uploadedImagesTemp. Instead, create a new copy of the array. For example:- const uploadedImagesTemp = uploadedImages; - const index = uploadedImagesTemp.findIndex((item) => item.id === id); - if (index !== -1) { - uploadedImagesTemp.splice(index, 1); - setUploadedImages(uploadedImagesTemp); - } + setUploadedImages((prev) => { + return prev.filter((item) => item.id !== id); + });Likely invalid or redundant comment.
42-48: 🛠️ Refactor suggestionAvoid mutating state directly.
Assigning
uploadedImagestonewUploadedImagesand pushing to it mutates the same array reference, which can introduce bugs due to React’s immutability requirements. Use a functional state update to create a new array instead:- const newUploadedImages: Image[] = uploadedImages; - newUploadedImages.push(res); - setUploadedImages(newUploadedImages); + setUploadedImages((prev) => [...prev, res]);Likely invalid or redundant comment.
src/features/user/BulkCodes/index.tsx (1)
75-78: Use object parameter pattern consistently.The refactor to pass
tenantandurlas a single object is clear. Ensure you handle the possibility thattenantConfig?.idcould beundefinedby verifying or providing a fallback to avoid runtime errors.src/features/user/ManagePayouts/screens/EditBankAccount.tsx (1)
44-50: Single object parameter usage looks good.Refactoring parameters into a single object improves maintainability and type safety. Confirm that
accountToEdit?.idis always available and consider a fallback to handle any undefined scenario gracefully.pages/sites/[slug]/[locale]/projects-archive/[p].tsx (1)
96-104: Consider verifyingtenantandlocalebefore using them.
IftenantConfig.id,currency, orlocaleis undefined, this request might fail. Adding a guard check or fallback could improve reliability.src/features/user/Account/EditModal.tsx (1)
129-135: Well-structured object parameters.
Converting to a single object improves readability and reduces parameter order errors. This is a good refactor.src/features/user/CompleteSignup/index.tsx (1)
179-183: Validatetenantbefore the request.
A null or undefinedtenantConfig?.idmay cause a 404 or similar error. Consider adding a guard to handle such cases.src/features/user/ManageProjects/components/DetailedAnalysis.tsx (1)
370-376: Refactor looks consistent.
Adopting the object-based API call pattern aligns well with the broader refactor. The typed data object approach simplifies maintenance.src/utils/apiRequests/api.ts (3)
11-14: LGTM! Well-structured base interface.The
BaseRequestOptionsinterface provides a clean foundation for other request interfaces.
Line range hint
224-255: LGTM! Clean implementation of postRequest.The function implementation is concise and follows the established pattern for error handling and response processing.
Line range hint
1-350: Verify function call updates across the codebase.Let's ensure all calls to these functions have been updated to use the new parameter structure.
✅ Verification successful
All API function calls are properly using object parameters
Based on the search results, all calls to the authenticated request functions (
getAuthenticatedRequest,postAuthenticatedRequest,putAuthenticatedRequest, anddeleteAuthenticatedRequest) across the codebase are correctly using object parameters with the expected properties. Here's what was found:
- No instances of direct function calls without object parameters were found
- All function calls use the correct parameter structure with properties like:
tenanturltokenlogoutUserdata(for POST/PUT)🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for direct function calls that might not be using object parameters rg -l "getAuthenticatedRequest\([^{]" || \ rg -l "postAuthenticatedRequest\([^{]" || \ rg -l "putAuthenticatedRequest\([^{]" || \ rg -l "deleteAuthenticatedRequest\([^{]" # Search for potential type errors in function calls ast-grep --pattern 'getAuthenticatedRequest($_)' ast-grep --pattern 'postAuthenticatedRequest($_)' ast-grep --pattern 'putAuthenticatedRequest($_)' ast-grep --pattern 'deleteAuthenticatedRequest($_)'Length of output: 9114
🧰 Tools
🪛 GitHub Check: CodeFactor
[warning] 45-45: src/utils/apiRequests/api.ts#L45
Unexpected any. Specify a different type. (@typescript-eslint/no-explicit-any)
[warning] 35-35: src/utils/apiRequests/api.ts#L35
Unexpected any. Specify a different type. (@typescript-eslint/no-explicit-any)
[warning] 44-44: src/utils/apiRequests/api.ts#L44
Unexpected any. Specify a different type. (@typescript-eslint/no-explicit-any)
[warning] 57-57: src/utils/apiRequests/api.ts#L57
Unexpected any. Specify a different type. (@typescript-eslint/no-explicit-any)src/features/user/Settings/EditProfile/AddressManagement/UnsetBillingAddress.tsx (1)
46-52: Refactor from positional arguments to an object parameter looks good.This change significantly improves readability by clearly labeling each parameter. Consider verifying the existence of
selectedAddressForAction.idbefore making the request to prevent potential 404 errors if the address is missing.✅ Verification successful
Let me gather more information about how
selectedAddressForActionis managed.
Let me check how the address action is triggered and managed.
The null check for
selectedAddressForActionis already implemented correctlyFrom the code analysis, I can see that:
- The parent component (
index.tsx) already has a null check:if (!selectedAddressForAction) return <></>before rendering theUnsetBillingAddresscomponent- The
selectedAddressForActionstate is properly managed through the address management flow, where it's set when an address action is triggered via theAddressActionsMenu- The address ID will always be available when making the request since:
- The component is only rendered when
selectedAddressForActionexists- The ID is accessed from the selected address object that was validated to exist
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # This script looks for calls to `UnsetBillingAddress` or references to `selectedAddressForAction` usage. # Objective: Confirm that we always pass a valid address ID when calling `unsetAddress`. rg -A 8 'UnsetBillingAddress'Length of output: 3120
Script:
#!/bin/bash # Look for where selectedAddressForAction is set/managed rg -A 5 "selectedAddressForAction.*="Length of output: 3138
Script:
#!/bin/bash # Look for where address actions are set and how they're handled rg -B 5 -A 10 "setAddressAction.*="Length of output: 16332
src/features/user/Settings/EditProfile/AddressManagement/EditAddress.tsx (1)
57-63: Positional-to-object refactor improves maintainability.The new structure makes it clear which parameters are being passed and allows for easy addition or removal of parameters in the future.
src/features/user/Settings/EditProfile/AddressManagement/UpdateAddressType.tsx (1)
48-54: Consistent object parameter usage facilitates clearer code.This change aligns well with the refactoring strategy across the codebase. Keep up the consistency for better maintainability and readability.
src/features/user/Settings/EditProfile/AddressManagement/AddAddress.tsx (1)
66-72: Well-structured object parameter passing.This approach simplifies the function call, reduces errors related to parameter ordering, and helps enforce stronger typing in TypeScript. Good job on the refactor!
pages/sites/[slug]/[locale]/claim/[type]/[code].tsx (1)
86-92: Excellent transition to an object-based request.Converting from multiple positional parameters to an object is consistent with the stated PR objectives. This change improves readability and reduces the potential for confusion in parameter ordering.
- Double-check that TypeScript interfaces or types capture all fields (tenant, url, data, token, logoutUser, etc.) to ensure strong type checking.
- Consider renaming the
tenantfield to something more descriptive, such astenantId, if that clarifies intent in the calling code elsewhere.src/features/user/PlanetCash/components/CreateAccountForm.tsx (1)
46-52: Refactor aligns with object-based approach; ensure optional ID is managed properlyGreat job refactoring the parameters into an object—this enhances maintainability and readability. However, if
tenantConfig?.idis critical, consider a fallback or validation logic in casetenantConfigis null or undefined. This prevents possible request failures if the tenant ID is missing or invalid.You may run the following script to confirm that all other calls to
postAuthenticatedRequestpass a validtenant:✅ Verification successful
Let me check the implementation of
postAuthenticatedRequestto understand how it handles optional tenant IDs.
Let me try a different pattern to find the implementation.
Let me try one last search to find the implementation.
Let me check the type definition for PostAuthRequestOptions to understand how the tenant parameter is handled.
Let me try one last search to find the interface or type definition.
Based on the search results from examining the codebase, I can provide a final response:
Optional tenant ID handling appears consistent across the codebase
From the search results, I found that all calls to
postAuthenticatedRequestconsistently handle the optional tenant ID in the same way using the optional chaining operator (?.). This pattern is used across multiple components including:
src/features/user/TreeMapper/Import/components/SampleTrees.tsxsrc/features/user/RegisterTrees/RegisterTreesWidget.tsxsrc/features/user/TreeMapper/Import/components/PlantingLocation.tsx- And others
The current implementation in
CreateAccountForm.tsxfollows this established pattern, suggesting that the API and application are designed to handle cases where the tenant ID might be undefined.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify other calls to postAuthenticatedRequest are updated to the new object-based signature and pass a valid tenant rg -A 5 'postAuthenticatedRequest\s*\(\s*\{'Length of output: 1667
Script:
#!/bin/bash # Find the implementation of postAuthenticatedRequest ast-grep --pattern 'postAuthenticatedRequest = async ({ $$ }) => { $$ }'Length of output: 74
Script:
#!/bin/bash # Find the implementation of postAuthenticatedRequest using ripgrep rg -A 10 "export const postAuthenticatedRequest"Length of output: 50
Script:
#!/bin/bash # Search for postAuthenticatedRequest function definition rg -g '!*.test.*' -g '!*.spec.*' "postAuthenticatedRequest" -B 2 -A 10Length of output: 48724
Script:
#!/bin/bash # Find the type definition for PostAuthRequestOptions rg -g '*.ts' -g '*.tsx' "type PostAuthRequestOptions" -A 10Length of output: 61
Script:
#!/bin/bash # Search for interface or type definition containing tenant and url rg -g '*.ts' -g '*.tsx' "interface.*\{[\s\S]*tenant.*\}" -A 10Length of output: 64
src/tenants/salesforce/OceanforceCampaign/components/ContentSection.tsx (1)
30-36: Good adoption of the new object-based parameter structure!This refactor improves readability and maintainability by bundling arguments into a single object. Consider adding or verifying TypeScript interfaces to further solidify type checking—especially for
tenant,url, andqueryParams. If not already done, ensure similar updates are correctly applied in other parts of the application referencinggetRequest.✅ Verification successful
Type definitions for getRequest are properly implemented and consistently used
Based on the verification results, I can confirm that:
- The
getRequestfunction is properly typed with a generic type parameterTand accepts aGetRequestOptionsparameter structure insrc/utils/apiRequests/api.ts- The object-based parameter structure is consistently used across the codebase, as seen in multiple components including the reviewed file and other similar implementations (VTOCampaign, Mangroves, etc.)
- All instances found follow the same pattern with properly structured parameters for
tenant,url, andqueryParamsThe implementation in ContentSection.tsx follows the established pattern correctly and maintains type safety through the existing type system.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the type definitions for the refactored getRequest calls across the entire codebase. ast-grep --pattern $'getRequest($_)' --lang ts,tsxLength of output: 164
Script:
#!/bin/bash # Search for getRequest usage patterns in TypeScript files ast-grep --pattern 'getRequest($_)' --lang typescript # Also search in TSX files separately ast-grep --pattern 'getRequest($_)' --lang tsx # Let's also search for type definitions related to getRequest rg "getRequest" -A 5 -B 5 --type typescriptLength of output: 2151
Script:
#!/bin/bash # Search for type definitions and interfaces related to getRequest rg "interface.*getRequest|type.*getRequest" -A 5 # Search for getRequest implementation to understand its type structure rg "export.*getRequest" -A 10 # Search for all TypeScript/TSX files that might contain getRequest usage fd "\.(ts|tsx)$" --exec rg "getRequest" -lLength of output: 2541
src/features/projectsV2/ProjectDetails/index.tsx (2)
63-71: Object parameter structure usage looks clean and maintainable.By transitioning from multiple positional parameters to a single object, this implementation enhances readability and reduces coupling between the parameters. The use of
tenantConfig.idand query parameters (_scope,currency,locale) appears consistent with the refactor's objectives. Just ensure that any optional parameters have corresponding default or fallback values where appropriate.
98-105: Version parameter consistency check.Including the explicit
version: '1.0.4'within the new object parameter aligns with the improved structure; however, verify that the endpoint or server side is prepared to handle this version parameter. If multiple version values are supported, consider centralizing them in a config to avoid duplication.src/features/user/TreeMapper/MySpecies/SpeciesAutoComplete.tsx (1)
62-69: Handle undefined tenantConfig safelyWhile this new object parameter structure is an improvement over the previous version, make sure to handle cases where
tenantConfig?.idmight be undefined. A missing tenant ID could lead to confusing errors or unintended fallbacks. Consider providing a default or adding explicit checks to avoid potential failures.For instance, you could apply a fallback tenant ID if
tenantConfig?.idis unavailable:-const res = await postRequest<SpeciesSuggestionType[]>({ - tenant: tenantConfig?.id, - url: `/suggest.php`, - data: { - q: value, - t: 'species', - }, -}); +const res = await postRequest<SpeciesSuggestionType[]>({ + tenant: tenantConfig?.id || 'defaultTenant', + url: '/suggest.php', + data: { + q: value, + t: 'species', + }, +});If you’d like me to generate a shell script to verify other usage of
tenantConfigor handle undefined scenarios across the codebase, let me know.pages/sites/[slug]/[locale]/profile/redeem/[code].tsx (1)
81-81: Great use of a typed generic.
By specifying<RedeemedCodeData>in yourpostAuthenticatedRequest, you ensure that the response is strongly typed, reducing error-prone casting later.src/tenants/salesforce/VTOCampaign/components/ProjectGrid.tsx (1)
22-30: Consider removing redundant tenant parameter usage or verify need for duplication.You appear to be passing
tenant: tenantConfig.idboth at the top level and withinqueryParams. While it might be intentional (e.g., for separate uses ingetRequestvs. query string), it's worth confirming you actually need tenant in both positions or whether you can remove one for clarity and simplicity.src/tenants/planet/LeaderBoard/components/Score.tsx (1)
33-38: Consider stricter type checks and handling potential missing tenant IDs.While consolidating the parameters greatly improves readability, ensure
tenantConfig?.idisn’t undefined. Consider either asserting or providing a fallback fortenantConfigwhen undefined. Additionally, confirm thatresfrompostRequestis always an array before calling.filter(...)by defining a proper response interface.To verify usage across the codebase, run:
src/features/user/Account/components/DownloadCodes.tsx (1)
41-41: Consider handling undefined tenant configuration.While using an object-based parameter promotes clarity and maintainability, if
tenantConfig?.idis undefined for any reason, this request might fail or behave unexpectedly. You might consider adding a guard or fallback logic to handle cases whentenantConfig?.idis not available.Here's a script that searches for unchecked usage of
tenantConfig?.idthroughout the codebase:src/features/user/ManagePayouts/screens/PayoutScheduleForm.tsx (3)
47-50: Refactored parameter structure aligns well with PR objectives.
This transition to a single object parameter significantly boosts readability and maintainability, matching the goals outlined in the PR summary.
48-48: Verify tenant ID fallback or handling fortenantConfig?.id.
IftenantConfigortenantConfig.idis undefined, the request may fail. Consider providing a fallback or validation to prevent runtime errors.
52-53: Confirm token presence before making API request.
Ensure thattokenis valid when calling the API request; otherwise, it could lead to unauthorized or invalid requests. Similarly, confirm thatlogoutUserbehavior is correct if the token is missing or expired.
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
src/utils/apiRequests/api.ts (1)
Line range hint
52-71: Address TODO comment and improve error handling.The TODO comment indicates missing error handling. Consider implementing consistent error handling similar to other request functions:
const response = await fetch(`${process.env.API_ENDPOINT}/app/profile`, { method: 'GET', headers: setHeaderForImpersonation(header, impersonationData), }); -// TODO: Add error handling after figuring out the nature of getAccountInfo function call with impersonatedEmail +if (!response.ok) { + throw new APIError(response.status, await response.json()); +} + +if (response.status === 204) { + return true; +} -return response; +return response.json();🧰 Tools
🪛 GitHub Check: CodeFactor
[warning] 56-56: src/utils/apiRequests/api.ts#L56
Unexpected any. Specify a different type. (@typescript-eslint/no-explicit-any)
🧹 Nitpick comments (2)
src/utils/apiRequests/api.ts (2)
11-50: Improve interface consistency and type safety.
Consider reordering properties consistently across interfaces:
token(if applicable)tenant- Required properties
- Optional properties
Consider using generics for better type safety in request interfaces:
-interface PostAuthRequestOptions<T> extends BaseRequestOptions { +interface PostAuthRequestOptions<TData = any> extends BaseRequestOptions { token: string | null; - data: T; + data: TData; logoutUser: (value?: string | undefined) => void; headers?: Record<string, string>; }
Line range hint
119-356: Consider extracting common request handling logic.The authenticated request functions share similar patterns for:
- Token validation
- Header construction
- Response processing
- Error handling
Consider creating a utility function to reduce code duplication:
interface BaseAuthenticatedRequestConfig { method: 'GET' | 'POST' | 'PUT' | 'DELETE'; token: string | null; tenant: string | undefined; url: string; logoutUser: (value?: string | undefined) => void; data?: unknown; headers?: Record<string, string>; } async function handleAuthenticatedRequest<T>({ method, token, tenant, url, logoutUser, data, headers = {}, }: BaseAuthenticatedRequestConfig): Promise<T> { if (!token || !validateToken(token)) { logoutUser(); throw new ClientError(INVALID_TOKEN_STATUS_CODE, { error_type: 'token_expired', error_code: 'token_expired', }); } const lang = localStorage.getItem('language') || 'en'; const baseHeaders = { 'Content-Type': 'application/json', 'tenant-key': `${tenant}`, 'X-SESSION-ID': await getsessionId(), Authorization: `Bearer ${token}`, 'x-locale': lang, ...headers, }; const response = await fetch(`${process.env.API_ENDPOINT}${url}`, { method, headers: setHeaderForImpersonation(baseHeaders), ...(data && { body: JSON.stringify(data) }), }); if (!response.ok) { throw new APIError(response.status, await response.json()); } return response.status === 204 ? (true as T) : response.json(); }This would simplify the implementation of all authenticated request functions and make future maintenance easier.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/features/user/ManageProjects/components/ProjectMedia.tsx(6 hunks)src/features/user/ManageProjects/index.tsx(3 hunks)src/utils/apiRequests/api.ts(7 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/features/user/ManageProjects/index.tsx
- src/features/user/ManageProjects/components/ProjectMedia.tsx
🧰 Additional context used
🪛 GitHub Check: CodeFactor
src/utils/apiRequests/api.ts
[warning] 56-56: src/utils/apiRequests/api.ts#L56
Unexpected any. Specify a different type. (@typescript-eslint/no-explicit-any)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
src/utils/apiRequests/api.ts (1)
52-56: 🛠️ Refactor suggestionReplace
anytype with a proper interface.Consider creating a dedicated interface for the profile response:
interface ProfileResponse { // Add relevant profile properties here } export async function getAccountInfo({ tenant, token, impersonationData, }: GetAccountInfoOptions): Promise<ProfileResponse>✅ Verification successful
Use
Promise<Response>instead ofanytypeThe function returns a fetch Response object, which is then handled by consumers with status checks and JSON parsing. Replace the return type as follows:
export async function getAccountInfo({ tenant, token, impersonationData, }: GetAccountInfoOptions): Promise<Response>🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash # Find usages of getAccountInfo to understand how the response is used rg "getAccountInfo\(" -A 2 # Look for the implementation of getAccountInfo ast-grep --pattern 'export async function getAccountInfo($_) { $$$ }' # Search for existing interfaces related to account/profile rg "interface.*(?:Account|Profile)" --type typescriptLength of output: 936
Script:
#!/bin/bash # Look for interfaces with correct file type rg "interface.*(?:Account|Profile)" # See the implementation of getAccountInfo rg -A 15 "export async function getAccountInfo" src/utils/apiRequests/api.ts # See how the response is used in UserPropsContext rg -A 10 "const resJson = await res.json\(\)" src/features/common/Layout/UserPropsContext.tsxLength of output: 2058
Script:
#!/bin/bash # Look for User type/interface definition rg "type User =|interface User" -A 5 # Check content of profile.d.ts cat src/features/common/types/profile.d.tsLength of output: 2657
🧰 Tools
🪛 GitHub Check: CodeFactor
[warning] 56-56: src/utils/apiRequests/api.ts#L56
Unexpected any. Specify a different type. (@typescript-eslint/no-explicit-any)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/utils/apiRequests/api.ts (2)
16-50: Standardize property ordering across interfacesFor better readability and maintainability, consider maintaining a consistent order of properties across all interfaces:
token(authentication)tenant(environment)- Core properties (
url,data)- Optional properties (
headers,queryParams, etc.)- Callback functions (
logoutUser)🧰 Tools
🪛 GitHub Check: CodeFactor
[warning] 45-45: src/utils/apiRequests/api.ts#L45
Unexpected any. Specify a different type. (@typescript-eslint/no-explicit-any)
[warning] 41-41: src/utils/apiRequests/api.ts#L41
Unexpected any. Specify a different type. (@typescript-eslint/no-explicit-any)
[warning] 31-31: src/utils/apiRequests/api.ts#L31
Unexpected any. Specify a different type. (@typescript-eslint/no-explicit-any)
Line range hint
52-74: Address TODO comment regarding error handlingThe function lacks proper error handling for impersonation scenarios.
Would you like me to help implement error handling for the impersonation case? I can propose a solution that aligns with the error handling patterns used in other authenticated requests.
🧰 Tools
🪛 GitHub Check: CodeFactor
[warning] 56-56: src/utils/apiRequests/api.ts#L56
Unexpected any. Specify a different type. (@typescript-eslint/no-explicit-any)
[warning] 45-45: src/utils/apiRequests/api.ts#L45
Unexpected any. Specify a different type. (@typescript-eslint/no-explicit-any)
[warning] 41-41: src/utils/apiRequests/api.ts#L41
Unexpected any. Specify a different type. (@typescript-eslint/no-explicit-any)
[warning] 31-31: src/utils/apiRequests/api.ts#L31
Unexpected any. Specify a different type. (@typescript-eslint/no-explicit-any)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/utils/apiRequests/api.ts(7 hunks)
🧰 Additional context used
🪛 GitHub Check: CodeFactor
src/utils/apiRequests/api.ts
[warning] 56-56: src/utils/apiRequests/api.ts#L56
Unexpected any. Specify a different type. (@typescript-eslint/no-explicit-any)
[warning] 45-45: src/utils/apiRequests/api.ts#L45
Unexpected any. Specify a different type. (@typescript-eslint/no-explicit-any)
[warning] 41-41: src/utils/apiRequests/api.ts#L41
Unexpected any. Specify a different type. (@typescript-eslint/no-explicit-any)
[warning] 31-31: src/utils/apiRequests/api.ts#L31
Unexpected any. Specify a different type. (@typescript-eslint/no-explicit-any)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (3)
src/utils/apiRequests/api.ts (3)
11-14: Well-structured base interface!The
BaseRequestOptionsinterface provides a solid foundation for other request interfaces with essential common properties.
Line range hint
119-173: Well-implemented error handling and token validation!The authenticated request functions maintain a consistent and robust pattern for:
- Token validation
- Error handling
- Response status handling
- Impersonation header setting
Also applies to: 174-223, 256-301, 302-351
Line range hint
119-351: Verify all call sites are updated to use the new parameter structureSince this is a significant refactor of the API utility functions, we should ensure all call sites are updated to use the new object parameter structure.
Run this script to find any potential missed call sites:
✅ Verification successful
All call sites are properly using the new parameter structure
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Find direct function calls that might not be updated to use the new parameter structure # Search for old-style function calls (multiple parameters) rg -l "getAuthenticatedRequest\([^{]" || \ rg -l "postAuthenticatedRequest\([^{]" || \ rg -l "putAuthenticatedRequest\([^{]" || \ rg -l "deleteAuthenticatedRequest\([^{]" || \ rg -l "postRequest\([^{]" || \ rg -l "getRequest\([^{]"Length of output: 218
Script:
#!/bin/bash # Search for actual function call patterns to verify parameter structure ast-grep --pattern 'getAuthenticatedRequest({$$$})' ast-grep --pattern 'postAuthenticatedRequest({$$$})' ast-grep --pattern 'putAuthenticatedRequest({$$$})' ast-grep --pattern 'deleteAuthenticatedRequest({$$$})' ast-grep --pattern 'postRequest({$$$})' # Also search with ripgrep for any usage patterns rg -A 2 "(get|post|put|delete)AuthenticatedRequest|postRequest" --type tsLength of output: 56594
| } | ||
| interface PutAuthRequestOptions extends BaseRequestOptions { | ||
| token: string | null; | ||
| data?: any; |
There was a problem hiding this comment.
Isn't data mandatory for a PUT request?
There was a problem hiding this comment.
Ignore this, we have 2 instances where data is not needed.
e.g.
const res = await putAuthenticatedRequest<ApiKeyResponse>({
tenant: tenantConfig?.id,
url: '/app/profile/apiKey',
token,
logoutUser,
});
```
TASK: Refactor
getAuthenticatedRequestParameter StructureProblem:
Solution:
Benefits: