Skip to content

Commit 3cb312d

Browse files
ulrikandersenulrikandersen
committed
feat: add PR comparison context and new file detection to OpenAPI diff viewer
- Display base branch and commit OID with clickable GitHub PR link in diff sidebar - Detect and show message when spec file is new (doesn't exist on base branch) - Fix infinite render loop by properly encoding URLs with spaces in filenames - Add PR number to GraphQL queries and pass through data models - Show comparison info for all branches with open PRs
1 parent 850bd84 commit 3cb312d

File tree

10 files changed

+120
-42
lines changed

10 files changed

+120
-42
lines changed

‎src/features/diff/data/IOasDiffCalculator.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ export interface DiffResult {
55
to: string
66
changes: DiffChange[]
77
error?: string | null
8+
isNewFile?: boolean
89
}
910

1011
export interface IOasDiffCalculator {

‎src/features/diff/data/OasDiffCalculator.ts‎

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -51,12 +51,25 @@ export class OasDiffCalculator implements IOasDiffCalculator {
5151
}
5252

5353
// Fetch spec content from both refs
54-
const spec1 = await this.githubClient.getRepositoryContent({
55-
repositoryOwner: owner,
56-
repositoryName: repository,
57-
path: path,
58-
ref: fromRef
59-
})
54+
let spec1
55+
let isNewFile = false
56+
57+
try {
58+
spec1 = await this.githubClient.getRepositoryContent({
59+
repositoryOwner: owner,
60+
repositoryName: repository,
61+
path: path,
62+
ref: fromRef
63+
})
64+
} catch (error) {
65+
// File doesn't exist in base ref - this is a new file
66+
return {
67+
from: fromRef,
68+
to: toRef,
69+
changes: [],
70+
isNewFile: true
71+
}
72+
}
6073

6174
const spec2 = await this.githubClient.getRepositoryContent({
6275
repositoryOwner: owner,

‎src/features/projects/data/GitHubProjectDataSource.ts‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -131,14 +131,17 @@ export default class GitHubProjectDataSource implements IProjectDataSource {
131131
path: file.name,
132132
ref: ref.id
133133
}),
134-
editURL: `https://github.com/${ownerName}/${repositoryName}/edit/${ref.name}/${file.name}`,
134+
editURL: `https://github.com/${ownerName}/${repositoryName}/edit/${ref.name}/${encodeURIComponent(file.name)}`,
135135
diffURL: this.getGitHubDiffURL({
136136
ownerName,
137137
repositoryName,
138138
path: file.name,
139139
baseRefOid: ref.baseRefOid,
140140
headRefOid: ref.id
141141
}),
142+
diffBaseBranch: ref.baseRef,
143+
diffBaseOid: ref.baseRefOid,
144+
diffPrUrl: ref.prNumber ? `https://github.com/${ownerName}/${repositoryName}/pull/${ref.prNumber}` : undefined,
142145
isDefault: false // initial value
143146
}
144147
}).sort((a, b) => a.name.localeCompare(b.name))
@@ -168,7 +171,8 @@ export default class GitHubProjectDataSource implements IProjectDataSource {
168171
path: string
169172
ref: string
170173
}): string {
171-
return `/api/blob/${ownerName}/${repositoryName}/${path}?ref=${ref}`
174+
const encodedPath = path.split('/').map(segment => encodeURIComponent(segment)).join('/')
175+
return `/api/blob/${ownerName}/${repositoryName}/${encodedPath}?ref=${ref}`
172176
}
173177

