Refactor issue:link, issue:branch, issue:apply to Action/Result pattern by mglaman · Pull Request #283 · mglaman/drupalorg-cli

mglaman · 2026-02-25T17:04:47Z

Summary

Adds GetIssueLinkAction, GetIssueBranchNameAction, and GetLatestIssuePatchAction with corresponding IssueLinkResult, IssueBranchResult, and IssuePatchResult DTOs
Refactors issue:link, issue:branch, and issue:apply commands to delegate to their respective Actions, removing direct client->getNode() and IssueCommandBase helper calls (buildBranchName, getIssueVersionBranchName, getLatestFile)
Branch/title computation logic is encapsulated in IssueBranchResult::fromIssueNode() and IssuePatchResult::fromIssueNodeAndFile() static factories
Apply::applyWithGit() signature changes from (IssueNode, string) to (IssuePatchResult, string)

Test plan

vendor/bin/phpunit — 29 tests pass including 3 new *ActionTest classes
vendor/bin/phpstan analyse src — level 6 clean, no errors
Manual: php bin/drupalorg issue:link 3383637 opens browser to drupal.org/node/3383637
Manual: php bin/drupalorg issue:branch / issue:apply in a Drupal git checkout — unchanged behavior

Notes

issue:patch and issue:interdiff are intentionally deferred — they need commentCount and fieldIssueFiles access patterns not yet covered by the current Result shape.

Closes #278

🤖 Generated with Claude Code

