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

✨ Add search page #207

Merged
merged 1 commit into from
Oct 2, 2024
Merged

✨ Add search page #207

merged 1 commit into from
Oct 2, 2024

Conversation

foysalit
Copy link
Contributor

@foysalit foysalit commented Oct 1, 2024

This PR adds a dedicated page to show top and latest posts for a search keyword which surfaces the same content as the app. This helps moderators to quickly find and proactively action content without having to go through the app.

Screenshot 2024-10-01 at 20 10 14

@arcalinea arcalinea temporarily deployed to search-content - ozone-staging PR #207 October 1, 2024 18:11 — with Render Destroyed
Copy link
Contributor

@matthieusieben matthieusieben left a comment

Choose a reason for hiding this comment

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

Looks pretty straight forward. I do have a small concern about caching of search results.

Comment on lines +16 to +29
const searchInfo = useInfiniteQuery({
queryKey: ['searchContent', { term, section }],
enabled: !!term,
queryFn: async ({ pageParam }) => {
const { data } = await labelerAgent.app.bsky.feed.searchPosts({
q: term,
limit: 30,
sort: section,
cursor: pageParam,
})
return data
},
getNextPageParam: (lastPage) => lastPage.cursor,
})
Copy link
Contributor

@matthieusieben matthieusieben Oct 2, 2024

Choose a reason for hiding this comment

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

I fear that this query could have two effects:

  1. a lot of entries end up being cached, significantly increasing the memory usage
  2. there isn't any mechanism to clear that cache so the user could end-up seeing stale results when trying to search for newer posts.

Is there a way to invalidate all searchContent cache entries, for example when the component gets destroyed ?

Alternatively, maybe a mutation would be more appropriate here ? (The browser's HTTP cache would provide the short term caching)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good point but I think this is actually what we want. when investigating search results, I don't think a 5m delay in seeing realtime content is a big issue for mods and in fact, we wouldn't want the results to change while they are scanning a specific page for a keyword so overall, the caching seems to be a good idea.
keep in mind that at any given moment, a mod will probably scan for only a few keywords so keyword specific caches can be kept around without overwhelming memory usage IMO.

Copy link
Contributor

@matthieusieben matthieusieben left a comment

Choose a reason for hiding this comment

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

If the caching ends-up being problematic, we can always adapt later ;-)

@foysalit foysalit merged commit f3faf0e into main Oct 2, 2024
3 checks passed
@matthieusieben matthieusieben deleted the search-content branch November 15, 2024 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants