Implement Filter by Volunteers by Cohorts API function

[jonathanqiao1]

No description provided.

[vercel]

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

Project	Deployment	Preview	Comments	Updated (UTC)
trcc	Ready	Preview	Comment	Nov 22, 2025 7:14pm

[Copilot]

Pull request overview

This PR implements a new API function to filter volunteers by cohorts using AND/OR operations. The function queries Supabase to retrieve volunteers who match specific cohort criteria (combinations of term and year), supporting both inclusive (OR) and exclusive (AND) filtering logic.

Key Changes

• Added getVolunteersByCohorts function with support for AND/OR filtering operations on volunteer cohorts
• Implemented database query logic using Supabase joins and filters
• Exported the new function through the API barrel file

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.

File	Description
src/lib/api/getVolunteersByCohorts.ts	New API function implementing cohort-based volunteer filtering with complex query logic and post-processing
src/lib/api/index.ts	Added export for the new getVolunteersByCohorts function to maintain barrel export pattern

---

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

[Copilot]

Missing input validation for the op parameter. The function accepts a FilterOp type but only handles "AND" and "OR" values. If the op value is something other than "AND", the function assumes it should perform an OR operation without explicitly checking.

Consider adding explicit validation or a default case to handle unexpected values more clearly, or at least documenting the assumption that non-"AND" operations default to OR behavior.

[Copilot]

Missing documentation for the main exported function. The function has complex behavior with multiple parameters and return types, but lacks JSDoc comments or other documentation to explain:

• What the function does
• The purpose and expected format of parameters (e.g., what is a valid CohortTuple?)
• The structure of the return value
• How the AND vs OR operators differ in behavior
• Example usage

This is especially important for a public API function that will be consumed by other parts of the codebase.

[LeandroHamaguchi]

Add a documentation for the getVolunteersByCohorts function. A comment on top of the function explaining parameters, return value, behaviour, AND vs OR operators, etc. is good.

[Copilot]

The OR filter is built and applied unconditionally even when the operator is "AND". This means the database query always performs an OR filter on the cohorts, and then the AND logic is applied as a post-processing step in JavaScript.

While this may work functionally, it's inefficient because:

1. The database returns all volunteers matching ANY of the cohorts
2. For AND operations, most of these results are then filtered out in memory

For better performance with AND operations, consider using a different database query strategy that leverages SQL's capabilities rather than post-processing in JavaScript.

Suggested change

	const orFilter = buildOrFilterString(values);
	// Perform Inner join to discard volunteers with 0 matching cohorts
	let query = client.from("Volunteers").select(`
	*,
	VolunteerCohorts!inner (
	Cohorts!inner (
	term,
	year
	)
	)
	`);
	// Apply or filter to the Cohorts table, returns table with only matching cohorts
	query = query.or(orFilter, { foreignTable: "VolunteerCohorts.Cohorts" });
	const { data, error } = await query;
	if (error) {
	console.error("Supabase Error:", error);
	return {
	data: null,
	error: error.message,
	status: 500,
	};
	}
	if (op === "AND") {
	// Cast data to the interface defined above, since table was joined above
	const volunteers = data as unknown as VolunteerQueryJoin[];
	const filteredData = filterForAndOperator(volunteers, values);
	return {
	data: filteredData,
	error: null,
	status: 200,
	};
	}
	return {
	data,
	error: null,
	status: 200,
	};
	if (op === "AND") {
	// For AND, build a filter that matches all cohorts using group/having
	// Build array of conditions for each cohort
	const conditions = values.map(
	([term, year]) => `(Cohorts.term = '${term}' AND Cohorts.year = ${year})`
	);
	const whereClause = conditions.join(" OR ");
	// Use SQL to group by volunteer and require count = values.length
	// Use Supabase's rpc or .query method for custom SQL
	// We'll use the Supabase SQL query method
	const sql = `
	SELECT
	Volunteers.*,
	json_agg(
	json_build_object(
	'term', Cohorts.term,
	'year', Cohorts.year
	)
	) AS matched_cohorts
	FROM Volunteers
	INNER JOIN VolunteerCohorts ON Volunteers.id = VolunteerCohorts.volunteer_id
	INNER JOIN Cohorts ON VolunteerCohorts.cohort_id = Cohorts.id
	WHERE ${whereClause}
	GROUP BY Volunteers.id
	HAVING COUNT(DISTINCT Cohorts.id) = ${values.length}
	`;
	const { data, error } = await client.rpc('execute_sql', { sql });
	if (error) {
	console.error("Supabase Error:", error);
	return {
	data: null,
	error: error.message,
	status: 500,
	};
	}
	return {
	data,
	error: null,
	status: 200,
	};
	} else {
	// OR logic: use current approach
	const orFilter = buildOrFilterString(values);
	let query = client.from("Volunteers").select(`
	*,
	VolunteerCohorts!inner (
	Cohorts!inner (
	term,
	year
	)
	)
	`);
	query = query.or(orFilter, { foreignTable: "VolunteerCohorts.Cohorts" });
	const { data, error } = await query;
	if (error) {
	console.error("Supabase Error:", error);
	return {
	data: null,
	error: error.message,
	status: 500,
	};
	}
	return {
	data,
	error: null,
	status: 200,
	};
	}

[LeandroHamaguchi]

This Copilot comment seems reasonable. Let's see if this implementation works after creating tests.

[Copilot]

The filter string construction is vulnerable to injection attacks. User-provided term values are directly interpolated into the query string without sanitization or escaping.

For example, if a term contains a double quote character ("), it could break out of the string delimiter and potentially manipulate the query structure: term.eq."Fall",year.eq.2023" could become malformed with input like Fall" or contain characters that break the Supabase query syntax.

Consider validating or sanitizing the term input, or using Supabase's parameterized query methods if available to prevent injection.

Suggested change

	function buildOrFilterString(values: CohortTuple[]): string {
	// Construct Supabase OR filter string for SQL query
	const conditions = values.map(
	([term, year]) => `and(term.eq."${term}",year.eq.${year})`
	// Escape double quotes and backslashes in term to prevent injection
	function escapeSupabaseString(str: string): string {
	return str.replace(/\\/g, '\\\\').replace(/"/g, '\\"');
	}
	function buildOrFilterString(values: CohortTuple[]): string {
	// Construct Supabase OR filter string for SQL query
	const conditions = values.map(
	([term, year]) => `and(term.eq."${escapeSupabaseString(term)}",year.eq.${year})`

[notjackl3]

You should change the branch name to from add-filter-by-cohort to backend/filter-by-cohort-api.

[notjackl3]

This only checks the count of cohorts, not which specific cohorts the volunteer has, you need to check for the content as well.

[LeandroHamaguchi]

This Copilot comment seems reasonable. Let's see if this implementation works after creating tests.

[LeandroHamaguchi]

Add a documentation for the getVolunteersByCohorts function. A comment on top of the function explaining parameters, return value, behaviour, AND vs OR operators, etc. is good.