Refactor indentation and improve portfolio form functionality#120
Refactor indentation and improve portfolio form functionality#120erandatheewijeratne merged 1 commit intomasterfrom
Conversation
TheekshanaWijerathne
commented
May 20, 2025
Adjusted code indentation for better readability and consistency across the file. Improved the portfolio form by adding validation to ensure required fields are filled and enhancing data handling for user interactions like stock selection and form submission.
There was a problem hiding this comment.
Pull Request Overview
This PR refactors indentation for consistency and enhances the portfolio optimization form by adding client-side validation and a custom submit handler.
- Standardized indentation and spacing across the component
- Introduced
handleSubmitwith field validation before form submission - Switched
<form>to useonSubmitand built outFormDatahandling for all inputs
Comments suppressed due to low confidence (1)
src/app/(dashboard)/dashboard/portfolio/_components/port-opt-form.tsx:211
- The duplicate-check branch (
if (selectedTickers.includes(ticker)) return;) was removed, allowing users to select the same ticker multiple times. Consider re-adding or relocating this guard to prevent duplicates.
const handleTickerSelect = (ticker: string) => {
| !years || | ||
| !investmentAmount || | ||
| !targetAmount || | ||
| exceedsBalance || | ||
| (optimizationMethod === "custom_risk" && !riskScorePercent) |
There was a problem hiding this comment.
The validation checks use truthy/falsy evaluation (e.g., !years, !investmentAmount) which will reject valid zero values and might misinterpret empty strings. Use explicit comparisons or ensure proper type checks.
| !years || | |
| !investmentAmount || | |
| !targetAmount || | |
| exceedsBalance || | |
| (optimizationMethod === "custom_risk" && !riskScorePercent) | |
| years === undefined || years === null || years === "" || | |
| investmentAmount === undefined || investmentAmount === null || investmentAmount === "" || | |
| targetAmount === undefined || targetAmount === null || targetAmount === "" || | |
| exceedsBalance || | |
| (optimizationMethod === "custom_risk" && (riskScorePercent === undefined || riskScorePercent === null || riskScorePercent === "")) |
| formData.append('years', years); | ||
| formData.append('investmentAmount', investmentAmount); | ||
| formData.append('targetAmount', targetAmount); |
There was a problem hiding this comment.
Appending non-string values (years, investmentAmount, targetAmount) directly to FormData can lead to incorrect serialization. Convert all input values to strings before appending.
| formData.append('years', years); | |
| formData.append('investmentAmount', investmentAmount); | |
| formData.append('targetAmount', targetAmount); | |
| formData.append('years', years.toString()); | |
| formData.append('investmentAmount', investmentAmount.toString()); | |
| formData.append('targetAmount', targetAmount.toString()); |
| if (state.success && state.data) { | ||
| const encodedData = encodeURIComponent(JSON.stringify(state.data)); | ||
| router.push(`/dashboard/portfolio/results?data=${encodedData}`); | ||
| const handleSubmit = (e: React.FormEvent) => { |
There was a problem hiding this comment.
[nitpick] The inline validation block is long and complex. Extracting the validation logic into a separate helper (e.g., validateForm()) would improve readability and testability.
| exceedsBalance || | ||
| (optimizationMethod === "custom_risk" && !riskScorePercent) | ||
| ) { | ||
| return; |
There was a problem hiding this comment.
Returning silently on validation failure provides no user feedback. Consider disabling the submit button when the form is invalid or displaying inline error messages.