Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 21 additions & 7 deletions src/VCS/Adapter/Git/GitHub.php
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,15 @@ public function createFile(string $owner, string $repositoryName, string $filepa
*/
public function createBranch(string $owner, string $repositoryName, string $newBranchName, string $oldBranchName): array
{
throw new Exception("Not implemented");
$latestCommit = $this->getLatestCommit($owner, $repositoryName, $oldBranchName);
$sha = $latestCommit['commitHash'];

$response = $this->call(self::METHOD_POST, "/repos/$owner/$repositoryName/git/refs", ['Authorization' => "Bearer $this->accessToken"], [
'ref' => "refs/heads/$newBranchName",
'sha' => $sha,
]);

return $response['body'] ?? [];
Comment on lines 183 to +192
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 createBranch silently swallows API errors

The method doesn't inspect the HTTP status code before returning. If GitHub rejects the request (e.g., 422 "Reference already exists", 404 branch not found, 403 permission denied), $response['body'] contains an error object like {"message":"Reference already exists",...} and createBranch returns it as if it succeeded. By contrast, listBranches in this same PR correctly gates its output on $statusCode < 200 || $statusCode >= 300. Callers of createBranch have no way to distinguish a successful ref payload from an error payload without parsing the message key themselves.

}

/**
Expand Down Expand Up @@ -738,22 +746,28 @@ public function getPullRequestFromBranch(string $owner, string $repositoryName,
*
* @param string $owner Owner name of the repository
* @param string $repositoryName Name of the GitHub repository
* @param int $perPage Number of branches to fetch per page
* @param int $page Page number to start fetching from
* @return array<string> List of branch names as array
*/
public function listBranches(string $owner, string $repositoryName): array
public function listBranches(string $owner, string $repositoryName, int $perPage = 100, int $page = 1): array
{
$url = "/repos/$owner/$repositoryName/branches";
$perPage = min(max($perPage, 1), 100);

$response = $this->call(self::METHOD_GET, $url, ['Authorization' => "Bearer $this->accessToken"]);
$response = $this->call(self::METHOD_GET, $url, ['Authorization' => "Bearer $this->accessToken"], [
'page' => $page,
'per_page' => $perPage,
]);

$statusCode = $response['headers']['status-code'] ?? 0;
$responseBody = $response['body'] ?? [];

$names = [];
foreach ($responseBody as $subarray) {
$names[] = $subarray['name'] ?? '';
if ($statusCode < 200 || $statusCode >= 300 || !is_array($responseBody)) {
return [];
}

return $names;
return array_values(array_map(fn ($branch) => $branch['name'] ?? '', $responseBody));
}

/**
Expand Down
49 changes: 49 additions & 0 deletions tests/VCS/Adapter/GitHubTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -525,6 +525,55 @@ public function testGetCommitWithInvalidHash(): void
}
}

public function testListBranchesPagination(): void
{
$repositoryName = 'test-list-branches-pages-' . \uniqid();
$this->vcsAdapter->createRepository(static::$owner, $repositoryName, false);

try {
$this->vcsAdapter->createFile(static::$owner, $repositoryName, 'README.md', '# Test');
$this->vcsAdapter->createBranch(static::$owner, $repositoryName, 'branch-a', static::$defaultBranch);
$this->vcsAdapter->createBranch(static::$owner, $repositoryName, 'branch-b', static::$defaultBranch);

/** @var GitHub $adapter */
$adapter = $this->vcsAdapter;

$page1 = $adapter->listBranches(static::$owner, $repositoryName, 1, 1);
$this->assertSame(['branch-a'], $page1);

$page2 = $adapter->listBranches(static::$owner, $repositoryName, 1, 2);
$this->assertSame(['branch-b'], $page2);

$all = $adapter->listBranches(static::$owner, $repositoryName, 100, 1);
$this->assertEqualsCanonicalizing([static::$defaultBranch, 'branch-a', 'branch-b'], $all);
} finally {
Comment thread
Meldiron marked this conversation as resolved.
$this->vcsAdapter->deleteRepository(static::$owner, $repositoryName);
}
}

public function testListBranchesEmptyRepository(): void
{
$repositoryName = 'test-list-branches-empty-' . \uniqid();
$this->vcsAdapter->createRepository(static::$owner, $repositoryName, false);

try {
$branches = $this->vcsAdapter->listBranches(static::$owner, $repositoryName);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If failing because of default branch, find a way to deleteBranch()


$this->assertIsArray($branches);
$this->assertEmpty($branches);
} finally {
$this->vcsAdapter->deleteRepository(static::$owner, $repositoryName);
}
}

public function testListBranchesNonExistingRepository(): void
{
$branches = $this->vcsAdapter->listBranches(static::$owner, 'non-existing-repo-' . \uniqid());

$this->assertIsArray($branches);
$this->assertEmpty($branches);
}

public function testGetLatestCommit(): void
{
$repositoryName = 'test-get-latest-commit-' . \uniqid();
Expand Down
Loading