From aba2ea586676e03e3f4361d75c994cda53e9d421 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20St=C3=B8vring?= Date: Sat, 11 Nov 2023 14:30:59 +0100 Subject: [PATCH 01/12] Removes file extension requirement from specification IDs --- __test__/common/utils/url.test.ts | 6 +-- src/common/utils/url.ts | 85 +++++++++++++++---------------- 2 files changed, 43 insertions(+), 48 deletions(-) diff --git a/__test__/common/utils/url.test.ts b/__test__/common/utils/url.test.ts index dd5968d6..f52e7c95 100644 --- a/__test__/common/utils/url.test.ts +++ b/__test__/common/utils/url.test.ts @@ -44,14 +44,14 @@ test("It reads path containing project, version, and specification with .yaml ex expect(specificationId).toBe("openapi.yaml") }) -test("It reads version containing a slash", async () => { +test("It parses specification without trailing file extension", async () => { const url = "/foo/bar/baz" const projectId = getProjectId(url) const versionId = getVersionId(url) const specificationId = getSpecificationId(url) expect(projectId).toEqual("foo") - expect(versionId).toEqual("bar/baz") - expect(specificationId).toBeUndefined() + expect(versionId).toEqual("bar") + expect(specificationId).toBe("baz") }) test("It read specification when version contains a slash", async () => { diff --git a/src/common/utils/url.ts b/src/common/utils/url.ts index 119afc89..507750b9 100644 --- a/src/common/utils/url.ts +++ b/src/common/utils/url.ts @@ -1,52 +1,47 @@ -export function getProjectId(url?: string) { - if (typeof window !== 'undefined') { - url = window.location.pathname;// remove first slash - } - url = url?.substring(1);// remove first slash - const firstSlash = url?.indexOf('/'); - let project = url ? decodeURI(url) : undefined - if (firstSlash != -1 && url) { - project = decodeURI(url.substring(0, firstSlash)); - } - return project +function getPathname(url?: string) { + if (typeof window !== "undefined") { + url = window.location.pathname + } + if (!url) { + return undefined + } + if (!url.startsWith("/")) { + return url + } + return url.substring(1) } -function getVersionAndSpecification(url?: string) { - const project = getProjectId(url) - if (url && project) { - const versionAndSpecification = url.substring(project.length + 2)// remove first slash - let specification: string | undefined = undefined; - let version = versionAndSpecification; - if (versionAndSpecification) { - const lastSlash = versionAndSpecification?.lastIndexOf('/'); - if (lastSlash != -1) { - const potentialSpecification = versionAndSpecification?.substring(lastSlash) - if (potentialSpecification?.endsWith('.yml') || potentialSpecification?.endsWith('.yaml')) { - version = versionAndSpecification?.substring(0, lastSlash) - specification = versionAndSpecification?.substring(lastSlash + 1) - } - } - } - return { - version, - specification - }; +function getProjectSelection(tmpPathname?: string) { + const pathname = getPathname(tmpPathname) + if (!pathname) { + return undefined + } + const comps = pathname.split("/") + if (comps.length == 0) { + return {} + } else if (comps.length == 1) { + return { projectId: comps[0] } + } else if (comps.length == 2) { + return { + projectId: comps[0], + versionId: comps[1] } - return {}; + } else { + const projectId = comps[0] + const versionId = comps.slice(1, -1).join("/") + const specificationId = comps[comps.length - 1] + return { projectId, versionId, specificationId } + } } -export function getVersionId(url?: string) { - if (typeof window !== 'undefined') { - url = window.location.pathname - } - const version = getVersionAndSpecification(url).version; - return version ? decodeURI(version) : undefined; +export function getProjectId(tmpPathname?: string) { + return getProjectSelection(tmpPathname)?.projectId } -export function getSpecificationId(url?: string) { - if (typeof window !== 'undefined') { - url = window.location.pathname - } - const specification = getVersionAndSpecification(url).specification; - return specification ? decodeURI(specification) : undefined; -} \ No newline at end of file +export function getVersionId(tmpPathname?: string) { + return getProjectSelection(tmpPathname)?.versionId +} + +export function getSpecificationId(tmpPathname?: string) { + return getProjectSelection(tmpPathname)?.specificationId +} From 7150813e2afc0c3e5e185100d89ce17c6289d054 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20St=C3=B8vring?= Date: Sat, 11 Nov 2023 14:31:23 +0100 Subject: [PATCH 02/12] Supports moving specification ID to version ID --- __test__/projects/getSelection.test.ts | 26 ++++++++++++++++++++ src/features/projects/domain/getSelection.ts | 22 ++++++++++++++++- 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/__test__/projects/getSelection.test.ts b/__test__/projects/getSelection.test.ts index b2b54c5f..02b22cc4 100644 --- a/__test__/projects/getSelection.test.ts +++ b/__test__/projects/getSelection.test.ts @@ -236,3 +236,29 @@ test("It returns a undefined specification when the selected specification canno expect(sut.version!.id).toEqual("bar") expect(sut.specification).toBeUndefined() }) + +test("It moves specification ID to version ID if needed", () => { + const sut = getSelection({ + projectId: "foo", + versionId: "bar", + specificationId: "baz", + projects: [{ + id: "foo", + name: "foo", + displayName: "foo", + versions: [{ + id: "bar/baz", + name: "bar", + isDefault: false, + specifications: [{ + id: "hello", + name: "hello.yml", + url: "https://example.com/hello.yml" + }] + }] + }] + }) + expect(sut.project!.id).toEqual("foo") + expect(sut.version!.id).toEqual("bar/baz") + expect(sut.specification!.id).toEqual("hello") +}) diff --git a/src/features/projects/domain/getSelection.ts b/src/features/projects/domain/getSelection.ts index a9aa2caf..50fd7f2e 100644 --- a/src/features/projects/domain/getSelection.ts +++ b/src/features/projects/domain/getSelection.ts @@ -29,8 +29,24 @@ export default function getSelection({ return {} } let version: Version | undefined + let didMoveSpecificationIdToVersionId = false if (versionId) { version = project.versions.find(e => e.id == versionId) + if (!version && specificationId && !isSpecificationIdFilename(specificationId)) { + // With the introduction of remote versions that are specified in the .shape-docs.yml + // configuration file, it has become impossible to tell if the last component in a URL + // is the specification ID or if it belongs to the version ID. Previously, we required + // specification IDs to end with either ".yml" or ".yaml" but that no longer makes + // sense when users can define versions. + // Instead we assume that the last component is the specification ID but if we cannot + // find a version with what we then believe to be the version ID, then we attempt to + // finding a version with the ID `{versionId}/{specificationId}` and if that succeeds, + // we select the first specification in that version by flagging that the ID of the + // specification is considered part of the version ID. + const longId = [versionId, specificationId].join("/") + version = project.versions.find(e => e.id == longId) + didMoveSpecificationIdToVersionId = version != undefined + } } else if (project.versions.length > 0) { version = project.versions[0] } @@ -38,10 +54,14 @@ export default function getSelection({ return { project } } let specification: OpenApiSpecification | undefined - if (specificationId) { + if (specificationId && !didMoveSpecificationIdToVersionId) { specification = version.specifications.find(e => e.id == specificationId) } else if (version.specifications.length > 0) { specification = version.specifications[0] } return { project, version, specification } +} + +function isSpecificationIdFilename(specificationId: string): boolean { + return specificationId.endsWith(".yml") || specificationId.endsWith(".yaml") } \ No newline at end of file From afa8e60999d241154fd4027e5c4fa3068c894bfa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20St=C3=B8vring?= Date: Sat, 11 Nov 2023 14:31:44 +0100 Subject: [PATCH 03/12] Reads remote versions from .shape-docs.yml --- .../projects/GitHubProjectDataSource.test.ts | 151 ++++++++++++++++++ .../projects/data/GitHubProjectDataSource.ts | 99 +++++++++--- .../projects/domain/IProjectConfig.ts | 12 ++ src/features/projects/domain/index.ts | 1 + 4 files changed, 237 insertions(+), 26 deletions(-) diff --git a/__test__/projects/GitHubProjectDataSource.test.ts b/__test__/projects/GitHubProjectDataSource.test.ts index 874caf3d..db8acb59 100644 --- a/__test__/projects/GitHubProjectDataSource.test.ts +++ b/__test__/projects/GitHubProjectDataSource.test.ts @@ -1091,3 +1091,154 @@ test("It identifies the default branch in returned versions", async () => { .map(e => e.name) expect(defaultVersionNames).toEqual(["development"]) }) + +test("It adds remote versions from the project configuration", async () => { + const rawProjectConfig = ` + remoteVersions: + - name: Anne + specifications: + - name: Huey + url: https://example.com/huey.yml + - name: Dewey + url: https://example.com/dewey.yml + - name: Bobby + specifications: + - name: Louie + url: https://example.com/louie.yml + ` + const sut = new GitHubProjectDataSource({ + dataSource: { + async getRepositories() { + return [{ + name: "foo", + owner: { + login: "acme" + }, + defaultBranchRef: { + name: "main", + target: { + oid: "12345678" + } + }, + configYml: { + text: rawProjectConfig + }, + branches: { + edges: [] + }, + tags: { + edges: [] + } + }] + } + } + }) + const projects = await sut.getProjects() + expect(projects[0].versions).toEqual([{ + id: "anne", + name: "Anne", + isDefault: false, + specifications: [{ + id: "huey", + name: "Huey", + url: "https://example.com/huey.yml" + }, { + id: "dewey", + name: "Dewey", + url: "https://example.com/dewey.yml" + }] + }, { + id: "bobby", + name: "Bobby", + isDefault: false, + specifications: [{ + id: "louie", + name: "Louie", + url: "https://example.com/louie.yml" + }] + }]) +}) + +test("It modifies ID of remote version if the ID already exists", async () => { + const rawProjectConfig = ` + remoteVersions: + - name: Bar + specifications: + - name: Baz + url: https://example.com/baz.yml + - name: Bar + specifications: + - name: Hello + url: https://example.com/hello.yml + ` + const sut = new GitHubProjectDataSource({ + dataSource: { + async getRepositories() { + return [{ + name: "foo", + owner: { + login: "acme" + }, + defaultBranchRef: { + name: "bar", + target: { + oid: "12345678" + } + }, + configYml: { + text: rawProjectConfig + }, + branches: { + edges: [{ + node: { + name: "bar", + target: { + oid: "12345678", + tree: { + entries: [{ + name: "openapi.yml" + }] + } + } + } + }] + }, + tags: { + edges: [] + } + }] + } + } + }) + const projects = await sut.getProjects() + expect(projects[0].versions).toEqual([{ + id: "bar", + name: "bar", + url: "https://github.com/acme/foo/tree/bar", + isDefault: true, + specifications: [{ + id: "openapi.yml", + name: "openapi.yml", + url: "/api/blob/acme/foo/openapi.yml?ref=12345678", + editURL: "https://github.com/acme/foo/edit/bar/openapi.yml" + }] + }, { + id: "bar1", + name: "Bar", + isDefault: false, + specifications: [{ + id: "baz", + name: "Baz", + url: "https://example.com/baz.yml" + }] + }, { + id: "bar2", + name: "Bar", + isDefault: false, + specifications: [{ + id: "hello", + name: "Hello", + url: "https://example.com/hello.yml" + }] + }]) +}) diff --git a/src/features/projects/data/GitHubProjectDataSource.ts b/src/features/projects/data/GitHubProjectDataSource.ts index bdf126c4..9225ceeb 100644 --- a/src/features/projects/data/GitHubProjectDataSource.ts +++ b/src/features/projects/data/GitHubProjectDataSource.ts @@ -3,10 +3,11 @@ import GitHubProjectRepository, { } from "./GitHubProjectRepository" import { Project, + Version, IProjectConfig, IProjectDataSource, - Version, - ProjectConfigParser + ProjectConfigParser, + ProjectConfigRemoteVersion, } from "../domain" interface IGitHubProjectRepositoryDataSource { @@ -48,14 +49,21 @@ export default class GitHubProjectDataSource implements IProjectDataSource { ref: repository.defaultBranchRef.target.oid }) } + const versions = this.sortVersions( + this.addRemoteVersions( + this.getVersions(repository), + config?.remoteVersions || [] + ), + repository.defaultBranchRef.name + ).filter(version => { + return version.specifications.length > 0 + }) const defaultName = repository.name.replace(/-openapi$/, "") return { id: defaultName, name: defaultName, displayName: config?.name || defaultName, - versions: this.getVersions(repository).filter(version => { - return version.specifications.length > 0 - }), + versions, imageURL: imageURL } } @@ -86,27 +94,7 @@ export default class GitHubProjectDataSource implements IProjectDataSource { ref: edge.node }) }) - const defaultBranchName = repository.defaultBranchRef.name - const candidateDefaultBranches = [ - defaultBranchName, "main", "master", "develop", "development" - ] - // Reverse them so the top-priority branches end up at the top of the list. - .reverse() - const allVersions = branchVersions.concat(tagVersions).sort((a: Version, b: Version) => { - return a.name.localeCompare(b.name) - }) - // Move the top-priority branches to the top of the list. - for (const candidateDefaultBranch of candidateDefaultBranches) { - const defaultBranchIndex = allVersions.findIndex((version: Version) => { - return version.name === candidateDefaultBranch - }) - if (defaultBranchIndex !== -1) { - const branchVersion = allVersions[defaultBranchIndex] - allVersions.splice(defaultBranchIndex, 1) - allVersions.splice(0, 0, branchVersion) - } - } - return allVersions + return branchVersions.concat(tagVersions) } private mapVersionFromRef({ @@ -163,4 +151,63 @@ export default class GitHubProjectDataSource implements IProjectDataSource { }): string { return `/api/blob/${ownerName}/${repositoryName}/${path}?ref=${ref}` } + + private addRemoteVersions( + existingVersions: Version[], + remoteVersions: ProjectConfigRemoteVersion[] + ): Version[] { + const versions = [...existingVersions] + const versionIds = versions.map(e => e.id) + for (const remoteVersion of remoteVersions) { + const baseVersionId = this.makeURLSafeID( + remoteVersion.id?.toLowerCase() || remoteVersion.name.toLowerCase() + ) + // If the version ID exists then we suffix it with a number to ensure unique versions. + // E.g. if "foo" already exists, we make it "foo1". + const existingVersionIdCount = versionIds.filter(e => e == baseVersionId).length + const versionId = baseVersionId + (existingVersionIdCount > 0 ? existingVersionIdCount : "") + const specifications = remoteVersion.specifications.map(e => { + return { + id: this.makeURLSafeID(e.name.toLowerCase()), + name: e.name, + url: `/api/proxy?url=${encodeURIComponent(e.url)}` + } + }) + versions.push({ + id: versionId, + name: remoteVersion.name, + specifications, + isDefault: false + }) + versionIds.push(baseVersionId) + } + return versions + } + + private sortVersions(versions: Version[], defaultBranchName: string): Version[] { + const candidateDefaultBranches = [ + defaultBranchName, "main", "master", "develop", "development" + ] + // Reverse them so the top-priority branches end up at the top of the list. + .reverse() + const copiedVersions = [...versions].sort((a, b) => { + return a.name.localeCompare(b.name) + }) + // Move the top-priority branches to the top of the list. + for (const candidateDefaultBranch of candidateDefaultBranches) { + const defaultBranchIndex = copiedVersions.findIndex(version => { + return version.name === candidateDefaultBranch + }) + if (defaultBranchIndex !== -1) { + const branchVersion = copiedVersions[defaultBranchIndex] + copiedVersions.splice(defaultBranchIndex, 1) + copiedVersions.splice(0, 0, branchVersion) + } + } + return copiedVersions + } + + private makeURLSafeID(str: string): string { + return str.replace(/\s/g, "") + } } diff --git a/src/features/projects/domain/IProjectConfig.ts b/src/features/projects/domain/IProjectConfig.ts index 3dc32a0e..b0c3f015 100644 --- a/src/features/projects/domain/IProjectConfig.ts +++ b/src/features/projects/domain/IProjectConfig.ts @@ -1,4 +1,16 @@ +export type ProjectConfigRemoteVersion = { + readonly id?: string + readonly name: string + readonly specifications: ProjectConfigRemoteSpecification[] +} + +export type ProjectConfigRemoteSpecification = { + readonly name: string + readonly url: string +} + export default interface IProjectConfig { readonly name?: string readonly image?: string + readonly remoteVersions?: ProjectConfigRemoteVersion[] } diff --git a/src/features/projects/domain/index.ts b/src/features/projects/domain/index.ts index 3d8006dc..63eca5d7 100644 --- a/src/features/projects/domain/index.ts +++ b/src/features/projects/domain/index.ts @@ -1,6 +1,7 @@ export { default as CachingProjectDataSource } from "./CachingProjectDataSource" export { default as getSelection } from "./getSelection" export type { default as IProjectConfig } from "./IProjectConfig" +export * from "./IProjectConfig" export type { default as IProjectDataSource } from "./IProjectDataSource" export type { default as OpenApiSpecification } from "./OpenApiSpecification" export type { default as Project } from "./Project" From 7805b8c1e316e7d5bf722c764b0fc27931ef41fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20St=C3=B8vring?= Date: Sat, 11 Nov 2023 14:31:52 +0100 Subject: [PATCH 04/12] Adds /api/proxy endpoint --- src/app/api/proxy/route.ts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 src/app/api/proxy/route.ts diff --git a/src/app/api/proxy/route.ts b/src/app/api/proxy/route.ts new file mode 100644 index 00000000..7245898c --- /dev/null +++ b/src/app/api/proxy/route.ts @@ -0,0 +1,17 @@ +import { NextRequest, NextResponse } from "next/server" +import { makeAPIErrorResponse } from "@/common" + +export async function GET(req: NextRequest) { + const rawURL = req.nextUrl.searchParams.get("url") + if (!rawURL) { + return makeAPIErrorResponse(400, "Missing \"url\" query parameter.") + } + let url: URL + try { + url = new URL(rawURL) + } catch { + return makeAPIErrorResponse(400, "Invalid \"url\" query parameter.") + } + const file = await fetch(url).then(r => r.blob()) + return new NextResponse(file, { status: 200 }) +} From 43cc10a72db4073729a74923e0364ff88d4a9903 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20St=C3=B8vring?= Date: Sat, 11 Nov 2023 15:11:12 +0100 Subject: [PATCH 05/12] Fixes unit tests --- __test__/projects/GitHubProjectDataSource.test.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/__test__/projects/GitHubProjectDataSource.test.ts b/__test__/projects/GitHubProjectDataSource.test.ts index db8acb59..4ab00e67 100644 --- a/__test__/projects/GitHubProjectDataSource.test.ts +++ b/__test__/projects/GitHubProjectDataSource.test.ts @@ -1141,11 +1141,11 @@ test("It adds remote versions from the project configuration", async () => { specifications: [{ id: "huey", name: "Huey", - url: "https://example.com/huey.yml" + url: `/api/proxy?url=${encodeURIComponent("https://example.com/huey.yml")}` }, { id: "dewey", name: "Dewey", - url: "https://example.com/dewey.yml" + url: `/api/proxy?url=${encodeURIComponent("https://example.com/dewey.yml")}` }] }, { id: "bobby", @@ -1154,7 +1154,7 @@ test("It adds remote versions from the project configuration", async () => { specifications: [{ id: "louie", name: "Louie", - url: "https://example.com/louie.yml" + url: `/api/proxy?url=${encodeURIComponent("https://example.com/louie.yml")}` }] }]) }) @@ -1229,7 +1229,7 @@ test("It modifies ID of remote version if the ID already exists", async () => { specifications: [{ id: "baz", name: "Baz", - url: "https://example.com/baz.yml" + url: `/api/proxy?url=${encodeURIComponent("https://example.com/baz.yml")}` }] }, { id: "bar2", @@ -1238,7 +1238,7 @@ test("It modifies ID of remote version if the ID already exists", async () => { specifications: [{ id: "hello", name: "Hello", - url: "https://example.com/hello.yml" + url: `/api/proxy?url=${encodeURIComponent("https://example.com/hello.yml")}` }] }]) }) From 8b5922b4418bb2a8a9d27775085d565aa4d8370b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20St=C3=B8vring?= Date: Sat, 11 Nov 2023 15:25:00 +0100 Subject: [PATCH 06/12] Simplifies project selection --- __test__/common/utils/url.test.ts | 85 ------------------- __test__/projects/getSelection.test.ts | 28 +++--- src/app/[...slug]/page.tsx | 19 ++--- src/app/page.tsx | 2 +- src/common/utils/index.ts | 1 - src/common/utils/url.ts | 47 ---------- src/features/projects/domain/getSelection.ts | 33 +++++-- .../projects/domain/projectNavigator.ts | 16 ++-- src/features/projects/view/ProjectsPage.tsx | 12 +-- .../projects/view/client/ProjectsPage.tsx | 39 +++------ 10 files changed, 65 insertions(+), 217 deletions(-) delete mode 100644 __test__/common/utils/url.test.ts delete mode 100644 src/common/utils/url.ts diff --git a/__test__/common/utils/url.test.ts b/__test__/common/utils/url.test.ts deleted file mode 100644 index f52e7c95..00000000 --- a/__test__/common/utils/url.test.ts +++ /dev/null @@ -1,85 +0,0 @@ -import { - getProjectId, - getVersionId, - getSpecificationId -} from "../../../src/common" - -test("It reads path containing project only", async () => { - const url = "/foo" - const projectId = getProjectId(url) - const versionId = getVersionId(url) - const specificationId = getSpecificationId(url) - expect(projectId).toEqual("foo") - expect(versionId).toBeUndefined() - expect(specificationId).toBeUndefined() -}) - -test("It reads path containing project and version", async () => { - const url = "/foo/bar" - const projectId = getProjectId(url) - const versionId = getVersionId(url) - const specificationId = getSpecificationId(url) - expect(projectId).toEqual("foo") - expect(versionId).toEqual("bar") - expect(specificationId).toBeUndefined() -}) - -test("It reads path containing project, version, and specification with .yml extension", async () => { - const url = "/foo/bar/openapi.yml" - const projectId = getProjectId(url) - const versionId = getVersionId(url) - const specificationId = getSpecificationId(url) - expect(projectId).toEqual("foo") - expect(versionId).toEqual("bar") - expect(specificationId).toBe("openapi.yml") -}) - -test("It reads path containing project, version, and specification with .yaml extension", async () => { - const url = "/foo/bar/openapi.yaml" - const projectId = getProjectId(url) - const versionId = getVersionId(url) - const specificationId = getSpecificationId(url) - expect(projectId).toEqual("foo") - expect(versionId).toEqual("bar") - expect(specificationId).toBe("openapi.yaml") -}) - -test("It parses specification without trailing file extension", async () => { - const url = "/foo/bar/baz" - const projectId = getProjectId(url) - const versionId = getVersionId(url) - const specificationId = getSpecificationId(url) - expect(projectId).toEqual("foo") - expect(versionId).toEqual("bar") - expect(specificationId).toBe("baz") -}) - -test("It read specification when version contains a slash", async () => { - const url = "/foo/bar/baz/openapi.yml" - const projectId = getProjectId(url) - const versionId = getVersionId(url) - const specificationId = getSpecificationId(url) - expect(projectId).toEqual("foo") - expect(versionId).toEqual("bar/baz") - expect(specificationId).toBe("openapi.yml") -}) - -test("It read specification when version contains three slashes", async () => { - const url = "/foo/bar/baz/hello/openapi.yml" - const projectId = getProjectId(url) - const versionId = getVersionId(url) - const specificationId = getSpecificationId(url) - expect(projectId).toEqual("foo") - expect(versionId).toEqual("bar/baz/hello") - expect(specificationId).toBe("openapi.yml") -}) - -test("It does not remove \"-openapi\" suffix from project", async () => { - const url = "/foo-openapi" - const projectId = getProjectId(url) - const versionId = getVersionId(url) - const specificationId = getSpecificationId(url) - expect(projectId).toEqual("foo-openapi") - expect(versionId).toBeUndefined() - expect(specificationId).toBeUndefined() -}) diff --git a/__test__/projects/getSelection.test.ts b/__test__/projects/getSelection.test.ts index 02b22cc4..a3979b44 100644 --- a/__test__/projects/getSelection.test.ts +++ b/__test__/projects/getSelection.test.ts @@ -1,7 +1,8 @@ import { getSelection } from "../../src/features/projects/domain" -test("It selects the first project when there is only one project", () => { +test("It selects the first project when there is only one project and path is empty", () => { const sut = getSelection({ + path: "", projects: [{ id: "foo", name: "foo", @@ -25,7 +26,7 @@ test("It selects the first project when there is only one project", () => { test("It selects the first version and specification of the specified project", () => { const sut = getSelection({ - projectId: "bar", + path: "/bar", projects: [{ id: "foo", name: "foo", @@ -63,8 +64,7 @@ test("It selects the first version and specification of the specified project", test("It selects the first specification of the specified project and version", () => { const sut = getSelection({ - projectId: "bar", - versionId: "baz2", + path: "/bar/baz2", projects: [{ id: "foo", name: "foo", @@ -98,8 +98,7 @@ test("It selects the first specification of the specified project and version", test("It selects the specification of the specified version", () => { const sut = getSelection({ - projectId: "bar", - versionId: "baz2", + path: "/bar/baz2", projects: [{ id: "foo", name: "foo", @@ -137,9 +136,7 @@ test("It selects the specification of the specified version", () => { test("It selects the specified project, version, and specification", () => { const sut = getSelection({ - projectId: "bar", - versionId: "baz2", - specificationId: "hello2", + path: "/bar/baz2/hello2", projects: [{ id: "foo", name: "foo", @@ -177,7 +174,7 @@ test("It selects the specified project, version, and specification", () => { test("It returns a undefined project, version, and specification when the selected project cannot be found", () => { const sut = getSelection({ - projectId: "foo", + path: "/foo", projects: [{ id: "bar", name: "bar", @@ -192,8 +189,7 @@ test("It returns a undefined project, version, and specification when the select test("It returns a undefined version and specification when the selected version cannot be found", () => { const sut = getSelection({ - projectId: "foo", - versionId: "bar", + path: "/foo/bar", projects: [{ id: "foo", name: "foo", @@ -213,9 +209,7 @@ test("It returns a undefined version and specification when the selected version test("It returns a undefined specification when the selected specification cannot be found", () => { const sut = getSelection({ - projectId: "foo", - versionId: "bar", - specificationId: "baz", + path: "/foo/bar/baz", projects: [{ id: "foo", name: "foo", @@ -239,9 +233,7 @@ test("It returns a undefined specification when the selected specification canno test("It moves specification ID to version ID if needed", () => { const sut = getSelection({ - projectId: "foo", - versionId: "bar", - specificationId: "baz", + path: "/foo/bar/baz", projects: [{ id: "foo", name: "foo", diff --git a/src/app/[...slug]/page.tsx b/src/app/[...slug]/page.tsx index 984a5cc9..76b93de3 100644 --- a/src/app/[...slug]/page.tsx +++ b/src/app/[...slug]/page.tsx @@ -1,4 +1,3 @@ -import { getProjectId, getSpecificationId, getVersionId } from "../../common" import SessionBarrier from "@/features/auth/view/SessionBarrier" import ProjectsPage from "@/features/projects/view/ProjectsPage" import { projectRepository } from "@/composition" @@ -6,26 +5,20 @@ import { projectRepository } from "@/composition" type PageParams = { slug: string | string[] } export default async function Page({ params }: { params: PageParams }) { - const url = getURL(params) return ( ) } -function getURL(params: PageParams) { - if (typeof params.slug === "string") { - return "/" + params.slug +function getPath(slug: string | string[]) { + if (typeof slug === "string") { + return "/" + slug } else { - return params.slug.reduce( - (previousValue, currentValue) => `${previousValue}/${currentValue}`, - "" - ) + return slug.reduce((e, acc) => `${e}/${acc}`, "") } -} \ No newline at end of file +} diff --git a/src/app/page.tsx b/src/app/page.tsx index 01dfa211..0d97feea 100644 --- a/src/app/page.tsx +++ b/src/app/page.tsx @@ -5,7 +5,7 @@ import { projectRepository } from "@/composition" export default async function Page() { return ( - + ) } diff --git a/src/common/utils/index.ts b/src/common/utils/index.ts index 33352dd4..0b808be9 100644 --- a/src/common/utils/index.ts +++ b/src/common/utils/index.ts @@ -1,4 +1,3 @@ export * from "./fetcher" export { default as fetcher } from "./fetcher" -export * from "./url" export { default as ZodJSONCoder } from "./ZodJSONCoder" diff --git a/src/common/utils/url.ts b/src/common/utils/url.ts deleted file mode 100644 index 507750b9..00000000 --- a/src/common/utils/url.ts +++ /dev/null @@ -1,47 +0,0 @@ -function getPathname(url?: string) { - if (typeof window !== "undefined") { - url = window.location.pathname - } - if (!url) { - return undefined - } - if (!url.startsWith("/")) { - return url - } - return url.substring(1) -} - -function getProjectSelection(tmpPathname?: string) { - const pathname = getPathname(tmpPathname) - if (!pathname) { - return undefined - } - const comps = pathname.split("/") - if (comps.length == 0) { - return {} - } else if (comps.length == 1) { - return { projectId: comps[0] } - } else if (comps.length == 2) { - return { - projectId: comps[0], - versionId: comps[1] - } - } else { - const projectId = comps[0] - const versionId = comps.slice(1, -1).join("/") - const specificationId = comps[comps.length - 1] - return { projectId, versionId, specificationId } - } -} - -export function getProjectId(tmpPathname?: string) { - return getProjectSelection(tmpPathname)?.projectId -} - -export function getVersionId(tmpPathname?: string) { - return getProjectSelection(tmpPathname)?.versionId -} - -export function getSpecificationId(tmpPathname?: string) { - return getProjectSelection(tmpPathname)?.specificationId -} diff --git a/src/features/projects/domain/getSelection.ts b/src/features/projects/domain/getSelection.ts index 50fd7f2e..b289e82b 100644 --- a/src/features/projects/domain/getSelection.ts +++ b/src/features/projects/domain/getSelection.ts @@ -4,19 +4,19 @@ import OpenApiSpecification from "./OpenApiSpecification" export default function getSelection({ projects, - projectId, - versionId, - specificationId, + path }: { projects: Project[], - projectId?: string, - versionId?: string, - specificationId?: string + path: string }): { project?: Project, version?: Version, specification?: OpenApiSpecification } { + if (path.startsWith("/")) { + path = path.substring(1) + } + let { projectId, versionId, specificationId } = guessSelection(path) // If no project is selected and the user only has a single project then we select that. if (!projectId && projects.length == 1) { projectId = projects[0].id @@ -62,6 +62,25 @@ export default function getSelection({ return { project, version, specification } } +function guessSelection(pathname: string) { + const comps = pathname.split("/") + if (comps.length == 0) { + return {} + } else if (comps.length == 1) { + return { projectId: comps[0] } + } else if (comps.length == 2) { + return { + projectId: comps[0], + versionId: comps[1] + } + } else { + const projectId = comps[0] + const versionId = comps.slice(1, -1).join("/") + const specificationId = comps[comps.length - 1] + return { projectId, versionId, specificationId } + } +} + function isSpecificationIdFilename(specificationId: string): boolean { return specificationId.endsWith(".yml") || specificationId.endsWith(".yaml") -} \ No newline at end of file +} diff --git a/src/features/projects/domain/projectNavigator.ts b/src/features/projects/domain/projectNavigator.ts index 6aa1ce8c..d85db8b3 100644 --- a/src/features/projects/domain/projectNavigator.ts +++ b/src/features/projects/domain/projectNavigator.ts @@ -39,26 +39,20 @@ const projectNavigator = { }, navigateIfNeeded( router: IProjectRouter, - urlComponents: { - projectId?: string, - versionId?: string, - specificationId?: string - }, selection: { projectId?: string, versionId?: string, specificationId?: string } ) { + if (typeof window === "undefined") { + return + } if (!selection.projectId || !selection.versionId || !selection.specificationId) { return } - if ( - urlComponents.projectId != selection.projectId || - urlComponents.versionId != selection.versionId || - urlComponents.specificationId != selection.specificationId - ) { - const path = `/${selection.projectId}/${selection.versionId}/${selection.specificationId}` + const path = `/${selection.projectId}/${selection.versionId}/${selection.specificationId}` + if (path !== window.location.pathname) { router.replace(path) } } diff --git a/src/features/projects/view/ProjectsPage.tsx b/src/features/projects/view/ProjectsPage.tsx index 5e5118d4..be16f64c 100644 --- a/src/features/projects/view/ProjectsPage.tsx +++ b/src/features/projects/view/ProjectsPage.tsx @@ -4,14 +4,10 @@ import ClientProjectsPage from "./client/ProjectsPage" export default async function ProjectsPage({ projectRepository, - projectId, - versionId, - specificationId + path }: { projectRepository: ProjectRepository - projectId?: string - versionId?: string - specificationId?: string + path: string }) { const isGuest = await session.getIsGuest() const projects = await projectRepository.get() @@ -19,9 +15,7 @@ export default async function ProjectsPage({ ) } diff --git a/src/features/projects/view/client/ProjectsPage.tsx b/src/features/projects/view/client/ProjectsPage.tsx index 0fa9d384..167965d8 100644 --- a/src/features/projects/view/client/ProjectsPage.tsx +++ b/src/features/projects/view/client/ProjectsPage.tsx @@ -16,15 +16,11 @@ import useSidebarOpen from "@/common/state/useSidebarOpen" export default function ProjectsPage({ enableGitHubLinks, projects: serverProjects, - projectId, - versionId, - specificationId + path }: { - enableGitHubLinks: boolean, + enableGitHubLinks: boolean projects?: Project[] - projectId?: string - versionId?: string - specificationId?: string + path: string }) { const router = useRouter() const theme = useTheme() @@ -32,12 +28,7 @@ export default function ProjectsPage({ const isDesktopLayout = useMediaQuery(theme.breakpoints.up("sm")) const { projects: clientProjects, error, isLoading: isClientLoading } = useProjects() const projects = isClientLoading ? (serverProjects || []) : clientProjects - const { project, version, specification } = getSelection({ - projects, - projectId, - versionId, - specificationId - }) + const { project, version, specification } = getSelection({ projects, path }) useEffect(() => { updateWindowTitle({ storage: document, @@ -49,20 +40,18 @@ export default function ProjectsPage({ }, [project, version, specification]) useEffect(() => { // Ensure the URL reflects the current selection of project, version, and specification. - const urlSelection = { projectId, versionId, specificationId } - const selection = { + projectNavigator.navigateIfNeeded(router, { projectId: project?.id, versionId: version?.id, specificationId: specification?.id - } - projectNavigator.navigateIfNeeded(router, urlSelection, selection) - }, [router, projectId, versionId, specificationId, project, version, specification]) + }) + }, [router, project, version, specification, project, version, specification]) useEffect(() => { // Show the sidebar if no project is selected. - if (projectId === undefined) { + if (project?.id === undefined) { setSidebarOpen(true) } - }, [projectId, setSidebarOpen]) + }, [project?.id, setSidebarOpen]) const selectProject = (project: Project) => { if (!isDesktopLayout) { setSidebarOpen(false) @@ -75,9 +64,9 @@ export default function ProjectsPage({ projectNavigator.navigateToVersion(router, project!, versionId, specification!.name) } const selectSpecification = (specificationId: string) => { - projectNavigator.navigate(router, projectId!, versionId!, specificationId) + projectNavigator.navigate(router, project!.id, version!.id, specificationId) } - const canCloseSidebar = projectId !== undefined + const canCloseSidebar = project?.id !== undefined const toggleSidebar = (isOpen: boolean) => { if (!isOpen && canCloseSidebar) { setSidebarOpen(false) @@ -94,7 +83,7 @@ export default function ProjectsPage({ } @@ -119,7 +108,7 @@ export default function ProjectsPage({ } > {/* If the user has not selected any project then we do not render any content */} - {projectId && + {project && ) -} +} \ No newline at end of file From bb67443f9b8483e7e299ee714c177bbbfd926fcd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20St=C3=B8vring?= Date: Sat, 11 Nov 2023 15:36:17 +0100 Subject: [PATCH 07/12] Fixes projectNavigator unit tests --- __test__/projects/projectNavigator.test.ts | 23 ++++++------------- .../projects/domain/projectNavigator.ts | 6 ++--- .../projects/view/client/ProjectsPage.tsx | 15 ++++++++---- 3 files changed, 20 insertions(+), 24 deletions(-) diff --git a/__test__/projects/projectNavigator.test.ts b/__test__/projects/projectNavigator.test.ts index 06c30ac1..441ae383 100644 --- a/__test__/projects/projectNavigator.test.ts +++ b/__test__/projects/projectNavigator.test.ts @@ -3,6 +3,7 @@ import { projectNavigator } from "../../src/features/projects/domain" test("It navigates to the correct path", async () => { let pushedPath: string | undefined const router = { + pathname: "/", push: (path: string) => { pushedPath = path }, @@ -39,6 +40,7 @@ test("It navigates to first specification when changing version", async () => { } let pushedPath: string | undefined const router = { + pathname: "/", push: (path: string) => { pushedPath = path }, @@ -91,6 +93,7 @@ test("It finds a specification with the same name when changing version", async } let pushedPath: string | undefined const router = { + pathname: "/", push: (path: string) => { pushedPath = path }, @@ -103,6 +106,7 @@ test("It finds a specification with the same name when changing version", async test("It skips navigating when URL matches selection", async () => { let didNavigate = false const router = { + pathname: "/foo/bar/baz", push: () => {}, replace: () => { didNavigate = true @@ -112,10 +116,6 @@ test("It skips navigating when URL matches selection", async () => { projectId: "foo", versionId: "bar", specificationId: "baz" - }, { - projectId: "foo", - versionId: "bar", - specificationId: "baz" }) expect(didNavigate).toBeFalsy() }) @@ -123,6 +123,7 @@ test("It skips navigating when URL matches selection", async () => { test("It navigates when project ID in URL does not match ID of selected project", async () => { let didNavigate = false const router = { + pathname: "/hello/bar/baz", push: () => {}, replace: () => { didNavigate = true @@ -132,10 +133,6 @@ test("It navigates when project ID in URL does not match ID of selected project" projectId: "foo", versionId: "bar", specificationId: "baz" - }, { - projectId: "hello", - versionId: "bar", - specificationId: "baz" }) expect(didNavigate).toBeTruthy() }) @@ -143,6 +140,7 @@ test("It navigates when project ID in URL does not match ID of selected project" test("It navigates when version ID in URL does not match ID of selected version", async () => { let didNavigate = false const router = { + pathname: "/foo/hello/baz", push: () => {}, replace: () => { didNavigate = true @@ -152,10 +150,6 @@ test("It navigates when version ID in URL does not match ID of selected version" projectId: "foo", versionId: "bar", specificationId: "baz" - }, { - projectId: "foo", - versionId: "hello", - specificationId: "baz" }) expect(didNavigate).toBeTruthy() }) @@ -163,6 +157,7 @@ test("It navigates when version ID in URL does not match ID of selected version" test("It navigates when specification ID in URL does not match ID of selected specification", async () => { let didNavigate = false const router = { + pathname: "/foo/bar/hello", push: () => {}, replace: () => { didNavigate = true @@ -172,10 +167,6 @@ test("It navigates when specification ID in URL does not match ID of selected sp projectId: "foo", versionId: "bar", specificationId: "baz" - }, { - projectId: "foo", - versionId: "bar", - specificationId: "hello" }) expect(didNavigate).toBeTruthy() }) diff --git a/src/features/projects/domain/projectNavigator.ts b/src/features/projects/domain/projectNavigator.ts index d85db8b3..8eb0aab4 100644 --- a/src/features/projects/domain/projectNavigator.ts +++ b/src/features/projects/domain/projectNavigator.ts @@ -1,6 +1,7 @@ import Project from "./Project" export interface IProjectRouter { + readonly pathname: string push(path: string): void replace(path: string): void } @@ -45,14 +46,11 @@ const projectNavigator = { specificationId?: string } ) { - if (typeof window === "undefined") { - return - } if (!selection.projectId || !selection.versionId || !selection.specificationId) { return } const path = `/${selection.projectId}/${selection.versionId}/${selection.specificationId}` - if (path !== window.location.pathname) { + if (path !== router.pathname) { router.replace(path) } } diff --git a/src/features/projects/view/client/ProjectsPage.tsx b/src/features/projects/view/client/ProjectsPage.tsx index 167965d8..1348e75c 100644 --- a/src/features/projects/view/client/ProjectsPage.tsx +++ b/src/features/projects/view/client/ProjectsPage.tsx @@ -23,6 +23,13 @@ export default function ProjectsPage({ path: string }) { const router = useRouter() + const projectNavigatorRouter = { + get pathname() { + return window.location.pathname + }, + push: router.push, + replace: router.replace + } const theme = useTheme() const [isSidebarOpen, setSidebarOpen] = useSidebarOpen() const isDesktopLayout = useMediaQuery(theme.breakpoints.up("sm")) @@ -40,7 +47,7 @@ export default function ProjectsPage({ }, [project, version, specification]) useEffect(() => { // Ensure the URL reflects the current selection of project, version, and specification. - projectNavigator.navigateIfNeeded(router, { + projectNavigator.navigateIfNeeded(projectNavigatorRouter, { projectId: project?.id, versionId: version?.id, specificationId: specification?.id @@ -58,13 +65,13 @@ export default function ProjectsPage({ } const version = project.versions[0] const specification = version.specifications[0] - projectNavigator.navigate(router, project.id, version.id, specification.id) + projectNavigator.navigate(projectNavigatorRouter, project.id, version.id, specification.id) } const selectVersion = (versionId: string) => { - projectNavigator.navigateToVersion(router, project!, versionId, specification!.name) + projectNavigator.navigateToVersion(projectNavigatorRouter, project!, versionId, specification!.name) } const selectSpecification = (specificationId: string) => { - projectNavigator.navigate(router, project!.id, version!.id, specificationId) + projectNavigator.navigate(projectNavigatorRouter, project!.id, version!.id, specificationId) } const canCloseSidebar = project?.id !== undefined const toggleSidebar = (isOpen: boolean) => { From c13b155164f2e6a38b8efbf813a52ca80862b9a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20St=C3=B8vring?= Date: Sat, 11 Nov 2023 15:36:37 +0100 Subject: [PATCH 08/12] Enables configuring specification ID --- .../projects/GitHubProjectDataSource.test.ts | 98 +++++++++++++++++++ .../projects/data/GitHubProjectDataSource.ts | 4 +- .../projects/domain/IProjectConfig.ts | 1 + 3 files changed, 101 insertions(+), 2 deletions(-) diff --git a/__test__/projects/GitHubProjectDataSource.test.ts b/__test__/projects/GitHubProjectDataSource.test.ts index 4ab00e67..14d0df5b 100644 --- a/__test__/projects/GitHubProjectDataSource.test.ts +++ b/__test__/projects/GitHubProjectDataSource.test.ts @@ -1242,3 +1242,101 @@ test("It modifies ID of remote version if the ID already exists", async () => { }] }]) }) + +test("It lets users specify the ID of a remote version", async () => { + const rawProjectConfig = ` + remoteVersions: + - id: some-version + name: Bar + specifications: + - name: Baz + url: https://example.com/baz.yml + ` + const sut = new GitHubProjectDataSource({ + dataSource: { + async getRepositories() { + return [{ + name: "foo", + owner: { + login: "acme" + }, + defaultBranchRef: { + name: "bar", + target: { + oid: "12345678" + } + }, + configYml: { + text: rawProjectConfig + }, + branches: { + edges: [] + }, + tags: { + edges: [] + } + }] + } + } + }) + const projects = await sut.getProjects() + expect(projects[0].versions).toEqual([{ + id: "some-version", + name: "Bar", + isDefault: false, + specifications: [{ + id: "baz", + name: "Baz", + url: `/api/proxy?url=${encodeURIComponent("https://example.com/baz.yml")}` + }] + }]) +}) + +test("It lets users specify the ID of a remote specification", async () => { + const rawProjectConfig = ` + remoteVersions: + - name: Bar + specifications: + - id: some-spec + name: Baz + url: https://example.com/baz.yml + ` + const sut = new GitHubProjectDataSource({ + dataSource: { + async getRepositories() { + return [{ + name: "foo", + owner: { + login: "acme" + }, + defaultBranchRef: { + name: "bar", + target: { + oid: "12345678" + } + }, + configYml: { + text: rawProjectConfig + }, + branches: { + edges: [] + }, + tags: { + edges: [] + } + }] + } + } + }) + const projects = await sut.getProjects() + expect(projects[0].versions).toEqual([{ + id: "bar", + name: "Bar", + isDefault: false, + specifications: [{ + id: "some-spec", + name: "Baz", + url: `/api/proxy?url=${encodeURIComponent("https://example.com/baz.yml")}` + }] + }]) +}) diff --git a/src/features/projects/data/GitHubProjectDataSource.ts b/src/features/projects/data/GitHubProjectDataSource.ts index 9225ceeb..3de5dda9 100644 --- a/src/features/projects/data/GitHubProjectDataSource.ts +++ b/src/features/projects/data/GitHubProjectDataSource.ts @@ -160,7 +160,7 @@ export default class GitHubProjectDataSource implements IProjectDataSource { const versionIds = versions.map(e => e.id) for (const remoteVersion of remoteVersions) { const baseVersionId = this.makeURLSafeID( - remoteVersion.id?.toLowerCase() || remoteVersion.name.toLowerCase() + (remoteVersion.id || remoteVersion.name).toLowerCase() ) // If the version ID exists then we suffix it with a number to ensure unique versions. // E.g. if "foo" already exists, we make it "foo1". @@ -168,7 +168,7 @@ export default class GitHubProjectDataSource implements IProjectDataSource { const versionId = baseVersionId + (existingVersionIdCount > 0 ? existingVersionIdCount : "") const specifications = remoteVersion.specifications.map(e => { return { - id: this.makeURLSafeID(e.name.toLowerCase()), + id: this.makeURLSafeID((e.id || e.name).toLowerCase()), name: e.name, url: `/api/proxy?url=${encodeURIComponent(e.url)}` } diff --git a/src/features/projects/domain/IProjectConfig.ts b/src/features/projects/domain/IProjectConfig.ts index b0c3f015..7f047f19 100644 --- a/src/features/projects/domain/IProjectConfig.ts +++ b/src/features/projects/domain/IProjectConfig.ts @@ -5,6 +5,7 @@ export type ProjectConfigRemoteVersion = { } export type ProjectConfigRemoteSpecification = { + readonly id?: string readonly name: string readonly url: string } From 4f64b6ddb221f9cbcb8ae1b4bdb3810aa934b3d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20St=C3=B8vring?= Date: Sat, 11 Nov 2023 15:36:46 +0100 Subject: [PATCH 09/12] Improves automatic specification ID --- src/features/projects/data/GitHubProjectDataSource.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/features/projects/data/GitHubProjectDataSource.ts b/src/features/projects/data/GitHubProjectDataSource.ts index 3de5dda9..40efddcc 100644 --- a/src/features/projects/data/GitHubProjectDataSource.ts +++ b/src/features/projects/data/GitHubProjectDataSource.ts @@ -208,6 +208,8 @@ export default class GitHubProjectDataSource implements IProjectDataSource { } private makeURLSafeID(str: string): string { - return str.replace(/\s/g, "") + return str + .replace(/ /g, "-") + .replace(/[^A-Za-z0-9-]/g, "") } } From ed6f3c71a03321f70587a1b9f2191b3ac087939d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20St=C3=B8vring?= Date: Sat, 11 Nov 2023 15:52:37 +0100 Subject: [PATCH 10/12] Improves navigation logic --- __test__/projects/projectNavigator.test.ts | 142 ++++++++++++------ .../projects/domain/WindowPathnameReader.ts | 8 + src/features/projects/domain/getSelection.ts | 3 +- src/features/projects/domain/index.ts | 3 +- .../projects/domain/projectNavigator.ts | 41 +++-- .../projects/view/client/ProjectsPage.tsx | 27 ++-- 6 files changed, 145 insertions(+), 79 deletions(-) create mode 100644 src/features/projects/domain/WindowPathnameReader.ts diff --git a/__test__/projects/projectNavigator.test.ts b/__test__/projects/projectNavigator.test.ts index 441ae383..e80d4d26 100644 --- a/__test__/projects/projectNavigator.test.ts +++ b/__test__/projects/projectNavigator.test.ts @@ -1,15 +1,21 @@ -import { projectNavigator } from "../../src/features/projects/domain" +import { ProjectNavigator } from "../../src/features/projects/domain" test("It navigates to the correct path", async () => { let pushedPath: string | undefined - const router = { - pathname: "/", - push: (path: string) => { - pushedPath = path + const sut = new ProjectNavigator({ + pathnameReader: { + get pathname() { + return "/" + } }, - replace: () => {} - } - projectNavigator.navigate(router, "foo", "bar", "hello.yml") + router: { + push: (path: string) => { + pushedPath = path + }, + replace: () => {} + } + }) + sut.navigate("foo", "bar", "hello.yml") expect(pushedPath).toEqual("/foo/bar/hello.yml") }) @@ -39,14 +45,20 @@ test("It navigates to first specification when changing version", async () => { }] } let pushedPath: string | undefined - const router = { - pathname: "/", - push: (path: string) => { - pushedPath = path + const sut = new ProjectNavigator({ + pathnameReader: { + get pathname() { + return "/" + } }, - replace: () => {} - } - projectNavigator.navigateToVersion(router, project, "hello", "baz.yml") + router: { + push: (path: string) => { + pushedPath = path + }, + replace: () => {} + } + }) + sut.navigateToVersion(project, "hello", "baz.yml") expect(pushedPath).toEqual("/foo/hello/world.yml") }) @@ -92,27 +104,39 @@ test("It finds a specification with the same name when changing version", async }] } let pushedPath: string | undefined - const router = { - pathname: "/", - push: (path: string) => { - pushedPath = path + const sut = new ProjectNavigator({ + pathnameReader: { + get pathname() { + return "/" + } }, - replace: () => {} - } - projectNavigator.navigateToVersion(router, project, "baz", "earth.yml") + router: { + push: (path: string) => { + pushedPath = path + }, + replace: () => {} + } + }) + sut.navigateToVersion(project, "baz", "earth.yml") expect(pushedPath).toEqual("/foo/baz/earth.yml") }) test("It skips navigating when URL matches selection", async () => { let didNavigate = false - const router = { - pathname: "/foo/bar/baz", - push: () => {}, - replace: () => { - didNavigate = true + const sut = new ProjectNavigator({ + pathnameReader: { + get pathname() { + return "/foo/bar/baz" + } + }, + router: { + push: () => {}, + replace: () => { + didNavigate = true + } } - } - projectNavigator.navigateIfNeeded(router, { + }) + sut.navigateIfNeeded({ projectId: "foo", versionId: "bar", specificationId: "baz" @@ -122,14 +146,20 @@ test("It skips navigating when URL matches selection", async () => { test("It navigates when project ID in URL does not match ID of selected project", async () => { let didNavigate = false - const router = { - pathname: "/hello/bar/baz", - push: () => {}, - replace: () => { - didNavigate = true + const sut = new ProjectNavigator({ + pathnameReader: { + get pathname() { + return "/hello/bar/baz" + } + }, + router: { + push: () => {}, + replace: () => { + didNavigate = true + } } - } - projectNavigator.navigateIfNeeded(router, { + }) + sut.navigateIfNeeded({ projectId: "foo", versionId: "bar", specificationId: "baz" @@ -139,14 +169,20 @@ test("It navigates when project ID in URL does not match ID of selected project" test("It navigates when version ID in URL does not match ID of selected version", async () => { let didNavigate = false - const router = { - pathname: "/foo/hello/baz", - push: () => {}, - replace: () => { - didNavigate = true + const sut = new ProjectNavigator({ + pathnameReader: { + get pathname() { + return "/foo/hello/baz" + } + }, + router: { + push: () => {}, + replace: () => { + didNavigate = true + } } - } - projectNavigator.navigateIfNeeded(router, { + }) + sut.navigateIfNeeded({ projectId: "foo", versionId: "bar", specificationId: "baz" @@ -156,14 +192,20 @@ test("It navigates when version ID in URL does not match ID of selected version" test("It navigates when specification ID in URL does not match ID of selected specification", async () => { let didNavigate = false - const router = { - pathname: "/foo/bar/hello", - push: () => {}, - replace: () => { - didNavigate = true + const sut = new ProjectNavigator({ + pathnameReader: { + get pathname() { + return "/foo/bar/hello" + } + }, + router: { + push: () => {}, + replace: () => { + didNavigate = true + } } - } - projectNavigator.navigateIfNeeded(router, { + }) + sut.navigateIfNeeded({ projectId: "foo", versionId: "bar", specificationId: "baz" diff --git a/src/features/projects/domain/WindowPathnameReader.ts b/src/features/projects/domain/WindowPathnameReader.ts new file mode 100644 index 00000000..a92874d6 --- /dev/null +++ b/src/features/projects/domain/WindowPathnameReader.ts @@ -0,0 +1,8 @@ +export default class WindowPathnameReader { + get pathname() { + if (typeof window === "undefined") { + return "" + } + return window.location.pathname + } +} diff --git a/src/features/projects/domain/getSelection.ts b/src/features/projects/domain/getSelection.ts index b289e82b..b76559ad 100644 --- a/src/features/projects/domain/getSelection.ts +++ b/src/features/projects/domain/getSelection.ts @@ -16,8 +16,9 @@ export default function getSelection({ if (path.startsWith("/")) { path = path.substring(1) } - let { projectId, versionId, specificationId } = guessSelection(path) + const { projectId: _projectId, versionId, specificationId } = guessSelection(path) // If no project is selected and the user only has a single project then we select that. + let projectId = _projectId if (!projectId && projects.length == 1) { projectId = projects[0].id } diff --git a/src/features/projects/domain/index.ts b/src/features/projects/domain/index.ts index 63eca5d7..84d75c9a 100644 --- a/src/features/projects/domain/index.ts +++ b/src/features/projects/domain/index.ts @@ -6,7 +6,8 @@ export type { default as IProjectDataSource } from "./IProjectDataSource" export type { default as OpenApiSpecification } from "./OpenApiSpecification" export type { default as Project } from "./Project" export { default as ProjectConfigParser } from "./ProjectConfigParser" -export { default as projectNavigator } from "./projectNavigator" +export { default as ProjectNavigator } from "./ProjectNavigator" export { default as ProjectRepository } from "./ProjectRepository" export { default as updateWindowTitle } from "./updateWindowTitle" export type { default as Version } from "./Version" +export { default as WindowPathnameReader } from "./WindowPathnameReader" diff --git a/src/features/projects/domain/projectNavigator.ts b/src/features/projects/domain/projectNavigator.ts index 8eb0aab4..1fcd8933 100644 --- a/src/features/projects/domain/projectNavigator.ts +++ b/src/features/projects/domain/projectNavigator.ts @@ -1,14 +1,29 @@ import Project from "./Project" -export interface IProjectRouter { +interface IPathnameReader { readonly pathname: string +} + +export interface IRouter { push(path: string): void replace(path: string): void } -const projectNavigator = { +type ProjectNavigatorConfig = { + readonly pathnameReader: IPathnameReader + readonly router: IRouter +} + +export default class ProjectNavigator { + private readonly pathnameReader: IPathnameReader + private readonly router: IRouter + + constructor(config: ProjectNavigatorConfig) { + this.pathnameReader = config.pathnameReader + this.router = config.router + } + navigateToVersion( - router: IProjectRouter, project: Project, versionId: string, preferredSpecificationName: string @@ -24,22 +39,22 @@ const projectNavigator = { return e.name == preferredSpecificationName }) if (candidateSpecification) { - router.push(`/${project.id}/${newVersion.id}/${candidateSpecification.id}`) + this.router.push(`/${project.id}/${newVersion.id}/${candidateSpecification.id}`) } else { const firstSpecification = newVersion.specifications[0] - router.push(`/${project.id}/${newVersion.id}/${firstSpecification.id}`) + this.router.push(`/${project.id}/${newVersion.id}/${firstSpecification.id}`) } - }, + } + navigate( - router: IProjectRouter, projectId: string, versionId: string, specificationId: string ) { - router.push(`/${projectId}/${versionId}/${specificationId}`) - }, + this.router.push(`/${projectId}/${versionId}/${specificationId}`) + } + navigateIfNeeded( - router: IProjectRouter, selection: { projectId?: string, versionId?: string, @@ -50,10 +65,8 @@ const projectNavigator = { return } const path = `/${selection.projectId}/${selection.versionId}/${selection.specificationId}` - if (path !== router.pathname) { - router.replace(path) + if (path !== this.pathnameReader.pathname) { + this.router.replace(path) } } } - -export default projectNavigator \ No newline at end of file diff --git a/src/features/projects/view/client/ProjectsPage.tsx b/src/features/projects/view/client/ProjectsPage.tsx index 1348e75c..7dd2a887 100644 --- a/src/features/projects/view/client/ProjectsPage.tsx +++ b/src/features/projects/view/client/ProjectsPage.tsx @@ -6,12 +6,18 @@ import { useTheme } from "@mui/material/styles" import useMediaQuery from "@mui/material/useMediaQuery" import SidebarContainer from "@/features/sidebar/view/client/SidebarContainer" import { useProjects } from "../../data" -import { Project, getSelection, projectNavigator, updateWindowTitle } from "../../domain" import ProjectList from "../ProjectList" import MainContent from "../MainContent" import MobileToolbar from "../toolbar/MobileToolbar" import TrailingToolbarItem from "../toolbar/TrailingToolbarItem" import useSidebarOpen from "@/common/state/useSidebarOpen" +import { + Project, + ProjectNavigator, + WindowPathnameReader, + getSelection, + updateWindowTitle, +} from "../../domain" export default function ProjectsPage({ enableGitHubLinks, @@ -23,19 +29,14 @@ export default function ProjectsPage({ path: string }) { const router = useRouter() - const projectNavigatorRouter = { - get pathname() { - return window.location.pathname - }, - push: router.push, - replace: router.replace - } const theme = useTheme() + const pathnameReader = new WindowPathnameReader() const [isSidebarOpen, setSidebarOpen] = useSidebarOpen() const isDesktopLayout = useMediaQuery(theme.breakpoints.up("sm")) const { projects: clientProjects, error, isLoading: isClientLoading } = useProjects() const projects = isClientLoading ? (serverProjects || []) : clientProjects const { project, version, specification } = getSelection({ projects, path }) + const projectNavigator = new ProjectNavigator({ router, pathnameReader }) useEffect(() => { updateWindowTitle({ storage: document, @@ -47,12 +48,12 @@ export default function ProjectsPage({ }, [project, version, specification]) useEffect(() => { // Ensure the URL reflects the current selection of project, version, and specification. - projectNavigator.navigateIfNeeded(projectNavigatorRouter, { + projectNavigator.navigateIfNeeded({ projectId: project?.id, versionId: version?.id, specificationId: specification?.id }) - }, [router, project, version, specification, project, version, specification]) + }, [project, version, specification]) useEffect(() => { // Show the sidebar if no project is selected. if (project?.id === undefined) { @@ -65,13 +66,13 @@ export default function ProjectsPage({ } const version = project.versions[0] const specification = version.specifications[0] - projectNavigator.navigate(projectNavigatorRouter, project.id, version.id, specification.id) + projectNavigator.navigate(project.id, version.id, specification.id) } const selectVersion = (versionId: string) => { - projectNavigator.navigateToVersion(projectNavigatorRouter, project!, versionId, specification!.name) + projectNavigator.navigateToVersion(project!, versionId, specification!.name) } const selectSpecification = (specificationId: string) => { - projectNavigator.navigate(projectNavigatorRouter, project!.id, version!.id, specificationId) + projectNavigator.navigate(project!.id, version!.id, specificationId) } const canCloseSidebar = project?.id !== undefined const toggleSidebar = (isOpen: boolean) => { From 0f51d4c39f62b28813f08aac593994a0e4ab65d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20St=C3=B8vring?= Date: Sat, 11 Nov 2023 16:18:31 +0100 Subject: [PATCH 11/12] Removes unneeded file --- src/features/projects/domain/WindowPathnameReader.ts | 8 -------- 1 file changed, 8 deletions(-) delete mode 100644 src/features/projects/domain/WindowPathnameReader.ts diff --git a/src/features/projects/domain/WindowPathnameReader.ts b/src/features/projects/domain/WindowPathnameReader.ts deleted file mode 100644 index a92874d6..00000000 --- a/src/features/projects/domain/WindowPathnameReader.ts +++ /dev/null @@ -1,8 +0,0 @@ -export default class WindowPathnameReader { - get pathname() { - if (typeof window === "undefined") { - return "" - } - return window.location.pathname - } -} From e9811e148f2ee50be053f0fcb6deef4195cc57f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20St=C3=B8vring?= Date: Mon, 13 Nov 2023 19:46:57 +0100 Subject: [PATCH 12/12] Adds trunk as candidate default branch --- src/features/projects/data/GitHubProjectDataSource.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/features/projects/data/GitHubProjectDataSource.ts b/src/features/projects/data/GitHubProjectDataSource.ts index 40efddcc..f94d58f6 100644 --- a/src/features/projects/data/GitHubProjectDataSource.ts +++ b/src/features/projects/data/GitHubProjectDataSource.ts @@ -186,7 +186,7 @@ export default class GitHubProjectDataSource implements IProjectDataSource { private sortVersions(versions: Version[], defaultBranchName: string): Version[] { const candidateDefaultBranches = [ - defaultBranchName, "main", "master", "develop", "development" + defaultBranchName, "main", "master", "develop", "development", "trunk" ] // Reverse them so the top-priority branches end up at the top of the list. .reverse()