Skip to content
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

Merged
merged 17 commits into from
Nov 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions apps/web/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
"@trpc/react-query": "11.0.0-rc.441",
"@trpc/server": "11.0.0-rc.441",
"dayjs": "^1.11.13",
"fuse.js": "^7.0.0",
"geist": "^1.3.0",
"lucide-react": "^0.436.0",
"next": "^14.2.4",
Expand Down
10 changes: 9 additions & 1 deletion apps/web/src/app/(pages)/(dashboard)/roles/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ export default function Roles({
searchParams,
}: {
searchParams?: {
search?: string;
cycle?: WorkTermType;
Copy link
Contributor

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 and term now since you renamed them in the new search params.

Copy link
Collaborator

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 and workEnvironment in favor of cycle and term but I might be wrong. This might an artifact of a merge. @suxls would recommend removing workTerm and workEnvironment from the searchParams object and testing.

term?: WorkEnvironmentType;
};
Expand Down Expand Up @@ -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(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

in general we want to avoid this kind of useEffect, but I'm assuming there was some kind of bug due to the fact that reviews are updated based on the child SearchFilter component?

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">
Expand Down
13 changes: 11 additions & 2 deletions apps/web/src/app/_components/search/search-filter.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,20 @@ const formSchema = z.object({

export type SearchFilterFormType = typeof formSchema;

export default function SearchFilter() {
interface SearchFilterProps {
Copy link
Contributor

Choose a reason for hiding this comment

The 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 ❤️ 🙈

Copy link
Contributor

Choose a reason for hiding this comment

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

i agree with this to make it easier for everyone to read

Copy link
Collaborator

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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 ?? "",
},
});

Expand Down
18 changes: 16 additions & 2 deletions packages/api/src/router/review.ts
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";
Expand All @@ -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(),
Expand All @@ -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 = {
Copy link
Collaborator

Choose a reason for hiding this comment

The 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

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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
Expand Down
13 changes: 7 additions & 6 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.