Skip to content

Commit 358e8cb

Browse files
supersonicbytesupersonicbyte
committed
Resolve PR comments
1 parent e286cae commit 358e8cb

File tree

10 files changed

+47
-39
lines changed

10 files changed

+47
-39
lines changed

‎FrontEnd/styles/package.scss‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@
6969
display: block;
7070
font-size: 11px;
7171
}
72-
72+
7373
li.archived {
7474
grid-column-start: span 2;
7575
background-image: var(--image-warning);

‎Sources/App/Controllers/API/API+PackageController+ForkedFromResult.swift‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,15 +22,19 @@ extension API.PackageController {
2222
case fromGitHub(url: String)
2323

2424
static func query(on database: Database, packageId: Package.Id) async throws -> ForkedFromResult? {
25-
let model = try await Joined3<Package, Repository, Version>.query(on: database, packageId: packageId).first()
26-
25+
let model = try await Joined3<Package, Repository, Version>
26+
.query(on: database, packageId: packageId, version: .defaultBranch)
27+
.first()
28+
2729
guard let repoName = model?.repository.name,
2830
let ownerName = model?.repository.ownerName,
29-
let owner = model?.repository.owner,
30-
let packageName = model?.version.packageName else {
31+
let owner = model?.repository.owner else {
3132
return nil
3233
}
3334

35+
// fallback to repo name if packageName is nil
36+
let packageName: String = model?.version.packageName ?? repoName
37+
3438
return ForkedFromResult.fromSPI(
3539
repository: repoName,
3640
owner: owner,

‎Sources/App/Controllers/API/API+PackageController+GetRoute+Model.swift‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -387,6 +387,17 @@ extension API.PackageController.GetRoute.Model {
387387
return url
388388
}
389389
}
390+
391+
392+
393+
var ownerURL: String? {
394+
switch self {
395+
case .fromSPI(_, let owner, _, _, _):
396+
return "https://github.com/\(owner)"
397+
case .fromGitHub:
398+
return nil
399+
}
400+
}
390401
}
391402

392403
}

‎Sources/App/Core/Query+Support/Joined3+Package.swift‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,11 +41,8 @@ extension Joined3 where M == Package, R1 == Repository, R2 == Version {
4141
.filter(Repository.self, \.$name, .custom("ilike"), repository)
4242
}
4343

44-
static func query(on database: Database, packageId: Package.Id) -> JoinedQueryBuilder<Self> {
45-
query(on: database,
46-
join: \Package.$id == \Repository.$package.$id, method: .inner,
47-
join: \Version.$package.$id == \Package.$id, method: .inner)
48-
.filter(Version.self, \Version.$latest == .defaultBranch)
49-
.filter(Package.self, \Package.$id == packageId)
44+
static func query(on database: Database, packageId: Package.Id, version: Version.Kind) -> JoinedQueryBuilder<Self> {
45+
query(on: database, version: version)
46+
.filter(Package.self, \Package.$id == packageId)
5047
}
5148
}

‎Sources/App/Models/Repository.swift‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -225,12 +225,6 @@ final class Repository: @unchecked Sendable, Model, Content {
225225
.first() ?? Repository(packageId: pkgId)
226226
}
227227

228-
static func find(on database: Database, for packageId: Package.Id) async throws -> Repository? {
229-
return try await Repository.query(on: database)
230-
.filter(\.$package.$id == packageId)
231-
.first()
232-
}
233-
234228
}
235229

236230

‎Sources/App/Views/PackageController/GetRoute.Model+ext.swift‎

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -187,33 +187,31 @@ extension API.PackageController.GetRoute.Model {
187187
let item: Node<HTML.BodyContext> = {
188188
switch forkedFromInfo {
189189
case .fromGitHub(let url):
190+
var text = url.replacingOccurrences(of: "https://github.com/", with: "")
191+
text = text.removingSuffix(".git")
190192
let repoURLNode: Node<HTML.BodyContext> = .a(
191193
.href(url),
192-
.text("repository")
194+
.text(text)
193195
)
194196
return .group(
195197
.text("Forked from "),
196198
repoURLNode,
197199
.text(".")
198200
)
199-
case .fromSPI(let packageName, _, let ownerName, _, let originalPackageName):
200-
let repoURLNode: Node<HTML.BodyContext>
201-
if packageName == originalPackageName {
202-
repoURLNode = .a(
203-
.href(forkedFromInfo.url),
204-
.text(ownerName)
205-
)
206-
207-
} else {
208-
repoURLNode = .a(
209-
.href(forkedFromInfo.url),
210-
.text("\(originalPackageName) by \(ownerName)")
211-
)
212-
}
213-
201+
case .fromSPI(_, _, let ownerName, _, let originalPackageName):
202+
let repoURLNode: Node<HTML.BodyContext> = .a(
203+
.href(forkedFromInfo.url),
204+
.text("\(originalPackageName)")
205+
)
206+
let ownerNode: Node<HTML.BodyContext> = .a(
207+
.href(forkedFromInfo.ownerURL ?? ""),
208+
.text("\(ownerName)")
209+
)
214210
return .group(
215211
.text("Forked from "),
216212
repoURLNode,
213+
.text(" by "),
214+
ownerNode,
217215
.text(".")
218216
)
219217
}

‎Tests/AppTests/API+PackageController+GetRoute+ModelTests.swift‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -448,7 +448,7 @@ class API_PackageController_GetRoute_ModelTests: SnapshotTestCase {
448448
var model = API.PackageController.GetRoute.Model.mock
449449
model.forkedFromInfo = .fromGitHub(url: "https://github.com/owner/repository.git")
450450
let renderedForkedFrom = model.forkedListItem().render()
451-
XCTAssertEqual(renderedForkedFrom, "<li class=\"forked\">Forked from <a href=\"https://github.com/owner/repository.git\">repository</a>.</li>")
451+
XCTAssertEqual(renderedForkedFrom, "<li class=\"forked\">Forked from <a href=\"https://github.com/owner/repository.git\">owner/repository</a>.</li>")
452452
}
453453

454454
func test_forkedFrom_spi_same_package_name_formatting() throws {
@@ -461,8 +461,9 @@ class API_PackageController_GetRoute_ModelTests: SnapshotTestCase {
461461
originalPackageName: "Test"
462462
)
463463
let url = SiteURL.package(.value("owner"), .value("repo"), nil).absoluteURL()
464+
var ownerUrl = model.forkedFromInfo?.ownerURL ?? ""
464465
let renderedForkedFrom = model.forkedListItem().render()
465-
XCTAssertEqual(renderedForkedFrom, "<li class=\"forked\">Forked from <a href=\"\(url)\">OriginalOwner</a>.</li>")
466+
XCTAssertEqual(renderedForkedFrom, "<li class=\"forked\">Forked from <a href=\"\(url)\">Test</a> by <a href=\"\(ownerUrl)\">OriginalOwner</a>.</li>")
466467
}
467468

468469
func test_forkedFrom_spi_different_package_name_formatting() throws {
@@ -475,8 +476,9 @@ class API_PackageController_GetRoute_ModelTests: SnapshotTestCase {
475476
originalPackageName: "Different"
476477
)
477478
let url = SiteURL.package(.value("owner"), .value("repo"), nil).absoluteURL()
479+
var ownerUrl = model.forkedFromInfo?.ownerURL ?? ""
478480
let renderedForkedFrom = model.forkedListItem().render()
479-
XCTAssertEqual(renderedForkedFrom, "<li class=\"forked\">Forked from <a href=\"\(url)\">Different by OriginalOwner</a>.</li>")
481+
XCTAssertEqual(renderedForkedFrom, "<li class=\"forked\">Forked from <a href=\"\(url)\">Different</a> by <a href=\"\(ownerUrl)\">OriginalOwner</a>.</li>")
480482
}
481483

482484
func test_BuildInfo_init() throws {

‎Tests/AppTests/__Snapshots__/API+PackageController+GetRoute+ModelTests/test_forked_from_github.1.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
<li class="forked">Forked from
2-
<a href="https://github.com/owner/repository.git">repository</a>.
2+
<a href="https://github.com/owner/repository.git">owner/repository</a>.
33
</li>

‎Tests/AppTests/__Snapshots__/API+PackageController+GetRoute+ModelTests/test_forked_from_spi_different_package_name.1.txt‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
11
<li class="forked">Forked from
2-
<a href="http://localhost:8080/owner/repo">Different by OriginalOwner</a>.
2+
<a href="http://localhost:8080/owner/repo">Different</a> by
3+
<a href="https://github.com/owner/repo">OriginalOwner</a>.
34
</li>

‎Tests/AppTests/__Snapshots__/API+PackageController+GetRoute+ModelTests/test_forked_from_spi_same_package_name.1.txt‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
11
<li class="forked">Forked from
2-
<a href="http://localhost:8080/owner/repo">OriginalOwner</a>.
2+
<a href="http://localhost:8080/owner/repo">Test</a> by
3+
<a href="https://github.com/owner/repo">OriginalOwner</a>.
34
</li>

0 commit comments

Comments
 (0)