Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions src/apps/onboarding/src/components/modal-add-work/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ const ModalAddWork: FC<ModalAddWorkProps> = (props: ModalAddWorkProps) => {
const [formErrors, setFormErrors] = useState<any>({

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[⚠️ maintainability]
Consider using a more specific type instead of any for formErrors to improve type safety and maintainability.

companyName: undefined,
endDate: undefined,
otherIndustry: undefined,
position: undefined,
startDate: undefined,
})
Expand Down Expand Up @@ -64,6 +65,10 @@ const ModalAddWork: FC<ModalAddWorkProps> = (props: ModalAddWorkProps) => {
errorTmp.endDate = 'Required'
}

if (workInfo.industry === 'Other' && !workInfo.otherIndustry?.trim()) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[❗❗ correctness]
The check for workInfo.otherIndustry?.trim() assumes otherIndustry is a string. Ensure workInfo.otherIndustry is always initialized as a string to avoid potential runtime errors.

errorTmp.otherIndustry = 'Please specify your industry'
}

setFormErrors(errorTmp)
return _.isEmpty(errorTmp)
}
Expand Down Expand Up @@ -164,6 +169,7 @@ const ModalAddWork: FC<ModalAddWorkProps> = (props: ModalAddWorkProps) => {
setWorkInfo({
...workInfo,
industry: event.target.value,
otherIndustry: event.target.value === 'Other' ? workInfo.otherIndustry : undefined,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[⚠️ correctness]
When setting otherIndustry to undefined, ensure that this does not unintentionally affect other parts of the application that may rely on otherIndustry being a string.

})
}}
name='industry'
Expand All @@ -172,6 +178,27 @@ const ModalAddWork: FC<ModalAddWorkProps> = (props: ModalAddWorkProps) => {
dirty
/>
</div>
{workInfo.industry === 'Other' && (
<div className='full-width'>
<InputText
name='otherIndustry'
label='Please specify your industry *'
value={workInfo.otherIndustry || ''}
onChange={function onChange(event: any) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[💡 maintainability]
Consider using a more specific type instead of any for the event parameter in the onChange function to improve type safety.

setWorkInfo({
...workInfo,
otherIndustry: event.target.value,
})
}}
placeholder='Enter your industry'
tabIndex={0}
type='text'
dirty
error={formErrors.otherIndustry}
maxLength={255}
/>
</div>
)}
<div className='d-flex gap-16 full-width flex-wrap'>
<div
className='flex-1'
Expand Down
2 changes: 2 additions & 0 deletions src/apps/onboarding/src/models/WorkInfo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ export default interface WorkInfo {
companyName?: string
position?: string
industry?: string
otherIndustry?: string
startDate?: Date
dateDescription?: string
description?: string
Expand All @@ -17,6 +18,7 @@ export const emptyWorkInfo: () => WorkInfo = () => ({
endDate: undefined,
id: 0,
industry: '',
otherIndustry: '',
position: '',
startDate: undefined,
})
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,13 @@ const ModifyWorkExpirenceModal: FC<ModifyWorkExpirenceModalProps> = (props: Modi
case 'startDate':
case 'endDate':
value = event as unknown as Date
break
case 'industry':
value = event.target.value
if (value !== 'Other') {
oldFormValues.otherIndustry = undefined

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[💡 readability]
Consider using delete oldFormValues.otherIndustry instead of setting it to undefined for better clarity and to avoid potential issues with object property checks.

}

break
default:
value = event.target.value
Expand Down Expand Up @@ -272,6 +279,13 @@ const ModifyWorkExpirenceModal: FC<ModifyWorkExpirenceModalProps> = (props: Modi
}
}

if (formValues.industry === 'Other' && !trim(formValues.otherIndustry as string)) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[⚠️ correctness]
The check for formValues.industry === 'Other' and !trim(formValues.otherIndustry as string) is correct, but ensure that formValues.otherIndustry is always a string before trimming to avoid runtime errors.

setFormErrors({
otherIndustry: 'Please specify your industry',
})
return
}

