Implement filter multiple columns API #45

a1-su · 2025-11-21T23:10:47Z

Implements the API to filter volunteers by multiple columns, including by general volunteer attributes, by cohort year and term, and by role name, in the GET /api/volunteers/filter/multiple endpoint.

Initial Database Setup
Volunteers

Roles

VolunteerRoles

Cohorts

VolunteerCohorts

Sample Queries
Global OR Operation

Global AND Operation

Invalid Input

vercel · 2025-11-21T23:10:53Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Preview	Comments	Updated (UTC)
trcc	Ready	Preview	Comment	Nov 21, 2025 11:10pm

Copilot

Pull request overview

This PR implements a multi-column filtering API for volunteers at GET /api/volunteers/filter/multiple. The endpoint supports filtering by volunteer attributes, cohort (term/year), and role names with configurable AND/OR operations both globally and within individual filters.

Key Changes:

New filtering endpoint supporting complex query combinations with nested AND/OR logic
Validation function to ensure filter structure and field names are valid
Support for three filtering strategies: general volunteer fields, role-based filtering, and cohort-based filtering

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 13 comments.

File	Description
`src/lib/api/index.ts`	Exports the new `filterMultipleColumns` and `validateFilter` functions
`src/lib/api/filterMultipleColumns.ts`	Core filtering logic with validation, type definitions, and database queries for volunteers, roles, and cohorts
`src/app/api/volunteers/filter/multiple/route.ts`	Next.js API route handler that parses query parameters, validates input, and returns filtered volunteer data

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-22T17:19:22Z

src/lib/api/filterMultipleColumns.ts