…rn (#278) Introduces GetIssueLinkAction, GetIssueBranchNameAction, and GetLatestIssuePatchAction with corresponding IssueLinkResult, IssueBranchResult, and IssuePatchResult DTOs. Updates the three commands to delegate to their respective Actions, removing direct client and IssueCommandBase helper calls. Adds unit tests for all three Actions. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Drupal.org added a /i/{nid} redirect, which is shorter than /node/{nid}. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

Refactors the issue:link, issue:branch, and issue:apply CLI commands to use the repository’s Action/Result pattern, moving issue-derived computations into Result DTO factories and adding unit tests for the new Actions.

Changes:

Added GetIssueLinkAction, GetIssueBranchNameAction, and GetLatestIssuePatchAction plus corresponding Issue*Result DTOs.
Refactored CLI commands (issue:link, issue:branch, issue:apply) to delegate to Actions and consume Result DTOs.
Added PHPUnit coverage for the new Actions and Result JSON serialization.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
tests/src/Action/Issue/GetLatestIssuePatchActionTest.php	Adds tests for latest-patch Action behavior and JSON serialization.
tests/src/Action/Issue/GetIssueLinkActionTest.php	Adds tests for link Action output and JSON serialization.
tests/src/Action/Issue/GetIssueBranchNameActionTest.php	Adds tests for branch-name Action output and JSON serialization.
src/Cli/Command/Issue/Link.php	Switches command to use `GetIssueLinkAction` result URL.
src/Cli/Command/Issue/Branch.php	Switches branch creation logic to use `GetIssueBranchNameAction` result fields.
src/Cli/Command/Issue/Apply.php	Switches patch lookup to `GetLatestIssuePatchAction`; adjusts git apply method signature.
src/Api/Result/Issue/IssuePatchResult.php	Introduces patch result DTO with computed branch/version fields and JSON serialization.
src/Api/Result/Issue/IssueLinkResult.php	Introduces link result DTO with JSON serialization.
src/Api/Result/Issue/IssueBranchResult.php	Introduces branch result DTO with computed branch/version fields and JSON serialization.
src/Api/Action/Issue/GetLatestIssuePatchAction.php	Introduces Action to fetch issue + latest patch file and return `IssuePatchResult`.
src/Api/Action/Issue/GetIssueLinkAction.php	Introduces Action to build issue URL result.
src/Api/Action/Issue/GetIssueBranchNameAction.php	Introduces Action to fetch issue and compute branch/version result.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-25T18:04:06Z

src/Api/Result/Issue/IssueBranchResult.php

+    public static function fromIssueNode(IssueNode $issue): self
+    {
+        $cleanTitle = preg_replace('/[^a-zA-Z0-9]+/', '_', $issue->title);
+        $cleanTitle = strtolower(substr((string) $cleanTitle, 0, 20));
+        $cleanTitle = (string) preg_replace('/(^_|_$)/', '', $cleanTitle);
+
+        $branchName = sprintf('%s-%s', $issue->nid, $cleanTitle);
+
+        $issueVersionBranch = $issue->fieldIssueVersion;
+        if ($issue->fieldProjectId === '3060') {
+            $issueVersionBranch = substr($issueVersionBranch, 0, 5);
+        } elseif (preg_match('/^(\d+\.\d+)\./', $issueVersionBranch, $matches)) {
+            $issueVersionBranch = $matches[1] . '.x';
+        } else {
+            $issueVersionBranch = substr($issueVersionBranch, 0, 6) . 'x';
+        }


Branch name + issue version branch computation logic is duplicated here and in IssuePatchResult::fromIssueNodeAndFile() (and also exists in IssueCommandBase). This duplication risks the algorithms diverging over time; consider extracting a shared helper (e.g., an IssueBranchName/IssueVersionBranchName utility or a shared factory used by both Results) and reusing it in both places.

Copilot · 2026-02-25T18:04:07Z

src/Api/Action/Issue/GetLatestIssuePatchAction.php

+    private function getLatestFile(IssueNode $issue): ?File
+    {
+        $files = array_filter(
+            $issue->fieldIssueFiles,
+            static function (IssueFile $value): bool {
+                return $value->display;
+            }
+        );
+        $files = array_reverse($files);
+        $files = array_map(
+            function (IssueFile $value): File {
+                return $this->client->getFile($value->fileId);
+            },
+            $files
+        );
+        $files = array_filter(
+            $files,
+            static function (File $file): bool {
+                return str_contains($file->name, '.patch') && !str_contains($file->name, 'do-not-test');
+            }
+        );
+        return count($files) > 0 ? reset($files) : null;


getLatestFile() duplicates IssueCommandBase::getLatestFile() logic. Keeping two copies makes it easy for CLI behavior and Action behavior to drift; consider moving this selection logic into a shared helper/service and having both the Action and the remaining CLI commands call it.

Copilot · 2026-02-25T18:04:07Z

src/Api/Action/Issue/GetIssueLinkAction.php

+    public function __invoke(string $nid): IssueLinkResult
+    {
+        return new IssueLinkResult(url: 'https://www.drupal.org/i/' . $nid);
+    }


GetIssueLinkAction now builds URLs as https://www.drupal.org/i/{nid}, but the CLI previously opened https://www.drupal.org/node/{nid} and the PR description/manual test plan still references /node/. If keeping behavior identical is required, switch back to /node/ (or update the PR description + any downstream expectations to match /i/ if that redirect is intentional).

Copilot · 2026-02-25T18:04:07Z

src/Cli/Command/Issue/Apply.php

+        $patchFileContents = file_get_contents($result->patchUrl);
+        $patchFileName = $result->patchFileName;
        file_put_contents($patchFileName, $patchFileContents);


file_get_contents() can return false (network issue, 404, TLS failure). That currently results in writing an empty patch file and then attempting to apply it, and unlink() may also warn if the file wasn't created. Add explicit error handling for the download and write steps (and only unlink when the file exists / was written).

Extracts API logic from all project:* commands into channel-agnostic Action classes returning typed Result DTOs, matching the pattern established for issue:* commands in #278/#283. - Add GetProjectLinkAction, GetProjectKanbanLinkAction (pure, no Client) - Add GetProjectReleasesAction, GetProjectIssuesAction, GetProjectReleaseNotesAction (Client-injected) - Add ProjectLinkResult (shared), ProjectReleasesResult, ProjectIssuesResult, ProjectReleaseNotesResult - Replace exit(1) in ReleaseNotes with RuntimeException caught in execute() - Move type/core filter switch logic into GetProjectIssuesAction - Add 14 PHPUnit tests covering happy paths, not-found errors, semver fallback, and JSON serialisation Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* Refactor project:* commands to Action/Result pattern (#279) Extracts API logic from all project:* commands into channel-agnostic Action classes returning typed Result DTOs, matching the pattern established for issue:* commands in #278/#283. - Add GetProjectLinkAction, GetProjectKanbanLinkAction (pure, no Client) - Add GetProjectReleasesAction, GetProjectIssuesAction, GetProjectReleaseNotesAction (Client-injected) - Add ProjectLinkResult (shared), ProjectReleasesResult, ProjectIssuesResult, ProjectReleaseNotesResult - Replace exit(1) in ReleaseNotes with RuntimeException caught in execute() - Move type/core filter switch logic into GetProjectIssuesAction - Add 14 PHPUnit tests covering happy paths, not-found errors, semver fallback, and JSON serialisation Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Fix trailing period missing from RuntimeException message Restores parity with the original exit(1) message in ReleaseNotes::execute(), which read "No release found for $version." (with period). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Eliminate duplicate getProject() calls by passing Project entity to actions ProjectCommandBase::initialize() already fetches and validates the project, storing it in $this->projectData. The three project actions were re-fetching it internally via getProject(), causing a redundant HTTP request per command. Change GetProjectReleasesAction, GetProjectIssuesAction, and GetProjectReleaseNotesAction to accept a Project entity as their first argument instead of a machine name string. Remove the null guard from each action (the caller is responsible for a valid Project). Update the three corresponding commands to pass $this->projectData. Update tests to construct Project::fromStdClass() and pass it directly, removing getProject() mocks and the now-inapplicable testInvokeThrowsWhenProjectNotFound cases. Also fix CLAUDE.md: Client.php has only retry middleware (429/503), not cache middleware as previously stated. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Type ProjectIssuesResult::\$issues as IssueNode[] Map raw stdClass issue objects through IssueNode::fromStdClass() in GetProjectIssuesAction, consistent with how ProjectReleasesResult uses Release[]. Update jsonSerialize() to emit an explicit array shape and use typed camelCase property access in the ProjectIssues command. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

mglaman and others added 5 commits February 25, 2026 11:04

Merge branch 'main' into issue-278-refactor-issue-commands-action-result

c8a33c3

phpcs

c9fd78e

Use /i/{nid} short URL for issue:link

7ae06aa

Drupal.org added a /i/{nid} redirect, which is shorter than /node/{nid}. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

mark apply deprecated

9e874b6

mglaman changed the title ~~Refactor issue:link, issue:branch, issue:apply to Action/Result pattern (#278)~~ Refactor issue:link, issue:branch, issue:apply to Action/Result pattern Feb 25, 2026

remove deprecation

9130d7b

mglaman requested a review from Copilot February 25, 2026 17:59

Copilot started reviewing on behalf of mglaman February 25, 2026 18:00 View session

Copilot AI reviewed Feb 25, 2026

View reviewed changes

mglaman merged commit 9c06392 into main Feb 25, 2026
13 checks passed

mglaman deleted the issue-278-refactor-issue-commands-action-result branch February 25, 2026 18:12

mglaman mentioned this pull request Feb 25, 2026

Refactor project:* commands to Action/Result pattern #284

Merged

2 tasks

This was referenced Feb 25, 2026

Refactor issue:patch, issue:interdiff, and issue:apply to Action/Result pattern #289

Merged

Refactor Commands to Action/Result Pattern for Multi-Channel Support #270

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Refactor issue:link, issue:branch, issue:apply to Action/Result pattern#283

Refactor issue:link, issue:branch, issue:apply to Action/Result pattern#283
mglaman merged 6 commits intomainfrom
issue-278-refactor-issue-commands-action-result

mglaman commented Feb 25, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 25, 2026

Uh oh!

Copilot AI Feb 25, 2026

Uh oh!

Copilot AI Feb 25, 2026

Uh oh!

Copilot AI Feb 25, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

mglaman commented Feb 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Notes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mglaman commented Feb 25, 2026 •

edited

Loading