From 76df5e76dd9f6e54f044d2af25ed830fa8c7208e Mon Sep 17 00:00:00 2001 From: Jayesh Somani Date: Mon, 27 Apr 2026 11:35:14 +0530 Subject: [PATCH 1/4] feat(gitlab): add pull request, comment and user support --- src/VCS/Adapter/Git/GitLab.php | 205 +++++++++++++++- tests/VCS/Adapter/GitLabTest.php | 404 +++++++++++++++++++++++++++++-- 2 files changed, 585 insertions(+), 24 deletions(-) diff --git a/src/VCS/Adapter/Git/GitLab.php b/src/VCS/Adapter/Git/GitLab.php index 493beaa5..cbb49f9a 100644 --- a/src/VCS/Adapter/Git/GitLab.php +++ b/src/VCS/Adapter/Git/GitLab.php @@ -465,22 +465,93 @@ public function createWebhook(string $owner, string $repositoryName, string $url public function createComment(string $owner, string $repositoryName, int $pullRequestNumber, string $comment): string { - throw new Exception("Not implemented"); + $ownerPath = $this->getOwnerPath($owner); + $projectPath = urlencode("{$ownerPath}/{$repositoryName}"); + $url = "/projects/{$projectPath}/merge_requests/{$pullRequestNumber}/notes"; + + $response = $this->call(self::METHOD_POST, $url, ['PRIVATE-TOKEN' => $this->accessToken], ['body' => $comment]); + + $responseHeaders = $response['headers'] ?? []; + $statusCode = $responseHeaders['status-code'] ?? 0; + if ($statusCode >= 400) { + throw new Exception("Failed to create comment: HTTP {$statusCode}"); + } + + $responseBody = $response['body'] ?? []; + if (!array_key_exists('id', $responseBody)) { + throw new Exception("Comment creation response is missing comment ID."); + } + + return (string) ($responseBody['id'] ?? ''); } public function getComment(string $owner, string $repositoryName, string $commentId): string { - throw new Exception("Not implemented"); + $ownerPath = $this->getOwnerPath($owner); + $projectPath = urlencode("{$ownerPath}/{$repositoryName}"); + + // We need MR iid to fetch a note — GitLab notes are MR-scoped + // Search across all MRs for this note + $url = "/projects/{$projectPath}/merge_requests?state=all&per_page=100"; + $response = $this->call(self::METHOD_GET, $url, ['PRIVATE-TOKEN' => $this->accessToken]); + $mrs = $response['body'] ?? []; + + foreach ($mrs as $mr) { + $mrIid = $mr['iid'] ?? 0; + $noteUrl = "/projects/{$projectPath}/merge_requests/{$mrIid}/notes/{$commentId}"; + $noteResponse = $this->call(self::METHOD_GET, $noteUrl, ['PRIVATE-TOKEN' => $this->accessToken]); + $noteHeaders = $noteResponse['headers'] ?? []; + if (($noteHeaders['status-code'] ?? 0) === 200) { + return $noteResponse['body']['body'] ?? ''; + } + } + + return ''; } public function updateComment(string $owner, string $repositoryName, int $commentId, string $comment): string { - throw new Exception("Not implemented"); + $ownerPath = $this->getOwnerPath($owner); + $projectPath = urlencode("{$ownerPath}/{$repositoryName}"); + + // Same issue — need MR iid. Search all MRs + $url = "/projects/{$projectPath}/merge_requests?state=all&per_page=100"; + $response = $this->call(self::METHOD_GET, $url, ['PRIVATE-TOKEN' => $this->accessToken]); + $mrs = $response['body'] ?? []; + + foreach ($mrs as $mr) { + $mrIid = $mr['iid'] ?? 0; + $noteUrl = "/projects/{$projectPath}/merge_requests/{$mrIid}/notes/{$commentId}"; + $noteResponse = $this->call(self::METHOD_PUT, $noteUrl, ['PRIVATE-TOKEN' => $this->accessToken], ['body' => $comment]); + $noteHeaders = $noteResponse['headers'] ?? []; + if (($noteHeaders['status-code'] ?? 0) === 200) { + return (string) $commentId; + } + } + + throw new Exception("Failed to update comment: comment ID {$commentId} not found in any merge request."); } public function getUser(string $username): array { - throw new Exception("Not implemented"); + $url = "/users?username=" . rawurlencode($username); + + $response = $this->call(self::METHOD_GET, $url, ['PRIVATE-TOKEN' => $this->accessToken]); + + $responseHeaders = $response['headers'] ?? []; + $statusCode = $responseHeaders['status-code'] ?? 0; + if ($statusCode >= 400) { + throw new Exception("Failed to get user: HTTP {$statusCode}"); + } + + $body = $response['body'] ?? []; + + // GitLab returns an array of users — return first match + if (empty($body[0])) { + throw new Exception("User not found: {$username}"); + } + + return $body[0]; } public function getOwnerName(string $installationId, ?int $repositoryId = null): string @@ -511,17 +582,123 @@ public function getOwnerName(string $installationId, ?int $repositoryId = null): public function getPullRequest(string $owner, string $repositoryName, int $pullRequestNumber): array { - throw new Exception("Not implemented"); + $ownerPath = $this->getOwnerPath($owner); + $projectPath = urlencode("{$ownerPath}/{$repositoryName}"); + $url = "/projects/{$projectPath}/merge_requests/{$pullRequestNumber}"; + + $response = $this->call(self::METHOD_GET, $url, ['PRIVATE-TOKEN' => $this->accessToken]); + + $responseHeaders = $response['headers'] ?? []; + $statusCode = $responseHeaders['status-code'] ?? 0; + if ($statusCode >= 400) { + throw new Exception("Failed to get merge request: HTTP {$statusCode}"); + } + + $mr = $response['body'] ?? []; + + // Normalize to match expected shape (consistent with Gitea/GitHub) + return [ + 'number' => $mr['iid'] ?? 0, + 'title' => $mr['title'] ?? '', + 'state' => $mr['state'] ?? '', + 'head' => [ + 'ref' => $mr['source_branch'] ?? '', + 'sha' => $mr['sha'] ?? '', + ], + 'base' => [ + 'ref' => $mr['target_branch'] ?? '', + ], + ]; } public function getPullRequestFiles(string $owner, string $repositoryName, int $pullRequestNumber): array { - throw new Exception("Not implemented"); + $ownerPath = $this->getOwnerPath($owner); + $projectPath = urlencode("{$ownerPath}/{$repositoryName}"); + + // Poll until diff is ready (patch_id_sha not null) + $maxAttempts = 10; + for ($attempt = 0; $attempt < $maxAttempts; $attempt++) { + $mrResponse = $this->call( + self::METHOD_GET, + "/projects/{$projectPath}/merge_requests/{$pullRequestNumber}", + ['PRIVATE-TOKEN' => $this->accessToken] + ); + $mrBody = $mrResponse['body'] ?? []; + if (($mrBody['patch_id_sha'] ?? null) !== null) { + break; + } + usleep(1000000); // 1 second + } + + // Fetch diffs with pagination + $allFiles = []; + $page = 1; + $perPage = 100; + + while (true) { + $url = "/projects/{$projectPath}/merge_requests/{$pullRequestNumber}/diffs?page={$page}&per_page={$perPage}"; + $response = $this->call(self::METHOD_GET, $url, ['PRIVATE-TOKEN' => $this->accessToken]); + + $responseHeaders = $response['headers'] ?? []; + $statusCode = $responseHeaders['status-code'] ?? 0; + if ($statusCode >= 400) { + throw new Exception("Failed to get merge request files: HTTP {$statusCode}"); + } + + $files = $response['body'] ?? []; + if (!is_array($files) || empty($files)) { + break; + } + + foreach ($files as $diff) { + $allFiles[] = [ + 'filename' => $diff['new_path'] ?? $diff['old_path'] ?? '', + ]; + } + + if (count($files) < $perPage) { + break; + } + $page++; + } + + return $allFiles; } public function getPullRequestFromBranch(string $owner, string $repositoryName, string $branch): array { - throw new Exception("Not implemented"); + $ownerPath = $this->getOwnerPath($owner); + $projectPath = urlencode("{$ownerPath}/{$repositoryName}"); + $url = "/projects/{$projectPath}/merge_requests?state=opened&source_branch=" . urlencode($branch); + + $response = $this->call(self::METHOD_GET, $url, ['PRIVATE-TOKEN' => $this->accessToken]); + + $responseHeaders = $response['headers'] ?? []; + $statusCode = $responseHeaders['status-code'] ?? 0; + if ($statusCode >= 400) { + throw new Exception("Failed to list merge requests: HTTP {$statusCode}"); + } + + $body = $response['body'] ?? []; + if (empty($body[0])) { + return []; + } + + $mr = $body[0]; + + return [ + 'number' => $mr['iid'] ?? 0, + 'title' => $mr['title'] ?? '', + 'state' => $mr['state'] ?? '', + 'head' => [ + 'ref' => $mr['source_branch'] ?? '', + 'sha' => $mr['sha'] ?? '', + ], + 'base' => [ + 'ref' => $mr['target_branch'] ?? '', + ], + ]; } public function listBranches(string $owner, string $repositoryName): array @@ -704,14 +881,24 @@ public function generateCloneCommand(string $owner, string $repositoryName, stri public function getEvent(string $event, string $payload): array { $payloadArray = json_decode($payload, true); - if ($payloadArray === false || $payloadArray === null) { + if ($payloadArray === null || !is_array($payloadArray)) { return []; } switch ($event) { case 'Push Hook': $commits = $payloadArray['commits'] ?? []; - $latestCommit = !empty($commits) ? $commits[0] : []; + $checkoutSha = $payloadArray['checkout_sha'] ?? ''; + $latestCommit = []; + foreach ($commits as $c) { + if (($c['id'] ?? '') === $checkoutSha) { + $latestCommit = $c; + break; + } + } + if (empty($latestCommit) && !empty($commits)) { + $latestCommit = $commits[0]; + } $ref = $payloadArray['ref'] ?? ''; // ref format: refs/heads/main $branch = str_replace('refs/heads/', '', $ref); diff --git a/tests/VCS/Adapter/GitLabTest.php b/tests/VCS/Adapter/GitLabTest.php index 3387613b..105c3b37 100644 --- a/tests/VCS/Adapter/GitLabTest.php +++ b/tests/VCS/Adapter/GitLabTest.php @@ -130,7 +130,31 @@ public function testGetDeletedRepositoryFails(): void public function testGetPullRequestFromBranch(): void { - $this->markTestSkipped('Not implemented for GitLab yet'); + $repositoryName = 'test-get-pr-from-branch-' . \uniqid(); + $this->vcsAdapter->createRepository(static::$owner, $repositoryName, false); + + try { + $this->vcsAdapter->createFile(static::$owner, $repositoryName, 'README.md', '# Test'); + $this->vcsAdapter->createBranch(static::$owner, $repositoryName, 'my-feature', static::$defaultBranch); + $this->vcsAdapter->createFile(static::$owner, $repositoryName, 'feature.txt', 'content', 'Add feature', 'my-feature'); + + $this->vcsAdapter->createPullRequest( + static::$owner, + $repositoryName, + 'Feature MR', + 'my-feature', + static::$defaultBranch + ); + + $result = $this->vcsAdapter->getPullRequestFromBranch(static::$owner, $repositoryName, 'my-feature'); + + $this->assertIsArray($result); + $this->assertNotEmpty($result); + $this->assertArrayHasKey('head', $result); + $this->assertSame('my-feature', $result['head']['ref'] ?? ''); + } finally { + $this->vcsAdapter->deleteRepository(static::$owner, $repositoryName); + } } public function testGetOwnerName(): void @@ -204,12 +228,70 @@ public function testSearchRepositoriesWithSearch(): void public function testCreateComment(): void { - $this->markTestSkipped('Not implemented for GitLab yet'); + $repositoryName = 'test-create-comment-' . \uniqid(); + $this->vcsAdapter->createRepository(static::$owner, $repositoryName, false); + + try { + $this->vcsAdapter->createFile(static::$owner, $repositoryName, 'README.md', '# Test'); + $this->vcsAdapter->createBranch(static::$owner, $repositoryName, 'test-branch', static::$defaultBranch); + $this->vcsAdapter->createFile(static::$owner, $repositoryName, 'test.txt', 'test', 'Add test', 'test-branch'); + + $pr = $this->vcsAdapter->createPullRequest( + static::$owner, + $repositoryName, + 'Test PR', + 'test-branch', + static::$defaultBranch + ); + + $prNumber = $pr['iid'] ?? 0; + $this->assertGreaterThan(0, $prNumber); + + $commentId = $this->vcsAdapter->createComment(static::$owner, $repositoryName, $prNumber, 'Test comment'); + + $this->assertNotEmpty($commentId); + $this->assertIsString($commentId); + $this->assertIsNumeric($commentId); + + $retrieved = $this->vcsAdapter->getComment(static::$owner, $repositoryName, $commentId); + $this->assertSame('Test comment', $retrieved); + } finally { + $this->vcsAdapter->deleteRepository(static::$owner, $repositoryName); + } } public function testUpdateComment(): void { - $this->markTestSkipped('Not implemented for GitLab yet'); + $repositoryName = 'test-update-comment-' . \uniqid(); + $this->vcsAdapter->createRepository(static::$owner, $repositoryName, false); + + try { + $this->vcsAdapter->createFile(static::$owner, $repositoryName, 'README.md', '# Test'); + $this->vcsAdapter->createBranch(static::$owner, $repositoryName, 'test-branch', static::$defaultBranch); + $this->vcsAdapter->createFile(static::$owner, $repositoryName, 'test.txt', 'test', 'Add test', 'test-branch'); + + $pr = $this->vcsAdapter->createPullRequest( + static::$owner, + $repositoryName, + 'Test PR', + 'test-branch', + static::$defaultBranch + ); + + $prNumber = $pr['iid'] ?? 0; + $this->assertGreaterThan(0, $prNumber); + + $commentId = $this->vcsAdapter->createComment(static::$owner, $repositoryName, $prNumber, 'Original comment'); + + $updatedCommentId = $this->vcsAdapter->updateComment(static::$owner, $repositoryName, (int)$commentId, 'Updated comment'); + + $this->assertSame($commentId, $updatedCommentId); + + $finalComment = $this->vcsAdapter->getComment(static::$owner, $repositoryName, $commentId); + $this->assertSame('Updated comment', $finalComment); + } finally { + $this->vcsAdapter->deleteRepository(static::$owner, $repositoryName); + } } public function testGenerateCloneCommand(): void @@ -601,7 +683,7 @@ public function testWebhookPullRequestEvent(): void }, 15000, 1000); $this->assertSame('merge_request', $payload['object_kind'] ?? ''); - $this->assertSame('open', $payload['object_attributes']['action'] ?? ''); + $this->assertContains($payload['object_attributes']['action'] ?? '', ['open', 'update']); } finally { $this->vcsAdapter->deleteRepository(static::$owner, $repositoryName); @@ -727,22 +809,118 @@ public function testGetRepositoryNameWithInvalidId(): void public function testGetComment(): void { - $this->markTestSkipped('Not implemented for GitLab yet'); + $repositoryName = 'test-get-comment-' . \uniqid(); + $this->vcsAdapter->createRepository(static::$owner, $repositoryName, false); + + try { + $this->vcsAdapter->createFile(static::$owner, $repositoryName, 'README.md', '# Test'); + $this->vcsAdapter->createBranch(static::$owner, $repositoryName, 'test-branch', static::$defaultBranch); + $this->vcsAdapter->createFile(static::$owner, $repositoryName, 'test.txt', 'test', 'Add test', 'test-branch'); + + $pr = $this->vcsAdapter->createPullRequest( + static::$owner, + $repositoryName, + 'Test PR', + 'test-branch', + static::$defaultBranch + ); + + $prNumber = $pr['iid'] ?? 0; + $commentId = $this->vcsAdapter->createComment(static::$owner, $repositoryName, $prNumber, 'Test comment'); + + $result = $this->vcsAdapter->getComment(static::$owner, $repositoryName, $commentId); + + $this->assertIsString($result); + $this->assertSame('Test comment', $result); + } finally { + $this->vcsAdapter->deleteRepository(static::$owner, $repositoryName); + } } public function testGetPullRequest(): void { - $this->markTestSkipped('Not implemented for GitLab yet'); + $repositoryName = 'test-get-pull-request-' . \uniqid(); + $this->vcsAdapter->createRepository(static::$owner, $repositoryName, false); + + try { + $this->vcsAdapter->createFile(static::$owner, $repositoryName, 'README.md', '# Test'); + $this->vcsAdapter->createBranch(static::$owner, $repositoryName, 'feature-branch', static::$defaultBranch); + $this->vcsAdapter->createFile(static::$owner, $repositoryName, 'feature.txt', 'feature content', 'Add feature', 'feature-branch'); + + $pr = $this->vcsAdapter->createPullRequest( + static::$owner, + $repositoryName, + 'Test MR', + 'feature-branch', + static::$defaultBranch, + 'Test MR description' + ); + + $prNumber = $pr['iid'] ?? 0; + $this->assertGreaterThan(0, $prNumber); + + $result = $this->vcsAdapter->getPullRequest(static::$owner, $repositoryName, $prNumber); + + $this->assertIsArray($result); + $this->assertArrayHasKey('number', $result); + $this->assertArrayHasKey('title', $result); + $this->assertArrayHasKey('state', $result); + $this->assertArrayHasKey('head', $result); + $this->assertArrayHasKey('base', $result); + $this->assertSame($prNumber, $result['number']); + $this->assertSame('Test MR', $result['title']); + $this->assertSame('opened', $result['state']); + } finally { + $this->vcsAdapter->deleteRepository(static::$owner, $repositoryName); + } } public function testGetPullRequestFiles(): void { - $this->markTestSkipped('Not implemented for GitLab yet'); + $repositoryName = 'test-get-pull-request-files-' . \uniqid(); + $this->vcsAdapter->createRepository(static::$owner, $repositoryName, false); + + try { + $this->vcsAdapter->createFile(static::$owner, $repositoryName, 'README.md', '# Test'); + $this->vcsAdapter->createBranch(static::$owner, $repositoryName, 'feature-branch', static::$defaultBranch); + $this->vcsAdapter->createFile(static::$owner, $repositoryName, 'feature.txt', 'feature content', 'Add feature', 'feature-branch'); + + $pr = $this->vcsAdapter->createPullRequest( + static::$owner, + $repositoryName, + 'Test MR Files', + 'feature-branch', + static::$defaultBranch + ); + + $prNumber = $pr['iid'] ?? 0; + $this->assertGreaterThan(0, $prNumber); + + $result = []; + $this->assertEventually(function () use (&$result, $repositoryName, $prNumber) { + $result = $this->vcsAdapter->getPullRequestFiles(static::$owner, $repositoryName, $prNumber); + $this->assertNotEmpty($result); + }, 15000, 1000); + + $this->assertIsArray($result); + $filenames = array_column($result, 'filename'); + $this->assertContains('feature.txt', $filenames); + } finally { + $this->vcsAdapter->deleteRepository(static::$owner, $repositoryName); + } } public function testGetPullRequestWithInvalidNumber(): void { - $this->markTestSkipped('Not implemented for GitLab yet'); + $repositoryName = 'test-get-pull-request-invalid-' . \uniqid(); + $this->vcsAdapter->createRepository(static::$owner, $repositoryName, false); + + try { + $this->expectException(\Exception::class); + $this->vcsAdapter->getPullRequest(static::$owner, $repositoryName, 99999); + } finally { + $this->vcsAdapter->deleteRepository(static::$owner, $repositoryName); + } } public function testGetRepositoryTree(): void @@ -872,17 +1050,18 @@ public function testListRepositoryLanguages(): void { $repositoryName = 'test-list-repository-languages-' . \uniqid(); $this->vcsAdapter->createRepository(static::$owner, $repositoryName, false); - + try { $this->vcsAdapter->createFile(static::$owner, $repositoryName, 'main.php', 'vcsAdapter->createFile(static::$owner, $repositoryName, 'script.js', 'console.log("test");'); - - sleep(5); // ← increase from 2 to 5 - - $languages = $this->vcsAdapter->listRepositoryLanguages(static::$owner, $repositoryName); - + + $languages = []; + $this->assertEventually(function () use (&$languages, $repositoryName) { + $languages = $this->vcsAdapter->listRepositoryLanguages(static::$owner, $repositoryName); + $this->assertNotEmpty($languages); + }, 30000, 2000); + $this->assertIsArray($languages); - $this->assertNotEmpty($languages); $this->assertContains('PHP', $languages); } finally { $this->vcsAdapter->deleteRepository(static::$owner, $repositoryName); @@ -933,4 +1112,199 @@ public function testListRepositoryContents(): void $this->vcsAdapter->deleteRepository(static::$owner, $repositoryName); } } + + public function testGetUser(): void + { + $result = $this->vcsAdapter->getUser('root'); + + $this->assertIsArray($result); + $this->assertArrayHasKey('id', $result); + $this->assertArrayHasKey('username', $result); + } + + public function testGetUserWithInvalidUsername(): void + { + $this->expectException(\Exception::class); + $this->vcsAdapter->getUser('non-existent-user-' . \uniqid()); + } + + public function testCreatePrivateRepository(): void + { + $repositoryName = 'test-create-private-repository-' . \uniqid(); + + $result = $this->vcsAdapter->createRepository(static::$owner, $repositoryName, true); + + try { + $this->assertIsArray($result); + $this->assertSame('private', $result['visibility']); + + $fetched = $this->vcsAdapter->getRepository(static::$owner, $repositoryName); + $this->assertSame('private', $fetched['visibility']); + } finally { + $this->vcsAdapter->deleteRepository(static::$owner, $repositoryName); + } + } + + public function testGetRepositoryWithNonExistingOwner(): void + { + $repositoryName = 'test-non-existing-owner-' . \uniqid(); + + $this->expectException(\Exception::class); + $this->vcsAdapter->getRepository('non-existing-owner-' . \uniqid(), $repositoryName); + } + + public function testCreateRepositoryWithInvalidName(): void + { + $repositoryName = 'invalid name with spaces'; + + try { + $this->vcsAdapter->createRepository(static::$owner, $repositoryName, false); + $this->fail('Expected exception for invalid repository name'); + } catch (\Exception $e) { + $this->assertTrue(true); + } + } + + public function testDeleteRepositoryTwiceFails(): void + { + $repositoryName = 'test-delete-repository-twice-' . \uniqid(); + $this->vcsAdapter->createRepository(static::$owner, $repositoryName, false); + $this->vcsAdapter->deleteRepository(static::$owner, $repositoryName); + + $this->expectException(\Exception::class); + $this->vcsAdapter->deleteRepository(static::$owner, $repositoryName); + } + + public function testDeleteNonExistingRepositoryFails(): void + { + $this->expectException(\Exception::class); + $this->vcsAdapter->deleteRepository(static::$owner, 'non-existing-repo-' . \uniqid()); + } + + public function testGetPullRequestFromBranchNoPR(): void + { + $repositoryName = 'test-get-pr-no-pr-' . \uniqid(); + $this->vcsAdapter->createRepository(static::$owner, $repositoryName, false); + + try { + $this->vcsAdapter->createFile(static::$owner, $repositoryName, 'README.md', '# Test'); + $this->vcsAdapter->createBranch(static::$owner, $repositoryName, 'lonely-branch', static::$defaultBranch); + + $result = $this->vcsAdapter->getPullRequestFromBranch(static::$owner, $repositoryName, 'lonely-branch'); + + $this->assertIsArray($result); + $this->assertEmpty($result); + } finally { + $this->vcsAdapter->deleteRepository(static::$owner, $repositoryName); + } + } + + public function testCreateCommentInvalidPR(): void + { + $repositoryName = 'test-comment-invalid-' . \uniqid(); + $this->vcsAdapter->createRepository(static::$owner, $repositoryName, false); + $this->vcsAdapter->createFile(static::$owner, $repositoryName, 'README.md', '# Test'); + + try { + $this->expectException(\Exception::class); + $this->vcsAdapter->createComment(static::$owner, $repositoryName, 99999, 'Test comment'); + } finally { + $this->vcsAdapter->deleteRepository(static::$owner, $repositoryName); + } + } + + public function testGetCommentInvalidId(): void + { + $repositoryName = 'test-get-comment-invalid-' . \uniqid(); + $this->vcsAdapter->createRepository(static::$owner, $repositoryName, false); + $this->vcsAdapter->createFile(static::$owner, $repositoryName, 'README.md', '# Test'); + + try { + $result = $this->vcsAdapter->getComment(static::$owner, $repositoryName, '99999999'); + + $this->assertIsString($result); + $this->assertSame('', $result); + } finally { + $this->vcsAdapter->deleteRepository(static::$owner, $repositoryName); + } + } + + public function testGetEventPushMatchesCheckoutSha(): void + { + // Multi-commit push — checkout_sha points to the SECOND commit + // Our implementation should match author/message to checkout_sha, not just take commits[0] + $payload = json_encode([ + 'object_kind' => 'push', + 'ref' => 'refs/heads/main', + 'checkout_sha' => 'def456', + 'project' => [ + 'name' => 'test-repo', + 'namespace' => 'test-org', + ], + 'commits' => [ + [ + 'id' => 'abc123', + 'message' => 'Older commit', + 'url' => 'http://example.com/commit/abc123', + 'author' => ['name' => 'Old Author'], + ], + [ + 'id' => 'def456', + 'message' => 'Head commit', + 'url' => 'http://example.com/commit/def456', + 'author' => ['name' => 'Head Author'], + ], + ], + ]); + + $result = $this->vcsAdapter->getEvent('Push Hook', $payload); + + $this->assertIsArray($result); + $this->assertSame('def456', $result['commitHash']); + // Must match HEAD commit, not first commit in array + $this->assertSame('Head Author', $result['commitAuthor']); + $this->assertSame('Head commit', $result['commitMessage']); + $this->assertSame('http://example.com/commit/def456', $result['commitUrl']); + } + + public function testValidateWebhookEventUsesPlainToken(): void + { + $secret = 'my-secret-token'; + $payload = '{"object_kind":"push"}'; + + // GitLab validates by direct token comparison, not HMAC + // So the "signature" sent in X-Gitlab-Token IS the secret itself + $this->assertTrue( + $this->vcsAdapter->validateWebhookEvent($payload, $secret, $secret) + ); + + // An HMAC of the payload should NOT validate — unlike Gitea/GitHub + $hmacSignature = hash_hmac('sha256', $payload, $secret); + $this->assertFalse( + $this->vcsAdapter->validateWebhookEvent($payload, $hmacSignature, $secret) + ); + + // Any wrong value fails + $this->assertFalse( + $this->vcsAdapter->validateWebhookEvent($payload, 'wrong-token', $secret) + ); + } + + public function testCreateOrganization(): void + { + $orgName = 'test-create-org-' . \uniqid(); + + $result = $this->vcsAdapter->createOrganization($orgName); + + $this->assertIsString($result); + $this->assertNotEmpty($result); + + // GitLab returns "id:path" format — both parts must be present + $this->assertStringContainsString(':', $result); + + $parts = explode(':', $result); + $this->assertCount(2, $parts); + $this->assertIsNumeric($parts[0]); // numeric group ID + $this->assertSame($orgName, $parts[1]); // path matches org name + } } From 8a688e865ec3aaf94c41a89aa13fbd51e4ded925 Mon Sep 17 00:00:00 2001 From: Jayesh Somani Date: Mon, 27 Apr 2026 14:11:17 +0530 Subject: [PATCH 2/4] fixed the lint issue and stan issue --- tests/VCS/Adapter/GitLabTest.php | 374 ++++++++++++++++++++++++++++++- 1 file changed, 364 insertions(+), 10 deletions(-) diff --git a/tests/VCS/Adapter/GitLabTest.php b/tests/VCS/Adapter/GitLabTest.php index fa4600ff..0b520a0e 100644 --- a/tests/VCS/Adapter/GitLabTest.php +++ b/tests/VCS/Adapter/GitLabTest.php @@ -130,7 +130,31 @@ public function testGetDeletedRepositoryFails(): void public function testGetPullRequestFromBranch(): void { - $this->markTestSkipped('Not implemented for GitLab yet'); + $repositoryName = 'test-get-pr-from-branch-' . \uniqid(); + $this->vcsAdapter->createRepository(static::$owner, $repositoryName, false); + + try { + $this->vcsAdapter->createFile(static::$owner, $repositoryName, 'README.md', '# Test'); + $this->vcsAdapter->createBranch(static::$owner, $repositoryName, 'my-feature', static::$defaultBranch); + $this->vcsAdapter->createFile(static::$owner, $repositoryName, 'feature.txt', 'content', 'Add feature', 'my-feature'); + + $this->vcsAdapter->createPullRequest( + static::$owner, + $repositoryName, + 'Feature MR', + 'my-feature', + static::$defaultBranch + ); + + $result = $this->vcsAdapter->getPullRequestFromBranch(static::$owner, $repositoryName, 'my-feature'); + + $this->assertIsArray($result); + $this->assertNotEmpty($result); + $this->assertArrayHasKey('head', $result); + $this->assertSame('my-feature', $result['head']['ref'] ?? ''); + } finally { + $this->vcsAdapter->deleteRepository(static::$owner, $repositoryName); + } } public function testGetOwnerName(): void @@ -204,12 +228,70 @@ public function testSearchRepositoriesWithSearch(): void public function testCreateComment(): void { - $this->markTestSkipped('Not implemented for GitLab yet'); + $repositoryName = 'test-create-comment-' . \uniqid(); + $this->vcsAdapter->createRepository(static::$owner, $repositoryName, false); + + try { + $this->vcsAdapter->createFile(static::$owner, $repositoryName, 'README.md', '# Test'); + $this->vcsAdapter->createBranch(static::$owner, $repositoryName, 'test-branch', static::$defaultBranch); + $this->vcsAdapter->createFile(static::$owner, $repositoryName, 'test.txt', 'test', 'Add test', 'test-branch'); + + $pr = $this->vcsAdapter->createPullRequest( + static::$owner, + $repositoryName, + 'Test PR', + 'test-branch', + static::$defaultBranch + ); + + $prNumber = $pr['iid'] ?? 0; + $this->assertGreaterThan(0, $prNumber); + + $commentId = $this->vcsAdapter->createComment(static::$owner, $repositoryName, $prNumber, 'Test comment'); + + $this->assertNotEmpty($commentId); + $this->assertIsString($commentId); + $this->assertIsNumeric($commentId); + + $retrieved = $this->vcsAdapter->getComment(static::$owner, $repositoryName, $commentId); + $this->assertSame('Test comment', $retrieved); + } finally { + $this->vcsAdapter->deleteRepository(static::$owner, $repositoryName); + } } public function testUpdateComment(): void { - $this->markTestSkipped('Not implemented for GitLab yet'); + $repositoryName = 'test-update-comment-' . \uniqid(); + $this->vcsAdapter->createRepository(static::$owner, $repositoryName, false); + + try { + $this->vcsAdapter->createFile(static::$owner, $repositoryName, 'README.md', '# Test'); + $this->vcsAdapter->createBranch(static::$owner, $repositoryName, 'test-branch', static::$defaultBranch); + $this->vcsAdapter->createFile(static::$owner, $repositoryName, 'test.txt', 'test', 'Add test', 'test-branch'); + + $pr = $this->vcsAdapter->createPullRequest( + static::$owner, + $repositoryName, + 'Test PR', + 'test-branch', + static::$defaultBranch + ); + + $prNumber = $pr['iid'] ?? 0; + $this->assertGreaterThan(0, $prNumber); + + $commentId = $this->vcsAdapter->createComment(static::$owner, $repositoryName, $prNumber, 'Original comment'); + + $updatedCommentId = $this->vcsAdapter->updateComment(static::$owner, $repositoryName, (int)$commentId, 'Updated comment'); + + $this->assertSame($commentId, $updatedCommentId); + + $finalComment = $this->vcsAdapter->getComment(static::$owner, $repositoryName, $commentId); + $this->assertSame('Updated comment', $finalComment); + } finally { + $this->vcsAdapter->deleteRepository(static::$owner, $repositoryName); + } } public function testGenerateCloneCommand(): void @@ -735,22 +817,118 @@ public function testGetRepositoryNameWithInvalidId(): void public function testGetComment(): void { - $this->markTestSkipped('Not implemented for GitLab yet'); + $repositoryName = 'test-get-comment-' . \uniqid(); + $this->vcsAdapter->createRepository(static::$owner, $repositoryName, false); + + try { + $this->vcsAdapter->createFile(static::$owner, $repositoryName, 'README.md', '# Test'); + $this->vcsAdapter->createBranch(static::$owner, $repositoryName, 'test-branch', static::$defaultBranch); + $this->vcsAdapter->createFile(static::$owner, $repositoryName, 'test.txt', 'test', 'Add test', 'test-branch'); + + $pr = $this->vcsAdapter->createPullRequest( + static::$owner, + $repositoryName, + 'Test PR', + 'test-branch', + static::$defaultBranch + ); + + $prNumber = $pr['iid'] ?? 0; + $commentId = $this->vcsAdapter->createComment(static::$owner, $repositoryName, $prNumber, 'Test comment'); + + $result = $this->vcsAdapter->getComment(static::$owner, $repositoryName, $commentId); + + $this->assertIsString($result); + $this->assertSame('Test comment', $result); + } finally { + $this->vcsAdapter->deleteRepository(static::$owner, $repositoryName); + } } public function testGetPullRequest(): void { - $this->markTestSkipped('Not implemented for GitLab yet'); + $repositoryName = 'test-get-pull-request-' . \uniqid(); + $this->vcsAdapter->createRepository(static::$owner, $repositoryName, false); + + try { + $this->vcsAdapter->createFile(static::$owner, $repositoryName, 'README.md', '# Test'); + $this->vcsAdapter->createBranch(static::$owner, $repositoryName, 'feature-branch', static::$defaultBranch); + $this->vcsAdapter->createFile(static::$owner, $repositoryName, 'feature.txt', 'feature content', 'Add feature', 'feature-branch'); + + $pr = $this->vcsAdapter->createPullRequest( + static::$owner, + $repositoryName, + 'Test MR', + 'feature-branch', + static::$defaultBranch, + 'Test MR description' + ); + + $prNumber = $pr['iid'] ?? 0; + $this->assertGreaterThan(0, $prNumber); + + $result = $this->vcsAdapter->getPullRequest(static::$owner, $repositoryName, $prNumber); + + $this->assertIsArray($result); + $this->assertArrayHasKey('number', $result); + $this->assertArrayHasKey('title', $result); + $this->assertArrayHasKey('state', $result); + $this->assertArrayHasKey('head', $result); + $this->assertArrayHasKey('base', $result); + $this->assertSame($prNumber, $result['number']); + $this->assertSame('Test MR', $result['title']); + $this->assertSame('opened', $result['state']); + } finally { + $this->vcsAdapter->deleteRepository(static::$owner, $repositoryName); + } } public function testGetPullRequestFiles(): void { - $this->markTestSkipped('Not implemented for GitLab yet'); + $repositoryName = 'test-get-pull-request-files-' . \uniqid(); + $this->vcsAdapter->createRepository(static::$owner, $repositoryName, false); + + try { + $this->vcsAdapter->createFile(static::$owner, $repositoryName, 'README.md', '# Test'); + $this->vcsAdapter->createBranch(static::$owner, $repositoryName, 'feature-branch', static::$defaultBranch); + $this->vcsAdapter->createFile(static::$owner, $repositoryName, 'feature.txt', 'feature content', 'Add feature', 'feature-branch'); + + $pr = $this->vcsAdapter->createPullRequest( + static::$owner, + $repositoryName, + 'Test MR Files', + 'feature-branch', + static::$defaultBranch + ); + + $prNumber = $pr['iid'] ?? 0; + $this->assertGreaterThan(0, $prNumber); + + $result = []; + $this->assertEventually(function () use (&$result, $repositoryName, $prNumber) { + $result = $this->vcsAdapter->getPullRequestFiles(static::$owner, $repositoryName, $prNumber); + $this->assertNotEmpty($result); + }, 15000, 1000); + + $this->assertIsArray($result); + $filenames = array_column($result, 'filename'); + $this->assertContains('feature.txt', $filenames); + } finally { + $this->vcsAdapter->deleteRepository(static::$owner, $repositoryName); + } } public function testGetPullRequestWithInvalidNumber(): void { - $this->markTestSkipped('Not implemented for GitLab yet'); + $repositoryName = 'test-get-pull-request-invalid-' . \uniqid(); + $this->vcsAdapter->createRepository(static::$owner, $repositoryName, false); + + try { + $this->expectException(\Exception::class); + $this->vcsAdapter->getPullRequest(static::$owner, $repositoryName, 99999); + } finally { + $this->vcsAdapter->deleteRepository(static::$owner, $repositoryName); + } } public function testGetRepositoryTree(): void @@ -884,9 +1062,11 @@ public function testListRepositoryLanguages(): void $this->vcsAdapter->createFile(static::$owner, $repositoryName, 'main.php', 'vcsAdapter->createFile(static::$owner, $repositoryName, 'script.js', 'console.log("test");'); - sleep(5); // ← increase from 2 to 5 - - $languages = $this->vcsAdapter->listRepositoryLanguages(static::$owner, $repositoryName); + $languages = []; + $this->assertEventually(function () use (&$languages, $repositoryName) { + $languages = $this->vcsAdapter->listRepositoryLanguages(static::$owner, $repositoryName); + $this->assertNotEmpty($languages); + }, 30000, 2000); $this->assertIsArray($languages); $this->assertNotEmpty($languages); @@ -940,4 +1120,178 @@ public function testListRepositoryContents(): void $this->vcsAdapter->deleteRepository(static::$owner, $repositoryName); } } + + public function testGetUser(): void + { + $result = $this->vcsAdapter->getUser('root'); + + $this->assertIsArray($result); + $this->assertArrayHasKey('id', $result); + $this->assertArrayHasKey('username', $result); + } + + public function testGetUserWithInvalidUsername(): void + { + $this->expectException(\Exception::class); + $this->vcsAdapter->getUser('non-existent-user-' . \uniqid()); + } + + public function testCreatePrivateRepository(): void + { + $repositoryName = 'test-create-private-repository-' . \uniqid(); + + $result = $this->vcsAdapter->createRepository(static::$owner, $repositoryName, true); + + try { + $this->assertIsArray($result); + $this->assertSame('private', $result['visibility']); + + $fetched = $this->vcsAdapter->getRepository(static::$owner, $repositoryName); + $this->assertSame('private', $fetched['visibility']); + } finally { + $this->vcsAdapter->deleteRepository(static::$owner, $repositoryName); + } + } + + public function testGetRepositoryWithNonExistingOwner(): void + { + $repositoryName = 'test-non-existing-owner-' . \uniqid(); + + $this->expectException(\Exception::class); + $this->vcsAdapter->getRepository('non-existing-owner-' . \uniqid(), $repositoryName); + } + + public function testCreateRepositoryWithInvalidName(): void + { + $repositoryName = 'invalid name with spaces'; + + try { + $this->vcsAdapter->createRepository(static::$owner, $repositoryName, false); + $this->fail('Expected exception for invalid repository name'); + } catch (\Exception $e) { + $this->assertTrue(true); + } + } + + public function testDeleteRepositoryTwiceFails(): void + { + $repositoryName = 'test-delete-repository-twice-' . \uniqid(); + $this->vcsAdapter->createRepository(static::$owner, $repositoryName, false); + $this->vcsAdapter->deleteRepository(static::$owner, $repositoryName); + + $this->expectException(\Exception::class); + $this->vcsAdapter->deleteRepository(static::$owner, $repositoryName); + } + + public function testDeleteNonExistingRepositoryFails(): void + { + $this->expectException(\Exception::class); + $this->vcsAdapter->deleteRepository(static::$owner, 'non-existing-repo-' . \uniqid()); + } + + public function testGetPullRequestFromBranchNoPR(): void + { + $repositoryName = 'test-get-pr-no-pr-' . \uniqid(); + $this->vcsAdapter->createRepository(static::$owner, $repositoryName, false); + + try { + $this->vcsAdapter->createFile(static::$owner, $repositoryName, 'README.md', '# Test'); + $this->vcsAdapter->createBranch(static::$owner, $repositoryName, 'lonely-branch', static::$defaultBranch); + + $result = $this->vcsAdapter->getPullRequestFromBranch(static::$owner, $repositoryName, 'lonely-branch'); + + $this->assertIsArray($result); + $this->assertEmpty($result); + } finally { + $this->vcsAdapter->deleteRepository(static::$owner, $repositoryName); + } + } + + public function testCreateCommentInvalidPR(): void + { + $repositoryName = 'test-comment-invalid-' . \uniqid(); + $this->vcsAdapter->createRepository(static::$owner, $repositoryName, false); + $this->vcsAdapter->createFile(static::$owner, $repositoryName, 'README.md', '# Test'); + + try { + $this->expectException(\Exception::class); + $this->vcsAdapter->createComment(static::$owner, $repositoryName, 99999, 'Test comment'); + } finally { + $this->vcsAdapter->deleteRepository(static::$owner, $repositoryName); + } + } + + public function testGetCommentInvalidId(): void + { + $repositoryName = 'test-get-comment-invalid-' . \uniqid(); + $this->vcsAdapter->createRepository(static::$owner, $repositoryName, false); + $this->vcsAdapter->createFile(static::$owner, $repositoryName, 'README.md', '# Test'); + + try { + $result = $this->vcsAdapter->getComment(static::$owner, $repositoryName, '99999999'); + + $this->assertIsString($result); + $this->assertSame('', $result); + } finally { + $this->vcsAdapter->deleteRepository(static::$owner, $repositoryName); + } + } + + public function testGetEventPushMatchesCheckoutSha(): void + { + $payload = json_encode([ + 'object_kind' => 'push', + 'ref' => 'refs/heads/main', + 'checkout_sha' => 'def456', + 'project' => [ + 'name' => 'test-repo', + 'namespace' => 'test-org', + ], + 'commits' => [ + [ + 'id' => 'abc123', + 'message' => 'Older commit', + 'url' => 'http://example.com/commit/abc123', + 'author' => ['name' => 'Old Author'], + ], + [ + 'id' => 'def456', + 'message' => 'Head commit', + 'url' => 'http://example.com/commit/def456', + 'author' => ['name' => 'Head Author'], + ], + ], + ]); + + if ($payload === false) { + $this->fail('Failed to encode JSON payload'); + } + + $result = $this->vcsAdapter->getEvent('Push Hook', $payload); + + $this->assertIsArray($result); + $this->assertSame('def456', $result['commitHash']); + $this->assertSame('Head Author', $result['commitAuthor']); + $this->assertSame('Head commit', $result['commitMessage']); + $this->assertSame('http://example.com/commit/def456', $result['commitUrl']); + } + + public function testValidateWebhookEventUsesPlainToken(): void + { + $secret = 'my-secret-token'; + $payload = '{"object_kind":"push"}'; + + $this->assertTrue( + $this->vcsAdapter->validateWebhookEvent($payload, $secret, $secret) + ); + + $hmacSignature = hash_hmac('sha256', $payload, $secret); + $this->assertFalse( + $this->vcsAdapter->validateWebhookEvent($payload, $hmacSignature, $secret) + ); + + $this->assertFalse( + $this->vcsAdapter->validateWebhookEvent($payload, 'wrong-token', $secret) + ); + } } From e86e6594cc47c80f83c3bc35cdea5ac1f144fae3 Mon Sep 17 00:00:00 2001 From: Jayesh Somani Date: Mon, 27 Apr 2026 14:48:30 +0530 Subject: [PATCH 3/4] updated with suggestions --- src/VCS/Adapter/Git/GitLab.php | 8 +++++++- tests/VCS/Adapter/GitLabTest.php | 2 +- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/VCS/Adapter/Git/GitLab.php b/src/VCS/Adapter/Git/GitLab.php index 6c023cba..cc0a7407 100644 --- a/src/VCS/Adapter/Git/GitLab.php +++ b/src/VCS/Adapter/Git/GitLab.php @@ -105,8 +105,14 @@ private function findMrIidForNote(string $projectPath, string $commentId): ?int while (true) { $url = "/projects/{$projectPath}/merge_requests?state=all&per_page=100&page={$page}"; $response = $this->call(self::METHOD_GET, $url, ['PRIVATE-TOKEN' => $this->accessToken]); + $responseHeaders = $response['headers'] ?? []; + $statusCode = $responseHeaders['status-code'] ?? 0; + if ($statusCode >= 400) { + throw new Exception("Failed to list merge requests: HTTP {$statusCode}"); + } + $mrs = $response['body'] ?? []; - + if (empty($mrs) || !is_array($mrs)) { break; } diff --git a/tests/VCS/Adapter/GitLabTest.php b/tests/VCS/Adapter/GitLabTest.php index 0b520a0e..8da97e59 100644 --- a/tests/VCS/Adapter/GitLabTest.php +++ b/tests/VCS/Adapter/GitLabTest.php @@ -683,7 +683,7 @@ public function testWebhookPullRequestEvent(): void }, 15000, 1000); $this->assertSame('merge_request', $payload['object_kind'] ?? ''); - $this->assertSame('open', $payload['object_attributes']['action'] ?? ''); + $this->assertContains($payload['object_attributes']['action'] ?? '', ['open', 'update']); } finally { $this->vcsAdapter->deleteRepository(static::$owner, $repositoryName); From c6f5ac420b0ed5eeae1b423bfd0ded336754adb7 Mon Sep 17 00:00:00 2001 From: Jayesh Somani Date: Wed, 13 May 2026 16:11:24 +0530 Subject: [PATCH 4/4] updated the ID fix for comments --- src/VCS/Adapter.php | 4 +- src/VCS/Adapter/Git/GitHub.php | 4 +- src/VCS/Adapter/Git/GitLab.php | 72 +++++++++----------------------- src/VCS/Adapter/Git/Gitea.php | 2 +- tests/VCS/Adapter/GitLabTest.php | 4 +- tests/VCS/Adapter/GiteaTest.php | 4 +- 6 files changed, 28 insertions(+), 62 deletions(-) diff --git a/src/VCS/Adapter.php b/src/VCS/Adapter.php index ab8c48cf..be9504de 100644 --- a/src/VCS/Adapter.php +++ b/src/VCS/Adapter.php @@ -182,11 +182,11 @@ abstract public function getComment(string $owner, string $repositoryName, strin * * @param string $owner The owner of the repository * @param string $repositoryName The name of the repository - * @param int $commentId The ID of the comment to update + * @param string $commentId The ID of the comment to update * @param string $comment The updated comment content * @return string The ID of the updated comment */ - abstract public function updateComment(string $owner, string $repositoryName, int $commentId, string $comment): string; + abstract public function updateComment(string $owner, string $repositoryName, string $commentId, string $comment): string; /** * Generates a clone command using app access token diff --git a/src/VCS/Adapter/Git/GitHub.php b/src/VCS/Adapter/Git/GitHub.php index 6e1c8de0..2a11cc7e 100644 --- a/src/VCS/Adapter/Git/GitHub.php +++ b/src/VCS/Adapter/Git/GitHub.php @@ -579,13 +579,13 @@ public function getComment(string $owner, string $repositoryName, string $commen * * @param string $owner The owner of the repository * @param string $repositoryName The name of the repository - * @param int $commentId The ID of the comment to update + * @param string $commentId The ID of the comment to update * @param string $comment The updated comment content * @return string The ID of the updated comment * * @throws Exception */ - public function updateComment(string $owner, string $repositoryName, int $commentId, string $comment): string + public function updateComment(string $owner, string $repositoryName, string $commentId, string $comment): string { $url = '/repos/' . $owner . '/' . $repositoryName . '/issues/comments/' . $commentId; diff --git a/src/VCS/Adapter/Git/GitLab.php b/src/VCS/Adapter/Git/GitLab.php index cc0a7407..e1e12eb7 100644 --- a/src/VCS/Adapter/Git/GitLab.php +++ b/src/VCS/Adapter/Git/GitLab.php @@ -99,42 +99,6 @@ private function getNamespaceId(string $owner): string return $owner; } - private function findMrIidForNote(string $projectPath, string $commentId): ?int - { - $page = 1; - while (true) { - $url = "/projects/{$projectPath}/merge_requests?state=all&per_page=100&page={$page}"; - $response = $this->call(self::METHOD_GET, $url, ['PRIVATE-TOKEN' => $this->accessToken]); - $responseHeaders = $response['headers'] ?? []; - $statusCode = $responseHeaders['status-code'] ?? 0; - if ($statusCode >= 400) { - throw new Exception("Failed to list merge requests: HTTP {$statusCode}"); - } - - $mrs = $response['body'] ?? []; - - if (empty($mrs) || !is_array($mrs)) { - break; - } - - foreach ($mrs as $mr) { - $mrIid = $mr['iid'] ?? 0; - $noteUrl = "/projects/{$projectPath}/merge_requests/{$mrIid}/notes/{$commentId}"; - $noteResponse = $this->call(self::METHOD_GET, $noteUrl, ['PRIVATE-TOKEN' => $this->accessToken]); - if (($noteResponse['headers']['status-code'] ?? 0) === 200) { - return $mrIid; - } - } - - if (count($mrs) < 100) { - break; - } - $page++; - } - - return null; - } - public function createRepository(string $owner, string $repositoryName, bool $private): array { $namespaceId = (int) $this->getNamespaceId($owner); @@ -518,7 +482,7 @@ public function createComment(string $owner, string $repositoryName, int $pullRe throw new Exception("Comment creation response is missing comment ID."); } - return (string) ($responseBody['id'] ?? ''); + return $pullRequestNumber . ':' . ($responseBody['id'] ?? ''); } public function getComment(string $owner, string $repositoryName, string $commentId): string @@ -526,34 +490,38 @@ public function getComment(string $owner, string $repositoryName, string $commen $ownerPath = $this->getOwnerPath($owner); $projectPath = urlencode("{$ownerPath}/{$repositoryName}"); - $mrIid = $this->findMrIidForNote($projectPath, $commentId); - if ($mrIid === null) { + $parts = explode(':', $commentId, 2); + if (count($parts) !== 2) { return ''; } - $noteUrl = "/projects/{$projectPath}/merge_requests/{$mrIid}/notes/{$commentId}"; - $noteResponse = $this->call(self::METHOD_GET, $noteUrl, ['PRIVATE-TOKEN' => $this->accessToken]); - return $noteResponse['body']['body'] ?? ''; + [$mrIid, $noteId] = $parts; + $url = "/projects/{$projectPath}/merge_requests/{$mrIid}/notes/{$noteId}"; + $response = $this->call(self::METHOD_GET, $url, ['PRIVATE-TOKEN' => $this->accessToken]); + + return $response['body']['body'] ?? ''; } - public function updateComment(string $owner, string $repositoryName, int $commentId, string $comment): string + public function updateComment(string $owner, string $repositoryName, string $commentId, string $comment): string { $ownerPath = $this->getOwnerPath($owner); $projectPath = urlencode("{$ownerPath}/{$repositoryName}"); - $mrIid = $this->findMrIidForNote($projectPath, (string) $commentId); - if ($mrIid === null) { - throw new Exception("Failed to update comment: comment ID {$commentId} not found in any merge request."); + $parts = explode(':', $commentId, 2); + if (count($parts) !== 2) { + throw new Exception("Invalid comment ID format: {$commentId}"); } - $noteUrl = "/projects/{$projectPath}/merge_requests/{$mrIid}/notes/{$commentId}"; - $noteResponse = $this->call(self::METHOD_PUT, $noteUrl, ['PRIVATE-TOKEN' => $this->accessToken], ['body' => $comment]); - $noteHeaders = $noteResponse['headers'] ?? []; - if (($noteHeaders['status-code'] ?? 0) !== 200) { - throw new Exception("Failed to update comment: HTTP " . ($noteHeaders['status-code'] ?? 0)); + [$mrIid, $noteId] = $parts; + $url = "/projects/{$projectPath}/merge_requests/{$mrIid}/notes/{$noteId}"; + $response = $this->call(self::METHOD_PUT, $url, ['PRIVATE-TOKEN' => $this->accessToken], ['body' => $comment]); + + $responseHeaders = $response['headers'] ?? []; + if (($responseHeaders['status-code'] ?? 0) !== 200) { + throw new Exception("Failed to update comment: HTTP " . ($responseHeaders['status-code'] ?? 0)); } - return (string) $commentId; + return $commentId; } public function getUser(string $username): array diff --git a/src/VCS/Adapter/Git/Gitea.php b/src/VCS/Adapter/Git/Gitea.php index bc6544ef..26a29145 100644 --- a/src/VCS/Adapter/Git/Gitea.php +++ b/src/VCS/Adapter/Git/Gitea.php @@ -569,7 +569,7 @@ public function getComment(string $owner, string $repositoryName, string $commen return $responseBody['body'] ?? ''; } - public function updateComment(string $owner, string $repositoryName, int $commentId, string $comment): string + public function updateComment(string $owner, string $repositoryName, string $commentId, string $comment): string { $url = "/repos/{$owner}/{$repositoryName}/issues/comments/{$commentId}"; diff --git a/tests/VCS/Adapter/GitLabTest.php b/tests/VCS/Adapter/GitLabTest.php index 8da97e59..9f1429b0 100644 --- a/tests/VCS/Adapter/GitLabTest.php +++ b/tests/VCS/Adapter/GitLabTest.php @@ -251,7 +251,6 @@ public function testCreateComment(): void $this->assertNotEmpty($commentId); $this->assertIsString($commentId); - $this->assertIsNumeric($commentId); $retrieved = $this->vcsAdapter->getComment(static::$owner, $repositoryName, $commentId); $this->assertSame('Test comment', $retrieved); @@ -283,7 +282,7 @@ public function testUpdateComment(): void $commentId = $this->vcsAdapter->createComment(static::$owner, $repositoryName, $prNumber, 'Original comment'); - $updatedCommentId = $this->vcsAdapter->updateComment(static::$owner, $repositoryName, (int)$commentId, 'Updated comment'); + $updatedCommentId = $this->vcsAdapter->updateComment(static::$owner, $repositoryName, $commentId, 'Updated comment'); $this->assertSame($commentId, $updatedCommentId); @@ -1229,7 +1228,6 @@ public function testGetCommentInvalidId(): void try { $result = $this->vcsAdapter->getComment(static::$owner, $repositoryName, '99999999'); - $this->assertIsString($result); $this->assertSame('', $result); } finally { diff --git a/tests/VCS/Adapter/GiteaTest.php b/tests/VCS/Adapter/GiteaTest.php index 4fafbf91..b2ab85c9 100644 --- a/tests/VCS/Adapter/GiteaTest.php +++ b/tests/VCS/Adapter/GiteaTest.php @@ -135,7 +135,7 @@ public function testCommentWorkflow(): void $this->assertSame($originalComment, $retrievedComment); $updatedCommentText = 'This comment has been updated'; - $updatedCommentId = $this->vcsAdapter->updateComment(static::$owner, $repositoryName, (int)$commentId, $updatedCommentText); + $updatedCommentId = $this->vcsAdapter->updateComment(static::$owner, $repositoryName, $commentId, $updatedCommentText); $this->assertSame($commentId, $updatedCommentId); @@ -677,7 +677,7 @@ public function testUpdateComment(): void $commentId = $this->vcsAdapter->createComment(static::$owner, $repositoryName, $prNumber, 'Original comment'); // Test updateComment - $updatedCommentId = $this->vcsAdapter->updateComment(static::$owner, $repositoryName, (int)$commentId, 'Updated comment'); + $updatedCommentId = $this->vcsAdapter->updateComment(static::$owner, $repositoryName, (string)$commentId, 'Updated comment'); $this->assertSame($commentId, $updatedCommentId);