Skip to content

Commit 84d582a

Browse files
committed
Cleaned up some issues with the GitHub code.
1 parent ce6bff6 commit 84d582a

File tree

5 files changed

+37
-14
lines changed

5 files changed

+37
-14
lines changed

CCMenu.xcodeproj/project.pbxproj

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@
8484
03DA98DE2B44B7320073F5BB /* GitHubWorkflowList.swift in Sources */ = {isa = PBXBuildFile; fileRef = 03DA98DD2B44B7320073F5BB /* GitHubWorkflowList.swift */; };
8585
03DA98E02B44BF620073F5BB /* GitHubPipelineBuilder.swift in Sources */ = {isa = PBXBuildFile; fileRef = 03DA98DF2B44BF620073F5BB /* GitHubPipelineBuilder.swift */; };
8686
03DA98E22B44CE1A0073F5BB /* GitHubAuthenticator.swift in Sources */ = {isa = PBXBuildFile; fileRef = 03DA98E12B44CE1A0073F5BB /* GitHubAuthenticator.swift */; };
87+
03DC9BBA2E240D170000E22E /* GitHubAPITests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 03DC9BB92E240D030000E22E /* GitHubAPITests.swift */; };
8788
03DD697C2B61A26900D7AD9D /* CCTrayPipelineBuilder.swift in Sources */ = {isa = PBXBuildFile; fileRef = 03DD697B2B61A26900D7AD9D /* CCTrayPipelineBuilder.swift */; };
8889
03DD697E2B646E3800D7AD9D /* NSColorExtensions.swift in Sources */ = {isa = PBXBuildFile; fileRef = 03DD697D2B646E3800D7AD9D /* NSColorExtensions.swift */; };
8990
03DF19072D025292007D1A64 /* ComboBox.swift in Sources */ = {isa = PBXBuildFile; fileRef = 03DF19062D025292007D1A64 /* ComboBox.swift */; };
@@ -218,6 +219,7 @@
218219
03DA98DD2B44B7320073F5BB /* GitHubWorkflowList.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = GitHubWorkflowList.swift; sourceTree = "<group>"; };
219220
03DA98DF2B44BF620073F5BB /* GitHubPipelineBuilder.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = GitHubPipelineBuilder.swift; sourceTree = "<group>"; };
220221
03DA98E12B44CE1A0073F5BB /* GitHubAuthenticator.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = GitHubAuthenticator.swift; sourceTree = "<group>"; };
222+
03DC9BB92E240D030000E22E /* GitHubAPITests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = GitHubAPITests.swift; sourceTree = "<group>"; };
221223
03DD697B2B61A26900D7AD9D /* CCTrayPipelineBuilder.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CCTrayPipelineBuilder.swift; sourceTree = "<group>"; };
222224
03DD697D2B646E3800D7AD9D /* NSColorExtensions.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NSColorExtensions.swift; sourceTree = "<group>"; };
223225
03DF19062D025292007D1A64 /* ComboBox.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ComboBox.swift; sourceTree = "<group>"; };
@@ -410,6 +412,7 @@
410412
03F9B899297D930A00FA866E /* CCTrayFeedReaderTests.swift */,
411413
2FA28075764358E9D929CECD /* CCTrayResponseParserTests.swift */,
412414
2FA2833D541ABE8B0D2E7615 /* GitHubResponseParserTests.swift */,
415+
03DC9BB92E240D030000E22E /* GitHubAPITests.swift */,
413416
0362EBEE2B5313120079DEFE /* NotificationFactoryTests.swift */,
414417
03F9B88F297C6EEC00FA866E /* CompactRelativeFormatStyleTests.swift */,
415418
3CB911D82B78EB3F009DF781 /* URLRequestExtensionTests.swift */,
@@ -850,6 +853,7 @@
850853
buildActionMask = 2147483647;
851854
files = (
852855
037AB21C297B4D5F00C33589 /* PipelineModelTests.swift in Sources */,
856+
03DC9BBA2E240D170000E22E /* GitHubAPITests.swift in Sources */,
853857
2FA28FBCC0398D63EF2D2C16 /* NSImageExtensionTests.swift in Sources */,
854858
03B021252A5D9F9D00B889BE /* MenuExtraModelTests.swift in Sources */,
855859
2FA28718B9A6BD8E1C32FF55 /* CCTrayResponseParserTests.swift in Sources */,

CCMenu/Source/Model/PipelineModel.swift

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -66,17 +66,14 @@ final class PipelineModel: ObservableObject {
6666
pipelines = references.compactMap({ Pipeline(legacyReference: $0) })
6767
}
6868
else {
69-
// TODO: Change to use API class
70-
let url = URL(string: "https://api.github.com/repos/ccmenu/ccmenu2/actions/workflows/build-and-test.yaml/runs?branch=main")!
69+
let url = GitHubAPI.feedUrl(owner: "ccmenu", repository: "ccmenu2", workflow: "build-and-test.yaml", branch: "main")
7170
pipelines = [ Pipeline(name: "ccmenu2 | build-and-test", feed: PipelineFeed(type: .github, url: url)) ]
7271
}
7372
#if DEBUG
7473
// TODO: Remove when GitLab UI exists
7574
let url = GitLabAPI.feedUrl(projectId: "66079563", branch: nil)
7675
#endif
7776
self.add(pipeline: Pipeline(name: "quvyn | build-and-test", feed: PipelineFeed(type: .gitlab, url: url)))
78-
// TODO: Remove before App Store release
79-
UserDefaults.active.removeObject(forKey: "GitHubToken")
8077
}
8178

