Fixes coordinator user filter in tag autocomplete#754
Conversation
nadavosa
left a comment
There was a problem hiding this comment.
Confirmed the fix is correct — checked the be repo directly: userListQuerySchema in src/server/schema/querystring.ts defines role as a top-level query param (not nested under filter, and additionalProperties: false means the old nested filter: { role } shape was silently dropped). So this isn't a guess, it matches the actual backend contract.
One cleanup note inline below.
| limit?: number; | ||
| sortOrder?: SortOrder; | ||
| filter?: FilterParam; | ||
| role?: UserRole; |
There was a problem hiding this comment.
This bolts a one-off role?: UserRole field onto the shared, resource-agnostic Params interface used by ~30 useGetQuery call sites app-wide. The hook already has a generic mechanism for exactly this (filter?: FilterParam, a Record<string, unknown>), used elsewhere for arbitrary resource filters, e.g. filter: { status: OpportunityStatusType.NEW } in NewestVolunteers.tsx/NewestOpportunities.tsx/OpportunityListController.tsx/AgentListController.tsx.
role is only ever set here, at this one call site. Since be's userListQuerySchema has additionalProperties: false and expects role as a flat param rather than nested, could this instead flow through the generic filter path if getReducedFilter flattened it accordingly (e.g. treating filter entries as top-level query keys), rather than adding a domain-specific field to the shared interface? Otherwise this sets a precedent: the next resource that needs a one-off top-level param gets its own bespoke field added here too, and Params keeps growing with fields most callers never use.
There was a problem hiding this comment.
thanks @nadavosa for reviewing! Tbh I'm not sure if there's a clean solution to this cleanup problem you highlighted considering in the be it doesn't accept filter: {role } anymore.
The main blocker here is that if we try to handle this inside the hook by flattening the filter object, it actually breaks the other 30 call sites. For example, if a query passes { filter: { status: 'new', role: 'coordinator' } }, we would have to write complex logic inside the generic hook to pluck role out to the top level while leaving status nested inside the object so the be views don't break.
My reasoning for adding it to the shared interface was that UserRole is a core concept that we will probably need in the future. Plus, since it’s optional, it won't affect any existing callers if left undefined.
That said, if you have something in mind, I'd love to hear it
There was a problem hiding this comment.
alternatively, the other solution would be to change it back to what it was in the be where role is nested within filter . Either way, the fe and be should be aligned
Description
Fetched users from autocomplete was not filtered to coordinators only. Possible the filter changed in
be. This corrects the filter so that only coordinators are shown.Related Issues
Currently no issue created but mentioned on #440
Changes
filter: {role: UserRole.COORDINATOR}torole: UserRole.COORDINATORScreenshots / Demos
If UI-related, add before/after or GIFs.
Checklist