implement filter-general function #44
There are no files selected for viewing
|
Contributor
There was a problem hiding this comment. We don't need to implement the route. |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,24 @@ | ||||||
| import { filter_general } from "@/lib/api/filter_general"; | ||||||
|
|
||||||
| export async function GET(request: Request) { | ||||||
| const { searchParams } = new URL(request.url); | ||||||
|
|
||||||
| const op = searchParams.get("op"); | ||||||
| const column = searchParams.get("column"); | ||||||
| const values = searchParams.getAll("values"); | ||||||
|
|
||||||
| if (!op || !column || values.length == 0) { | ||||||
|
There was a problem hiding this comment. Use strict equality operator (===) instead of loose equality (==) to avoid type coercion issues.
Suggested change
|
||||||
| return Response.json( | ||||||
| { data: null, error: "Missing op, column, or values." }, | ||||||
| { status: 400 } | ||||||
| ); | ||||||
| } | ||||||
|
|
||||||
| const result = await filter_general(op, column, values); | ||||||
|
|
||||||
| if (result.error) { | ||||||
| return Response.json(result, { status: 400 }); | ||||||
| } | ||||||
|
|
||||||
| return Response.json(result, { status: 200 }); | ||||||
| } | ||||||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,45 @@ | ||||||||
| import { createClient } from "../client/supabase/server"; | ||||||||
|
|
||||||||
| export async function filter_general( | ||||||||
|
Contributor
There was a problem hiding this comment. Similar comment to route naming. This function should be given a more descriptive name. The current name doesn't communicate what this method does. |
||||||||
| op: string, | ||||||||
|
There was a problem hiding this comment. The 'op' parameter allows any string value but only 'AND' and 'OR' are valid. Consider using a union type (op: 'AND' | 'OR') to enforce valid values at compile time and improve type safety.
Suggested change
Contributor
There was a problem hiding this comment. (nit) Not a big fan of the name
Contributor
There was a problem hiding this comment. We should rely on the TypeScript type system to enforce the validity of the value of I suggest you created either an Enum or a literal value type for type MatchType = "any" | "all";then you can type the function argument as follows: function filter_general(op: MatchType, ...)and the ts compiler will let you know if the function contract is being violated when you pass in the wrong values for |
||||||||
| column: string, | ||||||||
|
Contributor
There was a problem hiding this comment. Same comment as for OP. This should be a type or an enum. |
||||||||
| values: string[] | ||||||||
| ) { | ||||||||
|
Comment on lines
+3
to
+7
There was a problem hiding this comment. Missing JSDoc documentation for this public API function. Should include descriptions of parameters (op, column, values), the return type, and examples of valid operations. See getExample.ts comments for reference style.
Contributor
There was a problem hiding this comment. The return value of this function should be typed. |
||||||||
| const valid_columns = [ | ||||||||
| "name_org", | ||||||||
| "pseudonym", | ||||||||
| "pronouns", | ||||||||
| "email", | ||||||||
| "phone", | ||||||||
| "position", | ||||||||
| "opt_in_communication", | ||||||||
| ]; | ||||||||
|
Comment on lines
+8
to
+16
Contributor
There was a problem hiding this comment. We should get rid of this in favor of introducing a type or an enum for valid columns in a centralized helper in this codebase. |
||||||||
|
|
||||||||
| if (!valid_columns.includes(column)) { | ||||||||
| return { data: null, error: "Invalid column name" }; | ||||||||
| } | ||||||||
|
|
||||||||
| const client = await createClient(); | ||||||||
|
Contributor
There was a problem hiding this comment. This should be moved to be after all of the validation checks. This way we don't have to create a client before returning due to values being empty. |
||||||||
|
|
||||||||
| if (values.length == 0) { | ||||||||
|
There was a problem hiding this comment. Use strict equality operator (===) instead of loose equality (==) to avoid type coercion issues.
Suggested change
|
||||||||
| return { data: null, error: "No values provided." }; | ||||||||
| } | ||||||||
|
Comment on lines
+24
to
+26
There was a problem hiding this comment. The values.length check at line 24 is redundant since the route handler already validates this condition at line 10. This duplicate validation should be removed or the route handler's check should be removed to avoid redundancy.
Suggested change
|
||||||||
|
|
||||||||
| if (op == "AND") { | ||||||||
|
There was a problem hiding this comment. Use strict equality operator (===) instead of loose equality (==) to avoid type coercion issues. |
||||||||
| const uniqueValues = [...new Set(values)]; | ||||||||
|
|
||||||||
| if (uniqueValues.length > 1) { | ||||||||
| return { data: [], error: null }; // Return empty result if there are multiple unique values | ||||||||
| } | ||||||||
|
|
||||||||
| const value = uniqueValues[0]; | ||||||||
|
|
||||||||
| const result = await client.from("Volunteers").select().eq(column, value); | ||||||||
| return result; | ||||||||
| } | ||||||||
| if (op == "OR") { | ||||||||
|
There was a problem hiding this comment. Use strict equality operator (===) instead of loose equality (==) to avoid type coercion issues.
Suggested change
|
||||||||
| const result = await client.from("Volunteers").select().in(column, values); | ||||||||
| return result; | ||||||||
| } | ||||||||
| return { data: null, error: "Invalid operation." }; | ||||||||
| } | ||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the name of the directory
filter-generalis too generic. We want to convey more information about what this route does by giving it a more descriptive name.