IA-4691: refactor assignments page#2719
Conversation
quang-le
left a comment
quang-le
left a comment
There was a problem hiding this comment.
Great job! This interface finally makes some sense
Most comments are just nitpicks
2 are blocking:
- N+1 in the backend
- if--> else if in handleSave
Also, I some testing on the backend changes would be nice 😄
| rootTeam?: Team, | ||
| ) => { | ||
| const theme = useTheme(); | ||
| return (orgUnitId: number) => { |
|
|
||
| def get_geo_json(self, org_unit: OrgUnit): | ||
| if not hasattr(org_unit, "geo_json"): | ||
| if not hasattr(org_unit, "geo_json") or org_unit.geo_json is None: |
There was a problem hiding this comment.
You should update the test if it exists
There was a problem hiding this comment.
also, nitpick you could replace this with
if not self.get_has_geo_json(org_unit):|
|
||
| def get_iaso_profile_id(self, obj): | ||
| profile = getattr(obj, "iaso_profile", None) | ||
| return getattr(profile, "id", None) if profile else None |
| onColorChange: (color: string) => void; | ||
| }; | ||
|
|
||
| export const TeamRow: FunctionComponent<Props> = ({ |
There was a problem hiding this comment.
nitpick: I would name this one AssigneeRow or TeamTableRow, since it doesn't always display a team
| } = useGetAssignments({ planning: planningId }); | ||
| const { mutateAsync: saveAssignment, isLoading: isSaving } = | ||
| useSaveAssignment(); | ||
| const handleSaveAssignment = useCallback( |
There was a problem hiding this comment.
You could probably group this with useSaveAssignments in a single custom hook.
Not blocking
| delete safeParams.planningId; | ||
| delete safeParams.mode; |
There was a problem hiding this comment.
You could probably type params a bit more specifically, so we know that planningId and mode keys are always present
There was a problem hiding this comment.
This one is a bit tricky i'd say
| 'planningsList', | ||
| 'planningDetails', | ||
| 'planningOrgUnits', | ||
| 'planningSamplingResults', |
There was a problem hiding this comment.
Don't we also need planningRootOrgUnit and planningChildrenOrgUnits?
| zoom={defaultViewport.zoom} | ||
| scrollWheelZoom={false} | ||
| zoomControl={false} | ||
| contextmenu |
There was a problem hiding this comment.
I would split the different Panes into components for easier reading. Not blocking
There was a problem hiding this comment.
this map will move a lot i think, but yes, after
Co-authored-by: Quang Son Le <38907762+quang-le@users.noreply.github.com>
3 participants
What problem is this PR solving?
Actual page is not using sampling results or the target org unit type at all
Logic is totally different as previous.
We need to remove old code and start from scratch a new page/
We will implement features one by one, for now this PR should a llow a user to see the org units resulting from a sampling or not and assign user or team to an org unit.
Related JIRA tickets
IA-4691
Changes
How to test
Print screen / video
Screen.Recording.2026-02-04.at.12.55.27.mov
Notes
Doc
docs/pages/dev/how_to/openhexa-integration/openhexa-integration.en.md