From 5736083d50be18bb9216db4bf83d6742ff44aaae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C5=ABdolfs=20O=C5=A1i=C5=86=C5=A1?= Date: Wed, 9 Aug 2023 12:05:01 +0200 Subject: [PATCH] Fix not found and loading error screens MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rūdolfs Ošiņš --- src/App.svelte | 8 +- src/components/LoadError.svelte | 17 ++-- src/components/NotFound.svelte | 42 ---------- src/lib/router.ts | 12 +-- src/lib/router/definitions.ts | 12 +-- src/views/NotFound.svelte | 30 +++++++ src/views/home/router.ts | 44 +++++----- src/views/nodes/router.ts | 10 +-- src/views/projects/router.ts | 89 ++++++++++++--------- tests/support/fixtures.ts | 22 ++++- tests/support/peerManager.ts | 21 +++-- tests/visual/landingPage.spec.ts | 23 +++++- tests/visual/{seed.spec.ts => node.spec.ts} | 7 ++ tests/visual/notFound.spec.ts | 8 ++ tests/visual/project.spec.ts | 50 +++++++++++- 15 files changed, 240 insertions(+), 155 deletions(-) delete mode 100644 src/components/NotFound.svelte create mode 100644 src/views/NotFound.svelte rename tests/visual/{seed.spec.ts => node.spec.ts} (72%) create mode 100644 tests/visual/notFound.spec.ts diff --git a/src/App.svelte b/src/App.svelte index 30fb3c1bf..d966eede5 100644 --- a/src/App.svelte +++ b/src/App.svelte @@ -17,6 +17,7 @@ import Issues from "@app/views/projects/Issues.svelte"; import NewIssue from "@app/views/projects/Issue/New.svelte"; import Nodes from "@app/views/nodes/View.svelte"; + import NotFound from "@app/views/NotFound.svelte"; import Patch from "@app/views/projects/Patch.svelte"; import Patches from "@app/views/projects/Patches.svelte"; import Session from "@app/views/session/Index.svelte"; @@ -24,7 +25,6 @@ import LoadError from "@app/components/LoadError.svelte"; import Loading from "@app/components/Loading.svelte"; - import NotFound from "@app/components/NotFound.svelte"; const activeRouteStore = router.activeRouteStore; @@ -95,11 +95,7 @@ {:else if $activeRouteStore.resource === "loadError"} {:else if $activeRouteStore.resource === "notFound"} -
- -
+ {:else} {unreachable($activeRouteStore)} {/if} diff --git a/src/components/LoadError.svelte b/src/components/LoadError.svelte index 45d868dad..e78b1fbbd 100644 --- a/src/components/LoadError.svelte +++ b/src/components/LoadError.svelte @@ -1,10 +1,9 @@ @@ -22,15 +21,9 @@
🏜️
- {#if title} -
- {title} -
- {/if} - -
- +
+ {title}
+ +
diff --git a/src/components/NotFound.svelte b/src/components/NotFound.svelte deleted file mode 100644 index bf63aac38..000000000 --- a/src/components/NotFound.svelte +++ /dev/null @@ -1,42 +0,0 @@ - - - - -
-
🏜️
-
{title}
-
{subtitle}
- -
- -
-
diff --git a/src/lib/router.ts b/src/lib/router.ts index 4874d6a4e..633b437cb 100644 --- a/src/lib/router.ts +++ b/src/lib/router.ts @@ -76,7 +76,7 @@ export async function navigateToUrl( } else { await navigate(action, { resource: "notFound", - params: { url: relativeUrl }, + params: { title: "Page not found" }, }); } } @@ -241,8 +241,6 @@ export function routeToPath(route: Route): string { return `/session?id=${route.params.id}&sig=${route.params.signature}&pk=${route.params.publicKey}`; } else if (route.resource === "nodes") { return nodePath(route.params.baseUrl); - } else if (route.resource === "loadError") { - return ""; } else if ( route.resource === "project.source" || route.resource === "project.history" || @@ -254,10 +252,12 @@ export function routeToPath(route: Route): string { route.resource === "project.patch" ) { return projectRouteToPath(route); - } else if (route.resource === "booting") { + } else if ( + route.resource === "booting" || + route.resource === "notFound" || + route.resource === "loadError" + ) { return ""; - } else if (route.resource === "notFound") { - return route.params.url; } else { return utils.unreachable(route); } diff --git a/src/lib/router/definitions.ts b/src/lib/router/definitions.ts index 6825b31c7..44bdd4c56 100644 --- a/src/lib/router/definitions.ts +++ b/src/lib/router/definitions.ts @@ -13,9 +13,9 @@ interface BootingRoute { resource: "booting"; } -interface NotFoundRoute { +export interface NotFoundRoute { resource: "notFound"; - params: { url: string }; + params: { title: string }; } interface SessionRoute { @@ -23,10 +23,10 @@ interface SessionRoute { params: { id: string; signature: string; publicKey: string }; } -export interface LoadError { +export interface LoadErrorRoute { resource: "loadError"; params: { - title?: string; + title: string; errorMessage: string; stackTrace: string; }; @@ -35,7 +35,7 @@ export interface LoadError { export type Route = | BootingRoute | HomeRoute - | LoadError + | LoadErrorRoute | NotFoundRoute | ProjectRoute | NodesRoute @@ -44,7 +44,7 @@ export type Route = export type LoadedRoute = | BootingRoute | HomeLoadedRoute - | LoadError + | LoadErrorRoute | NotFoundRoute | ProjectLoadedRoute | NodesLoadedRoute diff --git a/src/views/NotFound.svelte b/src/views/NotFound.svelte new file mode 100644 index 000000000..48026ecb5 --- /dev/null +++ b/src/views/NotFound.svelte @@ -0,0 +1,30 @@ + + + + +
+
+
🏜️
+
{title}
+
+
diff --git a/src/views/home/router.ts b/src/views/home/router.ts index e3c0e8856..3ff4e854c 100644 --- a/src/views/home/router.ts +++ b/src/views/home/router.ts @@ -1,4 +1,4 @@ -import type { LoadError } from "@app/lib/router/definitions"; +import type { LoadErrorRoute } from "@app/lib/router/definitions"; import type { ProjectBaseUrl } from "@app/lib/search"; import type { WeeklyActivity } from "@app/lib/commit"; @@ -19,30 +19,22 @@ export interface HomeLoadedRoute { params: { projects: ProjectBaseUrlActivity[] }; } -export async function loadHomeRoute(): Promise { - try { - const projects = await getProjectsFromNodes(config.projects.pinned); - const results = await Promise.all( - projects.map(async projectNode => { - const activity = await loadProjectActivity( - projectNode.project.id, - projectNode.baseUrl, - ); - return { - ...projectNode, - activity, - }; - }), - ); +export async function loadHomeRoute(): Promise< + HomeLoadedRoute | LoadErrorRoute +> { + const projects = await getProjectsFromNodes(config.projects.pinned); + const results = await Promise.all( + projects.map(async projectNode => { + const activity = await loadProjectActivity( + projectNode.project.id, + projectNode.baseUrl, + ); + return { + ...projectNode, + activity, + }; + }), + ); - return { resource: "home", params: { projects: results } }; - } catch (error: any) { - return { - resource: "loadError", - params: { - errorMessage: "Could not load pinned projects.", - stackTrace: error.stack, - }, - }; - } + return { resource: "home", params: { projects: results } }; } diff --git a/src/views/nodes/router.ts b/src/views/nodes/router.ts index ab173b20f..d22705cd5 100644 --- a/src/views/nodes/router.ts +++ b/src/views/nodes/router.ts @@ -1,5 +1,5 @@ import type { BaseUrl, Project } from "@httpd-client"; -import type { LoadError } from "@app/lib/router/definitions"; +import type { NotFoundRoute } from "@app/lib/router/definitions"; import type { WeeklyActivity } from "@app/lib/commit"; import { HttpdClient } from "@httpd-client"; @@ -81,7 +81,7 @@ export function nodePath(baseUrl: BaseUrl) { export async function loadNodeRoute( params: NodesRouteParams, -): Promise { +): Promise { const api = new HttpdClient(params.baseUrl); try { const projectPageIndex = 0; @@ -102,11 +102,9 @@ export async function loadNodeRoute( }; } catch (error: any) { return { - resource: "loadError", + resource: "notFound", params: { - title: `${params.baseUrl.hostname}:${params.baseUrl.port}`, - errorMessage: "Not able to query this node.", - stackTrace: error.stack, + title: "Node not found", }, }; } diff --git a/src/views/projects/router.ts b/src/views/projects/router.ts index c22a9214d..c8f9a78a2 100644 --- a/src/views/projects/router.ts +++ b/src/views/projects/router.ts @@ -1,4 +1,7 @@ -import type { LoadError } from "@app/lib/router/definitions"; +import type { + LoadErrorRoute, + NotFoundRoute, +} from "@app/lib/router/definitions"; import type { BaseUrl, Blob, @@ -230,13 +233,13 @@ function parseRevisionToOid( export async function loadProjectRoute( route: ProjectRoute, -): Promise { +): Promise { const api = new HttpdClient(route.node); try { if (route.resource === "project.source") { - return loadTreeView(route); + return await loadTreeView(route); } else if (route.resource === "project.history") { - return loadHistoryView(route); + return await loadHistoryView(route); } else if (route.resource === "project.commit") { const [project, commit] = await Promise.all([ api.project.getById(route.project), @@ -252,33 +255,22 @@ export async function loadProjectRoute( }, }; } else if (route.resource === "project.issue") { - try { - const [project, issue] = await Promise.all([ - api.project.getById(route.project), - api.project.getIssueById(route.project, route.issue), - ]); - return { - resource: "project.issue", - params: { - baseUrl: route.node, - project, - issue, - }, - }; - } catch (error: any) { - return { - resource: "loadError", - params: { - title: route.issue, - errorMessage: "Not able to load this issue.", - stackTrace: error.stack, - }, - }; - } + const [project, issue] = await Promise.all([ + api.project.getById(route.project), + api.project.getIssueById(route.project, route.issue), + ]); + return { + resource: "project.issue", + params: { + baseUrl: route.node, + project, + issue, + }, + }; } else if (route.resource === "project.patch") { - return loadPatchView(route); + return await loadPatchView(route); } else if (route.resource === "project.issues") { - return loadIssuesView(route); + return await loadIssuesView(route); } else if (route.resource === "project.newIssue") { const project = await api.project.getById(route.project); return { @@ -289,19 +281,40 @@ export async function loadProjectRoute( }, }; } else if (route.resource === "project.patches") { - return loadPatchesView(route); + return await loadPatchesView(route); } else { return unreachable(route); } } catch (error: any) { - return { - resource: "loadError", - params: { - title: route.project, - errorMessage: "Not able to load this project.", - stackTrace: error.stack, - }, - }; + if (error?.status === 404) { + let subject; + + if (route.resource === "project.commit") { + subject = "Commit"; + } else if (route.resource === "project.issue") { + subject = "Issue"; + } else if (route.resource === "project.patch") { + subject = "Patch"; + } else { + subject = "Project"; + } + + return { + resource: "notFound", + params: { + title: `${subject} not found`, + }, + }; + } else { + return { + resource: "loadError", + params: { + title: "Could not load this project", + errorMessage: error.message, + stackTrace: error.stack, + }, + }; + } } } diff --git a/tests/support/fixtures.ts b/tests/support/fixtures.ts index 7157dad2a..208fc3a9a 100644 --- a/tests/support/fixtures.ts +++ b/tests/support/fixtures.ts @@ -245,6 +245,24 @@ export function appConfigWithFixture() { }, projects: { pinned: [ + { + name: "cobs", + id: "rad:z3fpY7nttPPa6MBnAv2DccHzQJnqe", + baseUrl: { + hostname: "127.0.0.1", + port: 8081, + scheme: "http", + }, + }, + { + name: "markdown", + id: "rad:z2tchH2Ti4LxRKdssPQYs6VHE5rsg", + baseUrl: { + hostname: "127.0.0.1", + port: 8081, + scheme: "http", + }, + }, { name: "source-browsing", id: "rad:z4BwwjPCFNVP27FwVbDFgwVwkjcir", @@ -349,11 +367,11 @@ export async function createSourceBrowsingFixture( await bob.git(["push", "rad"], { cwd: bobProjectPath }); await bob.waitForEvent( - { type: "refs-synced", remote: palm.nodeId, rid }, + { type: "refsSynced", remote: palm.nodeId, rid }, 2000, ); await bob.waitForEvent( - { type: "refs-synced", remote: alice.nodeId, rid }, + { type: "refsSynced", remote: alice.nodeId, rid }, 2000, ); await alice.stopNode(); diff --git a/tests/support/peerManager.ts b/tests/support/peerManager.ts index 98dca89d2..3e8d30821 100644 --- a/tests/support/peerManager.ts +++ b/tests/support/peerManager.ts @@ -24,29 +24,34 @@ export type RefsUpdate = export type NodeEvent = | { - type: "refs-fetched"; + type: "refsFetched"; remote: string; rid: string; updated: RefsUpdate[]; } | { - type: "refs-synced"; + type: "refsSynced"; remote: string; rid: string; } | { - type: "seed-discovered"; + type: "seedDiscovered"; rid: string; nid: string; } | { - type: "seed-dropped"; + type: "seedDropped"; nid: string; rid: string; } | { - type: "peer-connected"; + type: "peerConnected"; nid: string; + } + | { + type: "peerDisconnected"; + nid: string; + reason: string; }; export interface RoutingEntry { @@ -340,7 +345,7 @@ export class RadiclePeer { }); await this.waitForEvent( - { type: "seed-discovered", rid, nid: this.nodeId }, + { type: "seedDiscovered", rid, nid: this.nodeId }, 6000, ); } @@ -356,11 +361,11 @@ export class RadiclePeer { ); await this.waitForEvent( - { type: "peer-connected", nid: remote.nodeId }, + { type: "peerConnected", nid: remote.nodeId }, 1000, ); await remote.waitForEvent( - { type: "peer-connected", nid: this.nodeId }, + { type: "peerConnected", nid: this.nodeId }, 1000, ); } diff --git a/tests/visual/landingPage.spec.ts b/tests/visual/landingPage.spec.ts index b0846fe12..c6c8321b7 100644 --- a/tests/visual/landingPage.spec.ts +++ b/tests/visual/landingPage.spec.ts @@ -4,7 +4,7 @@ test.use({ customAppConfig: true, }); -test("landing page", async ({ page }) => { +test("pinned projects", async ({ page }) => { await page.addInitScript(() => { window.initializeTestStubs = () => { window.e2eTestStubs.FakeTimers.install({ @@ -19,3 +19,24 @@ test("landing page", async ({ page }) => { await page.goto("/", { waitUntil: "networkidle" }); await expect(page).toHaveScreenshot(); }); + +test("load error", async ({ page }) => { + await page.addInitScript(() => { + window.initializeTestStubs = () => { + window.e2eTestStubs.FakeTimers.install({ + now: new Date("November 24 2022 12:00:00").valueOf(), + shouldClearNativeTimers: true, + shouldAdvanceTime: false, + }); + }; + }); + + await page.route( + "**/api/v1/projects/rad:z4BwwjPCFNVP27FwVbDFgwVwkjcir", + route => route.fulfill({ status: 500 }), + ); + + await page.addInitScript(appConfigWithFixture); + await page.goto("/", { waitUntil: "networkidle" }); + await expect(page).toHaveScreenshot(); +}); diff --git a/tests/visual/seed.spec.ts b/tests/visual/node.spec.ts similarity index 72% rename from tests/visual/seed.spec.ts rename to tests/visual/node.spec.ts index 0ca867227..51c957dfb 100644 --- a/tests/visual/seed.spec.ts +++ b/tests/visual/node.spec.ts @@ -14,3 +14,10 @@ test("node page", async ({ page }) => { await page.goto("/nodes/radicle.local", { waitUntil: "networkidle" }); await expect(page).toHaveScreenshot(); }); + +test("node not found", async ({ page }) => { + await page.goto("/nodes/this.node.does.not.exist.xyz", { + waitUntil: "networkidle", + }); + await expect(page).toHaveScreenshot(); +}); diff --git a/tests/visual/notFound.spec.ts b/tests/visual/notFound.spec.ts new file mode 100644 index 000000000..286f89d2c --- /dev/null +++ b/tests/visual/notFound.spec.ts @@ -0,0 +1,8 @@ +import { expect, test } from "@tests/support/fixtures.js"; + +test("page not found", async ({ page }) => { + await page.goto("/this/page/does/not/exist", { + waitUntil: "networkidle", + }); + await expect(page).toHaveScreenshot(); +}); diff --git a/tests/visual/project.spec.ts b/tests/visual/project.spec.ts index 1b72aebf0..cc465bfea 100644 --- a/tests/visual/project.spec.ts +++ b/tests/visual/project.spec.ts @@ -6,12 +6,12 @@ import { aliceRemote, } from "@tests/support/fixtures.js"; -test("source tree page", async ({ page }) => { +test("source page", async ({ page }) => { await page.goto(sourceBrowsingUrl, { waitUntil: "networkidle" }); await expect(page).toHaveScreenshot(); }); -test("commits page", async ({ page }) => { +test("history page", async ({ page }) => { await page.addInitScript(() => { window.initializeTestStubs = () => { window.e2eTestStubs.FakeTimers.install({ @@ -68,3 +68,49 @@ test("diff selection", async ({ page }) => { ); await expect(page).toHaveScreenshot({ fullPage: true }); }); + +test("project load error", async ({ page }) => { + await page.goto( + `${sourceBrowsingUrl}/remotes/zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz`, + { waitUntil: "networkidle" }, + ); + await expect(page).toHaveScreenshot(); +}); + +test("project not found", async ({ page }) => { + await page.goto(`/nodes/127.0.0.1/rad:z4Vzzzzzzzzzzzzzzzzzzzzzzzzzz`, { + waitUntil: "networkidle", + }); + await expect(page).toHaveScreenshot(); +}); + +test("file not found", async ({ page }) => { + await page.goto(`${sourceBrowsingUrl}/tree/this.file.does.not.exist`, { + waitUntil: "networkidle", + }); + await expect(page).toHaveScreenshot(); +}); + +test("commit not found", async ({ page }) => { + await page.goto( + `${sourceBrowsingUrl}/commits/0000000000000000000000000000000000000000`, + { waitUntil: "networkidle" }, + ); + await expect(page).toHaveScreenshot(); +}); + +test("issue not found", async ({ page }) => { + await page.goto( + `${sourceBrowsingUrl}/issues/0000000000000000000000000000000000000000`, + { waitUntil: "networkidle" }, + ); + await expect(page).toHaveScreenshot(); +}); + +test("patch not found", async ({ page }) => { + await page.goto( + `${sourceBrowsingUrl}/patches/0000000000000000000000000000000000000000`, + { waitUntil: "networkidle" }, + ); + await expect(page).toHaveScreenshot(); +});