Skip to content

Commit 7c19185

Browse files
kevinortiz43claude
andcommitted
web/communities: drop verbose comments from community search
Removes the descriptive comment blocks flagged in review; keeps only terse notes for the non-obvious bits (disabling MUI's filter, the sentry test mock). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 678661d commit 7c19185

2 files changed

Lines changed: 2 additions & 26 deletions

File tree

app/web/features/communities/CommunitiesPage/CommunitySearch.test.tsx

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,7 @@ jest.mock("next/router", () => ({
2121
}),
2222
}));
2323

24-
// platform/sentry pulls in @sentry/nextjs, which crashes under jsdom; the test
25-
// wrapper imports it transitively via AuthProvider, so it must be mocked.
24+
// mock required: the wrapper pulls in @sentry/nextjs via AuthProvider, which crashes under jsdom.
2625
jest.mock("platform/sentry", () => {
2726
const mockCaptureException = jest.fn();
2827
return {
@@ -52,8 +51,6 @@ const parentedCommunity = (
5251
communityId,
5352
name,
5453
slug,
55-
// Ordered root -> immediate parent -> self (the node itself is the last entry,
56-
// matching the backend's get_node_parents_recursively output).
5754
parentsList: [
5855
{
5956
community: {
@@ -160,13 +157,11 @@ describe("CommunitySearch", () => {
160157
const input = screen.getByLabelText(t("communities:search_communities"));
161158
await user.type(input, "da");
162159

163-
// Wait for the debounce to apply the filter (Berlin doesn't contain "da").
164160
await waitFor(() => {
165161
expect(screen.queryByText("Berlin")).not.toBeInTheDocument();
166162
});
167163
expect(screen.getByText("Amsterdam")).toBeInTheDocument();
168164
expect(screen.getByText("Rotterdam")).toBeInTheDocument();
169-
// Short queries must not hit the server (backend requires >= 3 chars)
170165
expect(mockSearchCommunities).not.toHaveBeenCalled();
171166
});
172167

@@ -192,9 +187,6 @@ describe("CommunitySearch", () => {
192187
});
193188

194189
it("renders server results in the returned order, not the hidden hierarchy path (issue #9129)", async () => {
195-
// Server ranks Pyongyang first. A hidden-path sort would instead put Sydney
196-
// first ("Australia" < "North Korea"), so DOM order proves we honor the
197-
// server's name-relevance ranking and never re-sort by the parent path.
198190
mockSearchCommunities.mockResolvedValue(
199191
searchRes([
200192
parentedCommunity(5, "Pyongyang", "pyongyang", "North Korea"),
@@ -208,7 +200,6 @@ describe("CommunitySearch", () => {
208200
const input = screen.getByLabelText(t("communities:search_communities"));
209201
await user.type(input, "yan");
210202

211-
// Wait for the debounce + server results to replace the browse list.
212203
await waitFor(() => {
213204
expect(screen.getAllByRole("option")).toHaveLength(2);
214205
});
@@ -282,7 +273,6 @@ describe("CommunitySearch", () => {
282273
const input = screen.getByLabelText(t("communities:search_communities"));
283274
await user.type(input, "boom");
284275

285-
// No crash; the empty-results affordance is shown instead.
286276
await waitFor(() => {
287277
expect(
288278
screen.getByRole("link", { name: "Request this community!" }),

app/web/features/communities/CommunitiesPage/CommunitySearch.tsx

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -22,21 +22,14 @@ import { useEffect, useMemo, useState } from "react";
2222
import { communityCreationFormURL, routeToCommunity } from "routes";
2323
import { listAllCommunities, searchCommunities } from "service/communities";
2424

25-
// The visible name is what we rank/filter on (issue #9129). We only read the
26-
// fields both ListAllCommunities (CommunitySummary) and SearchCommunities
27-
// (Community) responses share.
2825
type CommunityOption = Pick<
2926
Community.AsObject,
3027
"communityId" | "name" | "slug" | "parentsList"
3128
>;
3229

33-
// Backend SearchCommunities requires >= 3 chars; below that we browse/filter the
34-
// full list client-side. 400 ms debounce mirrors the confirmed #8837 decision.
3530
const MIN_SEARCH_LENGTH = 3;
3631
const COMMUNITY_SEARCH_DEBOUNCE_MS = 400;
3732

38-
// parentsList is ordered root -> ... -> immediate parent -> self (the node itself
39-
// is the last entry), so the region context is the second-to-last entry.
4033
const regionOf = (option: CommunityOption): string => {
4134
const parents = option.parentsList;
4235
if (!parents || parents.length < 2) return "";
@@ -69,8 +62,6 @@ export default function CommunitySearch() {
6962
const query = debouncedInput.trim();
7063
const isSearching = query.length >= MIN_SEARCH_LENGTH;
7164

72-
// Server-side, name-ranked, typo-tolerant search (pg_trgm). react-query caches
73-
// by query string, so re-typing/backspacing hits the cache, not the network.
7465
const { data: searchData, isLoading: searchLoading } = useQuery<
7566
SearchCommunitiesRes.AsObject,
7667
RpcError
@@ -80,7 +71,6 @@ export default function CommunitySearch() {
8071
enabled: isSearching,
8172
});
8273

83-
// Full list for the browse / short-query state (dropdown opened, < 3 chars).
8474
const { data: browseData, isLoading: browseLoading } = useQuery<
8575
ListAllCommunitiesRes.AsObject,
8676
RpcError
@@ -90,9 +80,6 @@ export default function CommunitySearch() {
9080
enabled: !isSearching,
9181
});
9282

93-
// Ranked search results are used verbatim (no client re-sort). Browse results
94-
// are filtered by visible name and ordered region -> name so region groups stay
95-
// contiguous. Neither path uses the hidden hierarchical path (the #9129 bug).
9683
const options = useMemo<CommunityOption[]>(() => {
9784
if (isSearching) {
9885
return searchData?.communitiesList ?? [];
@@ -141,8 +128,7 @@ export default function CommunitySearch() {
141128
onInputChange={handleInputChange}
142129
onChange={handleChange}
143130
getOptionLabel={(option) => option.name}
144-
// Server results are already ranked (and typo-tolerant); browse results are
145-
// pre-filtered. Disable MUI's built-in filter so it can't drop/reorder them.
131+
// don't let MUI re-filter: it would drop the typo-tolerant server results
146132
filterOptions={(x) => x}
147133
groupBy={isSearching ? undefined : (option) => regionOf(option)}
148134
renderOption={(props, option) => {

0 commit comments

Comments
 (0)