const companyName: string | undefined = formValues.company as string | undefined
const startDateIso: string | undefined = formValues.startDate
? (formValues.startDate as Date).toISOString()
Expand All @@ -287,6 +301,7 @@ const ModifyWorkExpirenceModal: FC<ModifyWorkExpirenceModalProps> = (props: Modi
description: (formValues.description as string) || undefined,
endDate: endDateIso,
industry: formValues.industry,
otherIndustry: formValues.industry === 'Other' ? (formValues.otherIndustry as string) : undefined,
position: formValues.position,
startDate: startDateIso,
timePeriodFrom: startDateIso,
Expand Down Expand Up @@ -343,6 +358,7 @@ const ModifyWorkExpirenceModal: FC<ModifyWorkExpirenceModalProps> = (props: Modi
? new Date(work.timePeriodTo)
: (work.endDate ? new Date(work.endDate) : undefined),
industry: work.industry || '',
otherIndustry: work.otherIndustry || '',
position: (work.position || '') as string,
startDate: work.timePeriodFrom
? new Date(work.timePeriodFrom)
Expand Down Expand Up @@ -483,6 +499,21 @@ const ModifyWorkExpirenceModal: FC<ModifyWorkExpirenceModalProps> = (props: Modi
dirty
error={formErrors.industry}
/>
{formValues.industry === 'Other' && (
<InputText
name='otherIndustry'
label='Please specify your industry *'
error={formErrors.otherIndustry}
placeholder='Enter your industry'
dirty
tabIndex={0}
forceUpdateValue
type='text'
onChange={bind(handleFormValueChange, this, 'otherIndustry')}
value={formValues.otherIndustry as string}
maxLength={255}
/>
)}
<div className={styles.row}>
<InputDatePicker
label='Start Date'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,17 @@ const WorkExpirenceCard: FC<WorkExpirenceCardProps> = (props: WorkExpirenceCardP
{position || ''}
{
position && industry
? `, ${getIndustryOptionLabel(industry)}`
: (industry ? getIndustryOptionLabel(industry) : '')
? `, ${industry === 'Other'

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[⚠️ readability]
The nested ternary operators make this logic difficult to read and understand. Consider refactoring this into a separate function to improve readability and maintainability.

&& props.work.otherIndustry
? props.work.otherIndustry
: getIndustryOptionLabel(industry)}`
: (industry
? (industry === 'Other'
&& props.work.otherIndustry
? props.work.otherIndustry
: getIndustryOptionLabel(industry))
: ''
)
}
</p>
)}
Expand Down
1 change: 1 addition & 0 deletions src/libs/shared/lib/constants/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,5 @@ export const INDUSTRIES_OPTIONS: string[] = [
'Telecoms',
'Public sector',
'Travel & hospitality',
'Others',
]
4 changes: 3 additions & 1 deletion src/libs/shared/lib/utils/industry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ export const IndustryEnumToLabel: {
[key: string]: string
} = {
ConsumerGoods: 'Consumer goods',
Other: 'Others',
PublicSector: 'Public sector',
TechAndTechnologyService: 'Tech & technology services',
TravelAndHospitality: 'Travel & hospitality',
Expand All @@ -11,6 +12,7 @@ export const IndustryLabelToEnum: {
[key: string]: string
} = {
'Consumer goods': 'ConsumerGoods',
Others: 'Other',
'Public sector': 'PublicSector',
'Tech & technology services': 'TechAndTechnologyService',
'Travel & hospitality': 'TravelAndHospitality',
Expand All @@ -24,7 +26,7 @@ export const getIndustryOptionValue = (v: string): string => {
}

if (keys.includes(v)) {
return IndustryEnumToLabel[v]
return v

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[❗❗ correctness]
The change from IndustryEnumToLabel[v] to v in getIndustryOptionValue might lead to incorrect behavior if v is expected to be a key in IndustryEnumToLabel. This could result in returning the key itself instead of its corresponding label. Verify if this change aligns with the intended logic.

}

return v
Expand Down
Loading