Skip to content

Commit 30278af

Browse files
committed
fix: address code review feedback
- Add proper URI encoding for filenames with special characters - Improve 422 status handling to only accept 'Reference already exists' - Remove redundant targetBranch variable for code clarity
1 parent 8036c1c commit 30278af

File tree

1 file changed

+20
-10
lines changed

1 file changed

+20
-10
lines changed

src/ui/modules/githubRepository.ts

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -27,21 +27,18 @@ export class GithubRepository {
2727
const filepath = clientPayload.filename
2828

2929
try {
30-
// Use reference field (branch name) from settings, consistent with GitLab implementation
31-
const targetBranch = branch
32-
3330
// Check if branch exists
34-
const branchExists = await this._checkBranchExists(targetBranch)
31+
const branchExists = await this._checkBranchExists(branch)
3532

3633
if (!branchExists) {
3734
// Branch doesn't exist - create it from default branch
3835
const defaultBranch = await this._getDefaultBranch()
3936
const defaultBranchSHA = await this._getBranchSHA(defaultBranch)
40-
await this._createBranch(targetBranch, defaultBranchSHA)
37+
await this._createBranch(branch, defaultBranchSHA)
4138
}
4239

4340
// Check if file exists to get its SHA (required for updates)
44-
const fileSHA = await this._getFileSHA(filepath, targetBranch)
41+
const fileSHA = await this._getFileSHA(filepath, branch)
4542

4643
// Upload the file
4744
const uploadRequest = new XMLHttpRequest()
@@ -53,7 +50,7 @@ export class GithubRepository {
5350
content: encodedContent,
5451
commitMessage: clientPayload.commitMessage || `Update design tokens at ${Date.now()}`,
5552
filepath: filepath,
56-
branch: targetBranch,
53+
branch: branch,
5754
fileSHA: fileSHA
5855
})
5956
} catch (error) {
@@ -180,11 +177,24 @@ export class GithubRepository {
180177
}
181178

182179
const statusCode = request.status
183-
if (statusCode === 201 || statusCode === 422) {
180+
if (statusCode === 201) {
184181
resolve()
185182
return
186183
}
187184

185+
if (statusCode === 422) {
186+
// Check if branch already exists (acceptable) vs other validation errors
187+
try {
188+
const response = JSON.parse(request.responseText)
189+
if (response.message && response.message.includes('Reference already exists')) {
190+
resolve()
191+
return
192+
}
193+
} catch (_e) {
194+
// Fall through to reject if we can't parse response
195+
}
196+
}
197+
188198
reject({
189199
code: statusCode,
190200
message: request.response,
@@ -207,7 +217,7 @@ export class GithubRepository {
207217
const request = new XMLHttpRequest()
208218
request.open(
209219
'GET',
210-
`https://api.github.com/repos/${this.owner}/${this.repo}/contents/${filepath}?ref=${branch}`
220+
`https://api.github.com/repos/${this.owner}/${this.repo}/contents/${encodeURIComponent(filepath)}?ref=${branch}`
211221
)
212222
this._setRequestHeader(request)
213223

@@ -262,7 +272,7 @@ export class GithubRepository {
262272

263273
request.open(
264274
'PUT',
265-
`https://api.github.com/repos/${this.owner}/${this.repo}/contents/${filepath}`
275+
`https://api.github.com/repos/${this.owner}/${this.repo}/contents/${encodeURIComponent(filepath)}`
266276
)
267277
this._setRequestHeader(request)
268278

0 commit comments

Comments
 (0)