174178
private getGitHubDiffURL({
@@ -177,17 +181,18 @@ export default class GitHubProjectDataSource implements IProjectDataSource {
177181
path,
178182
baseRefOid,
179183
headRefOid
180-
}: {
181-
ownerName: string;
182-
repositoryName: string;
183-
path: string;
184-
baseRefOid: string | undefined;
184+
}: {
185+
ownerName: string;
186+
repositoryName: string;
187+
path: string;
188+
baseRefOid: string | undefined;
185189
headRefOid: string }
186190
): string | undefined {
187191
if (!baseRefOid) {
188192
return undefined
189193
} else {
190-
return `/api/diff/${ownerName}/${repositoryName}/${path}?baseRefOid=${baseRefOid}&to=${headRefOid}`
194+
const encodedPath = path.split('/').map(segment => encodeURIComponent(segment)).join('/')
195+
return `/api/diff/${ownerName}/${repositoryName}/${encodedPath}?baseRefOid=${baseRefOid}&to=${headRefOid}`
191196
}
192197
}
193198

‎src/features/projects/data/GitHubRepositoryDataSource.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ type GraphQLGitHubRepositoryRef = {
4747
}
4848

4949
type GraphQLPullRequest = {
50+
readonly number: number
5051
readonly headRefName: string
5152
readonly baseRefName: string
5253
readonly baseRefOid: string
@@ -127,6 +128,7 @@ export default class GitHubProjectDataSource implements IGitHubRepositoryDataSou
127128
name: branch.node.name,
128129
baseRef: pr?.baseRefName,
129130
baseRefOid: pr?.baseRefOid,
131+
prNumber: pr?.number,
130132
files: branch.node.target.tree.entries
131133
}
132134
})
@@ -168,6 +170,7 @@ export default class GitHubProjectDataSource implements IGitHubRepositoryDataSou
168170
pullRequests(first: 100, states: [OPEN]) {
169171
edges {
170172
node {
173+
number
171174
headRefName
172175
baseRefName
173176
baseRefOid
@@ -201,6 +204,7 @@ export default class GitHubProjectDataSource implements IGitHubRepositoryDataSou
201204
pullRequestEdges.forEach(edge => {
202205
const pr = edge.node
203206
pullRequests.set(pr.headRefName, {
207+
number: pr.number,
204208
headRefName: pr.headRefName,
205209
baseRefName: pr.baseRefName,
206210
baseRefOid: pr.baseRefOid

‎src/features/projects/domain/IGitHubRepositoryDataSource.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ export type GitHubRepositoryRef = {
2020
readonly name: string
2121
readonly baseRef?: string
2222
readonly baseRefOid?: string
23+
readonly prNumber?: number
2324
readonly files: {
2425
readonly name: string
2526
}[]

‎src/features/projects/domain/OpenApiSpecification.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@ export const OpenApiSpecificationSchema = z.object({
66
url: z.string(),
77
editURL: z.string().optional(),
88
diffURL: z.string().optional(),
9+
diffBaseBranch: z.string().optional(),
10+
diffBaseOid: z.string().optional(),
11+
diffPrUrl: z.string().optional(),
912
isDefault: z.boolean()
1013
})
1114

‎src/features/projects/domain/ProjectNavigator.ts‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,12 @@ export default class ProjectNavigator {
3535
return e.name == preferredSpecificationName
3636
})
3737
if (candidateSpecification) {
38-
this.router.push(`/${project.owner}/${project.name}/${newVersion.id}/${candidateSpecification.id}`)
38+
const encodedPath = this.encodePath(project.owner, project.name, newVersion.id, candidateSpecification.id)
39+
this.router.push(encodedPath)
3940
} else {
4041
const defaultOrFirstSpecification = getDefaultSpecification(newVersion)
41-
this.router.push(`/${project.owner}/${project.name}/${newVersion.id}/${defaultOrFirstSpecification.id}`)
42+
const encodedPath = this.encodePath(project.owner, project.name, newVersion.id, defaultOrFirstSpecification.id)
43+
this.router.push(encodedPath)
4244
}
4345
}
4446

@@ -48,9 +50,10 @@ export default class ProjectNavigator {
4850
versionId: string,
4951
specificationId: string
5052
) {
51-
this.router.push(`/${projectOwner}/${projectName}/${versionId}/${specificationId}`)
53+
const encodedPath = this.encodePath(projectOwner, projectName, versionId, specificationId)
54+
this.router.push(encodedPath)
5255
}
53-
56+
5457
navigateIfNeeded(selection: {
5558
projectOwner?: string
5659
projectName?: string
@@ -60,9 +63,15 @@ export default class ProjectNavigator {
6063
if (!selection.projectOwner || !selection.projectName || !selection.versionId || !selection.specificationId) {
6164
return
6265
}
63-
const path = `/${selection.projectOwner}/${selection.projectName}/${selection.versionId}/${selection.specificationId}`
66+
const path = this.encodePath(selection.projectOwner, selection.projectName, selection.versionId, selection.specificationId)
6467
if (path !== this.pathnameReader.pathname) {
6568
this.router.replace(path)
6669
}
6770
}
71+
72+
private encodePath(owner: string, projectName: string, versionId: string, specificationId: string): string {
73+
const encodedVersionId = versionId.split('/').map(segment => encodeURIComponent(segment)).join('/')
74+
const encodedSpecificationId = encodeURIComponent(specificationId)
75+
return `/${owner}/${projectName}/${encodedVersionId}/${encodedSpecificationId}`
76+
}
6877
}

‎src/features/projects/view/toolbar/TrailingToolbarItem.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ const TrailingToolbarItem = () => {
8383
</IconButton>
8484
</Tooltip>
8585
}
86-
86+
8787
</Stack>
8888
</>
8989
)

‎src/features/sidebar/data/useDiff.ts‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { DiffChange } from "../../diff/domain/DiffChange"
55
interface DiffData {
66
changes: DiffChange[]
77
error?: string | null
8+
isNewFile?: boolean
89
}
910

1011
export default function useDiff() {
@@ -70,6 +71,13 @@ export default function useDiff() {
7071
const resolvedChanges = resolvedData?.changes ?? []
7172
const resolvedLoading = hasDiffUrl ? loading : false
7273
const resolvedError = hasDiffUrl ? error : null
74+
const isNewFile = resolvedData?.isNewFile ?? false
7375

74-
return { data: resolvedData, loading: resolvedLoading, changes: resolvedChanges, error: resolvedError }
76+
return {
77+
data: resolvedData,
78+
loading: resolvedLoading,
79+
changes: resolvedChanges,
80+
error: resolvedError,
81+
isNewFile
82+
}
7583
}

‎src/features/sidebar/view/internal/diffbar/DiffContent.tsx‎

Lines changed: 55 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,15 @@
11
"use client"
22

3-
import { Alert, Box, Typography } from "@mui/material"
3+
import { Alert, Box, Typography, Link } from "@mui/material"
44
import { useState } from "react"
55
import useDiff from "@/features/sidebar/data/useDiff"
6+
import { useProjectSelection } from "@/features/projects/data"
67
import DiffList, { DiffListStatus } from "./components/DiffList"
78
import DiffDialog from "./components/DiffDialog"
89

910
const DiffContent = () => {
10-
const { data, loading, changes, error } = useDiff()
11+
const { data, loading, changes, error, isNewFile } = useDiff()
12+
const { specification } = useProjectSelection()
1113
const [selectedChange, setSelectedChange] = useState<number | null>(null)
1214

1315
const closeModal = () => setSelectedChange(null)
@@ -18,11 +20,13 @@ const DiffContent = () => {
1820
? "loading"
1921
: error
2022
? "error"
21-
: hasData && hasChanges
22-
? "ready"
23-
: hasData
24-
? "empty"
25-
: "idle"
23+
: isNewFile
24+
? "empty"
25+
: hasData && hasChanges
26+
? "ready"
27+
: hasData
28+
? "empty"
29+
: "idle"
2630

2731
return (
2832
<Box
@@ -39,6 +43,34 @@ const DiffContent = () => {
3943
What has changed?
4044
</Typography>
4145

46+
{specification?.diffBaseBranch && specification?.diffBaseOid && (
47+
<Typography variant="caption" color="text.secondary" sx={{ px: 1, pb: 1 }}>
48+
Comparing to:{" "}
49+
{specification.diffPrUrl ? (
50+
<Link
51+
href={specification.diffPrUrl}
52+
target="_blank"
53+
rel="noopener noreferrer"
54+
variant="caption"
55+
color="text.secondary"
56+
underline="hover"
57+
>
58+
{specification.diffBaseBranch} ({specification.diffBaseOid.substring(0, 7)})
59+
</Link>
60+
) : (
61+
`${specification.diffBaseBranch} (${specification.diffBaseOid.substring(0, 7)})`
62+
)}
63+
</Typography>
64+
)}
65+
66+
{isNewFile && (
67+
<Box sx={{ textAlign: "left", py: 1, px: 1 }}>
68+
<Typography variant="body0" color="text.secondary">
69+
This is a new file that doesn't exist on the base branch.
70+
</Typography>
71+
</Box>
72+
)}
73+
4274
{error ? (
4375
<Alert
4476
severity="error"
@@ -58,20 +90,22 @@ const DiffContent = () => {
5890
</Alert>
5991
) : null}
6092

61-
<Box
62-
sx={{
63-
flex: 1,
64-
overflowY: "auto",
65-
overflowX: "hidden",
66-
}}
67-
>
68-
<DiffList
69-
changes={changes}
70-
status={diffStatus}
71-
selectedChange={selectedChange}
72-
onClick={(i) => setSelectedChange(i)}
73-
/>
74-
</Box>
93+
{!isNewFile && (
94+
<Box
95+
sx={{
96+
flex: 1,
97+
overflowY: "auto",
98+
overflowX: "hidden",
99+
}}
100+
>
101+
<DiffList
102+
changes={changes}
103+
status={diffStatus}
104+
selectedChange={selectedChange}
105+
onClick={(i) => setSelectedChange(i)}
106+
/>
107+
</Box>
108+
)}
75109

76110
<DiffDialog
77111
open={selectedChange !== null}

0 commit comments

Comments
 (0)