-
Notifications
You must be signed in to change notification settings - Fork 0
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.
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
Implement fuse #91
Implement fuse #91
Changes from all commits
cc2b7e3
715d121
eef2133
65d72fd
7f4ba53
dcfaa20
7d32730
7f8a54b
4ac1a9a
447904f
127f49a
f206a25
7967c9b
39d99ba
b0dd465
61f0e80
31668f4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,6 +21,7 @@ export default function Roles({ | |
searchParams, | ||
}: { | ||
searchParams?: { | ||
search?: string; | ||
cycle?: WorkTermType; | ||
term?: WorkEnvironmentType; | ||
}; | ||
|
@@ -67,16 +68,23 @@ export default function Roles({ | |
]); | ||
|
||
const reviews = api.review.list.useQuery({ | ||
search: searchParams?.search, | ||
options: validationResult.success ? validationResult.data : {}, | ||
}); | ||
|
||
const [selectedReview, setSelectedReview] = useState<ReviewType | undefined>( | ||
reviews.data ? reviews.data[0] : undefined, | ||
); | ||
|
||
useEffect(() => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. in general we want to avoid this kind of |
||
if (reviews.data) { | ||
setSelectedReview(reviews.data[0]); | ||
} | ||
}, [reviews.data]); | ||
|
||
return ( | ||
<> | ||
<SearchFilter /> | ||
<SearchFilter search={searchParams?.search} /> | ||
{reviews.data && ( | ||
<div className="mb-8 grid h-[70dvh] w-4/5 grid-cols-5 gap-4 lg:w-3/4"> | ||
<div className="col-span-2 gap-3 overflow-scroll pr-4"> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,11 +16,20 @@ const formSchema = z.object({ | |
|
||
export type SearchFilterFormType = typeof formSchema; | ||
|
||
export default function SearchFilter() { | ||
interface SearchFilterProps { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i know this is so my inner fundies 1 ta but what are our thoughts on starting to us js doc to lightly document the functions. Espesh since dev turnover may be on the rise (oml we are getting old). Up to rishi hes TL i'm just throwing this out there ❤️ 🙈 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i agree with this to make it easier for everyone to read There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. YES pls document with JSDoc 🙏🏻 I remember we were doing it for a bit at the start of Spring but that fizzled out lol. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yessir |
||
search?: string; | ||
} | ||
|
||
/** | ||
* Handles searching logic, updates the search param base on user search and passes the text to backend with fuzzy searching. | ||
* @param param0 user input text that's passed to the fuzzy search | ||
* @returns the search bar with the user inputted text | ||
*/ | ||
export default function SearchFilter({ search }: SearchFilterProps) { | ||
const form = useForm<z.infer<typeof formSchema>>({ | ||
resolver: zodResolver(formSchema), | ||
defaultValues: { | ||
searchText: "", | ||
searchText: search ?? "", | ||
}, | ||
}); | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
import type { TRPCRouterRecord } from "@trpc/server"; | ||
import Fuse from "fuse.js"; | ||
import { z } from "zod"; | ||
|
||
import { and, desc, eq } from "@cooper/db"; | ||
|
@@ -10,6 +11,7 @@ export const reviewRouter = { | |
list: publicProcedure | ||
.input( | ||
z.object({ | ||
search: z.string().optional(), | ||
options: z | ||
.object({ | ||
cycle: z.enum(["SPRING", "FALL", "SUMMER"]).optional(), | ||
|
@@ -18,18 +20,30 @@ export const reviewRouter = { | |
.optional(), | ||
}), | ||
) | ||
.query(({ ctx, input }) => { | ||
.query(async ({ ctx, input }) => { | ||
const { options } = input; | ||
|
||
const conditions = [ | ||
options?.cycle && eq(Review.workTerm, options.cycle), | ||
options?.term && eq(Review.workEnvironment, options.term), | ||
].filter(Boolean); | ||
|
||
return ctx.db.query.Review.findMany({ | ||
const reviews = await ctx.db.query.Review.findMany({ | ||
orderBy: desc(Review.id), | ||
where: conditions.length > 0 ? and(...conditions) : undefined, | ||
}); | ||
|
||
if (!input.search) { | ||
return reviews; | ||
} | ||
|
||
const fuseOptions = { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we maybe abstract the fuzzy searching out to a helper? I might implement a custom linting rule that makes sure that every function should be 4 or less lines of code 😡 /s There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. will implement that on a separate branch |
||
keys: ["reviewHeadline", "textReview", "location"], | ||
}; | ||
|
||
const fuse = new Fuse(reviews, fuseOptions); | ||
|
||
return fuse.search(input.search).map((result) => result.item); | ||
}), | ||
|
||
getByRole: publicProcedure | ||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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.
it looks like we want to get rid of
cycle
andterm
now since you renamed them in the new search params.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.
Yeah I think we got rid of
workTerm
andworkEnvironment
in favor ofcycle
andterm
but I might be wrong. This might an artifact of a merge. @suxls would recommend removingworkTerm
andworkEnvironment
from thesearchParams
object and testing.