-
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?
Changes from 1 commit
ef2e23f
4a934a7
4c500e6
07d99a3
f91b377
afb42e3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,6 +25,25 @@ | |
allowedCountries: CountryType[]; | ||
} | ||
|
||
type SubmitData = { | ||
country: ExtendedCountryCode | string; | ||
activate: boolean; | ||
}; | ||
|
||
type PlanetCash = { | ||
id: string; | ||
isActive: boolean; | ||
ownerName: string; | ||
balance: number; | ||
debit: number; | ||
fee: number; | ||
creditLimit: number; | ||
currency: 'EUR' | 'USD'; | ||
country: 'DE' | 'ES' | 'US'; | ||
topUpThreshold: number; | ||
topUpAmount: number; | ||
}; | ||
|
||
const CreateAccountForm = ({ | ||
allowedCountries, | ||
isPlanetCashActive, | ||
|
@@ -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 commentThe 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 };
🧰 Tools🪛 GitHub Check: CodeFactor[notice] 64-64: src/features/user/PlanetCash/components/CreateAccountForm.tsx#L64 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @sunilsabatp Thanks for fixing the issue! 🎉 If you found this review helpful, would you consider giving us a shout-out on X? |
||
setIsProcessing(true); | ||
|
||
try { | ||
const res = await postAuthenticatedRequest({ | ||
const res = await postAuthenticatedRequest<PlanetCash, SubmitData>({ | ||
tenant: tenantConfig?.id, | ||
url: '/app/planetCash', | ||
data, | ||
|
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:
<p>{report.issueDate} </p>
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:
Length of output: 68
Script:
Length of output: 31912