-
Notifications
You must be signed in to change notification settings - Fork 1
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
Local unit changes modal #1584
Local unit changes modal #1584
Conversation
|
11f0b07
to
05aaf87
Compare
05aaf87
to
3e9dde2
Compare
3e9dde2
to
7a862f1
Compare
- Add function to get changes name and value - Add new file for key and label of the field
7a862f1
to
2aefe40
Compare
2aefe40
to
153f5c4
Compare
...ewContextAndStructure/NationalSocietyLocalUnits/LocalUnitsFormModal/LocalUnitsForm/index.tsx
Outdated
Show resolved
Hide resolved
const flattenedValues = flattenObject(newValues); | ||
const flattenedOldValues = flattenObject(oldValues); |
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.
I am not sure if we can use this approach. This fails if/when we have array elements.
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.
we will refactor this later. will add a TODO
health: { | ||
affiliation: health?.affiliation, | ||
functionality: health?.functionality, | ||
health_facility_type: health?.health_facility_type, |
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.
Any reason to write these out individually? Why not just spread this out?
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.
when we compare the value later, the comparison function checks for _display
values. We don't need these values for form fields. So we didn't spread them out.
...ewContextAndStructure/NationalSocietyLocalUnits/LocalUnitsFormModal/LocalUnitsForm/index.tsx
Outdated
Show resolved
Hide resolved
.../NationalSocietyLocalUnits/LocalUnitsFormModal/LocalUnitsForm/useLocalUnitFormFieldLabels.ts
Show resolved
Hide resolved
app/src/utils/common.ts
Outdated
export function compareArrays(newArray: unknown[], oldArray: unknown[]): boolean { | ||
if (newArray.length !== oldArray.length) { | ||
return false; | ||
} | ||
return newArray.every((id) => oldArray.includes(id)); | ||
} | ||
|
||
export function flattenObject<T extends Record<string, unknown>>( | ||
inputObject: T, | ||
prefix?: string, | ||
): Record<string, unknown> { | ||
return Object.entries(inputObject).reduce((acc, [key, value]) => { | ||
const newKey = prefix ? `${prefix}.${key}` : key; | ||
if (typeof value === 'object' && value !== null && !Array.isArray(value)) { | ||
return { ...acc, ...flattenObject(value as Record<string, unknown>, newKey) }; | ||
} | ||
return { ...acc, [newKey]: value }; | ||
}, {} as Record<string, unknown>); | ||
} | ||
|
||
export function getLastSegment(str: string, delimiter: string) { | ||
const parts = str.split(delimiter); | ||
return parts[parts.length - 1]; | ||
} |
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.
write tests
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.
Added a TODO
comment.
function getChangedFormFields( | ||
newValues: PartialLocalUnits, | ||
oldValues: LocalUnitResponse, | ||
formFieldOptions: LocalUnitOptions, | ||
) { |
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.
Write return type
function getChangedFormFields( | |
newValues: PartialLocalUnits, | |
oldValues: LocalUnitResponse, | |
formFieldOptions: LocalUnitOptions, | |
) { | |
function getChangedFormFields( | |
newValues: PartialLocalUnits, | |
oldValues: LocalUnitResponse, | |
formFieldOptions: LocalUnitOptions, | |
): { key: string; value: string }[] { |
const actualKey = getLastSegment(key, '.'); | ||
if (Array.isArray(newValue) && Array.isArray(oldValue)) { | ||
if (!compareArrays(newValue, oldValue)) { | ||
const options: Option[] = formFieldOptions?.[actualKey as keyof LocalUnitOptions]; | ||
const valuesLabels = newValue.map( | ||
(v: number) => options.find((option: Option) => option.id === v), | ||
).filter(isDefined).map((option) => option.name).join(', '); | ||
changedValues.push({ key, value: valuesLabels }); | ||
} | ||
} else if (newValue !== oldValue) { | ||
const options: Option[] = formFieldOptions?.[actualKey as keyof LocalUnitOptions]; | ||
if (isDefined(options)) { | ||
const valueLabel = options.find( | ||
(option: Option) => option.id === newValue, | ||
)?.name; | ||
changedValues.push({ key, value: valueLabel }); | ||
} else if (typeof newValue === 'boolean') { | ||
changedValues.push({ key, value: newValue ? 'Yes' : 'No' }); // TODO: use translations | ||
} else { | ||
changedValues.push({ key, value: newValue as string }); | ||
} | ||
} |
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.
add few comments on what is being done to make it more readable
...ewContextAndStructure/NationalSocietyLocalUnits/LocalUnitsFormModal/LocalUnitsForm/index.tsx
Show resolved
Hide resolved
...ewContextAndStructure/NationalSocietyLocalUnits/LocalUnitsFormModal/LocalUnitsForm/index.tsx
Outdated
Show resolved
Hide resolved
36763c9
to
0c4bffc
Compare
Addresses:
Depends on:
Changes
This PR doesn't introduce:
console.log
meant for debugging