From 3e5a583d277d4d5131eefdbd2bb36c8fd8993fa6 Mon Sep 17 00:00:00 2001 From: Matt Glaman Date: Fri, 28 Aug 2026 11:21:20 -0500 Subject: [PATCH 1/3] fix: derive project machine name from git remote when argument is omitted ProjectCommandBase checked InputInterface::hasArgument('project'), which only reports whether the argument is defined on the command, not whether the user passed it. It was always true, so the git remote fallback never ran and the null argument became an empty project name. The parsing now lives in ProjectRemote so it can be unit tested against SSH, HTTPS, and .git-less remote forms, and a missing derivation reports a clear error. Closes #346 Co-Authored-By: Claude Fable 5 --- src/Api/ProjectRemote.php | 30 ++++++++++++++++ .../Command/Project/ProjectCommandBase.php | 27 +++++++------- tests/src/ProjectRemoteTest.php | 36 +++++++++++++++++++ 3 files changed, 79 insertions(+), 14 deletions(-) create mode 100644 src/Api/ProjectRemote.php create mode 100644 tests/src/ProjectRemoteTest.php diff --git a/src/Api/ProjectRemote.php b/src/Api/ProjectRemote.php new file mode 100644 index 0000000..4db9ed0 --- /dev/null +++ b/src/Api/ProjectRemote.php @@ -0,0 +1,30 @@ +[A-Za-z0-9_-]+)(?:\.git)?/?$#'; + + public function __construct(public readonly string $machineName) + { + } + + public static function tryParse(string $remoteUrl): ?self + { + $matches = []; + if (preg_match(self::PATTERN, trim($remoteUrl), $matches) !== 1) { + return null; + } + return new self($matches['name']); + } +} diff --git a/src/Cli/Command/Project/ProjectCommandBase.php b/src/Cli/Command/Project/ProjectCommandBase.php index 6ac8569..4211595 100644 --- a/src/Cli/Command/Project/ProjectCommandBase.php +++ b/src/Cli/Command/Project/ProjectCommandBase.php @@ -3,6 +3,7 @@ namespace mglaman\DrupalOrgCli\Command\Project; use mglaman\DrupalOrg\Entity\Project; +use mglaman\DrupalOrg\ProjectRemote; use mglaman\DrupalOrgCli\Command\Command; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; @@ -29,17 +30,17 @@ protected function initialize(InputInterface $input, OutputInterface $output): v { parent::initialize($input, $output); - if (!$this->stdIn->hasArgument('project')) { + $projectName = $this->stdIn->getArgument('project'); + if (!is_string($projectName) || $projectName === '') { $this->debug("Argument project not provided. Trying to get it from the remote URL of the current repository."); - $this->projectName = $this->getProjectFromRemote(); - if ($this->projectName === '') { - $this->stdErr->writeln("Failed to find project / machine name from current Git repository."); + $remote = ProjectRemote::tryParse($this->getRemoteUrl()); + if ($remote === null) { + $this->stdErr->writeln("Could not determine the project from the git remote; pass the machine name as an argument."); exit(1); } - } else { - $projectName = $this->stdIn->getArgument('project') ?? ''; - $this->projectName = $projectName; + $projectName = $remote->machineName; } + $this->projectName = $projectName; // The kanban and link command doesn't need the project data from drupal.org, // but checking that the project exists makes sense for all project commands. @@ -53,17 +54,15 @@ protected function initialize(InputInterface $input, OutputInterface $output): v } /** - * Gets project from remote origin name. + * Gets the origin remote URL of the current repository. * * @return string - * The project name. + * The remote URL, or an empty string when there is no origin remote. */ - protected function getProjectFromRemote(): string + protected function getRemoteUrl(): string { - $process = new Process((array) 'git config --get remote.origin.url'); + $process = new Process(['git', 'config', '--get', 'remote.origin.url']); $process->run(); - $remote_url = trim($process->getOutput()); - preg_match('#.*\/(.*)\.git$#', $remote_url, $matches); - return $matches[1] ?? ''; + return trim($process->getOutput()); } } diff --git a/tests/src/ProjectRemoteTest.php b/tests/src/ProjectRemoteTest.php new file mode 100644 index 0000000..fd97ed2 --- /dev/null +++ b/tests/src/ProjectRemoteTest.php @@ -0,0 +1,36 @@ + + */ + public static function remoteUrlProvider(): array + { + return [ + 'ssh scp-style' => ['git@git.drupal.org:project/json_form_widget.git', 'json_form_widget'], + 'ssh scp-style on drupalcode' => ['git@git.drupalcode.org:project/json_form_widget.git', 'json_form_widget'], + 'https' => ['https://git.drupalcode.org/project/json_form_widget.git', 'json_form_widget'], + 'https without .git' => ['https://git.drupalcode.org/project/json_form_widget', 'json_form_widget'], + 'ssh scheme' => ['ssh://git@git.drupal.org/project/json_form_widget.git', 'json_form_widget'], + 'trailing newline from git output' => ["git@git.drupal.org:project/json_form_widget.git\n", 'json_form_widget'], + 'github remote' => ['git@github.com:mglaman/drupalorg-cli.git', null], + 'issue fork' => ['git@git.drupal.org:issue/json_form_widget-3000000.git', null], + 'empty' => ['', null], + ]; + } + + #[DataProvider('remoteUrlProvider')] + public function testTryParse(string $remoteUrl, ?string $expected): void + { + self::assertSame($expected, ProjectRemote::tryParse($remoteUrl)?->machineName); + } +} From 0ec653df1d7ffb4e9d9d92ed9f9a29d941caa1d6 Mon Sep 17 00:00:00 2001 From: Matt Glaman Date: Fri, 28 Aug 2026 11:26:01 -0500 Subject: [PATCH 2/3] fix: honor project qualifiers and repository remotes when resolving issue forks GitLab work-item ids on migrated projects share the number space with Drupal.org node ids, so a bare id can resolve to an unrelated project and build the wrong fork path. IssueProjectResolver now prefers an explicit project qualifier, then the project/ remote of the current repository, and only then the Drupal.org node lookup, failing loudly when the node and the repository disagree. A qualifier such as campaign#3615648 no longer triggers any Drupal.org request. Closes #359 Co-Authored-By: Claude Fable 5 --- .../references/gitlab-mr-contribution.md | 11 ++ src/Api/Action/Issue/GetIssueForkAction.php | 22 ++- .../Action/Issue/SetupIssueRemoteAction.php | 15 +- src/Api/Git/ProjectRemote.php | 78 ++++++++++ src/Api/GitLab/WorkItemRef.php | 11 ++ src/Api/IssueProjectResolver.php | 86 +++++++++++ src/Cli/Command/Issue/Checkout.php | 9 +- src/Cli/Command/Issue/GetFork.php | 5 +- src/Cli/Command/Issue/IssueCommandBase.php | 8 ++ src/Cli/Command/Issue/SetupRemote.php | 3 +- .../Action/Issue/GetIssueForkActionTest.php | 32 +++++ tests/src/Git/ProjectRemoteTest.php | 71 ++++++++++ tests/src/GitLab/WorkItemRefTest.php | 56 ++++++++ tests/src/IssueProjectResolverTest.php | 133 ++++++++++++++++++ 14 files changed, 523 insertions(+), 17 deletions(-) create mode 100644 src/Api/Git/ProjectRemote.php create mode 100644 src/Api/IssueProjectResolver.php create mode 100644 tests/src/Git/ProjectRemoteTest.php create mode 100644 tests/src/GitLab/WorkItemRefTest.php create mode 100644 tests/src/IssueProjectResolverTest.php diff --git a/skill-data/drupalorg-cli/references/gitlab-mr-contribution.md b/skill-data/drupalorg-cli/references/gitlab-mr-contribution.md index b9b9eb7..9cb49c8 100644 --- a/skill-data/drupalorg-cli/references/gitlab-mr-contribution.md +++ b/skill-data/drupalorg-cli/references/gitlab-mr-contribution.md @@ -30,6 +30,17 @@ drupalorg issue:setup-remote This is idempotent: if the remote already exists it skips the `git remote add` step and always runs `git fetch` to update remote refs. +The project that owns the fork is resolved in this order: + +1. An explicit qualifier (`project#` or a work-item URL). No Drupal.org + request is made. +2. The `project/` remote of the current repository, checked against the + Drupal.org node when one exists. A mismatch is an error. +3. The Drupal.org node lookup. + +GitLab work-item ids on migrated projects can collide with unrelated Drupal.org +node ids, so pass `project#` for work items whenever you know the project. + ### 3. Check out an issue branch ```bash diff --git a/src/Api/Action/Issue/GetIssueForkAction.php b/src/Api/Action/Issue/GetIssueForkAction.php index 8079a6a..97a610d 100644 --- a/src/Api/Action/Issue/GetIssueForkAction.php +++ b/src/Api/Action/Issue/GetIssueForkAction.php @@ -5,6 +5,7 @@ use mglaman\DrupalOrg\Action\ActionInterface; use mglaman\DrupalOrg\Client; use mglaman\DrupalOrg\GitLab\Client as GitLabClient; +use mglaman\DrupalOrg\IssueProjectResolver; use mglaman\DrupalOrg\Result\Issue\IssueForkResult; class GetIssueForkAction implements ActionInterface @@ -15,12 +16,21 @@ public function __construct( ) { } - public function __invoke(string $nid, ?string $projectMachineName = null): IssueForkResult - { - if ($projectMachineName === null) { - $issue = $this->client->getNode($nid); - $projectMachineName = $issue->fieldProjectMachineName; - } + /** + * @param string|null $projectMachineName + * Project from an explicit qualifier; skips the Drupal.org lookup. + * @param string|null $repositoryProject + * Project of the git repository the command runs in. + * + * @see IssueProjectResolver for the resolution order. + */ + public function __invoke( + string $nid, + ?string $projectMachineName = null, + ?string $repositoryProject = null, + ): IssueForkResult { + $projectMachineName = (new IssueProjectResolver($this->client)) + ->resolve($nid, $projectMachineName, $repositoryProject); $remoteName = $projectMachineName . '-' . $nid; $gitLabProjectPath = 'issue/' . $remoteName; diff --git a/src/Api/Action/Issue/SetupIssueRemoteAction.php b/src/Api/Action/Issue/SetupIssueRemoteAction.php index 3321384..842890c 100644 --- a/src/Api/Action/Issue/SetupIssueRemoteAction.php +++ b/src/Api/Action/Issue/SetupIssueRemoteAction.php @@ -17,10 +17,19 @@ public function __construct( ) { } - public function __invoke(string $nid): SetupIssueRemoteResult - { + /** + * @param string|null $projectMachineName + * Project from an explicit qualifier; skips the Drupal.org lookup. + * @param string|null $repositoryProject + * Project of the git repository the remote is added to. + */ + public function __invoke( + string $nid, + ?string $projectMachineName = null, + ?string $repositoryProject = null, + ): SetupIssueRemoteResult { $getFork = new GetIssueForkAction($this->client, $this->gitLabClient); - $fork = $getFork($nid); + $fork = $getFork($nid, $projectMachineName, $repositoryProject); $remoteName = $fork->remoteName; $sshUrl = $fork->sshUrl; diff --git a/src/Api/Git/ProjectRemote.php b/src/Api/Git/ProjectRemote.php new file mode 100644 index 0000000..14afb58 --- /dev/null +++ b/src/Api/Git/ProjectRemote.php @@ -0,0 +1,78 @@ + $remoteUrls + * Fetch URLs keyed by remote name. "origin" wins when it matches. + */ + public static function machineNameFromRemotes(array $remoteUrls): ?string + { + if (isset($remoteUrls['origin'])) { + $fromOrigin = self::machineNameFromUrl($remoteUrls['origin']); + if ($fromOrigin !== null) { + return $fromOrigin; + } + } + foreach ($remoteUrls as $url) { + $machineName = self::machineNameFromUrl($url); + if ($machineName !== null) { + return $machineName; + } + } + return null; + } + + /** + * Detects the project from the remotes of the repository at $cwd. + * Returns null outside a git repository or when no remote matches. + */ + public static function detect(?string $cwd = null): ?string + { + $process = new Process(['git', 'remote', '-v'], $cwd); + $process->run(); + if (!$process->isSuccessful()) { + return null; + } + return self::machineNameFromRemotes(self::parseRemoteList($process->getOutput())); + } + + /** + * @return array + */ + private static function parseRemoteList(string $output): array + { + $remoteUrls = []; + foreach (explode("\n", $output) as $line) { + if (preg_match('/^(\S+)\s+(\S+)\s+\(fetch\)$/', trim($line), $matches) === 1) { + $remoteUrls[$matches[1]] = $matches[2]; + } + } + return $remoteUrls; + } +} diff --git a/src/Api/GitLab/WorkItemRef.php b/src/Api/GitLab/WorkItemRef.php index e03d3d6..e81ca1d 100644 --- a/src/Api/GitLab/WorkItemRef.php +++ b/src/Api/GitLab/WorkItemRef.php @@ -23,6 +23,17 @@ public function __construct( ) { } + /** + * The Drupal.org project machine name, e.g. "campaign" for "project/campaign". + */ + public function projectMachineName(): string + { + if (str_starts_with($this->projectPath, 'project/')) { + return substr($this->projectPath, strlen('project/')); + } + return basename($this->projectPath); + } + public static function tryParse(string $input): ?self { $input = trim($input); diff --git a/src/Api/IssueProjectResolver.php b/src/Api/IssueProjectResolver.php new file mode 100644 index 0000000..35142fe --- /dev/null +++ b/src/Api/IssueProjectResolver.php @@ -0,0 +1,86 @@ +resolveAgainstRepository($nid, $repositoryProject); + } + + try { + $nodeProject = $this->client->getNode($nid)->fieldProjectMachineName; + } catch (\RuntimeException $e) { + throw new \RuntimeException( + sprintf('%s %s', $e->getMessage(), self::qualifierHint($nid)), + 0, + $e + ); + } + if ($nodeProject === '') { + throw new \RuntimeException(sprintf( + 'Could not resolve a project for issue %s. %s', + $nid, + self::qualifierHint($nid) + )); + } + return $nodeProject; + } + + private function resolveAgainstRepository(string $nid, string $repositoryProject): string + { + try { + $nodeProject = $this->client->getNode($nid)->fieldProjectMachineName; + } catch (\RuntimeException) { + // Not a Drupal.org issue node (for example a migrated work item), + // so the repository is the only source for the project. + return $repositoryProject; + } + + if ($nodeProject === '' || $nodeProject === $repositoryProject) { + return $repositoryProject; + } + + throw new \RuntimeException(sprintf( + 'Issue %1$s belongs to project "%2$s" on Drupal.org, but this repository is project "%3$s". ' + . 'Pass %3$s#%1$s for the GitLab work item or %2$s#%1$s for the Drupal.org issue.', + $nid, + $nodeProject, + $repositoryProject + )); + } + + private static function qualifierHint(string $nid): string + { + return sprintf('Pass the project explicitly as project#%s.', $nid); + } +} diff --git a/src/Cli/Command/Issue/Checkout.php b/src/Cli/Command/Issue/Checkout.php index ebe342d..486ff2f 100644 --- a/src/Cli/Command/Issue/Checkout.php +++ b/src/Cli/Command/Issue/Checkout.php @@ -4,6 +4,7 @@ use mglaman\DrupalOrg\Action\Issue\GetIssueForkAction; use mglaman\DrupalOrg\Action\Issue\SetupIssueRemoteAction; +use mglaman\DrupalOrg\Git\ProjectRemote; use mglaman\DrupalOrg\GitLab\Client as GitLabClient; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; @@ -25,8 +26,10 @@ protected function configure(): void protected function execute(InputInterface $input, OutputInterface $output): int { $gitLabClient = new GitLabClient(); + $explicitProject = $this->explicitProjectMachineName(); + $repositoryProject = ProjectRemote::detect(); $action = new GetIssueForkAction($this->client, $gitLabClient); - $fork = $action($this->nid); + $fork = $action($this->nid, $explicitProject, $repositoryProject); // Verify the remote exists locally; offer to set it up if missing. $checkRemote = new Process(['git', 'remote', 'get-url', $fork->remoteName]); @@ -58,7 +61,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int } try { $setupAction = new SetupIssueRemoteAction($this->client, $gitLabClient); - $setupResult = $setupAction($this->nid); + $setupResult = $setupAction($this->nid, $explicitProject, $repositoryProject); } catch (\RuntimeException $e) { $this->stdErr->writeln( sprintf('Failed to set up remote: %s', $e->getMessage()) @@ -69,7 +72,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int sprintf('Remote %s added and fetched.', $setupResult->remoteName) ); // Refresh fork data after setup so branches are populated. - $fork = $action($this->nid); + $fork = $action($this->nid, $explicitProject, $repositoryProject); } else { // Remote already exists; fetch to ensure tracking refs are up-to-date. $fetchProcess = new Process(['git', 'fetch', $fork->remoteName]); diff --git a/src/Cli/Command/Issue/GetFork.php b/src/Cli/Command/Issue/GetFork.php index 99e022a..0c056d9 100644 --- a/src/Cli/Command/Issue/GetFork.php +++ b/src/Cli/Command/Issue/GetFork.php @@ -31,10 +31,7 @@ protected function configure(): void protected function execute(InputInterface $input, OutputInterface $output): int { $action = new GetIssueForkAction($this->client, new GitLabClient()); - $machineName = $this->workItemRef !== null - ? substr($this->workItemRef->projectPath, strlen('project/')) - : null; - $result = $action($this->nid, $machineName); + $result = $action($this->nid, $this->explicitProjectMachineName()); $format = (string) $this->stdIn->getOption('format'); if ($this->writeFormatted($result, $format)) { diff --git a/src/Cli/Command/Issue/IssueCommandBase.php b/src/Cli/Command/Issue/IssueCommandBase.php index 72f014d..4206753 100644 --- a/src/Cli/Command/Issue/IssueCommandBase.php +++ b/src/Cli/Command/Issue/IssueCommandBase.php @@ -68,6 +68,14 @@ protected function initialize( } } + /** + * The project named by the nid argument's qualifier, if one was given. + */ + protected function explicitProjectMachineName(): ?string + { + return $this->workItemRef?->projectMachineName(); + } + /** * Initializes repository for current directory. */ diff --git a/src/Cli/Command/Issue/SetupRemote.php b/src/Cli/Command/Issue/SetupRemote.php index d83879f..81e2366 100644 --- a/src/Cli/Command/Issue/SetupRemote.php +++ b/src/Cli/Command/Issue/SetupRemote.php @@ -3,6 +3,7 @@ namespace mglaman\DrupalOrgCli\Command\Issue; use mglaman\DrupalOrg\Action\Issue\SetupIssueRemoteAction; +use mglaman\DrupalOrg\Git\ProjectRemote; use mglaman\DrupalOrg\GitLab\Client as GitLabClient; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; @@ -21,7 +22,7 @@ protected function configure(): void protected function execute(InputInterface $input, OutputInterface $output): int { $action = new SetupIssueRemoteAction($this->client, new GitLabClient()); - $result = $action($this->nid); + $result = $action($this->nid, $this->explicitProjectMachineName(), ProjectRemote::detect()); if ($result->alreadyExists) { $this->stdOut->writeln( diff --git a/tests/src/Action/Issue/GetIssueForkActionTest.php b/tests/src/Action/Issue/GetIssueForkActionTest.php index f1341f0..48f300c 100644 --- a/tests/src/Action/Issue/GetIssueForkActionTest.php +++ b/tests/src/Action/Issue/GetIssueForkActionTest.php @@ -64,6 +64,38 @@ public function testForkWithBranches(): void self::assertSame(['3383637-test-issue', 'main'], $result->branches); } + public function testExplicitProjectSkipsNodeLookup(): void + { + $client = $this->createMock(Client::class); + $client->expects(self::never())->method('getNode'); + + $gitLabClient = $this->createMock(GitLabClient::class); + $gitLabClient->method('getProject')->willThrowException(new \Exception('Not Found', 404)); + + $action = new GetIssueForkAction($client, $gitLabClient); + $result = $action('3615648', 'campaign'); + + self::assertSame('campaign-3615648', $result->remoteName); + self::assertSame('git@git.drupal.org:issue/campaign-3615648.git', $result->sshUrl); + self::assertSame('issue/campaign-3615648', $result->gitLabProjectPath); + } + + public function testRepositoryProjectForWorkItem(): void + { + $client = $this->createMock(Client::class); + $client->method('getNode') + ->willThrowException(new \RuntimeException('Node 3615635 was not found on Drupal.org.')); + + $gitLabClient = $this->createMock(GitLabClient::class); + $gitLabClient->method('getProject')->willThrowException(new \Exception('Not Found', 404)); + + $action = new GetIssueForkAction($client, $gitLabClient); + $result = $action('3615635', null, 'campaign'); + + self::assertSame('campaign-3615635', $result->remoteName); + self::assertSame('issue/campaign-3615635', $result->gitLabProjectPath); + } + public function testForkNotYetCreated(): void { $client = $this->createMock(Client::class); diff --git a/tests/src/Git/ProjectRemoteTest.php b/tests/src/Git/ProjectRemoteTest.php new file mode 100644 index 0000000..f046dac --- /dev/null +++ b/tests/src/Git/ProjectRemoteTest.php @@ -0,0 +1,71 @@ + + */ + public static function urlProvider(): array + { + return [ + 'ssh' => ['git@git.drupal.org:project/campaign.git', 'campaign'], + 'https' => ['https://git.drupalcode.org/project/campaign.git', 'campaign'], + 'https without suffix' => ['https://git.drupalcode.org/project/drupal', 'drupal'], + 'ssh scheme' => ['ssh://git@git.drupal.org/project/ai_context.git', 'ai_context'], + 'issue fork' => ['git@git.drupal.org:issue/campaign-3615648.git', null], + 'personal fork' => ['git@git.drupal.org:mglaman/campaign.git', null], + 'github mirror' => ['https://github.com/drupal/drupal.git', null], + 'empty' => ['', null], + ]; + } + + public function testOriginWinsOverOtherRemotes(): void + { + $machineName = ProjectRemote::machineNameFromRemotes([ + 'upstream' => 'git@git.drupal.org:project/drupal.git', + 'origin' => 'https://git.drupalcode.org/project/campaign.git', + ]); + + self::assertSame('campaign', $machineName); + } + + public function testFallsBackToAnyProjectRemote(): void + { + $machineName = ProjectRemote::machineNameFromRemotes([ + 'origin' => 'git@github.com:mglaman/campaign.git', + 'campaign-3615648' => 'git@git.drupal.org:issue/campaign-3615648.git', + 'drupal' => 'git@git.drupal.org:project/campaign.git', + ]); + + self::assertSame('campaign', $machineName); + } + + public function testNoProjectRemote(): void + { + self::assertNull(ProjectRemote::machineNameFromRemotes([])); + self::assertNull(ProjectRemote::machineNameFromRemotes([ + 'origin' => 'git@git.drupal.org:issue/campaign-3615648.git', + ])); + } + + public function testDetectOutsideRepository(): void + { + self::assertNull(ProjectRemote::detect(sys_get_temp_dir())); + } +} diff --git a/tests/src/GitLab/WorkItemRefTest.php b/tests/src/GitLab/WorkItemRefTest.php new file mode 100644 index 0000000..3d44706 --- /dev/null +++ b/tests/src/GitLab/WorkItemRefTest.php @@ -0,0 +1,56 @@ +projectPath); + self::assertSame($expectedId, $ref->issueId); + self::assertSame($expectedMachineName, $ref->projectMachineName()); + } + + /** + * @return array + */ + public static function parseProvider(): array + { + return [ + 'shorthand' => ['campaign#3615648', 'project/campaign', 3615648, 'campaign'], + 'project path' => ['project/campaign#3615648', 'project/campaign', 3615648, 'campaign'], + 'work item URL' => [ + 'https://git.drupalcode.org/project/campaign/-/work_items/3615635', + 'project/campaign', + 3615635, + 'campaign', + ], + 'issue URL' => [ + 'https://git.drupalcode.org/project/canvas/-/issues/3591806', + 'project/canvas', + 3591806, + 'canvas', + ], + 'bare nid' => ['3615648', null, null, null], + 'empty' => ['', null, null, null], + 'unknown URL' => ['https://git.drupalcode.org/project/campaign', null, null, null], + ]; + } +} diff --git a/tests/src/IssueProjectResolverTest.php b/tests/src/IssueProjectResolverTest.php new file mode 100644 index 0000000..35a5e8b --- /dev/null +++ b/tests/src/IssueProjectResolverTest.php @@ -0,0 +1,133 @@ +createMock(Client::class); + $client->expects(self::never())->method('getNode'); + + $resolver = new IssueProjectResolver($client); + + self::assertSame('campaign', $resolver->resolve('3615648', 'campaign')); + self::assertSame('campaign', $resolver->resolve('3615648', 'campaign', 'sdx')); + } + + public function testRepositoryProjectWinsWhenNodeIsMissing(): void + { + $client = $this->createMock(Client::class); + $client->method('getNode') + ->willThrowException(new \RuntimeException('Node 3615635 was not found on Drupal.org.')); + + $resolver = new IssueProjectResolver($client); + + self::assertSame('campaign', $resolver->resolve('3615635', null, 'campaign')); + } + + public function testRepositoryProjectConfirmedByNode(): void + { + $client = $this->createMock(Client::class); + $client->method('getNode')->willReturn(self::makeIssueNode('3383637', 'campaign')); + + $resolver = new IssueProjectResolver($client); + + self::assertSame('campaign', $resolver->resolve('3383637', null, 'campaign')); + } + + public function testRepositoryProjectWinsWhenNodeHasNoProject(): void + { + $client = $this->createMock(Client::class); + $client->method('getNode')->willReturn(self::makeIssueNode('3383637', '')); + + $resolver = new IssueProjectResolver($client); + + self::assertSame('campaign', $resolver->resolve('3383637', null, 'campaign')); + } + + public function testCollisionBetweenNodeAndRepositoryFails(): void + { + $client = $this->createMock(Client::class); + $client->method('getNode')->willReturn(self::makeIssueNode('3615648', 'sdx')); + + $resolver = new IssueProjectResolver($client); + + $this->expectException(\RuntimeException::class); + $this->expectExceptionMessage( + 'Issue 3615648 belongs to project "sdx" on Drupal.org, but this repository is project "campaign". ' + . 'Pass campaign#3615648 for the GitLab work item or sdx#3615648 for the Drupal.org issue.' + ); + $resolver->resolve('3615648', null, 'campaign'); + } + + public function testBareNidUsesNodeProject(): void + { + $client = $this->createMock(Client::class); + $client->method('getNode')->with('3383637')->willReturn(self::makeIssueNode('3383637', 'drupal')); + + $resolver = new IssueProjectResolver($client); + + self::assertSame('drupal', $resolver->resolve('3383637')); + } + + public function testBareNidWithoutProjectFails(): void + { + $client = $this->createMock(Client::class); + $client->method('getNode')->willReturn(self::makeIssueNode('3591806', '')); + + $resolver = new IssueProjectResolver($client); + + $this->expectException(\RuntimeException::class); + $this->expectExceptionMessage( + 'Could not resolve a project for issue 3591806. Pass the project explicitly as project#3591806.' + ); + $resolver->resolve('3591806'); + } + + public function testBareNidForMissingNodeFailsWithHint(): void + { + $client = $this->createMock(Client::class); + $client->method('getNode') + ->willThrowException(new \RuntimeException('Node 3615635 was not found on Drupal.org.')); + + $resolver = new IssueProjectResolver($client); + + $this->expectException(\RuntimeException::class); + $this->expectExceptionMessage( + 'Node 3615635 was not found on Drupal.org. Pass the project explicitly as project#3615635.' + ); + $resolver->resolve('3615635'); + } +} From 6678a7266b24411ded6f583537de914d96012357 Mon Sep 17 00:00:00 2001 From: Matt Glaman Date: Fri, 28 Aug 2026 11:29:07 -0500 Subject: [PATCH 3/3] refactor: reuse ProjectRemote from #370 for repository project detection Drops the duplicate URL parser in src/Api/Git and moves fromRemotes() and detect() onto ProjectRemote so a single parser handles project remotes. Callers read ->machineName from the value object. Co-Authored-By: Claude Fable 5 --- src/Api/Git/ProjectRemote.php | 78 --------------------------- src/Api/ProjectRemote.php | 54 +++++++++++++++++++ src/Cli/Command/Issue/Checkout.php | 4 +- src/Cli/Command/Issue/SetupRemote.php | 4 +- tests/src/Git/ProjectRemoteTest.php | 71 ------------------------ tests/src/ProjectRemoteTest.php | 35 ++++++++++++ 6 files changed, 93 insertions(+), 153 deletions(-) delete mode 100644 src/Api/Git/ProjectRemote.php delete mode 100644 tests/src/Git/ProjectRemoteTest.php diff --git a/src/Api/Git/ProjectRemote.php b/src/Api/Git/ProjectRemote.php deleted file mode 100644 index 14afb58..0000000 --- a/src/Api/Git/ProjectRemote.php +++ /dev/null @@ -1,78 +0,0 @@ - $remoteUrls - * Fetch URLs keyed by remote name. "origin" wins when it matches. - */ - public static function machineNameFromRemotes(array $remoteUrls): ?string - { - if (isset($remoteUrls['origin'])) { - $fromOrigin = self::machineNameFromUrl($remoteUrls['origin']); - if ($fromOrigin !== null) { - return $fromOrigin; - } - } - foreach ($remoteUrls as $url) { - $machineName = self::machineNameFromUrl($url); - if ($machineName !== null) { - return $machineName; - } - } - return null; - } - - /** - * Detects the project from the remotes of the repository at $cwd. - * Returns null outside a git repository or when no remote matches. - */ - public static function detect(?string $cwd = null): ?string - { - $process = new Process(['git', 'remote', '-v'], $cwd); - $process->run(); - if (!$process->isSuccessful()) { - return null; - } - return self::machineNameFromRemotes(self::parseRemoteList($process->getOutput())); - } - - /** - * @return array - */ - private static function parseRemoteList(string $output): array - { - $remoteUrls = []; - foreach (explode("\n", $output) as $line) { - if (preg_match('/^(\S+)\s+(\S+)\s+\(fetch\)$/', trim($line), $matches) === 1) { - $remoteUrls[$matches[1]] = $matches[2]; - } - } - return $remoteUrls; - } -} diff --git a/src/Api/ProjectRemote.php b/src/Api/ProjectRemote.php index 4db9ed0..cf3b4ff 100644 --- a/src/Api/ProjectRemote.php +++ b/src/Api/ProjectRemote.php @@ -4,6 +4,8 @@ namespace mglaman\DrupalOrg; +use Symfony\Component\Process\Process; + /** * The Drupal.org project a git remote URL points at. */ @@ -27,4 +29,56 @@ public static function tryParse(string $remoteUrl): ?self } return new self($matches['name']); } + + /** + * Picks the project remote from a repository's remotes. "origin" wins + * when it matches; issue forks and personal forks never match. + * + * @param array $remoteUrls + * Fetch URLs keyed by remote name. + */ + public static function fromRemotes(array $remoteUrls): ?self + { + if (isset($remoteUrls['origin'])) { + $fromOrigin = self::tryParse($remoteUrls['origin']); + if ($fromOrigin !== null) { + return $fromOrigin; + } + } + foreach ($remoteUrls as $url) { + $remote = self::tryParse($url); + if ($remote !== null) { + return $remote; + } + } + return null; + } + + /** + * Detects the project from the remotes of the repository at $cwd. + * Returns null outside a git repository or when no remote matches. + */ + public static function detect(?string $cwd = null): ?self + { + $process = new Process(['git', 'remote', '-v'], $cwd); + $process->run(); + if (!$process->isSuccessful()) { + return null; + } + return self::fromRemotes(self::parseRemoteList($process->getOutput())); + } + + /** + * @return array + */ + private static function parseRemoteList(string $output): array + { + $remoteUrls = []; + foreach (explode("\n", $output) as $line) { + if (preg_match('/^(\S+)\s+(\S+)\s+\(fetch\)$/', trim($line), $matches) === 1) { + $remoteUrls[$matches[1]] = $matches[2]; + } + } + return $remoteUrls; + } } diff --git a/src/Cli/Command/Issue/Checkout.php b/src/Cli/Command/Issue/Checkout.php index 486ff2f..611da4e 100644 --- a/src/Cli/Command/Issue/Checkout.php +++ b/src/Cli/Command/Issue/Checkout.php @@ -4,7 +4,7 @@ use mglaman\DrupalOrg\Action\Issue\GetIssueForkAction; use mglaman\DrupalOrg\Action\Issue\SetupIssueRemoteAction; -use mglaman\DrupalOrg\Git\ProjectRemote; +use mglaman\DrupalOrg\ProjectRemote; use mglaman\DrupalOrg\GitLab\Client as GitLabClient; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; @@ -27,7 +27,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int { $gitLabClient = new GitLabClient(); $explicitProject = $this->explicitProjectMachineName(); - $repositoryProject = ProjectRemote::detect(); + $repositoryProject = ProjectRemote::detect()?->machineName; $action = new GetIssueForkAction($this->client, $gitLabClient); $fork = $action($this->nid, $explicitProject, $repositoryProject); diff --git a/src/Cli/Command/Issue/SetupRemote.php b/src/Cli/Command/Issue/SetupRemote.php index 81e2366..36a475b 100644 --- a/src/Cli/Command/Issue/SetupRemote.php +++ b/src/Cli/Command/Issue/SetupRemote.php @@ -3,7 +3,7 @@ namespace mglaman\DrupalOrgCli\Command\Issue; use mglaman\DrupalOrg\Action\Issue\SetupIssueRemoteAction; -use mglaman\DrupalOrg\Git\ProjectRemote; +use mglaman\DrupalOrg\ProjectRemote; use mglaman\DrupalOrg\GitLab\Client as GitLabClient; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; @@ -22,7 +22,7 @@ protected function configure(): void protected function execute(InputInterface $input, OutputInterface $output): int { $action = new SetupIssueRemoteAction($this->client, new GitLabClient()); - $result = $action($this->nid, $this->explicitProjectMachineName(), ProjectRemote::detect()); + $result = $action($this->nid, $this->explicitProjectMachineName(), ProjectRemote::detect()?->machineName); if ($result->alreadyExists) { $this->stdOut->writeln( diff --git a/tests/src/Git/ProjectRemoteTest.php b/tests/src/Git/ProjectRemoteTest.php deleted file mode 100644 index f046dac..0000000 --- a/tests/src/Git/ProjectRemoteTest.php +++ /dev/null @@ -1,71 +0,0 @@ - - */ - public static function urlProvider(): array - { - return [ - 'ssh' => ['git@git.drupal.org:project/campaign.git', 'campaign'], - 'https' => ['https://git.drupalcode.org/project/campaign.git', 'campaign'], - 'https without suffix' => ['https://git.drupalcode.org/project/drupal', 'drupal'], - 'ssh scheme' => ['ssh://git@git.drupal.org/project/ai_context.git', 'ai_context'], - 'issue fork' => ['git@git.drupal.org:issue/campaign-3615648.git', null], - 'personal fork' => ['git@git.drupal.org:mglaman/campaign.git', null], - 'github mirror' => ['https://github.com/drupal/drupal.git', null], - 'empty' => ['', null], - ]; - } - - public function testOriginWinsOverOtherRemotes(): void - { - $machineName = ProjectRemote::machineNameFromRemotes([ - 'upstream' => 'git@git.drupal.org:project/drupal.git', - 'origin' => 'https://git.drupalcode.org/project/campaign.git', - ]); - - self::assertSame('campaign', $machineName); - } - - public function testFallsBackToAnyProjectRemote(): void - { - $machineName = ProjectRemote::machineNameFromRemotes([ - 'origin' => 'git@github.com:mglaman/campaign.git', - 'campaign-3615648' => 'git@git.drupal.org:issue/campaign-3615648.git', - 'drupal' => 'git@git.drupal.org:project/campaign.git', - ]); - - self::assertSame('campaign', $machineName); - } - - public function testNoProjectRemote(): void - { - self::assertNull(ProjectRemote::machineNameFromRemotes([])); - self::assertNull(ProjectRemote::machineNameFromRemotes([ - 'origin' => 'git@git.drupal.org:issue/campaign-3615648.git', - ])); - } - - public function testDetectOutsideRepository(): void - { - self::assertNull(ProjectRemote::detect(sys_get_temp_dir())); - } -} diff --git a/tests/src/ProjectRemoteTest.php b/tests/src/ProjectRemoteTest.php index fd97ed2..828564c 100644 --- a/tests/src/ProjectRemoteTest.php +++ b/tests/src/ProjectRemoteTest.php @@ -24,6 +24,7 @@ public static function remoteUrlProvider(): array 'trailing newline from git output' => ["git@git.drupal.org:project/json_form_widget.git\n", 'json_form_widget'], 'github remote' => ['git@github.com:mglaman/drupalorg-cli.git', null], 'issue fork' => ['git@git.drupal.org:issue/json_form_widget-3000000.git', null], + 'personal fork' => ['git@git.drupal.org:mglaman/campaign.git', null], 'empty' => ['', null], ]; } @@ -33,4 +34,38 @@ public function testTryParse(string $remoteUrl, ?string $expected): void { self::assertSame($expected, ProjectRemote::tryParse($remoteUrl)?->machineName); } + + public function testOriginWinsOverOtherRemotes(): void + { + $remote = ProjectRemote::fromRemotes([ + 'upstream' => 'git@git.drupal.org:project/drupal.git', + 'origin' => 'https://git.drupalcode.org/project/campaign.git', + ]); + + self::assertSame('campaign', $remote?->machineName); + } + + public function testFallsBackToAnyProjectRemote(): void + { + $remote = ProjectRemote::fromRemotes([ + 'origin' => 'git@github.com:mglaman/campaign.git', + 'campaign-3615648' => 'git@git.drupal.org:issue/campaign-3615648.git', + 'drupal' => 'git@git.drupal.org:project/campaign.git', + ]); + + self::assertSame('campaign', $remote?->machineName); + } + + public function testNoProjectRemote(): void + { + self::assertNull(ProjectRemote::fromRemotes([])); + self::assertNull(ProjectRemote::fromRemotes([ + 'origin' => 'git@git.drupal.org:issue/campaign-3615648.git', + ])); + } + + public function testDetectOutsideRepository(): void + { + self::assertNull(ProjectRemote::detect(sys_get_temp_dir())); + } }