+export type FilterTuple = {
+  mini_op: string;
+  field: string;
+  values: string[];


Type definition mismatch: FilterTuple.values is typed as string[], but for cohorts it actually contains arrays [term, year]. The type should be string[] | [string, string][] or use a union type to properly represent both cases.

Suggested change

values: string[];

values: string[] | [string, string][];

Copilot · 2025-11-22T17:19:23Z

src/lib/api/filterMultipleColumns.ts

+        const roleVolunteerIds: Set<number>[] = [];
+
+        for (const v of values) {
+          const query = client
+            .from("VolunteerRoles")
+            .select("volunteer_id, Roles!inner(name)");
+
+          const { data: cohortRows, error } = await query.eq("Roles.name", v);
+
+          if (error) return { error: error.message };
+
+          roleVolunteerIds.push(new Set(cohortRows.map((r) => r.volunteer_id)));
+        }
+
+        if (roleVolunteerIds.length > 0) {
+          volunteerIds = [
+            ...roleVolunteerIds.reduce((acc, cur) => {
+              return acc.intersection(cur);
+            }),
+          ];
+        }
+      }
+    } else if (filterField === "cohorts") {
+      // Cohort-specific filtering
+      if (mini_op === "OR") {
+        let query = client
+          .from("VolunteerCohorts")
+          .select("volunteer_id, Cohorts!inner(term, year)");
+
+        const orStatement = values
+          .map((v) => `and(term.eq.${v[0]},year.eq.${v[1]})`)
+          .join(",");
+        query = query.or(orStatement, { referencedTable: "Cohorts" });
+
+        const { data: cohortRows, error } = await query;
+        if (error) return { error: error.message };
+
+        volunteerIds = cohortRows.map((r) => r.volunteer_id);
+      } else {
+        const cohortVolunteerIds: Set<number>[] = [];
+
+        for (const v of values) {
+          const query = client
+            .from("VolunteerCohorts")
+            .select("volunteer_id, Cohorts!inner(term, year)");
+
+          const { data: cohortRows, error } = await query
+            .eq("Cohorts.term", v[0])
+            .eq("Cohorts.year", parseInt(v[1]));
+
+          if (error) return { error: error.message };
+
+          cohortVolunteerIds.push(
+            new Set(cohortRows.map((r) => r.volunteer_id))
+          );
+        }
+
+        if (cohortVolunteerIds.length > 0) {
+          volunteerIds = [
+            ...cohortVolunteerIds.reduce((acc, cur) => {
+              return acc.intersection(cur);
+            }),
+          ];
+        }


[nitpick] Code duplication in role and cohort filtering logic. Both the role filtering (lines 112-132) and cohort filtering (lines 151-175) blocks implement nearly identical AND operation logic. Consider extracting this into a reusable helper function to improve maintainability.

Copilot · 2025-11-22T17:19:23Z

src/lib/api/filterMultipleColumns.ts

+        const roleVolunteerIds: Set<number>[] = [];
+
+        for (const v of values) {
+          const query = client
+            .from("VolunteerRoles")
+            .select("volunteer_id, Roles!inner(name)");
+
+          const { data: cohortRows, error } = await query.eq("Roles.name", v);
+
+          if (error) return { error: error.message };
+
+          roleVolunteerIds.push(new Set(cohortRows.map((r) => r.volunteer_id)));
+        }
+
+        if (roleVolunteerIds.length > 0) {
+          volunteerIds = [
+            ...roleVolunteerIds.reduce((acc, cur) => {
+              return acc.intersection(cur);
+            }),
+          ];
+        }


N+1 query problem: When mini_op is "AND" for roles, a separate database query is executed for each value in the array (lines 114-124). For large value arrays, this creates a performance bottleneck. Consider refactoring to use a single query with aggregation/grouping to identify volunteers who have all specified roles.

Suggested change

const roleVolunteerIds: Set<number>[] = [];

for (const v of values) {

const query = client

.from("VolunteerRoles")

.select("volunteer_id, Roles!inner(name)");

const { data: cohortRows, error } = await query.eq("Roles.name", v);

if (error) return { error: error.message };

roleVolunteerIds.push(new Set(cohortRows.map((r) => r.volunteer_id)));

}

if (roleVolunteerIds.length > 0) {

volunteerIds = [

...roleVolunteerIds.reduce((acc, cur) => {

return acc.intersection(cur);

}),

];

}

// Fetch all volunteer-role pairs for the specified roles in a single query

const { data: roleRows, error } = await client

.from("VolunteerRoles")

.select("volunteer_id, Roles!inner(name)")

.in("Roles.name", values);

if (error) return { error: error.message };

// Group by volunteer_id and count the number of matching roles

const volunteerRoleCount: Record<number, number> = {};

for (const row of roleRows) {

if (volunteerRoleCount[row.volunteer_id]) {

volunteerRoleCount[row.volunteer_id]++;

} else {

volunteerRoleCount[row.volunteer_id] = 1;

}

}

// Only include volunteers who have all specified roles

volunteerIds = Object.entries(volunteerRoleCount)

.filter(([_, count]) => count === values.length)

.map(([volunteer_id, _]) => Number(volunteer_id));

Copilot · 2025-11-22T17:19:23Z

src/lib/api/filterMultipleColumns.ts

+  // Allow filter to be an empty array
+
+  for (const f of filtersList) {
+    if (!f.mini_op || !(f.mini_op == "AND" || f.mini_op == "OR"))


[nitpick] Case sensitivity inconsistency: The validation converts field names to lowercase (line 47) and the filtering logic also uses lowercase (line 96), but mini_op is validated case-sensitively (line 44). Consider normalizing mini_op to uppercase before validation for consistency and better user experience.

Suggested change

if (!f.mini_op || !(f.mini_op == "AND" || f.mini_op == "OR"))

if (

!f.mini_op ||

!(f.mini_op.toUpperCase() === "AND" || f.mini_op.toUpperCase() === "OR")

)

Copilot · 2025-11-22T17:19:23Z

src/lib/api/filterMultipleColumns.ts

+
+  let finalIds: Set<number>;
+
+  if (querySets.length === 0) return { data: [] };


[nitpick] The function can return early with an empty array (line 200) when querySets.length === 0, but this scenario should be impossible given the code flow. If filtersList.length === 0, the function returns early at line 90. Otherwise, at least one set is added to querySets. This check appears to be dead code and should be removed, or clarify if there's an edge case being handled.

Suggested change

if (querySets.length === 0) return { data: [] };

Copilot · 2025-11-22T17:19:24Z

src/lib/api/filterMultipleColumns.ts

+export async function filterMultipleColumns(
+  filtersList: FilterTuple[],
+  op: string
+): Promise<VolunteerFilterResponse> {


[nitpick] Missing documentation for complex filter logic. The function handles three different filtering strategies (general, roles, cohorts) with different query patterns. Add JSDoc comments explaining the expected input format, behavior of mini_op vs global op, and provide examples of valid filter inputs.

Copilot · 2025-11-22T17:19:24Z

src/lib/api/filterMultipleColumns.ts

+  error?: string;
+} {
+  if (!Array.isArray(filtersList))
+    return { valid: false, error: "'filter' must be an array" };


Error message inconsistency: the validation function returns "'filter' must be an array" (line 39) but the parameter is actually named 'filters_list' in the API route. This could confuse API consumers. Consider using the actual parameter name in the error message.

Suggested change

return { valid: false, error: "'filter' must be an array" };

return { valid: false, error: "'filtersList' must be an array" };

Copilot · 2025-11-22T17:19:24Z

src/lib/api/filterMultipleColumns.ts

+        const cohortVolunteerIds: Set<number>[] = [];
+
+        for (const v of values) {
+          const query = client
+            .from("VolunteerCohorts")
+            .select("volunteer_id, Cohorts!inner(term, year)");
+
+          const { data: cohortRows, error } = await query
+            .eq("Cohorts.term", v[0])
+            .eq("Cohorts.year", parseInt(v[1]));
+
+          if (error) return { error: error.message };
+
+          cohortVolunteerIds.push(
+            new Set(cohortRows.map((r) => r.volunteer_id))
+          );
+        }
+
+        if (cohortVolunteerIds.length > 0) {
+          volunteerIds = [
+            ...cohortVolunteerIds.reduce((acc, cur) => {
+              return acc.intersection(cur);
+            }),
+          ];
+        }


N+1 query problem: When mini_op is "AND" for cohorts, a separate database query is executed for each value in the array (lines 153-167). For large value arrays, this creates a performance bottleneck. Consider refactoring to use a single query with aggregation/grouping to identify volunteers who are in all specified cohorts.

Suggested change

const cohortVolunteerIds: Set<number>[] = [];

for (const v of values) {

const query = client

.from("VolunteerCohorts")

.select("volunteer_id, Cohorts!inner(term, year)");

const { data: cohortRows, error } = await query

.eq("Cohorts.term", v[0])

.eq("Cohorts.year", parseInt(v[1]));

if (error) return { error: error.message };

cohortVolunteerIds.push(

new Set(cohortRows.map((r) => r.volunteer_id))

);

}

if (cohortVolunteerIds.length > 0) {

volunteerIds = [

...cohortVolunteerIds.reduce((acc, cur) => {

return acc.intersection(cur);

}),

];

}

// Refactored: Single query for all cohort values, then group by volunteer_id

// Build filter for all cohort values

const orStatement = values

.map((v) => `and(term.eq.${v[0]},year.eq.${v[1]})`)

.join(",");

let query = client

.from("VolunteerCohorts")

.select("volunteer_id, Cohorts!inner(term, year)");

query = query.or(orStatement, { referencedTable: "Cohorts" });

const { data: cohortRows, error } = await query;

if (error) return { error: error.message };

// Group by volunteer_id and count matches

const volunteerCount: Record<number, number> = {};

for (const row of cohortRows) {

volunteerCount[row.volunteer_id] = (volunteerCount[row.volunteer_id] || 0) + 1;

}

// Only volunteers present in all cohorts

volunteerIds = Object.entries(volunteerCount)

.filter(([_, count]) => count === values.length)

.map(([volunteer_id, _]) => Number(volunteer_id));

Copilot · 2025-11-22T17:19:25Z

src/lib/api/filterMultipleColumns.ts

+        (v) =>
+          !Array.isArray(v) ||
+          v.length !== 2 ||
+          typeof v[0] !== "string" ||
+          !/^(Fall|Spring|Summer|Winter)$/i.test(v[0]) ||
+          typeof v[1] !== "string" ||
+          isNaN(parseInt(v[1]))


[nitpick] The cohort filter values use a string year that must be parseable as an integer, but the database query expects an integer. While parseInt(v[1]) is used in the query (line 160), the validation should ensure the parsed value is within a reasonable range (e.g., 1900-2100) to prevent potential issues with edge cases or malformed data.

Suggested change

(v) =>

!Array.isArray(v) ||

v.length !== 2 ||

typeof v[0] !== "string" ||

!/^(Fall|Spring|Summer|Winter)$/i.test(v[0]) ||

typeof v[1] !== "string" ||

isNaN(parseInt(v[1]))

(v) => {

if (

!Array.isArray(v) ||

v.length !== 2 ||

typeof v[0] !== "string" ||

!/^(Fall|Spring|Summer|Winter)$/i.test(v[0]) ||

typeof v[1] !== "string"

) {

return true;

}

const year = parseInt(v[1]);

if (isNaN(year) || year < 1900 || year > 2100) {

return true;

}

return false;

}

Copilot · 2025-11-22T17:19:25Z

src/lib/api/filterMultipleColumns.ts

+  if (!Array.isArray(filtersList))
+    return { valid: false, error: "'filter' must be an array" };
+
+  // Allow filter to be an empty array


[nitpick] Unnecessary empty comment or unclear intention. If this comment is meant to explain why empty arrays are allowed, it should be more explicit (e.g., "Empty filter arrays are allowed and will return all volunteers"). Otherwise, remove the empty comment.

Suggested change

// Allow filter to be an empty array

// Empty filter arrays are allowed and will return all volunteers

notjackl3 · 2025-11-26T23:53:26Z

src/app/api/volunteers/filter/multiple/route.ts

We don't need to implement route.ts file.

notjackl3 · 2025-11-27T00:05:46Z

src/lib/api/filterMultipleColumns.ts

+  // Allow filter to be an empty array
+
+  for (const f of filtersList) {
+    if (!f.mini_op || !(f.mini_op == "AND" || f.mini_op == "OR"))


Instead of checking f.mini_op == "AND" every time, you can create operation constants to improve consistency and reduce typos:

export const MINI_OP = { AND: "AND", OR: "OR", } as const;

Then do f.mini_op == MINI_OP.AND. Same goes to OR.

notjackl3 · 2025-11-27T00:10:45Z

src/lib/api/filterMultipleColumns.ts

+        query = query.in(filterField, values);
+      } else {
+        values.forEach((v) => {
+          query = query.eq(filterField, v);


I agree with Copilot, AND is not correct here, the loop you are doing will act as: query.eq(field, a).eq(field, b).eq(field, c)... which is incorrect.

notjackl3 · 2025-11-27T00:13:50Z

src/lib/api/filterMultipleColumns.ts

+            .from("VolunteerRoles")
+            .select("volunteer_id, Roles!inner(name)");
+
+          const { data: cohortRows, error } = await query.eq("Roles.name", v);


cohortRows should be roleRows instead, because it contains row data.

a1-su added 2 commits November 21, 2025 17:08

Implement filter multiple columns API function

99bd6d2

Fix role and cohort filtering

0b45ddd

a1-su linked an issue Nov 21, 2025 that may be closed by this pull request

BE 2 - Implement filter_multiple_columns API function #31

Open

1zhaohel requested a review from Copilot November 22, 2025 17:13

Copilot started reviewing on behalf of 1zhaohel November 22, 2025 17:14 View session

Copilot finished reviewing on behalf of 1zhaohel November 22, 2025 17:17

Copilot AI reviewed Nov 22, 2025

View reviewed changes

notjackl3 reviewed Nov 26, 2025

View reviewed changes

src/app/api/volunteers/filter/multiple/route.ts

Copy link

Contributor

notjackl3 Nov 26, 2025

Choose a reason for hiding this comment

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

We don't need to implement route.ts file.

notjackl3 reviewed Nov 27, 2025

View reviewed changes


		let finalIds: Set<number>;

		if (querySets.length === 0) return { data: [] };

	return { valid: false, error: "'filter' must be an array" };
	return { valid: false, error: "'filtersList' must be an array" };

	// Allow filter to be an empty array
	// Empty filter arrays are allowed and will return all volunteers

Implement filter multiple columns API #45

Are you sure you want to change the base?

Implement filter multiple columns API #45

Uh oh!

Conversation

a1-su commented Nov 21, 2025

Uh oh!

vercel bot commented Nov 21, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Nov 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 22, 2025

Choose a reason for hiding this comment

Uh oh!

notjackl3 Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

notjackl3 Nov 27, 2025

Choose a reason for hiding this comment

Uh oh!

notjackl3 Nov 27, 2025

Choose a reason for hiding this comment

Uh oh!

notjackl3 Nov 27, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants