Skip to content

Fixes coordinator user filter in tag autocomplete #754

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.

Sign up for GitHub

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

Merged
need4deed merged 6 commits into from
Jul 13, 2026
+3 −2
Merged

Fixes coordinator user filter in tag autocomplete #754

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ff9e606
changes param filter to role
DarrellRoberts Jul 2, 2026
8b0f3cd
Merge branch 'develop' into darrell/fix/tagging-users-not-filtered
DarrellRoberts Jul 8, 2026
e0e22f7
Merge branch 'develop' into darrell/fix/tagging-users-not-filtered
need4deed Jul 9, 2026
8a7138b
Merge branch 'develop' into darrell/fix/tagging-users-not-filtered
DarrellRoberts Jul 10, 2026
9310880
Merge branch 'develop' into darrell/fix/tagging-users-not-filtered
need4deed Jul 13, 2026
ec315f6
Merge branch 'develop' into darrell/fix/tagging-users-not-filtered
need4deed Jul 13, 2026
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
2 changes: 1 addition & 1 deletion src/components/Dashboard/Profile/sections/Comments/common/hooks/useCommentTag.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export function useCommentTag(
apiPath: apiPathUser,
params: {
sortOrder: SortOrder.NewToOld,
filter: { role: UserRole.COORDINATOR },
role: UserRole.COORDINATOR,
},
staleTime: cacheTTL,
enabled: !!setNewCommentText,
Expand Down
3 changes: 2 additions & 1 deletion src/hooks/useGetQuery.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { useQuery } from "@tanstack/react-query";
import axios, { AxiosResponse } from "axios";
import { Lang, SortOrder } from "need4deed-sdk";
import { Lang, SortOrder, UserRole } from "need4deed-sdk";
import { useParams } from "next/navigation";
import { useEffect } from "react";
import { useTranslation } from "react-i18next";
Expand Down Expand Up @@ -44,6 +44,7 @@ interface Params {
limit?: number;
sortOrder?: SortOrder;
filter?: FilterParam;
role?: UserRole;

@nadavosa nadavosa Jul 7, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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.

@DarrellRoberts DarrellRoberts Jul 8, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

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

@DarrellRoberts DarrellRoberts Jul 9, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

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

}
interface UseGetQuery {
apiPath: string;
Expand Down
Loading