From 1274ad9b461af70029c1e1a17b12b69a63466146 Mon Sep 17 00:00:00 2001 From: Myx Date: Tue, 9 Jun 2026 22:00:06 +0200 Subject: [PATCH] fix: search --- .agents/skills/playwright-e2e/SKILL.md | 2 +- agents-docs/LESSONS.md | 14 ++++ docs-site/docs/developer/dom-structure.md | 10 ++- .../developer/llm-plugin-builder-guide.md | 2 +- .../app/domains/server-directory/README.md | 2 +- .../server-directory-api.service.spec.ts | 77 +++++++++++++++++++ .../services/server-directory-api.service.ts | 35 +++++++-- .../servers-rail/servers-rail.component.html | 10 +-- .../servers-rail/servers-rail.component.ts | 8 +- 9 files changed, 141 insertions(+), 19 deletions(-) diff --git a/.agents/skills/playwright-e2e/SKILL.md b/.agents/skills/playwright-e2e/SKILL.md index 5bbc233..bc3d329 100644 --- a/.agents/skills/playwright-e2e/SKILL.md +++ b/.agents/skills/playwright-e2e/SKILL.md @@ -195,7 +195,7 @@ export class LoginPage { | ------------------- | ------------------ | ----------------------- | | `/login` | `LoginPage` | `LoginComponent` | | `/register` | `RegisterPage` | `RegisterComponent` | -| `/search` | `ServerSearchPage` | `ServerSearchComponent` | +| `/servers` | `FindServersPage` | `FindServersComponent` | | `/room/:roomId` | `ChatRoomPage` | `ChatRoomComponent` | | `/settings` | `SettingsPage` | `SettingsComponent` | | `/invite/:inviteId` | `InvitePage` | `InviteComponent` | diff --git a/agents-docs/LESSONS.md b/agents-docs/LESSONS.md index fd06f0c..fe0ef46 100644 --- a/agents-docs/LESSONS.md +++ b/agents-docs/LESSONS.md @@ -25,6 +25,20 @@ Durable rules for AI agents working on this project. Read this file at session s ## Lessons +### When renaming an Angular route, sweep every navigate/url-match/doc reference [routing] + +- **Trigger:** the find-servers route was renamed `/search` → `/servers` in `app.routes.ts`, but `servers-rail.component.ts` still called `router.navigate(['/search'])` (leave-server) and matched `startsWith('/search')` for the user-bar visibility signal, throwing `NG04002: 'search'` on leave and never showing the user-bar on the discovery page. +- **Rule:** after changing a `path:` in `app.routes.ts`, grep the whole repo for the old literal (`/search`) across `*.ts`/`*.html` (router calls, `startsWith`/url-match signals) and docs (`docs-site`, `.agents/skills/playwright-e2e/SKILL.md` route tables, domain READMEs) and update them all in the same change. +- **Why:** `router.navigate` to a non-existent path raises `NG04002` and aborts navigation, and stale `startsWith` matches silently break route-derived UI state — neither is caught by the build (string literals) and there was no `servers-rail` spec to catch it. +- **Example:** fixed `isOnServers`/`router.navigate(['/servers'])` in `servers-rail.component.{ts,html}`; canonical post-leave/discovery route is `/servers` (`FindServersComponent`), matching `DashboardComponent`'s `router.navigate(['/servers'])`. + +### Server discovery (featured/trending) must fan out across all online endpoints like search [server-directory] + +- **Trigger:** the `/servers` (find-servers) page showed no servers by default but found them as soon as the user typed in the search box. Discovery (`getDiscoveryServers`) queried only the *active* endpoint via `getApiBaseUrl()`, and when that endpoint is a discovery-unsupported production host (`signal.toju.app` / `signal-sweden.toju.app` in `DISCOVERY_UNSUPPORTED_HOSTS`) it short-circuited to `[]`; search meanwhile fans out across every online endpoint, so typing surfaced the servers that lived on other endpoints (e.g. localhost). +- **Rule:** make `getFeaturedServers`/`getTrendingServers` fan out across `getSearchableEndpoints()` with `forkJoin` + `deduplicateById` (mirroring all-endpoint search), and apply the `endpointSupportsServerDiscovery` gate *per endpoint* (skip → `[]`) instead of short-circuiting the whole request on the active endpoint. +- **Why:** the empty-query find-servers view renders discovery sections, not search results, so any divergence between discovery's endpoint set and search's endpoint set makes the default view look broken while search works. +- **Example:** `getDiscoveryServers` + `fetchDiscoveryFromEndpoint` in `server-directory-api.service.ts`; verified the live server returns 12 featured/12 trending while the active production host is gated out client-side. + ### Server registration needs `ownerPublicKey: oderId || id`, and must not be fire-and-forget [server-directory] [rooms] - **Trigger:** creating a server appeared to work (the creator landed in the room view) but the server didn't exist on the backend — invite-link creation and search both 404'd. `createRoom$` sent `ownerPublicKey: currentUser.oderId` with no fallback; on restored sessions `oderId` can be falsy (identify still works because it falls back to `id`), so `POST /api/servers` returned `400 Missing required fields`, and the `.subscribe()` swallowed the error while `createRoomSuccess` fired regardless. diff --git a/docs-site/docs/developer/dom-structure.md b/docs-site/docs/developer/dom-structure.md index 172819d..31e30a1 100644 --- a/docs-site/docs/developer/dom-structure.md +++ b/docs-site/docs/developer/dom-structure.md @@ -10,14 +10,20 @@ This page maps the app routes and important DOM areas. It is useful for plugin a | Route | Component | Purpose | | ---------------------------- | ------------------------- | --------------------------------------------------------------------- | -| `/` | Redirect | Redirects to `/search`. | +| `/` | Redirect | Redirects to `/dashboard`. | | `/login` | `LoginComponent` | User login. | | `/register` | `RegisterComponent` | User registration. | | `/invite/:inviteId` | `InviteComponent` | Resolve and accept invite links. | -| `/search` | `ServerSearchComponent` | Search and join servers. | +| `/dashboard` | `DashboardComponent` | Landing dashboard after sign-in. | +| `/people` | `FindPeopleComponent` | Discover and start direct messages with people. | +| `/servers` | `FindServersComponent` | Search, discover, and join servers. | +| `/create-server` | `CreateServerComponent` | Create a new server. | | `/room/:roomId` | `ChatRoomComponent` | Main server page with text, voice, members, and plugin panels. | | `/dm` | `DmWorkspaceComponent` | Direct-message workspace. | | `/dm/:conversationId` | `DmWorkspaceComponent` | A selected direct-message conversation. | +| `/pm` | `DmWorkspaceComponent` | Private-message workspace (alias of the DM workspace). | +| `/pm/:conversationId` | `DmWorkspaceComponent` | A selected private-message conversation. | +| `/call/:callId` | `PrivateCallComponent` | Active private (1:1) call. | | `/settings` | `SettingsComponent` | App, voice, server, plugin, desktop, theme, local API settings. | | `/plugin-store` | `PluginStoreComponent` | Browse plugin sources and install/update plugins. | | `/plugins/:pluginId/:pageId` | `PluginPageHostComponent` | Host for plugin app pages registered with `api.ui.registerAppPage()`. | diff --git a/docs-site/docs/developer/llm-plugin-builder-guide.md b/docs-site/docs/developer/llm-plugin-builder-guide.md index 1ebf1dd..3b8dda3 100644 --- a/docs-site/docs/developer/llm-plugin-builder-guide.md +++ b/docs-site/docs/developer/llm-plugin-builder-guide.md @@ -124,7 +124,7 @@ Important routes: | Route | Purpose | | ------------------------------- | ------------------------------------------------------------------- | -| `/search` | Search and join servers. | +| `/servers` | Search, discover, and join servers. | | `/room/:roomId` | Main server workspace with text, voice, members, and plugin panels. | | `/dm` and `/dm/:conversationId` | Direct-message workspace. | | `/settings` | App, voice, server, plugin, desktop, theme, and local API settings. | diff --git a/toju-app/src/app/domains/server-directory/README.md b/toju-app/src/app/domains/server-directory/README.md index e91947a..9ae97f3 100644 --- a/toju-app/src/app/domains/server-directory/README.md +++ b/toju-app/src/app/domains/server-directory/README.md @@ -160,7 +160,7 @@ Beyond free-text search, the directory exposes curated discovery lists that powe - `ServerDirectoryFacade.getFeaturedServers()` → `GET /api/servers/featured` - `ServerDirectoryFacade.getTrendingServers()` → `GET /api/servers/trending` -Both pass through `ServerDirectoryService` to `ServerDirectoryApiService.getFeaturedServers()` / `getTrendingServers()`, which share a private `getDiscoveryServers(path)` HTTP helper and normalise results into `ServerInfo[]` exactly like search. The server ranks featured servers (stable curation) and trending servers (recent activity) via `server-ranking.util.ts`; each route caps results at 50 (`parseDiscoveryLimit`). The discovery routes are registered before the parameterised `/:id` route so `featured`/`trending` are not captured as server IDs. +Both pass through `ServerDirectoryService` to `ServerDirectoryApiService.getFeaturedServers()` / `getTrendingServers()`, which share a private `getDiscoveryServers(path)` HTTP helper and normalise results into `ServerInfo[]` exactly like search. Discovery **fans out across every online endpoint** (`getSearchableEndpoints()` + `forkJoin`, deduplicated by ID), mirroring all-endpoint search — querying only the active endpoint made the default `/servers` view appear empty whenever that endpoint was a discovery-unsupported production host (`endpointSupportsServerDiscovery`), even though plenty of servers existed on other online endpoints. Each endpoint that is in `DISCOVERY_UNSUPPORTED_HOSTS` is skipped (returns `[]`) per-endpoint rather than short-circuiting the whole request. The server ranks featured servers (stable curation) and trending servers (recent activity) via `server-ranking.util.ts`; each route caps results at 50 (`parseDiscoveryLimit`). The discovery routes are registered before the parameterised `/:id` route so `featured`/`trending` are not captured as server IDs. `FindServersComponent` (`/servers`) composes these into discovery sections — **Recently active** (the user's saved rooms, capped at 6), **Featured servers**, and **Trending** — and renders them through the reusable `app-server-browser`. `DashboardComponent` (`/dashboard`) uses the same facade methods for its quick search results. diff --git a/toju-app/src/app/domains/server-directory/infrastructure/services/server-directory-api.service.spec.ts b/toju-app/src/app/domains/server-directory/infrastructure/services/server-directory-api.service.spec.ts index 4c79cda..225e0e6 100644 --- a/toju-app/src/app/domains/server-directory/infrastructure/services/server-directory-api.service.spec.ts +++ b/toju-app/src/app/domains/server-directory/infrastructure/services/server-directory-api.service.spec.ts @@ -80,3 +80,80 @@ describe('ServerDirectoryApiService discovery endpoints', () => { expect(result).toEqual([]); }); }); + +function createMultiEndpointHarness( + endpoints: { id: string; name: string; url: string; status: string }[], + getImpl: (url: string) => unknown +) { + const get = vi.fn((url: string) => of(getImpl(url) ?? { servers: [], total: 0 })); + const http = { get } as unknown as HttpClient; + const endpointState = { + activeServer: () => endpoints[0], + activeServers: () => endpoints, + servers: () => [], + resolveCanonicalEndpoint: (endpoint: unknown) => endpoint ?? null, + findServerByUrl: () => null, + sanitiseUrl: (value: string) => value + } as unknown as ServerEndpointStateService; + const injector = Injector.create({ + providers: [ + ServerDirectoryApiService, + { provide: HttpClient, useValue: http }, + { provide: ServerEndpointStateService, useValue: endpointState } + ] + }); + const service = runInInjectionContext(injector, () => injector.get(ServerDirectoryApiService)); + + return { service, get }; +} + +describe('ServerDirectoryApiService discovery fan-out', () => { + const endpoints = [ + { id: 'ep-1', name: 'Local', url: 'https://local.test', status: 'online' }, + { id: 'ep-2', name: 'Other', url: 'https://other.test', status: 'online' }, + { id: 'ep-3', name: 'Prod', url: 'https://signal.toju.app', status: 'online' } + ]; + + it('aggregates discovery results across all online endpoints', async () => { + const { service, get } = createMultiEndpointHarness(endpoints, (url) => { + if (url.startsWith('https://local.test')) + return { servers: [{ id: 's1', name: 'Alpha' }], total: 1 }; + + if (url.startsWith('https://other.test')) + return { servers: [{ id: 's2', name: 'Beta' }], total: 1 }; + + return { servers: [], total: 0 }; + }); + const result = await firstValueFrom(service.getFeaturedServers()); + + expect(result.map((server) => server.id).sort()).toEqual(['s1', 's2']); + const calledUrls = get.mock.calls.map((call) => call[0] as string); + + expect(calledUrls).toContain('https://local.test/api/servers/featured'); + expect(calledUrls).toContain('https://other.test/api/servers/featured'); + }); + + it('does not query discovery-unsupported hosts but still returns the supported endpoints', async () => { + const { service, get } = createMultiEndpointHarness(endpoints, (url) => { + if (url.startsWith('https://local.test')) + return { servers: [{ id: 's1', name: 'Alpha' }], total: 1 }; + + return { servers: [], total: 0 }; + }); + const result = await firstValueFrom(service.getTrendingServers()); + + expect(result.map((server) => server.id)).toEqual(['s1']); + const calledUrls = get.mock.calls.map((call) => call[0] as string); + + expect(calledUrls.some((url) => url.includes('signal.toju.app'))).toBe(false); + }); + + it('deduplicates the same server returned by multiple endpoints', async () => { + const { service } = createMultiEndpointHarness(endpoints, () => + ({ servers: [{ id: 'dup', name: 'Shared' }], total: 1 }) + ); + const result = await firstValueFrom(service.getFeaturedServers()); + + expect(result.map((server) => server.id)).toEqual(['dup']); + }); +}); diff --git a/toju-app/src/app/domains/server-directory/infrastructure/services/server-directory-api.service.ts b/toju-app/src/app/domains/server-directory/infrastructure/services/server-directory-api.service.ts index a9370ae..a8873de 100644 --- a/toju-app/src/app/domains/server-directory/infrastructure/services/server-directory-api.service.ts +++ b/toju-app/src/app/domains/server-directory/infrastructure/services/server-directory-api.service.ts @@ -297,18 +297,43 @@ export class ServerDirectoryApiService { } private getDiscoveryServers(kind: 'featured' | 'trending', limit?: number): Observable { - const baseUrl = this.resolveBaseServerUrl(); + const params = typeof limit === 'number' ? new HttpParams().set('limit', String(limit)) : undefined; + const onlineEndpoints = this.getSearchableEndpoints(); + // Fan discovery out across every online endpoint (mirroring search) so the + // default find-servers view isn't empty just because the *active* endpoint + // is a discovery-unsupported production host. Querying only the active + // endpoint made servers invisible until the user typed a search query. + if (onlineEndpoints.length === 0) { + return this.fetchDiscoveryFromEndpoint( + kind, + this.resolveBaseServerUrl(), + this.endpointState.activeServer(), + params + ); + } + + return forkJoin( + onlineEndpoints.map((endpoint) => + this.fetchDiscoveryFromEndpoint(kind, endpoint.url, endpoint, params) + ) + ).pipe(map((resultArrays) => this.deduplicateById(resultArrays.flat()))); + } + + private fetchDiscoveryFromEndpoint( + kind: 'featured' | 'trending', + baseUrl: string, + source: ServerEndpoint | null | undefined, + params?: HttpParams + ): Observable { if (!endpointSupportsServerDiscovery(baseUrl)) { return of([]); } - const params = typeof limit === 'number' ? new HttpParams().set('limit', String(limit)) : undefined; - return this.http - .get<{ servers: ServerInfo[]; total: number }>(`${this.getApiBaseUrl()}/servers/${kind}`, params ? { params } : {}) + .get<{ servers: ServerInfo[]; total: number }>(`${baseUrl}/api/servers/${kind}`, params ? { params } : {}) .pipe( - map((response) => this.normalizeServerList(response, this.endpointState.activeServer())), + map((response) => this.normalizeServerList(response, source ?? null)), catchError((error) => { console.error(`Failed to get ${kind} servers:`, error); return of([]); diff --git a/toju-app/src/app/features/servers/servers-rail/servers-rail.component.html b/toju-app/src/app/features/servers/servers-rail/servers-rail.component.html index 4e79efa..5b5a481 100644 --- a/toju-app/src/app/features/servers/servers-rail/servers-rail.component.html +++ b/toju-app/src/app/features/servers/servers-rail/servers-rail.component.html @@ -165,11 +165,11 @@
diff --git a/toju-app/src/app/features/servers/servers-rail/servers-rail.component.ts b/toju-app/src/app/features/servers/servers-rail/servers-rail.component.ts index bc5b2ff..2c040f9 100644 --- a/toju-app/src/app/features/servers/servers-rail/servers-rail.component.ts +++ b/toju-app/src/app/features/servers/servers-rail/servers-rail.component.ts @@ -100,12 +100,12 @@ export class ServersRailComponent { currentUser = this.store.selectSignal(selectCurrentUser); onlineUsers = this.store.selectSignal(selectOnlineUsers); bannedRoomLookup = signal>({}); - isOnSearch = toSignal( + isOnServers = toSignal( this.router.events.pipe( filter((navigationEvent): navigationEvent is NavigationEnd => navigationEvent instanceof NavigationEnd), - map((navigationEvent) => navigationEvent.urlAfterRedirects.startsWith('/search')) + map((navigationEvent) => navigationEvent.urlAfterRedirects.startsWith('/servers')) ), - { initialValue: this.router.url.startsWith('/search') } + { initialValue: this.router.url.startsWith('/servers') } ); isOnDirectMessage = toSignal( this.router.events.pipe( @@ -393,7 +393,7 @@ export class ServersRailComponent { if (isCurrentRoom) { this.optimisticSelectedRoomId.set(null); - this.router.navigate(['/search']); + this.router.navigate(['/servers']); } this.showLeaveConfirm.set(false);