Merge ForkedFromResult and ForkedFromInfo #3388
Conversation
| case .fromGitHub: | ||
| return nil | ||
| } | ||
| } |
There was a problem hiding this comment.
I've moved these two view related properties into an extension in the file under Views/PackageController/GetRoute.Model+ext.swift. That way, view related things don't distract from the basic definition and appear "closer" to where they're actually used.
| fundingLinks: result.repository.fundingLinks, | ||
| swift6Readiness: swift6Readiness, | ||
| forkedFromResult: forkedFromResult | ||
| forkedFromInfo: forkedFromInfo |
There was a problem hiding this comment.
The model init now doesn't do any logic with ForkedFromInfo - it just assigns it. That makes the tests simpler, see below.
| async let buildInfo = API.PackageController.BuildInfo.query(on: database, | ||
| owner: owner, | ||
| repository: repository) | ||
| async let forkedFromInfo = forkedFromInfo(on: database, fork: packageResult.repository.forkedFrom) |
There was a problem hiding this comment.
We're potentially running another query, using async let makes this concurrent.
| case fromSPI(originalOwner: String, | ||
| originalOwnerName: String, | ||
| originalRepo: String, | ||
| originalPackageName: String) |
There was a problem hiding this comment.
Note that the packageName: String field was actually unused.
|
|
||
| // validate | ||
| XCTAssertEqual(model.forkedFromInfo, API.PackageController.GetRoute.Model.ForkedFromInfo.fromGitHub(url: "https://github.com/example/repo.git")) | ||
| } |
There was a problem hiding this comment.
This test is now pretty much superfluous - we're passing through the simple ForkedFromInfo enum and there's no point testing the GH case.
| XCTAssertEqual(forkedFrom, .fromSPI(originalOwner: "original", | ||
| originalOwnerName: "OriginalOwner", | ||
| originalRepo: "original", | ||
| originalPackageName: "OriginalPkg")) |
There was a problem hiding this comment.
The part that we do want to test is the ForkedFromInfo.query and so I've converted test_init_forked_from_spi into a test for that. It also gets simpler, because we don't need as much set up just to test that query.
As discussed, merges ForkedFromResult into ForkedFrom Info.