fix: Bug - Fresh users have the server list in dashboard completely empty until anything searched
This commit is contained in:
@@ -53,12 +53,12 @@ Durable rules for AI agents working on this project. Read this file at session s
|
||||
- **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]
|
||||
### Server discovery must fan out across all endpoints and self-heal on 404 — never hardcode a host capability blocklist [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.
|
||||
- **Trigger:** the dashboard "Popular Servers" and `/servers` discovery view were empty for fresh users until they typed a search. The first fix added a static `DISCOVERY_UNSUPPORTED_HOSTS` blocklist (`signal.toju.app` / `signal-sweden.toju.app`) that short-circuited discovery to `[]`; the production hosts later shipped the `/featured` + `/trending` routes (verified `curl` → 200 with servers), so the stale blocklist kept blocking exactly the default endpoints a fresh account has while ungated search still surfaced them.
|
||||
- **Rule:** discovery (`getFeaturedServers`/`getTrendingServers`) must fan out across `getSearchableEndpoints()` with `forkJoin` + `deduplicateById` (mirroring all-endpoint search), and detect capability *at runtime* — on a `404` from `/api/servers/{featured,trending}`, fall back per-endpoint to the public `GET /api/servers` listing (`fetchPublicServerListForDiscovery`) instead of returning `[]`. Do not maintain a hardcoded list of hosts that "don't support" a route; it goes stale silently and the build can't catch it.
|
||||
- **Why:** legacy servers resolve `/featured` as `/servers/:id` and answer 404, so a 404→fallback keeps the default view populated everywhere without a blocklist; the empty-query view renders discovery sections (not search results), so any divergence between discovery and search makes it look broken while search works.
|
||||
- **Example:** `fetchDiscoveryFromEndpoint` + `fetchPublicServerListForDiscovery` in `server-directory-api.service.ts`; `e2e/tests/servers/server-discovery-default.spec.ts` proves a fresh account sees Popular Servers without searching AND that route-intercepting `/featured`+`/trending` to 404 still populates it via the fallback.
|
||||
|
||||
### Server registration needs `ownerPublicKey: oderId || id`, and must not be fire-and-forget [server-directory] [rooms]
|
||||
|
||||
|
||||
98
e2e/tests/servers/server-discovery-default.spec.ts
Normal file
98
e2e/tests/servers/server-discovery-default.spec.ts
Normal file
@@ -0,0 +1,98 @@
|
||||
import { test, expect } from '../../fixtures/multi-client';
|
||||
import { type Client } from '../../fixtures/multi-client';
|
||||
import { RegisterPage } from '../../pages/register.page';
|
||||
import { ServerSearchPage } from '../../pages/server-search.page';
|
||||
import { dashboardSearchInput, expectDashboardReady } from '../../helpers/dashboard';
|
||||
import { MULTI_DEVICE_PASSWORD, uniqueMultiDeviceName } from '../../helpers/multi-device-session';
|
||||
|
||||
/**
|
||||
* Regression coverage for: "Fresh users have the server list in dashboard
|
||||
* completely empty until anything searched."
|
||||
*
|
||||
* The directory exposes a curated discovery view (featured/trending) that must
|
||||
* populate the dashboard "Popular Servers" panel and the /servers page without
|
||||
* the user typing a search query. A stale client-side host blocklist used to
|
||||
* short-circuit discovery to [] for the default production endpoints, so servers
|
||||
* only appeared once a search ran. These tests prove the default view is
|
||||
* populated, and that discovery self-heals when an endpoint lacks the
|
||||
* featured/trending routes (older signal servers answer them with 404).
|
||||
*/
|
||||
async function createPublicServer(client: Client, username: string, serverName: string): Promise<void> {
|
||||
const register = new RegisterPage(client.page);
|
||||
|
||||
await register.goto();
|
||||
await register.register(username, 'Discovery Host', MULTI_DEVICE_PASSWORD);
|
||||
await expect(client.page).toHaveURL(/\/dashboard/, { timeout: 15_000 });
|
||||
|
||||
const search = new ServerSearchPage(client.page);
|
||||
|
||||
await search.createServer(serverName, { description: 'Public discovery server' });
|
||||
await expect(client.page).toHaveURL(/\/room\//, { timeout: 15_000 });
|
||||
}
|
||||
|
||||
function popularServersPanel(client: Client) {
|
||||
return client.page.locator('div.rounded-xl', { hasText: 'Popular Servers' });
|
||||
}
|
||||
|
||||
test.describe('Server discovery default view', () => {
|
||||
test.describe.configure({ timeout: 120_000, retries: 1 });
|
||||
|
||||
test('a fresh account sees public servers in Popular Servers without searching', async ({ createClient }) => {
|
||||
const suffix = uniqueMultiDeviceName('discovery-default');
|
||||
const serverName = `Discovery Default ${suffix}`;
|
||||
const host = await createClient();
|
||||
const visitor = await createClient();
|
||||
|
||||
await test.step('host registers and publishes a public server', async () => {
|
||||
await createPublicServer(host, `host_${suffix}`, serverName);
|
||||
});
|
||||
|
||||
await test.step('a brand-new account registers', async () => {
|
||||
const register = new RegisterPage(visitor.page);
|
||||
|
||||
await register.goto();
|
||||
await register.register(`visitor_${suffix}`, 'Discovery Visitor', MULTI_DEVICE_PASSWORD);
|
||||
await expectDashboardReady(visitor.page);
|
||||
});
|
||||
|
||||
await test.step('Popular Servers lists the public server with no search query entered', async () => {
|
||||
await expect(dashboardSearchInput(visitor.page)).toHaveValue('');
|
||||
await expect(popularServersPanel(visitor).getByText(serverName)).toBeVisible({ timeout: 30_000 });
|
||||
});
|
||||
});
|
||||
|
||||
test('discovery falls back to the public listing when featured/trending routes 404', async ({ createClient }) => {
|
||||
const suffix = uniqueMultiDeviceName('discovery-fallback');
|
||||
const serverName = `Discovery Fallback ${suffix}`;
|
||||
const host = await createClient();
|
||||
const visitor = await createClient();
|
||||
|
||||
await test.step('host registers and publishes a public server', async () => {
|
||||
await createPublicServer(host, `host_${suffix}`, serverName);
|
||||
});
|
||||
|
||||
await test.step('simulate a legacy signal server without featured/trending routes', async () => {
|
||||
const notFound = {
|
||||
status: 404,
|
||||
contentType: 'application/json',
|
||||
body: JSON.stringify({ error: 'Server not found', errorCode: 'SERVER_NOT_FOUND' })
|
||||
};
|
||||
|
||||
await visitor.page.route('**/api/servers/featured**', (route) => route.fulfill(notFound));
|
||||
await visitor.page.route('**/api/servers/trending**', (route) => route.fulfill(notFound));
|
||||
});
|
||||
|
||||
await test.step('a brand-new account registers against the legacy-style endpoint', async () => {
|
||||
const register = new RegisterPage(visitor.page);
|
||||
|
||||
await register.goto();
|
||||
await register.register(`visitor_${suffix}`, 'Discovery Visitor', MULTI_DEVICE_PASSWORD);
|
||||
await expectDashboardReady(visitor.page);
|
||||
});
|
||||
|
||||
await test.step('Popular Servers still lists the server via the public-listing fallback', async () => {
|
||||
await expect(dashboardSearchInput(visitor.page)).toHaveValue('');
|
||||
await expect(popularServersPanel(visitor).getByText(serverName)).toBeVisible({ timeout: 30_000 });
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -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. 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.
|
||||
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 even though plenty of servers existed on other online endpoints. Discovery is **self-healing for legacy servers**: when a `/featured` or `/trending` request returns `404` (older signal servers predate those routes and resolve them as `/servers/:id`), `fetchDiscoveryFromEndpoint` falls back per-endpoint to the public `GET /api/servers` listing (`fetchPublicServerListForDiscovery`) instead of returning `[]`, so a fresh user always sees available servers without searching. 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.
|
||||
|
||||
|
||||
@@ -1,19 +0,0 @@
|
||||
import {
|
||||
describe,
|
||||
expect,
|
||||
it
|
||||
} from 'vitest';
|
||||
|
||||
import { endpointSupportsServerDiscovery } from './server-discovery.rules';
|
||||
|
||||
describe('server-discovery.rules', () => {
|
||||
it('skips discovery calls for production signal hosts without featured/trending routes', () => {
|
||||
expect(endpointSupportsServerDiscovery('https://signal.toju.app')).toBe(false);
|
||||
expect(endpointSupportsServerDiscovery('https://signal-sweden.toju.app')).toBe(false);
|
||||
});
|
||||
|
||||
it('allows discovery on local and custom signal servers', () => {
|
||||
expect(endpointSupportsServerDiscovery('http://localhost:3001')).toBe(true);
|
||||
expect(endpointSupportsServerDiscovery('https://signal.example.com')).toBe(true);
|
||||
});
|
||||
});
|
||||
@@ -1,13 +0,0 @@
|
||||
/** Hostnames known to run older signal servers without featured/trending discovery routes. */
|
||||
const DISCOVERY_UNSUPPORTED_HOSTS = new Set(['signal.toju.app', 'signal-sweden.toju.app']);
|
||||
|
||||
/** Returns false when discovery endpoints are known to 404 on the active signal server. */
|
||||
export function endpointSupportsServerDiscovery(baseUrl: string): boolean {
|
||||
try {
|
||||
const hostname = new URL(baseUrl).hostname;
|
||||
|
||||
return !DISCOVERY_UNSUPPORTED_HOSTS.has(hostname);
|
||||
} catch {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
@@ -133,19 +133,19 @@ describe('ServerDirectoryApiService discovery fan-out', () => {
|
||||
expect(calledUrls).toContain('https://other.test/api/servers/featured');
|
||||
});
|
||||
|
||||
it('does not query discovery-unsupported hosts but still returns the supported endpoints', async () => {
|
||||
it('queries every online endpoint including production signal hosts', 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://signal.toju.app'))
|
||||
return { servers: [{ id: 'prod-1', name: 'Prod Server' }], total: 1 };
|
||||
|
||||
return { servers: [], total: 0 };
|
||||
});
|
||||
const result = await firstValueFrom(service.getTrendingServers());
|
||||
|
||||
expect(result.map((server) => server.id)).toEqual(['s1']);
|
||||
expect(result.map((server) => server.id)).toEqual(['prod-1']);
|
||||
const calledUrls = get.mock.calls.map((call) => call[0] as string);
|
||||
|
||||
expect(calledUrls.some((url) => url.includes('signal.toju.app'))).toBe(false);
|
||||
expect(calledUrls).toContain('https://signal.toju.app/api/servers/trending');
|
||||
});
|
||||
|
||||
it('deduplicates the same server returned by multiple endpoints', async () => {
|
||||
@@ -157,3 +157,83 @@ describe('ServerDirectoryApiService discovery fan-out', () => {
|
||||
expect(result.map((server) => server.id)).toEqual(['dup']);
|
||||
});
|
||||
});
|
||||
|
||||
function createObservableHarness(
|
||||
endpoints: { id: string; name: string; url: string; status: string }[],
|
||||
getImpl: (url: string) => unknown
|
||||
) {
|
||||
const get = vi.fn((url: string) => getImpl(url));
|
||||
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 fallback', () => {
|
||||
const endpoints = [{ id: 'ep-1', name: 'Legacy', url: 'https://local.test', status: 'online' }];
|
||||
|
||||
it('falls back to the public server listing when a discovery route returns 404', async () => {
|
||||
const { service, get } = createObservableHarness(endpoints, (url) => {
|
||||
if (url.includes('/api/servers/featured') || url.includes('/api/servers/trending')) {
|
||||
return throwError(() => ({ status: 404, error: { errorCode: 'SERVER_NOT_FOUND' } }));
|
||||
}
|
||||
|
||||
return of({ servers: [{ id: 'legacy-1', name: 'Legacy Server' }], total: 1 });
|
||||
});
|
||||
const result = await firstValueFrom(service.getFeaturedServers());
|
||||
|
||||
expect(result.map((server) => server.id)).toEqual(['legacy-1']);
|
||||
const calledUrls = get.mock.calls.map((call) => call[0] as string);
|
||||
|
||||
expect(calledUrls).toContain('https://local.test/api/servers/featured');
|
||||
expect(calledUrls).toContain('https://local.test/api/servers');
|
||||
});
|
||||
|
||||
it('forwards the discovery limit to the public-listing fallback', async () => {
|
||||
const { service, get } = createObservableHarness(endpoints, (url) => {
|
||||
if (url.includes('/api/servers/featured')) {
|
||||
return throwError(() => ({ status: 404 }));
|
||||
}
|
||||
|
||||
return of({ servers: [], total: 0 });
|
||||
});
|
||||
|
||||
await firstValueFrom(service.getFeaturedServers(7));
|
||||
|
||||
const fallbackCall = get.mock.calls.find((call) => call[0] === 'https://local.test/api/servers');
|
||||
|
||||
expect(fallbackCall).toBeDefined();
|
||||
expect((fallbackCall?.[1] as { params: HttpParams }).params.get('limit')).toBe('7');
|
||||
});
|
||||
|
||||
it('does not fall back to the public listing on non-404 errors', async () => {
|
||||
const { service, get } = createObservableHarness(endpoints, (url) => {
|
||||
if (url.includes('/api/servers/featured')) {
|
||||
return throwError(() => ({ status: 500 }));
|
||||
}
|
||||
|
||||
return of({ servers: [{ id: 'unexpected', name: 'Unexpected' }], total: 1 });
|
||||
});
|
||||
const result = await firstValueFrom(service.getFeaturedServers());
|
||||
|
||||
expect(result).toEqual([]);
|
||||
const calledUrls = get.mock.calls.map((call) => call[0] as string);
|
||||
|
||||
expect(calledUrls).not.toContain('https://local.test/api/servers');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -35,7 +35,6 @@ import type {
|
||||
UnbanServerMemberRequest
|
||||
} from '../../domain/models/server-directory.model';
|
||||
import type { RoomSignalSourceInput } from '../../domain/logic/room-signal-source.logic';
|
||||
import { endpointSupportsServerDiscovery } from '../../domain/logic/server-discovery.rules';
|
||||
|
||||
interface ServerLookupError {
|
||||
status?: number;
|
||||
@@ -50,6 +49,16 @@ function isServerNotFoundError(error: unknown): boolean {
|
||||
return lookupError?.status === 404 && lookupError.error?.errorCode === 'SERVER_NOT_FOUND';
|
||||
}
|
||||
|
||||
/**
|
||||
* Older signal servers predate the `/featured` and `/trending` routes, so they
|
||||
* resolve the request to `/servers/:id` and answer `404`. Treat any 404 from a
|
||||
* discovery route as "this endpoint has no discovery routes" and fall back to
|
||||
* the public server listing.
|
||||
*/
|
||||
function isDiscoveryRouteUnavailable(error: unknown): boolean {
|
||||
return (error as ServerLookupError)?.status === 404;
|
||||
}
|
||||
|
||||
@Injectable({ providedIn: 'root' })
|
||||
export class ServerDirectoryApiService {
|
||||
private readonly http = inject(HttpClient);
|
||||
@@ -302,8 +311,8 @@ export class ServerDirectoryApiService {
|
||||
|
||||
// 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.
|
||||
// happens to be one without populated discovery lists. Querying only the
|
||||
// active endpoint made servers invisible until the user typed a search query.
|
||||
if (onlineEndpoints.length === 0) {
|
||||
return this.fetchDiscoveryFromEndpoint(
|
||||
kind,
|
||||
@@ -326,21 +335,40 @@ export class ServerDirectoryApiService {
|
||||
source: ServerEndpoint | null | undefined,
|
||||
params?: HttpParams
|
||||
): Observable<ServerInfo[]> {
|
||||
if (!endpointSupportsServerDiscovery(baseUrl)) {
|
||||
return of([]);
|
||||
}
|
||||
|
||||
return this.http
|
||||
.get<{ servers: ServerInfo[]; total: number }>(`${baseUrl}/api/servers/${kind}`, params ? { params } : {})
|
||||
.pipe(
|
||||
map((response) => this.normalizeServerList(response, source ?? null)),
|
||||
catchError((error) => {
|
||||
// Self-heal against legacy signal servers that lack discovery routes:
|
||||
// fall back to the public listing so the default view is never empty
|
||||
// just because an endpoint can't rank featured/trending servers.
|
||||
if (isDiscoveryRouteUnavailable(error)) {
|
||||
return this.fetchPublicServerListForDiscovery(baseUrl, source, params);
|
||||
}
|
||||
|
||||
console.error(`Failed to get ${kind} servers:`, error);
|
||||
return of([]);
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
private fetchPublicServerListForDiscovery(
|
||||
baseUrl: string,
|
||||
source: ServerEndpoint | null | undefined,
|
||||
params?: HttpParams
|
||||
): Observable<ServerInfo[]> {
|
||||
return this.http
|
||||
.get<{ servers: ServerInfo[]; total: number }>(`${baseUrl}/api/servers`, params ? { params } : {})
|
||||
.pipe(
|
||||
map((response) => this.normalizeServerList(response, source ?? null)),
|
||||
catchError((error) => {
|
||||
console.error('Failed to load servers for discovery fallback:', error);
|
||||
return of([]);
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
private resolveBaseServerUrl(selector?: ServerSourceSelector): string {
|
||||
const resolvedEndpoint = this.resolveEndpoint(selector);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user