-
Notifications
You must be signed in to change notification settings - Fork 16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Replace any with Generic Types in POST and PUT Requests #2358
base: develop
Are you sure you want to change the base?
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 comprehensive type safety improvements across multiple components in the project. The changes focus on explicitly defining data structures for various submission processes, such as redeeming codes, managing projects, registering trees, and handling user profiles. By adding specific type definitions and updating function signatures, the code becomes more robust and self-documenting, reducing potential runtime errors and improving type checking. Changes
Suggested labels
Poem
Possibly related PRs
Finishing Touches
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (7)
src/features/user/Settings/EditProfile/AddressManagement/AddAddress.tsx (1)
70-76
: Consider using type inference for bodyToSend.The explicit type annotation on
bodyToSend
is redundant since TypeScript can infer the type from the generic parameter inpostAuthenticatedRequest
.- const bodyToSend: SubmitData = { + const bodyToSend = {src/features/user/ManagePayouts/screens/AddBankAccount.tsx (1)
20-33
: Consider documenting field requirements.The
AccountData
interface is well-structured, but adding JSDoc comments for fields with specific format requirements (likeroutingNumber
,bic
, etc.) would improve maintainability.src/features/user/Profile/ProfileCard/RedeemModal/index.tsx (1)
49-57
: Consider simplifying the type parameters.The multi-line generic type parameters could be simplified for better readability while maintaining type safety.
- const res = await postAuthenticatedRequest< - RedeemedCodeData, - RedeemCodeSubmitData - >({ + const res = await postAuthenticatedRequest<RedeemedCodeData, RedeemCodeSubmitData>({src/features/user/RegisterTrees/RegisterTrees/UploadImages.tsx (1)
26-29
: Consider adding validation constraints.The
SubmitData
type could benefit from JSDoc comments specifying the expected format ofimageFile
(base64, URL, etc.) and any length constraints fordescription
.src/utils/apiRequests/api.ts (1)
Line range hint
36-45
: Consider extending type safety to other interfaces.For consistency, consider updating
PutAuthRequestOptions
andPostRequestOptions
to also use generic types instead ofany
.-interface PutAuthRequestOptions extends BaseRequestOptions { +interface PutAuthRequestOptions<D = unknown> extends BaseRequestOptions { token: string | null; - data?: any; + data?: D; logoutUser: (value?: string | undefined) => void; } -interface PostRequestOptions extends BaseRequestOptions { +interface PostRequestOptions<D = unknown> extends BaseRequestOptions { - data: any; + data: D; }src/features/user/TreeMapper/Import/components/PlantingLocation.tsx (1)
Line range hint
345-353
: Consider adding error handling for date parsing.The date transformation could fail if
data.plantDate
is invalid. Consider adding error handling around the date parsing.- plantDate: new Date(data.plantDate).toISOString(), + plantDate: data.plantDate instanceof Date + ? data.plantDate.toISOString() + : new Date(data.plantDate).toISOString(),src/features/user/ManageProjects/components/BasicDetails.tsx (1)
Line range hint
339-413
: Consider using type narrowing for better type safety.While the type annotations are good, the code could benefit from type narrowing to handle the different project types more safely.
Consider using a type guard:
- const submitData: SubmitData = - purpose === 'trees' + const submitData: SubmitData = + purpose === 'trees' + ? ((): SubmitDataTrees => ({ name: data.name, // ... other fields }))() + : ((): SubmitDataConservation => ({ purpose: 'conservation', // ... other fields }))();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
pages/sites/[slug]/[locale]/claim/[type]/[code].tsx
(2 hunks)pages/sites/[slug]/[locale]/profile/redeem/[code].tsx
(2 hunks)src/features/user/BulkCodes/forms/IssueCodesForm.tsx
(1 hunks)src/features/user/ManagePayouts/screens/AddBankAccount.tsx
(2 hunks)src/features/user/ManageProjects/components/BasicDetails.tsx
(3 hunks)src/features/user/ManageProjects/components/ProjectCertificates.tsx
(2 hunks)src/features/user/ManageProjects/components/ProjectMedia.tsx
(2 hunks)src/features/user/ManageProjects/components/ProjectSites.tsx
(3 hunks)src/features/user/ManageProjects/components/ProjectSpending.tsx
(2 hunks)src/features/user/PlanetCash/components/CreateAccountForm.tsx
(2 hunks)src/features/user/Profile/ProfileCard/RedeemModal/index.tsx
(2 hunks)src/features/user/RegisterTrees/RegisterTrees/UploadImages.tsx
(2 hunks)src/features/user/RegisterTrees/RegisterTreesWidget.tsx
(2 hunks)src/features/user/Settings/EditProfile/AddressManagement/AddAddress.tsx
(2 hunks)src/features/user/TreeMapper/Import/components/PlantingLocation.tsx
(4 hunks)src/utils/apiRequests/api.ts
(2 hunks)
🧰 Additional context used
📓 Learnings (2)
src/features/user/Settings/EditProfile/AddressManagement/AddAddress.tsx (1)
Learnt from: mohitb35
PR: Plant-for-the-Planet-org/planet-webapp#2286
File: src/features/user/Settings/EditProfile/AddressManagement/AddAddress.tsx:18-24
Timestamp: 2024-12-12T06:39:07.424Z
Learning: In 'src/features/user/Settings/EditProfile/AddressManagement/AddAddress.tsx', the `FormData` type intentionally allows `undefined` or `null` values for fields like `address`, `address2`, `city`, `zipCode`, and `state` to accommodate optional input.
src/features/user/Profile/ProfileCard/RedeemModal/index.tsx (1)
Learnt from: mohitb35
PR: Plant-for-the-Planet-org/planet-webapp#2315
File: src/features/common/RedeemCode/SuccessfullyRedeemed.tsx:30-39
Timestamp: 2024-11-25T13:14:59.282Z
Learning: In `src/features/common/RedeemCode/SuccessfullyRedeemed.tsx`, additional null checks for `redeemedCodeData` and its properties are not necessary.
🪛 GitHub Check: CodeFactor
src/features/user/PlanetCash/components/CreateAccountForm.tsx
[notice] 64-64: src/features/user/PlanetCash/components/CreateAccountForm.tsx#L64
'Data' is not defined. (no-undef)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (29)
src/features/user/Settings/EditProfile/AddressManagement/AddAddress.tsx (1)
32-40
: LGTM! Well-structured type definition.The
SubmitData
type properly extends the existingFormData
fields while adding the requiredcountry
andtype
fields. The nullability of the fields aligns with the intended behavior mentioned in the retrieved learnings.src/features/user/ManagePayouts/screens/AddBankAccount.tsx (1)
50-57
: LGTM! Proper type safety implementation.The transformation of form data to
AccountData
and the usage of generic types inpostAuthenticatedRequest
ensures type safety throughout the request chain.src/features/user/Profile/ProfileCard/RedeemModal/index.tsx (1)
4-4
: LGTM! Improved type safety.Good improvement in type safety by:
- Importing the specific
RedeemCodeSubmitData
type- Narrowing the
redeemingCode
parameter type fromstring | undefined
tostring
Also applies to: 47-47
src/features/user/RegisterTrees/RegisterTrees/UploadImages.tsx (1)
Line range hint
44-55
: LGTM! Proper type safety implementation.The implementation correctly uses the
SubmitData
type and properly handles the API request with generic type parameters.src/features/user/PlanetCash/components/CreateAccountForm.tsx (2)
28-45
: LGTM! Well-structured type definitions.The new type definitions are comprehensive and provide good type safety for the PlanetCash functionality.
Line range hint
68-73
: LGTM! Type-safe API request.The
postAuthenticatedRequest
call correctly uses generic types for both response and request data.🧰 Tools
🪛 GitHub Check: CodeFactor
[notice] 64-64: src/features/user/PlanetCash/components/CreateAccountForm.tsx#L64
'Data' is not defined. (no-undef)pages/sites/[slug]/[locale]/profile/redeem/[code].tsx (2)
11-11
: LGTM! Clean type import.Good practice importing the shared type definition for code consistency.
77-86
: LGTM! Type-safe code submission.The code submission logic is well-structured with proper type safety through the use of
RedeemCodeSubmitData
.pages/sites/[slug]/[locale]/claim/[type]/[code].tsx (2)
38-40
: LGTM! Clean type definition.The
RedeemCodeSubmitData
type is well-defined and properly exported for reuse across the application.
86-94
: LGTM! Type-safe implementation.The code submission implementation correctly uses the defined type and generic parameters.
src/utils/apiRequests/api.ts (2)
29-34
: LGTM! Generic interface enhancement.Good improvement making the interface generic for better type safety.
174-181
: LGTM! Enhanced function signature.The function signature properly implements the generic interface and provides a sensible default for the data type.
src/features/user/ManageProjects/components/ProjectCertificates.tsx (2)
51-55
: LGTM! Type definition improves code clarity.The
SubmitData
type properly defines the structure of certificate submission data, enhancing type safety.
Line range hint
94-99
: LGTM! Generic type parameters enhance type safety.The
postAuthenticatedRequest
call now properly specifies both the response type (Certificate
) and request data type (SubmitData
).src/features/user/BulkCodes/forms/IssueCodesForm.tsx (1)
155-158
: LGTM! Enhanced type safety for API request.The
postAuthenticatedRequest
call now properly specifies both the response type (Donation
) and request data type (PrepaidDonationRequest
), improving type safety and code clarity.Also applies to: 159-166
src/features/user/RegisterTrees/RegisterTreesWidget.tsx (3)
64-70
: LGTM! Clear separation of concerns in type definitions.The
SubmitData
type properly defines the API submission structure, while keeping it separate from the form's state type (FormData
). This separation helps maintain a clear boundary between UI state and API contracts.
194-201
: LGTM! Proper data transformation.The transformation from form data to submission data is explicit and type-safe, ensuring all required fields are properly formatted.
Line range hint
202-211
: LGTM! Type-safe API request.The
postAuthenticatedRequest
call correctly specifies both the response type (ContributionProperties
) and request data type (SubmitData
).src/features/user/TreeMapper/Import/components/PlantingLocation.tsx (2)
171-179
: LGTM! Well-defined submission type.The
SubmitData
type clearly defines the structure of the planting location submission, with proper typing for geospatial data.
Line range hint
356-366
: LGTM! Type-safe API request implementation.The
postAuthenticatedRequest
call properly specifies both the response type (PlantLocationType
) and request data type (SubmitData
).src/features/user/ManageProjects/components/ProjectSites.tsx (3)
11-15
: LGTM! Good use of specific GeoJSON types.The imports from the 'geojson' library improve type safety for geographical data handling.
265-269
: LGTM! Well-structured type definition.The
SubmitData
type properly defines the shape of the data being submitted, with appropriate types for each field:
name
: stringgeometry
: GeoJson with specific Geometry and Properties typesstatus
: string
382-389
: LGTM! Enhanced type safety in API requests.Good use of the
SubmitData
type in thepostAuthenticatedRequest
call, ensuring type safety for both request and response data.src/features/user/ManageProjects/components/BasicDetails.tsx (2)
69-85
: LGTM! Well-structured base interface.The
SubmitDataBase
interface provides a clear foundation with all common fields between trees and conservation projects. Good use of specific types and the fixed 'EUR' currency type.
87-95
: LGTM! Clear type separation for different project types.Good separation of concerns with distinct interfaces for trees and conservation projects:
SubmitDataTrees
adds tree-specific fieldsSubmitDataConservation
adds conservation-specific fieldssrc/features/user/ManageProjects/components/ProjectMedia.tsx (2)
36-40
: LGTM! Clear type definition for media uploads.The
SubmitData
type properly defines the structure for media uploads with appropriate types for each field.
93-100
: LGTM! Enhanced type safety in API requests.Good use of the
SubmitData
type in thepostAuthenticatedRequest
call, ensuring type safety for both request and response data.src/features/user/ManageProjects/components/ProjectSpending.tsx (2)
59-63
: LGTM! Well-defined type for spending data.The
SubmitData
type properly defines the structure for spending records with appropriate types for each field.
95-102
: LGTM! Enhanced type safety in API requests.Good use of the
SubmitData
type in thepostAuthenticatedRequest
call, ensuring type safety for both request and response data.
@@ -42,11 +61,11 @@ | |||
|
|||
const onSubmit = async (event: FormEvent<HTMLFormElement>) => { | |||
event.preventDefault(); | |||
const data = { country: country, activate: !isPlanetCashActive }; | |||
const data: Data = { country: country, activate: !isPlanetCashActive }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix undefined type 'Data'.
The type 'Data' is not defined. Replace it with the newly defined 'SubmitData' type.
- const data: Data = { country: country, activate: !isPlanetCashActive };
+ const data: SubmitData = { country: country, activate: !isPlanetCashActive };
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 GitHub Check: CodeFactor
[notice] 64-64: src/features/user/PlanetCash/components/CreateAccountForm.tsx#L64
'Data' is not defined. (no-undef)
const submitData: SubmitData = { | ||
issueDate: issueDate.getFullYear(), | ||
certifierName: certifierName, | ||
pdfFile: pdf, | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
The review comment is valid - there's data loss when converting full date to year.
The code shows an inconsistency:
- The UI collects full date information through MuiDatePicker
- The display component shows the full date string:
<p>{report.issueDate} </p>
- However, during submission, the full date is reduced to just the year:
issueDate: issueDate.getFullYear()
This unnecessary data transformation results in loss of month and day information that the UI is designed to collect and display.
🔗 Analysis chain
Consider preserving full date information.
The issueDate
is being converted to just the year, which loses month and day information. Consider storing the full date if this information might be valuable.
Run this script to check if other parts of the system rely on or display the full date:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for usage of certificate issue dates in the codebase
rg -A 3 "issueDate" --type ts --type tsx
Length of output: 68
Script:
#!/bin/bash
# Search for issueDate in TypeScript files
echo "=== Searching in .ts and .tsx files ==="
rg "issueDate" . -g "*.ts" -g "*.tsx" -B 2 -A 2
echo -e "\n=== Looking for certificate-related interfaces/types ==="
ast-grep --pattern 'interface $_ {
$$$
issueDate
$$$
}'
echo -e "\n=== Looking for date formatting/display patterns ==="
rg "new Date|Date\." . -g "*.ts" -g "*.tsx" -B 1 -A 1
Length of output: 31912
This PR refactors the
POST
andPUT
authenticated requests by replacing the any type with more specific generic types to improve type safety and code maintainability.Changes Made