8279
private func savePipelinesToUserDefaults() {

CCMenu/Source/Server Monitor/GitHubAPI.swift

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -100,11 +100,10 @@ class GitHubAPI {
100100

101101
// MARK: - feed
102102

103-
// TODO: fix path overwriting bug (see GitLab implementation)
104103
static func feedUrl(owner: String, repository: String, workflow: String, branch: String?) -> URL {
105104
// see https://docs.github.com/en/rest/actions/workflow-runs?apiVersion=2022-11-28#list-workflow-runs-for-a-workflow
106-
var components = URLComponents(string: baseURL(forAPI: true))!
107-
components.path = String(format: "/repos/%@/%@/actions/workflows/%@/runs", owner, repository, workflow)
105+
let url = baseURL(forAPI: true).appending(path: "/repos/\(owner)/\(repository)/actions/workflows/\(workflow)/runs")
106+
var components = URLComponents(url: url, resolvingAgainstBaseURL: true)!
108107
if let branch {
109108
components.appendQueryItem(URLQueryItem(name: "branch", value: branch))
110109
}
@@ -145,17 +144,19 @@ class GitHubAPI {
145144

146145
// MARK: - helper functions
147146

148-
private static func baseURL(forAPI: Bool) -> String {
147+
private static func baseURL(forAPI: Bool) -> URL {
148+
var urlString = forAPI ? "https://api.github.com" : "https://github.com"
149149
let defaultsKey = forAPI ? "GitHubAPIBaseURL" : "GitHubBaseURL"
150150
if let defaultsBaseURL = UserDefaults.active.string(forKey: defaultsKey) {
151-
return defaultsBaseURL
151+
urlString = defaultsBaseURL
152152
}
153-
return forAPI ? "https://api.github.com" : "https://github.com"
153+
guard let url = URL(string: urlString) else { fatalError("Invalid base URL \(urlString)") }
154+
return url
154155
}
155156

156-
private static func makeRequest(method: String = "GET", baseUrl: String, path: String, params: Dictionary<String, String> = [:], token: String? = nil) -> URLRequest {
157-
var components = URLComponents(string: baseUrl)!
158-
components.path = path // TODO: check for path overwriting issues
157+
private static func makeRequest(method: String = "GET", baseUrl: URL, path: String, params: Dictionary<String, String> = [:], token: String? = nil) -> URLRequest {
158+
let url = baseUrl.appending(path: path)
159+
var components = URLComponents(url: url, resolvingAgainstBaseURL: true)!
159160
components.queryItems = params.map({ URLQueryItem(name: $0.key, value: $0.value) })
160161
// TODO: Consider filtering token when the URL is overwritten via defaults
161162
return makeRequest(method: method, url: components.url!, token: token)

CCMenu/Source/Server Monitor/GitHubResponseParser.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ class GitHubResponseParser {
5757
var messageParts: [String] = []
5858
if let event = run["event"] as? String {
5959
let prettified = event.replacingOccurrences(of: "_", with: " ").capitalized
60-
messageParts.append("\(prettified)")
60+
messageParts.append(prettified)
6161
}
6262
if let displayTitle = run["display_title"] as? String {
6363
messageParts.append(displayTitle)

CCMenuTests/GitHubAPITests.swift

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
/*
2+
* Copyright (c) Erik Doernenburg and contributors
3+
* Licensed under the Apache License, Version 2.0 (the "License"); you may
4+
* not use these files except in compliance with the License.
5+
*/
6+
7+
import XCTest
8+
@testable import CCMenu
9+
10+
class GitHubAPITests: XCTestCase {
11+
12+
func testConstructsCorrectRequestPathWhenBaseURLContainsPath() throws {
13+
UserDefaults.active.set("https://dev.some-enterprise.com/github", forKey: "GitHubAPIBaseURL")
14+
let request = GitHubAPI.requestForUser(user: "testuser", token: nil)
15+
UserDefaults.active.removeObject(forKey: "CCMenuGitHubAPIBaseURL")
16+
17+
XCTAssertEqual("/github/users/testuser", request.url?.path())
18+
}
19+
20+
}
21+

0 commit comments

Comments
 (0)