Feature/use api hook3 #2477
Feature/use api hook3 #2477
Conversation
-remove unnecessary useCallback hook
…n ProjectCertificates
…n ProjectSpending
- Removed unused `token` prop from `BasicDetails` and `ProjectSites` components
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis pull request refactors API interactions across multiple parts of the application. The changes replace several legacy authenticated request functions with a centralized Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (2)
src/features/user/ManageProjects/index.tsx (2)
285-291: 🛠️ Refactor suggestionInconsistent prop passing to child components
The
tokenprop is still being passed to child components (ProjectMedia,DetailedAnalysis, andProjectSpending), but the parent component now uses theuseApihook for API calls. This suggests that these child components might not have been completely refactored yet.Consider refactoring these child components to also use the
useApihook instead of directly passing the token prop. This would make the API call pattern consistent throughout the application and complete the refactoring objective.Also applies to: 296-306, 320-325
159-161: 💡 Verification agent🧩 Analysis chain
Verify token usage consistency
The component still relies on the
tokenprop for conditional logic, but the refactored API calls don't explicitly pass it. This suggests that theuseApihook internally manages authentication.
🏁 Script executed:
#!/bin/bash # Check if child components have been similarly refactored fd "ProjectMedia.tsx|DetailedAnalysis.tsx|ProjectSpending.tsx" --type f | xargs grep -l "useApi" # Check if token prop is still used in these components fd "ProjectMedia.tsx|DetailedAnalysis.tsx|ProjectSpending.tsx" --type f | xargs grep -A 5 "token="Length of output: 914
Action Required: Revisit Token Propagation Consistency with
useApi
- In
src/features/user/ManageProjects/index.tsx, the code conditionally callsfetchProjectDetails()usingif (projectGUID && token), yet the refactored API calls do not explicitly pass thetokenbecause theuseApihook appears to manage authentication internally.- Verification shows that child components—such as
DetailedAnalysis.tsx—still receive thetokenprop (e.g.,token={token}), indicating an inconsistent token usage pattern.- Please confirm whether the token prop is still necessary. If the
useApihook fully handles authentication, consider updating the conditional logic and removing redundant token propagation to streamline the codebase.
🧹 Nitpick comments (1)
src/features/user/ManagePayouts/index.tsx (1)
48-57: Consider using optional chaining operator for safer property access.The refactored API calls look good, but there's an opportunity to improve the code by using optional chaining.
if (setProgress) { - setProgress(70); + setProgress?.(70); }Similarly on lines 74-76:
- if (setProgress) { - setProgress(100); - setTimeout(() => setProgress(0), 1000); - } + setProgress?.(100); + setTimeout(() => setProgress?.(0), 1000);Also applies to: 63-79
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
pages/sites/[slug]/[locale]/profile/projects/[id].tsx(3 hunks)src/features/common/types/project.d.ts(0 hunks)src/features/user/ManagePayouts/index.tsx(4 hunks)src/features/user/ManagePayouts/screens/AddBankAccount.tsx(1 hunks)src/features/user/ManagePayouts/screens/EditBankAccount.tsx(2 hunks)src/features/user/ManagePayouts/screens/PayoutScheduleForm.tsx(3 hunks)src/features/user/ManageProjects/ProjectsContainer.tsx(2 hunks)src/features/user/ManageProjects/components/BasicDetails.tsx(5 hunks)src/features/user/ManageProjects/components/DetailedAnalysis.tsx(5 hunks)src/features/user/ManageProjects/components/ProjectCertificates.tsx(7 hunks)src/features/user/ManageProjects/components/ProjectMedia.tsx(8 hunks)src/features/user/ManageProjects/components/ProjectSites.tsx(10 hunks)src/features/user/ManageProjects/components/ProjectSpending.tsx(5 hunks)src/features/user/ManageProjects/index.tsx(5 hunks)
💤 Files with no reviewable changes (1)
- src/features/common/types/project.d.ts
🧰 Additional context used
🧬 Code Definitions (2)
pages/sites/[slug]/[locale]/profile/projects/[id].tsx (1)
src/features/common/types/project.d.ts (2)
ProfileProjectTrees(311-358)ProfileProjectConservation(277-309)
src/features/user/ManageProjects/index.tsx (1)
src/features/common/types/project.d.ts (3)
ManageProjectsProps(121-125)ProfileProjectTrees(311-358)ProfileProjectConservation(277-309)
🪛 Biome (1.9.4)
src/features/user/ManagePayouts/index.tsx
[error] 66-66: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: ESLint on Ubuntu
- GitHub Check: Analyze (javascript)
🔇 Additional comments (78)
src/features/user/ManagePayouts/screens/AddBankAccount.tsx (4)
16-16: Import for useApi hook addedThe introduction of the useApi hook is a positive change that aligns with the PR objective of refactoring API calls throughout the app.
21-21: Using postApiAuthenticated from the useApi hookThis change effectively replaces the previous direct usage of postAuthenticatedRequest while maintaining the same functionality.
32-37: Formation of accountData is unchangedThe refactoring preserves the existing accountData structure, ensuring backward compatibility.
39-44: API call refactored to use useApi hookThe API call has been simplified by using postApiAuthenticated, removing the need for explicit tenant configuration and token management. The type safety is maintained through the generic types.
src/features/user/ManageProjects/ProjectsContainer.tsx (4)
21-21: Import for useApi hook addedThe introduction of the useApi hook aligns with the PR objective of centralizing API calls.
108-108: Using getApiAuthenticated from the useApi hookThis change effectively replaces the getAuthenticatedRequest function while removing the dependency on tenant configuration.
116-119: API call refactored to use useApi hookThe API call has been simplified by using getApiAuthenticated, maintaining the same functionality with cleaner code. The queryParams are now passed directly as part of the options object.
112-112: Note token usage in useEffectThe token is still being used in the useEffect dependency array (line 133), which is correct as it's needed to determine when to load projects. This demonstrates careful refactoring that preserves necessary dependencies.
pages/sites/[slug]/[locale]/profile/projects/[id].tsx (4)
36-36: Import for useApi hook addedThe addition of the useApi hook import maintains consistency with the refactoring pattern across the application.
50-50: Using getApiAuthenticated from the useApi hookThis change effectively replaces the previous direct usage of getAuthenticatedRequest.
75-77: Simplified API call using useApi hookThe API call has been significantly simplified by removing the need for explicit tenant, token, and logoutUser parameters. The URL is now directly passed to getApiAuthenticated, maintaining type safety through generic types.
56-57: User context variables retainedThe component still properly uses the user and contextLoaded variables from useUserProps, which are essential for determining when to load the project data.
src/features/user/ManagePayouts/screens/PayoutScheduleForm.tsx (3)
16-16: Import for useApi hook addedThe useApi hook import is consistent with the refactoring pattern observed in other files.
34-35: Simplified useUserProps and added useApi hookThe component no longer directly uses token and logoutUser from useUserProps, as these are now handled internally by the useApi hook. This reduces the component's responsibilities and simplifies the code.
49-51: API call refactored to use useApi hookThe API call has been simplified to use putApiAuthenticated from the useApi hook. The payload is now passed directly as an object property, making the code more readable and maintainable.
src/features/user/ManagePayouts/screens/EditBankAccount.tsx (1)
19-19: Good API call refactoring using theuseApihook.The refactoring successfully replaces the legacy
putAuthenticatedRequestwith the centralizeduseApihook'sputApiAuthenticatedmethod, which simplifies parameter handling by removing the need for explicit tenant and token management. This change aligns well with the PR objective of standardizing API interactions.Also applies to: 27-27, 45-48
src/features/user/ManagePayouts/index.tsx (1)
2-2: Clean import refactoring.The import changes correctly implement the PR's goal of centralizing API requests through the
useApihook. The removal ofuseCallbackdependency is also handled properly.Also applies to: 18-18, 44-44
src/features/user/ManageProjects/components/ProjectSpending.tsx (3)
30-30: Good implementation of type definition for API payload.The addition of the
ProjectExpensePayloadtype enhances type safety and code clarity. TheuseApihook integration is well-implemented.Also applies to: 54-58, 76-78
91-103: Clean API call refactoring.The refactored implementation correctly uses the new payload type and the
useApihook'spostApiAuthenticatedmethod, maintaining the same functionality while providing better type safety.
154-156: Simplified API call parameters.The refactored API calls for
deleteApiAuthenticatedandgetApiAuthenticatedare correctly implemented with a cleaner interface that removes redundant tenant and token management.Also applies to: 170-173
src/features/user/ManageProjects/components/ProjectCertificates.tsx (3)
26-26: Good implementation of type definition for certificate submission.The addition of the
CertificateSubmissionPayloadtype enhances type safety. TheuseApihook integration is properly implemented with the required methods.Also applies to: 45-49, 59-60
81-91: Clean API call refactoring for certificate operations.The API calls for submitting, retrieving, and deleting certificates have been successfully refactored to use the
useApihook's methods, providing a more consistent and maintainable approach to API interactions.Also applies to: 131-134, 170-172
233-236: Good addition ofe.preventDefault()in delete handler.Adding
e.preventDefault()to the delete button click handler is a good practice to prevent any unintended form submissions or navigation that might occur when clicking the button.src/features/user/ManageProjects/components/ProjectSites.tsx (12)
11-15: Leverage GeoJSON types for clarity and safety
Defining these imports helps clarify geographic data structures. Good move for ensuring type safety.
43-43: Refactor to centralized API hook
Adopting a single hook to manage requests aligns with the PR objective, reducing complexity and improving consistency.
55-59: Introducing dedicated update payload
Defining a typed payload for site updates enforces a clear contract between client and server.
61-61: Streamlining creation logic
Using the same structure for creation and updates (alias) eliminates duplication and simplifies maintenance.
77-77: Hook-based PUT method
Switching toputApiAuthenticatedfrom the hook ensures consistent request configuration under the new API approach.
104-104: Payload ensures strong typing
TheUpdateSitePayloadusage here ensures correct data shape is sent to the server.
111-116: Consistent error & loading handling
Usingtry/catchplussetIsUploadingDatafosters a robust user experience and error handling path. Nicely done.
269-270: Expanded usage of the API hook
Extracting multiple methods (deleteApiAuthenticated,postApiAuthenticated,getApiAuthenticated) reduces repeated boilerplate.
343-348: Synchronous approach for sites listing
Good usage ofgetApiAuthenticated. Error handling withsetErrorsand redirect is consistent with the rest of the code.
374-378: Leveraging shared payload
CreateSitePayloadunifies creation logic, improving consistency with the update path.
381-386: POST request with typed responses
postApiAuthenticated<Site, CreateSitePayload>ensures reliability in data shaping and consistent error handling.
415-415: Clean removal of project site
ThedeleteApiAuthenticatedcall neatly aligns with the new hook-based pattern.src/features/user/ManageProjects/components/DetailedAnalysis.tsx (11)
33-33: Consistent API usage
ImportinguseApihere standardizes request handling in line with the refactoring.
96-99: Base metadata for reusability
Defining a shared base fosters common fields across project types, reducing duplication.
101-110: Extend base for tree metadata
CombiningBaseMetadatawith tree-specific fields clarifies domain processes and simplifies the code.
112-119: Extend base for conservation metadata
Similarly leveragingBaseMetadatasuits conservation flows while maintaining a consistent structure.
121-123: Type for tree submission
Creating a dedicated shape clarifies the data contract for tree projects.
125-127: Type for conservation submission
Splitting the data shape keeps the code explicit and robust.
129-129: Unified submission type
A union of both specialized types ensures flexible usage with minimal code overhead.
144-144: Adopting the new hook
PullingputApiAuthenticatedfromuseApistandardizes request patterns across the codebase.
350-359: Constructing base metadata
Building acommonFieldobject ensures shared attributes are properly enforced and easy to maintain.
361-361: Switching between tree vs. conservation data
UsingSubmitProjectDataunion type simplifies branching logic when building final payload.
401-404: PUT request with typed payload
Utilizing the new approach ensures consistent error handling, and typed requests safeguard correctness.src/features/user/ManageProjects/components/ProjectMedia.tsx (16)
27-27: Centralizing API calls
ImportinguseApihelps unify request logic across the module. Good step.
29-33: Adding typed payload for images
imageFile,description, andisDefaultfields explicitly define the upload contract.
35-37: Video data shape
Declaring aProjectVideoUpdatePayloadclarifies the expected field for updating video URLs.
39-41: Default image payload
Another well-defined structure for toggling default images.
43-48: Consistent shape for default images
TheSetDefaultImageinterface complements the toggle logic for default images in the UI.
50-52: Caption data
Explicitly typed caption payload ensures modifications adhere to the expected format.
64-69: Batch import of API methods
Deconstructing all methods fromuseApistandardizes how the code handles get, delete, post, and put calls.
87-89: Optimize fetch flow
getApiAuthenticatedcall includes integrated error handling, aligning with your new approach.
105-109: Preparing typed payload on upload
UploadImagePayloadenforces clarity: minimal adjustments needed for expansions.
112-117: POST request for uploading images
postApiAuthenticatedusage ensures code readability and consistency with other modules.
178-182: Explicit video URL submission
TheProjectVideoUpdatePayloadusage ensures correct data structure for the request.
185-188: PUT request to update video
This refactor centralizes logic and handles errors effectively.
200-203: Setting default image payload
Marking a default image withisDefault: trueis straightforward and typed.
206-211: PUT request for setting default image
Consistent approach for toggling boolean fields keeps the code cohesive.
231-234: Caption update flow
Capturing user-provided captions at blur event adheres to typical UX patterns.
237-242: PUT request for updating captions
By referencingUploadCaptionPayload, you ensure the server receives the correct data.src/features/user/ManageProjects/components/BasicDetails.tsx (10)
39-39: Centralized API import
ImportinguseApiprimes this component for unified request handling.
64-80: Defining shared base
BaseProjectDatacaptures consistently required fields, reducing duplication across project types.
82-86: Tree project specialization
Enriching the base with tree-specific fields (likeclassification) segments the logic cleanly.
88-90: Conservation specialization
Markingpurpose: 'conservation'helps differentiate logic in the union type.
92-92: Unified shape for submission
Combining variants intoSubmitProjectDatayields a flexible approach for different project types.
115-115: Importing put/post from the hook
Keeps requests consistent; removing old request logic tidies up the code.
332-350: Extracting common fields
Clearly separatingcommonFieldsfosters better maintainability and a simpler mental model for shared data.
351-353: Defining final payload
Branching logic fortreesvs.conservationneatly extends the base fields.
366-371: PUT approach for existing projects
Proper usage ofputApiAuthenticatedensures existing records are updated with robust error handling.
381-384: POST logic for new projects
Creating a project viapostApiAuthenticatedis aligned with the rest of the refactoring.src/features/user/ManageProjects/index.tsx (6)
23-23: Good addition of the useApi hookThe introduction of the
useApihook centralizes API interaction logic and reduces the need for multiple import statements. This aligns with the PR objective of refactoring API calls.
35-41: Well-defined payload types improve type safetyAdding explicit types for API payloads (
ReviewRequestPayloadandPublishStatusPayload) enhances type safety and code clarity. This makes the code more maintainable and helps prevent potential bugs when working with API requests.
51-51: Appropriate usage of the useApi hookThe destructuring of
putApiAuthenticatedandgetApiAuthenticatedfrom theuseApihook provides a cleaner approach to handle authenticated API calls, removing the need for separate tenant and token parameters in each request.
110-115: Improved API call structure in submitForReviewThe refactored API call in
submitForReviewnow uses proper generic types for both request and response, and simplifies the parameters by using a payload object. This is more concise and type-safe than the previous implementation.
131-136: Consistent API call refactoring in handlePublishChangeThe refactoring of
handlePublishChangefollows the same pattern assubmitForReview, ensuring consistency across the component. Good use of generic types to ensure type safety for both request and response.
149-151:✅ Verification successful
Simplified GET request in fetchProjectDetails
The implementation of
getApiAuthenticatedinfetchProjectDetailssimplifies the API call by removing the need for tenant and token parameters. However, there's a potential issue to consider.The code still checks for
tokenat line 159 before callingfetchProjectDetails(), but the new API call doesn't explicitly use it. Let's verify that theuseApihook properly handles authentication:
🏁 Script executed:
#!/bin/bash # Verify that useApi hook handles authentication internally grep -A 20 "export const useApi" "src/hooks/useApi.ts"Length of output: 593
Authentication Handling and Simplified GET Request in fetchProjectDetails
The refactored
getApiAuthenticatedcall infetchProjectDetailsnow omits explicit tenant and token parameters. Although a token check is still present at line 159 before invokingfetchProjectDetails(), our verification of theuseApihook confirms it internally handles authentication (by retrieving the token viauseUserProps). This internal handling ensures that token management remains secure and consistent, so the pre-check merely adds an extra layer of safety.
There was a problem hiding this comment.
There are a number of warnings here, suggesting that the intervention options may need updating. This can be done separately
const [interventionOptions, setInterventionOptions] = React.useState<
InterventionOption[]
>([
['assisting-seed-rain', false],
['control-remove-livestock', false],
['cut-suppressing-grass', false],
['direct-seeding', false],
['enrichment-planting', false],
['establish-firebreaks', false],
['fire-patrols', false],
['fire-suppression-team', false],
['liberating-regenerants', false],
['maintenance', false],
['marking-regenerants', false],
['other-interventions', false],
['planting-trees', false],
['removal-contaminated-soil', false],
['removal-invasive-species', false],
['soil-improvement', false],
['stop-tree-harvesting', false],
]);
```
There was a problem hiding this comment.
Will address in a separate PR
src/features/user/ManageProjects/components/ProjectCertificates.tsx
Outdated
Show resolved
Hide resolved
- Standardize API payload type naming with ApiPayload suffix - Create clear type hierarchies using base types and extensions - Use more descriptive variable names for API payloads - Consolidate duplicate types where appropriate These changes improve code readability, maintainability, and type safety throughout the project creation flow without changing functionality.
…outs - avoids confusion with generic FormData type
There was a problem hiding this comment.
@sunilsabatp I've made some updates to the types for consistency. Otherwise this looks good to go.
Before merging, please go through the comments.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/features/user/ManagePayouts/components/BankDetailsForm.tsx (1)
15-28: Consider using more specific types for numeric values.The
payoutMinAmountis defined as an optional string, but it represents a numeric value. Consider using a more appropriate type likenumber | undefinedto avoid type conversion issues later in the code.export type AccountFormData = { currency: string; - payoutMinAmount?: string; + payoutMinAmount?: number; bankName: string; bankCountry: string; bankAddress: string; holderName: string; holderAddress: string; accountNumber: string; routingNumber: string; bic: string; branchCode: string; remarks: string; };If you make this change, you'll need to update the related form handling code and validation logic as well.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/features/user/ManagePayouts/components/BankDetailsForm.tsx(4 hunks)src/features/user/ManagePayouts/screens/AddBankAccount.tsx(1 hunks)src/features/user/ManagePayouts/screens/EditBankAccount.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/features/user/ManagePayouts/screens/AddBankAccount.tsx
- src/features/user/ManagePayouts/screens/EditBankAccount.tsx
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: ESLint on Ubuntu
- GitHub Check: Analyze (javascript)
🔇 Additional comments (5)
src/features/user/ManagePayouts/components/BankDetailsForm.tsx (5)
15-28: Type renamed fromFormDatatoAccountFormData- good choice.This rename avoids potential conflicts with the built-in browser
FormDatainterface while also making the type name more descriptive of its purpose. The new name clearly indicates that this data structure is used for account form handling.
33-33: LGTM: Props interface update is consistent with type renaming.The props interface has been properly updated to use the new
AccountFormDatatype for thehandleSavefunction parameter.
37-54: LGTM: Helper function return type updated consistently.The
extractFormValuesfunction's return type has been updated to use the newAccountFormDatatype, maintaining consistency throughout the file.🧰 Tools
🪛 GitHub Check: CodeFactor
[notice] 37-54: src/features/user/ManagePayouts/components/BankDetailsForm.tsx#L37-L54
Complex Method
68-71: LGTM: Form hook type updated appropriately.The
useFormhook's generic type parameter has been updated to useAccountFormData, maintaining type consistency throughout the component.
93-95: LGTM: Submission handler type updated consistently.The
onSubmitfunction parameter type has been updated to useAccountFormData, completing the consistent type renaming throughout the component.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/features/user/ManageProjects/components/ProjectCertificates.tsx (3)
125-125: Consider reviewing dependency array in onDrop callbackThe
uploadedFilesis included in the dependency array but doesn't appear to be used within the callback function. This might cause unnecessary re-renders.- }, [uploadedFiles] + }, []
148-151: Review useEffect dependency arrayThe effect depends on both
projectGUIDandtokenin the condition check, but onlyprojectGUIDis included in the dependency array. Consider addingtokento ensure the effect runs correctly when token changes.- }, [projectGUID]); + }, [projectGUID, token]);
117-119: Consider removing console logsThere are several console logs in the component that should be removed or replaced with proper error logging before production deployment.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/features/user/ManageProjects/components/ProjectCertificates.tsx(7 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: ESLint on Ubuntu
🔇 Additional comments (8)
src/features/user/ManageProjects/components/ProjectCertificates.tsx (8)
26-26: Clean API hook import addedGood job adding the useApi hook import which aligns with the PR objective of refactoring API calls.
45-49: Good type definition for certificate payloadAdding the
CertificateApiPayloadtype improves type safety for the API payload structure. This makes the code more maintainable and helps prevent potential runtime errors.
59-60: Clean implementation of useApi hookGood refactoring to use the centralized
useApihook instead of separate authenticated request functions.
81-85: Properly typed certificate payloadGood use of the new type definition for the certificate payload.
88-93: Well-typed API request with proper generic parametersThe API call is now properly typed with both request and response types, which improves type safety.
133-136: Clean implementation of typed API requestGood refactoring of the GET request with proper type parameters and query structure.
172-174: Simplified delete API callThe delete API call has been properly simplified by removing unnecessary parameters.
234-234: Improved button behaviorAdding
type="button"prevents unintended form submissions when clicking the delete button.
Refactored API calls using useApi() hook and defined types for managing project form payloads.
Affected pages: