listerv check and reccomendation#4
Conversation
benkoppe
left a comment
benkoppe
left a comment
There was a problem hiding this comment.
I left some comments, but I'm mostly curious about how everything work look & work with an extension as opposed to a typical project. All of this will probably look the same, I'm guessing? I think convex will also work perfectly. I also appreciate the lack of React useEffects, we should definitely avoid it as much as possible.
| @@ -0,0 +1,40 @@ | |||
| import React from "react"; | |||
There was a problem hiding this comment.
Is this import required when it's unused? Genuinely asking BTW I don't use react that much haha
| @@ -0,0 +1,74 @@ | |||
| import React from "react"; | |||
|
|
||
| export default function App() { | ||
| //Example emails | ||
| const emails = [ |
There was a problem hiding this comment.
We can probably already start putting mock data in the convex similar to what they do in the quickstart if we want
| "friend@gmail.com" | ||
| ]; | ||
| //Will need to be modified depending on our database schema | ||
| function detectListserv(emails) { |
There was a problem hiding this comment.
Do all functions need to be defined with useMemo in react? Or is that an anti-pattern.
| function detectListserv(emails) { | ||
| const results = []; | ||
|
|
||
| for (const email of emails) { |
There was a problem hiding this comment.
Regardless of my above comment, when our state is defined with convex in the future this will probably need to be derived state with useMemo.
anchen9
left a comment
anchen9
left a comment
There was a problem hiding this comment.
Great work overall! I can really see how the functions are coming together!
|
|
||
| const results = detectListserv(emails); | ||
|
|
||
| return ( |
There was a problem hiding this comment.
Hi Nikhill! Great work on researching the recommendation algorithms and listserv detection so far! For the future, it's good practice to try and keep the frontend and backend responsibilities more separate (i.e. only functions in the convex folder)! Of course just for prototyping and quick testing it's fine, but I think testing with just json input and printing the output to terminal is a good quick alternative just to check if your function is producing the results you want!
| const username = email.split("@")[0]; | ||
|
|
||
| if (username.includes("-l")) { | ||
| results.push("LISTSERV"); |
There was a problem hiding this comment.
I wonder if pushing a boolean to results for detecting isListserv might be more maintainable for the future!
benkoppe
force-pushed
the
listserv-check/Nikhill
branch
from
3805482 to
3f232a1
Compare
3 participants
Summary
This PR provides two different files. One is for detecting listserv emails indicated by a "-l" in their email signature and the other is an algorithm to reccomend specific events to different users based on shared tags between the events and the user's profile.
This pull request is the first step towards implementing the full listserv reccomendation process.
Remaining TODOs:
Depends on #{number of PR}
Test Plan
Notes
Breaking Changes